diff mbox

[drm/nouveau] GeForce 8600 GT boot/suspend grumbling

Message ID 1500180236.5334.7.camel@gmx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Galbraith July 16, 2017, 4:43 a.m. UTC
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 and GTX 980.

---
 drivers/gpu/drm/drm_vblank.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Ilia Mirkin July 16, 2017, 3:59 p.m. UTC | #1
On Sun, Jul 16, 2017 at 12:43 AM, Mike Galbraith <efault@gmx.de> wrote:
> 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 and GTX 980.

The modeset done by drm_atomic_helper_suspend (called previously to
that *_fini) should already take care of disabling vblanks, I think.
So the vblank_off calls can just be done when we're not doing an
atomic modeset [drm_drv_uses_atomic_modeset(dev)] -- this is all very
confusing since pre-nv50 uses legacy modesets, while nv50+ has been
moved to atomic, but they share a bunch of helpers =/
Ilia Mirkin July 19, 2017, 8:10 p.m. UTC | #2
I believe the solution is to not call drm_crtc_vblank_off for atomic
modesetting in nouveau_display_fini. I think Ben's working on it.

On Wed, Jul 19, 2017 at 1:25 PM, Tobias Klausmann
<tobias.johannes.klausmann@mni.thm.de> wrote:
> mimic the behavior of vblank_disable_fn(), another caller of
> drm_vblank_disable_and_save().
>
> This avoids oopsing, while trying to disable vblank on a not connected display:
>
> [   12.768079] WARNING: CPU: 0 PID: 274 at drivers/gpu/drm/drm_vblank.c:609 drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
> [   12.768080] Modules linked in: bnep snd_hda_codec_hdmi rtsx_usb_sdmmc uvcvideo rtsx_usb_ms mmc_core videobuf2_vmalloc memstick videobuf2_memops videobuf2_v4l2 videobuf2_core rtsx_usb videodev btusb btrtl arc4 snd_hda_codec_realtek snd_hda_codec_generic joydev nls_iso8859_1 hid_multitouch nls_cp437 intel_rapl x86_pkg_temp_thermal intel_powerclamp vfat coretemp fat kvm_intel iTCO_wdt iTCO_vendor_support kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc aesni_intel ath10k_pci snd_hda_intel ath10k_core aes_x86_64 snd_hda_codec crypto_simd ath glue_helper cryptd snd_hda_core mac80211 snd_hwdep snd_pcm pcspkr r8169 cfg80211 mii snd_timer acer_wmi snd sparse_keymap wmi_bmof idma64 hci_uart virt_dma mei_me soundcore i2c_i801 mei btbcm shpchp intel_lpss_pci intel_pch_thermal
> [   12.768130]  serdev btqca ucsi_acpi btintel typec_ucsi thermal typec bluetooth ecdh_generic battery ac pinctrl_sunrisepoint rfkill intel_lpss_acpi pinctrl_intel intel_lpss acpi_pad nouveau serio_raw i915 mxm_wmi ttm i2c_algo_bit drm_kms_helper xhci_pci syscopyarea sysfillrect sysimgblt xhci_hcd fb_sys_fops usbcore drm i2c_hid wmi video button sg efivarfs
> [   12.768158] CPU: 0 PID: 274 Comm: kworker/0:2 Not tainted 4.12.0-desktop-debug-drm+ #2
> [   12.768160] Hardware name: Acer Aspire VN7-593G/Pluto_KLS, BIOS V1.04 03/30/2017
> [   12.768164] Workqueue: pm pm_runtime_work
> [   12.768166] task: ffff889bf1627040 task.stack: ffff9541013e4000
> [   12.768180] RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
> [   12.768181] RSP: 0018:ffff9541013e7b30 EFLAGS: 00010086
> [   12.768183] RAX: 000000000000001c RBX: ffff889b4cebd000 RCX: 0000000000000004
> [   12.768184] RDX: 0000000080000004 RSI: ffffffff87a2d952 RDI: 00000000ffffffff
> [   12.768186] RBP: ffff9541013e7b90 R08: 0000000000000001 R09: 000000000000039f
> [   12.768187] R10: ffffffffc05fe530 R11: 0000000000000000 R12: 0000000000000000
> [   12.768188] R13: ffff9541013e7ba4 R14: ffff889bf0426088 R15: ffff889bf0426000
> [   12.768190] FS:  0000000000000000(0000) GS:ffff889bfec00000(0000) knlGS:0000000000000000
> [   12.768191] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   12.768192] CR2: 000000edb16580b8 CR3: 000000020cc09000 CR4: 00000000003406f0
> [   12.768193] Call Trace:
> [   12.768198]  ? enqueue_task_fair+0x64/0x600
> [   12.768211]  ? drm_get_last_vbltimestamp+0x47/0x70 [drm]
> [   12.768223]  ? drm_update_vblank_count+0x65/0x240 [drm]
> [   12.768227]  ? pci_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_uevent_env+0x111/0x5e0
> [   12.768425]  ? pci_pm_runtime_resume+0xa0/0xa0
> [   12.768427]  ? rpm_callback+0x1f/0x70
> [   12.768429]  ? pci_pm_runtime_resume+0xa0/0xa0
> [   12.768431]  ? rpm_suspend+0x11f/0x640
> [   12.768441]  ? drm_fb_helper_hotplug_event+0x9a/0xe0 [drm_kms_helper]
> [   12.768447]  ? output_poll_execute+0x17b/0x1a0 [drm_kms_helper]
> [   12.768449]  ? pm_runtime_work+0x64/0xa0
> [   12.768453]  ? process_one_work+0x1db/0x410
> [   12.768456]  ? worker_thread+0x47/0x3d0
> [   12.768459]  ? process_one_work+0x410/0x410
> [   12.768461]  ? kthread+0x117/0x130
> [   12.768463]  ? kthread_create_on_node+0x40/0x40
> [   12.768466]  ? ret_from_fork+0x25/0x30
> [   12.768468] Code: 80 3d 26 f3 01 00 00 0f 85 ad fd ff ff 48 8b 43 20 48 c7 c7 31 a2 20 c0 c6 05 0e f3 01 00 01 48 8b b0 60 01 00 00 e8 75 2e ec c6 <0f> ff e9 88 fd ff ff 31 f6 44 88 55 b0 e8 38 fa ed c6 44 0f b6
> [   12.768508] ---[ end trace d9bb853af3659bd5 ]---
>
> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
> ---
>  drivers/gpu/drm/drm_vblank.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index a233a6be934a..4a21756bf2bd 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1140,8 +1140,11 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
>
>         /* Avoid redundant vblank disables without previous
>          * drm_crtc_vblank_on(). */
> -       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
> +       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || (!vblank->inmodeset &&
> +               vblank->enabled)) {
> +               DRM_DEBUG("disabling vblank on crtc %u\n", pipe);
>                 drm_vblank_disable_and_save(dev, pipe);
> +       }
>
>         wake_up(&vblank->queue);
>
> --
> 2.13.2
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
Daniel Vetter July 20, 2017, 8:13 a.m. UTC | #3
On Wed, Jul 19, 2017 at 04:10:50PM -0400, Ilia Mirkin wrote:
> I believe the solution is to not call drm_crtc_vblank_off for atomic
> modesetting in nouveau_display_fini. I think Ben's working on it.

Yes, the goal of vblank_on/off was very much to not paper over driver bugs
with clever tricks like these. If the driver cant keep track of its
vblank, something has gone wrong, and the core should _not_ fix it up.
Otherwise we're back to the old style vblank horror show.

Thanks, Daniel

> 
> On Wed, Jul 19, 2017 at 1:25 PM, Tobias Klausmann
> <tobias.johannes.klausmann@mni.thm.de> wrote:
> > mimic the behavior of vblank_disable_fn(), another caller of
> > drm_vblank_disable_and_save().
> >
> > This avoids oopsing, while trying to disable vblank on a not connected display:
> >
> > [   12.768079] WARNING: CPU: 0 PID: 274 at drivers/gpu/drm/drm_vblank.c:609 drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
> > [   12.768080] Modules linked in: bnep snd_hda_codec_hdmi rtsx_usb_sdmmc uvcvideo rtsx_usb_ms mmc_core videobuf2_vmalloc memstick videobuf2_memops videobuf2_v4l2 videobuf2_core rtsx_usb videodev btusb btrtl arc4 snd_hda_codec_realtek snd_hda_codec_generic joydev nls_iso8859_1 hid_multitouch nls_cp437 intel_rapl x86_pkg_temp_thermal intel_powerclamp vfat coretemp fat kvm_intel iTCO_wdt iTCO_vendor_support kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc aesni_intel ath10k_pci snd_hda_intel ath10k_core aes_x86_64 snd_hda_codec crypto_simd ath glue_helper cryptd snd_hda_core mac80211 snd_hwdep snd_pcm pcspkr r8169 cfg80211 mii snd_timer acer_wmi snd sparse_keymap wmi_bmof idma64 hci_uart virt_dma mei_me soundcore i2c_i801 mei btbcm shpchp intel_lpss_pci intel_pch_thermal
> > [   12.768130]  serdev btqca ucsi_acpi btintel typec_ucsi thermal typec bluetooth ecdh_generic battery ac pinctrl_sunrisepoint rfkill intel_lpss_acpi pinctrl_intel intel_lpss acpi_pad nouveau serio_raw i915 mxm_wmi ttm i2c_algo_bit drm_kms_helper xhci_pci syscopyarea sysfillrect sysimgblt xhci_hcd fb_sys_fops usbcore drm i2c_hid wmi video button sg efivarfs
> > [   12.768158] CPU: 0 PID: 274 Comm: kworker/0:2 Not tainted 4.12.0-desktop-debug-drm+ #2
> > [   12.768160] Hardware name: Acer Aspire VN7-593G/Pluto_KLS, BIOS V1.04 03/30/2017
> > [   12.768164] Workqueue: pm pm_runtime_work
> > [   12.768166] task: ffff889bf1627040 task.stack: ffff9541013e4000
> > [   12.768180] RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
> > [   12.768181] RSP: 0018:ffff9541013e7b30 EFLAGS: 00010086
> > [   12.768183] RAX: 000000000000001c RBX: ffff889b4cebd000 RCX: 0000000000000004
> > [   12.768184] RDX: 0000000080000004 RSI: ffffffff87a2d952 RDI: 00000000ffffffff
> > [   12.768186] RBP: ffff9541013e7b90 R08: 0000000000000001 R09: 000000000000039f
> > [   12.768187] R10: ffffffffc05fe530 R11: 0000000000000000 R12: 0000000000000000
> > [   12.768188] R13: ffff9541013e7ba4 R14: ffff889bf0426088 R15: ffff889bf0426000
> > [   12.768190] FS:  0000000000000000(0000) GS:ffff889bfec00000(0000) knlGS:0000000000000000
> > [   12.768191] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   12.768192] CR2: 000000edb16580b8 CR3: 000000020cc09000 CR4: 00000000003406f0
> > [   12.768193] Call Trace:
> > [   12.768198]  ? enqueue_task_fair+0x64/0x600
> > [   12.768211]  ? drm_get_last_vbltimestamp+0x47/0x70 [drm]
> > [   12.768223]  ? drm_update_vblank_count+0x65/0x240 [drm]
> > [   12.768227]  ? pci_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_uevent_env+0x111/0x5e0
> > [   12.768425]  ? pci_pm_runtime_resume+0xa0/0xa0
> > [   12.768427]  ? rpm_callback+0x1f/0x70
> > [   12.768429]  ? pci_pm_runtime_resume+0xa0/0xa0
> > [   12.768431]  ? rpm_suspend+0x11f/0x640
> > [   12.768441]  ? drm_fb_helper_hotplug_event+0x9a/0xe0 [drm_kms_helper]
> > [   12.768447]  ? output_poll_execute+0x17b/0x1a0 [drm_kms_helper]
> > [   12.768449]  ? pm_runtime_work+0x64/0xa0
> > [   12.768453]  ? process_one_work+0x1db/0x410
> > [   12.768456]  ? worker_thread+0x47/0x3d0
> > [   12.768459]  ? process_one_work+0x410/0x410
> > [   12.768461]  ? kthread+0x117/0x130
> > [   12.768463]  ? kthread_create_on_node+0x40/0x40
> > [   12.768466]  ? ret_from_fork+0x25/0x30
> > [   12.768468] Code: 80 3d 26 f3 01 00 00 0f 85 ad fd ff ff 48 8b 43 20 48 c7 c7 31 a2 20 c0 c6 05 0e f3 01 00 01 48 8b b0 60 01 00 00 e8 75 2e ec c6 <0f> ff e9 88 fd ff ff 31 f6 44 88 55 b0 e8 38 fa ed c6 44 0f b6
> > [   12.768508] ---[ end trace d9bb853af3659bd5 ]---
> >
> > Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
> > ---
> >  drivers/gpu/drm/drm_vblank.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index a233a6be934a..4a21756bf2bd 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -1140,8 +1140,11 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
> >
> >         /* Avoid redundant vblank disables without previous
> >          * drm_crtc_vblank_on(). */
> > -       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
> > +       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || (!vblank->inmodeset &&
> > +               vblank->enabled)) {
> > +               DRM_DEBUG("disabling vblank on crtc %u\n", pipe);
> >                 drm_vblank_disable_and_save(dev, pipe);
> > +       }
> >
> >         wake_up(&vblank->queue);
> >
> > --
> > 2.13.2
> >
> > _______________________________________________
> > Nouveau mailing list
> > Nouveau@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
Tobias Klausmann July 20, 2017, 9:58 p.m. UTC | #4
Mh ok,

