Hi Nirmoy and Jonathan,
> > > > @@ -202,6 +202,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
> > > > {
> > > > struct intel_engine_cs *engine = rq->engine;
> > > > + /*
> > > > + * Aux invalidations on Aux CCS platforms require
> > > > + * memory traffic is quiesced prior.
> > > I see that we are doing aux inval on EMIT_INVALIDATE so it make sense to
> > >
> > > do if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915) )
> > >
> > This is agreeable, though I don't think there's any instances of us calling gen12_emit_flush_rcs with a blank mode,
> > since that wouldn't accomplish anything. So I don't think the additional check/safety net is necessary, but it doesn't
> > hurt to have.
so... do we agree here that we don't add anything? I don't really
mind...
Or, I can queue up a patch 5 adding this "pedantic" check and we
can discuss it separately.
> > > > + */
> > > > + if (!HAS_FLAT_CCS(engine->i915))
> > > > + mode |= EMIT_FLUSH;
> > > I think this generic EMIT_FLUSH is not enough. I seeing some missing
> > > flags for PIPE_CONTROL
> > >
> > > As per https://gfxspecs.intel.com/Predator/Home/Index/43904. It makes
> > > sense to move this to a
> > >
> > > new function given the complexity of PIPE_CONTROL flags requires for this.
> > >
> > I'm assuming when you're talking about the missing flags for PIPE_CONTROL, you're
> > referring to CCS Flush, correct? Because every other flag is already covered in the
> > EMIT_FLUSH path.
>
> Yes, CCS Flush and I don't see a L3 fabric flush as well.
>
>
> >
> > I feel like I had this conversation with Matt while the internal version was
> > developed back in February, and the consensus was that the CCS Flush
> > requirement was already covered.
>
> Wasn't aware of this, would be nice to have a confirmation and a comment so
> we
>
> don't get confused in future.
>
> > On the other hand, it looks like the CCS Flush
> > requirement was only recently added back in May, so it might be worth
> > double-checking at the very least.
> >
> > Although... if CCS Flush is a missing flag, I wonder how we're supposed to set it,
> > as there doesn’t appear to be a definition for such a flag in intel_gpu_commands.h...
>
>
> Yes, not yet but we should add a flag for that.
Is it OK if I add in the comment that EMIT_FLUSH covers the CCS
flushing?
Andi
