Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> The initial-breadcrumb is used to mark the end of the awaiting and the
> beginning of the user payload. We verify that we do not start the user
> payload before all signaler are completed, checking our semaphore setup
> by looking for the initial breadcrumb being written too early. We also
> want to ensure that we do not add semaphore waits after we have already
> closed the semaphore section, an issue for later deferred waits.
>
I see that you go from timeline property to a per request property.
But the last sentence in this commit message escapes me. Do we need
this for proxy fence waiting of is this patch just for enhancing
the assertion/gem_bug_on resolution on semaphore boundaries?
-Mika
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 5 ++++-
> drivers/gpu/drm/i915/i915_request.c | 7 ++++++-
> drivers/gpu/drm/i915/i915_request.h | 27 ++++++++++++++++++++-------
> 3 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index c5591248dafb..f93f13d20b5a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1894,7 +1894,7 @@ static void defer_request(struct i915_request *rq, struct list_head * const pl)
> continue;
>
> /* No waiter should start before its signaler */
> - GEM_BUG_ON(w->context->timeline->has_initial_breadcrumb &&
> + GEM_BUG_ON(i915_request_has_initial_breadcrumb(w) &&
> i915_request_started(w) &&
> !i915_request_completed(rq));
>
> @@ -3501,6 +3501,7 @@ static int gen8_emit_init_breadcrumb(struct i915_request *rq)
> {
> u32 *cs;
>
> + GEM_BUG_ON(i915_request_has_initial_breadcrumb(rq));
> if (!i915_request_timeline(rq)->has_initial_breadcrumb)
> return 0;
>
> @@ -3527,6 +3528,8 @@ static int gen8_emit_init_breadcrumb(struct i915_request *rq)
> /* Record the updated position of the request's payload */
> rq->infix = intel_ring_offset(rq, cs);
>
> + __set_bit(I915_FENCE_FLAG_INITIAL_BREADCRUMB, &rq->fence.flags);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 2d5b98549ddc..00b7c4eb3f32 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -951,6 +951,7 @@ __emit_semaphore_wait(struct i915_request *to,
> u32 *cs;
>
> GEM_BUG_ON(INTEL_GEN(to->i915) < 8);
> + GEM_BUG_ON(i915_request_has_initial_breadcrumb(to));
>
> /* We need to pin the signaler's HWSP until we are finished reading. */
> err = intel_timeline_read_hwsp(from, to, &hwsp_offset);
> @@ -1000,6 +1001,9 @@ emit_semaphore_wait(struct i915_request *to,
> if (!intel_context_use_semaphores(to->context))
> goto await_fence;
>
> + if (i915_request_has_initial_breadcrumb(to))
> + goto await_fence;
> +
> if (!rcu_access_pointer(from->hwsp_cacheline))
> goto await_fence;
>
> @@ -1256,7 +1260,8 @@ __i915_request_await_execution(struct i915_request *to,
> * immediate execution, and so we must wait until it reaches the
> * active slot.
> */
> - if (intel_engine_has_semaphores(to->engine)) {
> + if (intel_engine_has_semaphores(to->engine) &&
> + !i915_request_has_initial_breadcrumb(to)) {
> err = __emit_semaphore_wait(to, from, from->fence.seqno - 1);
> if (err < 0)
> return err;
> diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
> index d8ce908e1346..98ae2dc82371 100644
> --- a/drivers/gpu/drm/i915/i915_request.h
> +++ b/drivers/gpu/drm/i915/i915_request.h
> @@ -84,19 +84,26 @@ enum {
> I915_FENCE_FLAG_PQUEUE,
>
> /*
> - * I915_FENCE_FLAG_SIGNAL - this request is currently on signal_list
> + * I915_FENCE_FLAG_HOLD - this request is currently on hold
> *
> - * Internal bookkeeping used by the breadcrumb code to track when
> - * a request is on the various signal_list.
> + * This request has been suspended, pending an ongoing investigation.
> */
> - I915_FENCE_FLAG_SIGNAL,
> + I915_FENCE_FLAG_HOLD,
>
> /*
> - * I915_FENCE_FLAG_HOLD - this request is currently on hold
> + * I915_FENCE_FLAG_INITIAL_BREADCRUMB - this request has the initial
> + * breadcrumb that marks the end of semaphore waits and start of the
> + * user payload.
> + */
> + I915_FENCE_FLAG_INITIAL_BREADCRUMB,
> +
> + /*
> + * I915_FENCE_FLAG_SIGNAL - this request is currently on signal_list
> *
> - * This request has been suspended, pending an ongoing investigation.
> + * Internal bookkeeping used by the breadcrumb code to track when
> + * a request is on the various signal_list.
> */
> - I915_FENCE_FLAG_HOLD,
> + I915_FENCE_FLAG_SIGNAL,
>
> /*
> * I915_FENCE_FLAG_NOPREEMPT - this request should not be preempted
> @@ -390,6 +397,12 @@ static inline bool i915_request_in_priority_queue(const struct i915_request *rq)
> return test_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
> }
>
> +static inline bool
> +i915_request_has_initial_breadcrumb(const struct i915_request *rq)
> +{
> + return test_bit(I915_FENCE_FLAG_INITIAL_BREADCRUMB, &rq->fence.flags);
> +}
> +
> /**
> * Returns true if seq1 is later than seq2.
> */
> --
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
