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@chris-wilson.co.uk Cc: Mika Kuoppala mika.kuoppala@linux.intel.com Cc: stable@vger.kernel.org # 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