| Message ID | 20240829-concurrent-wb-v1-11-502b16ae2ebb@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm/dpu: Add Concurrent Writeback Support for DPU 10.x+ | expand |
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@quicinc.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. 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
On Fri, 30 Aug 2024 at 22:28, Jessica Zhang <quic_jesszhan@quicinc.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@quicinc.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. > > 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
On 9/5/2024 6:30 AM, Dmitry Baryshkov wrote: > On Tue, Sep 03, 2024 at 06:04:13PM GMT, Jessica Zhang wrote: >> >> >> On 8/30/2024 3:16 PM, Dmitry Baryshkov wrote: >>> On Fri, 30 Aug 2024 at 22:28, Jessica Zhang <quic_jesszhan@quicinc.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@quicinc.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. > > To have a single point where all the blocks are handled, pretty much > like we have a single catalog where all blocks are allocated. Note how > e.g. how MERGE_3D is handled. Or how we return harware instances for > INTF or WB. Got it, seems like you're leaning towards having CWB as a completely independent hardware block with its own dpu_hw_cwb file and struct. FWIW, we did consider this approach at the very beginning, but decided to go with having the CWB registers configured by dpu_hw_wb under the hood because we thought it would be overkill to create a completely new struct just to program 2 registers via 1 function op. We ended up adding the CWB mux programming to dpu_hw_wb because CWB is closely tied with WB and it mirrored how downstream code was programming CWB mux [1] If you prefer to have CWB mux completely independent, I can switch to that instead. [1] https://android.googlesource.com/kernel/msm-extra/display-drivers/+/e18d8e759a344ad4d86b31bbf8160cfe4c65b772/msm/sde/sde_hw_wb.c#265 > >> >>> >>>> >>>> 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 > > -- > With best wishes > Dmitry
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); + + 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);
Add support for allocating the concurrent writeback mux as part of the WB allocation Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.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(-)