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

[Chris Wilson]

On Mon, Sep 01, 2014 at 11:23:20AM +0300, Ville Syrjälä wrote:
> On Thu, Jul 10, 2014 at 08:31:21PM +0100, Chris Wilson wrote:
> > Rewrite commit 31685c258e0b0ad6aa486c5ec001382cf8a64212
> > Author: Deepak S <deepak.s at 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 at chris-wilson.co.uk>
> > Cc: Deepak S <deepak.s at linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Cc: Daniel Vetter <daniel.vetter at 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.

Hmm, it's actually worse than that (see the last_adj). Ok, I should
split this out and tidy it up more carefully.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre