[Intel-gfx] [PATCH dii-client v1.1] drm/i915/mtl: Extend Wa_22011802037 to MTL A-step

[Lucas De Marchi]

On Thu, Mar 16, 2023 at 01:41:43PM -0700, Radhakrishna Sripada wrote:
>From: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep at 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 at intel.com>
>Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at 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