search for: nouveau_display_suspend

On Tue, 2017-07-11 at 13:51 -0400, Ilia Mirkin wrote: > Some details that may be useful in analysis of the bug: > > 1. lspci -nn -d 10de: 01:00.0 VGA compatible controller [0300]: NVIDIA Corporation GM204 [GeForce GTX 980] [10de:13c0] (rev a1) 01:00.1 Audio device [0403]: NVIDIA Corporation GM204 High Definition Audio Controller [10de:0fbb] (rev a1 > 2. What displays, if any, you

...- if (ret < 0) { - drm_atomic_state_put(state); - state = ERR_PTR(ret); - goto unlock; - } - -unlock: - if (PTR_ERR(state) == -EDEADLK) { - drm_modeset_backoff(&ctx); - goto retry; - } - - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - return state; -} - int nouveau_display_suspend(struct drm_device *dev, bool runtime) { @@ -744,7 +633,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime) if (drm_drv_uses_atomic_modeset(dev)) { if (!runtime) { - disp->suspend = nouveau_atomic_suspend(dev); + disp->suspend = drm_atomic_helper_suspend(dev); if...

Currently, it appears that nouveau_do_suspend() forgets to roll back suspending fbcon and suspending the device LEDs. We also currently skip the entire rollback process if nouveau_display_suspend() fails. So, fix that. Signed-off-by: Lyude Paul <lyude at redhat.com> Cc: stable at vger.kernel.org Cc: Lukas Wunner <lukas at wunner.de> Cc: Karol Herbst <karolherbst at gmail.com> --- drivers/gpu/drm/nouveau/nouveau_drm.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 de...

...rm = nouveau_drm(dev); @@ -450,6 +450,9 @@ nouveau_display_fini(struct drm_device *dev, bool suspend) } drm_connector_list_iter_end(&conn_iter); + if (!runtime) + cancel_work_sync(&drm->hpd_work); + drm_kms_helper_poll_disable(dev); disp->fini(dev); } @@ -618,11 +621,11 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime) } } - nouveau_display_fini(dev, true); + nouveau_display_fini(dev, true, runtime); return 0; } - nouveau_display_fini(dev, true); + nouveau_display_fini(dev, true, runtime); list_for_each_entry(crtc, &dev->mode_config.crtc_list, hea...

...- if (ret < 0) { - drm_atomic_state_put(state); - state = ERR_PTR(ret); - goto unlock; - } - -unlock: - if (PTR_ERR(state) == -EDEADLK) { - drm_modeset_backoff(&ctx); - goto retry; - } - - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - return state; -} - int nouveau_display_suspend(struct drm_device *dev, bool runtime) { @@ -740,7 +629,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime) if (drm_drv_uses_atomic_modeset(dev)) { if (!runtime) { - disp->suspend = nouveau_atomic_suspend(dev); + disp->suspend = drm_atomic_helper_suspend(dev); if...

...; - goto unlock; > - } > - > -unlock: > - if (PTR_ERR(state) == -EDEADLK) { > - drm_modeset_backoff(&ctx); > - goto retry; > - } > - > - drm_modeset_drop_locks(&ctx); > - drm_modeset_acquire_fini(&ctx); > - return state; > -} > - > int > nouveau_display_suspend(struct drm_device *dev, bool runtime) > { > @@ -744,7 +633,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime) > > if (drm_drv_uses_atomic_modeset(dev)) { > if (!runtime) { > - disp->suspend = nouveau_atomic_suspend(dev); > + disp->suspend = drm...

...ndle hotplugs since we won't have them enabled */ + if (runtime) + WRITE_ONCE(drm->acpi_hpd_enabled, true); + /* disable hotplug interrupts */ drm_connector_list_iter_begin(dev, &conn_iter); nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) { @@ -619,11 +633,11 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime) } } - nouveau_display_fini(dev, true); + nouveau_display_fini(dev, true, runtime); return 0; } - nouveau_display_fini(dev, true); + nouveau_display_fini(dev, true, runtime); list_for_each_entry(crtc, &dev->mode_config.crtc_list, hea...

...ci_pm_runtime_resume+0xa0/0xa0 > [ 12.768238] ? drm_vblank_disable_and_save+0x55/0xc0 [drm] > [ 12.768250] ? drm_crtc_vblank_off+0xa9/0x1e0 [drm] > [ 12.768253] ? pci_pm_runtime_resume+0xa0/0xa0 > [ 12.768299] ? nouveau_display_fini+0x56/0xd0 [nouveau] > [ 12.768339] ? nouveau_display_suspend+0x51/0x110 [nouveau] > [ 12.768378] ? nouveau_do_suspend+0x76/0x1c0 [nouveau] > [ 12.768413] ? nouveau_pmops_runtime_suspend+0x54/0xb0 [nouveau] > [ 12.768416] ? pci_pm_runtime_suspend+0x5c/0x160 > [ 12.768419] ? __rpm_callback+0xb6/0x1e0 > [ 12.768423] ? kobject_ueven...

...f7ba8] drm_update_vblank_count at ffffffffa020b3e1 [drm] #15 [ffffc900039f7c10] drm_vblank_disable_and_save at ffffffffa020bbe9 [drm] #16 [ffffc900039f7c40] drm_crtc_vblank_off at ffffffffa020c3c0 [drm] #17 [ffffc900039f7cb0] nouveau_display_fini at ffffffffa048a4d6 [nouveau] #18 [ffffc900039f7ce0] nouveau_display_suspend at ffffffffa048ac4f [nouveau] #19 [ffffc900039f7d00] nouveau_do_suspend at ffffffffa047e5ec [nouveau] #20 [ffffc900039f7d38] nouveau_pmops_suspend at ffffffffa047e77d [nouveau] #21 [ffffc900039f7d50] pci_pm_suspend at ffffffff813b1ff0 #22 [ffffc900039f7d80] dpm_run_callback at ffffffff814c4dbd #23...

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()

...gt; [ 12.768238] ? drm_vblank_disable_and_save+0x55/0xc0 [drm] >>> [ 12.768250] ? drm_crtc_vblank_off+0xa9/0x1e0 [drm] >>> [ 12.768253] ? pci_pm_runtime_resume+0xa0/0xa0 >>> [ 12.768299] ? nouveau_display_fini+0x56/0xd0 [nouveau] >>> [ 12.768339] ? nouveau_display_suspend+0x51/0x110 [nouveau] >>> [ 12.768378] ? nouveau_do_suspend+0x76/0x1c0 [nouveau] >>> [ 12.768413] ? nouveau_pmops_runtime_suspend+0x54/0xb0 [nouveau] >>> [ 12.768416] ? pci_pm_runtime_suspend+0x5c/0x160 >>> [ 12.768419] ? __rpm_callback+0xb6/0x1e0 >...

...d(struct drm_device *dev, bool runtime) if (dev->mode_config.num_crtc) { NV_INFO(drm, "suspending console...\n"); - nouveau_fbcon_set_suspend(dev, 1); + nouveau_fbcon_set_suspend(dev, FBINFO_STATE_SUSPENDED, true); NV_INFO(drm, "suspending display...\n"); ret = nouveau_display_suspend(dev, runtime); if (ret) @@ -635,7 +635,7 @@ nouveau_do_resume(struct drm_device *dev, bool runtime) NV_INFO(drm, "resuming display...\n"); nouveau_display_resume(dev, runtime); NV_INFO(drm, "resuming console...\n"); - nouveau_fbcon_set_suspend(dev, 0); + nouveau_fb...

On Sat, 2017-07-15 at 14:52 -0400, Ilia Mirkin wrote: > > OK, so this issue appears to be that we're calling > drm_crtc_vblank_off() on a crtc for which vblank is already disabled. > My guess is that this happens because the crtc is disabled. > > Not sure what the proper check is to see if vblanks are already disabled... Seems so, the below shut up suspend for both 8600 GT

Signed-off-by: Lyude Paul <lyude at redhat.com> --- drivers/gpu/drm/nouveau/nouveau_display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index ec7861457b84..b2a93e3fa67b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -387,6 +387,7 @@

...MST but browsing through drm_dp_mst_topology.c I notice that drm_dp_mst_link_probe_work() is the ->work element in drm_dp_mst_topology_mgr() and is queued on HPD. I further notice that the work item is flushed on ->runtime_suspend: nouveau_pmops_runtime_suspend() nouveau_do_suspend() nouveau_display_suspend() nouveau_display_fini() disp->fini() == nv50_display_fini() nv50_mstm_fini() drm_dp_mst_topology_mgr_suspend() flush_work(&mgr->work); And before the work item is flushed, the HPD source is quiesced. So it looks like drm_dp_mst_link_probe_work() can only ev...

...tional weirdness, which I assumed you had since you were talking about STR.) My best guess is that gf119_head_vblank_put either has a bogus head id (should be in the 0..3 range) which causes it to do an out-of-bounds read on MMIO space, or that the MMIO mapping has already been removed by the time nouveau_display_suspend runs. Adding Ben Skeggs for additional insight. Some display stuff did change for 4.13 for GM20x+ boards. If it's not too much trouble, a bisect would be pretty useful. Cheers, -ilia

...king > about STR.) Yup, garden variety deskside box. > My best guess is that gf119_head_vblank_put either has a bogus head id > (should be in the 0..3 range) which causes it to do an out-of-bounds > read on MMIO space, or that the MMIO mapping has already been removed > by the time nouveau_display_suspend runs. Adding Ben Skeggs for > additional insight. > > Some display stuff did change for 4.13 for GM20x+ boards. If it's not > too much trouble, a bisect would be pretty useful. Vacation -> back to work happens in the very early AM, so bisection will have to wait a bit. -Mike

