search for: nouveau_fbcon_set_suspend

The FBIOPUT_CON2FBMAP ioctl takes a console_lock(). When this is called while nouveau was runtime suspended, a deadlock would occur due to nouveau_fbcon_set_suspend also trying to obtain console_lock(). Fix this by delaying the drm_fb_helper_set_suspend call. Based on the i915 code (which was done for performance reasons though). Cc: Chris Wilson <chris at chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter at ffwll.ch> Signed-off-by: Peter Wu &...

On Tue, Jul 12, 2016 at 06:49:34PM +0200, Peter Wu wrote: > The FBIOPUT_CON2FBMAP ioctl takes a console_lock(). When this is called > while nouveau was runtime suspended, a deadlock would occur due to > nouveau_fbcon_set_suspend also trying to obtain console_lock(). > > Fix this by delaying the drm_fb_helper_set_suspend call. Based on the > i915 code (which was done for performance reasons though). > > Cc: Chris Wilson <chris at chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch...

On Tue, Jul 12, 2016 at 06:49:34PM +0200, Peter Wu wrote: > The FBIOPUT_CON2FBMAP ioctl takes a console_lock(). When this is called > while nouveau was runtime suspended, a deadlock would occur due to > nouveau_fbcon_set_suspend also trying to obtain console_lock(). > > Fix this by delaying the drm_fb_helper_set_suspend call. Based on the > i915 code (which was done for performance reasons though). > > Cc: Chris Wilson <chris at chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch...

On Wed, Jul 13, 2016 at 11:54:49AM +0200, Daniel Vetter wrote: > On Tue, Jul 12, 2016 at 06:49:34PM +0200, Peter Wu wrote: > > The FBIOPUT_CON2FBMAP ioctl takes a console_lock(). When this is called > > while nouveau was runtime suspended, a deadlock would occur due to > > nouveau_fbcon_set_suspend also trying to obtain console_lock(). > > > > Fix this by delaying the drm_fb_helper_set_suspend call. Based on the > > i915 code (which was done for performance reasons though). > > > > Cc: Chris Wilson <chris at chris-wilson.co.uk> > > Cc: Daniel Vetter...

On Tue, Jul 12, 2016 at 06:49:34PM +0200, Peter Wu wrote: > The FBIOPUT_CON2FBMAP ioctl takes a console_lock(). When this is called > while nouveau was runtime suspended, a deadlock would occur due to > nouveau_fbcon_set_suspend also trying to obtain console_lock(). > > Fix this by delaying the drm_fb_helper_set_suspend call. Based on the > i915 code (which was done for performance reasons though). > > Cc: Chris Wilson <chris at chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch...

On Wed, Jul 13, 2016 at 06:17:47PM +0100, Chris Wilson wrote: > On Tue, Jul 12, 2016 at 06:49:34PM +0200, Peter Wu wrote: > > The FBIOPUT_CON2FBMAP ioctl takes a console_lock(). When this is called > > while nouveau was runtime suspended, a deadlock would occur due to > > nouveau_fbcon_set_suspend also trying to obtain console_lock(). > > > > Fix this by delaying the drm_fb_helper_set_suspend call. Based on the > > i915 code (which was done for performance reasons though). > > > > Cc: Chris Wilson <chris at chris-wilson.co.uk> > > Cc: Daniel Vetter...

...tainted ------------------------------------------------------- sh/1017 is trying to acquire lock: ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0 but task is already holding lock: (console_lock){+.+.+.}, at: [<ffffffffa0204d35>] nouveau_fbcon_set_suspend+0x25/0xc0 [nouveau] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (console_lock){+.+.+.}: [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60 [<ffffffff810916ea>] lock_acquire+0x5a/0x70 [<ffffffff810407a7>] co...

...ex 5ea8fe992484..db56e9b6b6af 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -674,10 +674,11 @@ nouveau_do_suspend(struct drm_device *dev, bool runtime) if (dev->mode_config.num_crtc) { NV_DEBUG(drm, "suspending console...\n"); nouveau_fbcon_set_suspend(dev, 1); + NV_DEBUG(drm, "suspending display...\n"); ret = nouveau_display_suspend(dev, runtime); if (ret) - return ret; + goto fail_fbcon; } NV_DEBUG(drm, "evicting buffers...\n"); @@ -719,7 +720,14 @@ nouveau_do_suspend(struct drm_device *dev, bool runtime)...

...v); nouveau_accel_fini(drm); nouveau_hwmon_fini(dev); @@ -550,6 +553,8 @@ nouveau_do_suspend(struct drm_device *dev, bool runtime) struct nouveau_cli *cli; int ret; + nouveau_led_suspend(dev); + if (dev->mode_config.num_crtc) { NV_INFO(drm, "suspending console...\n"); nouveau_fbcon_set_suspend(dev, 1); @@ -638,6 +643,8 @@ nouveau_do_resume(struct drm_device *dev, bool runtime) nouveau_fbcon_set_suspend(dev, 0); } + nouveau_led_resume(dev); + return 0; } diff --git a/drm/nouveau/nouveau_drm.h b/drm/nouveau/nouveau_drm.h index 5c363ed..b148dcb 100644 --- a/drm/nouveau/nouveau_d...

...runtime) >> struct nouveau_cli *cli; >> int ret; >> >> + nouveau_led_suspend(dev); >> + >> if (dev->mode_config.num_crtc) { >> NV_INFO(drm, "suspending console...\n"); >> nouveau_fbcon_set_suspend(dev, 1); >> @@ -638,6 +643,8 @@ nouveau_do_resume(struct drm_device *dev, bool runtime) >> nouveau_fbcon_set_suspend(dev, 0); >> } >> >> + nouveau_led_resume(dev); >> + >> return 0; >> } >> >> dif...

