Skip to content

Commit f26a9e9

Browse files
icklejlahtine-intel
authored andcommitted
drm/i915/gt: Detect if we miss WaIdleLiteRestore
In order to avoid confusing the HW, we must never submit an empty ring during lite-restore, that is we should always advance the RING_TAIL before submitting to stay ahead of the RING_HEAD. Normally this is prevented by keeping a couple of spare NOPs in the request->wa_tail so that on resubmission we can advance the tail. This relies on the request only being resubmitted once, which is the normal condition as it is seen once for ELSP[1] and then later in ELSP[0]. On preemption, the requests are unwound and the tail reset back to the normal end point (as we know the request is incomplete and therefore its RING_HEAD is even earlier). However, if this w/a should fail we would try and resubmit the request with the RING_TAIL already set to the location of this request's wa_tail potentially causing a GPU hang. We can spot when we do try and incorrectly resubmit without advancing the RING_TAIL and spare any embarrassment by forcing the context restore. In the case of preempt-to-busy, we leave the requests running on the HW while we unwind. As the ring is still live, we cannot rewind our rq->tail without forcing a reload so leave it set to rq->wa_tail and only force a reload if we resubmit after a lite-restore. (Normally, the forced reload will be a part of the preemption event.) Fixes: 22b7a42 ("drm/i915/execlists: Preempt-to-busy") Closes: https://gitlab.freedesktop.org/drm/intel/issues/673 Signed-off-by: Chris Wilson <[email protected]> Cc: Mika Kuoppala <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Cc: [email protected] Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit 82c69bf) Signed-off-by: Joonas Lahtinen <[email protected]>
1 parent 3ce8209 commit f26a9e9

File tree

1 file changed

+21
-25
lines changed

1 file changed

+21
-25
lines changed

drivers/gpu/drm/i915/gt/intel_lrc.c

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -845,12 +845,6 @@ static const u8 *reg_offsets(const struct intel_engine_cs *engine)
845845
}
846846
}
847847

848-
static void unwind_wa_tail(struct i915_request *rq)
849-
{
850-
rq->tail = intel_ring_wrap(rq->ring, rq->wa_tail - WA_TAIL_BYTES);
851-
assert_ring_tail_valid(rq->ring, rq->tail);
852-
}
853-
854848
static struct i915_request *
855849
__unwind_incomplete_requests(struct intel_engine_cs *engine)
856850
{
@@ -863,12 +857,10 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
863857
list_for_each_entry_safe_reverse(rq, rn,
864858
&engine->active.requests,
865859
sched.link) {
866-
867860
if (i915_request_completed(rq))
868861
continue; /* XXX */
869862

870863
__i915_request_unsubmit(rq);
871-
unwind_wa_tail(rq);
872864

873865
/*
874866
* Push the request back into the queue for later resubmission.
@@ -1161,13 +1153,29 @@ execlists_schedule_out(struct i915_request *rq)
11611153
i915_request_put(rq);
11621154
}
11631155

1164-
static u64 execlists_update_context(const struct i915_request *rq)
1156+
static u64 execlists_update_context(struct i915_request *rq)
11651157
{
11661158
struct intel_context *ce = rq->hw_context;
1167-
u64 desc;
1159+
u64 desc = ce->lrc_desc;
1160+
u32 tail;
11681161

1169-
ce->lrc_reg_state[CTX_RING_TAIL] =
1170-
intel_ring_set_tail(rq->ring, rq->tail);
1162+
/*
1163+
* WaIdleLiteRestore:bdw,skl
1164+
*
1165+
* We should never submit the context with the same RING_TAIL twice
1166+
* just in case we submit an empty ring, which confuses the HW.
1167+
*
1168+
* We append a couple of NOOPs (gen8_emit_wa_tail) after the end of
1169+
* the normal request to be able to always advance the RING_TAIL on
1170+
* subsequent resubmissions (for lite restore). Should that fail us,
1171+
* and we try and submit the same tail again, force the context
1172+
* reload.
1173+
*/
1174+
tail = intel_ring_set_tail(rq->ring, rq->tail);
1175+
if (unlikely(ce->lrc_reg_state[CTX_RING_TAIL] == tail))
1176+
desc |= CTX_DESC_FORCE_RESTORE;
1177+
ce->lrc_reg_state[CTX_RING_TAIL] = tail;
1178+
rq->tail = rq->wa_tail;
11711179

11721180
/*
11731181
* Make sure the context image is complete before we submit it to HW.
@@ -1186,13 +1194,11 @@ static u64 execlists_update_context(const struct i915_request *rq)
11861194
*/
11871195
mb();
11881196

1189-
desc = ce->lrc_desc;
1190-
ce->lrc_desc &= ~CTX_DESC_FORCE_RESTORE;
1191-
11921197
/* Wa_1607138340:tgl */
11931198
if (IS_TGL_REVID(rq->i915, TGL_REVID_A0, TGL_REVID_A0))
11941199
desc |= CTX_DESC_FORCE_RESTORE;
11951200

1201+
ce->lrc_desc &= ~CTX_DESC_FORCE_RESTORE;
11961202
return desc;
11971203
}
11981204

@@ -1703,16 +1709,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
17031709

17041710
return;
17051711
}
1706-
1707-
/*
1708-
* WaIdleLiteRestore:bdw,skl
1709-
* Apply the wa NOOPs to prevent
1710-
* ring:HEAD == rq:TAIL as we resubmit the
1711-
* request. See gen8_emit_fini_breadcrumb() for
1712-
* where we prepare the padding after the
1713-
* end of the request.
1714-
*/
1715-
last->tail = last->wa_tail;
17161712
}
17171713
}
17181714

0 commit comments

Comments
 (0)