diff mbox series

[RFC,4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control

Message ID 20231009-topic-sm8550-graphics-sspp-split-clk-v1-4-806c0dee4e43@linaro.org (mailing list archive)
State New, archived
Headers show
Series drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split | expand

Commit Message

Neil Armstrong Oct. 9, 2023, 4:36 p.m. UTC 
Now clk_ctrl IDs can be optional and the clk_ctrl_reg can be specified
on the SSPP & WB caps directly, pass the SSPP & WB hw struct to the
qos & limit params then call the clk_force_ctrl() op accordingly.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 37 +++++++++++++++-------
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
 4 files changed, 40 insertions(+), 22 deletions(-)

Comments

Dmitry Baryshkov Oct. 9, 2023, 5:07 p.m. UTC | #1 
On 09/10/2023 19:36, Neil Armstrong wrote:
> Now clk_ctrl IDs can be optional and the clk_ctrl_reg can be specified
> on the SSPP & WB caps directly, pass the SSPP & WB hw struct to the
> qos & limit params then call the clk_force_ctrl() op accordingly.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 37 +++++++++++++++-------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
>   4 files changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> index 78037a697633..e4dfe0be7207 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> @@ -45,6 +45,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>   	struct dpu_vbif_set_ot_params ot_params;
>   
>   	memset(&ot_params, 0, sizeof(ot_params));
> +	ot_params.wb = hw_wb;
>   	ot_params.xin_id = hw_wb->caps->xin_id;
>   	ot_params.num = hw_wb->idx - WB_0;
>   	ot_params.width = phys_enc->cached_mode.hdisplay;
> @@ -52,7 +53,6 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>   	ot_params.is_wfd = true;
>   	ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode);
>   	ot_params.vbif_idx = hw_wb->caps->vbif_idx;
> -	ot_params.clk_ctrl = hw_wb->caps->clk_ctrl;
>   	ot_params.rd = false;
>   
>   	dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
> @@ -81,9 +81,9 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>   	hw_wb = phys_enc->hw_wb;
>   
>   	memset(&qos_params, 0, sizeof(qos_params));
> +	qos_params.wb = hw_wb;
>   	qos_params.vbif_idx = hw_wb->caps->vbif_idx;
>   	qos_params.xin_id = hw_wb->caps->xin_id;
> -	qos_params.clk_ctrl = hw_wb->caps->clk_ctrl;
>   	qos_params.num = hw_wb->idx - WB_0;
>   	qos_params.is_rt = false;
>   
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index c2aaaded07ed..b0b662068377 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -350,6 +350,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>   	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
>   
>   	memset(&ot_params, 0, sizeof(ot_params));
> +	ot_params.sspp = pipe->sspp;
>   	ot_params.xin_id = pipe->sspp->cap->xin_id;
>   	ot_params.num = pipe->sspp->idx - SSPP_NONE;
>   	ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
> @@ -357,7 +358,6 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>   	ot_params.is_wfd = !pdpu->is_rt_pipe;
>   	ot_params.frame_rate = frame_rate;
>   	ot_params.vbif_idx = VBIF_RT;
> -	ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>   	ot_params.rd = true;
>   
>   	dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
> @@ -377,16 +377,15 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
>   
>   	memset(&qos_params, 0, sizeof(qos_params));
>   	qos_params.vbif_idx = VBIF_RT;
> -	qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
> +	qos_params.sspp = pipe->sspp;
>   	qos_params.xin_id = pipe->sspp->cap->xin_id;
>   	qos_params.num = pipe->sspp->idx - SSPP_VIG0;
>   	qos_params.is_rt = pdpu->is_rt_pipe;
>   
> -	DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n",
> +	DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n",
>   			qos_params.num,
>   			qos_params.vbif_idx,
> -			qos_params.xin_id, qos_params.is_rt,
> -			qos_params.clk_ctrl);
> +			qos_params.xin_id, qos_params.is_rt);
>   
>   	dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
>   }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> index 2ae5cba1848b..a79559084a91 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> @@ -158,11 +158,19 @@ static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif,
>   	return ot_lim;
>   }
>   
> -static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
> -					  unsigned int clk_ctrl,
> +static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_sspp *sspp,
> +					  struct dpu_hw_wb *wb,
> +					  struct dpu_hw_mdp *mdp,
>   					  bool enable)
>   {
> -	return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable);
> +	if (sspp && sspp->cap->clk_ctrl_reg)
> +		return sspp->ops.setup_clk_force_ctrl(sspp, enable);
> +	else if (wb && wb->caps->clk_ctrl_reg)
> +		return wb->ops.setup_clk_force_ctrl(wb, enable);
> +	else

