Re: [Intel-gfx] [PATCH 1/4] drm/i915: Move setting of rps thresholds to init

[Rodrigo Vivi <rodrigo.vivi@kernel.org>]

On Mon, May 22, 2023 at 12:59:25PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Since 36d516be867c ("drm/i915/gt: Switch to manual evaluation of RPS")
> thresholds are invariant so lets move their setting to init time.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@kernel.org>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_rps.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index e68a99205599..791097eb9bfd 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -671,7 +671,6 @@ static void rps_set_power(struct intel_rps *rps, int new_power)
>  {
>  	struct intel_gt *gt = rps_to_gt(rps);
>  	struct intel_uncore *uncore = gt->uncore;
> -	u32 threshold_up = 0, threshold_down = 0; /* in % */
>  	u32 ei_up = 0, ei_down = 0;
>  
>  	lockdep_assert_held(&rps->power.mutex);
> @@ -679,9 +678,6 @@ static void rps_set_power(struct intel_rps *rps, int new_power)
>  	if (new_power == rps->power.mode)
>  		return;
>  
> -	threshold_up = 95;
> -	threshold_down = 85;
> -
>  	/* Note the units here are not exactly 1us, but 1280ns. */
>  	switch (new_power) {
>  	case LOW_POWER:
> @@ -708,17 +704,22 @@ static void rps_set_power(struct intel_rps *rps, int new_power)
>  
>  	GT_TRACE(gt,
>  		 "changing power mode [%d], up %d%% @ %dus, down %d%% @ %dus\n",
> -		 new_power, threshold_up, ei_up, threshold_down, ei_down);
> +		 new_power,
> +		 rps->power.up_threshold, ei_up,
> +		 rps->power.down_threshold, ei_down);
>  
>  	set(uncore, GEN6_RP_UP_EI,
>  	    intel_gt_ns_to_pm_interval(gt, ei_up * 1000));
>  	set(uncore, GEN6_RP_UP_THRESHOLD,
> -	    intel_gt_ns_to_pm_interval(gt, ei_up * threshold_up * 10));
> +	    intel_gt_ns_to_pm_interval(gt,
> +				       ei_up * rps->power.up_threshold * 10));
>  
>  	set(uncore, GEN6_RP_DOWN_EI,
>  	    intel_gt_ns_to_pm_interval(gt, ei_down * 1000));
>  	set(uncore, GEN6_RP_DOWN_THRESHOLD,
> -	    intel_gt_ns_to_pm_interval(gt, ei_down * threshold_down * 10));
> +	    intel_gt_ns_to_pm_interval(gt,
> +				       ei_down *
> +				       rps->power.down_threshold * 10));
>  
>  	set(uncore, GEN6_RP_CONTROL,
>  	    (GRAPHICS_VER(gt->i915) > 9 ? 0 : GEN6_RP_MEDIA_TURBO) |
> @@ -730,8 +731,6 @@ static void rps_set_power(struct intel_rps *rps, int new_power)
>  
>  skip_hw_write:
>  	rps->power.mode = new_power;
> -	rps->power.up_threshold = threshold_up;
> -	rps->power.down_threshold = threshold_down;
>  }
>  
>  static void gen6_rps_set_thresholds(struct intel_rps *rps, u8 val)
> @@ -1557,10 +1556,12 @@ void intel_rps_enable(struct intel_rps *rps)
>  		return;
>  
>  	GT_TRACE(rps_to_gt(rps),
> -		 "min:%x, max:%x, freq:[%d, %d]\n",
> +		 "min:%x, max:%x, freq:[%d, %d], thresholds:[%u, %u]\n",
>  		 rps->min_freq, rps->max_freq,
>  		 intel_gpu_freq(rps, rps->min_freq),
> -		 intel_gpu_freq(rps, rps->max_freq));
> +		 intel_gpu_freq(rps, rps->max_freq),
> +		 rps->power.up_threshold,
> +		 rps->power.down_threshold);
>  
>  	GEM_BUG_ON(rps->max_freq < rps->min_freq);
>  	GEM_BUG_ON(rps->idle_freq > rps->max_freq);
> @@ -2013,6 +2014,10 @@ void intel_rps_init(struct intel_rps *rps)
>  		}
>  	}
>  
> +	/* Set default thresholds in % */
> +	rps->power.up_threshold = 95;
> +	rps->power.down_threshold = 85;
> +
>  	/* Finally allow us to boost to max by default */
>  	rps->boost_freq = rps->max_freq;
>  	rps->idle_freq = rps->min_freq;
> -- 
> 2.39.2
>