On Wed, Apr 19, 2023 at 02:40:33PM -0700, Matt Atwood wrote:
> On Tue, Apr 18, 2023 at 03:04:45PM -0700, Radhakrishna Sripada wrote:
> > From: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@xxxxxxxxx>
> >
> > Wa_22011802037 was being applied to all graphics_ver 11 & 12. This patch
> > updates the if statement to apply the W/A to right platforms and extends
> > it to MTL-M:A step.
> >
> Bspec: 53509
> > v1.1: Fix checkpatch warning.
> > v2: Change the check to reflect the wa at other palces(Lucas)
> s/palces/places.
> >
> > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
> With that.
> Reviewed-by: Matt Atwood <matthew.s.atwood@xxxxxxxxx>
> > Signed-off-by: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@xxxxxxxxx>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx>
It doesn't look like this patch is complete? It's only changing one
condition for Wa_22011802037, even though there are several in the code.
>From a quick grep, you're still missing updates for at least
guc_ctl_wa_flags, execlists_reset_prepare, and __intel_engine_stop_cs.
Since this workaround is a complicated one that touches so many areas of
the code, and has a complex platform list, it's probably time to factor
the condition out into a needs_wa_22011802037() helper or something.
Matt
> > ---
> > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > index 88e881b100cf..ee3e8352637f 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -1629,16 +1629,16 @@ static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub)
> >
> > static void guc_engine_reset_prepare(struct intel_engine_cs *engine)
> > {
> > - if (!IS_GRAPHICS_VER(engine->i915, 11, 12))
> > - return;
> > -
> > - intel_engine_stop_cs(engine);
> > -
> > /*
> > * Wa_22011802037: In addition to stopping the cs, we need
> > * to wait for any pending mi force wakeups
> > */
> > - intel_engine_wait_for_pending_mi_fw(engine);
> > + if (IS_MTL_GRAPHICS_STEP(engine->i915, M, STEP_A0, STEP_B0) ||
> > + (GRAPHICS_VER(engine->i915) >= 11 &&
> > + GRAPHICS_VER_FULL(engine->i915) < IP_VER(12, 70))) {
> > + intel_engine_stop_cs(engine);
> > + intel_engine_wait_for_pending_mi_fw(engine);
> > + }
> > }
> >
> > static void guc_reset_nop(struct intel_engine_cs *engine)
> > --
> > 2.34.1
> >
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
