Re: [PATCH v2 06/14] drm/msm/dpu: rework core_perf debugfs overrides

[Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>]

On 7/4/2023 8:03 AM, Dmitry Baryshkov wrote:
> Currently debugfs provides separate 'modes' to override calculated
> MDP_CLK rate and interconnect bandwidth votes. Change that to allow
> overriding individual values (e.g. one can override just clock or just
> average bandwidth vote).
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>

Can you break this out into a separate series on its own?

I would like to spend more time on this and dont want to hold up the 
rest of the cleanup part of the series for this.

> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 87 ++-----------------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 10 ---
>   2 files changed, 9 insertions(+), 88 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 42c03a5f1f3e..23a29f159eff 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -17,20 +17,6 @@
>   #include "dpu_crtc.h"
>   #include "dpu_core_perf.h"
>   
> -/**
> - * enum dpu_perf_mode - performance tuning mode
> - * @DPU_PERF_MODE_NORMAL: performance controlled by user mode client
> - * @DPU_PERF_MODE_MINIMUM: performance bounded by minimum setting
> - * @DPU_PERF_MODE_FIXED: performance bounded by fixed setting
> - * @DPU_PERF_MODE_MAX: maximum value, used for error checking
> - */
> -enum dpu_perf_mode {
> -	DPU_PERF_MODE_NORMAL,
> -	DPU_PERF_MODE_MINIMUM,
> -	DPU_PERF_MODE_FIXED,
> -	DPU_PERF_MODE_MAX
> -};
> -
>   /**
>    * _dpu_core_perf_calc_bw() - to calculate BW per crtc
>    * @kms:  pointer to the dpu_kms
> @@ -229,18 +215,16 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
>   	if (!kms->num_paths)
>   		return 0;
>   
> -	if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
> -		avg_bw = 0;
> -		peak_bw = 0;
> -	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
> +	dpu_core_perf_aggregate(crtc->dev, dpu_crtc_get_client_type(crtc), &perf);
> +
> +	avg_bw = div_u64(perf.bw_ctl, 1000); /*Bps_to_icc*/
> +	peak_bw = perf.max_per_pipe_ib;
> +
> +	if (kms->perf.fix_core_ab_vote)
>   		avg_bw = kms->perf.fix_core_ab_vote;
> -		peak_bw = kms->perf.fix_core_ib_vote;
> -	} else {
> -		dpu_core_perf_aggregate(crtc->dev, dpu_crtc_get_client_type(crtc), &perf);
>   
> -		avg_bw = div_u64(perf.bw_ctl, 1000); /*Bps_to_icc*/
> -		peak_bw = perf.max_per_pipe_ib;
> -	}
> +	if (kms->perf.fix_core_ib_vote)
> +		peak_bw = kms->perf.fix_core_ib_vote;
>   
>   	avg_bw /= kms->num_paths;
>   
> @@ -294,12 +278,9 @@ static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
>   	struct drm_crtc *crtc;
>   	struct dpu_crtc_state *dpu_cstate;
>   
> -	if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED)
> +	if (kms->perf.fix_core_clk_rate)
>   		return kms->perf.fix_core_clk_rate;
>   
> -	if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM)
> -		return kms->perf.max_core_clk_rate;
> -
>   	drm_for_each_crtc(crtc, kms->dev) {
>   		if (crtc->enabled) {
>   			dpu_cstate = to_dpu_crtc_state(crtc->state);
> @@ -416,54 +397,6 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>   
>   #ifdef CONFIG_DEBUG_FS
>   
> -static ssize_t _dpu_core_perf_mode_write(struct file *file,
> -		    const char __user *user_buf, size_t count, loff_t *ppos)
> -{
> -	struct dpu_core_perf *perf = file->private_data;
> -	u32 perf_mode = 0;
> -	int ret;
> -
> -	ret = kstrtouint_from_user(user_buf, count, 0, &perf_mode);
> -	if (ret)
> -		return ret;
> -
> -	if (perf_mode >= DPU_PERF_MODE_MAX)
> -		return -EINVAL;
> -
> -	if (perf_mode == DPU_PERF_MODE_FIXED) {
> -		DRM_INFO("fix performance mode\n");
> -	} else if (perf_mode == DPU_PERF_MODE_MINIMUM) {
> -		/* run the driver with max clk and BW vote */
> -		DRM_INFO("minimum performance mode\n");
> -	} else if (perf_mode == DPU_PERF_MODE_NORMAL) {
> -		/* reset the perf tune params to 0 */
> -		DRM_INFO("normal performance mode\n");
> -	}
> -	perf->perf_tune.mode = perf_mode;
> -
> -	return count;
> -}
> -
> -static ssize_t _dpu_core_perf_mode_read(struct file *file,
> -			char __user *buff, size_t count, loff_t *ppos)
> -{
> -	struct dpu_core_perf *perf = file->private_data;
> -	int len;
> -	char buf[128];
> -
> -	len = scnprintf(buf, sizeof(buf),
> -			"mode %d\n",
> -			perf->perf_tune.mode);
> -
> -	return simple_read_from_buffer(buff, count, ppos, buf, len);
> -}
> -
> -static const struct file_operations dpu_core_perf_mode_fops = {
> -	.open = simple_open,
> -	.read = _dpu_core_perf_mode_read,
> -	.write = _dpu_core_perf_mode_write,
> -};
> -
>   int dpu_core_perf_debugfs_init(struct dpu_kms *dpu_kms, struct dentry *parent)
>   {
>   	struct dpu_core_perf *perf = &dpu_kms->perf;
> @@ -488,8 +421,6 @@ int dpu_core_perf_debugfs_init(struct dpu_kms *dpu_kms, struct dentry *parent)
>   			(u32 *)&catalog->perf->min_llcc_ib);
>   	debugfs_create_u32("min_dram_ib", 0600, entry,
>   			(u32 *)&catalog->perf->min_dram_ib);
> -	debugfs_create_file("perf_mode", 0600, entry,
> -			(u32 *)perf, &dpu_core_perf_mode_fops);
>   	debugfs_create_u64("fix_core_clk_rate", 0600, entry,
>   			&perf->fix_core_clk_rate);
>   	debugfs_create_u64("fix_core_ib_vote", 0600, entry,
> 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 c965dfbc3007..1b791b170c6b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> @@ -26,14 +26,6 @@ struct dpu_core_perf_params {
>   	u64 core_clk_rate;
>   };
>   
> -/**
> - * struct dpu_core_perf_tune - definition of performance tuning control
> - * @mode: performance mode
> - */
> -struct dpu_core_perf_tune {
> -	u32 mode;
> -};
> -
>   /**
>    * struct dpu_core_perf - definition of core performance context
>    * @dev: Pointer to drm device
> @@ -42,7 +34,6 @@ struct dpu_core_perf_tune {
>    * @core_clk: Pointer to the core clock
>    * @core_clk_rate: current core clock rate
>    * @max_core_clk_rate: maximum allowable core clock rate
> - * @perf_tune: debug control for performance tuning
>    * @enable_bw_release: debug control for bandwidth release
>    * @fix_core_clk_rate: fixed core clock request in Hz used in mode 2
>    * @fix_core_ib_vote: fixed core ib vote in bps used in mode 2
> @@ -55,7 +46,6 @@ struct dpu_core_perf {
>   	struct clk *core_clk;
>   	u64 core_clk_rate;
>   	u64 max_core_clk_rate;
> -	struct dpu_core_perf_tune perf_tune;
>   	u32 enable_bw_release;
>   	u64 fix_core_clk_rate;
>   	u64 fix_core_ib_vote;