Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: Jani Nikula <jani.nikula@intel.com>,
	intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Cc: lucas.demarchi@intel.com, rodrigo.vivi@intel.com,
	matthew.d.roper@intel.com
Subject: [PATCH v2] drm/i915/display: identify display steppings in display probe
Date: Wed, 21 Aug 2024 12:50:36 +0300	[thread overview]
Message-ID: <20240821095036.2044654-1-jani.nikula@intel.com> (raw)
In-Reply-To: <65e319048c369fbbd5018badfa24e457ca4e956e.1724180287.git.jani.nikula@intel.com>

Both i915 and xe have code to identify display steppings. Start
deduplicating this by, uh, adding a third copy in display code. This is
not yet used for anything other than debug logging. We'll switch over
later.

For platforms before GMD ID, attach the mapping from PCI revision to
stepping in the platform and subplatform descriptors. This is a
considerably cleaner approach than having it completely separate.

Also add a separate field for stepping in display runtime info,
preserving the value from GMD ID.

v2: Handle NULL subdesc (Matt)

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_display_device.c   | 225 +++++++++++++++++-
 .../drm/i915/display/intel_display_device.h   |   3 +-
 .../drm/xe/compat-i915-headers/intel_step.h   |   1 +
 3 files changed, 217 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
index b28d55fa0c3a..dfa923672492 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.c
+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
@@ -16,14 +16,25 @@
 #include "intel_display_power.h"
 #include "intel_display_reg_defs.h"
 #include "intel_fbc.h"
+#include "intel_step.h"
 
 __diag_push();
 __diag_ignore_all("-Woverride-init", "Allow field initialization overrides for display info");
 
