[Intel-gfx] [PATCH] drm/i915: Special handling for bonded requests

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Our generic mechanism for specifying aligned request start,
I915_EXEC_FENCE_SUBMIT, has a bit too relaxed rules when it comes to the
actual use case of media scalability on Tigerlake.

While the submit fence can provide the aligned start, the  actual media
pipeline expects that execution remains in lockstep from then onwards.
Otherwise the hardware cannot handle well one of the pair getting
preempted leading to GPU hangs and corrupted media playback.

This puts us in a difficult position where the only visible solution,
which does not involve adding new uapi, is to give more meaning to the
generic submit fence. At least when used between the video engines.

The special semantics this patch applies in that case are two fold. First
is that both of the aligned pairs will be marked as non-preemptable and
second is ensuring both are not sharing ELSP ports with any other context.

Non-preemptable property will ensure that once the start has been aligned
the requests remain executing until completion.

Single port ELSP property will ensure there are no possible inversions
between different independent pairs of aligned requests.

Going forward we can think of introducing new uapi to request this
behaviour as a separate, more explicit flag, and at that point retire this
semi-generic special handling.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Suggested-by: Xiaogang Li <xiaogang.li at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_context.h |  2 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c     | 46 +++++++++++++++++++++----
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 07be021882cc..576d11c0cdd9 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -212,7 +212,7 @@ intel_context_force_single_submission(const struct intel_context *ce)
 static inline void
 intel_context_set_single_submission(struct intel_context *ce)
 {
-	__set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags);
+	set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags);
 }
 
 static inline bool
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 3214a4ecc31a..88cf20fd92c8 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1734,8 +1734,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 
 static bool ctx_single_port_submission(const struct intel_context *ce)
 {
-	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
-		intel_context_force_single_submission(ce));
+	return intel_context_force_single_submission(ce);
 }
 
 static bool can_merge_ctx(const struct intel_context *prev,
@@ -4929,9 +4928,41 @@ static void execlists_park(struct intel_engine_cs *engine)
 	cancel_timer(&engine->execlists.preempt);
 }
 
+static void
+mark_bonded_pair(struct i915_request *rq, struct i915_request *signal)
+{
+	/*
+	 * Give (temporary) special meaning to a pair requests with requested
+	 * aligned start along the video engines.
+	 *
+	 * They should be non-preemptable and have all ELSP ports to themselves
+	 * to avoid any deadlocks caused by inversions.
+	 *
+	 * Gen11+
+	 */
+	if (INTEL_GEN(rq->i915) < 11 ||
+	    rq->engine->class != VIDEO_DECODE_CLASS ||
+	    rq->engine->class != signal->engine->class)
+		return;
+
+	set_bit(I915_FENCE_FLAG_NOPREEMPT, &rq->fence.flags);
+	set_bit(I915_FENCE_FLAG_NOPREEMPT, &signal->fence.flags);
+
+	intel_context_set_single_submission(rq->context);
+	intel_context_set_single_submission(signal->context);
+}
+
+static void
+execlists_bond_execute(struct i915_request *rq, struct dma_fence *signal)
+{
+	mark_bonded_pair(rq, to_request(signal));
+}
+
 void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
 {
 	engine->submit_request = execlists_submit_request;
+	if (engine->class == VIDEO_DECODE_CLASS)
+		engine->bond_execute = execlists_bond_execute;
 	engine->schedule = i915_schedule;
 	engine->execlists.tasklet.func = execlists_submission_tasklet;
 
@@ -5602,15 +5633,18 @@ virtual_find_bond(struct virtual_engine *ve,
 }
 
 static void
-virtual_bond_execute(struct i915_request *rq, struct dma_fence *signal)
+virtual_bond_execute(struct i915_request *rq, struct dma_fence *sigfence)
 {
 	struct virtual_engine *ve = to_virtual_engine(rq->engine);
+	struct i915_request *signal = to_request(sigfence);
 	intel_engine_mask_t allowed, exec;
 	struct ve_bond *bond;
 
-	allowed = ~to_request(signal)->engine->mask;
+	mark_bonded_pair(rq, signal);
+
+	allowed = ~signal->engine->mask;
 
-	bond = virtual_find_bond(ve, to_request(signal)->engine);
+	bond = virtual_find_bond(ve, signal->engine);
 	if (bond)
 		allowed &= bond->sibling_mask;
 
@@ -5620,7 +5654,7 @@ virtual_bond_execute(struct i915_request *rq, struct dma_fence *signal)
 		;
 
 	/* Prevent the master from being re-run on the bonded engines */
-	to_request(signal)->execution_mask &= ~allowed;
+	signal->execution_mask &= ~allowed;
 }
 
 struct intel_context *
-- 
2.20.1