[Intel-gfx] [PATCH 1/3] drm/i915/ringbuffer: EMIT_INVALIDATE *before* switch context

[Chris Wilson]

Quoting Ville Syrjälä (2019-04-23 16:36:47)
> 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:

Sadly, it probably Chinese whispers from our code comments. I have
recollection of us having a comment about expected ordering and not
obeying because we "always invalidate before the batch". I have some
recollection of seeing related instructions in the ye old bspec, but I
cannot lay my hands on anything concreter right now.
 
> "[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?

That's very interesting. If the invalidate is preventing the loading of
dangling pointer, then yes, it fits with the observation in this patch.

There's also a comment about ensure we flush media before switching to a
3D context (which we do by virtue of the flush-everything after a
request -- or we should!).

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

Proof is in the eating. Ta,
-Chris