This is what I wanted to avoid.

If we move the caller function to the sspp / WB, we will not need this 
kind of wrapper.

> +		return mdp->ops.setup_clk_force_ctrl(mdp,
> +				sspp ? sspp->cap->clk_ctrl : wb->caps->clk_ctrl,
> +				enable);
>   }
>   
>   /**
> @@ -190,9 +198,13 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>   		return;
>   	}
>   
> -	if (!mdp->ops.setup_clk_force_ctrl ||
> -			!vbif->ops.set_limit_conf ||
> -			!vbif->ops.set_halt_ctrl)
> +	if ((!params->sspp && !params->wb) ||
> +	    (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
> +	    (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
> +	    !mdp->ops.setup_clk_force_ctrl)
> +		return;
> +
> +	if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
>   		return;
>   
>   	/* set write_gather_en for all write clients */
> @@ -207,7 +219,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>   	trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim,
>   		params->vbif_idx);
>   
> -	forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
> +	forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);

I'd suggest removing the setup_clk_force_ctrl from 
dpu_vbif_set_ot_limit() and dpu_vbif_set_qos_remap(). Instead make 
dpu_plane / dpu_encoder_phys_wb call into dpu_hw_sspp / dpu_hw_wb, which 
will enable the clock, call dpu_vbif then disable the clock.

In my opinion this is simpler than the condition in the previous chunk.

