[PATCH] drm/i915: Handle error paths during watermark sanitization properly

[PATCH] drm/i915: Handle error paths during watermark sanitization properly

[Matt Roper]

sanitize_watermarks() does not properly handle errors returned by
drm_atomic_helper_duplicate_state().  EDEADLK should trigger a modeset
backoff and retry; for other errors we need to drop locks before
returning.

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
I think this might fix the problems that blew up the CI system recently.

 drivers/gpu/drm/i915/intel_display.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 391cc7f..fdff460 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15316,8 +15316,13 @@ retry:
 	}
 
 	state = drm_atomic_helper_duplicate_state(dev, &ctx);
-	if (WARN_ON(IS_ERR(state)))
-		return;
+	ret = PTR_ERR_OR_ZERO(state);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	} else if (WARN_ON(ret)) {
+		goto fail;
+	}
 
 	ret = intel_atomic_check(dev, state);
 	if (ret) {
@@ -15345,6 +15350,7 @@ retry:
 	}
 
 	drm_atomic_state_free(state);
+fail:
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 }
-- 
2.1.4

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

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on e2edf9a8b66bd170acf15d13861bdf0c20a18f09 drm-intel-nightly: 2016y-01m-11d-14h-56m-49s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (skl-i7k-2) UNSTABLE
                dmesg-warn -> PASS       (bdw-ultra)

bdw-nuci7        total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultra        total:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:141  pass:114  dwarn:3   dfail:0   fail:0   skip:24 
byt-nuc          total:141  pass:119  dwarn:7   dfail:0   fail:0   skip:15 
hsw-brixbox      total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
hsw-xps12        total:138  pass:133  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430s        total:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2        total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2        total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1135/

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

[PATCH] drm/i915: Handle error paths during watermark sanitization properly (v2)

[Matt Roper]

sanitize_watermarks() does not properly handle errors returned by
drm_atomic_helper_duplicate_state().  Make failures drop locks before
returning.  We also change the lock of connection_mutex to a
drm_modeset_lock_all_ctx() to make sure any EDEADLK's are handled
earlier.

v2: Change call to lock connetion_mutex with a call to
    drm_modeset_lock_all_ctx().  This ensures that any lock contention
    is handled earlier and drm_atomic_helper_duplicate_state() won't
    return EDEADLK.

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
Hopefully a fix for the issue that the CI system tripped over.

 drivers/gpu/drm/i915/intel_display.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 391cc7f..5fc942a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15307,7 +15307,7 @@ static void sanitize_watermarks(struct drm_device *dev)
 	 */
 	drm_modeset_acquire_init(&ctx, 0);
 retry:
-	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
+	ret = drm_modeset_lock_all_ctx(dev, &ctx);
 	if (ret == -EDEADLK) {
 		drm_modeset_backoff(&ctx);
 		goto retry;
@@ -15317,7 +15317,7 @@ retry:
 
 	state = drm_atomic_helper_duplicate_state(dev, &ctx);
 	if (WARN_ON(IS_ERR(state)))
-		return;
+		goto fail;
 
 	ret = intel_atomic_check(dev, state);
 	if (ret) {
@@ -15345,6 +15345,7 @@ retry:
 	}
 
 	drm_atomic_state_free(state);
+fail:
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 }
-- 
2.1.4

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

Re: [PATCH] drm/i915: Handle error paths during watermark sanitization properly (v2)

[Maarten Lankhorst]

Hey,

Op 11-01-16 om 19:27 schreef Matt Roper:
> sanitize_watermarks() does not properly handle errors returned by
> drm_atomic_helper_duplicate_state().  Make failures drop locks before
> returning.  We also change the lock of connection_mutex to a
> drm_modeset_lock_all_ctx() to make sure any EDEADLK's are handled
> earlier.
>
This patch still doesn't handle failure from intel_atomic_check and
lock_all_ctx correctly. If you fix that you can add:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Handle error paths during watermark sanitization properly (v3)

[Matt Roper]

sanitize_watermarks() does not properly handle errors returned by
drm_atomic_helper_duplicate_state().  Make failures drop locks before
returning.  We also change the lock of connection_mutex to a
drm_modeset_lock_all_ctx() to make sure any EDEADLK's are handled
earlier.

v2: Change call to lock connetion_mutex with a call to
    drm_modeset_lock_all_ctx().  This ensures that any lock contention
    is handled earlier and drm_atomic_helper_duplicate_state() won't
    return EDEADLK. (Maarten)

v3: Drop locks properly in more error paths. (Maarten)

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index abfb5ba..07ca19b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15353,17 +15353,17 @@ static void sanitize_watermarks(struct drm_device *dev)
 	 */
 	drm_modeset_acquire_init(&ctx, 0);
 retry:
-	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
+	ret = drm_modeset_lock_all_ctx(dev, &ctx);
 	if (ret == -EDEADLK) {
 		drm_modeset_backoff(&ctx);
 		goto retry;
 	} else if (WARN_ON(ret)) {
-		return;
+		goto fail;
 	}
 
 	state = drm_atomic_helper_duplicate_state(dev, &ctx);
 	if (WARN_ON(IS_ERR(state)))
-		return;
+		goto fail;
 
 	/*
 	 * Hardware readout is the only time we don't want to calculate
@@ -15386,7 +15386,7 @@ retry:
 		 * BIOS-programmed watermarks untouched and hope for the best.
 		 */
 		WARN(true, "Could not determine valid watermarks for inherited state\n");
-		return;
+		goto fail;
 	}
 
 	/* Write calculated watermark values back */
@@ -15399,6 +15399,7 @@ retry:
 	}
 
 	drm_atomic_state_free(state);
+fail:
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 }
-- 
2.1.4

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

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on a90796840c30dac6d9907439bf98d1d08046c49d drm-intel-nightly: 2016y-01m-11d-17h-22m-54s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (bdw-nuci7)
                pass       -> DMESG-WARN (skl-i7k-2) UNSTABLE
                pass       -> DMESG-WARN (bdw-ultra)
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (byt-nuc) UNSTABLE

bdw-nuci7        total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultra        total:138  pass:131  dwarn:1   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc          total:141  pass:124  dwarn:2   dfail:0   fail:0   skip:15 
hsw-brixbox      total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
ilk-hp8440p      total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430s        total:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2        total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2        total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1136/

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

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on 37f6c2ae666fbba9eff4355115252b8b0fd43050 drm-intel-nightly: 2016y-01m-12d-14h-25m-44s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (skl-i5k-2) UNSTABLE
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a-frame-sequence:
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup read-crc-pipe-c:
                dmesg-warn -> PASS       (bdw-ultra)

bdw-nuci7        total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultra        total:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc          total:141  pass:123  dwarn:3   dfail:0   fail:0   skip:15 
hsw-brixbox      total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
hsw-xps12        total:138  pass:133  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:141  pass:100  dwarn:4   dfail:0   fail:0   skip:37 
ivb-t430s        total:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2        total:141  pass:107  dwarn:26  dfail:0   fail:0   skip:8  
skl-i7k-2        total:141  pass:107  dwarn:26  dfail:0   fail:0   skip:8  
snb-dellxps      total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1151/

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