[Intel-gfx] [PATCH 07/11] drm/i915/skl: Updated the gen9_enable_rps function

[Daniel Vetter]

On Wed, Mar 11, 2015 at 12:27:59PM -0700, Jesse Barnes wrote:
> On 03/05/2015 09:37 PM, akash.goel at intel.com wrote:
> > From: Akash Goel <akash.goel at intel.com>
> > 
> > On SKL, GT frequency is programmed in units of 16.66 MHZ units compared
> > to 50 MHZ for older platforms. Also the time value specified for Up/Down EI &
> > Up/Down thresholds are expressed in units of 1.33 us, compared to 1.28
> > us for older platforms. So updated the gen9_enable_rps function as per that.
> > 
> > v2: Updated to use new macro GT_INTERVAL_FROM_US
> > 
> > v3: Removed the initial setup of certain registers, from gen9_enable_rps,
> >     which gets overridden later from gen6_set_rps (Damien)
> > 
> > v4: Removed the enabling of rps interrupts, from gen9_enable_rps.
> >     To be done from intel_gen6_powersave_work only, as done for other
> >     platforms also.
> > 
> > Signed-off-by: Akash Goel <akash.goel at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 28 +++++++++++++---------------
> >  1 file changed, 13 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index c49950f..6273c282 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4132,23 +4132,21 @@ static void gen9_enable_rps(struct drm_device *dev)
> >  
> >  	gen6_init_rps_frequencies(dev);
> >  
> > -	I915_WRITE(GEN6_RPNSWREQ, 0xc800000);
> > -	I915_WRITE(GEN6_RC_VIDEO_FREQ, 0xc800000);
> > -
> > -	I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 0xf4240);
> > -	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, 0x12060000);
> > -	I915_WRITE(GEN6_RP_UP_THRESHOLD, 0xe808);
> > -	I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 0x3bd08);
> > -	I915_WRITE(GEN6_RP_UP_EI, 0x101d0);
> > -	I915_WRITE(GEN6_RP_DOWN_EI, 0x55730);
> > +	/* Program defaults and thresholds for RPS*/
> > +	I915_WRITE(GEN6_RC_VIDEO_FREQ,
> > +		GEN9_FREQUENCY(dev_priv->rps.rp1_freq));
> > +
> > +	/* 1 second timeout*/
> > +	I915_WRITE(GEN6_RP_DOWN_TIMEOUT,
> > +		GT_INTERVAL_FROM_US(dev_priv, 1000000));
> > +
> >  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 0xa);
> > -	I915_WRITE(GEN6_PMINTRMSK, 0x6);
> > -	I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |
> > -		   GEN6_RP_MEDIA_HW_MODE | GEN6_RP_MEDIA_IS_GFX |
> > -		   GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG |
> > -		   GEN6_RP_DOWN_IDLE_AVG);
> >  
> > -	gen6_enable_rps_interrupts(dev);
> > +	/* Leaning on the below call to gen6_set_rps to program/setup the
> > +	 * Up/Down EI & threshold registers, as well as the RP_CONTROL,
> > +	 * RP_INTERRUPT_LIMITS & RPNSWREQ registers */
> > +	dev_priv->rps.power = HIGH_POWER; /* force a reset */
> 
> Are you also sure that dev_priv->rps.cur_freq != min_freq_softlimit at
> this point?  That's the condition for calling into the threshold update
> function (maybe gen6_set_rps should check both variables though).

gen6 uses the same trick, so I hope it's safe. And indeed a bit there's a
call in both functions to gen6_init_rps_frequences which clears cur_freq
to 0. Not the clearest code though, maybe we should move that right to
above the call to gen6_set_rps. There's the added confusion that vlv/chv
works different.

Anyway that's material for a different patch, if at all.

> > +	gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> >  
> >  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> >  }
> > 
> 
> I'm assuming these match the latest SKL PM bits, but either way can be
> updated later based on tuning.
> 
> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

Merged up to this patch, still waiting for some final review on the
remaining ones.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch