[Intel-gfx] [PATCH] drm/atomic: Make drm_atomic_legacy_backoff reset crtc->acquire_ctx

2016-06-23 Thread Daniel Vetter
On Thu, Jun 23, 2016 at 01:45:06PM +0200, Maarten Lankhorst wrote:
> Atomic updates may acquire more state than initially locked through
> drm_modeset_lock_crtc, running with heavy stress can cause a
> WARN_ON(crtc->acquire_ctx) in drm_modeset_lock_crtc:
> 
> [  601.491296] [ cut here ]
> [  601.491366] WARNING: CPU: 0 PID: 2411 at
> drivers/gpu/drm/drm_modeset_lock.c:191 drm_modeset_lock_crtc+0xeb/0xf0 [drm]
> [  601.491369] Modules linked in: drm i915 drm_kms_helper
> [  601.491414] CPU: 0 PID: 2411 Comm: kms_cursor_lega Tainted: G U 
> 4.7.0-rc4-patser+ #4798
> [  601.491417] Hardware name: Intel Corporation Skylake Client
> [  601.491420]   88044d153c98 812ead28 
> 
> [  601.491425]   88044d153cd8 810868e6 
> 00bf58058030
> [  601.491431]  880088b415e8 880458058030 88008a271548 
> 88008a271568
> [  601.491436] Call Trace:
> [  601.491443]  [] dump_stack+0x4d/0x65
> [  601.491447]  [] __warn+0xc6/0xe0
> [  601.491452]  [] warn_slowpath_null+0x18/0x20
> [  601.491472]  [] drm_modeset_lock_crtc+0xeb/0xf0 [drm]
> [  601.491491]  [] drm_mode_cursor_common+0x66/0x180 [drm]
> [  601.491509]  [] drm_mode_cursor_ioctl+0x3c/0x40 [drm]
> [  601.491524]  [] drm_ioctl+0x14d/0x530 [drm]
> [  601.491540]  [] ? drm_mode_setcrtc+0x520/0x520 [drm]
> [  601.491545]  [] ? handle_mm_fault+0x106b/0x1430
> [  601.491550]  [] ? stop_one_cpu+0x61/0x70
> [  601.491556]  [] do_vfs_ioctl+0x8d/0x570
> [  601.491560]  [] ? security_file_ioctl+0x3e/0x60
> [  601.491565]  [] SyS_ioctl+0x74/0x80
> [  601.491571]  [] ? posix_get_monotonic_raw+0xc/0x10
> [  601.491576]  [] entry_SYSCALL_64_fastpath+0x13/0x8f
> [  601.491581] ---[ end trace 56f3d3d85f000d00 ]---
> 
> For good measure, test mode_config.acquire_ctx too, although this should
> never happen.
> 
> Testcase: kms_cursor_legacy
> Signed-off-by: Maarten Lankhorst 

It's kinda a bug in the skl wm code which unecessarily looks at other
plane state.

Reviewed-by: Daniel Vetter 

Given that this is fairly hard to hit, but will blow in really strange
ways in practice I think also justified for backporting.

Cc: stable at vger.kernel.org

Dave, can you pls pick up for -fixes directly?

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c | 27 ++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index d99ab2f6663f..3cee084e9d28 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1299,14 +1299,39 @@ EXPORT_SYMBOL(drm_atomic_add_affected_planes);
>   */
>  void drm_atomic_legacy_backoff(struct drm_atomic_state *state)
>  {
> + struct drm_device *dev = state->dev;
> + unsigned crtc_mask = 0;
> + struct drm_crtc *crtc;
>   int ret;
> + bool global = false;
> +
> + drm_for_each_crtc(crtc, dev) {
> + if (crtc->acquire_ctx != state->acquire_ctx)
> + continue;
> +
> + crtc_mask |= drm_crtc_mask(crtc);
> + crtc->acquire_ctx = NULL;
> + }
> +
> + if (WARN_ON(dev->mode_config.acquire_ctx == state->acquire_ctx)) {
> + global = true;
> +
> + dev->mode_config.acquire_ctx = NULL;
> + }
>  
>  retry:
>   drm_modeset_backoff(state->acquire_ctx);
>  
> - ret = drm_modeset_lock_all_ctx(state->dev, state->acquire_ctx);
> + ret = drm_modeset_lock_all_ctx(dev, state->acquire_ctx);
>   if (ret)
>   goto retry;
> +
> + drm_for_each_crtc(crtc, dev)
> + if (drm_crtc_mask(crtc) & crtc_mask)
> + crtc->acquire_ctx = state->acquire_ctx;
> +
> + if (global)
> + dev->mode_config.acquire_ctx = state->acquire_ctx;
>  }
>  EXPORT_SYMBOL(drm_atomic_legacy_backoff);
>  
> -- 
> 2.5.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH] drm/atomic: Make drm_atomic_legacy_backoff reset crtc->acquire_ctx

