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

[Lucas De Marchi]

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 at intel.com>
>Cc: Matt Roper <matthew.d.roper at intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Jani Nikula <jani.nikula at intel.com>
>Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>Signed-off-by: Aditya Swarup <aditya.swarup at 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
>