[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0e5dc874-0b50-4a6b-ba98-83cb01f7cce6@quicinc.com>
Date: Tue, 3 Sep 2024 18:04:13 -0700
From: Jessica Zhang <quic_jesszhan@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: Rob Clark <robdclark@...il.com>, <quic_abhinavk@...cinc.com>,
Sean Paul
<sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
"David
Airlie" <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
"Maarten
Lankhorst" <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard
<mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, <quic_ebharadw@...cinc.com>,
<linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
Rob Clark <robdclark@...omium.org>
Subject: Re: [PATCH 11/21] drm/msm/dpu: Add RM support for allocating CWB
On 8/30/2024 3:16 PM, Dmitry Baryshkov wrote:
> On Fri, 30 Aug 2024 at 22:28, Jessica Zhang <quic_jesszhan@...cinc.com> wrote:
>>
>>
>>
>> On 8/30/2024 10:18 AM, Dmitry Baryshkov wrote:
>>> On Thu, Aug 29, 2024 at 01:48:32PM GMT, Jessica Zhang wrote:
>>>> Add support for allocating the concurrent writeback mux as part of the
>>>> WB allocation
>>>>
>>>> Signed-off-by: Jessica Zhang <quic_jesszhan@...cinc.com>
>>>> ---
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 ++++-
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 30 +++++++++++++++++++++++++++--
>>>> 2 files changed, 32 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>>>> index c17d2d356f7a..c43cb55fe1d2 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>>>> @@ -1,5 +1,7 @@
>>>> /* SPDX-License-Identifier: GPL-2.0-only */
>>>> -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
>>>> +/*
>>>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>>> + * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
>>>> */
>>>>
>>>> #ifndef _DPU_HW_MDSS_H
>>>> @@ -352,6 +354,7 @@ struct dpu_mdss_color {
>>>> #define DPU_DBG_MASK_DSPP (1 << 10)
>>>> #define DPU_DBG_MASK_DSC (1 << 11)
>>>> #define DPU_DBG_MASK_CDM (1 << 12)
>>>> +#define DPU_DBG_MASK_CWB (1 << 13)
>>>>
>>>> /**
>>>> * struct dpu_hw_tear_check - Struct contains parameters to configure
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>>> index bc99b04eae3a..738e9a081b10 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>>> @@ -1,9 +1,10 @@
>>>> // SPDX-License-Identifier: GPL-2.0-only
>>>> /*
>>>> * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
>>>> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>>>> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>>> */
>>>>
>>>> +#include <drm/drm_managed.h>
>>>> #include "msm_drv.h"
>>>> #define pr_fmt(fmt) "[drm:%s] " fmt, __func__
>>>> #include "dpu_kms.h"
>>>> @@ -34,6 +35,7 @@ int dpu_rm_init(struct drm_device *dev,
>>>> void __iomem *mmio)
>>>> {
>>>> int rc, i;
>>>> + struct dpu_hw_blk_reg_map *cwb_reg_map;
>>>>
>>>> if (!rm || !cat || !mmio) {
>>>> DPU_ERROR("invalid kms\n");
>>>> @@ -100,11 +102,35 @@ int dpu_rm_init(struct drm_device *dev,
>>>> rm->hw_intf[intf->id - INTF_0] = hw;
>>>> }
>>>>
>>>> + if (cat->cwb_count > 0) {
>>>> + cwb_reg_map = drmm_kzalloc(dev,
>>>> + sizeof(*cwb_reg_map) * cat->cwb_count,
>>>> + GFP_KERNEL);
>>>
>>> Please move CWB block pointers to dpu_rm. There is no need to allocate a
>>> separate array.
>>
>> Hi Dmitry,
>>
>> Sorry, I'm not sure what you mean here. Can you clarify your comment?
>>
>> This is just allocating an array of the CWB register addresses so that
>> the hw_wb block can use it to configure the CWB mux registers.
>
> Excuse me. I asked to make the cwb_reg_map array a part of the
> existing dpu_rm structure. This way other subblocks can access it
> through dpu_rm API.
Got it, thanks for the clarification. Just wondering, is the intent here
to add CWB to rm's get_assigned_resourced?
The CWB registers will be handled by hw_wb and isn't referenced anywhere
outside of hw_wb (aside from when it's being allocated and passed into
hw_wb_init) so I'm not sure what's the benefit of adding it to the
dpu_rm struct.
>
>>
>> Thanks,
>>
>> Jessica Zhang
>>
>>>
>>>> +
>>>> + if (!cwb_reg_map) {
>>>> + DPU_ERROR("failed cwb object creation\n");
>>>> + return -ENOMEM;
>>>> + }
>>>> + }
>>>> +
>>>> +
>>>> + for (i = 0; i < cat->cwb_count; i++) {
>>>> + struct dpu_hw_blk_reg_map *cwb = &cwb_reg_map[i];
>>>> +
>>>> + cwb->blk_addr = mmio + cat->cwb[i].base;
>>>> + cwb->log_mask = DPU_DBG_MASK_CWB;
>>>> + }
>>>> +
>>>> for (i = 0; i < cat->wb_count; i++) {
>>>> struct dpu_hw_wb *hw;
>>>> const struct dpu_wb_cfg *wb = &cat->wb[i];
>>>>
>>>> - hw = dpu_hw_wb_init(dev, wb, mmio, cat->mdss_ver);
>>>> + if (cat->cwb)
>>>> + hw = dpu_hw_wb_init_with_cwb(dev, wb, mmio,
>>>> + cat->mdss_ver, cwb_reg_map);
>>>> + else
>>>> + hw = dpu_hw_wb_init(dev, wb, mmio, cat->mdss_ver);
>>>> +
>>>> if (IS_ERR(hw)) {
>>>> rc = PTR_ERR(hw);
>>>> DPU_ERROR("failed wb object creation: err %d\n", rc);
>>>>
>>>> --
>>>> 2.34.1
>>>>
>>>
>>> --
>>> With best wishes
>>> Dmitry
>
>
>
> --
> With best wishes
> Dmitry
Powered by blists - more mailing lists