>   
>   	vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
>   
> @@ -220,7 +232,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>   	vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
>   
>   	if (forced_on)
> -		dpu_vbif_setup_clk_force_ctrl(mdp,  params->clk_ctrl, false);
> +		dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
>   }
>   
>   void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
> @@ -245,7 +257,10 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>   		return;
>   	}
>   
> -	if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) {
> +	if ((!params->sspp && !params->wb) ||
> +	    (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
> +	    (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
> +	    !mdp->ops.setup_clk_force_ctrl || !vbif->ops.set_qos_remap) {
>   		DRM_DEBUG_ATOMIC("qos remap not supported\n");
>   		return;
>   	}
> @@ -258,7 +273,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>   		return;
>   	}
>   
> -	forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
> +	forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
>   
>   	for (i = 0; i < qos_tbl->npriority_lvl; i++) {
>   		DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
> @@ -269,7 +284,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>   	}
>   
>   	if (forced_on)
> -		dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
> +		dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
>   }
>   
>   void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> index ab490177d886..a4fe76e390d9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> @@ -7,7 +7,12 @@
>   
>   #include "dpu_kms.h"
>   
> +struct dpu_hw_sspp;
> +struct dpu_hw_wb;
> +
>   struct dpu_vbif_set_ot_params {
> +	struct dpu_hw_sspp *sspp;
> +	struct dpu_hw_wb *wb;
>   	u32 xin_id;
>   	u32 num;
>   	u32 width;
> @@ -16,28 +21,27 @@ struct dpu_vbif_set_ot_params {
>   	bool rd;
>   	bool is_wfd;
>   	u32 vbif_idx;
> -	u32 clk_ctrl;
>   };
>   
>   struct dpu_vbif_set_memtype_params {
>   	u32 xin_id;
>   	u32 vbif_idx;
> -	u32 clk_ctrl;
>   	bool is_cacheable;
>   };
>   
>   /**
>    * struct dpu_vbif_set_qos_params - QoS remapper parameter
> + * @sspp: backing SSPP
>    * @vbif_idx: vbif identifier
>    * @xin_id: client interface identifier
> - * @clk_ctrl: clock control identifier of the xin
>    * @num: pipe identifier (debug only)
>    * @is_rt: true if pipe is used in real-time use case
>    */
>   struct dpu_vbif_set_qos_params {
> +	struct dpu_hw_sspp *sspp;
> +	struct dpu_hw_wb *wb;
>   	u32 vbif_idx;
>   	u32 xin_id;
> -	u32 clk_ctrl;
>   	u32 num;
>   	bool is_rt;
>   };
>
Neil Armstrong Oct. 10, 2023, 7:58 a.m. UTC | #2 
On 09/10/2023 19:07, Dmitry Baryshkov wrote:
> On 09/10/2023 19:36, Neil Armstrong wrote:
>> Now clk_ctrl IDs can be optional and the clk_ctrl_reg can be specified
>> on the SSPP & WB caps directly, pass the SSPP & WB hw struct to the
>> qos & limit params then call the clk_force_ctrl() op accordingly.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 37 +++++++++++++++-------
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
>>   4 files changed, 40 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> index 78037a697633..e4dfe0be7207 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> @@ -45,6 +45,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>>       struct dpu_vbif_set_ot_params ot_params;
>>       memset(&ot_params, 0, sizeof(ot_params));
>> +    ot_params.wb = hw_wb;
>>       ot_params.xin_id = hw_wb->caps->xin_id;
>>       ot_params.num = hw_wb->idx - WB_0;
>>       ot_params.width = phys_enc->cached_mode.hdisplay;
>> @@ -52,7 +53,6 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>>       ot_params.is_wfd = true;
>>       ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode);
>>       ot_params.vbif_idx = hw_wb->caps->vbif_idx;
>> -    ot_params.clk_ctrl = hw_wb->caps->clk_ctrl;
>>       ot_params.rd = false;
>>       dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
>> @@ -81,9 +81,9 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>>       hw_wb = phys_enc->hw_wb;
>>       memset(&qos_params, 0, sizeof(qos_params));
>> +    qos_params.wb = hw_wb;
>>       qos_params.vbif_idx = hw_wb->caps->vbif_idx;
>>       qos_params.xin_id = hw_wb->caps->xin_id;
>> -    qos_params.clk_ctrl = hw_wb->caps->clk_ctrl;
>>       qos_params.num = hw_wb->idx - WB_0;
>>       qos_params.is_rt = false;
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index c2aaaded07ed..b0b662068377 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -350,6 +350,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>>       struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
>>       memset(&ot_params, 0, sizeof(ot_params));
>> +    ot_params.sspp = pipe->sspp;
>>       ot_params.xin_id = pipe->sspp->cap->xin_id;
>>       ot_params.num = pipe->sspp->idx - SSPP_NONE;
>>       ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
>> @@ -357,7 +358,6 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>>       ot_params.is_wfd = !pdpu->is_rt_pipe;
>>       ot_params.frame_rate = frame_rate;
>>       ot_params.vbif_idx = VBIF_RT;
>> -    ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>>       ot_params.rd = true;
>>       dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
>> @@ -377,16 +377,15 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
>>       memset(&qos_params, 0, sizeof(qos_params));
>>       qos_params.vbif_idx = VBIF_RT;
>> -    qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>> +    qos_params.sspp = pipe->sspp;
>>       qos_params.xin_id = pipe->sspp->cap->xin_id;
>>       qos_params.num = pipe->sspp->idx - SSPP_VIG0;
>>       qos_params.is_rt = pdpu->is_rt_pipe;
>> -    DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n",
>> +    DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n",
>>               qos_params.num,
>>               qos_params.vbif_idx,
>> -            qos_params.xin_id, qos_params.is_rt,
>> -            qos_params.clk_ctrl);
>> +            qos_params.xin_id, qos_params.is_rt);
>>       dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
>>   }
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
>> index 2ae5cba1848b..a79559084a91 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
>> @@ -158,11 +158,19 @@ static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif,
>>       return ot_lim;
>>   }
>> -static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
>> -                      unsigned int clk_ctrl,
>> +static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_sspp *sspp,
>> +                      struct dpu_hw_wb *wb,
>> +                      struct dpu_hw_mdp *mdp,
>>                         bool enable)
>>   {
>> -    return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable);
>> +    if (sspp && sspp->cap->clk_ctrl_reg)
>> +        return sspp->ops.setup_clk_force_ctrl(sspp, enable);
>> +    else if (wb && wb->caps->clk_ctrl_reg)
>> +        return wb->ops.setup_clk_force_ctrl(wb, enable);
>> +    else
> 
> This is what I wanted to avoid.
> 
> If we move the caller function to the sspp / WB, we will not need this kind of wrapper.

