From patchwork Tue Jun 14 18:50:57 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 9176525 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7285660772 for ; Tue, 14 Jun 2016 18:51:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5A3232793B for ; Tue, 14 Jun 2016 18:51:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4F07B2824A; Tue, 14 Jun 2016 18:51:58 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BB5192793B for ; Tue, 14 Jun 2016 18:51:57 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 095CC6E806; Tue, 14 Jun 2016 18:51:42 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id E6B5C6E7FA for ; Tue, 14 Jun 2016 18:51:20 +0000 (UTC) Received: by mail-wm0-x241.google.com with SMTP id n184so641181wmn.1 for ; Tue, 14 Jun 2016 11:51:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=DjGJKw7yD6Jf2Tcx+nH6O4n4dSRJ8dvxL9UlJGesoRA=; b=eqkK6EVLUOqPSnpipoHPfXzRXaQyTsyD5ZEg+vuTihq/u7pyT40vTkNLl/aElzh1Sz prBiPTDiGwC6c8ToczSCfK5w21ryfWjlZde2vPkwQwpVp2vCciA8pfsb8e5jmcIpEoNN RA06i4pxlhgRBZMnQQQ+Z44k+B3h8L/84wpz0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=DjGJKw7yD6Jf2Tcx+nH6O4n4dSRJ8dvxL9UlJGesoRA=; b=U711QDgquyqVuZN31SMImjbQ7VAKo0VnWG62xeOrWuehkcgmB935wZSmBJ/me7ayOH lFM1K+HrjzQkI9uuTzSx/6fUj1vEB8tTCfZjYVVBsQopiq7rvE39rb0oU1u7URf+bsmY pHIu3N7C+pjqBmswDxvXT9T9GzVXlbE+kP3q+SI8h3WQiZxJKeT3t0bb+Nq3aQ3tLXAn XI+ZwjEEihJjO9A0h+cXS9cbN/ZPcX1uvuDhXBJEjkXQxMbEaqY477gETmDiQjaL18mE yd6uKqWFc1Tyoa5eb/JXmY2/4wrbr0mYlF1vZ0hkaHdl6Ts7vfpo8BM3qYFkUYsL+Dnj IUDQ== X-Gm-Message-State: ALyK8tKYUaETpl9MvnW6gYlLf0jWQonPjA8I4COcjKRJohc6bIE3INbu0GdkevDA/y9PVg== X-Received: by 10.194.117.35 with SMTP id kb3mr8197798wjb.136.1465930278691; Tue, 14 Jun 2016 11:51:18 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:56b5:0:ac27:b86c:7764:9429]) by smtp.gmail.com with ESMTPSA id xs9sm31416882wjc.11.2016.06.14.11.51.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 14 Jun 2016 11:51:18 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH 02/14] drm: Hide hw.lock cleanup in filp->release better Date: Tue, 14 Jun 2016 20:50:57 +0200 Message-Id: <1465930269-7883-3-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.8.1 In-Reply-To: <1465930269-7883-1-git-send-email-daniel.vetter@ffwll.ch> References: <1465930269-7883-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development , Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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. Signed-off-by: Daniel Vetter Reviewed-by: Chris Wilson Reviewed-by: Emil Velikov --- drivers/gpu/drm/drm_fops.c | 17 +--- drivers/gpu/drm/drm_legacy.h | 8 +- drivers/gpu/drm/drm_lock.c | 238 ++++++++++++++++++++++--------------------- 3 files changed, 126 insertions(+), 137 deletions(-) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index a27bc7cda975..2fd4f42b907a 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -338,18 +338,6 @@ out_prime_destroy: return ret; } -static void drm_master_release(struct drm_device *dev, struct file *filp) -{ - struct drm_file *file_priv = filp->private_data; - - 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)); - } -} - static void drm_events_release(struct drm_file *file_priv) { struct drm_device *dev = file_priv->minor->dev; @@ -468,9 +456,8 @@ int drm_release(struct inode *inode, struct file *filp) (long)old_encode_dev(file_priv->minor->kdev->devt), dev->open_count); - /* if the master has gone away we can't do anything with the lock */ - if (file_priv->minor->master) - drm_master_release(dev, filp); + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + drm_legacy_lock_release(dev, filp); if (drm_core_check_feature(dev, DRIVER_HAVE_DMA)) drm_legacy_reclaim_buffers(dev, file_priv); diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h index d3b6ee357a2b..c6f422e879dd 100644 --- a/drivers/gpu/drm/drm_legacy.h +++ b/drivers/gpu/drm/drm_legacy.h @@ -88,14 +88,10 @@ struct drm_agp_mem { struct list_head head; }; -/* - * Generic Userspace Locking-API - */ - -int drm_legacy_i_have_hw_lock(struct drm_device *d, struct drm_file *f); +/* drm_lock.c */ int drm_legacy_lock(struct drm_device *d, void *v, struct drm_file *f); int drm_legacy_unlock(struct drm_device *d, void *v, struct drm_file *f); -int drm_legacy_lock_free(struct drm_lock_data *lock, unsigned int ctx); +void drm_legacy_lock_release(struct drm_device *dev, struct file *filp); /* DMA support */ int drm_legacy_dma_setup(struct drm_device *dev); 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)); + } +}