diff mbox series

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

Message ID 20240829-concurrent-wb-v1-11-502b16ae2ebb@quicinc.com (mailing list archive)
State New, archived
Headers show
Series drm/msm/dpu: Add Concurrent Writeback Support for DPU 10.x+ | expand

Commit Message

Jessica Zhang Aug. 29, 2024, 8:48 p.m. UTC
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(-)

Comments

Dmitry Baryshkov Aug. 30, 2024, 5:18 p.m. UTC | #1
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.

> +
> +		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
>
Jessica Zhang Aug. 30, 2024, 7:28 p.m. UTC | #2
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
Dmitry Baryshkov Aug. 30, 2024, 10:16 p.m. UTC | #3
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
Jessica Zhang Sept. 4, 2024, 1:04 a.m. UTC | #4
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.

> 
>>
>> 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
Dmitry Baryshkov Sept. 5, 2024, 1:30 p.m. UTC | #5
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. 

> 
> > 
> > > 
> > > 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
Jessica Zhang Sept. 6, 2024, 4:53 p.m. UTC | #6
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
Dmitry Baryshkov Sept. 6, 2024, 5:58 p.m. UTC | #7
On Fri, 6 Sept 2024 at 19:53, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>
>
>
> 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.

Well, I'd suggest to check a separate single-function interface
approach. The reason is pretty simple: for DPU 3.x/4.x we'd need a
completely different programming approach. And if at some point we
will consider going for an sblk / non-sblk implementation for DPU 8.x+
/ 5.x-7.x we'd also need to change that. Thus I think it's better to
have less ties between hw_wb and hw_cwb.

>
> [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 mbox series

Patch

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);