drm: Hide hw.lock cleanup in filp->release better

A few things:
- Rename the cleanup function from drm_master_release to
  drm_legacy_lock_release. It doesn't relase any master stuff, but
  just the legacy hw lock.
- Hide it in drm_lock.c, which allows us to make a few more functions
  static in there. To avoid forward decl we need to shuffle the code a
  bit though.
- Push the check for ->master into the function itself.
- Only call this for !DRIVER_MODESET.

End result: Another place that takes struct_mutex gone for good for
modern drivers.

v2: Remove leftover comment.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465930269-7883-3-git-send-email-daniel.vetter@ffwll.ch

1 files changed, 122 insertions, 116 deletions

diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index daa2ff12101b..0fb8f9e73486 100644
--- a/drivers/gpu/drm/drm_lock.c
+++ b/drivers/gpu/drm/drm_lock.c
@@ -41,6 +41,110 @@
 static int drm_lock_take(struct drm_lock_data *lock_data, unsigned int context);
 
 /**
+ * Take the heavyweight lock.
+ *
+ * \param lock lock pointer.
+ * \param context locking context.
+ * \return one if the lock is held, or zero otherwise.
+ *
+ * Attempt to mark the lock as held by the given context, via the \p cmpxchg instruction.
+ */
+static
+int drm_lock_take(struct drm_lock_data *lock_data,
+                  unsigned int context)
+{
+        unsigned int old, new, prev;
+        volatile unsigned int *lock = &lock_data->hw_lock->lock;
+
+        spin_lock_bh(&lock_data->spinlock);
+        do {
+                old = *lock;
+                if (old & _DRM_LOCK_HELD)
+                        new = old | _DRM_LOCK_CONT;
+                else {
+                        new = context | _DRM_LOCK_HELD |
+                                ((lock_data->user_waiters + lock_data->kernel_waiters > 1) ?
+                                 _DRM_LOCK_CONT : 0);
+                }
+                prev = cmpxchg(lock, old, new);
+        } while (prev != old);
+        spin_unlock_bh(&lock_data->spinlock);
+
+        if (_DRM_LOCKING_CONTEXT(old) == context) {
+                if (old & _DRM_LOCK_HELD) {
+                        if (context != DRM_KERNEL_CONTEXT) {
+                                DRM_ERROR("%d holds heavyweight lock\n",
+                                          context);
+                        }
+                        return 0;
+                }
+        }
+
+        if ((_DRM_LOCKING_CONTEXT(new)) == context && (new & _DRM_LOCK_HELD)) {
+                /* Have lock */
+                return 1;
+        }
+        return 0;
+}
+
+/**
+ * This takes a lock forcibly and hands it to context.  Should ONLY be used
+ * inside *_unlock to give lock to kernel before calling *_dma_schedule.
+ *
+ * \param dev DRM device.
+ * \param lock lock pointer.
+ * \param context locking context.
+ * \return always one.
+ *
+ * Resets the lock file pointer.
+ * Marks the lock as held by the given context, via the \p cmpxchg instruction.
+ */
+static int drm_lock_transfer(struct drm_lock_data *lock_data,
+                             unsigned int context)
+{
+        unsigned int old, new, prev;
+        volatile unsigned int *lock = &lock_data->hw_lock->lock;
+
+        lock_data->file_priv = NULL;
+        do {
+                old = *lock;
+                new = context | _DRM_LOCK_HELD;
+                prev = cmpxchg(lock, old, new);
+        } while (prev != old);
+        return 1;
+}
+
+static int drm_legacy_lock_free(struct drm_lock_data *lock_data,
+                                unsigned int context)
+{
+        unsigned int old, new, prev;
+        volatile unsigned int *lock = &lock_data->hw_lock->lock;
+
+        spin_lock_bh(&lock_data->spinlock);
+        if (lock_data->kernel_waiters != 0) {
+                drm_lock_transfer(lock_data, 0);
+                lock_data->idle_has_lock = 1;
+                spin_unlock_bh(&lock_data->spinlock);
+                return 1;
+        }
+        spin_unlock_bh(&lock_data->spinlock);
+
+        do {
+                old = *lock;
+                new = _DRM_LOCKING_CONTEXT(old);
+                prev = cmpxchg(lock, old, new);
+        } while (prev != old);
+
+        if (_DRM_LOCK_IS_HELD(old) && _DRM_LOCKING_CONTEXT(old) != context) {
+                DRM_ERROR("%d freed heavyweight lock held by %d\n",
+                          context, _DRM_LOCKING_CONTEXT(old));
+                return 1;
+        }
+        wake_up_interruptible(&lock_data->lock_queue);
+        return 0;
+}
+
+/**
  * Lock ioctl.
  *
  * \param inode device inode.
@@ -165,120 +269,6 @@ int drm_legacy_unlock(struct drm_device *dev, void *data, struct drm_file *file_
 }
 
 /**
- * Take the heavyweight lock.
- *
- * \param lock lock pointer.
- * \param context locking context.
- * \return one if the lock is held, or zero otherwise.
- *
- * Attempt to mark the lock as held by the given context, via the \p cmpxchg instruction.
- */
-static
-int drm_lock_take(struct drm_lock_data *lock_data,
-                  unsigned int context)
-{
-        unsigned int old, new, prev;
-        volatile unsigned int *lock = &lock_data->hw_lock->lock;
-
-        spin_lock_bh(&lock_data->spinlock);
-        do {
-                old = *lock;
-                if (old & _DRM_LOCK_HELD)
-                        new = old | _DRM_LOCK_CONT;
-                else {
-                        new = context | _DRM_LOCK_HELD |
-                                ((lock_data->user_waiters + lock_data->kernel_waiters > 1) ?
-                                 _DRM_LOCK_CONT : 0);
-                }
-                prev = cmpxchg(lock, old, new);
-        } while (prev != old);
-        spin_unlock_bh(&lock_data->spinlock);
-
-        if (_DRM_LOCKING_CONTEXT(old) == context) {
-                if (old & _DRM_LOCK_HELD) {
-                        if (context != DRM_KERNEL_CONTEXT) {
-                                DRM_ERROR("%d holds heavyweight lock\n",
-                                          context);
-                        }
-                        return 0;
-                }
-        }
-
-        if ((_DRM_LOCKING_CONTEXT(new)) == context && (new & _DRM_LOCK_HELD)) {
-                /* Have lock */
-                return 1;
-        }
-        return 0;
-}
-
-/**
- * This takes a lock forcibly and hands it to context.  Should ONLY be used
- * inside *_unlock to give lock to kernel before calling *_dma_schedule.
- *
- * \param dev DRM device.
- * \param lock lock pointer.
- * \param context locking context.
- * \return always one.
- *
- * Resets the lock file pointer.
- * Marks the lock as held by the given context, via the \p cmpxchg instruction.
- */
-static int drm_lock_transfer(struct drm_lock_data *lock_data,
-                             unsigned int context)
-{
-        unsigned int old, new, prev;
-        volatile unsigned int *lock = &lock_data->hw_lock->lock;
-
-        lock_data->file_priv = NULL;
-        do {
-                old = *lock;
-                new = context | _DRM_LOCK_HELD;
-                prev = cmpxchg(lock, old, new);
-        } while (prev != old);
-        return 1;
-}
-
-/**
- * Free lock.
- *
- * \param dev DRM device.
- * \param lock lock.
- * \param context context.
- *
- * Resets the lock file pointer.
- * Marks the lock as not held, via the \p cmpxchg instruction. Wakes any task
- * waiting on the lock queue.
- */
-int drm_legacy_lock_free(struct drm_lock_data *lock_data, unsigned int context)
-{
-        unsigned int old, new, prev;
-        volatile unsigned int *lock = &lock_data->hw_lock->lock;
-
-        spin_lock_bh(&lock_data->spinlock);
-        if (lock_data->kernel_waiters != 0) {
-                drm_lock_transfer(lock_data, 0);
-                lock_data->idle_has_lock = 1;
-                spin_unlock_bh(&lock_data->spinlock);
-                return 1;
-        }
-        spin_unlock_bh(&lock_data->spinlock);
-
-        do {
-                old = *lock;
-                new = _DRM_LOCKING_CONTEXT(old);
-                prev = cmpxchg(lock, old, new);
-        } while (prev != old);
-
-        if (_DRM_LOCK_IS_HELD(old) && _DRM_LOCKING_CONTEXT(old) != context) {
-                DRM_ERROR("%d freed heavyweight lock held by %d\n",
-                          context, _DRM_LOCKING_CONTEXT(old));
-                return 1;
-        }
-        wake_up_interruptible(&lock_data->lock_queue);
-        return 0;
-}
-
-/**
  * This function returns immediately and takes the hw lock
  * with the kernel context if it is free, otherwise it gets the highest priority when and if
  * it is eventually released.
@@ -330,11 +320,27 @@ void drm_legacy_idlelock_release(struct drm_lock_data *lock_data)
 }
 EXPORT_SYMBOL(drm_legacy_idlelock_release);
 
-int drm_legacy_i_have_hw_lock(struct drm_device *dev,
-                              struct drm_file *file_priv)
+static int drm_legacy_i_have_hw_lock(struct drm_device *dev,
+                                     struct drm_file *file_priv)
 {
         struct drm_master *master = file_priv->master;
         return (file_priv->lock_count && master->lock.hw_lock &&
                 _DRM_LOCK_IS_HELD(master->lock.hw_lock->lock) &&
                 master->lock.file_priv == file_priv);
 }
+
+void drm_legacy_lock_release(struct drm_device *dev, struct file *filp)
+{
+        struct drm_file *file_priv = filp->private_data;
+
+        /* if the master has gone away we can't do anything with the lock */
+        if (!file_priv->minor->master)
+                return;
+
+        if (drm_legacy_i_have_hw_lock(dev, file_priv)) {
+                DRM_DEBUG("File %p released, freeing lock for context %d\n",
+                          filp, _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));
+                drm_legacy_lock_free(&file_priv->master->lock,
+                                     _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));
+        }
+}