On Fri, Mar 10, 2017 at 04:10:29PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx> # v4.1+
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > 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