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.demar...@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.