paper over in nouveau_display_fini until Ben comes up with a better idea
then?!


Greetings,

Tobias


On 7/20/17 10:13 AM, Daniel Vetter wrote:
> On Wed, Jul 19, 2017 at 04:10:50PM -0400, Ilia Mirkin wrote:
>> I believe the solution is to not call drm_crtc_vblank_off for atomic
>> modesetting in nouveau_display_fini. I think Ben's working on it.
> Yes, the goal of vblank_on/off was very much to not paper over driver bugs
> with clever tricks like these. If the driver cant keep track of its
> vblank, something has gone wrong, and the core should _not_ fix it up.
> Otherwise we're back to the old style vblank horror show.
>
> Thanks, Daniel
>
>> On Wed, Jul 19, 2017 at 1:25 PM, Tobias Klausmann
>> <tobias.johannes.klausmann@mni.thm.de> wrote:
>>> mimic the behavior of vblank_disable_fn(), another caller of
>>> drm_vblank_disable_and_save().
>>>
>>> This avoids oopsing, while trying to disable vblank on a not connected display:
>>>
>>> [   12.768079] WARNING: CPU: 0 PID: 274 at drivers/gpu/drm/drm_vblank.c:609 drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
>>> [   12.768080] Modules linked in: bnep snd_hda_codec_hdmi rtsx_usb_sdmmc uvcvideo rtsx_usb_ms mmc_core videobuf2_vmalloc memstick videobuf2_memops videobuf2_v4l2 videobuf2_core rtsx_usb videodev btusb btrtl arc4 snd_hda_codec_realtek snd_hda_codec_generic joydev nls_iso8859_1 hid_multitouch nls_cp437 intel_rapl x86_pkg_temp_thermal intel_powerclamp vfat coretemp fat kvm_intel iTCO_wdt iTCO_vendor_support kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc aesni_intel ath10k_pci snd_hda_intel ath10k_core aes_x86_64 snd_hda_codec crypto_simd ath glue_helper cryptd snd_hda_core mac80211 snd_hwdep snd_pcm pcspkr r8169 cfg80211 mii snd_timer acer_wmi snd sparse_keymap wmi_bmof idma64 hci_uart virt_dma mei_me soundcore i2c_i801 mei btbcm shpchp intel_lpss_pci intel_pch_thermal
>>> [   12.768130]  serdev btqca ucsi_acpi btintel typec_ucsi thermal typec bluetooth ecdh_generic battery ac pinctrl_sunrisepoint rfkill intel_lpss_acpi pinctrl_intel intel_lpss acpi_pad nouveau serio_raw i915 mxm_wmi ttm i2c_algo_bit drm_kms_helper xhci_pci syscopyarea sysfillrect sysimgblt xhci_hcd fb_sys_fops usbcore drm i2c_hid wmi video button sg efivarfs
>>> [   12.768158] CPU: 0 PID: 274 Comm: kworker/0:2 Not tainted 4.12.0-desktop-debug-drm+ #2
>>> [   12.768160] Hardware name: Acer Aspire VN7-593G/Pluto_KLS, BIOS V1.04 03/30/2017
>>> [   12.768164] Workqueue: pm pm_runtime_work
>>> [   12.768166] task: ffff889bf1627040 task.stack: ffff9541013e4000
>>> [   12.768180] RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
>>> [   12.768181] RSP: 0018:ffff9541013e7b30 EFLAGS: 00010086
>>> [   12.768183] RAX: 000000000000001c RBX: ffff889b4cebd000 RCX: 0000000000000004
>>> [   12.768184] RDX: 0000000080000004 RSI: ffffffff87a2d952 RDI: 00000000ffffffff
>>> [   12.768186] RBP: ffff9541013e7b90 R08: 0000000000000001 R09: 000000000000039f
>>> [   12.768187] R10: ffffffffc05fe530 R11: 0000000000000000 R12: 0000000000000000
>>> [   12.768188] R13: ffff9541013e7ba4 R14: ffff889bf0426088 R15: ffff889bf0426000
>>> [   12.768190] FS:  0000000000000000(0000) GS:ffff889bfec00000(0000) knlGS:0000000000000000
>>> [   12.768191] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [   12.768192] CR2: 000000edb16580b8 CR3: 000000020cc09000 CR4: 00000000003406f0
>>> [   12.768193] Call Trace:
>>> [   12.768198]  ? enqueue_task_fair+0x64/0x600
>>> [   12.768211]  ? drm_get_last_vbltimestamp+0x47/0x70 [drm]
>>> [   12.768223]  ? drm_update_vblank_count+0x65/0x240 [drm]
>>> [   12.768227]  ? pci_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_uevent_env+0x111/0x5e0
>>> [   12.768425]  ? pci_pm_runtime_resume+0xa0/0xa0
>>> [   12.768427]  ? rpm_callback+0x1f/0x70
>>> [   12.768429]  ? pci_pm_runtime_resume+0xa0/0xa0
>>> [   12.768431]  ? rpm_suspend+0x11f/0x640
>>> [   12.768441]  ? drm_fb_helper_hotplug_event+0x9a/0xe0 [drm_kms_helper]
>>> [   12.768447]  ? output_poll_execute+0x17b/0x1a0 [drm_kms_helper]
>>> [   12.768449]  ? pm_runtime_work+0x64/0xa0
>>> [   12.768453]  ? process_one_work+0x1db/0x410
>>> [   12.768456]  ? worker_thread+0x47/0x3d0
>>> [   12.768459]  ? process_one_work+0x410/0x410
>>> [   12.768461]  ? kthread+0x117/0x130
>>> [   12.768463]  ? kthread_create_on_node+0x40/0x40
>>> [   12.768466]  ? ret_from_fork+0x25/0x30
>>> [   12.768468] Code: 80 3d 26 f3 01 00 00 0f 85 ad fd ff ff 48 8b 43 20 48 c7 c7 31 a2 20 c0 c6 05 0e f3 01 00 01 48 8b b0 60 01 00 00 e8 75 2e ec c6 <0f> ff e9 88 fd ff ff 31 f6 44 88 55 b0 e8 38 fa ed c6 44 0f b6
>>> [   12.768508] ---[ end trace d9bb853af3659bd5 ]---
>>>
>>> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
>>> ---
>>>  drivers/gpu/drm/drm_vblank.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>>> index a233a6be934a..4a21756bf2bd 100644
>>> --- a/drivers/gpu/drm/drm_vblank.c
>>> +++ b/drivers/gpu/drm/drm_vblank.c
>>> @@ -1140,8 +1140,11 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
>>>
>>>         /* Avoid redundant vblank disables without previous
>>>          * drm_crtc_vblank_on(). */
>>> -       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
>>> +       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || (!vblank->inmodeset &&
>>> +               vblank->enabled)) {
>>> +               DRM_DEBUG("disabling vblank on crtc %u\n", pipe);
>>>                 drm_vblank_disable_and_save(dev, pipe);
>>> +       }
>>>
>>>         wake_up(&vblank->queue);
>>>
>>> --
>>> 2.13.2
>>>
>>> _______________________________________________
>>> Nouveau mailing list
>>> Nouveau@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
Daniel Vetter July 20, 2017, 10:05 p.m. UTC | #5
On Thu, Jul 20, 2017 at 11:58 PM, Tobias Klausmann
<tobias.johannes.klausmann@mni.thm.de> wrote:
> Mh ok,
>
> paper over in nouveau_display_fini until Ben comes up with a better idea
> then?!

