public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
@ 2016-07-13 13:32 ville.syrjala
  2016-07-13 13:37 ` David Weinehall
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: ville.syrjala @ 2016-07-13 13:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: David Weinehall

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bspec tells us to keep bashing the PCU for up to 3ms when trying to
inform it about an upcoming change in the cdclk frequency. Currently
we only keep at it for 15*10usec (+ whatever delays gets added by
the sandybridge_pcode_read() itself). Let's change the limit to 3ms.

I decided to keep 10 usec delay per iteration for now, even though
the spec doesn't really tell us to do that.

Cc: David Weinehall <david.weinehall@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index be3b2cab2640..90f26f0e2571 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5691,15 +5691,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
 
 static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
 {
-	unsigned int i;
-
-	for (i = 0; i < 15; i++) {
-		if (skl_cdclk_pcu_ready(dev_priv))
-			return true;
-		udelay(10);
-	}
-
-	return false;
+	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
 }
 
 static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk, int vco)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
  2016-07-13 13:32 [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL ville.syrjala
@ 2016-07-13 13:37 ` David Weinehall
  2016-07-13 14:07 ` ✗ Ro.CI.BAT: failure for " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: David Weinehall @ 2016-07-13 13:37 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, David Weinehall

On Wed, Jul 13, 2016 at 04:32:03PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bspec tells us to keep bashing the PCU for up to 3ms when trying to
> inform it about an upcoming change in the cdclk frequency. Currently
> we only keep at it for 15*10usec (+ whatever delays gets added by
> the sandybridge_pcode_read() itself). Let's change the limit to 3ms.
> 
> I decided to keep 10 usec delay per iteration for now, even though
> the spec doesn't really tell us to do that.
> 
> Cc: David Weinehall <david.weinehall@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I've been using this patch for two weeks now on both of my Skylake
machines without any problems. Without this patch I've managed to
trigger:

"[drm:skl_set_cdclk [i915]] *ERROR* failed to inform PCU about cdclk
change"


Tested-by: David Weinehall <david.weinehall@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index be3b2cab2640..90f26f0e2571 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5691,15 +5691,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
>  
>  static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
>  {
> -	unsigned int i;
> -
> -	for (i = 0; i < 15; i++) {
> -		if (skl_cdclk_pcu_ready(dev_priv))
> -			return true;
> -		udelay(10);
> -	}
> -
> -	return false;
> +	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
>  }
>  
>  static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk, int vco)
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Ro.CI.BAT: failure for drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
  2016-07-13 13:32 [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL ville.syrjala
  2016-07-13 13:37 ` David Weinehall
@ 2016-07-13 14:07 ` Patchwork
  2016-07-18 12:57   ` Ville Syrjälä
  2016-07-13 18:42 ` [PATCH] " Chris Wilson
  2016-07-14 14:39 ` Daniel Vetter
  3 siblings, 1 reply; 7+ messages in thread
From: Patchwork @ 2016-07-13 14:07 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
URL   : https://patchwork.freedesktop.org/series/9815/
State : failure

== Summary ==

Series 9815v1 drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
http://patchwork.freedesktop.org/api/1.0/series/9815/revisions/1/mbox

Test drv_hangman:
        Subgroup error-state-basic:
                dmesg-warn -> PASS       (ro-bdw-i7-5600u)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (ro-bdw-i7-5557U)
Test gem_sync:
        Subgroup basic-store-each:
                pass       -> DMESG-FAIL (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (ro-skl3-i5-6260u)

fi-kbl-qkkr      total:237  pass:174  dwarn:27  dfail:2   fail:7   skip:27 
fi-skl-i5-6260u  total:237  pass:189  dwarn:27  dfail:2   fail:7   skip:12 
fi-skl-i7-6700k  total:237  pass:200  dwarn:2   dfail:0   fail:9   skip:26 
fi-snb-i7-2600   total:237  pass:188  dwarn:0   dfail:0   fail:9   skip:40 
ro-bdw-i5-5250u  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16 
ro-bdw-i7-5557U  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16 
ro-bdw-i7-5600u  total:237  pass:195  dwarn:1   dfail:1   fail:9   skip:31 
ro-bsw-n3050     total:217  pass:170  dwarn:0   dfail:0   fail:4   skip:42 
ro-byt-n2820     total:237  pass:177  dwarn:0   dfail:0   fail:7   skip:38 
ro-hsw-i3-4010u  total:237  pass:199  dwarn:5   dfail:2   fail:7   skip:24 
ro-hsw-i7-4770r  total:237  pass:204  dwarn:0   dfail:0   fail:9   skip:24 
ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63 
ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58 
ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33 
ro-skl3-i5-6260u total:237  pass:213  dwarn:3   dfail:0   fail:9   skip:12 
ro-snb-i7-2620M  total:237  pass:187  dwarn:0   dfail:0   fail:9   skip:41 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1484/

e8b1f0b drm-intel-nightly: 2016y-07m-13d-10h-57m-19s UTC integration manifest
6179e9d drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
  2016-07-13 13:32 [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL ville.syrjala
  2016-07-13 13:37 ` David Weinehall
  2016-07-13 14:07 ` ✗ Ro.CI.BAT: failure for " Patchwork
@ 2016-07-13 18:42 ` Chris Wilson
  2016-07-14 14:39 ` Daniel Vetter
  3 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2016-07-13 18:42 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, David Weinehall

On Wed, Jul 13, 2016 at 04:32:03PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bspec tells us to keep bashing the PCU for up to 3ms when trying to
> inform it about an upcoming change in the cdclk frequency. Currently
> we only keep at it for 15*10usec (+ whatever delays gets added by
> the sandybridge_pcode_read() itself). Let's change the limit to 3ms.
> 
> I decided to keep 10 usec delay per iteration for now, even though
> the spec doesn't really tell us to do that.
> 
> Cc: David Weinehall <david.weinehall@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Code does what it says on the tin,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Just slightly sadden to see a new _wait_for(), but with the 10us
interval this can be replaced with wait_for_us(..., 3000);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
  2016-07-13 13:32 [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL ville.syrjala
                   ` (2 preceding siblings ...)
  2016-07-13 18:42 ` [PATCH] " Chris Wilson
@ 2016-07-14 14:39 ` Daniel Vetter
  2016-07-27 13:32   ` Ville Syrjälä
  3 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2016-07-14 14:39 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, David Weinehall

On Wed, Jul 13, 2016 at 04:32:03PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bspec tells us to keep bashing the PCU for up to 3ms when trying to
> inform it about an upcoming change in the cdclk frequency. Currently
> we only keep at it for 15*10usec (+ whatever delays gets added by
> the sandybridge_pcode_read() itself). Let's change the limit to 3ms.
> 
> I decided to keep 10 usec delay per iteration for now, even though
> the spec doesn't really tell us to do that.
> 
> Cc: David Weinehall <david.weinehall@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Needs

Cc: stable@vger.kernel.org

and probably also a Fixes: line?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index be3b2cab2640..90f26f0e2571 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5691,15 +5691,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
>  
>  static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
>  {
> -	unsigned int i;
> -
> -	for (i = 0; i < 15; i++) {
> -		if (skl_cdclk_pcu_ready(dev_priv))
> -			return true;
> -		udelay(10);
> -	}
> -
> -	return false;
> +	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
>  }
>  
>  static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk, int vco)
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: ✗ Ro.CI.BAT: failure for drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
  2016-07-13 14:07 ` ✗ Ro.CI.BAT: failure for " Patchwork
@ 2016-07-18 12:57   ` Ville Syrjälä
  0 siblings, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2016-07-18 12:57 UTC (permalink / raw)
  To: intel-gfx

On Wed, Jul 13, 2016 at 02:07:41PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
> URL   : https://patchwork.freedesktop.org/series/9815/
> State : failure
> 
> == Summary ==
> 
> Series 9815v1 drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
> http://patchwork.freedesktop.org/api/1.0/series/9815/revisions/1/mbox
> 
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 dmesg-warn -> PASS       (ro-bdw-i7-5600u)
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 pass       -> DMESG-WARN (ro-bdw-i7-5557U)

[  321.992179] [drm:intel_dp_link_training_clock_recovery [i915]] *ERROR* failed to enable link training
[  322.075896] [drm:intel_dp_start_link_train [i915]] *ERROR* failed to start channel equalization
[  324.007603] [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun

https://bugs.freedesktop.org/show_bug.cgi?id=96614

> Test gem_sync:
>         Subgroup basic-store-each:
>                 pass       -> DMESG-FAIL (ro-bdw-i7-5600u)

(gem_sync:5897) CRITICAL: Test assertion failure function store_ring, file gem_sync.c:281:
(gem_sync:5897) CRITICAL: Failed assertion: intel_detect_and_clear_missed_interrupts(fd) == 0
(gem_sync:5897) CRITICAL: error: 18 != 0
Subtest basic-store-each failed.
**** DEBUG ****
(gem_sync:5897) CRITICAL: Test assertion failure function store_ring, file gem_sync.c:281:
(gem_sync:5897) CRITICAL: Failed assertion: intel_detect_and_clear_missed_interrupts(fd) == 0
(gem_sync:5897) CRITICAL: error: 18 != 0
****  END  ****

[   84.728418] [drm:i915_hangcheck_elapsed [i915]] *ERROR* Hangcheck timer elapsed... video enhancement ring idle

Filed a new bug:
https://bugs.freedesktop.org/show_bug.cgi?id=96974

> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-a:
>                 dmesg-warn -> PASS       (ro-skl3-i5-6260u)
> 
> fi-kbl-qkkr      total:237  pass:174  dwarn:27  dfail:2   fail:7   skip:27 
> fi-skl-i5-6260u  total:237  pass:189  dwarn:27  dfail:2   fail:7   skip:12 
> fi-skl-i7-6700k  total:237  pass:200  dwarn:2   dfail:0   fail:9   skip:26 
> fi-snb-i7-2600   total:237  pass:188  dwarn:0   dfail:0   fail:9   skip:40 
> ro-bdw-i5-5250u  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16 
> ro-bdw-i7-5557U  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16 
> ro-bdw-i7-5600u  total:237  pass:195  dwarn:1   dfail:1   fail:9   skip:31 
> ro-bsw-n3050     total:217  pass:170  dwarn:0   dfail:0   fail:4   skip:42 
> ro-byt-n2820     total:237  pass:177  dwarn:0   dfail:0   fail:7   skip:38 
> ro-hsw-i3-4010u  total:237  pass:199  dwarn:5   dfail:2   fail:7   skip:24 
> ro-hsw-i7-4770r  total:237  pass:204  dwarn:0   dfail:0   fail:9   skip:24 
> ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63 
> ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58 
> ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33 
> ro-skl3-i5-6260u total:237  pass:213  dwarn:3   dfail:0   fail:9   skip:12 
> ro-snb-i7-2620M  total:237  pass:187  dwarn:0   dfail:0   fail:9   skip:41 
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1484/
> 
> e8b1f0b drm-intel-nightly: 2016y-07m-13d-10h-57m-19s UTC integration manifest
> 6179e9d drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
  2016-07-14 14:39 ` Daniel Vetter
@ 2016-07-27 13:32   ` Ville Syrjälä
  0 siblings, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2016-07-27 13:32 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, David Weinehall

On Thu, Jul 14, 2016 at 04:39:00PM +0200, Daniel Vetter wrote:
> On Wed, Jul 13, 2016 at 04:32:03PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Bspec tells us to keep bashing the PCU for up to 3ms when trying to
> > inform it about an upcoming change in the cdclk frequency. Currently
> > we only keep at it for 15*10usec (+ whatever delays gets added by
> > the sandybridge_pcode_read() itself). Let's change the limit to 3ms.
> > 
> > I decided to keep 10 usec delay per iteration for now, even though
> > the spec doesn't really tell us to do that.
> > 
> > Cc: David Weinehall <david.weinehall@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Needs
> 
> Cc: stable@vger.kernel.org
> 
> and probably also a Fixes: line?

Fixes: 5d96d8afcfbb ("drm/i915/skl: Deinit/init the display at suspend/resume")

and pushed to dinq. Thanks for the review/testing.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 10 +---------
> >  1 file changed, 1 insertion(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index be3b2cab2640..90f26f0e2571 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5691,15 +5691,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
> >  
> >  static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
> >  {
> > -	unsigned int i;
> > -
> > -	for (i = 0; i < 15; i++) {
> > -		if (skl_cdclk_pcu_ready(dev_priv))
> > -			return true;
> > -		udelay(10);
> > -	}
> > -
> > -	return false;
> > +	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
> >  }
> >  
> >  static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk, int vco)
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2016-07-27 13:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-13 13:32 [PATCH] drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL ville.syrjala
2016-07-13 13:37 ` David Weinehall
2016-07-13 14:07 ` ✗ Ro.CI.BAT: failure for " Patchwork
2016-07-18 12:57   ` Ville Syrjälä
2016-07-13 18:42 ` [PATCH] " Chris Wilson
2016-07-14 14:39 ` Daniel Vetter
2016-07-27 13:32   ` 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