[Intel-gfx] [PATCH 1/2] drm/i915/execlists: Add a paranoid flush of the CSB pointers upon reset

[Chris Wilson]

Quoting Mika Kuoppala (2019-09-12 08:51:38)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > After a GPU reset, we need to drain all the CS events so that we have an
> > accurate picture of the execlists state at the time of the reset. Be
> > paranoid and force a read of the CSB write pointer from memory.
> >
> > 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 | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 3d83c7e0d9de..61a38a4ccbca 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2836,6 +2836,10 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
> >       struct i915_request *rq;
> >       u32 *regs;
> >  
> > +     mb(); /* paranoia: read the CSB pointers from after the reset */
> > +     clflush(execlists->csb_write);
> > +     mb();
> > +
> 
> We know there is always a cost. We do invalidate the csb
> on each pass on process_csb.
> 
> Add csb_write in to invalidate_csb entries along
> with mbs. Rename it to invalidate_csb and use it
> always?
> 
> By doing so, we could prolly throw out the rmb() at
> the start of the process_csb as we would have invalidated
> the write pointer along with the entries we read,
> on previous pass.

No. That rmb is essential for the read ordering at that moment in time.

All I have in mind here is a delay, not really a barrier per se, just
this is a nice way of saying no speculation either.
-Chris