[Intel-gfx] [PATCH] drm/i915: Flush the CSB pointer reset
mika.kuoppala at linux.intel.com
Fri Apr 12 12:42:30 UTC 2019
Chris Wilson <chris at chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2019-04-12 13:22:12)
>> Chris Wilson <chris at chris-wilson.co.uk> writes:
>> > The HW resets it CSB tail pointer on resetting the engine. Most of the
>> > time. In case it doesn't (and for system resume) we write the expected
>> > value anyway. For extra paranoia, flush the write before we invalidate
>> > the cacheline.
>> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
>> > ---
>> > drivers/gpu/drm/i915/intel_lrc.c | 1 +
>> > 1 file changed, 1 insertion(+)
>> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> > index 3cb788a223ef..4e0a351bfbca 100644
>> > --- a/drivers/gpu/drm/i915/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> > @@ -1866,6 +1866,7 @@ static void reset_csb_pointers(struct intel_engine_execlists *execlists)
>> > */
>> > execlists->csb_head = reset_value;
>> > WRITE_ONCE(*execlists->csb_write, reset_value);
>> > + wmb(); /* Make sure this is visible to HW (paranoia?) */
>> As alternative tho, how about we invalidate not only the csb portion
>> but the all things included in hswp.
> We only pull the CSB into the cache. Easy excuse.
>> Then read the the mmio tail/head,
>> assert they are pointing correctly and match hwsp, if not, correct
>> and log?
> We've just reset the GPU, we know the mmio are garbage (powercontext)
> until the first interrupt after execlists submission. Remember the fun
> we used to have with tracking the interrupt before reading the mmio?
True, the mmio is useless. We could fortify the debugs so
that we expect first csb write to 0. Not that we detected
I was thinking the smb_store_mb() but I like the wmb() better
in this case. Everything before this sync point instead of
Acked-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
More information about the Intel-gfx