Commit 56cc279b authored by Ville Syrjälä's avatar Ville Syrjälä Committed by Daniel Vetter

drm: Fix deadlock between event_lock and vbl_lock/vblank_time_lock

Currently both drm_irq.c and several drivers call drm_vblank_put()
while holding event_lock. Now that drm_vblank_put() can disable the
vblank interrupt directly it may need to grab vbl_lock and
vblank_time_lock. That causes deadlocks since we take the locks
in the opposite order in two places in drm_irq.c. So let's make
sure the locking order is always event_lock->vbl_lock->vblank_time_lock.

In drm_vblank_off() pull up event_lock from underneath vbl_lock. Hold
the event_lock across the whole operation to make sure we only send
out the events that were on the queue when we disabled the interrupt,
and not ones that got added just after (assuming drm_vblank_on() already
managed to get called somewhere between).

To sort the other deadlock pull the event_lock out from
drm_handle_vblank_events() into drm_handle_vblank() to be taken outside
vblank_time_lock. Add the appropriate assert_spin_locked() to
drm_handle_vblank_events().
Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 8a51d5be
...@@ -1037,14 +1037,25 @@ void drm_vblank_off(struct drm_device *dev, int crtc) ...@@ -1037,14 +1037,25 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
unsigned long irqflags; unsigned long irqflags;
unsigned int seq; unsigned int seq;
spin_lock_irqsave(&dev->vbl_lock, irqflags); spin_lock_irqsave(&dev->event_lock, irqflags);
spin_lock(&dev->vbl_lock);
vblank_disable_and_save(dev, crtc); vblank_disable_and_save(dev, crtc);
wake_up(&vblank->queue); wake_up(&vblank->queue);
/*
* Prevent subsequent drm_vblank_get() from re-enabling
* the vblank interrupt by bumping the refcount.
*/
if (!vblank->inmodeset) {
atomic_inc(&vblank->refcount);
vblank->inmodeset = 1;
}
spin_unlock(&dev->vbl_lock);
/* Send any queued vblank events, lest the natives grow disquiet */ /* Send any queued vblank events, lest the natives grow disquiet */
seq = drm_vblank_count_and_time(dev, crtc, &now); seq = drm_vblank_count_and_time(dev, crtc, &now);
spin_lock(&dev->event_lock);
list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) { list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
if (e->pipe != crtc) if (e->pipe != crtc)
continue; continue;
...@@ -1055,18 +1066,7 @@ void drm_vblank_off(struct drm_device *dev, int crtc) ...@@ -1055,18 +1066,7 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
drm_vblank_put(dev, e->pipe); drm_vblank_put(dev, e->pipe);
send_vblank_event(dev, e, seq, &now); send_vblank_event(dev, e, seq, &now);
} }
spin_unlock(&dev->event_lock); spin_unlock_irqrestore(&dev->event_lock, irqflags);
/*
* Prevent subsequent drm_vblank_get() from re-enabling
* the vblank interrupt by bumping the refcount.
*/
if (!vblank->inmodeset) {
atomic_inc(&vblank->refcount);
vblank->inmodeset = 1;
}
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
} }
EXPORT_SYMBOL(drm_vblank_off); EXPORT_SYMBOL(drm_vblank_off);
...@@ -1446,12 +1446,11 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) ...@@ -1446,12 +1446,11 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc)
{ {
struct drm_pending_vblank_event *e, *t; struct drm_pending_vblank_event *e, *t;
struct timeval now; struct timeval now;
unsigned long flags;
unsigned int seq; unsigned int seq;
seq = drm_vblank_count_and_time(dev, crtc, &now); assert_spin_locked(&dev->event_lock);
spin_lock_irqsave(&dev->event_lock, flags); seq = drm_vblank_count_and_time(dev, crtc, &now);
list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) { list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
if (e->pipe != crtc) if (e->pipe != crtc)
...@@ -1467,8 +1466,6 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) ...@@ -1467,8 +1466,6 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc)
send_vblank_event(dev, e, seq, &now); send_vblank_event(dev, e, seq, &now);
} }
spin_unlock_irqrestore(&dev->event_lock, flags);
trace_drm_vblank_event(crtc, seq); trace_drm_vblank_event(crtc, seq);
} }
...@@ -1491,15 +1488,18 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) ...@@ -1491,15 +1488,18 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc)
if (!dev->num_crtcs) if (!dev->num_crtcs)
return false; return false;
spin_lock_irqsave(&dev->event_lock, irqflags);
/* Need timestamp lock to prevent concurrent execution with /* Need timestamp lock to prevent concurrent execution with
* vblank enable/disable, as this would cause inconsistent * vblank enable/disable, as this would cause inconsistent
* or corrupted timestamps and vblank counts. * 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. */ /* Vblank irq handling disabled. Nothing to do. */
if (!vblank->enabled) { if (!vblank->enabled) {
spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); spin_unlock(&dev->vblank_time_lock);
spin_unlock_irqrestore(&dev->event_lock, irqflags);
return false; return false;
} }
...@@ -1539,10 +1539,13 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) ...@@ -1539,10 +1539,13 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc)
crtc, (int) diff_ns); crtc, (int) diff_ns);
} }
spin_unlock(&dev->vblank_time_lock);
wake_up(&vblank->queue); wake_up(&vblank->queue);
drm_handle_vblank_events(dev, crtc); drm_handle_vblank_events(dev, crtc);
spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); spin_unlock_irqrestore(&dev->event_lock, irqflags);
return true; return true;
} }
EXPORT_SYMBOL(drm_handle_vblank); EXPORT_SYMBOL(drm_handle_vblank);
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment