Re: [PATCH 1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Apr 19, 2019 at 12:17:47PM +0100, Chris Wilson wrote:
> Despite what I think the prm recommends, commit f2253bd9859b
> ("drm/i915/ringbuffer: EMIT_INVALIDATE after switch context") turned out
> to be a huge mistake when enabling Ironlake contexts as the GPU would
> hang on either a MI_FLUSH or PIPE_CONTROL immediately following the
> MI_SET_CONTEXT of an active mesa context (more vanilla contexts, e.g.
> simple rendercopies with igt, do not suffer).

Where is the recommendation you mention? I couldn't immediately find it
in the docs. I did find the following statemtement:

"[DevCTG+]: For the invalidate operation of the pipe control, the
 following pointers are affected. The
 invalidate operation affects the restore of these packets. If the pipe
 control invalidate operation is completed
 before the context save, the indirect pointers will not be restored from
 memory.
 1. Pipeline State Pointer
 2. Media State Pointer
 3. Constant Buffer Packet"

Which maybe has something to do with this ordering?

But it's all black magic anyways. If it works it works.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3844581f622c..8feb2d9b7b60 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1882,12 +1882,12 @@ static int ring_request_alloc(struct i915_request *request)
>  	 */
>  	request->reserved_space += LEGACY_REQUEST_SIZE;
>  
> -	ret = switch_context(request);
> +	/* Unconditionally invalidate GPU caches and TLBs. */
> +	ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
>  	if (ret)
>  		return ret;
>  
> -	/* Unconditionally invalidate GPU caches and TLBs. */
> -	ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
> +	ret = switch_context(request);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx