[Intel-gfx] [PATCH] drm/i915/execlists: stall on render flush before writing seqno

[Chris Wilson]

Quoting Mika Kuoppala (2019-08-28 10:00:35)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Quoting Chris Wilson (2019-08-27 12:54:13)
> >> Quite rarely we see that the CS completion event fires before the
> >> breadcrumb is coherent. Try rearranging the breadcrumb write sequence
> >> such that the CS_STALL is on the post-sync write pipecontrol.
> >> 
> >> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/gt/intel_lrc.c | 17 ++++++++---------
> >>  1 file changed, 8 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >> index 80a3f1dbb456..669e8bd9f830 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >> @@ -2961,18 +2961,17 @@ static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
> >>  
> >>  static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
> >>  {
> >> -       cs = gen8_emit_ggtt_write_rcs(cs,
> >> -                                     request->fence.seqno,
> >> -                                     request->timeline->hwsp_offset,
> >> -                                     PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> >> -                                     PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> >> -                                     PIPE_CONTROL_DC_FLUSH_ENABLE);
> >> -
> >>         /* XXX flush+write+CS_STALL all in one upsets gem_concurrent_blt:kbl */
> >>         cs = gen8_emit_pipe_control(cs,
> >> -                                   PIPE_CONTROL_FLUSH_ENABLE |
> >> -                                   PIPE_CONTROL_CS_STALL,
> >> +                                   PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> >> +                                   PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> >> +                                   PIPE_CONTROL_DC_FLUSH_ENABLE,
> >>                                     0);
> >> +       cs = gen8_emit_ggtt_write_rcs(cs,
> >> +                                     request->fence.seqno,
> >> +                                     request->timeline->hwsp_offset,
> >> +                                     PIPE_CONTROL_FLUSH_ENABLE |
> >
> > Or perhaps we need PIPE_CONTROL_DC_FLUSH_ENABLE here.
> >
> > I think that might make more sense (replace DC_FLUSH with whatever might
> > flush the post-sync write).
> 
> Would it make sense to try to be as much similar as possible
> with the ->emit_flush?

That's where we started and had to refine due to being able to detect
incoherency. It's not as if we we used emit_flush(EMIT_FLUSH) for
anything other than a delay in emit_pdp...

drivers/gpu/drm/i915/gem/i915_gem_context.c:            err = engine->emit_flush(rq, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_lrc.c:    ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_ringbuffer.c:             ret = engine->emit_flush(rq, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_ringbuffer.c:             ret = engine->emit_flush(rq, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_ringbuffer.c:             ret = engine->emit_flush(rq, EMIT_FLUSH);
drivers/gpu/drm/i915/gt/intel_ringbuffer.c:     ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_workarounds.c:    ret = rq->engine->emit_flush(rq, EMIT_BARRIER);
drivers/gpu/drm/i915/gt/intel_workarounds.c:    ret = rq->engine->emit_flush(rq, EMIT_BARRIER);
drivers/gpu/drm/i915/gvt/mmio_context.c:        ret = req->engine->emit_flush(req, EMIT_BARRIER);
drivers/gpu/drm/i915/gvt/mmio_context.c:        ret = req->engine->emit_flush(req, EMIT_BARRIER);

The EMIT_FLUSH in the ringbuffer pd update is also purely arbitrary.
-Chris