[PATCH] drm/i915/gt: Avoid duplicating CCS mode workaround

[Andi Shyti]

Hi Lucas,

> > > > @@ -2897,7 +2897,9 @@ engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal
> > > >          */
> > > >         if (engine->flags & I915_ENGINE_FIRST_RENDER_COMPUTE) {
> > > >                 general_render_compute_wa_init(engine, wal);
> > > > -               ccs_engine_wa_mode(engine, wal);
> > > > +
> > > > +               if (engine->class == COMPUTE_CLASS)
> > > > +                       ccs_engine_wa_mode(engine, wal);
> > > 
> > > FIRST_RENDER_COMPUTE is meant to only be on the first engine of either
> > > rcs or ccs (which share certain register domains), one engine.
> > > 
> > > It looks like that was broken by
> > > 
> > > 	commit 1bfc03b1375244f9029bb448ee8224b3b6dae99f
> > > 	Author: Lucas De Marchi <lucas.demarchi at intel.com>
> 
> yep, my bad.
> 
> > > 	Date:   Tue Mar 19 23:03:03 2024 -0700
> > > 
> > > 	    drm/i915: Remove special handling for !RCS_MASK()
> > 
> > Aha! So the logic here[*] breaks the meaning of
> > I915_ENGINE_FIRST_RENDER_COMPUTE, becasue, other than PVC, we
> > forgot that we have DG2 that needs the special check that uses
> > !RCS_MASK().
> 
> no, that would mean a DG2 without render engine.

OK, I don't know the history, I thought that the idea was to
remove support for PVC, the only multi-CCS machine that once i915
supported other than DG2.

> The previous check to enable I915_ENGINE_FIRST_RENDER_COMPUTE was:
> 
> 	if ((engine->class == COMPUTE_CLASS && !RCS_MASK(engine->gt) &&
> 	     __ffs(CCS_MASK(engine->gt)) == engine->instance) ||
> 	     engine->class == RENDER_CLASS)
> 
> i.e. if render is fused off, it enables it in the first compute engine.
> Otherwise it enables it in the render.
> 
> And assuming we don't have platforms with render fused off (which still
> holds true as far as I'm aware), that became:
> 
> 	if ((engine->class == COMPUTE_CLASS || engine->class == RENDER_CLASS) &&
> 	    __ffs(CCS_MASK(engine->gt) | RCS_MASK(engine->gt)) == engine->instance)

The difference is that this applies in two cases: it's true for
the first CCS we encounter and also for the only RCS. Arshad
noticed that we end up applying the workaround twice.

So the !RCS_MASK(gt) check is still needed.

Alternatively, as Matt suggested, we could assign
I915_ENGINE_FIRST_RENDER_COMPUTE only to the RCS.

I have a slight preference for the way it was done before because
it make the logic clearer.

Thanks,
Andi

> It was supposed to mean the same thing... but doesn't as engine->instance
> starts from 0 for each class.