Quoting Tvrtko Ursulin (2020-05-27 09:53:22)
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> 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.
Nak.
> 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.
As CI will say, such behaviour will already need a new flag. Forcing
no-preemption should be a privileged flag, so I would expect some
acknowledge that this is a HW problem that we have to workaround.
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Suggested-by: Xiaogang Li <xiaogang.li@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> 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.
This sentence needs expanding, because you are implying that this is an
issue we need to address in the code. If there is a deadlock resulting
from incorrect submission ordering, that is a bug in the code.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