...head) { if (nouveau_crtc(crtc)->index == pipe) { struct drm_display_mode *mode; - if (dev->mode_config.funcs->atomic_commit) + if (drm_drv_uses_atomic_modeset(dev)) mode = &crtc->state->adjusted_mode; else mode = &crtc->hwmode; @@ -738,7 +738,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime) struct nouveau_display *disp = nouveau_display(dev); struct drm_crtc *crtc; - if (dev->mode_config.funcs->atomic_commit) { + if (drm_drv_uses_atomic_modeset(dev)) { if (!runtime) { disp->suspend = nouveau_atomic_suspend(dev); if (IS_ER...

Does what it says on the label, a lot of these calls are already handled somewhere else and don't appear to be here for a legitimate reason anymore. Lyude Paul (3): drm/nouveau: Remove useless poll_enable() call in switcheroo_set_state() drm/nouveau: Remove useless poll_disable() call in switcheroo_set_state() drm/nouveau: Remove useless poll_enable() call in drm_load()

Next version of https://patchwork.freedesktop.org/series/48131/ Only changes are new A-Bs and R-Bs Lyude Paul (3): drm/nouveau: Remove useless poll_enable() call in switcheroo_set_state() drm/nouveau: Remove useless poll_disable() call in switcheroo_set_state() drm/nouveau: Remove useless poll_enable() call in drm_load() drivers/gpu/drm/nouveau/nouveau_drm.c | 4 +---