On Tue, Jul 25, 2023 at 11:21:34AM +0200, Andi Shyti wrote:
> Hi Matt,
>
> > +/*
> > + * Wa_22011802037 requires that we (or the GuC) ensure that no command
> > + * streamers are executing MI_FORCE_WAKE while an engine reset is
> > initiated.
> > + */
> > +bool intel_engine_reset_needs_wa_22011802037(struct intel_gt *gt)
>
> I've seen this format in a recent Jonathan's patch and I see it
> becoming a pattern in the future. Should we already agree on the
> naming? Would intel_needs_wa_22011802037() be sufficient? Or a
When a helper like this is static to one function, I usually just use
needs_wa_#####() as a name. But when it's exported and used in several
files, I think it's best to give it a meaningful prefix where possible.
In this case intel_reset.c doesn't use a consistent namespace like some
of our other files, but intel_engine_reset_* seemed like an appropriate
prefix that clarifies where this code comes from and what it's general
scope is.
> prefix as intel_wa_* for all the similar functions?
I had a series a year or two ago that disassociated workaround bounds
from workaround implementations and changed all workaround conditions
into something like 'if (I915_WA(foo))' but we ultimately abandoned that
on i915 and shifted the effort over to the Xe driver instead (where the
"OOB" workarounds follow a somewhat similar idea).
Matt
>
> Andi
>
> > +{
> > + if (GRAPHICS_VER(gt->i915) < 11)
> > + return false;
> > +
> > + if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0))
> > + return true;
> > +
> > + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70))
> > + return false;
> > +
> > + return true;
> > +}
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
