[Intel-gfx] [PATCH v2 1/9] drm/i915: Consolidate condition for Wa_22011802037

[Matt Roper]

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