Quoting Chris Wilson (2020-06-09 13:28:56)
> In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> reload when rewinding RING_TAIL"), we placed the check for rewinding a
> context on actually submitting the next request in that context. This
> was so that we only had to check once, and could do so with precision
> avoiding as many forced restores as possible. For example, to ensure
> that we can resubmit the same request a couple of times, we include a
> small wa_tail such that on the next submission, the ring->tail will
> appear to move forwards when resubmitting the same request. This is very
> common as it will happen for every lite-restore to fill the second port
> after a context switch.
>
> However, intel_ring_direction() is limited in precision to movements of
> upto half the ring size. The consequence being that if we tried to
> unwind many requests, we could exceed half the ring and flip the sense
> of the direction, so missing a force restore. As no request can be
> greater than half the ring (i.e. 2048 bytes in the smallest case), we
> can check for rollback incrementally. As we check against the tail that
> would be submitted, we do not lose any sensitivity and allow lite
> restores for the simple case. We still need to double check upon
> submitting the context, to allow for multiple preemptions and
> resubmissions.
>
> Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v5.4+
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 +-
> drivers/gpu/drm/i915/gt/intel_lrc.c | 21 +++-
> drivers/gpu/drm/i915/gt/intel_ring.c | 4 +
> drivers/gpu/drm/i915/gt/selftest_mocs.c | 18 ++-
> drivers/gpu/drm/i915/gt/selftest_ring.c | 110 ++++++++++++++++++
> .../drm/i915/selftests/i915_mock_selftests.h | 1 +
> 6 files changed, 154 insertions(+), 4 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index e5141a897786..0a05301e00fb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
> struct measure_breadcrumb {
> struct i915_request rq;
> struct intel_ring ring;
> - u32 cs[1024];
> + u32 cs[2048];
> };
>
> static int measure_breadcrumb_dw(struct intel_context *ce)
> @@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
>
> frame->ring.vaddr = frame->cs;
> frame->ring.size = sizeof(frame->cs);
> + frame->ring.wrap =
> + BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
> frame->ring.effective_size = frame->ring.size;
> intel_ring_update_space(&frame->ring);
> frame->rq.ring = &frame->ring;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a057f7a2a521..f66274e60bb6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
> list_move(&rq->sched.link, pl);
> set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>
> + /* Check for rollback incrementally */
> + if (intel_ring_direction(rq->ring,
> + intel_ring_wrap(rq->ring,
> + rq->tail),
> + rq->ring->tail) <= 0)
> + rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
We could be a bit more cheeky in that the problem only occurs if we
rollback far enough that there is a danger is mistaking the rollback for
a forward update.
-Chris
