drm_vblank_off() requires callers to hold the event_lock, while itself locking vbl_time and vblank_time_lock. This defines a dependency chain that conflicts with the one in drm_handle_vblank() where we first lock vblank_time_lock and then the event_lock. Fix this by reversing the locking order in drm_handle_vblank().
This should've triggered a lockdep warning in the exynos driver, the rest of the drivers were ok, since they are not using drm_vblank_off(), or as in the case of the intel driver were not holding the event_lock when calling drm_vblank_off(). This latter issue is addressed in the next patch.
Signed-off-by: Imre Deak imre.deak@intel.com --- drivers/gpu/drm/drm_irq.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)
Tested with i915, the rest of the drivers should be checked with lockdep enabled.
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 3a3d0ce..2193d4a 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1236,17 +1236,21 @@ done: return ret; }
+/** + * drm_handle_vblank_events - send pending vblank events + * @dev: DRM device + * @crtc: crtc where the vblank(s) happened + * + * Must be called with @dev->event_lock held. + */ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) { struct drm_pending_vblank_event *e, *t; struct timeval now; - unsigned long flags; unsigned int seq;
seq = drm_vblank_count_and_time(dev, crtc, &now);
- spin_lock_irqsave(&dev->event_lock, flags); - list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) { if (e->pipe != crtc) continue; @@ -1266,8 +1270,6 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) e->event.sequence); }
- spin_unlock_irqrestore(&dev->event_lock, flags); - trace_drm_vblank_event(crtc, seq); }
@@ -1285,21 +1287,22 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) s64 diff_ns; struct timeval tvblank; unsigned long irqflags; + bool ret = false;
if (!dev->num_crtcs) return false;
+ spin_lock_irqsave(&dev->event_lock, irqflags); + /* Need timestamp lock to prevent concurrent execution with * vblank enable/disable, as this would cause inconsistent * or corrupted timestamps and vblank counts. */ - spin_lock_irqsave(&dev->vblank_time_lock, irqflags); + spin_lock(&dev->vblank_time_lock);
/* Vblank irq handling disabled. Nothing to do. */ - if (!dev->vblank_enabled[crtc]) { - spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); - return false; - } + if (!dev->vblank_enabled[crtc]) + goto unlock_ret;
/* Fetch corresponding timestamp for this vblank interval from * driver and store it in proper slot of timestamp ringbuffer. @@ -1340,7 +1343,12 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) DRM_WAKEUP(&dev->vbl_queue[crtc]); drm_handle_vblank_events(dev, crtc);
- spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); - return true; + ret = true; + +unlock_ret: + spin_unlock(&dev->vblank_time_lock); + spin_unlock_irqrestore(&dev->event_lock, irqflags); + + return ret; } EXPORT_SYMBOL(drm_handle_vblank);