No paper needed, just don't call drm_vblank_off for the atomic case.
Not sure why that patch isn't landed yet, it should be simple.
-Daniel

>
> Greetings,
>
> Tobias
>
>
> On 7/20/17 10:13 AM, Daniel Vetter wrote:
>> On Wed, Jul 19, 2017 at 04:10:50PM -0400, Ilia Mirkin wrote:
>>> I believe the solution is to not call drm_crtc_vblank_off for atomic
>>> modesetting in nouveau_display_fini. I think Ben's working on it.
>> Yes, the goal of vblank_on/off was very much to not paper over driver bugs
>> with clever tricks like these. If the driver cant keep track of its
>> vblank, something has gone wrong, and the core should _not_ fix it up.
>> Otherwise we're back to the old style vblank horror show.
>>
>> Thanks, Daniel
>>
>>> On Wed, Jul 19, 2017 at 1:25 PM, Tobias Klausmann
>>> <tobias.johannes.klausmann@mni.thm.de> wrote:
>>>> mimic the behavior of vblank_disable_fn(), another caller of
>>>> drm_vblank_disable_and_save().
>>>>
>>>> This avoids oopsing, while trying to disable vblank on a not connected display:
>>>>
>>>> [   12.768079] WARNING: CPU: 0 PID: 274 at drivers/gpu/drm/drm_vblank.c:609 drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
>>>> [   12.768080] Modules linked in: bnep snd_hda_codec_hdmi rtsx_usb_sdmmc uvcvideo rtsx_usb_ms mmc_core videobuf2_vmalloc memstick videobuf2_memops videobuf2_v4l2 videobuf2_core rtsx_usb videodev btusb btrtl arc4 snd_hda_codec_realtek snd_hda_codec_generic joydev nls_iso8859_1 hid_multitouch nls_cp437 intel_rapl x86_pkg_temp_thermal intel_powerclamp vfat coretemp fat kvm_intel iTCO_wdt iTCO_vendor_support kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc aesni_intel ath10k_pci snd_hda_intel ath10k_core aes_x86_64 snd_hda_codec crypto_simd ath glue_helper cryptd snd_hda_core mac80211 snd_hwdep snd_pcm pcspkr r8169 cfg80211 mii snd_timer acer_wmi snd sparse_keymap wmi_bmof idma64 hci_uart virt_dma mei_me soundcore i2c_i801 mei btbcm shpchp intel_lpss_pci intel_pch_thermal
>>>> [   12.768130]  serdev btqca ucsi_acpi btintel typec_ucsi thermal typec bluetooth ecdh_generic battery ac pinctrl_sunrisepoint rfkill intel_lpss_acpi pinctrl_intel intel_lpss acpi_pad nouveau serio_raw i915 mxm_wmi ttm i2c_algo_bit drm_kms_helper xhci_pci syscopyarea sysfillrect sysimgblt xhci_hcd fb_sys_fops usbcore drm i2c_hid wmi video button sg efivarfs
>>>> [   12.768158] CPU: 0 PID: 274 Comm: kworker/0:2 Not tainted 4.12.0-desktop-debug-drm+ #2
>>>> [   12.768160] Hardware name: Acer Aspire VN7-593G/Pluto_KLS, BIOS V1.04 03/30/2017
>>>> [   12.768164] Workqueue: pm pm_runtime_work
>>>> [   12.768166] task: ffff889bf1627040 task.stack: ffff9541013e4000
>>>> [   12.768180] RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
>>>> [   12.768181] RSP: 0018:ffff9541013e7b30 EFLAGS: 00010086
>>>> [   12.768183] RAX: 000000000000001c RBX: ffff889b4cebd000 RCX: 0000000000000004
>>>> [   12.768184] RDX: 0000000080000004 RSI: ffffffff87a2d952 RDI: 00000000ffffffff
>>>> [   12.768186] RBP: ffff9541013e7b90 R08: 0000000000000001 R09: 000000000000039f
>>>> [   12.768187] R10: ffffffffc05fe530 R11: 0000000000000000 R12: 0000000000000000
>>>> [   12.768188] R13: ffff9541013e7ba4 R14: ffff889bf0426088 R15: ffff889bf0426000
>>>> [   12.768190] FS:  0000000000000000(0000) GS:ffff889bfec00000(0000) knlGS:0000000000000000
>>>> [   12.768191] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [   12.768192] CR2: 000000edb16580b8 CR3: 000000020cc09000 CR4: 00000000003406f0
>>>> [   12.768193] Call Trace:
>>>> [   12.768198]  ? enqueue_task_fair+0x64/0x600
>>>> [   12.768211]  ? drm_get_last_vbltimestamp+0x47/0x70 [drm]
>>>> [   12.768223]  ? drm_update_vblank_count+0x65/0x240 [drm]
>>>> [   12.768227]  ? pci_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_uevent_env+0x111/0x5e0
>>>> [   12.768425]  ? pci_pm_runtime_resume+0xa0/0xa0
>>>> [   12.768427]  ? rpm_callback+0x1f/0x70
>>>> [   12.768429]  ? pci_pm_runtime_resume+0xa0/0xa0
>>>> [   12.768431]  ? rpm_suspend+0x11f/0x640
>>>> [   12.768441]  ? drm_fb_helper_hotplug_event+0x9a/0xe0 [drm_kms_helper]
>>>> [   12.768447]  ? output_poll_execute+0x17b/0x1a0 [drm_kms_helper]
>>>> [   12.768449]  ? pm_runtime_work+0x64/0xa0
>>>> [   12.768453]  ? process_one_work+0x1db/0x410
>>>> [   12.768456]  ? worker_thread+0x47/0x3d0
>>>> [   12.768459]  ? process_one_work+0x410/0x410
>>>> [   12.768461]  ? kthread+0x117/0x130
>>>> [   12.768463]  ? kthread_create_on_node+0x40/0x40
>>>> [   12.768466]  ? ret_from_fork+0x25/0x30
>>>> [   12.768468] Code: 80 3d 26 f3 01 00 00 0f 85 ad fd ff ff 48 8b 43 20 48 c7 c7 31 a2 20 c0 c6 05 0e f3 01 00 01 48 8b b0 60 01 00 00 e8 75 2e ec c6 <0f> ff e9 88 fd ff ff 31 f6 44 88 55 b0 e8 38 fa ed c6 44 0f b6
>>>> [   12.768508] ---[ end trace d9bb853af3659bd5 ]---
>>>>
>>>> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
>>>> ---
>>>>  drivers/gpu/drm/drm_vblank.c | 5 ++++-
>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>>>> index a233a6be934a..4a21756bf2bd 100644
>>>> --- a/drivers/gpu/drm/drm_vblank.c
>>>> +++ b/drivers/gpu/drm/drm_vblank.c
>>>> @@ -1140,8 +1140,11 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
>>>>
>>>>         /* Avoid redundant vblank disables without previous
>>>>          * drm_crtc_vblank_on(). */
>>>> -       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
>>>> +       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || (!vblank->inmodeset &&
>>>> +               vblank->enabled)) {
>>>> +               DRM_DEBUG("disabling vblank on crtc %u\n", pipe);
>>>>                 drm_vblank_disable_and_save(dev, pipe);
>>>> +       }
>>>>
>>>>         wake_up(&vblank->queue);
>>>>
>>>> --
>>>> 2.13.2
>>>>
>>>> _______________________________________________
>>>> Nouveau mailing list
>>>> Nouveau@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
>>> _______________________________________________
>>> Nouveau mailing list
>>> Nouveau@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
diff mbox

Patch

--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -323,6 +323,14 @@  void drm_vblank_disable_and_save(struct
 	spin_lock_irqsave(&dev->vblank_time_lock, irqflags);
 
 	/*
+	 * Always update the count and timestamp to maintain the
+	 * appearance that the counter has been ticking all along until
+	 * this time. This makes the count account for the entire time
+	 * between drm_crtc_vblank_on() and drm_crtc_vblank_off().
+	 */
+	drm_update_vblank_count(dev, pipe, false);
+
+	/*
 	 * Only disable vblank interrupts if they're enabled. This avoids
 	 * calling the ->disable_vblank() operation in atomic context with the
 	 * hardware potentially runtime suspended.
@@ -332,14 +340,6 @@  void drm_vblank_disable_and_save(struct
 		vblank->enabled = false;
 	}
 
-	/*
-	 * Always update the count and timestamp to maintain the
-	 * appearance that the counter has been ticking all along until
-	 * this time. This makes the count account for the entire time
-	 * between drm_crtc_vblank_on() and drm_crtc_vblank_off().
-	 */
-	drm_update_vblank_count(dev, pipe, false);
-
 	spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
 }
 
@@ -605,7 +605,7 @@  bool drm_calc_vbltimestamp_from_scanoutp
 	 */
 	if (mode->crtc_clock == 0) {
 		DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe);
-		WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev));
+		WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev) && vblank->enabled);
 
 		return false;
 	}