[Intel-gfx] [PATCH 00/10] Get stepping info from RUNTIME_INFO->step

[Intel-gfx] [PATCH 00/10] Get stepping info from RUNTIME_INFO->step

[Anusha Srivatsa]

The changes are added on top of Matt's series:
https://patchwork.freedesktop.org/series/92299/
This series modifies the way we get stepping indo for DMC 
to load the right firmware for the right stepping/substepping
combinations.

Since we have a lookup table for BXT in intel_dmc.c and BXT
stepping changes were missing from Matt's series, I have added a
patch for it.

Anusha Srivatsa (3):
  drm/i915/bxt: Use revid->stepping tables
  drm/i915/step: Add intel_step_name() helper
  drm/i915/dmc: Modify intel_get_stepping_info()

Matt Roper (7):
  drm/i915: Make pre-production detection use direct revid comparison
  drm/i915/skl: Use revid->stepping tables
  drm/i915/icl: Use revid->stepping tables
  drm/i915/jsl_ehl: Use revid->stepping tables
  drm/i915/rkl: Use revid->stepping tables
  drm/i915/dg1: Use revid->stepping tables
  drm/i915/cnl: Drop all workarounds

 .../drm/i915/display/intel_display_power.c    |   2 +-
 drivers/gpu/drm/i915/display/intel_dmc.c      |  51 ++-----
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   2 +-
 drivers/gpu/drm/i915/display/intel_psr.c      |   4 +-
 drivers/gpu/drm/i915/gt/intel_region_lmem.c   |   2 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c   |  81 ++--------
 drivers/gpu/drm/i915/i915_drv.c               |   8 +-
 drivers/gpu/drm/i915/i915_drv.h               |  80 ++--------
 drivers/gpu/drm/i915/intel_pm.c               |   2 +-
 drivers/gpu/drm/i915/intel_step.c             | 142 +++++++++++++++++-
 drivers/gpu/drm/i915/intel_step.h             |   8 +
 11 files changed, 187 insertions(+), 195 deletions(-)

-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 01/10] drm/i915: Make pre-production detection use direct revid comparison

[Anusha Srivatsa]

From: Matt Roper <matthew.d.roper@intel.com>

Although we're converting our workarounds to use a revid->stepping
lookup table, the function that detects pre-production hardware should
continue to compare against PCI revision ID values directly.  These are
listed in the bspec as integers, so it's easier to confirm their
correctness if we just use an integer literal rather than a symbolic
name anyway.

Since the BXT, GLK, and CNL revid macros were never used in any
workaround code, just remove them completely.

Bspec: 13620, 19131, 13626, 18329
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c   |  8 ++++----
 drivers/gpu/drm/i915/i915_drv.h   | 24 ------------------------
 drivers/gpu/drm/i915/intel_step.h |  1 +
 3 files changed, 5 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 30d8cd8c69b1..90136995f5eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -271,10 +271,10 @@ static void intel_detect_preproduction_hw(struct drm_i915_private *dev_priv)
 	bool pre = false;
 
 	pre |= IS_HSW_EARLY_SDV(dev_priv);
-	pre |= IS_SKL_REVID(dev_priv, 0, SKL_REVID_F0);
-	pre |= IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST);
-	pre |= IS_KBL_GT_STEP(dev_priv, 0, STEP_A0);
-	pre |= IS_GLK_REVID(dev_priv, 0, GLK_REVID_A2);
+	pre |= IS_SKYLAKE(dev_priv) && INTEL_REVID(dev_priv) < 0x6;
+	pre |= IS_BROXTON(dev_priv) && INTEL_REVID(dev_priv) < 0xA;
+	pre |= IS_KABYLAKE(dev_priv) && INTEL_REVID(dev_priv) < 0x1;
+	pre |= IS_GEMINILAKE(dev_priv) && INTEL_REVID(dev_priv) < 0x3;
 
 	if (pre) {
 		drm_err(&dev_priv->drm, "This is a pre-production stepping. "
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d14cda2ff923..4f2a61cb024a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1520,35 +1520,11 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 
 #define IS_SKL_REVID(p, since, until) (IS_SKYLAKE(p) && IS_REVID(p, since, until))
 
-#define BXT_REVID_A0		0x0
-#define BXT_REVID_A1		0x1
-#define BXT_REVID_B0		0x3
-#define BXT_REVID_B_LAST	0x8
-#define BXT_REVID_C0		0x9
-
-#define IS_BXT_REVID(dev_priv, since, until) \
-	(IS_BROXTON(dev_priv) && IS_REVID(dev_priv, since, until))
-
 #define IS_KBL_GT_STEP(dev_priv, since, until) \
 	(IS_KABYLAKE(dev_priv) && IS_GT_STEP(dev_priv, since, until))
 #define IS_KBL_DISPLAY_STEP(dev_priv, since, until) \
 	(IS_KABYLAKE(dev_priv) && IS_DISPLAY_STEP(dev_priv, since, until))
 
-#define GLK_REVID_A0		0x0
-#define GLK_REVID_A1		0x1
-#define GLK_REVID_A2		0x2
-#define GLK_REVID_B0		0x3
-
-#define IS_GLK_REVID(dev_priv, since, until) \
-	(IS_GEMINILAKE(dev_priv) && IS_REVID(dev_priv, since, until))
-
-#define CNL_REVID_A0		0x0
-#define CNL_REVID_B0		0x1
-#define CNL_REVID_C0		0x2
-
-#define IS_CNL_REVID(p, since, until) \
-	(IS_CANNONLAKE(p) && IS_REVID(p, since, until))
-
 #define ICL_REVID_A0		0x0
 #define ICL_REVID_A2		0x1
 #define ICL_REVID_B0		0x3
diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
index 958a8bb5d677..8efacef6ab31 100644
--- a/drivers/gpu/drm/i915/intel_step.h
+++ b/drivers/gpu/drm/i915/intel_step.h
@@ -22,6 +22,7 @@ struct intel_step_info {
 enum intel_step {
 	STEP_NONE = 0,
 	STEP_A0,
+	STEP_A1,
 	STEP_A2,
 	STEP_B0,
 	STEP_B1,
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 02/10] drm/i915/skl: Use revid->stepping tables

[Anusha Srivatsa]

From: Matt Roper <matthew.d.roper@intel.com>

Switch SKL to use a revid->stepping table as we're trying to do on all
platforms going forward.  Also add some additional stepping definitions
for completeness, even if we don't have any workarounds tied to them.

Note that SKL has a case where a newer revision ID corresponds to an
older GT/disp stepping (0x9 -> STEP_J0, 0xA -> STEP_I1).  Also, the lack
of a revision ID 0x8 in the table is intentional and not an oversight.
We'll re-write the KBL-specific comment to make it clear that these kind
of quirks are expected.

Finally, since we're already touching the KBL area too, let's rename the
KBL table to match the naming convention used by all of the other
platforms.

Bspec: 13626
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c |  2 +-
 drivers/gpu/drm/i915/i915_drv.h             | 11 +------
 drivers/gpu/drm/i915/intel_step.c           | 35 ++++++++++++++++-----
 drivers/gpu/drm/i915/intel_step.h           |  4 +++
 4 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index d9a5a445ceec..6dfd564e078f 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -883,7 +883,7 @@ skl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 		    GEN8_EU_GAUNIT_CLOCK_GATE_DISABLE);
 
 	/* WaInPlaceDecompressionHang:skl */
-	if (IS_SKL_REVID(i915, SKL_REVID_H0, REVID_FOREVER))
+	if (IS_SKL_GT_STEP(i915, STEP_H0, STEP_FOREVER))
 		wa_write_or(wal,
 			    GEN9_GAMT_ECO_REG_RW_IA,
 			    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4f2a61cb024a..775057626ee6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1509,16 +1509,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define IS_TGL_Y(dev_priv) \
 	IS_SUBPLATFORM(dev_priv, INTEL_TIGERLAKE, INTEL_SUBPLATFORM_ULX)
 
-#define SKL_REVID_A0		0x0
-#define SKL_REVID_B0		0x1
-#define SKL_REVID_C0		0x2
-#define SKL_REVID_D0		0x3
-#define SKL_REVID_E0		0x4
-#define SKL_REVID_F0		0x5
-#define SKL_REVID_G0		0x6
-#define SKL_REVID_H0		0x7
-
-#define IS_SKL_REVID(p, since, until) (IS_SKYLAKE(p) && IS_REVID(p, since, until))
+#define IS_SKL_GT_STEP(p, since, until) (IS_SKYLAKE(p) && IS_GT_STEP(p, since, until))
 
 #define IS_KBL_GT_STEP(dev_priv, since, until) \
 	(IS_KABYLAKE(dev_priv) && IS_GT_STEP(dev_priv, since, until))
diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
index ba9479a67521..bfd63f56c200 100644
--- a/drivers/gpu/drm/i915/intel_step.c
+++ b/drivers/gpu/drm/i915/intel_step.c
@@ -7,15 +7,31 @@
 #include "intel_step.h"
 
 /*
- * KBL revision ID ordering is bizarre; higher revision ID's map to lower
- * steppings in some cases.  So rather than test against the revision ID
- * directly, let's map that into our own range of increasing ID's that we
- * can test against in a regular manner.
+ * Some platforms have unusual ways of mapping PCI revision ID to GT/display
+ * steppings.  E.g., in some cases a higher PCI revision may translate to a
+ * lower stepping of the GT and/or display IP.  This file provides lookup
+ * tables to map the PCI revision into a standard set of stepping values that
+ * can be compared numerically.
+ *
+ * Also note that some revisions/steppings may have been set aside as
+ * placeholders but never materialized in real hardware; in those cases there
+ * may be jumps in the revision IDs or stepping values in the tables below.
  */
 
+static const struct intel_step_info skl_revid_step_tbl[] = {
+	[0x0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
+	[0x1] = { .gt_step = STEP_B0, .display_step = STEP_B0 },
+	[0x2] = { .gt_step = STEP_C0, .display_step = STEP_C0 },
+	[0x3] = { .gt_step = STEP_D0, .display_step = STEP_D0 },
+	[0x4] = { .gt_step = STEP_E0, .display_step = STEP_E0 },
+	[0x5] = { .gt_step = STEP_F0, .display_step = STEP_F0 },
+	[0x6] = { .gt_step = STEP_G0, .display_step = STEP_G0 },
+	[0x7] = { .gt_step = STEP_H0, .display_step = STEP_H0 },
+	[0x9] = { .gt_step = STEP_J0, .display_step = STEP_J0 },
+	[0xA] = { .gt_step = STEP_I1, .display_step = STEP_I1 },
+};
 
-/* FIXME: what about REVID_E0 */
-static const struct intel_step_info kbl_revids[] = {
+static const struct intel_step_info kbl_revid_step_tbl[] = {
 	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
 	[1] = { .gt_step = STEP_B0, .display_step = STEP_B0 },
 	[2] = { .gt_step = STEP_C0, .display_step = STEP_B0 },
@@ -74,8 +90,11 @@ void intel_step_init(struct drm_i915_private *i915)
 		revids = tgl_revid_step_tbl;
 		size = ARRAY_SIZE(tgl_revid_step_tbl);
 	} else if (IS_KABYLAKE(i915)) {
-		revids = kbl_revids;
-		size = ARRAY_SIZE(kbl_revids);
+		revids = kbl_revid_step_tbl;
+		size = ARRAY_SIZE(kbl_revid_step_tbl);
+	} else if (IS_SKYLAKE(i915)) {
+		revids = skl_revid_step_tbl;
+		size = ARRAY_SIZE(skl_revid_step_tbl);
 	}
 
 	/* Not using the stepping scheme for the platform yet. */
diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
index 8efacef6ab31..41567d9b7c35 100644
--- a/drivers/gpu/drm/i915/intel_step.h
+++ b/drivers/gpu/drm/i915/intel_step.h
@@ -32,6 +32,10 @@ enum intel_step {
 	STEP_E0,
 	STEP_F0,
 	STEP_G0,
+	STEP_H0,
+	STEP_I0,
+	STEP_I1,
+	STEP_J0,
 	STEP_FUTURE,
 	STEP_FOREVER,
 };
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 03/10] drm/i915/icl: Use revid->stepping tables

[Anusha Srivatsa]

From: Matt Roper <matthew.d.roper@intel.com>

Switch ICL to use a revid->stepping table as we're trying to do on all
platforms going forward.  While we're at it, let's include some
additional steppings that have popped up, even if we don't yet have any
workarounds tied to those steppings (we probably need to audit our
workaround list soon to see if any of the bounds have moved or if new
workarounds have appeared).

Note that the current bspec table is missing information about how to
map PCI revision ID to GT/display steppings; it only provides an SoC
stepping.  The mapping to GT/display steppings (which aren't always the
same as the SoC stepping) used to be in the bspec, but was apparently
dropped during an update in Nov 2019; I've made my changes here based on
an older bspec snapshot that still had the necessary information.  We've
requested that the missing information be restored.

Bspec: 21441  # pre-Nov 2019 snapshot
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 12 ++++++------
 drivers/gpu/drm/i915/i915_drv.h             | 10 ++--------
 drivers/gpu/drm/i915/intel_step.c           | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_step.h           |  2 ++
 4 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 6dfd564e078f..e2d8acb8c1c9 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -557,7 +557,7 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
 	/* Wa_1604370585:icl (pre-prod)
 	 * Formerly known as WaPushConstantDereferenceHoldDisable
 	 */
-	if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_B0))
+	if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_B0))
 		wa_masked_en(wal, GEN7_ROW_CHICKEN2,
 			     PUSH_CONSTANT_DEREF_DISABLE);
 
@@ -573,12 +573,12 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
 	/* Wa_2006611047:icl (pre-prod)
 	 * Formerly known as WaDisableImprovedTdlClkGating
 	 */
-	if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0))
+	if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_A0))
 		wa_masked_en(wal, GEN7_ROW_CHICKEN2,
 			     GEN11_TDL_CLOCK_GATING_FIX_DISABLE);
 
 	/* Wa_2006665173:icl (pre-prod) */
-	if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0))
+	if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_A0))
 		wa_masked_en(wal, GEN11_COMMON_SLICE_CHICKEN3,
 			     GEN11_BLEND_EMB_FIX_DISABLE_IN_RCC);
 
@@ -1023,13 +1023,13 @@ icl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 		    GAMW_ECO_DEV_CTX_RELOAD_DISABLE);
 
 	/* Wa_1405779004:icl (pre-prod) */
-	if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0))
+	if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_A0))
 		wa_write_or(wal,
 			    SLICE_UNIT_LEVEL_CLKGATE,
 			    MSCUNIT_CLKGATE_DIS);
 
 	/* Wa_1406838659:icl (pre-prod) */
-	if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_B0))
+	if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_B0))
 		wa_write_or(wal,
 			    INF_UNIT_LEVEL_CLKGATE,
 			    CGPSF_CLKGATE_DIS);
@@ -1725,7 +1725,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			    PMFLUSHDONE_LNEBLK);
 
 		/* Wa_1406609255:icl (pre-prod) */
-		if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_B0))
+		if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_B0))
 			wa_write_or(wal,
 				    GEN7_SARCHKMD,
 				    GEN7_DISABLE_DEMAND_PREFETCH);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 775057626ee6..e26ff8624945 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1516,14 +1516,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define IS_KBL_DISPLAY_STEP(dev_priv, since, until) \
 	(IS_KABYLAKE(dev_priv) && IS_DISPLAY_STEP(dev_priv, since, until))
 
-#define ICL_REVID_A0		0x0
-#define ICL_REVID_A2		0x1
-#define ICL_REVID_B0		0x3
-#define ICL_REVID_B2		0x4
-#define ICL_REVID_C0		0x5
-
-#define IS_ICL_REVID(p, since, until) \
-	(IS_ICELAKE(p) && IS_REVID(p, since, until))
+#define IS_ICL_GT_STEP(p, since, until) \
+	(IS_ICELAKE(p) && IS_GT_STEP(p, since, until))
 
 #define EHL_REVID_A0            0x0
 #define EHL_REVID_B0            0x1
diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
index bfd63f56c200..4d8248cf67d3 100644
--- a/drivers/gpu/drm/i915/intel_step.c
+++ b/drivers/gpu/drm/i915/intel_step.c
@@ -42,6 +42,15 @@ static const struct intel_step_info kbl_revid_step_tbl[] = {
 	[7] = { .gt_step = STEP_G0, .display_step = STEP_C0 },
 };
 
+static const struct intel_step_info icl_revid_step_tbl[] = {
+	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
+	[3] = { .gt_step = STEP_B0, .display_step = STEP_B0 },
+	[4] = { .gt_step = STEP_B2, .display_step = STEP_B2 },
+	[5] = { .gt_step = STEP_C0, .display_step = STEP_C0 },
+	[6] = { .gt_step = STEP_C1, .display_step = STEP_C1 },
+	[7] = { .gt_step = STEP_D0, .display_step = STEP_D0 },
+};
+
 static const struct intel_step_info tgl_uy_revid_step_tbl[] = {
 	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
 	[1] = { .gt_step = STEP_B0, .display_step = STEP_C0 },
@@ -89,6 +98,9 @@ void intel_step_init(struct drm_i915_private *i915)
 	} else if (IS_TIGERLAKE(i915)) {
 		revids = tgl_revid_step_tbl;
 		size = ARRAY_SIZE(tgl_revid_step_tbl);
+	} else if (IS_ICELAKE(i915)) {
+		revids = icl_revid_step_tbl;
+		size = ARRAY_SIZE(icl_revid_step_tbl);
 	} else if (IS_KABYLAKE(i915)) {
 		revids = kbl_revid_step_tbl;
 		size = ARRAY_SIZE(kbl_revid_step_tbl);
diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
index 41567d9b7c35..3e8b2babd9da 100644
--- a/drivers/gpu/drm/i915/intel_step.h
+++ b/drivers/gpu/drm/i915/intel_step.h
@@ -26,7 +26,9 @@ enum intel_step {
 	STEP_A2,
 	STEP_B0,
 	STEP_B1,
+	STEP_B2,
 	STEP_C0,
+	STEP_C1,
 	STEP_D0,
 	STEP_D1,
 	STEP_E0,
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 04/10] drm/i915/jsl_ehl: Use revid->stepping tables

[Anusha Srivatsa]

From: Matt Roper <matthew.d.roper@intel.com>

Switch JSL/EHL to use a revid->stepping table as we're trying to do on
all platforms going forward.

Bspec: 29153
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 2 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c   | 2 +-
 drivers/gpu/drm/i915/i915_drv.h               | 9 ++++-----
 drivers/gpu/drm/i915/intel_step.c             | 8 ++++++++
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 882bfd499e55..dfc31b682848 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2674,7 +2674,7 @@ static bool
 ehl_combo_pll_div_frac_wa_needed(struct drm_i915_private *i915)
 {
 	return ((IS_PLATFORM(i915, INTEL_ELKHARTLAKE) &&
-		 IS_JSL_EHL_REVID(i915, EHL_REVID_B0, REVID_FOREVER)) ||
+		 IS_JSL_EHL_DISPLAY_STEP(i915, STEP_B0, STEP_FOREVER)) ||
 		 IS_TIGERLAKE(i915) || IS_ALDERLAKE_P(i915)) &&
 		 i915->dpll.ref_clks.nssc == 38400;
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index e2d8acb8c1c9..4c0c15bbdac2 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1043,7 +1043,7 @@ icl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 
 	/* Wa_1607087056:icl,ehl,jsl */
 	if (IS_ICELAKE(i915) ||
-	    IS_JSL_EHL_REVID(i915, EHL_REVID_A0, EHL_REVID_A0))
+	    IS_JSL_EHL_GT_STEP(i915, STEP_A0, STEP_A0))
 		wa_write_or(wal,
 			    SLICE_UNIT_LEVEL_CLKGATE,
 			    L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e26ff8624945..78db92bbb1c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1519,11 +1519,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define IS_ICL_GT_STEP(p, since, until) \
 	(IS_ICELAKE(p) && IS_GT_STEP(p, since, until))
 
-#define EHL_REVID_A0            0x0
-#define EHL_REVID_B0            0x1
-
-#define IS_JSL_EHL_REVID(p, since, until) \
-	(IS_JSL_EHL(p) && IS_REVID(p, since, until))
+#define IS_JSL_EHL_GT_STEP(p, since, until) \
+	(IS_JSL_EHL(p) && IS_GT_STEP(p, since, until))
+#define IS_JSL_EHL_DISPLAY_STEP(p, since, until) \
+	(IS_JSL_EHL(p) && IS_DISPLAY_STEP(p, since, until))
 
 #define IS_TGL_DISPLAY_STEP(__i915, since, until) \
 	(IS_TIGERLAKE(__i915) && \
diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
index 4d8248cf67d3..61666a3dd672 100644
--- a/drivers/gpu/drm/i915/intel_step.c
+++ b/drivers/gpu/drm/i915/intel_step.c
@@ -51,6 +51,11 @@ static const struct intel_step_info icl_revid_step_tbl[] = {
 	[7] = { .gt_step = STEP_D0, .display_step = STEP_D0 },
 };
 
+static const struct intel_step_info jsl_ehl_revid_step_tbl[] = {
+	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
+	[1] = { .gt_step = STEP_B0, .display_step = STEP_B0 },
+};
+
 static const struct intel_step_info tgl_uy_revid_step_tbl[] = {
 	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
 	[1] = { .gt_step = STEP_B0, .display_step = STEP_C0 },
@@ -98,6 +103,9 @@ void intel_step_init(struct drm_i915_private *i915)
 	} else if (IS_TIGERLAKE(i915)) {
 		revids = tgl_revid_step_tbl;
 		size = ARRAY_SIZE(tgl_revid_step_tbl);
+	} else if (IS_JSL_EHL(i915)) {
+		revids = jsl_ehl_revid_step_tbl;
+		size = ARRAY_SIZE(jsl_ehl_revid_step_tbl);
 	} else if (IS_ICELAKE(i915)) {
 		revids = icl_revid_step_tbl;
 		size = ARRAY_SIZE(icl_revid_step_tbl);
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 05/10] drm/i915/rkl: Use revid->stepping tables

[Anusha Srivatsa]

From: Matt Roper <matthew.d.roper@intel.com>

Switch RKL to use a revid->stepping table as we're trying to do on all
platforms going forward.

Bspec: 44501
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 4 ++--
 drivers/gpu/drm/i915/i915_drv.h          | 8 ++------
 drivers/gpu/drm/i915/intel_step.c        | 9 +++++++++
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 9643624fe160..74b2aa3c2946 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -594,7 +594,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	if (intel_dp->psr.psr2_sel_fetch_enabled) {
 		/* WA 1408330847 */
 		if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A0) ||
-		    IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_A0))
+		    IS_RKL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A0))
 			intel_de_rmw(dev_priv, CHICKEN_PAR1_1,
 				     DIS_RAM_BYPASS_PSR2_MAN_TRACK,
 				     DIS_RAM_BYPASS_PSR2_MAN_TRACK);
@@ -1342,7 +1342,7 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp)
 	/* WA 1408330847 */
 	if (intel_dp->psr.psr2_sel_fetch_enabled &&
 	    (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A0) ||
-	     IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_A0)))
+	     IS_RKL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A0)))
 		intel_de_rmw(dev_priv, CHICKEN_PAR1_1,
 			     DIS_RAM_BYPASS_PSR2_MAN_TRACK, 0);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 78db92bbb1c6..592e7177202e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1536,12 +1536,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 	(IS_TIGERLAKE(__i915) && !(IS_TGL_U(__i915) || IS_TGL_Y(__i915)) && \
 	 IS_GT_STEP(__i915, since, until))
 
-#define RKL_REVID_A0		0x0
-#define RKL_REVID_B0		0x1
-#define RKL_REVID_C0		0x4
-
-#define IS_RKL_REVID(p, since, until) \
-	(IS_ROCKETLAKE(p) && IS_REVID(p, since, until))
+#define IS_RKL_DISPLAY_STEP(p, since, until) \
+	(IS_ROCKETLAKE(p) && IS_DISPLAY_STEP(p, since, until))
 
 #define DG1_REVID_A0		0x0
 #define DG1_REVID_B0		0x1
diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
index 61666a3dd672..1593ab25f41a 100644
--- a/drivers/gpu/drm/i915/intel_step.c
+++ b/drivers/gpu/drm/i915/intel_step.c
@@ -69,6 +69,12 @@ static const struct intel_step_info tgl_revid_step_tbl[] = {
 	[1] = { .gt_step = STEP_B0, .display_step = STEP_D0 },
 };
 
+static const struct intel_step_info rkl_revid_step_tbl[] = {
+	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
+	[1] = { .gt_step = STEP_B0, .display_step = STEP_B0 },
+	[4] = { .gt_step = STEP_C0, .display_step = STEP_C0 },
+};
+
 static const struct intel_step_info adls_revid_step_tbl[] = {
 	[0x0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
 	[0x1] = { .gt_step = STEP_A0, .display_step = STEP_A2 },
@@ -97,6 +103,9 @@ void intel_step_init(struct drm_i915_private *i915)
 	} else if (IS_ALDERLAKE_S(i915)) {
 		revids = adls_revid_step_tbl;
 		size = ARRAY_SIZE(adls_revid_step_tbl);
+	} else if (IS_ROCKETLAKE(i915)) {
+		revids = rkl_revid_step_tbl;
+		size = ARRAY_SIZE(rkl_revid_step_tbl);
 	} else if (IS_TGL_U(i915) || IS_TGL_Y(i915)) {
 		revids = tgl_uy_revid_step_tbl;
 		size = ARRAY_SIZE(tgl_uy_revid_step_tbl);
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 06/10] drm/i915/dg1: Use revid->stepping tables

[Anusha Srivatsa]

From: Matt Roper <matthew.d.roper@intel.com>

Switch DG1 to use a revid->stepping table as we're trying to do on all
platforms going forward.

This removes the last use of IS_REVID() and REVID_FOREVER, so remove
those now-unused macros as well to prevent their accidental use on
future platforms.

Bspec: 44463
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 .../gpu/drm/i915/display/intel_display_power.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_region_lmem.c    |  2 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c    | 10 +++++-----
 drivers/gpu/drm/i915/i915_drv.h                | 18 ++++--------------
 drivers/gpu/drm/i915/intel_pm.c                |  2 +-
 drivers/gpu/drm/i915/intel_step.c              |  8 ++++++++
 6 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 285380079aab..975a7e25cea5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -5799,7 +5799,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
 	int config, i;
 
 	if (IS_ALDERLAKE_S(dev_priv) ||
-	    IS_DG1_REVID(dev_priv, DG1_REVID_A0, DG1_REVID_A0) ||
+	    IS_DG1_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A0) ||
 	    IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0))
 		/* Wa_1409767108:tgl,dg1,adl-s */
 		table = wa_1409767108_buddy_page_masks;
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 1f43aba2e9e2..50d11a84e7a9 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -157,7 +157,7 @@ intel_gt_setup_fake_lmem(struct intel_gt *gt)
 static bool get_legacy_lowmem_region(struct intel_uncore *uncore,
 				     u64 *start, u32 *size)
 {
-	if (!IS_DG1_REVID(uncore->i915, DG1_REVID_A0, DG1_REVID_B0))
+	if (!IS_DG1_GT_STEP(uncore->i915, STEP_A0, STEP_B0))
 		return false;
 
 	*start = 0;
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 4c0c15bbdac2..62321e9149db 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1111,7 +1111,7 @@ dg1_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 	gen12_gt_workarounds_init(i915, wal);
 
 	/* Wa_1607087056:dg1 */
-	if (IS_DG1_REVID(i915, DG1_REVID_A0, DG1_REVID_A0))
+	if (IS_DG1_GT_STEP(i915, STEP_A0, STEP_A0))
 		wa_write_or(wal,
 			    SLICE_UNIT_LEVEL_CLKGATE,
 			    L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
@@ -1522,7 +1522,7 @@ static void dg1_whitelist_build(struct intel_engine_cs *engine)
 	tgl_whitelist_build(engine);
 
 	/* GEN:BUG:1409280441:dg1 */
-	if (IS_DG1_REVID(engine->i915, DG1_REVID_A0, DG1_REVID_A0) &&
+	if (IS_DG1_GT_STEP(engine->i915, STEP_A0, STEP_A0) &&
 	    (engine->class == RENDER_CLASS ||
 	     engine->class == COPY_ENGINE_CLASS))
 		whitelist_reg_ext(w, RING_ID(engine->mmio_base),
@@ -1592,7 +1592,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 {
 	struct drm_i915_private *i915 = engine->i915;
 
-	if (IS_DG1_REVID(i915, DG1_REVID_A0, DG1_REVID_A0) ||
+	if (IS_DG1_GT_STEP(i915, STEP_A0, STEP_A0) ||
 	    IS_TGL_UY_GT_STEP(i915, STEP_A0, STEP_A0)) {
 		/*
 		 * Wa_1607138336:tgl[a0],dg1[a0]
@@ -1638,7 +1638,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 	}
 
 	if (IS_ALDERLAKE_P(i915) || IS_ALDERLAKE_S(i915) ||
-	    IS_DG1_REVID(i915, DG1_REVID_A0, DG1_REVID_A0) ||
+	    IS_DG1_GT_STEP(i915, STEP_A0, STEP_A0) ||
 	    IS_ROCKETLAKE(i915) || IS_TIGERLAKE(i915)) {
 		/* Wa_1409804808:tgl,rkl,dg1[a0],adl-s,adl-p */
 		wa_masked_en(wal, GEN7_ROW_CHICKEN2,
@@ -1652,7 +1652,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 	}
 
 
-	if (IS_DG1_REVID(i915, DG1_REVID_A0, DG1_REVID_A0) ||
+	if (IS_DG1_GT_STEP(i915, STEP_A0, STEP_A0) ||
 	    IS_ROCKETLAKE(i915) || IS_TIGERLAKE(i915)) {
 		/*
 		 * Wa_1607030317:tgl
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 592e7177202e..496c468229fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1317,19 +1317,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 #define IS_DISPLAY_VER(i915, from, until) \
 	(DISPLAY_VER(i915) >= (from) && DISPLAY_VER(i915) <= (until))
 
-#define REVID_FOREVER		0xff
 #define INTEL_REVID(dev_priv)	(to_pci_dev((dev_priv)->drm.dev)->revision)
 
 #define HAS_DSB(dev_priv)	(INTEL_INFO(dev_priv)->display.has_dsb)
 
-/*
- * Return true if revision is in range [since,until] inclusive.
- *
- * Use 0 for open-ended since, and REVID_FOREVER for open-ended until.
- */
-#define IS_REVID(p, since, until) \
-	(INTEL_REVID(p) >= (since) && INTEL_REVID(p) <= (until))
-
 #define INTEL_DISPLAY_STEP(__i915) (RUNTIME_INFO(__i915)->step.display_step)
 #define INTEL_GT_STEP(__i915) (RUNTIME_INFO(__i915)->step.gt_step)
 
@@ -1539,11 +1530,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define IS_RKL_DISPLAY_STEP(p, since, until) \
 	(IS_ROCKETLAKE(p) && IS_DISPLAY_STEP(p, since, until))
 
-#define DG1_REVID_A0		0x0
-#define DG1_REVID_B0		0x1
-
-#define IS_DG1_REVID(p, since, until) \
-	(IS_DG1(p) && IS_REVID(p, since, until))
+#define IS_DG1_GT_STEP(p, since, until) \
+	(IS_DG1(p) && IS_GT_STEP(p, since, until))
+#define IS_DG1_DISPLAY_STEP(p, since, until) \
+	(IS_DG1(p) && IS_DISPLAY_STEP(p, since, until))
 
 #define IS_ADLS_DISPLAY_STEP(__i915, since, until) \
 	(IS_ALDERLAKE_S(__i915) && \
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5fdb96e7d266..b933c9dc823a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7390,7 +7390,7 @@ static void dg1_init_clock_gating(struct drm_i915_private *dev_priv)
 	gen12lp_init_clock_gating(dev_priv);
 
 	/* Wa_1409836686:dg1[a0] */
-	if (IS_DG1_REVID(dev_priv, DG1_REVID_A0, DG1_REVID_A0))
+	if (IS_DG1_GT_STEP(dev_priv, STEP_A0, STEP_A0))
 		intel_uncore_write(&dev_priv->uncore, GEN9_CLKGATE_DIS_3, intel_uncore_read(&dev_priv->uncore, GEN9_CLKGATE_DIS_3) |
 			   DPT_GATING_DIS);
 }
diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
index 1593ab25f41a..c4ce02d22828 100644
--- a/drivers/gpu/drm/i915/intel_step.c
+++ b/drivers/gpu/drm/i915/intel_step.c
@@ -75,6 +75,11 @@ static const struct intel_step_info rkl_revid_step_tbl[] = {
 	[4] = { .gt_step = STEP_C0, .display_step = STEP_C0 },
 };
 
+static const struct intel_step_info dg1_revid_step_tbl[] = {
+	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
+	[1] = { .gt_step = STEP_B0, .display_step = STEP_B0 },
+};
+
 static const struct intel_step_info adls_revid_step_tbl[] = {
 	[0x0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
 	[0x1] = { .gt_step = STEP_A0, .display_step = STEP_A2 },
@@ -103,6 +108,9 @@ void intel_step_init(struct drm_i915_private *i915)
 	} else if (IS_ALDERLAKE_S(i915)) {
 		revids = adls_revid_step_tbl;
 		size = ARRAY_SIZE(adls_revid_step_tbl);
+	} else if (IS_DG1(i915)) {
+		revids = dg1_revid_step_tbl;
+		size = ARRAY_SIZE(dg1_revid_step_tbl);
 	} else if (IS_ROCKETLAKE(i915)) {
 		revids = rkl_revid_step_tbl;
 		size = ARRAY_SIZE(rkl_revid_step_tbl);
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 07/10] drm/i915/cnl: Drop all workarounds

[Anusha Srivatsa]

From: Matt Roper <matthew.d.roper@intel.com>

All of the Cannon Lake hardware that came out had graphics fused off,
and our userspace drivers have already dropped their support for the
platform; CNL-specific code in i915 that isn't inherited by subsequent
platforms is effectively dead code.  Let's remove all of the
CNL-specific workarounds as a quick and easy first step.

References: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6899
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 55 ---------------------
 1 file changed, 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 62321e9149db..9b257a394305 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -514,35 +514,6 @@ static void cfl_ctx_workarounds_init(struct intel_engine_cs *engine,
 		     GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
 }
 
-static void cnl_ctx_workarounds_init(struct intel_engine_cs *engine,
-				     struct i915_wa_list *wal)
-{
-	/* WaForceContextSaveRestoreNonCoherent:cnl */
-	wa_masked_en(wal, CNL_HDC_CHICKEN0,
-		     HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT);
-
-	/* WaDisableReplayBufferBankArbitrationOptimization:cnl */
-	wa_masked_en(wal, COMMON_SLICE_CHICKEN2,
-		     GEN8_SBE_DISABLE_REPLAY_BUF_OPTIMIZATION);
-
-	/* WaPushConstantDereferenceHoldDisable:cnl */
-	wa_masked_en(wal, GEN7_ROW_CHICKEN2, PUSH_CONSTANT_DEREF_DISABLE);
-
-	/* FtrEnableFastAnisoL1BankingFix:cnl */
-	wa_masked_en(wal, HALF_SLICE_CHICKEN3, CNL_FAST_ANISO_L1_BANKING_FIX);
-
-	/* WaDisable3DMidCmdPreemption:cnl */
-	wa_masked_dis(wal, GEN8_CS_CHICKEN1, GEN9_PREEMPT_3D_OBJECT_LEVEL);
-
-	/* WaDisableGPGPUMidCmdPreemption:cnl */
-	wa_masked_field_set(wal, GEN8_CS_CHICKEN1,
-			    GEN9_PREEMPT_GPGPU_LEVEL_MASK,
-			    GEN9_PREEMPT_GPGPU_COMMAND_LEVEL);
-
-	/* WaDisableEarlyEOT:cnl */
-	wa_masked_en(wal, GEN8_ROW_CHICKEN, DISABLE_EARLY_EOT);
-}
-
 static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
 				     struct i915_wa_list *wal)
 {
@@ -704,8 +675,6 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
 		gen12_ctx_workarounds_init(engine, wal);
 	else if (GRAPHICS_VER(i915) == 11)
 		icl_ctx_workarounds_init(engine, wal);
-	else if (IS_CANNONLAKE(i915))
-		cnl_ctx_workarounds_init(engine, wal);
 	else if (IS_COFFEELAKE(i915) || IS_COMETLAKE(i915))
 		cfl_ctx_workarounds_init(engine, wal);
 	else if (IS_GEMINILAKE(i915))
@@ -982,15 +951,6 @@ icl_wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
 	wa_write_clr_set(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr);
 }
 
-static void
-cnl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
-{
-	/* WaInPlaceDecompressionHang:cnl */
-	wa_write_or(wal,
-		    GEN9_GAMT_ECO_REG_RW_IA,
-		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
-}
-
 static void
 icl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
@@ -1140,8 +1100,6 @@ gt_init_workarounds(struct drm_i915_private *i915, struct i915_wa_list *wal)
 		gen12_gt_workarounds_init(i915, wal);
 	else if (GRAPHICS_VER(i915) == 11)
 		icl_gt_workarounds_init(i915, wal);
-	else if (IS_CANNONLAKE(i915))
-		cnl_gt_workarounds_init(i915, wal);
 	else if (IS_COFFEELAKE(i915) || IS_COMETLAKE(i915))
 		cfl_gt_workarounds_init(i915, wal);
 	else if (IS_GEMINILAKE(i915))
@@ -1418,17 +1376,6 @@ static void cml_whitelist_build(struct intel_engine_cs *engine)
 	cfl_whitelist_build(engine);
 }
 
-static void cnl_whitelist_build(struct intel_engine_cs *engine)
-{
-	struct i915_wa_list *w = &engine->whitelist;
-
-	if (engine->class != RENDER_CLASS)
-		return;
-
-	/* WaEnablePreemptionGranularityControlByUMD:cnl */
-	whitelist_reg(w, GEN8_CS_CHICKEN1);
-}
-
 static void icl_whitelist_build(struct intel_engine_cs *engine)
 {
 	struct i915_wa_list *w = &engine->whitelist;
@@ -1542,8 +1489,6 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine)
 		tgl_whitelist_build(engine);
 	else if (GRAPHICS_VER(i915) == 11)
 		icl_whitelist_build(engine);
-	else if (IS_CANNONLAKE(i915))
-		cnl_whitelist_build(engine);
 	else if (IS_COMETLAKE(i915))
 		cml_whitelist_build(engine);
 	else if (IS_COFFEELAKE(i915))
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 08/10] drm/i915/bxt: Use revid->stepping tables

[Anusha Srivatsa]

Switch BXT to use a revid->stepping table as we're trying to do on all
platforms going forward.

Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_step.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
index c4ce02d22828..99c0d3df001b 100644
--- a/drivers/gpu/drm/i915/intel_step.c
+++ b/drivers/gpu/drm/i915/intel_step.c
@@ -31,6 +31,15 @@ static const struct intel_step_info skl_revid_step_tbl[] = {
 	[0xA] = { .gt_step = STEP_I1, .display_step = STEP_I1 },
 };
 
+static const struct intel_step_info bxt_revids[] = {
+	[0] = { .gt_step = STEP_A0 },
+	[1] = { .gt_step = STEP_A1 },
+	[2] = { .gt_step = STEP_A2 },
+	[6] = { .gt_step = STEP_B0 },
+	[7] = { .gt_step = STEP_B1 },
+	[8] = { .gt_step = STEP_B2 },
+};
+
 static const struct intel_step_info kbl_revid_step_tbl[] = {
 	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
 	[1] = { .gt_step = STEP_B0, .display_step = STEP_B0 },
@@ -129,6 +138,9 @@ void intel_step_init(struct drm_i915_private *i915)
 	} else if (IS_KABYLAKE(i915)) {
 		revids = kbl_revid_step_tbl;
 		size = ARRAY_SIZE(kbl_revid_step_tbl);
+	} else if (IS_BROXTON(i915)) {
+		revids = bxt_revids;
+		size = ARRAY_SIZE(bxt_revids);
 	} else if (IS_SKYLAKE(i915)) {
 		revids = skl_revid_step_tbl;
 		size = ARRAY_SIZE(skl_revid_step_tbl);
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 09/10] drm/i915/step: Add intel_step_name() helper

[Anusha Srivatsa]

Add a helper to convert the step info to string.
This is specifically useful when we want to load a specific
firmware for a given stepping/substepping combination.

Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_step.c | 58 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_step.h |  1 +
 2 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
index 99c0d3df001b..9af7f30b777e 100644
--- a/drivers/gpu/drm/i915/intel_step.c
+++ b/drivers/gpu/drm/i915/intel_step.c
@@ -182,3 +182,61 @@ void intel_step_init(struct drm_i915_private *i915)
 
 	RUNTIME_INFO(i915)->step = step;
 }
+
+const char *intel_step_name(enum intel_step step) {
+	switch (step) {
+	case STEP_A0:
+		return "A0";
+		break;
+	case STEP_A1:
+		return "A1";
+		break;
+	case STEP_A2:
+		return "A2";
+		break;
+	case STEP_B0:
+		return "B0";
+		break;
+	case STEP_B1:
+		return "B1";
+		break;
+	case STEP_B2:
+		return "B2";
+		break;
+	case STEP_C0:
+		return "C0";
+		break;
+	case STEP_C1:
+		return "C1";
+		break;
+	case STEP_D0:
+		return "D0";
+		break;
+	case STEP_D1:
+		return "D1";
+		break;
+	case STEP_E0:
+		return "E0";
+		break;
+	case STEP_F0:
+		return "F0";
+		break;
+	case STEP_G0:
+		return "G0";
+		break;
+	case STEP_H0:
+		return "H0";
+		break;
+	case STEP_I0:
+		return "I0";
+		break;
+	case STEP_I1:
+		return "I1";
+		break;
+	case STEP_J0:
+		return "J0";
+		break;
+	default:
+		return "**";
+	}
+}
diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
index 3e8b2babd9da..2fbe51483472 100644
--- a/drivers/gpu/drm/i915/intel_step.h
+++ b/drivers/gpu/drm/i915/intel_step.h
@@ -43,5 +43,6 @@ enum intel_step {
 };
 
 void intel_step_init(struct drm_i915_private *i915);
+const char *intel_step_name(enum intel_step step);
 
 #endif /* __INTEL_STEP_H__ */
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 10/10] drm/i915/dmc: Modify intel_get_stepping_info()

[Anusha Srivatsa]

With all platforms having the tepping info in intel_step.c,
it makes no sense to maintain a separate lookup table
in intel_dmc.c Let modify intel_Get_stepping_info()
to grab stepping info from the central location towards
which everything is moving.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc.c | 51 +++++-------------------
 1 file changed, 9 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index f8789d4543bf..895bee8f9782 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -247,50 +247,16 @@ bool intel_dmc_has_payload(struct drm_i915_private *i915)
 	return i915->dmc.dmc_info[DMC_FW_MAIN].payload;
 }
 
-static const struct stepping_info skl_stepping_info[] = {
-	{'A', '0'}, {'B', '0'}, {'C', '0'},
-	{'D', '0'}, {'E', '0'}, {'F', '0'},
-	{'G', '0'}, {'H', '0'}, {'I', '0'},
-	{'J', '0'}, {'K', '0'}
-};
-
-static const struct stepping_info bxt_stepping_info[] = {
-	{'A', '0'}, {'A', '1'}, {'A', '2'},
-	{'B', '0'}, {'B', '1'}, {'B', '2'}
-};
-
-static const struct stepping_info icl_stepping_info[] = {
-	{'A', '0'}, {'A', '1'}, {'A', '2'},
-	{'B', '0'}, {'B', '2'},
-	{'C', '0'}
-};
-
-static const struct stepping_info no_stepping_info = { '*', '*' };
-
 static const struct stepping_info *
-intel_get_stepping_info(struct drm_i915_private *dev_priv)
+intel_get_stepping_info(struct drm_i915_private *dev_priv,
+			struct stepping_info *si)
 {
-	const struct stepping_info *si;
-	unsigned int size;
-
-	if (IS_ICELAKE(dev_priv)) {
-		size = ARRAY_SIZE(icl_stepping_info);
-		si = icl_stepping_info;
-	} else if (IS_SKYLAKE(dev_priv)) {
-		size = ARRAY_SIZE(skl_stepping_info);
-		si = skl_stepping_info;
-	} else if (IS_BROXTON(dev_priv)) {
-		size = ARRAY_SIZE(bxt_stepping_info);
-		si = bxt_stepping_info;
-	} else {
-		size = 0;
-		si = NULL;
-	}
-
-	if (INTEL_REVID(dev_priv) < size)
-		return si + INTEL_REVID(dev_priv);
+	struct intel_step_info step = RUNTIME_INFO(dev_priv)->step;
+	const char *step_name = intel_step_name(step.display_step);
 
-	return &no_stepping_info;
+	si->stepping = step_name[0];
+        si->substepping = step_name[1];
+	return si;
 }
 
 static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
@@ -616,7 +582,8 @@ static void parse_dmc_fw(struct drm_i915_private *dev_priv,
 	struct intel_package_header *package_header;
 	struct intel_dmc_header_base *dmc_header;
 	struct intel_dmc *dmc = &dev_priv->dmc;
-	const struct stepping_info *si = intel_get_stepping_info(dev_priv);
+	struct stepping_info display_info = { '*', '*'};
+	const struct stepping_info *si = intel_get_stepping_info(dev_priv, &display_info);
 	u32 readcount = 0;
 	u32 r, offset;
 	int id;
-- 
2.32.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Get stepping info from RUNTIME_INFO->step

[Patchwork]

== Series Details ==

Series: Get stepping info from RUNTIME_INFO->step
URL   : https://patchwork.freedesktop.org/series/92346/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c109c3b789aa drm/i915: Make pre-production detection use direct revid comparison
8ad43b5ade6f drm/i915/skl: Use revid->stepping tables
-:54: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'p' - possible side-effects?
#54: FILE: drivers/gpu/drm/i915/i915_drv.h:1512:
+#define IS_SKL_GT_STEP(p, since, until) (IS_SKYLAKE(p) && IS_GT_STEP(p, since, until))

total: 0 errors, 0 warnings, 1 checks, 85 lines checked
daee55a59631 drm/i915/icl: Use revid->stepping tables
-:93: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'p' - possible side-effects?
#93: FILE: drivers/gpu/drm/i915/i915_drv.h:1519:
+#define IS_ICL_GT_STEP(p, since, until) \
+	(IS_ICELAKE(p) && IS_GT_STEP(p, since, until))

total: 0 errors, 0 warnings, 1 checks, 94 lines checked
2f61797e047b drm/i915/jsl_ehl: Use revid->stepping tables
-:51: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'p' - possible side-effects?
#51: FILE: drivers/gpu/drm/i915/i915_drv.h:1522:
+#define IS_JSL_EHL_GT_STEP(p, since, until) \
+	(IS_JSL_EHL(p) && IS_GT_STEP(p, since, until))

-:53: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'p' - possible side-effects?
#53: FILE: drivers/gpu/drm/i915/i915_drv.h:1524:
+#define IS_JSL_EHL_DISPLAY_STEP(p, since, until) \
+	(IS_JSL_EHL(p) && IS_DISPLAY_STEP(p, since, until))

total: 0 errors, 0 warnings, 2 checks, 51 lines checked
95363c8b9dc9 drm/i915/rkl: Use revid->stepping tables
-:48: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'p' - possible side-effects?
#48: FILE: drivers/gpu/drm/i915/i915_drv.h:1539:
+#define IS_RKL_DISPLAY_STEP(p, since, until) \
+	(IS_ROCKETLAKE(p) && IS_DISPLAY_STEP(p, since, until))

total: 0 errors, 0 warnings, 1 checks, 51 lines checked
da4af914b48b drm/i915/dg1: Use revid->stepping tables
-:124: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'p' - possible side-effects?
#124: FILE: drivers/gpu/drm/i915/i915_drv.h:1533:
+#define IS_DG1_GT_STEP(p, since, until) \
+	(IS_DG1(p) && IS_GT_STEP(p, since, until))

-:126: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'p' - possible side-effects?
#126: FILE: drivers/gpu/drm/i915/i915_drv.h:1535:
+#define IS_DG1_DISPLAY_STEP(p, since, until) \
+	(IS_DG1(p) && IS_DISPLAY_STEP(p, since, until))

total: 0 errors, 0 warnings, 2 checks, 118 lines checked
2bc809b73f67 drm/i915/cnl: Drop all workarounds
5b936277263e drm/i915/bxt: Use revid->stepping tables
de45975f5a75 drm/i915/step: Add intel_step_name() helper
-:22: ERROR:OPEN_BRACE: open brace '{' following function definitions go on the next line
#22: FILE: drivers/gpu/drm/i915/intel_step.c:186:
+const char *intel_step_name(enum intel_step step) {

-:26: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#26: FILE: drivers/gpu/drm/i915/intel_step.c:190:
+		return "A0";
+		break;

-:29: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#29: FILE: drivers/gpu/drm/i915/intel_step.c:193:
+		return "A1";
+		break;

-:32: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#32: FILE: drivers/gpu/drm/i915/intel_step.c:196:
+		return "A2";
+		break;

-:35: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#35: FILE: drivers/gpu/drm/i915/intel_step.c:199:
+		return "B0";
+		break;

-:38: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#38: FILE: drivers/gpu/drm/i915/intel_step.c:202:
+		return "B1";
+		break;

-:41: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#41: FILE: drivers/gpu/drm/i915/intel_step.c:205:
+		return "B2";
+		break;

-:44: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#44: FILE: drivers/gpu/drm/i915/intel_step.c:208:
+		return "C0";
+		break;

-:47: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#47: FILE: drivers/gpu/drm/i915/intel_step.c:211:
+		return "C1";
+		break;

-:50: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#50: FILE: drivers/gpu/drm/i915/intel_step.c:214:
+		return "D0";
+		break;

-:53: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#53: FILE: drivers/gpu/drm/i915/intel_step.c:217:
+		return "D1";
+		break;

-:56: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#56: FILE: drivers/gpu/drm/i915/intel_step.c:220:
+		return "E0";
+		break;

-:59: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#59: FILE: drivers/gpu/drm/i915/intel_step.c:223:
+		return "F0";
+		break;

-:62: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#62: FILE: drivers/gpu/drm/i915/intel_step.c:226:
+		return "G0";
+		break;

-:65: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#65: FILE: drivers/gpu/drm/i915/intel_step.c:229:
+		return "H0";
+		break;

-:68: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#68: FILE: drivers/gpu/drm/i915/intel_step.c:232:
+		return "I0";
+		break;

-:71: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#71: FILE: drivers/gpu/drm/i915/intel_step.c:235:
+		return "I1";
+		break;

-:74: WARNING:UNNECESSARY_BREAK: break is not useful after a return
#74: FILE: drivers/gpu/drm/i915/intel_step.c:238:
+		return "J0";
+		break;

total: 1 errors, 17 warnings, 0 checks, 67 lines checked
2ecb4107afb2 drm/i915/dmc: Modify intel_get_stepping_info()
-:72: ERROR:CODE_INDENT: code indent should use tabs where possible
#72: FILE: drivers/gpu/drm/i915/display/intel_dmc.c:258:
+        si->substepping = step_name[1];$

-:72: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#72: FILE: drivers/gpu/drm/i915/display/intel_dmc.c:258:
+        si->substepping = step_name[1];$

total: 1 errors, 1 warnings, 0 checks, 66 lines checked


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Get stepping info from RUNTIME_INFO->step

[Patchwork]

== Series Details ==

Series: Get stepping info from RUNTIME_INFO->step
URL   : https://patchwork.freedesktop.org/series/92346/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/display/intel_display.c:1896:21:    expected struct i915_vma *[assigned] vma
+drivers/gpu/drm/i915/display/intel_display.c:1896:21:    got void [noderef] __iomem *[assigned] iomem
+drivers/gpu/drm/i915/display/intel_display.c:1896:21: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1412:34:    expected struct i915_address_space *vm
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1412:34:    got struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1412:34: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25:    expected struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25:    got struct i915_address_space *
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34:    expected struct i915_address_space *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34:    got struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_reset.c:1396:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gt/intel_ring_submission.c:1210:24: warning: Using plain integer as NULL pointer
+drivers/gpu/drm/i915/i915_perf.c:1434:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1488:15: warning: memset with byte count of 16777216
+./include/asm-generic/bitops/find.h:112:45: warning: shift count is negative (-262080)
+./include/asm-generic/bitops/find.h:32:31: warning: shift count is negative (-262080)
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for Get stepping info from RUNTIME_INFO->step

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 3428 bytes --]

== Series Details ==

Series: Get stepping info from RUNTIME_INFO->step
URL   : https://patchwork.freedesktop.org/series/92346/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10320 -> Patchwork_20560
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_20560 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_20560, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20560/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_20560:

### IGT changes ###

#### Possible regressions ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-bxt-dsi:         [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10320/fi-bxt-dsi/igt@core_hotunplug@unbind-rebind.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20560/fi-bxt-dsi/igt@core_hotunplug@unbind-rebind.html

  
Known issues
------------

  Here are the changes found in Patchwork_20560 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_sync@basic-each:
    - fi-bdw-5557u:       [PASS][3] -> [INCOMPLETE][4] ([i915#2944])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10320/fi-bdw-5557u/igt@gem_sync@basic-each.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20560/fi-bdw-5557u/igt@gem_sync@basic-each.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-bdw-5557u:       [FAIL][5] ([i915#2722] / [i915#3744]) -> [FAIL][6] ([i915#2722])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10320/fi-bdw-5557u/igt@runner@aborted.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20560/fi-bdw-5557u/igt@runner@aborted.html

  
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#2944]: https://gitlab.freedesktop.org/drm/intel/issues/2944
  [i915#3744]: https://gitlab.freedesktop.org/drm/intel/issues/3744


Participating hosts (40 -> 39)
------------------------------

  Missing    (1): fi-bsw-cyan 


Build changes
-------------

  * Linux: CI_DRM_10320 -> Patchwork_20560

  CI-20190529: 20190529
  CI_DRM_10320: 7d61ab4a59bcbb206324b6a430748b4c15dd8adb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6132: 61fb9cdf2a9132e3618c8b08b9d20fec0c347831 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20560: 2ecb4107afb2f9dfa4c87599cdb00c099b3de1e3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2ecb4107afb2 drm/i915/dmc: Modify intel_get_stepping_info()
de45975f5a75 drm/i915/step: Add intel_step_name() helper
5b936277263e drm/i915/bxt: Use revid->stepping tables
2bc809b73f67 drm/i915/cnl: Drop all workarounds
da4af914b48b drm/i915/dg1: Use revid->stepping tables
95363c8b9dc9 drm/i915/rkl: Use revid->stepping tables
2f61797e047b drm/i915/jsl_ehl: Use revid->stepping tables
daee55a59631 drm/i915/icl: Use revid->stepping tables
8ad43b5ade6f drm/i915/skl: Use revid->stepping tables
c109c3b789aa drm/i915: Make pre-production detection use direct revid comparison

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20560/index.html

[-- Attachment #1.2: Type: text/html, Size: 4224 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/10] drm/i915/bxt: Use revid->stepping tables

[Matt Roper]

On Thu, Jul 08, 2021 at 04:18:19PM -0700, Anusha Srivatsa wrote:
> Switch BXT to use a revid->stepping table as we're trying to do on all
> platforms going forward.
> 
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_step.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
> index c4ce02d22828..99c0d3df001b 100644
> --- a/drivers/gpu/drm/i915/intel_step.c
> +++ b/drivers/gpu/drm/i915/intel_step.c
> @@ -31,6 +31,15 @@ static const struct intel_step_info skl_revid_step_tbl[] = {
>  	[0xA] = { .gt_step = STEP_I1, .display_step = STEP_I1 },
>  };
>  
> +static const struct intel_step_info bxt_revids[] = {
> +	[0] = { .gt_step = STEP_A0 },
> +	[1] = { .gt_step = STEP_A1 },
> +	[2] = { .gt_step = STEP_A2 },
> +	[6] = { .gt_step = STEP_B0 },
> +	[7] = { .gt_step = STEP_B1 },
> +	[8] = { .gt_step = STEP_B2 },

I realize the mistake originates from the #define's that you're
replacing with these tables, but the values in this table aren't the
correct GT/display steppings, but rather the SoC stepping; that's the
wrong thing for us to be matching on for workarounds, DMC versions, etc.
You want to use column #4 of the bspec table, not column #2.

Also we need to update this to use the proper revisions from the bspec;
most of the ones you have here were temporary placeholders before the
platform was released and the actual revisions that showed up in real
hardware are higher than any of your table entries.  If we take into
account the right-most column of the bspec we'd actually want:

        static  const struct intel_step_info bxt_revids[] = {
                [0xA] = { .gt_step = STEP_C0 },
                [0xB] = { .gt_step = STEP_C0 },
                [0xC] = { .gt_step = STEP_D0 },
                [0xD] = { .gt_step = STEP_E0 },
        };


Matt

> +};
> +
>  static const struct intel_step_info kbl_revid_step_tbl[] = {
>  	[0] = { .gt_step = STEP_A0, .display_step = STEP_A0 },
>  	[1] = { .gt_step = STEP_B0, .display_step = STEP_B0 },
> @@ -129,6 +138,9 @@ void intel_step_init(struct drm_i915_private *i915)
>  	} else if (IS_KABYLAKE(i915)) {
>  		revids = kbl_revid_step_tbl;
>  		size = ARRAY_SIZE(kbl_revid_step_tbl);
> +	} else if (IS_BROXTON(i915)) {
> +		revids = bxt_revids;
> +		size = ARRAY_SIZE(bxt_revids);
>  	} else if (IS_SKYLAKE(i915)) {
>  		revids = skl_revid_step_tbl;
>  		size = ARRAY_SIZE(skl_revid_step_tbl);
> -- 
> 2.32.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/10] drm/i915/step: Add intel_step_name() helper

[Matt Roper]

On Thu, Jul 08, 2021 at 04:18:20PM -0700, Anusha Srivatsa wrote:
> Add a helper to convert the step info to string.
> This is specifically useful when we want to load a specific
> firmware for a given stepping/substepping combination.

What if we use macros to generate the per-stepping code here as well as
the stepping values in the enum?

In intel_step.h:

        #define STEPPING_NAME_LIST(func) \
                func(A0)
                func(A1)
                func(A2)
                func(B0)
                ...

        #define STEPPING_ENUM_VAL(name)  STEP_##name,

        enum intel_step {
                STEP_NONE = 0,
                STEPPING_NAME_LIST(STEPPING_ENUM_VAL)
                STEP_FUTURE,
                STEP_FOREVER,
        };

and in intel_step.c:

        #define STEPPING_NAME_CASE(name)        \
                case STEP_##name:               \
                        return #name;           \
                        break;

        const char *intel_step_name(enum intel_step step) {
                switch(step) {
                STEPPING_NAME_LIST(STEPPING_NAME_CASE)

                default:
                        return "**";
                }
        }

This has the advantage that anytime a new stepping is added (in
STEPPING_NAME_LIST) it will generate a new "STEP_XX" enum value and a
new case statement to return "XX" as the name; we won't have to remember
to update two separate places in the code.


Matt

> 
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_step.c | 58 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_step.h |  1 +
>  2 files changed, 59 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
> index 99c0d3df001b..9af7f30b777e 100644
> --- a/drivers/gpu/drm/i915/intel_step.c
> +++ b/drivers/gpu/drm/i915/intel_step.c
> @@ -182,3 +182,61 @@ void intel_step_init(struct drm_i915_private *i915)
>  
>  	RUNTIME_INFO(i915)->step = step;
>  }
> +
> +const char *intel_step_name(enum intel_step step) {
> +	switch (step) {
> +	case STEP_A0:
> +		return "A0";
> +		break;
> +	case STEP_A1:
> +		return "A1";
> +		break;
> +	case STEP_A2:
> +		return "A2";
> +		break;
> +	case STEP_B0:
> +		return "B0";
> +		break;
> +	case STEP_B1:
> +		return "B1";
> +		break;
> +	case STEP_B2:
> +		return "B2";
> +		break;
> +	case STEP_C0:
> +		return "C0";
> +		break;
> +	case STEP_C1:
> +		return "C1";
> +		break;
> +	case STEP_D0:
> +		return "D0";
> +		break;
> +	case STEP_D1:
> +		return "D1";
> +		break;
> +	case STEP_E0:
> +		return "E0";
> +		break;
> +	case STEP_F0:
> +		return "F0";
> +		break;
> +	case STEP_G0:
> +		return "G0";
> +		break;
> +	case STEP_H0:
> +		return "H0";
> +		break;
> +	case STEP_I0:
> +		return "I0";
> +		break;
> +	case STEP_I1:
> +		return "I1";
> +		break;
> +	case STEP_J0:
> +		return "J0";
> +		break;
> +	default:
> +		return "**";
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
> index 3e8b2babd9da..2fbe51483472 100644
> --- a/drivers/gpu/drm/i915/intel_step.h
> +++ b/drivers/gpu/drm/i915/intel_step.h
> @@ -43,5 +43,6 @@ enum intel_step {
>  };
>  
>  void intel_step_init(struct drm_i915_private *i915);
> +const char *intel_step_name(enum intel_step step);
>  
>  #endif /* __INTEL_STEP_H__ */
> -- 
> 2.32.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/10] drm/i915/step: Add intel_step_name() helper

[Lucas De Marchi]

On Thu, Jul 08, 2021 at 09:16:16PM -0700, Matt Roper wrote:
>On Thu, Jul 08, 2021 at 04:18:20PM -0700, Anusha Srivatsa wrote:
>> Add a helper to convert the step info to string.
>> This is specifically useful when we want to load a specific
>> firmware for a given stepping/substepping combination.
>
>What if we use macros to generate the per-stepping code here as well as
>the stepping values in the enum?
>
>In intel_step.h:
>
>        #define STEPPING_NAME_LIST(func) \
>                func(A0)
>                func(A1)
>                func(A2)
>                func(B0)
>                ...
>
>        #define STEPPING_ENUM_VAL(name)  STEP_##name,
>
>        enum intel_step {
>                STEP_NONE = 0,
>                STEPPING_NAME_LIST(STEPPING_ENUM_VAL)
>                STEP_FUTURE,
>                STEP_FOREVER,
>        };
>
>and in intel_step.c:
>
>        #define STEPPING_NAME_CASE(name)        \
>                case STEP_##name:               \
>                        return #name;           \
>                        break;
>
>        const char *intel_step_name(enum intel_step step) {
>                switch(step) {
>                STEPPING_NAME_LIST(STEPPING_NAME_CASE)
>
>                default:
>                        return "**";
>                }
>        }
>
>This has the advantage that anytime a new stepping is added (in
>STEPPING_NAME_LIST) it will generate a new "STEP_XX" enum value and a
>new case statement to return "XX" as the name; we won't have to remember
>to update two separate places in the code.

my other idea in the first iterations of this patch was to turn the
stepping into u16 and then do something like (untested crap code below):

	#define make_step(a, b)	((a - 'A') << 8, (b - '0'))

	#define intel_step_name(s) ({
		char ret[3];
		ret[0] = ((s) >> 8) + 'A';
		ret[1] = ((s) & 0xff) + '0';
		ret[2] = '\0';
		ret;
	})

	enum intel_step {
		STEP_NONE = -1,
		STEP_A0 = make_step('A', '0'),
		...
	}

Or even not bother with the 'A'/'0' addition/subraction since 8 bits is
enough for all the letters and numbers.

If we keep it u8, then we are limited to step P7 (assuming we have 2
reserved entries at the end),. It may or may not be sufficient (it
currently is)

better? worse?

Lucas De Marchi

>
>
>Matt
>
>>
>> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_step.c | 58 +++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_step.h |  1 +
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
>> index 99c0d3df001b..9af7f30b777e 100644
>> --- a/drivers/gpu/drm/i915/intel_step.c
>> +++ b/drivers/gpu/drm/i915/intel_step.c
>> @@ -182,3 +182,61 @@ void intel_step_init(struct drm_i915_private *i915)
>>
>>  	RUNTIME_INFO(i915)->step = step;
>>  }
>> +
>> +const char *intel_step_name(enum intel_step step) {
>> +	switch (step) {
>> +	case STEP_A0:
>> +		return "A0";
>> +		break;
>> +	case STEP_A1:
>> +		return "A1";
>> +		break;
>> +	case STEP_A2:
>> +		return "A2";
>> +		break;
>> +	case STEP_B0:
>> +		return "B0";
>> +		break;
>> +	case STEP_B1:
>> +		return "B1";
>> +		break;
>> +	case STEP_B2:
>> +		return "B2";
>> +		break;
>> +	case STEP_C0:
>> +		return "C0";
>> +		break;
>> +	case STEP_C1:
>> +		return "C1";
>> +		break;
>> +	case STEP_D0:
>> +		return "D0";
>> +		break;
>> +	case STEP_D1:
>> +		return "D1";
>> +		break;
>> +	case STEP_E0:
>> +		return "E0";
>> +		break;
>> +	case STEP_F0:
>> +		return "F0";
>> +		break;
>> +	case STEP_G0:
>> +		return "G0";
>> +		break;
>> +	case STEP_H0:
>> +		return "H0";
>> +		break;
>> +	case STEP_I0:
>> +		return "I0";
>> +		break;
>> +	case STEP_I1:
>> +		return "I1";
>> +		break;
>> +	case STEP_J0:
>> +		return "J0";
>> +		break;
>> +	default:
>> +		return "**";
>> +	}
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
>> index 3e8b2babd9da..2fbe51483472 100644
>> --- a/drivers/gpu/drm/i915/intel_step.h
>> +++ b/drivers/gpu/drm/i915/intel_step.h
>> @@ -43,5 +43,6 @@ enum intel_step {
>>  };
>>
>>  void intel_step_init(struct drm_i915_private *i915);
>> +const char *intel_step_name(enum intel_step step);
>>
>>  #endif /* __INTEL_STEP_H__ */
>> --
>> 2.32.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>-- 
>Matt Roper
>Graphics Software Engineer
>VTT-OSGC Platform Enablement
>Intel Corporation
>(916) 356-2795
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/10] drm/i915/step: Add intel_step_name() helper

[Srivatsa, Anusha]

> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Friday, July 9, 2021 10:53 AM
> To: Roper, Matthew D <matthew.d.roper@intel.com>
> Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
> gfx@lists.freedesktop.org; Jani Nikula <jani.nikula@linux.intel.com>
> Subject: Re: [Intel-gfx] [PATCH 09/10] drm/i915/step: Add intel_step_name()
> helper
> 
> On Thu, Jul 08, 2021 at 09:16:16PM -0700, Matt Roper wrote:
> >On Thu, Jul 08, 2021 at 04:18:20PM -0700, Anusha Srivatsa wrote:
> >> Add a helper to convert the step info to string.
> >> This is specifically useful when we want to load a specific firmware
> >> for a given stepping/substepping combination.
> >
> >What if we use macros to generate the per-stepping code here as well as
> >the stepping values in the enum?
> >
> >In intel_step.h:
> >
> >        #define STEPPING_NAME_LIST(func) \
> >                func(A0)
> >                func(A1)
> >                func(A2)
> >                func(B0)
> >                ...
> >
> >        #define STEPPING_ENUM_VAL(name)  STEP_##name,
> >
> >        enum intel_step {
> >                STEP_NONE = 0,
> >                STEPPING_NAME_LIST(STEPPING_ENUM_VAL)
> >                STEP_FUTURE,
> >                STEP_FOREVER,
> >        };
> >
> >and in intel_step.c:
> >
> >        #define STEPPING_NAME_CASE(name)        \
> >                case STEP_##name:               \
> >                        return #name;           \
> >                        break;
> >
> >        const char *intel_step_name(enum intel_step step) {
> >                switch(step) {
> >                STEPPING_NAME_LIST(STEPPING_NAME_CASE)
> >
> >                default:
> >                        return "**";
> >                }
> >        }
> >
> >This has the advantage that anytime a new stepping is added (in
> >STEPPING_NAME_LIST) it will generate a new "STEP_XX" enum value and a
> >new case statement to return "XX" as the name; we won't have to
> >remember to update two separate places in the code.
> 
> my other idea in the first iterations of this patch was to turn the stepping into
> u16 and then do something like (untested crap code below):
> 
> 	#define make_step(a, b)	((a - 'A') << 8, (b - '0'))
> 
> 	#define intel_step_name(s) ({
> 		char ret[3];
> 		ret[0] = ((s) >> 8) + 'A';
> 		ret[1] = ((s) & 0xff) + '0';
> 		ret[2] = '\0';
> 		ret;
> 	})
> 
> 	enum intel_step {
> 		STEP_NONE = -1,
> 		STEP_A0 = make_step('A', '0'),
> 		...
> 	}
> 
> Or even not bother with the 'A'/'0' addition/subraction since 8 bits is enough
> for all the letters and numbers.
> 
> If we keep it u8, then we are limited to step P7 (assuming we have 2
> reserved entries at the end),. It may or may not be sufficient (it currently is)
> 
> better? worse?

I feel If Matt's solution is more scalable, better to go with it.

Anusha
> Lucas De Marchi
> 
> >
> >
> >Matt
> >
> >>
> >> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> >> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_step.c | 58
> >> +++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_step.h |
> >> 1 +
> >>  2 files changed, 59 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_step.c
> >> b/drivers/gpu/drm/i915/intel_step.c
> >> index 99c0d3df001b..9af7f30b777e 100644
> >> --- a/drivers/gpu/drm/i915/intel_step.c
> >> +++ b/drivers/gpu/drm/i915/intel_step.c
> >> @@ -182,3 +182,61 @@ void intel_step_init(struct drm_i915_private
> >> *i915)
> >>
> >>  	RUNTIME_INFO(i915)->step = step;
> >>  }
> >> +
> >> +const char *intel_step_name(enum intel_step step) {
> >> +	switch (step) {
> >> +	case STEP_A0:
> >> +		return "A0";
> >> +		break;
> >> +	case STEP_A1:
> >> +		return "A1";
> >> +		break;
> >> +	case STEP_A2:
> >> +		return "A2";
> >> +		break;
> >> +	case STEP_B0:
> >> +		return "B0";
> >> +		break;
> >> +	case STEP_B1:
> >> +		return "B1";
> >> +		break;
> >> +	case STEP_B2:
> >> +		return "B2";
> >> +		break;
> >> +	case STEP_C0:
> >> +		return "C0";
> >> +		break;
> >> +	case STEP_C1:
> >> +		return "C1";
> >> +		break;
> >> +	case STEP_D0:
> >> +		return "D0";
> >> +		break;
> >> +	case STEP_D1:
> >> +		return "D1";
> >> +		break;
> >> +	case STEP_E0:
> >> +		return "E0";
> >> +		break;
> >> +	case STEP_F0:
> >> +		return "F0";
> >> +		break;
> >> +	case STEP_G0:
> >> +		return "G0";
> >> +		break;
> >> +	case STEP_H0:
> >> +		return "H0";
> >> +		break;
> >> +	case STEP_I0:
> >> +		return "I0";
> >> +		break;
> >> +	case STEP_I1:
> >> +		return "I1";
> >> +		break;
> >> +	case STEP_J0:
> >> +		return "J0";
> >> +		break;
> >> +	default:
> >> +		return "**";
> >> +	}
> >> +}
> >> diff --git a/drivers/gpu/drm/i915/intel_step.h
> >> b/drivers/gpu/drm/i915/intel_step.h
> >> index 3e8b2babd9da..2fbe51483472 100644
> >> --- a/drivers/gpu/drm/i915/intel_step.h
> >> +++ b/drivers/gpu/drm/i915/intel_step.h
> >> @@ -43,5 +43,6 @@ enum intel_step {
> >>  };
> >>
> >>  void intel_step_init(struct drm_i915_private *i915);
> >> +const char *intel_step_name(enum intel_step step);
> >>
> >>  #endif /* __INTEL_STEP_H__ */
> >> --
> >> 2.32.0
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >--
> >Matt Roper
> >Graphics Software Engineer
> >VTT-OSGC Platform Enablement
> >Intel Corporation
> >(916) 356-2795
> >_______________________________________________
> >Intel-gfx mailing list
> >Intel-gfx@lists.freedesktop.org
> >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/10] drm/i915/step: Add intel_step_name() helper

[Lucas De Marchi]

On Fri, Jul 09, 2021 at 11:36:09AM -0700, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi@intel.com>
>> Sent: Friday, July 9, 2021 10:53 AM
>> To: Roper, Matthew D <matthew.d.roper@intel.com>
>> Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
>> gfx@lists.freedesktop.org; Jani Nikula <jani.nikula@linux.intel.com>
>> Subject: Re: [Intel-gfx] [PATCH 09/10] drm/i915/step: Add intel_step_name()
>> helper
>>
>> On Thu, Jul 08, 2021 at 09:16:16PM -0700, Matt Roper wrote:
>> >On Thu, Jul 08, 2021 at 04:18:20PM -0700, Anusha Srivatsa wrote:
>> >> Add a helper to convert the step info to string.
>> >> This is specifically useful when we want to load a specific firmware
>> >> for a given stepping/substepping combination.
>> >
>> >What if we use macros to generate the per-stepping code here as well as
>> >the stepping values in the enum?
>> >
>> >In intel_step.h:
>> >
>> >        #define STEPPING_NAME_LIST(func) \
>> >                func(A0)
>> >                func(A1)
>> >                func(A2)
>> >                func(B0)
>> >                ...
>> >
>> >        #define STEPPING_ENUM_VAL(name)  STEP_##name,
>> >
>> >        enum intel_step {
>> >                STEP_NONE = 0,
>> >                STEPPING_NAME_LIST(STEPPING_ENUM_VAL)
>> >                STEP_FUTURE,
>> >                STEP_FOREVER,
>> >        };
>> >
>> >and in intel_step.c:
>> >
>> >        #define STEPPING_NAME_CASE(name)        \
>> >                case STEP_##name:               \
>> >                        return #name;           \
>> >                        break;
>> >
>> >        const char *intel_step_name(enum intel_step step) {
>> >                switch(step) {
>> >                STEPPING_NAME_LIST(STEPPING_NAME_CASE)
>> >
>> >                default:
>> >                        return "**";
>> >                }
>> >        }
>> >
>> >This has the advantage that anytime a new stepping is added (in
>> >STEPPING_NAME_LIST) it will generate a new "STEP_XX" enum value and a
>> >new case statement to return "XX" as the name; we won't have to
>> >remember to update two separate places in the code.
>>
>> my other idea in the first iterations of this patch was to turn the stepping into
>> u16 and then do something like (untested crap code below):
>>
>> 	#define make_step(a, b)	((a - 'A') << 8, (b - '0'))
>>
>> 	#define intel_step_name(s) ({
>> 		char ret[3];
>> 		ret[0] = ((s) >> 8) + 'A';
>> 		ret[1] = ((s) & 0xff) + '0';
>> 		ret[2] = '\0';
>> 		ret;
>> 	})
>>
>> 	enum intel_step {
>> 		STEP_NONE = -1,
>> 		STEP_A0 = make_step('A', '0'),
>> 		...
>> 	}
>>
>> Or even not bother with the 'A'/'0' addition/subraction since 8 bits is enough
>> for all the letters and numbers.
>>
>> If we keep it u8, then we are limited to step P7 (assuming we have 2
>> reserved entries at the end),. It may or may not be sufficient (it currently is)
>>
>> better? worse?
>
>I feel If Matt's solution is more scalable, better to go with it.

both scale the same from what I can see. So, in the end I think the
consideration would be:

	- how much magic do they bring? (less is more... and subjective)
	- .ko size increase considering the new tables for new
	  platforms (may be negligible)

Lucas De Marchi

>
>Anusha
>> Lucas De Marchi
>>
>> >
>> >
>> >Matt
>> >
>> >>
>> >> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
>> >> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/intel_step.c | 58
>> >> +++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_step.h |
>> >> 1 +
>> >>  2 files changed, 59 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/intel_step.c
>> >> b/drivers/gpu/drm/i915/intel_step.c
>> >> index 99c0d3df001b..9af7f30b777e 100644
>> >> --- a/drivers/gpu/drm/i915/intel_step.c
>> >> +++ b/drivers/gpu/drm/i915/intel_step.c
>> >> @@ -182,3 +182,61 @@ void intel_step_init(struct drm_i915_private
>> >> *i915)
>> >>
>> >>  	RUNTIME_INFO(i915)->step = step;
>> >>  }
>> >> +
>> >> +const char *intel_step_name(enum intel_step step) {
>> >> +	switch (step) {
>> >> +	case STEP_A0:
>> >> +		return "A0";
>> >> +		break;
>> >> +	case STEP_A1:
>> >> +		return "A1";
>> >> +		break;
>> >> +	case STEP_A2:
>> >> +		return "A2";
>> >> +		break;
>> >> +	case STEP_B0:
>> >> +		return "B0";
>> >> +		break;
>> >> +	case STEP_B1:
>> >> +		return "B1";
>> >> +		break;
>> >> +	case STEP_B2:
>> >> +		return "B2";
>> >> +		break;
>> >> +	case STEP_C0:
>> >> +		return "C0";
>> >> +		break;
>> >> +	case STEP_C1:
>> >> +		return "C1";
>> >> +		break;
>> >> +	case STEP_D0:
>> >> +		return "D0";
>> >> +		break;
>> >> +	case STEP_D1:
>> >> +		return "D1";
>> >> +		break;
>> >> +	case STEP_E0:
>> >> +		return "E0";
>> >> +		break;
>> >> +	case STEP_F0:
>> >> +		return "F0";
>> >> +		break;
>> >> +	case STEP_G0:
>> >> +		return "G0";
>> >> +		break;
>> >> +	case STEP_H0:
>> >> +		return "H0";
>> >> +		break;
>> >> +	case STEP_I0:
>> >> +		return "I0";
>> >> +		break;
>> >> +	case STEP_I1:
>> >> +		return "I1";
>> >> +		break;
>> >> +	case STEP_J0:
>> >> +		return "J0";
>> >> +		break;
>> >> +	default:
>> >> +		return "**";
>> >> +	}
>> >> +}
>> >> diff --git a/drivers/gpu/drm/i915/intel_step.h
>> >> b/drivers/gpu/drm/i915/intel_step.h
>> >> index 3e8b2babd9da..2fbe51483472 100644
>> >> --- a/drivers/gpu/drm/i915/intel_step.h
>> >> +++ b/drivers/gpu/drm/i915/intel_step.h
>> >> @@ -43,5 +43,6 @@ enum intel_step {
>> >>  };
>> >>
>> >>  void intel_step_init(struct drm_i915_private *i915);
>> >> +const char *intel_step_name(enum intel_step step);
>> >>
>> >>  #endif /* __INTEL_STEP_H__ */
>> >> --
>> >> 2.32.0
>> >>
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> >--
>> >Matt Roper
>> >Graphics Software Engineer
>> >VTT-OSGC Platform Enablement
>> >Intel Corporation
>> >(916) 356-2795
>> >_______________________________________________
>> >Intel-gfx mailing list
>> >Intel-gfx@lists.freedesktop.org
>> >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx