On Mon, Jul 17, 2023 at 11:52:25PM +0200, Andi Shyti wrote:
> Hi Matt,
>
> On Mon, Jul 17, 2023 at 01:31:03PM -0700, Matt Roper wrote:
> > On Mon, Jul 17, 2023 at 10:54:37AM -0700, Matt Roper wrote:
> > > On Mon, Jul 17, 2023 at 07:30:55PM +0200, Andi Shyti wrote:
> > > > From: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
> > > >
> > > > All memory traffic must be quiesced before requesting
> > > > an aux invalidation on platforms that use Aux CCS.
> > > >
> > > > Fixes: 972282c4cf24 ("drm/i915/gen12: Add aux table invalidate for all engines")
> > > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
> > > > Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> > > > Cc: <stable@xxxxxxxxxxxxxxx> # v5.8+
> > > > ---
> > > > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 7 +++++++
> > > > 1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> > > > index 563efee055602..bee3b7dc595cf 100644
> > > > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> > > > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> > > > @@ -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.
> > > > + */
> > > > + if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915))
> > >
> > > It's a pre-existing mistake in drm-tip at the moment, but we shouldn't
> > > assume !flatccs always implies auxccs. PVC has neither, and there may
> > > be other similar platforms in the future. We should probably add a
> > > helper function for AuxCCS, similar to what we added to the Xe driver
> > > recently:
> > >
> > > https://patchwork.freedesktop.org/patch/539304/?series=118334&rev=1
>
> Currently that is done in patch 6...
Are you sure? Patch #6 consolidates things a bit, but is still incorrectly
assuming flatccs = !auxccs:
if (HAS_FLAT_CCS(engine->i915))
return _MMIO(0);
>
> > BTW, since this patch didn't handle it I was expecting to see another
> > patch in the series that quiesces memory for the non-RCS/CCS engines,
> > but it looks like there isn't one yet. So we should probably add the
> > necessary MI_FLUSH_DW logic for the other engines to this patch as well.
>
> ... where also other engines are handles as well. I left this
> patch as it is in order to preserve the authorship and it's
> original form.
I don't see it being handled in patch #6. That performs invalidation on
more engines than we were before, but it doesn't add the missing quiesce
logic as far as I can see.
Matt
>
> Maybe in patch 6 I can add the extra check for PVC as you did for
> XE.
>
> Thanks,
> Andi
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
