* [PATCH] drm/omap: fix incorrect lock state
@ 2020-08-19 10:30 Tomi Valkeinen
2020-08-19 12:17 ` Laurent Pinchart
0 siblings, 1 reply; 2+ messages in thread
From: Tomi Valkeinen @ 2020-08-19 10:30 UTC (permalink / raw)
To: dri-devel, Laurent Pinchart; +Cc: Tomi Valkeinen
After commit 92cc68e35863c1c61c449efa2b2daef6e9926048 ("drm/vblank: Use
spin_(un)lock_irq() in drm_crtc_vblank_on()") omapdrm locking is broken:
WARNING: inconsistent lock state
5.8.0-rc2-00483-g92cc68e35863 #13 Tainted: G W
--------------------------------
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
ea98222c (&dev->event_lock#2){?.+.}-{2:2}, at: drm_handle_vblank+0x4c/0x520 [drm]
{HARDIRQ-ON-W} state was registered at:
trace_hardirqs_on+0x9c/0x1ec
_raw_spin_unlock_irq+0x20/0x58
omap_crtc_atomic_enable+0x54/0xa0 [omapdrm]
drm_atomic_helper_commit_modeset_enables+0x218/0x270 [drm_kms_helper]
omap_atomic_commit_tail+0x48/0xc4 [omapdrm]
commit_tail+0x9c/0x190 [drm_kms_helper]
drm_atomic_helper_commit+0x154/0x188 [drm_kms_helper]
drm_client_modeset_commit_atomic+0x228/0x268 [drm]
drm_client_modeset_commit_locked+0x60/0x1d0 [drm]
drm_client_modeset_commit+0x24/0x40 [drm]
drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa8 [drm_kms_helper]
drm_fb_helper_set_par+0x2c/0x5c [drm_kms_helper]
drm_fb_helper_hotplug_event.part.0+0xa0/0xbc [drm_kms_helper]
drm_kms_helper_hotplug_event+0x24/0x30 [drm_kms_helper]
output_poll_execute+0x1a8/0x1c0 [drm_kms_helper]
process_one_work+0x268/0x800
worker_thread+0x30/0x4e0
kthread+0x164/0x190
ret_from_fork+0x14/0x20
The reason for this is that omapdrm calls drm_crtc_vblank_on() while
holding event_lock taken with spin_lock_irq().
It is not clear why drm_crtc_vblank_on() and drm_crtc_vblank_get() are
called while holding event_lock. I don't see any problem with moving
those calls outside the lock, which is what this patch does.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/gpu/drm/omapdrm/omap_crtc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 6d40914675da..328a4a74f534 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -451,11 +451,12 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc,
if (omap_state->manually_updated)
return;
- spin_lock_irq(&crtc->dev->event_lock);
drm_crtc_vblank_on(crtc);
+
ret = drm_crtc_vblank_get(crtc);
WARN_ON(ret != 0);
+ spin_lock_irq(&crtc->dev->event_lock);
omap_crtc_arm_event(crtc);
spin_unlock_irq(&crtc->dev->event_lock);
}
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/omap: fix incorrect lock state
2020-08-19 10:30 [PATCH] drm/omap: fix incorrect lock state Tomi Valkeinen
@ 2020-08-19 12:17 ` Laurent Pinchart
0 siblings, 0 replies; 2+ messages in thread
From: Laurent Pinchart @ 2020-08-19 12:17 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: dri-devel
Hi Tomi,
Thank you for the patch.
On Wed, Aug 19, 2020 at 01:30:21PM +0300, Tomi Valkeinen wrote:
> After commit 92cc68e35863c1c61c449efa2b2daef6e9926048 ("drm/vblank: Use
> spin_(un)lock_irq() in drm_crtc_vblank_on()") omapdrm locking is broken:
>
> WARNING: inconsistent lock state
> 5.8.0-rc2-00483-g92cc68e35863 #13 Tainted: G W
> --------------------------------
> inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
> swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
> ea98222c (&dev->event_lock#2){?.+.}-{2:2}, at: drm_handle_vblank+0x4c/0x520 [drm]
> {HARDIRQ-ON-W} state was registered at:
> trace_hardirqs_on+0x9c/0x1ec
> _raw_spin_unlock_irq+0x20/0x58
> omap_crtc_atomic_enable+0x54/0xa0 [omapdrm]
> drm_atomic_helper_commit_modeset_enables+0x218/0x270 [drm_kms_helper]
> omap_atomic_commit_tail+0x48/0xc4 [omapdrm]
> commit_tail+0x9c/0x190 [drm_kms_helper]
> drm_atomic_helper_commit+0x154/0x188 [drm_kms_helper]
> drm_client_modeset_commit_atomic+0x228/0x268 [drm]
> drm_client_modeset_commit_locked+0x60/0x1d0 [drm]
> drm_client_modeset_commit+0x24/0x40 [drm]
> drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa8 [drm_kms_helper]
> drm_fb_helper_set_par+0x2c/0x5c [drm_kms_helper]
> drm_fb_helper_hotplug_event.part.0+0xa0/0xbc [drm_kms_helper]
> drm_kms_helper_hotplug_event+0x24/0x30 [drm_kms_helper]
> output_poll_execute+0x1a8/0x1c0 [drm_kms_helper]
> process_one_work+0x268/0x800
> worker_thread+0x30/0x4e0
> kthread+0x164/0x190
> ret_from_fork+0x14/0x20
>
> The reason for this is that omapdrm calls drm_crtc_vblank_on() while
> holding event_lock taken with spin_lock_irq().
>
> It is not clear why drm_crtc_vblank_on() and drm_crtc_vblank_get() are
> called while holding event_lock. I don't see any problem with moving
> those calls outside the lock, which is what this patch does.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
I've had a quick look at other callers of drm_crtc_vblank_on() and none
of them call it with a spinlock held in the same function. I have
however only checked locally, not up the call stack.
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 6d40914675da..328a4a74f534 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -451,11 +451,12 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc,
> if (omap_state->manually_updated)
> return;
>
> - spin_lock_irq(&crtc->dev->event_lock);
> drm_crtc_vblank_on(crtc);
> +
> ret = drm_crtc_vblank_get(crtc);
> WARN_ON(ret != 0);
>
> + spin_lock_irq(&crtc->dev->event_lock);
> omap_crtc_arm_event(crtc);
> spin_unlock_irq(&crtc->dev->event_lock);
> }
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-08-19 12:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-19 10:30 [PATCH] drm/omap: fix incorrect lock state Tomi Valkeinen
2020-08-19 12:17 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.