* 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: ✗ 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-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: [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