Re: [PATCH 11/21] drm/msm/dpu: Add RM support for allocating CWB

[Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>]

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@xxxxxxxxxxx>
> > ---
> >   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