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

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@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@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@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.

-Mika


>  	process_csb(engine); /* drain preemption events */
>  
>  	/* Following the reset, we need to reload the CSB read/write pointers */
> -- 
> 2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx