[Intel-gfx] [PATCH v3 1/3] drm/i915: Stop using RP_DOWN_EI on Baytrail

[Chris Wilson]

On Fri, Mar 10, 2017 at 04:10:29PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > On Baytrail, we manually calculate busyness over the evaluation interval
> > to avoid issues with miscaluations with RC6 enabled. However, it turns
> > out that the DOWN_EI interrupt generator is completely bust - it
> > operates in two modes, continuous or never. Neither of which are
> > conducive to good behaviour. Stop unmask the DOWN_EI interrupt and just
> > compute everything from the UP_EI which does seem to correspond to the
> > desired interval.
> >
> > v2: Fixup gen6_rps_pm_mask() as well
> > v3: Inline vlv_c0_above() to combine the now identical elapsed
> > calculation for up/down and simplify the threshold testing
> >
> > Fixes: 43cf3bf084ba ("drm/i915: Improved w/a for rps on Baytrail")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Cc: <stable at vger.kernel.org> # v4.1+
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h |  2 +-
> >  drivers/gpu/drm/i915/i915_irq.c | 73 ++++++++++++++++-------------------------
> >  drivers/gpu/drm/i915/intel_pm.c |  5 +--
> >  3 files changed, 32 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index c52aee5141ca..b399c8fe5ac1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1377,7 +1377,7 @@ struct intel_gen6_power_mgmt {
> >  	unsigned boosts;
> >  
> >  	/* manual wa residency calculations */
> > -	struct intel_rps_ei up_ei, down_ei;
> > +	struct intel_rps_ei ei;
> >  
> >  	/*
> >  	 * Protects RPS/RC6 register access and PCU communication.
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index d91078f8e9d4..b3f9181ef314 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1083,68 +1083,51 @@ static void vlv_c0_read(struct drm_i915_private *dev_priv,
> >  	ei->media_c0 = I915_READ(VLV_MEDIA_C0_COUNT);
> >  }
> >  
> > -static bool vlv_c0_above(struct drm_i915_private *dev_priv,
> > -			 const struct intel_rps_ei *old,
> > -			 const struct intel_rps_ei *now,
> > -			 int threshold)
> > -{
> > -	u64 time, c0;
> > -	unsigned int mul = 100;
> > -
> > -	if (old->cz_clock == 0)
> > -		return false;
> > -
> > -	if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
> > -		mul <<= 8;
> > -
> > -	time = now->cz_clock - old->cz_clock;
> > -	time *= threshold * dev_priv->czclk_freq;
> > -
> > -	/* Workload can be split between render + media, e.g. SwapBuffers
> > -	 * being blitted in X after being rendered in mesa. To account for
> > -	 * this we need to combine both engines into our activity counter.
> > -	 */
> > -	c0 = now->render_c0 - old->render_c0;
> > -	c0 += now->media_c0 - old->media_c0;
> > -	c0 *= mul * VLV_CZ_CLOCK_TO_MILLI_SEC;
> > -
> > -	return c0 >= time;
> > -}
> > -
> >  void gen6_rps_reset_ei(struct drm_i915_private *dev_priv)
> >  {
> > -	vlv_c0_read(dev_priv, &dev_priv->rps.down_ei);
> > -	dev_priv->rps.up_ei = dev_priv->rps.down_ei;
> > +	memset(&dev_priv->rps.ei, 0, sizeof(dev_priv->rps.ei));
> >  }
> >  
> >  static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir)
> >  {
> > +	const struct intel_rps_ei *prev = &dev_priv->rps.ei;
> >  	struct intel_rps_ei now;
> >  	u32 events = 0;
> >  
> > -	if ((pm_iir & (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED)) == 0)
> > +	if ((pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) == 0)
> >  		return 0;
> >  
> >  	vlv_c0_read(dev_priv, &now);
> >  	if (now.cz_clock == 0)
> >  		return 0;
> >  
> > -	if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) {
> > -		if (!vlv_c0_above(dev_priv,
> > -				  &dev_priv->rps.down_ei, &now,
> > -				  dev_priv->rps.down_threshold))
> > -			events |= GEN6_PM_RP_DOWN_THRESHOLD;
> > -		dev_priv->rps.down_ei = now;
> > -	}
> > +	if (prev->cz_clock) {
> > +		u64 time, c0;
> > +		unsigned int mul;
> >  
> > -	if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
> > -		if (vlv_c0_above(dev_priv,
> > -				 &dev_priv->rps.up_ei, &now,
> > -				 dev_priv->rps.up_threshold))
> > -			events |= GEN6_PM_RP_UP_THRESHOLD;
> > -		dev_priv->rps.up_ei = now;
> > +		mul = VLV_CZ_CLOCK_TO_MILLI_SEC * 100; /* scale to % */
> 
> scale to % as our thresholds are in %. But tiny bikeshed, so

You can have /* scale to % threshold */ but no more!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre