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

stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB
@ 2016-04-20 13:43 ville.syrjala
  2016-04-20 13:57 ` Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: ville.syrjala @ 2016-04-20 13:43 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Akash Goel, Chris Wilson

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.

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB
  2016-04-20 13:43 [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB ville.syrjala
@ 2016-04-20 13:57 ` Chris Wilson
  2016-04-20 15:09 ` Ville Syrjälä
  2016-04-21 12:20 ` [Intel-gfx] " Patrik Jakobsson
  2 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2016-04-20 13:57 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, stable, Akash Goel

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.
> 
> 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>

Wow. The change is semantically sound, so

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB
  2016-04-20 13:43 [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB ville.syrjala
  2016-04-20 13:57 ` Chris Wilson
@ 2016-04-20 15:09 ` Ville Syrjälä
  2016-04-21 12:20 ` [Intel-gfx] " Patrik Jakobsson
  2 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2016-04-20 15:09 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Akash Goel, Chris Wilson

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>

The patch subject should probably read
"drm/i915: Make RPS EI/thresholds multiple of 25 on SNB-BDW"
since it does change more than SNB. I'll fix that up when/if I push.

> 
> 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.
> 
> 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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB
  2016-04-20 13:43 [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB ville.syrjala
  2016-04-20 13:57 ` Chris Wilson
  2016-04-20 15:09 ` Ville Syrjälä
@ 2016-04-21 12:20 ` Patrik Jakobsson
  2016-04-22 14:26   ` Ville Syrjälä
  2016-04-22 17:35   ` Ville Syrjälä
  2 siblings, 2 replies; 6+ messages in thread
From: Patrik Jakobsson @ 2016-04-21 12:20 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, Akash Goel, stable

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>

> 
> 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 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB
  2016-04-21 12:20 ` [Intel-gfx] " Patrik Jakobsson
@ 2016-04-22 14:26   ` Ville Syrjälä
  2016-04-22 17:35   ` Ville Syrjälä
  1 sibling, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2016-04-22 14:26 UTC (permalink / raw)
  To: intel-gfx, Akash Goel, stable

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?

I think Chris or the perf team might complain if things go downhill too
much.

> 
> OTOH impact should be really small and since this fixes a real problem:
> 
> Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> 
> > 
> > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB
  2016-04-21 12:20 ` [Intel-gfx] " Patrik Jakobsson
  2016-04-22 14:26   ` Ville Syrjälä
@ 2016-04-22 17:35   ` Ville Syrjälä
  1 sibling, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2016-04-22 17:35 UTC (permalink / raw)
  To: intel-gfx, Akash Goel, stable

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-04-22 17:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-20 13:43 [PATCH] drm/i915: Make RPS EI/thresholds multiple of 25 on SNB ville.syrjala
2016-04-20 13:57 ` Chris Wilson
2016-04-20 15:09 ` Ville Syrjälä
2016-04-21 12:20 ` [Intel-gfx] " Patrik Jakobsson
2016-04-22 14:26   ` Ville Syrjälä
2016-04-22 17:35   ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).