Quoting Chris Wilson (2019-12-08 23:33:08) > Be careful that we never submit the same request again if we have > already applied the LiteRestore w/a upon it -- as the HW may complete > the request as we submit the ELSP and so become confused by the request > to execute an empty ring. > > To submit the same request in ELSP[0] three times (so triggering the > LiteRestore w/a and resubmitting with it already applied) would require > preemption, and on preemption we should be unwinding the request tail as > we know the HW hasn't advanced beyond the normal time. So in practice, > this should never occur... > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > --- > git add fail for the GEM_WARN_ON tell-tale > --- > drivers/gpu/drm/i915/gt/intel_lrc.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index c7ea8a055005..fcd9bb771223 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -1776,16 +1776,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine) > > return; > } > - > - /* > - * WaIdleLiteRestore:bdw,skl > - * Apply the wa NOOPs to prevent > - * ring:HEAD == rq:TAIL as we resubmit the > - * request. See gen8_emit_fini_breadcrumb() for > - * where we prepare the padding after the > - * end of the request. > - */ > - last->tail = last->wa_tail; > } > } > > @@ -1943,6 +1933,24 @@ static void execlists_dequeue(struct intel_engine_cs *engine) > ctx_single_port_submission(rq->hw_context)) > goto done; > > + /* > + * WaIdleLiteRestore:bdw,skl > + * > + * If we have already submitted this request > + * using the wa_tail, we race with the HW and > + * HEAD may reach wa_tail before it processes > + * the ELSP[]. If it sees a context with an > + * empty ring, the HW gets confused. > + * > + * To prevent subsequent resubmission (lite > + * restores) with an empty ring, we emitted a > + * couple of NOOPs in gen8_emit_wa_tail() > + * beyond the normal end of the request. > + */ > + if (GEM_WARN_ON(last->tail == last->wa_tail)) > + goto done; > + > + last->tail = last->wa_tail; Bah, this means we update the tail before the current submission, so while the warn is meaningful, the ELSP[] is b0rked. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