I tried it, but it requires passing the mdp pointer to the setup_clk_force_ctrl op,
which is IMHO not super clean... or if you have a way to get dpu_hw_mdp from
within hw_sspp/hw_wb it would help.

> 
>> +        return mdp->ops.setup_clk_force_ctrl(mdp,
>> +                sspp ? sspp->cap->clk_ctrl : wb->caps->clk_ctrl,
>> +                enable);
>>   }
>>   /**
>> @@ -190,9 +198,13 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>>           return;
>>       }
>> -    if (!mdp->ops.setup_clk_force_ctrl ||
>> -            !vbif->ops.set_limit_conf ||
>> -            !vbif->ops.set_halt_ctrl)
>> +    if ((!params->sspp && !params->wb) ||
>> +        (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
>> +        (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
>> +        !mdp->ops.setup_clk_force_ctrl)
>> +        return;
>> +
>> +    if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
>>           return;
>>       /* set write_gather_en for all write clients */
>> @@ -207,7 +219,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>>       trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim,
>>           params->vbif_idx);
>> -    forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
>> +    forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
> 
> I'd suggest removing the setup_clk_force_ctrl from dpu_vbif_set_ot_limit() and dpu_vbif_set_qos_remap(). Instead make dpu_plane / dpu_encoder_phys_wb call into dpu_hw_sspp / dpu_hw_wb, which will enable the clock, call dpu_vbif then disable the clock.
> 
> In my opinion this is simpler than the condition in the previous chunk.

Indeed this is a nice option, but the hw_mdp pointer requirement into hw_sspp/hw_wb
still puzzles me.

> 
>>       vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
>> @@ -220,7 +232,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>>       vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
>>       if (forced_on)
>> -        dpu_vbif_setup_clk_force_ctrl(mdp,  params->clk_ctrl, false);
>> +        dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
>>   }
>>   void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>> @@ -245,7 +257,10 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>>           return;
>>       }
>> -    if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) {
>> +    if ((!params->sspp && !params->wb) ||
>> +        (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
>> +        (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
>> +        !mdp->ops.setup_clk_force_ctrl || !vbif->ops.set_qos_remap) {
>>           DRM_DEBUG_ATOMIC("qos remap not supported\n");
>>           return;
>>       }
>> @@ -258,7 +273,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>>           return;
>>       }
>> -    forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
>> +    forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
>>       for (i = 0; i < qos_tbl->npriority_lvl; i++) {
>>           DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
>> @@ -269,7 +284,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>>       }
>>       if (forced_on)
>> -        dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
>> +        dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
>>   }
>>   void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms)
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
>> index ab490177d886..a4fe76e390d9 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
>> @@ -7,7 +7,12 @@
>>   #include "dpu_kms.h"
>> +struct dpu_hw_sspp;
>> +struct dpu_hw_wb;
>> +
>>   struct dpu_vbif_set_ot_params {
>> +    struct dpu_hw_sspp *sspp;
>> +    struct dpu_hw_wb *wb;
>>       u32 xin_id;
>>       u32 num;
>>       u32 width;
>> @@ -16,28 +21,27 @@ struct dpu_vbif_set_ot_params {
>>       bool rd;
>>       bool is_wfd;
>>       u32 vbif_idx;
>> -    u32 clk_ctrl;
>>   };
>>   struct dpu_vbif_set_memtype_params {
>>       u32 xin_id;
>>       u32 vbif_idx;
>> -    u32 clk_ctrl;
>>       bool is_cacheable;
>>   };
>>   /**
>>    * struct dpu_vbif_set_qos_params - QoS remapper parameter
>> + * @sspp: backing SSPP
>>    * @vbif_idx: vbif identifier
>>    * @xin_id: client interface identifier
>> - * @clk_ctrl: clock control identifier of the xin
>>    * @num: pipe identifier (debug only)
>>    * @is_rt: true if pipe is used in real-time use case
>>    */
>>   struct dpu_vbif_set_qos_params {
>> +    struct dpu_hw_sspp *sspp;
>> +    struct dpu_hw_wb *wb;
>>       u32 vbif_idx;
>>       u32 xin_id;
>> -    u32 clk_ctrl;
>>       u32 num;
>>       bool is_rt;
>>   };
>>
> 

Thanks,
Neil
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 78037a697633..e4dfe0be7207 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -45,6 +45,7 @@  static void dpu_encoder_phys_wb_set_ot_limit(
 	struct dpu_vbif_set_ot_params ot_params;
 
 	memset(&ot_params, 0, sizeof(ot_params));
+	ot_params.wb = hw_wb;
 	ot_params.xin_id = hw_wb->caps->xin_id;
 	ot_params.num = hw_wb->idx - WB_0;
 	ot_params.width = phys_enc->cached_mode.hdisplay;
@@ -52,7 +53,6 @@  static void dpu_encoder_phys_wb_set_ot_limit(
 	ot_params.is_wfd = true;
 	ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode);
 	ot_params.vbif_idx = hw_wb->caps->vbif_idx;
-	ot_params.clk_ctrl = hw_wb->caps->clk_ctrl;
 	ot_params.rd = false;
 
 	dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
@@ -81,9 +81,9 @@  static void dpu_encoder_phys_wb_set_qos_remap(
 	hw_wb = phys_enc->hw_wb;
 
 	memset(&qos_params, 0, sizeof(qos_params));
+	qos_params.wb = hw_wb;
 	qos_params.vbif_idx = hw_wb->caps->vbif_idx;
 	qos_params.xin_id = hw_wb->caps->xin_id;
-	qos_params.clk_ctrl = hw_wb->caps->clk_ctrl;
 	qos_params.num = hw_wb->idx - WB_0;
 	qos_params.is_rt = false;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c2aaaded07ed..b0b662068377 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -350,6 +350,7 @@  static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
 	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
 
 	memset(&ot_params, 0, sizeof(ot_params));
+	ot_params.sspp = pipe->sspp;
 	ot_params.xin_id = pipe->sspp->cap->xin_id;
 	ot_params.num = pipe->sspp->idx - SSPP_NONE;
 	ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
@@ -357,7 +358,6 @@  static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
 	ot_params.is_wfd = !pdpu->is_rt_pipe;
 	ot_params.frame_rate = frame_rate;
 	ot_params.vbif_idx = VBIF_RT;
-	ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
 	ot_params.rd = true;
 
 	dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
@@ -377,16 +377,15 @@  static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
 
 	memset(&qos_params, 0, sizeof(qos_params));
 	qos_params.vbif_idx = VBIF_RT;
-	qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
+	qos_params.sspp = pipe->sspp;
 	qos_params.xin_id = pipe->sspp->cap->xin_id;
 	qos_params.num = pipe->sspp->idx - SSPP_VIG0;
 	qos_params.is_rt = pdpu->is_rt_pipe;
 
-	DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n",
+	DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n",
 			qos_params.num,
 			qos_params.vbif_idx,
-			qos_params.xin_id, qos_params.is_rt,
-			qos_params.clk_ctrl);
+			qos_params.xin_id, qos_params.is_rt);
 
 	dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
index 2ae5cba1848b..a79559084a91 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
@@ -158,11 +158,19 @@  static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif,
 	return ot_lim;
 }
 
