Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> We may try to preempt the currently executing request, only to find that
> after unravelling all the dependencies that the original executing
> context is still the earliest in the topological sort and re-submitted
> back to HW (if we do detect some change in the ELSP that requires
> re-submission). However, due to the way we check for wrap-around during
> the unravelling, we mark any context that has been submitted just once
> (i.e. with the rq->wa_tail set, but the ring->tail earlier) as
> potentially wrapping and requiring a forced restore on resubmission.
> This was expected to be not a problem, as it was anticipated that most
> unwinding for preemption would result in a context switch and the few
> that did not would be lost in the noise. It did not take long for
> someone to find one particular workload where the cost of those extra
> context restores was measurable.
>
> However, since we know the wa_tail is of fixed size, and we know that a
> request must be larger than the wa_tail itself, we can safely maintain
> the check for request wrapping and check against a slightly future point
> in the ring that includes an expected wa_tail. (That is if the
> ring->tail is already set to rq->wa_tail, including another 8 bytes in
> the check does not invalidate the incremental wrap detection.)
>
> Fixes: 8ab3a3812aa9 ("drm/i915/gt: Incrementally check for rewinding")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: Bruce Chang <yu.bruce.chang@xxxxxxxxx>
> Cc: Ramalingam C <ramalingam.c@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v5.4+
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 287537089c77..3aa05588834b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1140,9 +1140,8 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>
> /* Check in case we rollback so far we wrap [size/2] */
My parser hickups. /* Make sure rollback doesn't make hardware think we
went forward */
But for this patch,
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> if (intel_ring_direction(rq->ring,
> - intel_ring_wrap(rq->ring,
> - rq->tail),
> - rq->ring->tail) > 0)
> + rq->tail,
> + rq->ring->tail + 8) > 0)
> rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
>
> active = rq;
> --
> 2.20.1
