drm/i915/bxt,glk: Increase PCODE timeouts during CDCLK freq changing

There is no requirement for doing the PCODE request polling atomically, so do that only for a short time switching to sleeping poll afterwards. The specification requires a 150usec timeout for the change notification, so let's use that for the atomic poll. Do the extra 2ms poll - needed as a workaround on BXT/GLK - in sleeping mode.

v2:
- rebase on v2 of patchset dropping the sandybridge_pcode_read/write refactoring (Chris)

Cc Chris Wilson <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/bxt, glk: Increase PCODE timeouts during CDCLK freq changing
URL : https://patchwork.freedesktop.org/series/37344/
State : failure

== Summary ==

Test kms_rotation_crc:

        Subgroup sprite-rotation-180:
                pass       -> FAIL       (shard-apl)

Test kms_vblank:

        Subgroup pipe-b-ts-continuation-dpms-suspend:
                pass       -> INCOMPLETE (shard-hsw)
Test perf:
        Subgroup oa-exponents:
                fail       -> PASS       (shard-apl) fdo#102254
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_flip:
        Subgroup 2x-plain-flip-fb-recreate:
                fail       -> PASS       (shard-hsw) fdo#100368 +1
        Subgroup 2x-flip-vs-expired-vblank-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#102887
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-b-planes:
                dmesg-warn -> PASS       (shard-snb)
Test kms_cursor_legacy:
        Subgroup cursorb-vs-flipa-atomic:
                pass       -> SKIP       (shard-hsw)

fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887

shard-apl        total:2838 pass:1750 dwarn:1   dfail:0   fail:23  skip:1064 time:12657s
shard-hsw        total:2826 pass:1726 dwarn:1   dfail:0   fail:11  skip:1086 time:11649s
shard-snb        total:2838 pass:1330 dwarn:1   dfail:0   fail:10  skip:1497 time:6619s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7823/shards.html

Currently we see sporadic timeouts during CDCLK changing both on BXT and GLK as reported by the Bugzilla: ticket. It's easy to reproduce this by changing the frequency in a tight loop after blanking the display. The upper bound for the completion time is 800us based on my tests, so increase it from the current 500us to 2ms; with that I couldn't trigger the problem either on BXT or GLK.

Note that timeouts happened during both the change notification and the voltage level setting PCODE request. (For the latter one BSpec doesn't require us to wait for completion before further HW programming.)

This issue is similar to
2c7d0602c815 ("drm/i915/gen9: Fix PCODE polling during CDCLK change notification")
but there the PCODE request does complete (as shown by the mbox busy flag), only the reply we get from PCODE indicates a failure. So there we keep resending the request until a success reply, here we just have to increase the timeout for the one PCODE request we send.

v2:
- s/snb_pcode_request/sandybridge_pcode_write_timeout/ (Ville)

Cc Chris Wilson <[email protected]>

Cc Ville Syrjälä <[email protected]>

Cc <[email protected]> # v4.4+

Acked-by Chris Wilson <[email protected]> (v1)

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

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h    |  6 +++++-
 drivers/gpu/drm/i915/intel_cdclk.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/i915/intel_pm.c    |  6 +++---
 3 files changed, 25 insertions(+), 9 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +5 -1

index 454d8f937fae..7e83d0d3e632 100644
@@ -3723,7 +3723,11 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
 					    struct intel_display_error_state *error);
 
 int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val);
-int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val);
+int sandybridge_pcode_write_timeout(struct drm_i915_private *dev_priv, u32 mbox,
+				    u32 val, int timeout_us);
+#define sandybridge_pcode_write(dev_priv, mbox, val)	\
+	sandybridge_pcode_write_timeout(dev_priv, mbox, val, 500)
+
 int skl_pcode_request(struct drm_i915_private *dev_priv, u32 mbox, u32 request,
 		      u32 reply_mask, u32 reply, int timeout_base_ms);
 

drivers/gpu/drm/i915/intel_cdclk.c +17 -5

index c4392ea34a3d..a423b674fcec 100644
@@ -1370,10 +1370,15 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		break;
 	}
 
-	/* Inform power controller of upcoming frequency change */
+	/*
+	 * Inform power controller of upcoming frequency change. BSpec
+	 * requires us to wait up to 150usec, but that leads to timeouts;
+	 * the 2ms used here is based on experiment.
+	 */
 	mutex_lock(&dev_priv->pcu_lock);
-	ret = sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
-				      0x80000000);
+	ret = sandybridge_pcode_write_timeout(dev_priv,
+					      HSW_PCODE_DE_WRITE_FREQ_REQ,
+					      0x80000000, 2000);
 	mutex_unlock(&dev_priv->pcu_lock);
 
 	if (ret) {

@@ -1404,8 +1409,15 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 	I915_WRITE(CDCLK_CTL, val);
 
 	mutex_lock(&dev_priv->pcu_lock);
-	ret = sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
-				      cdclk_state->voltage_level);
+	/*
+	 * The timeout isn't specified, the 2ms used here is based on
+	 * experiment.
+	 * FIXME: Waiting for the request completion could be delayed until
+	 * the next PCODE request based on BSpec.
+	 */
+	ret = sandybridge_pcode_write_timeout(dev_priv,
+					      HSW_PCODE_DE_WRITE_FREQ_REQ,
+					      cdclk_state->voltage_level, 2000);
 	mutex_unlock(&dev_priv->pcu_lock);
 
 	if (ret) {

drivers/gpu/drm/i915/intel_pm.c +3 -3

index 0b92ea1dbd40..a6098932be5a 100644
@@ -9169,8 +9169,8 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val
 	return 0;
 }
 
-int sandybridge_pcode_write(struct drm_i915_private *dev_priv,
-			    u32 mbox, u32 val)
+int sandybridge_pcode_write_timeout(struct drm_i915_private *dev_priv,
+				    u32 mbox, u32 val, int timeout_us)
 {
 	int status;
 

@@ -9193,7 +9193,7 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv,
 
 	if (__intel_wait_for_register_fw(dev_priv,
 					 GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 0,
-					 500, 0, NULL)) {
+					 timeout_us, 0, NULL)) {
 		DRM_ERROR("timeout waiting for pcode write of 0x%08x to mbox %x to finish for %ps\n",
 			  val, mbox, __builtin_return_address(0));
 		return -ETIMEDOUT;

Signature · 1 line

2.13.2