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 <[email protected]> Signed-off-by: Chris Wilson <[email protected]> Cc: Christian König <[email protected]> --- 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 -- 2.23.0.rc1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