2016-06-23 Thread Maarten Lankhorst
Atomic updates may acquire more state than initially locked through
drm_modeset_lock_crtc, running with heavy stress can cause a
WARN_ON(crtc->acquire_ctx) in drm_modeset_lock_crtc:

[  601.491296] [ cut here ]
[  601.491366] WARNING: CPU: 0 PID: 2411 at
drivers/gpu/drm/drm_modeset_lock.c:191 drm_modeset_lock_crtc+0xeb/0xf0 [drm]
[  601.491369] Modules linked in: drm i915 drm_kms_helper
[  601.491414] CPU: 0 PID: 2411 Comm: kms_cursor_lega Tainted: G U 
4.7.0-rc4-patser+ #4798
[  601.491417] Hardware name: Intel Corporation Skylake Client
[  601.491420]   88044d153c98 812ead28 

[  601.491425]   88044d153cd8 810868e6 
00bf58058030
[  601.491431]  880088b415e8 880458058030 88008a271548 
88008a271568
[  601.491436] Call Trace:
[  601.491443]  [] dump_stack+0x4d/0x65
[  601.491447]  [] __warn+0xc6/0xe0
[  601.491452]  [] warn_slowpath_null+0x18/0x20
[  601.491472]  [] drm_modeset_lock_crtc+0xeb/0xf0 [drm]
[  601.491491]  [] drm_mode_cursor_common+0x66/0x180 [drm]
[  601.491509]  [] drm_mode_cursor_ioctl+0x3c/0x40 [drm]
[  601.491524]  [] drm_ioctl+0x14d/0x530 [drm]
[  601.491540]  [] ? drm_mode_setcrtc+0x520/0x520 [drm]
[  601.491545]  [] ? handle_mm_fault+0x106b/0x1430
[  601.491550]  [] ? stop_one_cpu+0x61/0x70
[  601.491556]  [] do_vfs_ioctl+0x8d/0x570
[  601.491560]  [] ? security_file_ioctl+0x3e/0x60
[  601.491565]  [] SyS_ioctl+0x74/0x80
[  601.491571]  [] ? posix_get_monotonic_raw+0xc/0x10
[  601.491576]  [] entry_SYSCALL_64_fastpath+0x13/0x8f
[  601.491581] ---[ end trace 56f3d3d85f000d00 ]---

For good measure, test mode_config.acquire_ctx too, although this should
never happen.

Testcase: kms_cursor_legacy
Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index d99ab2f6663f..3cee084e9d28 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1299,14 +1299,39 @@ EXPORT_SYMBOL(drm_atomic_add_affected_planes);
  */
 void drm_atomic_legacy_backoff(struct drm_atomic_state *state)
 {
+   struct drm_device *dev = state->dev;
+   unsigned crtc_mask = 0;
+   struct drm_crtc *crtc;
int ret;
+   bool global = false;
+
+   drm_for_each_crtc(crtc, dev) {
+   if (crtc->acquire_ctx != state->acquire_ctx)
+   continue;
+
+   crtc_mask |= drm_crtc_mask(crtc);
+   crtc->acquire_ctx = NULL;
+   }
+
+   if (WARN_ON(dev->mode_config.acquire_ctx == state->acquire_ctx)) {
+   global = true;
+
+   dev->mode_config.acquire_ctx = NULL;
+   }

 retry:
drm_modeset_backoff(state->acquire_ctx);

-   ret = drm_modeset_lock_all_ctx(state->dev, state->acquire_ctx);
+   ret = drm_modeset_lock_all_ctx(dev, state->acquire_ctx);
if (ret)
goto retry;
+
+   drm_for_each_crtc(crtc, dev)
+   if (drm_crtc_mask(crtc) & crtc_mask)
+   crtc->acquire_ctx = state->acquire_ctx;
+
+   if (global)
+   dev->mode_config.acquire_ctx = state->acquire_ctx;
 }
 EXPORT_SYMBOL(drm_atomic_legacy_backoff);

-- 
2.5.5