[Intel-gfx] [PATCH] drm/i915/tgl: Suspend pre-parser across GTT invalidations

[Daniele Ceraolo Spurio]

On 9/14/19 1:25 AM, Chris Wilson wrote:
> Before we execute a batch, we must first issue any and all TLB
> invalidations so that batch picks up the new page table entries.
> Tigerlake's preparser is weakening our post-sync CS_STALL inside the
> invalidate pipe-control and allowing the loading of the batch buffer
> before we have setup its page table (and so it loads the wrong page and
> executes indefinitely).
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
> Suggestions welcome as this does not seem intended behaviour, so I
> suspect there is a strong pipecontrol flag we are missing.

When I discussed the pre-parser with the HW team the feedback I got was 
that the only way to make sure we don't race the memory update is to 
either leave enough CL of space or turn the parser off like you did 
below. That discussion was about actual physical memory access though 
and not TLB.
Anyway, turning off the parser around the pipe control, if it fixes the 
problem, shouldn't be too bad since the main performance advantage of 
the parser is expected inside the user batch. The alternative would 
probably be to stop invalidating the TLBs from within the ring and 
switch to MMIO invalidations when lite-restoring a new request in the 
ring (the CS will implicitly stop the parser and invalidate everything 
on a full ctx switch).

Daniele

> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a3f0e4999744..a9e690c303cc 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2796,11 +2796,14 @@ static int gen11_emit_flush_render(struct i915_request *request,
>   		flags |= PIPE_CONTROL_QW_WRITE;
>   		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
>   
> -		cs = intel_ring_begin(request, 6);
> +		cs = intel_ring_begin(request, 8);
>   		if (IS_ERR(cs))
>   			return PTR_ERR(cs);
>   
> +		*cs++ = MI_ARB_CHECK | 1 << 8 | 1;
>   		cs = gen8_emit_pipe_control(cs, flags, scratch_addr);
> +		*cs++ = MI_ARB_CHECK | 1 << 8;
> +
>   		intel_ring_advance(request, cs);
>   	}
>   
>