[PATCH] drm/vblank: WARN_ON nested use of drm_vblank_on/off

[Tomeu Vizoso]

On 22 June 2015 at 14:02, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> We should never nest these since in theory kms drivers should know
> when a pipe is on/off and call the corresponding enable/disable
> functions for the vblank helper code only once. But for historical
> reasons (the shared-with-ums version of this code in modeset_pre/post
> needed to be able to cope with silly userspace that lost track of
> things) we still have this bit of "robustness" around.
>
> Enforce this with a WARN_ON, preparing to eventually rip out this
> special handling.

Hello,

just wanted to mention that this warning triggered on tegra124 and
that I find it a bit disruptive:

WARNING: CPU: 0 PID: 52 at drivers/gpu/drm/drm_irq.c:1323
drm_vblank_on+0x10c/0x130()
Modules linked in:
CPU: 0 PID: 52 Comm: kworker/0:1 Not tainted
4.1.0-next-20150625ccu-00024-gd8cbda3 #2295
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
Workqueue: events output_poll_execute
[<c021b12c>] (unwind_backtrace) from [<c0215388>] (show_stack+0x20/0x24)
[<c0215388>] (show_stack) from [<c0a9637c>] (dump_stack+0x8c/0x9c)
[<c0a9637c>] (dump_stack) from [<c0256b58>] (warn_slowpath_common+0x94/0xc4)
[<c0256b58>] (warn_slowpath_common) from [<c0256c44>]
(warn_slowpath_null+0x2c/0x34)
[<c0256c44>] (warn_slowpath_null) from [<c0686d38>] (drm_vblank_on+0x10c/0x130)
[<c0686d38>] (drm_vblank_on) from [<c0686d84>] (drm_crtc_vblank_on+0x28/0x2c)
[<c0686d84>] (drm_crtc_vblank_on) from [<c06cd518>]
(tegra_crtc_commit+0x18/0x1c)
[<c06cd518>] (tegra_crtc_commit) from [<c067b014>]
(drm_atomic_helper_commit_modeset_enables+0xa0/0x198)
[<c067b014>] (drm_atomic_helper_commit_modeset_enables) from
[<c06cb068>] (tegra_atomic_complete.isra.1+0x38/0x5c)
[<c06cb068>] (tegra_atomic_complete.isra.1) from [<c06cb0f4>]
(tegra_atomic_commit+0x68/0x98)
[<c06cb0f4>] (tegra_atomic_commit) from [<c06a1690>]
(drm_atomic_commit+0x54/0x74)
[<c06a1690>] (drm_atomic_commit) from [<c067b6ec>]
(drm_atomic_helper_set_config+0x1b4/0x414)
[<c067b6ec>] (drm_atomic_helper_set_config) from [<c068f43c>]
(drm_mode_set_config_internal+0x68/0xe4)
[<c068f43c>] (drm_mode_set_config_internal) from [<c067d34c>]
(restore_fbdev_mode+0xe8/0x108)
[<c067d34c>] (restore_fbdev_mode) from [<c067f390>]
(drm_fb_helper_restore_fbdev_mode_unlocked+0x2c/0x68)
[<c067f390>] (drm_fb_helper_restore_fbdev_mode_unlocked) from
[<c067f3f4>] (drm_fb_helper_set_par+0x28/0x44)
[<c067f3f4>] (drm_fb_helper_set_par) from [<c067f328>]
(drm_fb_helper_hotplug_event+0xcc/0x108)
[<c067f328>] (drm_fb_helper_hotplug_event) from [<c06cce98>]
(tegra_fb_output_poll_changed+0x28/0x2c)
[<c06cce98>] (tegra_fb_output_poll_changed) from [<c0674228>]
(drm_kms_helper_hotplug_event+0x34/0x38)
[<c0674228>] (drm_kms_helper_hotplug_event) from [<c06743dc>]
(output_poll_execute+0x158/0x198)
[<c06743dc>] (output_poll_execute) from [<c026d830>]
(process_one_work+0x154/0x48c)
[<c026d830>] (process_one_work) from [<c026dbbc>] (worker_thread+0x54/0x568)
[<c026dbbc>] (worker_thread) from [<c02739ec>] (kthread+0xec/0x104)
[<c02739ec>] (kthread) from [<c0211228>] (ret_from_fork+0x14/0x2c)


Regards,

Tomeu

> Cc: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu at intel.com>
> Cc: Gaurav K Singh <gaurav.k.singh at intel.com>
> Cc: Michel Dänzer <michel.daenzer at amd.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/drm_irq.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index f9cc68fbd2a3..3819465abe22 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1219,6 +1219,8 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
>         vblank_disable_and_save(dev, crtc);
>         wake_up(&vblank->queue);
>
> +       WARN_ON(vblank->inmodeset);
> +
>         /*
>          * Prevent subsequent drm_vblank_get() from re-enabling
>          * the vblank interrupt by bumping the refcount.
> @@ -1318,6 +1320,8 @@ void drm_vblank_on(struct drm_device *dev, int crtc)
>                 return;
>
>         spin_lock_irqsave(&dev->vbl_lock, irqflags);
> +       WARN_ON(!vblank->inmodeset);
> +
>         /* Drop our private "prevent drm_vblank_get" refcount */
>         if (vblank->inmodeset) {
>                 atomic_dec(&vblank->refcount);
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel