| Message ID | 20230316204143.1768749-1-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [dii-client,v1.1] drm/i915/mtl: Extend Wa_22011802037 to MTL A-step | expand |
On Thu, Mar 16, 2023 at 01:41:43PM -0700, Radhakrishna Sripada wrote: >From: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@intel.com> > >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:A0. it should be any A stepping, not just A0. But the code is correct, it's only here that is wrong. btw wrong subject-prefix here, not sure CI will pick it up for execution. > >v1.1: Fix checkpatch warning. > >Signed-off-by: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@intel.com> >Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> >--- > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >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..a099406dcc38 100644 >--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >@@ -1629,7 +1629,9 @@ 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)) >+ 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)))) the double negation + parenthesis + line wrap make it hard to read. It seems that in commit 0667429ce68e ("drm/i915/reset: Add additional steps for Wa_22011802037 for execlist backend") the Wa comment got misplaced as the call to intel_engine_stop_cs() is part of the Wa handling, no? +Umesh Maybe let's change to a positive check and move the Wa comment to be above the check? static void guc_engine_reset_prepare(struct intel_engine_cs *engine) { /* * Wa_22011802037: stop the cs and wait for any pending mi force * wakeups */ if (IS_MTL_GRAPHICS_STEP(gt->i915, M, STEP_A0, STEP_B0) || (GRAPHICS_VER(gt->i915) >= 11 && GRAPHICS_VER_FULL(gt->i915) < IP_VER(12, 70))) { intel_engine_stop_cs(engine); intel_engine_wait_for_pending_mi_fw(engine); } } This matches the condition checked everywhere else in the driver: $ git grep Wa_22011802037 drivers/gpu/drm/i915/gt/intel_engine_cs.c: * Wa_22011802037: Prior to doing a reset, ensure CS is drivers/gpu/drm/i915/gt/intel_engine_cs.c: * Wa_22011802037:gen12: In addition to stopping the cs, we need to wait for any drivers/gpu/drm/i915/gt/intel_execlists_submission.c: * Wa_22011802037: In addition to stopping the cs, we need drivers/gpu/drm/i915/gt/uc/intel_guc.c: /* Wa_22011802037: graphics version 11/12 */ drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c: * Wa_22011802037: In addition to stopping the cs, we need Btw then comments about graphics versions didn't age well: they are not matching the code anymore Lucas De Marchi
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..a099406dcc38 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1629,7 +1629,9 @@ 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)) + 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)))) return; intel_engine_stop_cs(engine);