Re: [Intel-gfx] [PATCH v12 6/7] drm/i915/mtl: find the best QGV point for the SAGV configuration

[Imre Deak <imre.deak@intel.com>]

On Thu, Jun 01, 2023 at 07:03:49PM +0300, Vinod Govindapillai wrote:
> From MTL onwards, we need to find the best QGV point based on
> the required data rate and pass the peak BW of that point to
> the punit to lock the corresponding QGV point.
> 
> v1: Fix for warning from kernel test robot
> 
> Bspec: 64636
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/r/202305280253.Ab8bRV2w-lkp@intel.com/
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/r/202305280253.Ab8bRV2w-lkp@intel.com/
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 87 ++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_bw.h |  6 ++
>  2 files changed, 91 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index b1cbeda0b2e3..7672963dc49c 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -803,6 +803,85 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state)
>  	return to_intel_bw_state(bw_state);
>  }
>  
> +static int mtl_find_qgv_points(struct drm_i915_private *i915,
> +			       unsigned int data_rate,
> +			       unsigned int num_active_planes,
> +			       const struct intel_bw_state *old_bw_state,
> +			       struct intel_bw_state *new_bw_state)
> +{
> +	unsigned int best_rate = UINT_MAX;
> +	unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
> +	unsigned int qgv_peak_bw  = 0;
> +	int i;
> +	int ret;
> +
> +	ret = intel_atomic_lock_global_state(&new_bw_state->base);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * If SAGV cannot be enabled, disable the pcode SAGV by passing all 1's
> +	 * for qgv peak bw in PM Demand request. So assign UINT_MAX if SAGV is
> +	 * not enabled. PM Demand code will clamp the value for the register
> +	 */
> +	if (!intel_can_enable_sagv(i915, new_bw_state)) {
> +		new_bw_state->qgv_point_peakbw = UINT_MAX;
> +		drm_dbg_kms(&i915->drm, "No SAGV, use UINT_MAX as peak bw.");
> +		goto out;
> +	}
> +
> +	/*
> +	 * Find the best QGV point by comparing the data_rate with max data rate
> +	 * offered per plane group
> +	 */
> +	for (i = 0; i < num_qgv_points; i++) {
> +		unsigned int bw_index =
> +			tgl_max_bw_index(i915, num_active_planes, i);
> +		unsigned int max_data_rate;
> +
> +		if (bw_index >= ARRAY_SIZE(i915->display.bw.max))
> +			continue;
> +
> +		max_data_rate = i915->display.bw.max[bw_index].deratedbw[i];
> +
> +		if (max_data_rate < data_rate)
> +			continue;
> +
> +		if (max_data_rate - data_rate < best_rate) {
> +			best_rate = max_data_rate - data_rate;
> +			qgv_peak_bw = i915->display.bw.max[bw_index].peakbw[i];
> +		}
> +
> +		drm_dbg_kms(&i915->drm, "QGV point %d: max bw %d required %d qgv_peak_bw: %d\n",
> +			    i, max_data_rate, data_rate, qgv_peak_bw);
> +	}
> +
> +	drm_dbg_kms(&i915->drm, "Matching peaks QGV bw: %d for required data rate: %d\n",
> +		    qgv_peak_bw, data_rate);
> +
> +	/*
> +	 * The display configuration cannot be supported if no QGV point
> +	 * satisfying the required data rate is found
> +	 */
> +	if (qgv_peak_bw == 0) {
> +		drm_dbg_kms(&i915->drm, "No QGV points for bw %d for display configuration(%d active planes).\n",
> +			    data_rate, num_active_planes);
> +		return -EINVAL;
> +	}
> +
> +	/* MTL PM DEMAND expects QGV BW parameter in multiples of 100 mbps */
> +	new_bw_state->qgv_point_peakbw = DIV_ROUND_CLOSEST(qgv_peak_bw, 100);
> +
> +out:
> +	if (new_bw_state->qgv_point_peakbw != old_bw_state->qgv_point_peakbw)  {
> +		ret = intel_atomic_serialize_global_state(&new_bw_state->base);

qgv_point_beakbw is used as a parameter for the pmdemand command, for
which there is a check later whether programming it is required or not.
So why is global state serialized here?

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int icl_find_qgv_points(struct drm_i915_private *i915,
>  			       unsigned int data_rate,
>  			       unsigned int num_active_planes,
> @@ -928,8 +1007,12 @@ static int intel_bw_check_qgv_points(struct drm_i915_private *i915,
>  
>  	data_rate = DIV_ROUND_UP(data_rate, 1000);
>  
> -	return icl_find_qgv_points(i915, data_rate, num_active_planes,
> -				   old_bw_state, new_bw_state);
> +	if (DISPLAY_VER(i915) >= 14)
> +		return mtl_find_qgv_points(i915, data_rate, num_active_planes,
> +					   old_bw_state, new_bw_state);
> +	else
> +		return icl_find_qgv_points(i915, data_rate, num_active_planes,
> +					   old_bw_state, new_bw_state);
>  }
>  
>  static bool intel_bw_state_changed(struct drm_i915_private *i915,
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
> index f20292143745..67ae66a3fcdd 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -34,6 +34,12 @@ struct intel_bw_state {
>  	/* bitmask of active pipes */
>  	u8 active_pipes;
>  
> +	/*
> +	 * From MTL onwards, to lock a QGV point, punit expects the peak BW of
> +	 * the selected QGV point as the parameter in multiples of 100MB/s
> +	 */
> +	unsigned int qgv_point_peakbw;
> +
>  	/*
>  	 * Current QGV points mask, which restricts
>  	 * some particular SAGV states, not to confuse
> -- 
> 2.34.1
>