Return -EDEADLK when trying to cancel a running hrtimer as part of enable_vblank(). Reacquire vblank_time_lock and try again. The vblank helper drm_vblank_disable_and_save() and the vblank IRQ handler hold the spin lock vblank_time_lock at some point. This can result in a deadlock if vblank timer's disable code in drm_crtc_vblank_cancel_timer() calls hrtimer_cancel() directly: the helper wais for the timer function to complete and the timer function waits for the helper to release vblank.time_lock. drm_crtc_vblank_cancel_timer() works around this problem by calling hrtimer_try_to_cancel() instead. For re-enabling the vblank timer, drm_crtc_vblank_start_timer() must wait for the vblank timer to stop. It runs into the same deadlock problem while waiting for the timer function to complete. The solution to this problem is to release and reacquire the spin lock vblank_time_lock if hrtimer_try_to_cancel() signals a running timer function. Releasing the lock allows for the timer function to make progress. Signal the possible deadlock with -EDEADLK and perform the necessary steps. No other driver uses -EDEADLK in their enable_vblank, so the change should not interfere. Tested with bochs on qemu. Closes: https://lore.kernel.org/dri-devel/20260914111129.300691F00898@smtp.kernel.org/ Signed-off-by: Thomas Zimmermann Fixes: 74afeb812850 ("drm/vblank: Add vblank timer") Cc: Thomas Zimmermann Cc: Louis Chauvet Cc: Javier Martinez Canillas Cc: David Airlie Cc: Simona Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: dri-devel@lists.freedesktop.org Cc: # v6.19+ --- I also tried to move hrtimer_cancel() out of the critical section. The result was fairly fragile. So returning an error and re-acquiring the lock was the easier solution. --- drivers/gpu/drm/drm_vblank.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 5162250e8813..135efde86298 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -1182,10 +1182,12 @@ static int __enable_vblank(struct drm_device *dev, unsigned int pipe) static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe) { struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe); - int ret = 0; + int ret; assert_spin_locked(&dev->vbl_lock); +retry: + ret = 0; spin_lock(&dev->vblank_time_lock); if (!vblank->enabled) { @@ -1199,7 +1201,11 @@ static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe) ret = __enable_vblank(dev, pipe); drm_dbg_core(dev, "enabling vblank on crtc %u, ret: %d\n", pipe, ret); - if (ret) { + if (ret == -EDEADLK) { + spin_unlock(&dev->vblank_time_lock); + cpu_relax(); + goto retry; + } else if (ret) { atomic_dec(&vblank->refcount); } else { drm_update_vblank_count(dev, pipe, 0); @@ -2232,11 +2238,14 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc) CLOCK_MONOTONIC, HRTIMER_MODE_REL); } else { /* - * Timer should not be active. If it is, wait for the - * previous cancel operations to finish. + * Timer should not be running. If it is, signal cancellation + * again and restart to release locks. */ - while (hrtimer_active(&vtimer->timer)) - hrtimer_try_to_cancel(&vtimer->timer); + if (hrtimer_try_to_cancel(&vtimer->timer) == -1) { + scoped_guard(spinlock_irqsave, &vtimer->interval_lock) + vtimer->interval = 0; + return -EDEADLK; + } } drm_calc_timestamping_constants(crtc, &crtc->mode); -- 2.56.0