From patchwork Mon Aug 19 09:59:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11100599 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6BC7C14DB for ; Mon, 19 Aug 2019 09:59:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5ADBC2864F for ; Mon, 19 Aug 2019 09:59:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4F04A28662; Mon, 19 Aug 2019 09:59:44 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id BBA2028658 for ; Mon, 19 Aug 2019 09:59:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E83596E0F2; Mon, 19 Aug 2019 09:59:41 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 23DB56E0F0; Mon, 19 Aug 2019 09:59:40 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 18186724-1500050 for multiple; Mon, 19 Aug 2019 10:59:30 +0100 From: Chris Wilson To: dri-devel@lists.freedesktop.org Subject: [PATCH 3/3] dma-fence: Set the timestamp after the notifying the cb_list Date: Mon, 19 Aug 2019 10:59:28 +0100 Message-Id: <20190819095928.32091-3-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.23.0.rc1 In-Reply-To: <20190819095928.32091-1-chris@chris-wilson.co.uk> References: <20190819095928.32091-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org, =?utf-8?q?Christian_K=C3=B6nig?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Now that the timestamp and cb_list share the same slot in the fence, with the current order of setting the timestamp before notifying the cb_list, we have to take a temporary copy of the list. If we don't set the timestamp, we can simply process the list in situ. This also gives us the advantage that we get a signal when the cb_list is complete, useful in a few cases that need to serialise against the cb_list. Suggested-by: Christian König Signed-off-by: Chris Wilson Cc: Christian König --- drivers/dma-buf/dma-fence.c | 41 +++++++----------- drivers/dma-buf/st-dma-fence.c | 8 +--- drivers/dma-buf/sync_file.c | 5 +-- drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 28 +----------- include/linux/dma-fence.h | 47 ++++++++++++++++++++- 5 files changed, 65 insertions(+), 64 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 2c136aee3e79..972a4b90b820 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -111,47 +111,36 @@ u64 dma_fence_context_alloc(unsigned num) EXPORT_SYMBOL(dma_fence_context_alloc); /** - * dma_fence_signal_locked - signal completion of a fence + * __dma_fence_signal_locked - Perform signaling of a fence * @fence: the fence to signal + * @timestamp: the timsetamp of fence completion * - * Signal completion for software callbacks on a fence, this will unblock - * dma_fence_wait() calls and run all the callbacks added with - * dma_fence_add_callback(). Can be called multiple times, but since a fence - * can only go from the unsignaled to the signaled state and not back, it will - * only be effective the first time. + * See dma_fence_signal() for the typical interface. * - * Unlike dma_fence_signal(), this function must be called with &dma_fence.lock - * held. + * This provides the low-level operations required upon signaling a fence, + * such as processing the callback list and setting the timestamp. * - * Returns 0 on success and a negative error value when @fence has been - * signalled already. + * Requires the caller to hold the fence->lock and already have marked the + * fence as signaled in an exclusive manner. + * + * Great care must be taken in calling this function! */ -int dma_fence_signal_locked(struct dma_fence *fence) +void __dma_fence_signal_locked(struct dma_fence *fence, ktime_t timestamp) { struct dma_fence_cb *cur, *tmp; - struct list_head cb_list; lockdep_assert_held(fence->lock); - if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, - &fence->flags))) - return -EINVAL; - - /* Stash the cb_list before replacing it with the timestamp */ - list_replace(&fence->cb_list, &cb_list); - - fence->timestamp = ktime_get(); - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags); - trace_dma_fence_signaled(fence); - - list_for_each_entry_safe(cur, tmp, &cb_list, node) { + list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) { INIT_LIST_HEAD(&cur->node); cur->func(fence, cur); } - return 0; + fence->timestamp = timestamp; /* overwrites fence->cb_list */ + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags); + trace_dma_fence_signaled(fence); } -EXPORT_SYMBOL(dma_fence_signal_locked); +EXPORT_SYMBOL(__dma_fence_signal_locked); /** * dma_fence_signal - signal completion of a fence diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c index a365dc7440e5..1fba51a5a46b 100644 --- a/drivers/dma-buf/st-dma-fence.c +++ b/drivers/dma-buf/st-dma-fence.c @@ -434,12 +434,6 @@ struct race_thread { int id; }; -static void __wait_for_callbacks(struct dma_fence *f) -{ - spin_lock_irq(f->lock); - spin_unlock_irq(f->lock); -} - static int thread_signal_callback(void *arg) { const struct race_thread *t = arg; @@ -478,7 +472,7 @@ static int thread_signal_callback(void *arg) if (!cb.seen) { dma_fence_wait(f2, false); - __wait_for_callbacks(f2); + dma_fence_wait_for_notify(f2); } if (!READ_ONCE(cb.seen)) { diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 25c5c071645b..f801dabb33a4 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -384,9 +384,8 @@ static int sync_fill_fence_info(struct dma_fence *fence, sizeof(info->driver_name)); info->status = dma_fence_get_status(fence); - while (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) && - !test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags)) - cpu_relax(); + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + dma_fence_wait_for_notify(fence); info->timestamp_ns = test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags) ? ktime_to_ns(fence->timestamp) : diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c index 09c68dda2098..dbfb3b5348c4 100644 --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c @@ -105,29 +105,6 @@ __dma_fence_signal(struct dma_fence *fence) return !test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags); } -static void -__dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp) -{ - fence->timestamp = timestamp; - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags); - trace_dma_fence_signaled(fence); -} - -static void -__dma_fence_signal__notify(struct dma_fence *fence, - const struct list_head *list) -{ - struct dma_fence_cb *cur, *tmp; - - lockdep_assert_held(fence->lock); - lockdep_assert_irqs_disabled(); - - list_for_each_entry_safe(cur, tmp, list, node) { - INIT_LIST_HEAD(&cur->node); - cur->func(fence, cur); - } -} - void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) { struct intel_breadcrumbs *b = &engine->breadcrumbs; @@ -187,12 +164,9 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) list_for_each_safe(pos, next, &signal) { struct i915_request *rq = list_entry(pos, typeof(*rq), signal_link); - struct list_head cb_list; spin_lock(&rq->lock); - list_replace(&rq->fence.cb_list, &cb_list); - __dma_fence_signal__timestamp(&rq->fence, timestamp); - __dma_fence_signal__notify(&rq->fence, &cb_list); + __dma_fence_signal_locked(&rq->fence, timestamp); spin_unlock(&rq->lock); i915_request_put(rq); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 3347c54f3a87..b93c83f240c2 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -357,8 +357,36 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) } while (1); } +void __dma_fence_signal_locked(struct dma_fence *fence, ktime_t timestamp); + +/** + * dma_fence_signal_locked - signal completion of a fence + * @fence: the fence to signal + * + * Signal completion for software callbacks on a fence, this will unblock + * dma_fence_wait() calls and run all the callbacks added with + * dma_fence_add_callback(). Can be called multiple times, but since a fence + * can only go from the unsignaled to the signaled state and not back, it will + * only be effective the first time. + * + * Unlike dma_fence_signal(), this function must be called with &dma_fence.lock + * held. + * + * Returns 0 on success and a negative error value when @fence has been + * signalled already. + */ +static inline int dma_fence_signal_locked(struct dma_fence *fence) +{ + if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, + &fence->flags))) + return -EINVAL; + + __dma_fence_signal_locked(fence, ktime_get()); + return 0; +} + int dma_fence_signal(struct dma_fence *fence); -int dma_fence_signal_locked(struct dma_fence *fence); + signed long dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout); int dma_fence_add_callback(struct dma_fence *fence, @@ -426,6 +454,23 @@ dma_fence_is_signaled(struct dma_fence *fence) return false; } +/** + * dma_fence_wait_for_notify - Wait until the notifications are complete + * @fence: the fence to wait on + * + * After marking the fence as signaled, a number of operations but we + * are completely done, from notifying all the listeners culminating + * in setting the timestamp on the fence. This signals the completion + * of all the callbacks and the end of the siganling operation. + */ +static inline void +dma_fence_wait_for_notify(struct dma_fence *fence) +{ + WARN_ON(!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)); + while (!test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags)) + cpu_relax(); +} + /** * __dma_fence_is_later - return if f1 is chronologically later than f2 * @f1: the first fence's seqno