From 1a75a222f5ca1063249b5c92972d32dcc3c8966e Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 14 Jun 2016 20:50:57 +0200 Subject: 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 Reviewed-by: Emil Velikov Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1465930269-7883-3-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_lock.c | 238 +++++++++++++++++++++++---------------------- 1 file changed, 122 insertions(+), 116 deletions(-) (limited to 'drivers/gpu/drm/drm_lock.c') 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 @@ -40,6 +40,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. * @@ -164,120 +268,6 @@ int drm_legacy_unlock(struct drm_device *dev, void *data, struct drm_file *file_ return 0; } -/** - * 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 @@ -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)); + } +} -- cgit