Re: [PATCH v6 2/3] drm/i915: Parse and set stepping for platforms with GMD

From: Lucas De Marchi <lucas.demarchi@intel.com>
To: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v6 2/3] drm/i915: Parse and set stepping for platforms with GMD
Date: Fri, 23 Sep 2022 10:45:07 -0700	[thread overview]
Message-ID: <20220923174507.b5vzhxbpjkzhbfeu@ldmartin-desk2.lan> (raw)
In-Reply-To: <20220916014648.1310346-3-radhakrishna.sripada@intel.com>

On Thu, Sep 15, 2022 at 06:46:47PM -0700, Radhakrishna Sripada wrote:
>From: José Roberto de Souza <jose.souza@intel.com>
>
>Expand the current stepping convention to accommodate the GMD
>stepping info. Typically GMD step maps to letter stepping
>by "A + step %4" and number to "A + step /4" i.e, GMD step
>0 maps to STEP_A0, 1 to _A1, 2 to _A2, 3 to _A3, 4 to STEP_B0...
>
>Future platforms might break this formulae and may require a table
>mapping to decode GMD step compatible with the convention.
>
>v2:
> - Pass the updated ip version structure
>v3:
> - Skip using GMD to step table(MattR)
>
>Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>Cc: Matt Roper <matthew.d.roper@intel.com>
>Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>---
> drivers/gpu/drm/i915/intel_step.c | 25 +++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_step.h | 24 +++++++++++++++++++++++-
> 2 files changed, 48 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
>index 42b3133d8387..91e7c51991b0 100644
>--- a/drivers/gpu/drm/i915/intel_step.c
>+++ b/drivers/gpu/drm/i915/intel_step.c
>@@ -135,6 +135,19 @@ static const struct intel_step_info adlp_n_revids[] = {
> 	[0x0] = { COMMON_GT_MEDIA_STEP(A0), .display_step = STEP_D0 },
> };
>
>+static u8 gmd_to_intel_step(struct drm_i915_private *i915,
>+			    struct ip_version *gmd)
>+{
>+	u8 step = gmd->step + STEP_A0;
>+
>+	if (step >= STEP_FUTURE) {
>+		drm_dbg(&i915->drm, "Using future steppings\n");
>+		return STEP_FUTURE;
>+	}
>+
>+	return step;
>+}
>+
> static void pvc_step_init(struct drm_i915_private *i915, int pci_revid);
>
> void intel_step_init(struct drm_i915_private *i915)
>@@ -144,6 +157,18 @@ void intel_step_init(struct drm_i915_private *i915)
> 	int revid = INTEL_REVID(i915);
> 	struct intel_step_info step = {};
>
>+	if (HAS_GMD_ID(i915)) {
>+		step.graphics_step = gmd_to_intel_step(i915,
>+						       &RUNTIME_INFO(i915)->graphics.ip);
>+		step.media_step = gmd_to_intel_step(i915,
>+						    &RUNTIME_INFO(i915)->media.ip);
>+		step.display_step = gmd_to_intel_step(i915,
>+						      &RUNTIME_INFO(i915)->display.ip);
>+		RUNTIME_INFO(i915)->step = step;
>+
>+		return;
>+	}
>+
> 	if (IS_PONTEVECCHIO(i915)) {
> 		pvc_step_init(i915, revid);
> 		return;
>diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
>index a6b12bfa9744..57b9928ddca6 100644
>--- a/drivers/gpu/drm/i915/intel_step.h
>+++ b/drivers/gpu/drm/i915/intel_step.h
>@@ -23,21 +23,43 @@ struct intel_step_info {

missing comment in this struct that it's expected to have 4 number steps
per letter. I don't want someone to add a stepping B4 in future without
realizing that will render gmd_to_intel_step()  invalid

with that added: Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi