Re: [PATCH v4 4/9] drm/msm/dpu: make fix_core_ab_vote consistent with fix_core_ib_vote

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

On 1/5/2025 7:07 PM, Dmitry Baryshkov wrote:
> The fix_core_ab_vote is an average bandwidth value, used for bandwidth
> overrides in several cases. However there is an internal inconsistency:
> fix_core_ib_vote is defined in KBps, while fix_core_ab_vote is defined
> in Bps.
> 
> Fix that by changing the type of the variable to u32 and using * 1000ULL
> multiplier when setting up the dpu_core_perf_params::bw_ctl value.
> 

Actually after looking at this, I have another question.

How did you conclude that fix_core_ib_vote is in KBps?

min_dram_ib is in KBps in the catalog but how is fix_core_ib_vote?

It depends on the interpretation perhaps. If the debugfs was supposed to 
operate under the expectation that the provided value will be 
pre-multiplied by 1000 and given then that explains why it was not 
multiplied again.

And I cross-checked some of the internal usages of the debugfs, the 
values provided to it were in Bps and not KBps.

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 4 ++--
>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 7263ab63a692554cd51a7fd91bd6250330179240..7cabc8f26908cfd2dbbffebd7c70fc37d9159733 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -125,7 +125,7 @@ static void _dpu_core_perf_calc_crtc(const struct dpu_core_perf *core_perf,
>   		perf->max_per_pipe_ib = 0;
>   		perf->core_clk_rate = 0;
>   	} else if (core_perf->perf_tune.mode == DPU_PERF_MODE_FIXED) {
> -		perf->bw_ctl = core_perf->fix_core_ab_vote;
> +		perf->bw_ctl = core_perf->fix_core_ab_vote * 1000ULL;
>   		perf->max_per_pipe_ib = core_perf->fix_core_ib_vote;
>   		perf->core_clk_rate = core_perf->fix_core_clk_rate;
>   	} else {
> @@ -479,7 +479,7 @@ int dpu_core_perf_debugfs_init(struct dpu_kms *dpu_kms, struct dentry *parent)
>   			&perf->fix_core_clk_rate);
>   	debugfs_create_u32("fix_core_ib_vote", 0600, entry,
>   			&perf->fix_core_ib_vote);
> -	debugfs_create_u64("fix_core_ab_vote", 0600, entry,
> +	debugfs_create_u32("fix_core_ab_vote", 0600, entry,
>   			&perf->fix_core_ab_vote);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> index ca4595b4ec217697849af02446b23ed0857a0295..5e07119c14c6a9ed3413d0eaddbd93df5cc3f79d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> @@ -51,7 +51,7 @@ struct dpu_core_perf {
>   	u32 enable_bw_release;
>   	u64 fix_core_clk_rate;
>   	u32 fix_core_ib_vote;
> -	u64 fix_core_ab_vote;
> +	u32 fix_core_ab_vote;
>   };
>   
>   int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>