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: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

1 files changed, 25 insertions, 22 deletions

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b4460bf0e0e4..9353609c6770 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1037,14 +1037,25 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
         unsigned long irqflags;
         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);
         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 */
         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) {
                 if (e->pipe != crtc)
                         continue;
@@ -1055,18 +1066,7 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
                 drm_vblank_put(dev, e->pipe);
                 send_vblank_event(dev, e, seq, &now);
         }
-        spin_unlock(&dev->event_lock);
-
-        /*
-         * 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);
+        spin_unlock_irqrestore(&dev->event_lock, irqflags);
 }
 EXPORT_SYMBOL(drm_vblank_off);
 
@@ -1446,12 +1446,11 @@ 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);
+        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) {
                 if (e->pipe != crtc)
@@ -1467,8 +1466,6 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc)
                 send_vblank_event(dev, e, seq, &now);
         }
 
-        spin_unlock_irqrestore(&dev->event_lock, flags);
-
         trace_drm_vblank_event(crtc, seq);
 }
 
@@ -1491,15 +1488,18 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc)
         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 (!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;
         }
 
@@ -1539,10 +1539,13 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc)
                           crtc, (int) diff_ns);
         }
 
+        spin_unlock(&dev->vblank_time_lock);
+
         wake_up(&vblank->queue);
         drm_handle_vblank_events(dev, crtc);
 
-        spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
+        spin_unlock_irqrestore(&dev->event_lock, irqflags);
+
         return true;
 }
 EXPORT_SYMBOL(drm_handle_vblank);