Commit b5773a36 authored by Chris Wilson's avatar Chris Wilson
Browse files

drm/i915/execlists: Suppress mere WAIT preemption



WAIT is occasionally suppressed by virtue of preempted requests being
promoted to NEWCLIENT if they have not all ready received that boost.
Make this consistent for all WAIT boosts that they are not allowed to
preempt executing contexts and are merely granted the right to be at the
front of the queue for the next execution slot. This is in keeping with
the desire that the WAIT boost be a minor tweak that does not give
excessive promotion to its user and open ourselves to trivial abuse.

The problem with the inconsistent WAIT preemption becomes more apparent
as the preemption is propagated across the engines, where one engine may
preempt and the other not, and we be relying on the exact execution
order being consistent across engines (e.g. using HW semaphores to
coordinate parallel execution).

v2: Also protect GuC submission from false preemption loops.
v3: Build bug safeguards and better debug messages for st.
v4: Do the priority bumping in unsubmit (i.e. on preemption/reset
unwind), applying it earlier during submit causes out-of-order execution
combined with execute fences.
v5: Call sw_fence_fini for our dummy request (Matthew)

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190228220639.3173-1-chris@chris-wilson.co.uk
parent bd5d6781
Loading
Loading
Loading
Loading
+15 −0
Original line number Diff line number Diff line
@@ -358,11 +358,14 @@ void __i915_request_submit(struct i915_request *request)

	/* We may be recursing from the signal callback of another i915 fence */
	spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);

	GEM_BUG_ON(test_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags));
	set_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags);

	if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags) &&
	    !i915_request_enable_breadcrumb(request))
		intel_engine_queue_breadcrumbs(engine);

	spin_unlock(&request->lock);

	engine->emit_fini_breadcrumb(request,
@@ -406,10 +409,22 @@ void __i915_request_unsubmit(struct i915_request *request)

	/* We may be recursing from the signal callback of another i915 fence */
	spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);

	/*
	 * As we do not allow WAIT to preempt inflight requests,
	 * once we have executed a request, along with triggering
	 * any execution callbacks, we must preserve its ordering
	 * within the non-preemptible FIFO.
	 */
	BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
	request->sched.attr.priority |= __NO_PREEMPTION;

	if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags))
		i915_request_cancel_breadcrumb(request);

	GEM_BUG_ON(!test_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags));
	clear_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags);

	spin_unlock(&request->lock);

	/* Transfer back from the global per-engine timeline to per-context */
+0 −1
Original line number Diff line number Diff line
@@ -324,7 +324,6 @@ static void __i915_schedule(struct i915_request *rq,
			if (node_signaled(p->signaler))
				continue;

			GEM_BUG_ON(p->signaler->attr.priority < node->attr.priority);
			if (prio > READ_ONCE(p->signaler->attr.priority))
				list_move_tail(&p->dfs_link, &dfs);
		}
+2 −0
Original line number Diff line number Diff line
@@ -33,6 +33,8 @@ enum {
#define I915_PRIORITY_WAIT	((u8)BIT(0))
#define I915_PRIORITY_NEWCLIENT	((u8)BIT(1))

#define __NO_PREEMPTION (I915_PRIORITY_WAIT)

struct i915_sched_attr {
	/**
	 * @priority: execution and service priority
+1 −1
Original line number Diff line number Diff line
@@ -720,7 +720,7 @@ static inline int rq_prio(const struct i915_request *rq)

static inline int port_prio(const struct execlist_port *port)
{
	return rq_prio(port_request(port));
	return rq_prio(port_request(port)) | __NO_PREEMPTION;
}

static bool __guc_dequeue(struct intel_engine_cs *engine)
+8 −1
Original line number Diff line number Diff line
@@ -188,6 +188,12 @@ static inline int rq_prio(const struct i915_request *rq)
	return rq->sched.attr.priority;
}

static int effective_prio(const struct i915_request *rq)
{
	/* Restrict mere WAIT boosts from triggering preemption */
	return rq_prio(rq) | __NO_PREEMPTION;
}

static int queue_prio(const struct intel_engine_execlists *execlists)
{
	struct i915_priolist *p;
@@ -208,7 +214,7 @@ static int queue_prio(const struct intel_engine_execlists *execlists)
static inline bool need_preempt(const struct intel_engine_cs *engine,
				const struct i915_request *rq)
{
	const int last_prio = rq_prio(rq);
	int last_prio;

	if (!intel_engine_has_preemption(engine))
		return false;
@@ -228,6 +234,7 @@ static inline bool need_preempt(const struct intel_engine_cs *engine,
	 * preempt. If that hint is stale or we may be trying to preempt
	 * ourselves, ignore the request.
	 */
	last_prio = effective_prio(rq);
	if (!__execlists_need_preempt(engine->execlists.queue_priority_hint,
				      last_prio))
		return false;
Loading