On Thu, Mar 09, 2017 at 05:24:14PM +0200, Mika Kuoppala wrote: > Chris Wilson <[email protected]> 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 > > > > Fixes: 43cf3bf084ba ("drm/i915: Improved w/a for rps on Baytrail") > > Signed-off-by: Chris Wilson <[email protected]> > > Cc: Mika Kuoppala <[email protected]> > > Cc: <[email protected]> # v4.1+ > > Signed-off-by: Chris Wilson <[email protected]> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/i915_irq.c | 28 +++++++++------------------- > > drivers/gpu/drm/i915/intel_pm.c | 5 +++-- > > 3 files changed, 13 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 9c5b3dd9f6f1..d70bbbd6a5fd 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1366,7 +1366,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 bc70e2c451b2..29b10bab38b6 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -1050,10 +1050,10 @@ static void vlv_c0_read(struct drm_i915_private > > *dev_priv, > > } > > > > 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) > > { > > + const struct intel_rps_ei *old = &dev_priv->rps.ei; > > u64 time, c0; > > unsigned int mul = 100; > > > > @@ -1079,8 +1079,7 @@ static bool vlv_c0_above(struct drm_i915_private > > *dev_priv, > > > > 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)); > > With this change you will always downclock with the first change > after reset. Is this desired? (due to busy handling the rampup?).
No, it was just noticing the skip for vlv_c0_above without thinking about the effect of ! for DOWN. > I would have made it so that vlv_c0_above returns true if > the value was reset. Then we would be on the safe side going > upwards for the first tick. Easier, just don't evaluate vlv_v0_above. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