-static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
-					  unsigned int clk_ctrl,
+static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_sspp *sspp,
+					  struct dpu_hw_wb *wb,
+					  struct dpu_hw_mdp *mdp,
 					  bool enable)
 {
-	return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable);
+	if (sspp && sspp->cap->clk_ctrl_reg)
+		return sspp->ops.setup_clk_force_ctrl(sspp, enable);
+	else if (wb && wb->caps->clk_ctrl_reg)
+		return wb->ops.setup_clk_force_ctrl(wb, enable);
+	else
+		return mdp->ops.setup_clk_force_ctrl(mdp,
+				sspp ? sspp->cap->clk_ctrl : wb->caps->clk_ctrl,
+				enable);
 }
 
 /**
@@ -190,9 +198,13 @@  void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	if (!mdp->ops.setup_clk_force_ctrl ||
-			!vbif->ops.set_limit_conf ||
-			!vbif->ops.set_halt_ctrl)
+	if ((!params->sspp && !params->wb) ||
+	    (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
+	    (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
+	    !mdp->ops.setup_clk_force_ctrl)
+		return;
+
+	if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
 		return;
 
 	/* set write_gather_en for all write clients */
@@ -207,7 +219,7 @@  void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 	trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim,
 		params->vbif_idx);
 
-	forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
+	forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
 
 	vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
 
@@ -220,7 +232,7 @@  void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 	vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
 
 	if (forced_on)
-		dpu_vbif_setup_clk_force_ctrl(mdp,  params->clk_ctrl, false);
+		dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
 }
 
 void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
@@ -245,7 +257,10 @@  void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) {
+	if ((!params->sspp && !params->wb) ||
+	    (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
+	    (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
+	    !mdp->ops.setup_clk_force_ctrl || !vbif->ops.set_qos_remap) {
 		DRM_DEBUG_ATOMIC("qos remap not supported\n");
 		return;
 	}
@@ -258,7 +273,7 @@  void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
+	forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
 
 	for (i = 0; i < qos_tbl->npriority_lvl; i++) {
 		DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
@@ -269,7 +284,7 @@  void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 	}
 
 	if (forced_on)
-		dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
+		dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
 }
 
 void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
index ab490177d886..a4fe76e390d9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
@@ -7,7 +7,12 @@ 
 
 #include "dpu_kms.h"
 
+struct dpu_hw_sspp;
+struct dpu_hw_wb;
+
 struct dpu_vbif_set_ot_params {
+	struct dpu_hw_sspp *sspp;
+	struct dpu_hw_wb *wb;
 	u32 xin_id;
 	u32 num;
 	u32 width;
@@ -16,28 +21,27 @@  struct dpu_vbif_set_ot_params {
 	bool rd;
 	bool is_wfd;
 	u32 vbif_idx;
-	u32 clk_ctrl;
 };
 
 struct dpu_vbif_set_memtype_params {
 	u32 xin_id;
 	u32 vbif_idx;
-	u32 clk_ctrl;
 	bool is_cacheable;
 };
 
 /**
  * struct dpu_vbif_set_qos_params - QoS remapper parameter
+ * @sspp: backing SSPP
  * @vbif_idx: vbif identifier
  * @xin_id: client interface identifier
- * @clk_ctrl: clock control identifier of the xin
  * @num: pipe identifier (debug only)
  * @is_rt: true if pipe is used in real-time use case
  */
 struct dpu_vbif_set_qos_params {
+	struct dpu_hw_sspp *sspp;
+	struct dpu_hw_wb *wb;
 	u32 vbif_idx;
 	u32 xin_id;
-	u32 clk_ctrl;
 	u32 num;
 	bool is_rt;
 };