Re: [PATCH 5/7] drm/i915: Take forcewake for setting the RPS thresholds

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Take forcewake for the entire duration of reprogramming the RPS
> thresholds. By itself it should not achieve much as instead of going
> into the FIFO, we force the device to wake for the reprograming, but it
> should help in regards to the next patch that introduces a read.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

The recommendation is to keep it during init. And this is
part of our init and reinit to different values, this makes
a lot of sense. This kind of approach was tried to byt
hangs and had a significant change to the repeability.

But that test didnt have rps off during reinit and the
forcewake was not as exclusive as this version.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 44 +++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a40ad32d76eb..3041cd4988a6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4869,25 +4869,31 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>  		break;
>  	}
>  
> -	I915_WRITE(GEN6_RP_UP_EI,
> -		   GT_INTERVAL_FROM_US(dev_priv, ei_up));
> -	I915_WRITE(GEN6_RP_UP_THRESHOLD,
> -		   GT_INTERVAL_FROM_US(dev_priv,
> -				       ei_up * threshold_up / 100));
> -
> -	I915_WRITE(GEN6_RP_DOWN_EI,
> -		   GT_INTERVAL_FROM_US(dev_priv, ei_down));
> -	I915_WRITE(GEN6_RP_DOWN_THRESHOLD,
> -		   GT_INTERVAL_FROM_US(dev_priv,
> -				       ei_down * threshold_down / 100));
> -
> -	I915_WRITE(GEN6_RP_CONTROL,
> -		   GEN6_RP_MEDIA_TURBO |
> -		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
> -		   GEN6_RP_MEDIA_IS_GFX |
> -		   GEN6_RP_ENABLE |
> -		   GEN6_RP_UP_BUSY_AVG |
> -		   GEN6_RP_DOWN_IDLE_AVG);
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
> +
> +	I915_WRITE_FW(GEN6_RP_UP_EI,
> +		      GT_INTERVAL_FROM_US(dev_priv, ei_up));
> +	I915_WRITE_FW(GEN6_RP_UP_THRESHOLD,
> +		      GT_INTERVAL_FROM_US(dev_priv,
> +					  ei_up * threshold_up / 100));
> +
> +	I915_WRITE_FW(GEN6_RP_DOWN_EI,
> +		      GT_INTERVAL_FROM_US(dev_priv, ei_down));
> +	I915_WRITE_FW(GEN6_RP_DOWN_THRESHOLD,
> +		      GT_INTERVAL_FROM_US(dev_priv,
> +					  ei_down * threshold_down / 100));
> +
> +	I915_WRITE_FW(GEN6_RP_CONTROL,
> +		      GEN6_RP_MEDIA_TURBO |
> +		      GEN6_RP_MEDIA_HW_NORMAL_MODE |
> +		      GEN6_RP_MEDIA_IS_GFX |
> +		      GEN6_RP_ENABLE |
> +		      GEN6_RP_UP_BUSY_AVG |
> +		      GEN6_RP_DOWN_IDLE_AVG);
> +
> +	intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
> +	spin_unlock_irq(&dev_priv->uncore.lock);
>  
>  	dev_priv->rps.power = new_power;
>  	dev_priv->rps.up_threshold = threshold_up;
> -- 
> 2.11.0