diff mbox series

[2/2] drm/i915/tgl: Add bound checks and simplify TGL REVID macros

Message ID 20201126070041.369530-2-aditya.swarup@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/tgl: Fix REVID macros for TGL to fetch correct stepping | expand

Commit Message

Aditya Swarup Nov. 26, 2020, 7 a.m. UTC
Add bound checks for TGL REV ID array. Since, there might
be a possibility of using older kernels on latest platform
revisions, resulting in out of bounds access for rev ID array.
In this scenario, use the latest rev ID available and apply
those WAs.

Also, simplify GT macro for TGL rev ID to reuse tgl_revids_get().

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c |  8 ++---
 drivers/gpu/drm/i915/i915_drv.h             | 36 +++++++++++++--------
 2 files changed, 26 insertions(+), 18 deletions(-)

Comments

Lucas De Marchi Nov. 26, 2020, 7:37 a.m. UTC | #1
On Wed, Nov 25, 2020 at 11:00:41PM -0800, Aditya Swarup wrote:
>Add bound checks for TGL REV ID array. Since, there might
>be a possibility of using older kernels on latest platform
>revisions, resulting in out of bounds access for rev ID array.
>In this scenario, use the latest rev ID available and apply
>those WAs.
>
>Also, simplify GT macro for TGL rev ID to reuse tgl_revids_get().
>
>Cc: José Roberto de Souza <jose.souza@intel.com>
>Cc: Matt Roper <matthew.d.roper@intel.com>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Cc: Jani Nikula <jani.nikula@intel.com>
>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>---
> drivers/gpu/drm/i915/gt/intel_workarounds.c |  8 ++---
> drivers/gpu/drm/i915/i915_drv.h             | 36 +++++++++++++--------
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>index a82554baa6ac..5e2563529b5f 100644
>--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>@@ -1250,13 +1250,13 @@ tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
> 	gen12_gt_workarounds_init(i915, wal);
>
> 	/* Wa_1409420604:tgl */
>-	if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
>+	if (IS_TGL_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))

not seeing what's preventing this WA to be applied on !(U || Y) sku now.
See below.

> 		wa_write_or(wal,
> 			    SUBSLICE_UNIT_LEVEL_CLKGATE2,
> 			    CPSSUNIT_CLKGATE_DIS);
>
> 	/* Wa_1607087056:tgl also know as BUG:1409180338 */
>-	if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
>+	if (IS_TGL_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> 		wa_write_or(wal,
> 			    SLICE_UNIT_LEVEL_CLKGATE,
> 			    L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
>@@ -1734,7 +1734,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) ||
>-	    IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
>+	    IS_TGL_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> 		/*
> 		 * Wa_1607138336:tgl[a0],dg1[a0]
> 		 * Wa_1607063988:tgl[a0],dg1[a0]
>@@ -1744,7 +1744,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> 			    GEN12_DISABLE_POSH_BUSY_FF_DOP_CG);
> 	}
>
>-	if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
>+	if (IS_TGL_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> 		/*
> 		 * Wa_1606679103:tgl
> 		 * (see also Wa_1606682166:icl)
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 0a3ee4f9dc0a..3d0ef6b60337 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -1572,16 +1572,30 @@ enum {
> 	TGL_REVID_D0,
> };
>
>-extern const struct i915_rev_steppings tgl_uy_revids[];
>-extern const struct i915_rev_steppings tgl_revids[];
>+#define TGL_UY_REVIDS_SIZE	4
>+#define TGL_REVIDS_SIZE		2
>+
>+extern const struct i915_rev_steppings tgl_uy_revids[TGL_UY_REVIDS_SIZE];
>+extern const struct i915_rev_steppings tgl_revids[TGL_REVIDS_SIZE];
>
> static inline const struct i915_rev_steppings *
> tgl_revids_get(struct drm_i915_private *dev_priv)
> {
>-	if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv))
>-		return &tgl_uy_revids[INTEL_REVID(dev_priv)];
>-	else
>-		return &tgl_revids[INTEL_REVID(dev_priv)];
>+	u8 revid = INTEL_REVID(dev_priv);
>+	u8 size;
>+	const struct i915_rev_steppings *tgl_revid_tbl;
>+
>+	if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
>+		tgl_revid_tbl = tgl_uy_revids;
>+		size = ARRAY_SIZE(tgl_uy_revids);
>+	} else {
>+		tgl_revid_tbl = tgl_revids;
>+		size = ARRAY_SIZE(tgl_revids);
>+	}
>+
>+	revid = min_t(u8, revid, size - 1);
>+
>+	return &tgl_revid_tbl[revid];
> }
>
> #define IS_TGL_DISP_REVID(p, since, until) \
>@@ -1589,16 +1603,10 @@ tgl_revids_get(struct drm_i915_private *dev_priv)
> 	 tgl_revids_get(p)->disp_stepping >= (since) && \
> 	 tgl_revids_get(p)->disp_stepping <= (until))
>
>-#define IS_TGL_UY_GT_REVID(p, since, until) \
>-	((IS_TGL_U(p) || IS_TGL_Y(p)) && \
>-	 tgl_uy_revids[INTEL_REVID(p)].gt_stepping >= (since) && \
>-	 tgl_uy_revids[INTEL_REVID(p)].gt_stepping <= (until))
>-
> #define IS_TGL_GT_REVID(p, since, until) \
> 	(IS_TIGERLAKE(p) && \
>-	 !(IS_TGL_U(p) || IS_TGL_Y(p)) && \

so, previously this check would prevent the WA to be applied if it's not
U or Y sku. Now, in that case you are going to simply apply it. 
See comment in intel_workarounds.c:

/* Same GT stepping between tgl_uy_revids and tgl_revids don't mean the same HW */
const struct i915_rev_steppings tgl_revids[] = {

Lucas De Marchi

>-	 tgl_revids[INTEL_REVID(p)].gt_stepping >= (since) && \
>-	 tgl_revids[INTEL_REVID(p)].gt_stepping <= (until))
>+	 tgl_revids_get(p)->gt_stepping >= (since) && \
>+	 tgl_revids_get(p)->gt_stepping <= (until))
>
> #define RKL_REVID_A0		0x0
> #define RKL_REVID_B0		0x1
>-- 
>2.27.0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index a82554baa6ac..5e2563529b5f 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1250,13 +1250,13 @@  tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 	gen12_gt_workarounds_init(i915, wal);
 
 	/* Wa_1409420604:tgl */
-	if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
+	if (IS_TGL_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
 		wa_write_or(wal,
 			    SUBSLICE_UNIT_LEVEL_CLKGATE2,
 			    CPSSUNIT_CLKGATE_DIS);
 
 	/* Wa_1607087056:tgl also know as BUG:1409180338 */
-	if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
+	if (IS_TGL_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
 		wa_write_or(wal,
 			    SLICE_UNIT_LEVEL_CLKGATE,
 			    L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
@@ -1734,7 +1734,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) ||
-	    IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
+	    IS_TGL_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
 		/*
 		 * Wa_1607138336:tgl[a0],dg1[a0]
 		 * Wa_1607063988:tgl[a0],dg1[a0]
@@ -1744,7 +1744,7 @@  rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			    GEN12_DISABLE_POSH_BUSY_FF_DOP_CG);
 	}
 
-	if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
+	if (IS_TGL_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
 		/*
 		 * Wa_1606679103:tgl
 		 * (see also Wa_1606682166:icl)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0a3ee4f9dc0a..3d0ef6b60337 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1572,16 +1572,30 @@  enum {
 	TGL_REVID_D0,
 };
 
-extern const struct i915_rev_steppings tgl_uy_revids[];
-extern const struct i915_rev_steppings tgl_revids[];
+#define TGL_UY_REVIDS_SIZE	4
+#define TGL_REVIDS_SIZE		2
+
+extern const struct i915_rev_steppings tgl_uy_revids[TGL_UY_REVIDS_SIZE];
+extern const struct i915_rev_steppings tgl_revids[TGL_REVIDS_SIZE];
 
 static inline const struct i915_rev_steppings *
 tgl_revids_get(struct drm_i915_private *dev_priv)
 {
-	if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv))
-		return &tgl_uy_revids[INTEL_REVID(dev_priv)];
-	else
-		return &tgl_revids[INTEL_REVID(dev_priv)];
+	u8 revid = INTEL_REVID(dev_priv);
+	u8 size;
+	const struct i915_rev_steppings *tgl_revid_tbl;
+
+	if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
+		tgl_revid_tbl = tgl_uy_revids;
+		size = ARRAY_SIZE(tgl_uy_revids);
+	} else {
+		tgl_revid_tbl = tgl_revids;
+		size = ARRAY_SIZE(tgl_revids);
+	}
+
+	revid = min_t(u8, revid, size - 1);
+
+	return &tgl_revid_tbl[revid];
 }
 
 #define IS_TGL_DISP_REVID(p, since, until) \
@@ -1589,16 +1603,10 @@  tgl_revids_get(struct drm_i915_private *dev_priv)
 	 tgl_revids_get(p)->disp_stepping >= (since) && \
 	 tgl_revids_get(p)->disp_stepping <= (until))
 
-#define IS_TGL_UY_GT_REVID(p, since, until) \
-	((IS_TGL_U(p) || IS_TGL_Y(p)) && \
-	 tgl_uy_revids[INTEL_REVID(p)].gt_stepping >= (since) && \
-	 tgl_uy_revids[INTEL_REVID(p)].gt_stepping <= (until))
-
 #define IS_TGL_GT_REVID(p, since, until) \
 	(IS_TIGERLAKE(p) && \
-	 !(IS_TGL_U(p) || IS_TGL_Y(p)) && \
-	 tgl_revids[INTEL_REVID(p)].gt_stepping >= (since) && \
-	 tgl_revids[INTEL_REVID(p)].gt_stepping <= (until))
+	 tgl_revids_get(p)->gt_stepping >= (since) && \
+	 tgl_revids_get(p)->gt_stepping <= (until))
 
 #define RKL_REVID_A0		0x0
 #define RKL_REVID_B0		0x1