...quot;); nouveau_display_resume(dev); } return 0; diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index b035317..46e37c0 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -514,10 +514,10 @@ void nouveau_fbcon_set_suspend(struct drm_device *dev, int state) struct nouveau_drm *drm = nouveau_drm(dev); console_lock(); if (state == 0) - nouveau_fbcon_save_disable_accel(dev); + nouveau_fbcon_restore_accel(dev); fb_set_suspend(drm->fbcon->helper.fbdev, state); if (state == 1) - nouveau_fbcon_restore_acc...

...v); nouveau_accel_fini(drm); nouveau_hwmon_fini(dev); @@ -561,6 +564,8 @@ nouveau_do_suspend(struct drm_device *dev, bool runtime) struct nouveau_cli *cli; int ret; + nouveau_led_suspend(dev); + if (dev->mode_config.num_crtc) { NV_INFO(drm, "suspending console...\n"); nouveau_fbcon_set_suspend(dev, 1); @@ -649,6 +654,8 @@ nouveau_do_resume(struct drm_device *dev, bool runtime) nouveau_fbcon_set_suspend(dev, 0); } + nouveau_led_resume(dev); + return 0; } diff --git a/drm/nouveau/nouveau_drv.h b/drm/nouveau/nouveau_drv.h index 822a021..c0e2b32 100644 --- a/drm/nouveau/nouveau_d...

...but that did not gave any hint. I found it by reading the source code. > > It might be useful if you can open a bug report and attach the > information in there [1] I didn't opened a report because there are already many reports about garbled console. I just thought that the actual nouveau_fbcon_set_suspend() is obviously doing the wrong thing and the fix is too easy. And the remaining snow on the console, which other users get, are other causes. Actually reproducible is this (also with my patch): 1. Fresh boot up, run mplayer (-vo xv), switch to fullscreen: GPU lockup. If I do a s2ram before, it w...

As it turns out, on cards that actually have CRTCs on them we're already calling drm_kms_helper_poll_enable(drm_dev) from nouveau_display_resume() before we call it in nouveau_pmops_runtime_resume(). This leads us to accidentally trying to enable polling twice, which results in a potential deadlock between the RPM locks and drm_dev->mode_config.mutex if we end up trying to enable polling

..._suspend(struct drm_device *dev, bool runtime) > struct nouveau_cli *cli; > int ret; > > + nouveau_led_suspend(dev); > + > if (dev->mode_config.num_crtc) { > NV_INFO(drm, "suspending console...\n"); > nouveau_fbcon_set_suspend(dev, 1); > @@ -638,6 +643,8 @@ nouveau_do_resume(struct drm_device *dev, bool runtime) > nouveau_fbcon_set_suspend(dev, 0); > } > > + nouveau_led_resume(dev); > + > return 0; > } > > diff --git a/drm/nouveau/nouveau_drm.h b/drm/no...

..._suspend(struct drm_device *dev, bool runtime) > struct nouveau_cli *cli; > int ret; > > + nouveau_led_suspend(dev); > + > if (dev->mode_config.num_crtc) { > NV_INFO(drm, "suspending console...\n"); > nouveau_fbcon_set_suspend(dev, 1); > @@ -638,6 +643,8 @@ nouveau_do_resume(struct drm_device *dev, bool runtime) > nouveau_fbcon_set_suspend(dev, 0); > } > > + nouveau_led_resume(dev); > + > return 0; > } > > diff --git a/drm/nouveau/nouveau_drm.h b/drm/no...

..._suspend(struct drm_device *dev, bool runtime) > struct nouveau_cli *cli; > int ret; > > + nouveau_led_suspend(dev); > + > if (dev->mode_config.num_crtc) { > NV_INFO(drm, "suspending console...\n"); > nouveau_fbcon_set_suspend(dev, 1); > @@ -638,6 +643,8 @@ nouveau_do_resume(struct drm_device *dev, bool runtime) > nouveau_fbcon_set_suspend(dev, 0); > } > > + nouveau_led_resume(dev); > + > return 0; > } > > diff --git a/drm/nouveau/nouveau_drm.h b/drm/no...

...changed = nouveau_fbcon_output_poll_changed, }; diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 85c1f10bc2b6..8cf966690963 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -466,6 +466,7 @@ nouveau_fbcon_set_suspend_work(struct work_struct *work) console_unlock(); if (state == FBINFO_STATE_RUNNING) { + nouveau_fbcon_hotplug_resume(drm->fbcon); pm_runtime_mark_last_busy(drm->dev->dev); pm_runtime_put_sync(drm->dev->dev); } @@ -487,6 +488,61 @@ nouveau_fbcon_set_suspend(struct drm_d...

...changed = nouveau_fbcon_output_poll_changed, }; diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 85c1f10bc2b6..c7e86e9232a6 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -466,6 +466,7 @@ nouveau_fbcon_set_suspend_work(struct work_struct *work) console_unlock(); if (state == FBINFO_STATE_RUNNING) { + nouveau_fbcon_hotplug_resume(drm->fbcon); pm_runtime_mark_last_busy(drm->dev->dev); pm_runtime_put_sync(drm->dev->dev); } @@ -487,6 +488,76 @@ nouveau_fbcon_set_suspend(struct drm_d...

This is the latest version of https://patchwork.freedesktop.org/series/46815/ I moved everything out of fb_helper and back into nouveau, because it seems that other drivers actually do have this handled already as far as I can tell. Lyude Paul (13): drm/nouveau: Fix bogus drm_kms_helper_poll_enable() placement drm/nouveau: Remove duplicate poll_enable() in pmops_runtime_suspend()