+struct stepping_desc {
+	const enum intel_step *map; /* revid to step map */
+	size_t size; /* map size */
+};
+
+#define STEP_INFO(_map)				\
+	.step_info.map = _map,			\
+	.step_info.size = ARRAY_SIZE(_map)
+
 struct subplatform_desc {
 	enum intel_display_subplatform subplatform;
 	const char *name;
 	const u16 *pciidlist;
+	struct stepping_desc step_info;
 };
 
 struct platform_desc {
@@ -31,6 +42,7 @@ struct platform_desc {
 	const char *name;
 	const struct subplatform_desc *subplatforms;
 	const struct intel_display_device_info *info; /* NULL for GMD ID */
+	struct stepping_desc step_info;
 };
 
 #define PLATFORM(_platform)			 \
@@ -610,6 +622,13 @@ static const u16 skl_ulx_ids[] = {
 	0
 };
 
+static const enum intel_step skl_steppings[] = {
+	[0x6] = STEP_G0,
+	[0x7] = STEP_H0,
+	[0x9] = STEP_J0,
+	[0xA] = STEP_I1,
+};
+
 static const struct platform_desc skl_desc = {
 	PLATFORM(SKYLAKE),
 	.subplatforms = (const struct subplatform_desc[]) {
@@ -618,6 +637,7 @@ static const struct platform_desc skl_desc = {
 		{},
 	},
 	.info = &skl_display,
+	STEP_INFO(skl_steppings),
 };
 
 static const u16 kbl_ult_ids[] = {
@@ -634,6 +654,16 @@ static const u16 kbl_ulx_ids[] = {
 	0
 };
 
+static const enum intel_step kbl_steppings[] = {
+	[1] = STEP_B0,
+	[2] = STEP_B0,
+	[3] = STEP_B0,
+	[4] = STEP_C0,
+	[5] = STEP_B1,
+	[6] = STEP_B1,
+	[7] = STEP_C0,
+};
+
 static const struct platform_desc kbl_desc = {
 	PLATFORM(KABYLAKE),
 	.subplatforms = (const struct subplatform_desc[]) {
@@ -642,6 +672,7 @@ static const struct platform_desc kbl_desc = {
 		{},
 	},
 	.info = &skl_display,
+	STEP_INFO(kbl_steppings),
 };
 
 static const u16 cfl_ult_ids[] = {
@@ -706,6 +737,13 @@ static const struct platform_desc cml_desc = {
 		BIT(TRANSCODER_DSI_A) | BIT(TRANSCODER_DSI_C), \
 	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C)
 
+static const enum intel_step bxt_steppings[] = {
+	[0xA] = STEP_C0,
+	[0xB] = STEP_C0,
+	[0xC] = STEP_D0,
+	[0xD] = STEP_E0,
+};
+
 static const struct platform_desc bxt_desc = {
 	PLATFORM(BROXTON),
 	.info = &(const struct intel_display_device_info) {
@@ -714,6 +752,11 @@ static const struct platform_desc bxt_desc = {
 
 		.__runtime_defaults.ip.ver = 9,
 	},
+	STEP_INFO(bxt_steppings),
+};
+
+static const enum intel_step glk_steppings[] = {
+	[3] = STEP_B0,
 };
 
 static const struct platform_desc glk_desc = {
@@ -725,6 +768,7 @@ static const struct platform_desc glk_desc = {
 
 		.__runtime_defaults.ip.ver = 10,
 	},
+	STEP_INFO(glk_steppings),
 };
 
 #define ICL_DISPLAY \
@@ -773,6 +817,10 @@ static const u16 icl_port_f_ids[] = {
 	0
 };
 
+static const enum intel_step icl_steppings[] = {
+	[7] = STEP_D0,
+};
+
 static const struct platform_desc icl_desc = {
 	PLATFORM(ICELAKE),
 	.subplatforms = (const struct subplatform_desc[]) {
@@ -784,6 +832,7 @@ static const struct platform_desc icl_desc = {
 
 		.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D) | BIT(PORT_E),
 	},
+	STEP_INFO(icl_steppings),
 };
 
 static const struct intel_display_device_info jsl_ehl_display = {
@@ -792,14 +841,21 @@ static const struct intel_display_device_info jsl_ehl_display = {
 	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D),
 };
 
+static const enum intel_step jsl_ehl_steppings[] = {
+	[0] = STEP_A0,
+	[1] = STEP_B0,
+};
+
 static const struct platform_desc jsl_desc = {
 	PLATFORM(JASPERLAKE),
 	.info = &jsl_ehl_display,
+	STEP_INFO(jsl_ehl_steppings),
 };
 
 static const struct platform_desc ehl_desc = {
 	PLATFORM(ELKHARTLAKE),
 	.info = &jsl_ehl_display,
+	STEP_INFO(jsl_ehl_steppings),
 };
 
 #define XE_D_DISPLAY \
@@ -850,10 +906,23 @@ static const u16 tgl_uy_ids[] = {
 	0
 };
 
+static const enum intel_step tgl_steppings[] = {
+	[0] = STEP_B0,
+	[1] = STEP_D0,
+};
+
+static const enum intel_step tgl_uy_steppings[] = {
+	[0] = STEP_A0,
+	[1] = STEP_C0,
+	[2] = STEP_C0,
+	[3] = STEP_D0,
+};
+
 static const struct platform_desc tgl_desc = {
 	PLATFORM(TIGERLAKE),
 	.subplatforms = (const struct subplatform_desc[]) {
-		{ INTEL_DISPLAY_TIGERLAKE_UY, "UY", tgl_uy_ids },
+		{ INTEL_DISPLAY_TIGERLAKE_UY, "UY", tgl_uy_ids,
+		  STEP_INFO(tgl_uy_steppings) },
 		{},
 	},
 	.info = &(const struct intel_display_device_info) {
@@ -866,6 +935,12 @@ static const struct platform_desc tgl_desc = {
 		.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) |
 		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4) | BIT(PORT_TC5) | BIT(PORT_TC6),
 	},
+	STEP_INFO(tgl_steppings),
+};
+
+static const enum intel_step dg1_steppings[] = {
+	[0] = STEP_A0,
+	[1] = STEP_B0,
 };
 
 static const struct platform_desc dg1_desc = {
@@ -876,6 +951,13 @@ static const struct platform_desc dg1_desc = {
 		.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) |
 		BIT(PORT_TC1) | BIT(PORT_TC2),
 	},
+	STEP_INFO(dg1_steppings),
+};
+
+static const enum intel_step rkl_steppings[] = {
+	[0] = STEP_A0,
+	[1] = STEP_B0,
+	[4] = STEP_C0,
 };
 
 static const struct platform_desc rkl_desc = {
@@ -892,6 +974,7 @@ static const struct platform_desc rkl_desc = {
 		.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) |
 		BIT(PORT_TC1) | BIT(PORT_TC2),
 	},
+	STEP_INFO(rkl_steppings),
 };
 
 static const u16 adls_rpls_ids[] = {
@@ -899,10 +982,24 @@ static const u16 adls_rpls_ids[] = {
 	0
 };
 
+static const enum intel_step adl_s_steppings[] = {
+	[0x0] = STEP_A0,
+	[0x1] = STEP_A2,
+	[0x4] = STEP_B0,
+	[0x8] = STEP_B0,
+	[0xC] = STEP_C0,
+};
+
+static const enum intel_step adl_s_rpl_s_steppings[] = {
+	[0x4] = STEP_D0,
+	[0xC] = STEP_C0,
+};
+
 static const struct platform_desc adl_s_desc = {
 	PLATFORM(ALDERLAKE_S),
 	.subplatforms = (const struct subplatform_desc[]) {
-		{ INTEL_DISPLAY_ALDERLAKE_S_RAPTORLAKE_S, "RPL-S", adls_rpls_ids },
+		{ INTEL_DISPLAY_ALDERLAKE_S_RAPTORLAKE_S, "RPL-S", adls_rpls_ids,
+		  STEP_INFO(adl_s_rpl_s_steppings) },
 		{},
 	},
 	.info = &(const struct intel_display_device_info) {
@@ -913,6 +1010,7 @@ static const struct platform_desc adl_s_desc = {
 		.__runtime_defaults.port_mask = BIT(PORT_A) |
 		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4),
 	},
+	STEP_INFO(adl_s_steppings),
 };
 
 #define XE_LPD_FEATURES \
@@ -986,15 +1084,34 @@ static const u16 adlp_rplp_ids[] = {
 	0
 };
 
+static const enum intel_step adl_p_steppings[] = {
+	[0x0] = STEP_A0,
+	[0x4] = STEP_B0,
+	[0x8] = STEP_C0,
+	[0xC] = STEP_D0,
+};
+
+static const enum intel_step adl_p_adl_n_steppings[] = {
+	[0x0] = STEP_D0,
+};
+
+static const enum intel_step adl_p_rpl_pu_steppings[] = {
+	[0x4] = STEP_E0,
+};
+
 static const struct platform_desc adl_p_desc = {
 	PLATFORM(ALDERLAKE_P),
 	.subplatforms = (const struct subplatform_desc[]) {
-		{ INTEL_DISPLAY_ALDERLAKE_P_ALDERLAKE_N, "ADL-N", adlp_adln_ids },
-		{ INTEL_DISPLAY_ALDERLAKE_P_RAPTORLAKE_U, "RPL-U", adlp_rplu_ids },
-		{ INTEL_DISPLAY_ALDERLAKE_P_RAPTORLAKE_P, "RPL-P", adlp_rplp_ids },
+		{ INTEL_DISPLAY_ALDERLAKE_P_ALDERLAKE_N, "ADL-N", adlp_adln_ids,
+		  STEP_INFO(adl_p_adl_n_steppings) },
+		{ INTEL_DISPLAY_ALDERLAKE_P_RAPTORLAKE_P, "RPL-P", adlp_rplp_ids,
+		  STEP_INFO(adl_p_rpl_pu_steppings) },
+		{ INTEL_DISPLAY_ALDERLAKE_P_RAPTORLAKE_U, "RPL-U", adlp_rplu_ids,
+		  STEP_INFO(adl_p_rpl_pu_steppings) },
 		{},
 	},
 	.info = &xe_lpd_display,
+	STEP_INFO(adl_p_steppings),
 };
 
 static const struct intel_display_device_info xe_hpd_display = {
@@ -1023,12 +1140,33 @@ static const u16 dg2_g12_ids[] = {
 	0
 };
 
+static const enum intel_step dg2_g10_steppings[] = {
+	[0x0] = STEP_A0,
+	[0x1] = STEP_A0,
+	[0x4] = STEP_B0,
+	[0x8] = STEP_C0,
+};
+
+static const enum intel_step dg2_g11_steppings[] = {
+	[0x0] = STEP_B0,
+	[0x4] = STEP_C0,
+	[0x5] = STEP_C0,
+};
+
+static const enum intel_step dg2_g12_steppings[] = {
+	[0x0] = STEP_C0,
+	[0x1] = STEP_C0,
+};
+
 static const struct platform_desc dg2_desc = {
 	PLATFORM(DG2),
 	.subplatforms = (const struct subplatform_desc[]) {
-		{ INTEL_DISPLAY_DG2_G10, "G10", dg2_g10_ids },
-		{ INTEL_DISPLAY_DG2_G11, "G11", dg2_g11_ids },
-		{ INTEL_DISPLAY_DG2_G12, "G12", dg2_g12_ids },
+		{ INTEL_DISPLAY_DG2_G10, "G10", dg2_g10_ids,
+		  STEP_INFO(dg2_g10_steppings) },
+		{ INTEL_DISPLAY_DG2_G11, "G11", dg2_g11_ids,
+		  STEP_INFO(dg2_g11_steppings) },
+		{ INTEL_DISPLAY_DG2_G12, "G12", dg2_g12_ids,
+		  STEP_INFO(dg2_g12_steppings) },
 		{},
 	},
 	.info = &xe_hpd_display,
@@ -1261,13 +1399,66 @@ find_subplatform_desc(struct pci_dev *pdev, const struct platform_desc *desc)
 	return NULL;
 }
 
+static enum intel_step get_pre_gmdid_step(struct intel_display *display,
+					  const struct stepping_desc *main,
+					  const struct stepping_desc *sub)
+{
+	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
+	const enum intel_step *map = main->map;
+	int size = main->size;
+	int revision = pdev->revision;
+	enum intel_step step;
+
+	/* subplatform stepping info trumps main platform info */
+	if (sub && sub->map && sub->size) {
+		map = sub->map;
+		size = sub->size;
+	}
+
+	/* not all platforms define steppings, and it's fine */
+	if (!map || !size)
+		return STEP_NONE;
+
+	if (revision < size && map[revision] != STEP_NONE) {
+		step = map[revision];
+	} else {
+		drm_warn(display->drm, "Unknown revision 0x%02x\n", revision);
+
+		/*
+		 * If we hit a gap in the revision to step map, use the information
+		 * for the next revision.
+		 *
+		 * This may be wrong in all sorts of ways, especially if the
+		 * steppings in the array are not monotonically increasing, but
+		 * it's better than defaulting to 0.
+		 */
+		while (revision < size && map[revision] == STEP_NONE)
+			revision++;
+
+		if (revision < size) {
+			drm_dbg_kms(display->drm, "Using display stepping for revision 0x%02x\n",
+				    revision);
+			step = map[revision];
+		} else {
+			drm_dbg_kms(display->drm, "Using future display stepping\n");
+			step = STEP_FUTURE;
+		}
+	}
+
+	drm_WARN_ON(display->drm, step == STEP_NONE);
+
+	return step;
+}
+
 void intel_display_device_probe(struct drm_i915_private *i915)
 {
+	struct intel_display *display = &i915->display;
 	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
 	const struct intel_display_device_info *info;
 	struct intel_display_ip_ver ip_ver = {};
 	const struct platform_desc *desc;
 	const struct subplatform_desc *subdesc;
+	enum intel_step step;
 
 	/* Add drm device backpointer as early as possible. */
 	i915->display.drm = &i915->drm;
@@ -1307,13 +1498,25 @@ void intel_display_device_probe(struct drm_i915_private *i915)
 		DISPLAY_RUNTIME_INFO(i915)->subplatform = subdesc->subplatform;
 	}
 
-	if (ip_ver.ver || ip_ver.rel || ip_ver.step)
+	if (ip_ver.ver || ip_ver.rel || ip_ver.step) {
 		DISPLAY_RUNTIME_INFO(i915)->ip = ip_ver;
+		step = STEP_A0 + ip_ver.step;
+		if (step > STEP_FUTURE) {
+			drm_dbg_kms(display->drm, "Using future display stepping\n");
+			step = STEP_FUTURE;
+		}
+	} else {
+		step = get_pre_gmdid_step(display, &desc->step_info,
+					  subdesc ? &subdesc->step_info : NULL);
+	}
+
+	DISPLAY_RUNTIME_INFO(i915)->step = step;
 
-	drm_info(&i915->drm, "Found %s%s%s (device ID %04x) display version %u.%02u\n",
+	drm_info(&i915->drm, "Found %s%s%s (device ID %04x) display version %u.%02u stepping %s\n",
 		 desc->name, subdesc ? "/" : "", subdesc ? subdesc->name : "",
 		 pdev->device, DISPLAY_RUNTIME_INFO(i915)->ip.ver,
-		 DISPLAY_RUNTIME_INFO(i915)->ip.rel);
+		 DISPLAY_RUNTIME_INFO(i915)->ip.rel,
+		 step != STEP_NONE ? intel_step_name(step) : "N/A");
 
 	return;
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index ccf1710cb9df..4615c3ba60aa 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -201,8 +201,9 @@ struct intel_display_runtime_info {
 	struct intel_display_ip_ver {
 		u16 ver;
 		u16 rel;
-		u16 step;
+		u16 step; /* hardware */
 	} ip;
+	int step; /* symbolic */
 
 	u32 rawclk_freq;
 
diff --git a/drivers/gpu/drm/xe/compat-i915-headers/intel_step.h b/drivers/gpu/drm/xe/compat-i915-headers/intel_step.h
index ee3f45b668b9..2cf13a572ab0 100644
--- a/drivers/gpu/drm/xe/compat-i915-headers/intel_step.h
+++ b/drivers/gpu/drm/xe/compat-i915-headers/intel_step.h
@@ -8,6 +8,7 @@
 
 #include "xe_step.h"
 
+#define intel_step xe_step
 #define intel_step_name xe_step_name
 
 #endif /* __INTEL_STEP_H__ */
-- 
2.39.2


  parent reply	other threads:[~2024-08-21  9:50 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-20 19:00 [PATCH 00/10] drm/i915/display: identify display steppings in display code Jani Nikula
2024-08-20 19:00 ` [PATCH 01/10] drm/xe/display: fix compat IS_DISPLAY_STEP() range end Jani Nikula
2024-08-20 19:31   ` Lucas De Marchi
2024-08-20 22:29     ` Matt Roper
2024-08-20 19:00 ` [PATCH 02/10] drm/xe/display: remove intel_display_step_name() to simplify Jani Nikula
2024-08-20 22:32   ` Matt Roper
2024-08-20 19:00 ` [PATCH 03/10] drm/xe/display: remove the unused compat HAS_GMD_ID() Jani Nikula
2024-08-20 22:40   ` Matt Roper
2024-08-20 19:00 ` [PATCH 04/10] drm/xe/step: define more steppings E-J Jani Nikula
2024-08-20 22:48   ` Matt Roper
2024-08-20 19:00 ` [PATCH 05/10] drm/i915/display: rename IS_DISPLAY_IP_RANGE() to IS_DISPLAY_VER_FULL() Jani Nikula
2024-08-20 22:54   ` Matt Roper
2024-08-20 19:00 ` [PATCH 06/10] drm/i915/display: rename IS_DISPLAY_IP_STEP() to IS_DISPLAY_VER_STEP() Jani Nikula
2024-08-20 22:56   ` Matt Roper
2024-08-20 19:00 ` [PATCH 07/10] drm/i915/display: identify display steppings in display probe Jani Nikula
2024-08-20 23:52   ` Matt Roper
2024-08-21  8:59     ` Jani Nikula
2024-08-21  9:50   ` Jani Nikula [this message]
2024-08-21 13:48     ` [PATCH v2] " Matt Roper
2024-08-21 13:52     ` Gustavo Sousa
2024-08-21 14:30       ` Jani Nikula
2024-08-20 19:00 ` [PATCH 08/10] drm/i915/display: switch to display detected steppings Jani Nikula
2024-08-21  0:01   ` Matt Roper
2024-08-20 19:00 ` [PATCH 09/10] drm/i915: remove display stepping handling Jani Nikula
2024-08-21  0:04   ` Matt Roper
2024-08-21  0:08     ` Matt Roper
2024-08-20 19:00 ` [PATCH 10/10] drm/xe: " Jani Nikula
2024-08-21  0:04   ` Matt Roper
2024-08-20 19:30 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: identify display steppings in display code Patchwork
2024-08-20 19:30 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-08-20 19:40 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-08-21 10:38 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: identify display steppings in display code (rev2) Patchwork
2024-08-21 10:38 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-08-21 10:49 ` ✓ Fi.CI.BAT: success " Patchwork
2024-08-21 12:22 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-08-22 10:52 ` [PATCH 00/10] drm/i915/display: identify display steppings in display code Jani Nikula
2024-08-22 12:32   ` Lucas De Marchi
2024-08-22 13:29     ` Jani Nikula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240821095036.2044654-1-jani.nikula@intel.com \
    --to=jani.nikula@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=matthew.d.roper@intel.com \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox