Re: [PATCH 4/7] drm/i915: Improved w/a for rps on Baytrail

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jul 10, 2014 at 08:31:21PM +0100, Chris Wilson wrote:
> Rewrite commit 31685c258e0b0ad6aa486c5ec001382cf8a64212
> Author: Deepak S <deepak.s@linux.intel.com>
> Date:   Thu Jul 3 17:33:01 2014 -0400
> 
>     drm/i915/vlv: WA for Turbo and RC6 to work together.
> 
> Other than code clarity, the major improvement is to disable the extra
> interrupts generated when idle.  However, the reclocking remains rather
> slow under the new manual regime, in particular it fails to downclock as
> quickly as desired.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c      | 166 ++++++++++++-----------------------
>  drivers/gpu/drm/i915/i915_reg.h      |   4 +-
>  drivers/gpu/drm/i915/intel_display.c |   2 +
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>  drivers/gpu/drm/i915/intel_pm.c      |  13 +++
>  5 files changed, 73 insertions(+), 114 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 8e19d031c05d..2db5dbb87ced 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
<snip>
> @@ -1433,14 +1376,14 @@ static void gen6_pm_rps_work(struct work_struct *work)
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
> +	pm_iir |= vlv_wa_c0_ei(dev_priv, pm_iir);
> +
>  	adj = dev_priv->rps.last_adj;
>  	if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
>  		if (adj > 0)
>  			adj *= 2;
> -		else {
> -			/* CHV needs even encode values */
> -			adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1;
> -		}
> +		else
> +			adj = 1;
>  		new_delay = dev_priv->rps.cur_freq + adj;
>  
>  		/*
> @@ -1455,15 +1398,11 @@ static void gen6_pm_rps_work(struct work_struct *work)
>  		else
>  			new_delay = dev_priv->rps.min_freq_softlimit;
>  		adj = 0;
> -	} else if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
> -		new_delay = vlv_calc_delay_from_C0_counters(dev_priv);
>  	} else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) {
>  		if (adj < 0)
>  			adj *= 2;
> -		else {
> -			/* CHV needs even encode values */
> -			adj = IS_CHERRYVIEW(dev_priv) ? -2 : -1;
> -		}
> +		else
> +			adj = -1;
>  		new_delay = dev_priv->rps.cur_freq + adj;
>  	} else { /* unknown event */
>  		new_delay = dev_priv->rps.cur_freq;
> @@ -1475,6 +1414,9 @@ static void gen6_pm_rps_work(struct work_struct *work)
>  	new_delay = clamp_t(int, new_delay,
>  			    dev_priv->rps.min_freq_softlimit,
>  			    dev_priv->rps.max_freq_softlimit);
> +	/* CHV needs even encode values */
> +	if (IS_CHERRYVIEW(dev_priv))
> +		new_delay = new_delay & ~1;

This will effectively make the first up interrupt a nop. The current
code is the way it is precisely to avoid that. I guess it's not a huge
problem but still seems silly to not satisfy the GPU when it wants moar
speed.

-- 
Ville Syrjälä
Intel OTC