Re: [Intel-gfx] [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org,
	Akash Goel <akash.goel@intel.com>,
	stable@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB
Date: Fri, 22 Apr 2016 20:35:07 +0300	[thread overview]
Message-ID: <20160422173507.GW4329@intel.com> (raw)
In-Reply-To: <20160421122035.GA3581@patrik-desktop.isw.intel.com>

On Thu, Apr 21, 2016 at 02:20:35PM +0200, Patrik Jakobsson wrote:
> On Wed, Apr 20, 2016 at 04:43:56PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjï¿½lï¿½ <ville.syrjala@linux.intel.com>
> > 
> > Somehow my SNB GT1 (Dell XPS 8300) gets very unhappy around
> > GPU hangs if the RPS EI/thresholds aren't suitably aligned.
> > It seems like scheduling/timer interupts stop working somehow
> > and things get stuck eg. in usleep_range().
> > 
> > I bisected the problem down to
> > commit 8a5864377b12 ("drm/i915/skl: Restructured the gen6_set_rps_thresholds function")
> > I observed that before all the values were at least multiples of 25,
> > but afterwards they are not. And rounding things up to the next multiple
> > of 25 does seem to help, so lets' do that. I also tried roundup(..., 5)
> > but that wasn't sufficient. Also I have no idea if we might need this sort of
> > thing on gen9+ as well.
> 
> Do we need to test for performance regressions on stuff like this? And if so,
> who do we ping about this?
> 
> OTOH impact should be really small and since this fixes a real problem:
> 
> Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

Pushed to dinq. Thanks for reviews/acks.

> 
> > 
> > These are the original EI/thresholds:
> >  LOW_POWER
> >   GEN6_RP_UP_EI          12500
> >   GEN6_RP_UP_THRESHOLD   11800
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 21250
> >  BETWEEN
> >   GEN6_RP_UP_EI          10250
> >   GEN6_RP_UP_THRESHOLD    9225
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 18750
> >  HIGH_POWER
> >   GEN6_RP_UP_EI           8000
> >   GEN6_RP_UP_THRESHOLD    6800
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 15000
> > 
> > These are after 8a5864377b12:
> >  LOW_POWER
> >   GEN6_RP_UP_EI          12500
> >   GEN6_RP_UP_THRESHOLD   11875
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 21250
> >  BETWEEN
> >   GEN6_RP_UP_EI          10156
> >   GEN6_RP_UP_THRESHOLD    9140
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 18750
> >  HIGH_POWER
> >   GEN6_RP_UP_EI           7812
> >   GEN6_RP_UP_THRESHOLD    6640
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 15000
> > 
> > And these are what we have after this patch:
> >  LOW_POWER
> >   GEN6_RP_UP_EI          12500
> >   GEN6_RP_UP_THRESHOLD   11875
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 21250
> >  BETWEEN
> >   GEN6_RP_UP_EI          10175
> >   GEN6_RP_UP_THRESHOLD    9150
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 18750
> >  HIGH_POWER
> >   GEN6_RP_UP_EI           7825
> >   GEN6_RP_UP_THRESHOLD    6650
> >   GEN6_RP_DOWN_EI        25000
> >   GEN6_RP_DOWN_THRESHOLD 15000
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Akash Goel <akash.goel@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Testcase: igt/kms_pipe_crc_basic/hang-read-crc-pipe-B
> > Fixes: 8a5864377b12 ("drm/i915/skl: Restructured the gen6_set_rps_thresholds function")
> > Signed-off-by: Ville Syrjï¿½lï¿½ <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index c21b71c86a6b..08f01f4470cd 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2948,7 +2948,14 @@ enum skl_disp_power_wells {
> >  #define GEN6_RP_STATE_CAP	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
> >  #define BXT_RP_STATE_CAP        _MMIO(0x138170)
> >  
> > -#define INTERVAL_1_28_US(us)	(((us) * 100) >> 7)
> > +/*
> > + * Make these a multiple of magic 25 to avoid SNB (eg. Dell XPS
> > + * 8300) freezing up around GPU hangs. Looks as if even
> > + * scheduling/timer interrupts start misbehaving if the RPS
> > + * EI/thresholds are "bad", leading to a very sluggish or even
> > + * frozen machine.
> > + */
> > +#define INTERVAL_1_28_US(us)	roundup(((us) * 100) >> 7, 25)
> >  #define INTERVAL_1_33_US(us)	(((us) * 3)   >> 2)
> >  #define INTERVAL_0_833_US(us)	(((us) * 6) / 5)
> >  #define GT_INTERVAL_FROM_US(dev_priv, us) (IS_GEN9(dev_priv) ? \
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 

-- 
Ville Syrjï¿½lï¿½
Intel OTC