Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds
@ 2020-02-26  1:45 José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 02/14] drm/i915/tgl: Extend Wa_1409825376 stepping José Roberto de Souza
                   ` (17 more replies)
  0 siblings, 18 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

Spliting GT and display revisions id to correctly apply workarounds
because we have some issues that were fixed in display B0 but no
hardware was made with B0 stepping, so to keep consistent with BSpec
splitting it from GT and adding this adittional handling.

BSpec: 52890
BSpec: 44455
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: James Ausmus <james.ausmus@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 .../drm/i915/display/intel_display_power.c    |  2 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  2 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c   | 10 +++---
 drivers/gpu/drm/i915/i915_drv.h               | 36 +++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c               |  2 +-
 5 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 6e25a1317161..82af963106ab 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
 	const struct buddy_page_mask *table;
 	int i;
 
-	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
+	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
 		/* Wa_1409767108: tgl */
 		table = wa_1409767108_buddy_page_masks;
 	else
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 39b0125b7143..852981d533a8 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
 		/*
 		 * Wa_1604544889:tgl
 		 */
-		if (IS_TGL_REVID(request->i915, TGL_REVID_A0, TGL_REVID_A0)) {
+		if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
 			flags = 0;
 			flags |= PIPE_CONTROL_CS_STALL;
 			flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 887e0dc701f7..bc6114b6dc8f 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
 	 * the read of FF_MODE2.
 	 */
 	wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
-	       IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 :
-			    FF_MODE2_TDS_TIMER_MASK);
+	       IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
+			       TGL_GT_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK);
 }
 
 static void
@@ -931,13 +931,13 @@ static void
 tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
 	/* Wa_1409420604:tgl */
-	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
+	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
 		wa_write_or(wal,
 			    SUBSLICE_UNIT_LEVEL_CLKGATE2,
 			    CPSSUNIT_CLKGATE_DIS);
 
 	/* Wa_1409180338:tgl */
-	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
+	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
 		wa_write_or(wal,
 			    SLICE_UNIT_LEVEL_CLKGATE,
 			    L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
@@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 {
 	struct drm_i915_private *i915 = engine->i915;
 
-	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
+	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
 		/* Wa_1606700617:tgl */
 		wa_masked_en(wal,
 			     GEN9_CS_DEBUG_MODE1,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ea13fc0b409b..4fa01f8d3f33 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define IS_ICL_REVID(p, since, until) \
 	(IS_ICELAKE(p) && IS_REVID(p, since, until))
 
-#define TGL_REVID_A0		0x0
+#define TGL_GT_REVID_A0		0x0
 
-#define IS_TGL_REVID(p, since, until) \
+#define IS_TGL_GT_REVID(p, since, until) \
 	(IS_TIGERLAKE(p) && IS_REVID(p, since, until))
 
+/*
+ * revid=0x0 = DISP_REVID_A0
+ * revid=0x1 = DISP_REVID_C0
+ * revid=0x2 = DISP_REVID_D0
+ *
+ * So ids bellow will not match PCI revid and the function bellow is used.
+ */
+#define TGL_DISP_REVID_A0 0x0
+#define TGL_DISP_REVID_B0 0x1
+#define TGL_DISP_REVID_C0 0x2
+#define TGL_DISP_REVID_D0 0x3
+
+static inline bool
+_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
+{
+	const u8 gt2_disp_revid[] = {
+		TGL_DISP_REVID_A0,
+		TGL_DISP_REVID_C0,
+		TGL_DISP_REVID_D0
+	};
+	u8 disp_revid;
+
+	if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
+		disp_revid = TGL_DISP_REVID_D0;
+	else
+		disp_revid = gt2_disp_revid[INTEL_REVID(p)];
+
+	return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid <= until;
+}
+
+#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p, since, until)
+
 #define IS_LP(dev_priv)	(INTEL_INFO(dev_priv)->is_lp)
 #define IS_GEN9_LP(dev_priv)	(IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
 #define IS_GEN9_BC(dev_priv)	(IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 22aa205793e5..49484d5f5f84 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
 		   I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
 
 	/* Wa_1409825376:tgl (pre-prod)*/
-	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
+	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
 		I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
 			   TGL_VRH_GATING_DIS);
 }
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 02/14] drm/i915/tgl: Extend Wa_1409825376 stepping
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26 23:43   ` Radhakrishna Sripada
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 03/14] drm/i915/tgl: Implement Wa_1409804808 José Roberto de Souza
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

This workaround is only fixed in C0 stepping to extend it to B0 too.

BSpec: 52890
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 49484d5f5f84..e6d9ec124db8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
 		   I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
 
 	/* Wa_1409825376:tgl (pre-prod)*/
-	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
+	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_B0))
 		I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
 			   TGL_VRH_GATING_DIS);
 }
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 03/14] drm/i915/tgl: Implement Wa_1409804808
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 02/14] drm/i915/tgl: Extend Wa_1409825376 stepping José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 04/14] drm/i915/tgl: Implement Wa_1806527549 José Roberto de Souza
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

This workaround the CS not done issue on PIPE_CONTROL.

v2:
- replaced BIT() by REG_BIT() in all GEN7_ROW_CHICKEN2() bits
- shortened the name of the new bit

BSpec: 52890
BSpec: 46218
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 ++++++
 drivers/gpu/drm/i915/i915_reg.h             | 5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index bc6114b6dc8f..8139f1443bd7 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1367,6 +1367,12 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			     GEN12_DISABLE_EARLY_READ);
 	}
 
+	if (IS_TIGERLAKE(i915)) {
+		/* Wa_1409804808:tgl */
+		wa_masked_en(wal, GEN7_ROW_CHICKEN2,
+			     GEN12_PUSH_CONST_DEREF_HOLD_DIS);
+	}
+
 	if (IS_GEN(i915, 11)) {
 		/* This is not an Wa. Enable for better image quality */
 		wa_masked_en(wal,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f45b5e86ec63..7edd5dfbd585 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9149,8 +9149,9 @@ enum {
 #define   THROTTLE_12_5				(7 << 2)
 #define   DISABLE_EARLY_EOT			(1 << 1)
 
-#define GEN7_ROW_CHICKEN2		_MMIO(0xe4f4)
-#define GEN12_DISABLE_EARLY_READ	BIT(14)
+#define GEN7_ROW_CHICKEN2			_MMIO(0xe4f4)
+#define   GEN12_DISABLE_EARLY_READ		REG_BIT(14)
+#define   GEN12_PUSH_CONST_DEREF_HOLD_DIS	REG_BIT(8)
 
 #define GEN7_ROW_CHICKEN2_GT2		_MMIO(0xf4f4)
 #define   DOP_CLOCK_GATING_DISABLE	(1 << 0)
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 04/14] drm/i915/tgl: Implement Wa_1806527549
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 02/14] drm/i915/tgl: Extend Wa_1409825376 stepping José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 03/14] drm/i915/tgl: Implement Wa_1409804808 José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 05/14] drm/i915/tgl: Add Wa_1409085225, Wa_14010229206 José Roberto de Souza
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

This will whitelist the HIZ_CHICKEN register so mesa can disable the
optimizations and avoid hang when using D16_UNORM.

v2: moved to the right place and used the right function() (Chris)

Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Rafael Antognolli <rafael.antognolli@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 8139f1443bd7..0d80de88ccf3 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1264,6 +1264,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine)
 
 		/* Wa_1808121037:tgl */
 		whitelist_reg(w, GEN7_COMMON_SLICE_CHICKEN1);
+
+		/* Wa_1806527549:tgl */
+		whitelist_reg(w, HIZ_CHICKEN);
 		break;
 	default:
 		break;
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 05/14] drm/i915/tgl: Add Wa_1409085225, Wa_14010229206
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (2 preceding siblings ...)
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 04/14] drm/i915/tgl: Implement Wa_1806527549 José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 06/14] drm/i915/tgl: Extend Wa_1606931601 for all steppings José Roberto de Souza
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

From: Matt Atwood <matthew.s.atwood@intel.com>

Disable Push Constant buffer addition for TGL.

v2: typos, add additional Wa reference
v3: use REG_BIT macro, move to rcs_engine_wa_init, clean up commit
message.

Bspec: 52890
Cc: Rafael Antognolli <rafael.antognolli@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 ++++++
 drivers/gpu/drm/i915/i915_reg.h             | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 0d80de88ccf3..1d1aa3967add 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1368,6 +1368,12 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 		wa_masked_en(wal,
 			     GEN7_ROW_CHICKEN2,
 			     GEN12_DISABLE_EARLY_READ);
+
+		/*
+		 * Wa_1409085225:tgl
+		 * Wa_14010229206:tgl
+		 */
+		wa_masked_en(wal, GEN9_ROW_CHICKEN4, GEN12_DISABLE_TDL_PUSH);
 	}
 
 	if (IS_TIGERLAKE(i915)) {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7edd5dfbd585..e2abd910ae80 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9158,6 +9158,9 @@ enum {
 #define   PUSH_CONSTANT_DEREF_DISABLE	(1 << 8)
 #define   GEN11_TDL_CLOCK_GATING_FIX_DISABLE	(1 << 1)
 
+#define GEN9_ROW_CHICKEN4		_MMIO(0xe48c)
+#define   GEN12_DISABLE_TDL_PUSH	REG_BIT(9)
+
 #define HSW_ROW_CHICKEN3		_MMIO(0xe49c)
 #define  HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE    (1 << 6)
 
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 06/14] drm/i915/tgl: Extend Wa_1606931601 for all steppings
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (3 preceding siblings ...)
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 05/14] drm/i915/tgl: Add Wa_1409085225, Wa_14010229206 José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 07/14] drm/i915/tgl: Add note to Wa_1607297627 José Roberto de Souza
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

From: Anusha Srivatsa <anusha.srivatsa@intel.com>

According to BSpec. Wa_1606931601 applies for all TGL steppings.
This patch moves the WA implementation out of A0 only block of
rcs_engine_wa_init().

The WA is has also been referred to by an alternate name
Wa_1607090982.

Bspec: 46045, 52890

Fixes: 3873fd1a43c7 ("drm/i915: Use engine wa list for Wa_1607090982")
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 1d1aa3967add..bc0af522542b 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1364,11 +1364,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			    GEN7_FF_THREAD_MODE,
 			    GEN12_FF_TESSELATION_DOP_GATE_DISABLE);
 
-		/* Wa_1606931601:tgl */
-		wa_masked_en(wal,
-			     GEN7_ROW_CHICKEN2,
-			     GEN12_DISABLE_EARLY_READ);
-
 		/*
 		 * Wa_1409085225:tgl
 		 * Wa_14010229206:tgl
@@ -1377,6 +1372,9 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 	}
 
 	if (IS_TIGERLAKE(i915)) {
+		/* Wa_1606931601:tgl */
+		wa_masked_en(wal, GEN7_ROW_CHICKEN2, GEN12_DISABLE_EARLY_READ);
+
 		/* Wa_1409804808:tgl */
 		wa_masked_en(wal, GEN7_ROW_CHICKEN2,
 			     GEN12_PUSH_CONST_DEREF_HOLD_DIS);
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 07/14] drm/i915/tgl: Add note to Wa_1607297627
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (4 preceding siblings ...)
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 06/14] drm/i915/tgl: Extend Wa_1606931601 for all steppings José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 08/14] drm/i915/tgl: Add note about Wa_1607063988 José Roberto de Souza
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

Add note about the confliting information in BSpec about this WA.

BSpec: 52890
Acked-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index bc0af522542b..143ff3daab0d 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1343,9 +1343,13 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			    GEN9_CTX_PREEMPT_REG,
 			    GEN12_DISABLE_POSH_BUSY_FF_DOP_CG);
 
-		/* Wa_1607030317:tgl */
-		/* Wa_1607186500:tgl */
-		/* Wa_1607297627:tgl */
+		/*
+		 * Wa_1607030317:tgl
+		 * Wa_1607186500:tgl
+		 * Wa_1607297627:tgl there is 3 entries for this WA on BSpec, 2
+		 * of then says it is fixed on B0 the other one says it is
+		 * permanent
+		 */
 		wa_masked_en(wal,
 			     GEN6_RC_SLEEP_PSMI_CONTROL,
 			     GEN12_WAIT_FOR_EVENT_POWER_DOWN_DISABLE |
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 08/14] drm/i915/tgl: Add note about Wa_1607063988
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (5 preceding siblings ...)
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 07/14] drm/i915/tgl: Add note to Wa_1607297627 José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 09/14] drm/i915/tgl: Extend Wa_1409767108 to B0 José Roberto de Souza
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

This issue workaround in Wa_1607063988 has the same fix as
Wa_1607138336, so just adding a note in the code.

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 143ff3daab0d..92cc412cd93c 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1338,7 +1338,10 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			     GEN9_CS_DEBUG_MODE1,
 			     FF_DOP_CLOCK_GATE_DISABLE);
 
-		/* Wa_1607138336:tgl */
+		/*
+		 * Wa_1607138336:tgl
+		 * Wa_1607063988:tgl
+		 */
 		wa_write_or(wal,
 			    GEN9_CTX_PREEMPT_REG,
 			    GEN12_DISABLE_POSH_BUSY_FF_DOP_CG);
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 09/14] drm/i915/tgl: Extend Wa_1409767108 to B0
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (6 preceding siblings ...)
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 08/14] drm/i915/tgl: Add note about Wa_1607063988 José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 10/14] drm/i915/tgl: Fix the Wa number of a fix José Roberto de Souza
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

This Wa will also be needed by B0 stepping.

BSpec: 52890
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 82af963106ab..134879385df6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
 	const struct buddy_page_mask *table;
 	int i;
 
-	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
+	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_B0))
 		/* Wa_1409767108: tgl */
 		table = wa_1409767108_buddy_page_masks;
 	else
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 10/14] drm/i915/tgl: Fix the Wa number of a fix
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (7 preceding siblings ...)
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 09/14] drm/i915/tgl: Extend Wa_1409767108 to B0 José Roberto de Souza
@ 2020-02-26  1:45 ` José Roberto de Souza
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 11/14] drm/i915/tgl: Add note about Wa_1409142259 José Roberto de Souza
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:45 UTC (permalink / raw)
  To: intel-gfx

The Wa number for this fix is Wa_1607087056 the BSpec bug id is
1607087056, just updating to match BSpec.

BSpec: 52890
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 92cc412cd93c..e7ca0eab2700 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -936,7 +936,7 @@ tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 			    SUBSLICE_UNIT_LEVEL_CLKGATE2,
 			    CPSSUNIT_CLKGATE_DIS);
 
-	/* Wa_1409180338:tgl */
+	/* Wa_1607087056:tgl also know as BUG:1409180338 */
 	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
 		wa_write_or(wal,
 			    SLICE_UNIT_LEVEL_CLKGATE,
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 11/14] drm/i915/tgl: Add note about Wa_1409142259
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (8 preceding siblings ...)
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 10/14] drm/i915/tgl: Fix the Wa number of a fix José Roberto de Souza
@ 2020-02-26  1:46 ` José Roberto de Souza
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 12/14] drm/i915/tgl: Restrict Wa_1408615072 to GT A0 stepping José Roberto de Souza
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:46 UTC (permalink / raw)
  To: intel-gfx

Different issues with the same fix, so justing adding
Wa_1409142259, Wa_1409252684, Wa_1409217633, Wa_1409207793,
Wa_1409178076 and 1408979724 to the comment so other devs can check if
this Was were implemetend with a simple grep.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index e7ca0eab2700..b7b13372e287 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -582,7 +582,15 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
 {
 	u32 val;
 
-	/* Wa_1409142259:tgl */
+	/*
+	 * Wa_1409142259:tgl
+	 * Wa_1409347922:tgl
+	 * Wa_1409252684:tgl
+	 * Wa_1409217633:tgl
+	 * Wa_1409207793:tgl
+	 * Wa_1409178076:tgl
+	 * Wa_1408979724:tgl
+	 */
 	WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,
 			  GEN12_DISABLE_CPS_AWARE_COLOR_PIPE);
 
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 12/14] drm/i915/tgl: Restrict Wa_1408615072 to GT A0 stepping
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (9 preceding siblings ...)
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 11/14] drm/i915/tgl: Add note about Wa_1409142259 José Roberto de Souza
@ 2020-02-26  1:46 ` José Roberto de Souza
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 13/14] drm/i915/tgl: Add Wa number to WaAllowPMDepthAndInvocationCountAccessFromUMD José Roberto de Souza
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:46 UTC (permalink / raw)
  To: intel-gfx

It is fixed in B0 stepping.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e6d9ec124db8..9df150f877b1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6838,8 +6838,9 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
 	unsigned int i;
 
 	/* Wa_1408615072:tgl */
-	intel_uncore_rmw(&dev_priv->uncore, UNSLICE_UNIT_LEVEL_CLKGATE2,
-			 0, VSUNIT_CLKGATE_DIS_TGL);
+	if (IS_TGL_GT_REVID(dev_priv, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
+		intel_uncore_rmw(&dev_priv->uncore, UNSLICE_UNIT_LEVEL_CLKGATE2,
+				 0, VSUNIT_CLKGATE_DIS_TGL);
 
 	/* This is not a WA. Enable VD HCP & MFX_ENC powergate */
 	for (i = 0; i < I915_MAX_VCS; i++) {
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 13/14] drm/i915/tgl: Add Wa number to WaAllowPMDepthAndInvocationCountAccessFromUMD
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (10 preceding siblings ...)
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 12/14] drm/i915/tgl: Restrict Wa_1408615072 to GT A0 stepping José Roberto de Souza
@ 2020-02-26  1:46 ` José Roberto de Souza
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 14/14] drm/i915/tgl: Implement Wa_1407901919 José Roberto de Souza
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:46 UTC (permalink / raw)
  To: intel-gfx

Just to make easier to check that the Wa was implemetend when
comparing to the number in BSpec.

BSpec: 52890
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index b7b13372e287..164b5e82e3e3 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1259,6 +1259,7 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine)
 	case RENDER_CLASS:
 		/*
 		 * WaAllowPMDepthAndInvocationCountAccessFromUMD:tgl
+		 * Wa_1408556865:tgl
 		 *
 		 * This covers 4 registers which are next to one another :
 		 *   - PS_INVOCATION_COUNT
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [PATCH v2 14/14] drm/i915/tgl: Implement Wa_1407901919
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (11 preceding siblings ...)
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 13/14] drm/i915/tgl: Add Wa number to WaAllowPMDepthAndInvocationCountAccessFromUMD José Roberto de Souza
@ 2020-02-26  1:46 ` José Roberto de Souza
  2020-02-26 14:57   ` Chris Wilson
  2020-02-26 15:06 ` [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds Tvrtko Ursulin
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 24+ messages in thread
From: José Roberto de Souza @ 2020-02-26  1:46 UTC (permalink / raw)
  To: intel-gfx

This will fix a memory coherence issue.

BSpec: 52890
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c |  6 ++++++
 drivers/gpu/drm/i915/i915_reg.h             | 20 +++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 164b5e82e3e3..b3bb3dd90f02 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -606,6 +606,12 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
 	wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
 	       IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
 			       TGL_GT_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK);
+
+	/* Wa_1407901919:tgl */
+	wa_add(wal, ICL_HDC_MODE, HDC_COHERENT_ACCESS_L1_CACHE_DIS |
+	       HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W, 0,
+	       HDC_COHERENT_ACCESS_L1_CACHE_DIS |
+	       HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e2abd910ae80..3f592636f982 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7892,15 +7892,17 @@ enum {
 #define  GEN8_LQSC_FLUSH_COHERENT_LINES		(1 << 21)
 
 /* GEN8 chicken */
-#define HDC_CHICKEN0				_MMIO(0x7300)
-#define CNL_HDC_CHICKEN0			_MMIO(0xE5F0)
-#define ICL_HDC_MODE				_MMIO(0xE5F4)
-#define  HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE	(1 << 15)
-#define  HDC_FENCE_DEST_SLM_DISABLE		(1 << 14)
-#define  HDC_DONOT_FETCH_MEM_WHEN_MASKED	(1 << 11)
-#define  HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT	(1 << 5)
-#define  HDC_FORCE_NON_COHERENT			(1 << 4)
-#define  HDC_BARRIER_PERFORMANCE_DISABLE	(1 << 10)
+#define HDC_CHICKEN0					_MMIO(0x7300)
+#define CNL_HDC_CHICKEN0				_MMIO(0xE5F0)
+#define ICL_HDC_MODE					_MMIO(0xE5F4)
+#define  HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE		REG_BIT(15)
+#define  HDC_FENCE_DEST_SLM_DISABLE			REG_BIT(14)
+#define  HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W	REG_BIT(13)
+#define  HDC_COHERENT_ACCESS_L1_CACHE_DIS		REG_BIT(12)
+#define  HDC_DONOT_FETCH_MEM_WHEN_MASKED		REG_BIT(11)
+#define  HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT	REG_BIT(5)
+#define  HDC_FORCE_NON_COHERENT				REG_BIT(4)
+#define  HDC_BARRIER_PERFORMANCE_DISABLE		REG_BIT(10)
 
 #define GEN8_HDC_CHICKEN1			_MMIO(0x7304)
 
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [PATCH v2 14/14] drm/i915/tgl: Implement Wa_1407901919
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 14/14] drm/i915/tgl: Implement Wa_1407901919 José Roberto de Souza
@ 2020-02-26 14:57   ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2020-02-26 14:57 UTC (permalink / raw)
  To: José Roberto de Souza, intel-gfx

Quoting José Roberto de Souza (2020-02-26 01:46:03)
> This will fix a memory coherence issue.
> 
> BSpec: 52890
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c |  6 ++++++
>  drivers/gpu/drm/i915/i915_reg.h             | 20 +++++++++++---------
>  2 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 164b5e82e3e3..b3bb3dd90f02 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -606,6 +606,12 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>         wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
>                IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
>                                TGL_GT_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK);
> +
> +       /* Wa_1407901919:tgl */
> +       wa_add(wal, ICL_HDC_MODE, HDC_COHERENT_ACCESS_L1_CACHE_DIS |
> +              HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W, 0,
> +              HDC_COHERENT_ACCESS_L1_CACHE_DIS |
> +              HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W);

Dropping by to say that is hard to read. Could you perhaps use
whitespace for visual grouping to an advantage?

	wa_write(wal, ICL_HDC_MODE,
		 HDC_COHERENT_ACCESS_L1_CACHE_DIS |
		 HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W);
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (12 preceding siblings ...)
  2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 14/14] drm/i915/tgl: Implement Wa_1407901919 José Roberto de Souza
@ 2020-02-26 15:06 ` Tvrtko Ursulin
  2020-02-26 18:42 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,01/14] " Patchwork
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2020-02-26 15:06 UTC (permalink / raw)
  To: José Roberto de Souza, intel-gfx


On 26/02/2020 01:45, José Roberto de Souza wrote:
> Spliting GT and display revisions id to correctly apply workarounds
> because we have some issues that were fixed in display B0 but no
> hardware was made with B0 stepping, so to keep consistent with BSpec
> splitting it from GT and adding this adittional handling.
> 
> BSpec: 52890
> BSpec: 44455
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   .../drm/i915/display/intel_display_power.c    |  2 +-
>   drivers/gpu/drm/i915/gt/intel_lrc.c           |  2 +-
>   drivers/gpu/drm/i915/gt/intel_workarounds.c   | 10 +++---
>   drivers/gpu/drm/i915/i915_drv.h               | 36 +++++++++++++++++--
>   drivers/gpu/drm/i915/intel_pm.c               |  2 +-
>   5 files changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 6e25a1317161..82af963106ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
>   	const struct buddy_page_mask *table;
>   	int i;
>   
> -	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
>   		/* Wa_1409767108: tgl */
>   		table = wa_1409767108_buddy_page_masks;
>   	else
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 39b0125b7143..852981d533a8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
>   		/*
>   		 * Wa_1604544889:tgl
>   		 */
> -		if (IS_TGL_REVID(request->i915, TGL_REVID_A0, TGL_REVID_A0)) {
> +		if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
>   			flags = 0;
>   			flags |= PIPE_CONTROL_CS_STALL;
>   			flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 887e0dc701f7..bc6114b6dc8f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>   	 * the read of FF_MODE2.
>   	 */
>   	wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
> -	       IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 :
> -			    FF_MODE2_TDS_TIMER_MASK);
> +	       IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
> +			       TGL_GT_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK);
>   }
>   
>   static void
> @@ -931,13 +931,13 @@ static void
>   tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
>   {
>   	/* Wa_1409420604:tgl */
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
>   		wa_write_or(wal,
>   			    SUBSLICE_UNIT_LEVEL_CLKGATE2,
>   			    CPSSUNIT_CLKGATE_DIS);
>   
>   	/* Wa_1409180338:tgl */
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
>   		wa_write_or(wal,
>   			    SLICE_UNIT_LEVEL_CLKGATE,
>   			    L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> @@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>   {
>   	struct drm_i915_private *i915 = engine->i915;
>   
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
>   		/* Wa_1606700617:tgl */
>   		wa_masked_en(wal,
>   			     GEN9_CS_DEBUG_MODE1,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ea13fc0b409b..4fa01f8d3f33 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>   #define IS_ICL_REVID(p, since, until) \
>   	(IS_ICELAKE(p) && IS_REVID(p, since, until))
>   
> -#define TGL_REVID_A0		0x0
> +#define TGL_GT_REVID_A0		0x0
>   
> -#define IS_TGL_REVID(p, since, until) \
> +#define IS_TGL_GT_REVID(p, since, until) \
>   	(IS_TIGERLAKE(p) && IS_REVID(p, since, until))
>   
> +/*
> + * revid=0x0 = DISP_REVID_A0
> + * revid=0x1 = DISP_REVID_C0
> + * revid=0x2 = DISP_REVID_D0
> + *
> + * So ids bellow will not match PCI revid and the function bellow is used.
> + */
> +#define TGL_DISP_REVID_A0 0x0
> +#define TGL_DISP_REVID_B0 0x1
> +#define TGL_DISP_REVID_C0 0x2
> +#define TGL_DISP_REVID_D0 0x3
> +
> +static inline bool
> +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)

Why p and not the usual i915?

> +{
> +	const u8 gt2_disp_revid[] = {
> +		TGL_DISP_REVID_A0,
> +		TGL_DISP_REVID_C0,
> +		TGL_DISP_REVID_D0
> +	};
> +	u8 disp_revid;
> +
> +	if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
> +		disp_revid = TGL_DISP_REVID_D0;

Do you want a GEM_WARN_ON here to notice code needs updating?

> +	else
> +		disp_revid = gt2_disp_revid[INTEL_REVID(p)];
> +
> +	return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid <= until;
> +}

Overall the function feels maybe a bit large-ish for a static inline. 
Perhaps there will be just a few call sites I don't know.. Or how about 
about putting display revid into runtime info?

Or option of leaving IS_TGL_REVID as is and only adding 
IS_TGL_DISP_REVID for a smaller series?

Just throwing ideas around.

Regards,

Tvrtko

> +
> +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p, since, until)
> +
>   #define IS_LP(dev_priv)	(INTEL_INFO(dev_priv)->is_lp)
>   #define IS_GEN9_LP(dev_priv)	(IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
>   #define IS_GEN9_BC(dev_priv)	(IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 22aa205793e5..49484d5f5f84 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
>   		   I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
>   
>   	/* Wa_1409825376:tgl (pre-prod)*/
> -	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
>   		I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
>   			   TGL_VRH_GATING_DIS);
>   }
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,01/14] drm/i915/tgl: Split GT and display workarounds
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (13 preceding siblings ...)
  2020-02-26 15:06 ` [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds Tvrtko Ursulin
@ 2020-02-26 18:42 ` Patchwork
  2020-02-26 19:08 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2020-02-26 18:42 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,01/14] drm/i915/tgl: Split GT and display workarounds
URL   : https://patchwork.freedesktop.org/series/73934/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
78f68ec9e707 drm/i915/tgl: Split GT and display workarounds
-:98: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'p' - possible side-effects?
#98: FILE: drivers/gpu/drm/i915/i915_drv.h:1579:
+#define IS_TGL_GT_REVID(p, since, until) \
 	(IS_TIGERLAKE(p) && IS_REVID(p, since, until))

total: 0 errors, 0 warnings, 1 checks, 102 lines checked
c844fca8f0ab drm/i915/tgl: Extend Wa_1409825376 stepping
636c7aa65c45 drm/i915/tgl: Implement Wa_1409804808
33c977b2b167 drm/i915/tgl: Implement Wa_1806527549
48fc18342e19 drm/i915/tgl: Add Wa_1409085225, Wa_14010229206
41b18f808a31 drm/i915/tgl: Extend Wa_1606931601 for all steppings
c1bfb255ece7 drm/i915/tgl: Add note to Wa_1607297627
a9caaf045a2d drm/i915/tgl: Add note about Wa_1607063988
5a6cb2d06c81 drm/i915/tgl: Extend Wa_1409767108 to B0
89be33cb7055 drm/i915/tgl: Fix the Wa number of a fix
2b971433ed1e drm/i915/tgl: Add note about Wa_1409142259
4663f5dc469f drm/i915/tgl: Restrict Wa_1408615072 to GT A0 stepping
91ad7e7c3b57 drm/i915/tgl: Add Wa number to WaAllowPMDepthAndInvocationCountAccessFromUMD
3f7e7be95819 drm/i915/tgl: Implement Wa_1407901919

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,01/14] drm/i915/tgl: Split GT and display workarounds
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (14 preceding siblings ...)
  2020-02-26 18:42 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,01/14] " Patchwork
@ 2020-02-26 19:08 ` Patchwork
  2020-02-27  0:02 ` [Intel-gfx] [PATCH v2 01/14] " Lucas De Marchi
  2020-02-27  6:23 ` Jani Nikula
  17 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2020-02-26 19:08 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,01/14] drm/i915/tgl: Split GT and display workarounds
URL   : https://patchwork.freedesktop.org/series/73934/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8008 -> Patchwork_16713
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16713 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16713, 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_16713/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@dmabuf@sanitycheck:
    - fi-bwr-2160:        [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8008/fi-bwr-2160/igt@dmabuf@sanitycheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16713/fi-bwr-2160/igt@dmabuf@sanitycheck.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@bad-flink:
    - fi-tgl-y:           [PASS][3] -> [DMESG-WARN][4] ([CI#94] / [i915#402])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8008/fi-tgl-y/igt@gem_flink_basic@bad-flink.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16713/fi-tgl-y/igt@gem_flink_basic@bad-flink.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-icl-y:           [DMESG-FAIL][5] ([fdo#108569]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8008/fi-icl-y/igt@i915_selftest@live_execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16713/fi-icl-y/igt@i915_selftest@live_execlists.html

  * igt@kms_addfb_basic@bad-pitch-0:
    - fi-tgl-y:           [DMESG-WARN][7] ([CI#94] / [i915#402]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8008/fi-tgl-y/igt@kms_addfb_basic@bad-pitch-0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16713/fi-tgl-y/igt@kms_addfb_basic@bad-pitch-0.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [i915#1233]: https://gitlab.freedesktop.org/drm/intel/issues/1233
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402


Participating hosts (52 -> 44)
------------------------------

  Additional (1): fi-byt-n2820 
  Missing    (9): fi-ilk-m540 fi-hsw-4200u fi-bsw-n3050 fi-skl-6770hq fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8008 -> Patchwork_16713

  CI-20190529: 20190529
  CI_DRM_8008: 13b6e2575f2c05722679bc1c9d0b97c13bde49a1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5469: 4f875016eb1ebc211b8aadb280ae16c7e6cdc8ba @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16713: 3f7e7be95819fb6c8e2d3451578465b5d0f61f6d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3f7e7be95819 drm/i915/tgl: Implement Wa_1407901919
91ad7e7c3b57 drm/i915/tgl: Add Wa number to WaAllowPMDepthAndInvocationCountAccessFromUMD
4663f5dc469f drm/i915/tgl: Restrict Wa_1408615072 to GT A0 stepping
2b971433ed1e drm/i915/tgl: Add note about Wa_1409142259
89be33cb7055 drm/i915/tgl: Fix the Wa number of a fix
5a6cb2d06c81 drm/i915/tgl: Extend Wa_1409767108 to B0
a9caaf045a2d drm/i915/tgl: Add note about Wa_1607063988
c1bfb255ece7 drm/i915/tgl: Add note to Wa_1607297627
41b18f808a31 drm/i915/tgl: Extend Wa_1606931601 for all steppings
48fc18342e19 drm/i915/tgl: Add Wa_1409085225, Wa_14010229206
33c977b2b167 drm/i915/tgl: Implement Wa_1806527549
636c7aa65c45 drm/i915/tgl: Implement Wa_1409804808
c844fca8f0ab drm/i915/tgl: Extend Wa_1409825376 stepping
78f68ec9e707 drm/i915/tgl: Split GT and display workarounds

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16713/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [PATCH v2 02/14] drm/i915/tgl: Extend Wa_1409825376 stepping
  2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 02/14] drm/i915/tgl: Extend Wa_1409825376 stepping José Roberto de Souza
@ 2020-02-26 23:43   ` Radhakrishna Sripada
  0 siblings, 0 replies; 24+ messages in thread
From: Radhakrishna Sripada @ 2020-02-26 23:43 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

On Tue, Feb 25, 2020 at 05:45:51PM -0800, José Roberto de Souza wrote:
> This workaround is only fixed in C0 stepping to extend it to B0 too.
> 
> BSpec: 52890
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 49484d5f5f84..e6d9ec124db8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
>  		   I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
>  
>  	/* Wa_1409825376:tgl (pre-prod)*/
> -	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
> +	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_B0))
>  		I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
>  			   TGL_VRH_GATING_DIS);
>  }
> -- 
> 2.25.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (15 preceding siblings ...)
  2020-02-26 19:08 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2020-02-27  0:02 ` Lucas De Marchi
  2020-02-27  0:42   ` Souza, Jose
  2020-02-27  6:23 ` Jani Nikula
  17 siblings, 1 reply; 24+ messages in thread
From: Lucas De Marchi @ 2020-02-27  0:02 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics

On Tue, Feb 25, 2020 at 5:45 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> Spliting GT and display revisions id to correctly apply workarounds
> because we have some issues that were fixed in display B0 but no
> hardware was made with B0 stepping, so to keep consistent with BSpec
> splitting it from GT and adding this adittional handling.
>
> BSpec: 52890
> BSpec: 44455
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    |  2 +-
>  drivers/gpu/drm/i915/gt/intel_lrc.c           |  2 +-
>  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 10 +++---
>  drivers/gpu/drm/i915/i915_drv.h               | 36 +++++++++++++++++--
>  drivers/gpu/drm/i915/intel_pm.c               |  2 +-
>  5 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 6e25a1317161..82af963106ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
>         const struct buddy_page_mask *table;
>         int i;
>
> -       if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> +       if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
>                 /* Wa_1409767108: tgl */
>                 table = wa_1409767108_buddy_page_masks;
>         else
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 39b0125b7143..852981d533a8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
>                 /*
>                  * Wa_1604544889:tgl
>                  */
> -               if (IS_TGL_REVID(request->i915, TGL_REVID_A0, TGL_REVID_A0)) {
> +               if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
>                         flags = 0;
>                         flags |= PIPE_CONTROL_CS_STALL;
>                         flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 887e0dc701f7..bc6114b6dc8f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>          * the read of FF_MODE2.
>          */
>         wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
> -              IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 :
> -                           FF_MODE2_TDS_TIMER_MASK);
> +              IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
> +                              TGL_GT_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK);
>  }
>
>  static void
> @@ -931,13 +931,13 @@ static void
>  tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
>  {
>         /* Wa_1409420604:tgl */
> -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
>                 wa_write_or(wal,
>                             SUBSLICE_UNIT_LEVEL_CLKGATE2,
>                             CPSSUNIT_CLKGATE_DIS);
>
>         /* Wa_1409180338:tgl */
> -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
>                 wa_write_or(wal,
>                             SLICE_UNIT_LEVEL_CLKGATE,
>                             L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> @@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>  {
>         struct drm_i915_private *i915 = engine->i915;
>
> -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
>                 /* Wa_1606700617:tgl */
>                 wa_masked_en(wal,
>                              GEN9_CS_DEBUG_MODE1,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ea13fc0b409b..4fa01f8d3f33 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define IS_ICL_REVID(p, since, until) \
>         (IS_ICELAKE(p) && IS_REVID(p, since, until))
>
> -#define TGL_REVID_A0           0x0
> +#define TGL_GT_REVID_A0                0x0
>
> -#define IS_TGL_REVID(p, since, until) \
> +#define IS_TGL_GT_REVID(p, since, until) \
>         (IS_TIGERLAKE(p) && IS_REVID(p, since, until))
>
> +/*
> + * revid=0x0 = DISP_REVID_A0
> + * revid=0x1 = DISP_REVID_C0
> + * revid=0x2 = DISP_REVID_D0
> + *
> + * So ids bellow will not match PCI revid and the function bellow is used.
> + */
> +#define TGL_DISP_REVID_A0 0x0
> +#define TGL_DISP_REVID_B0 0x1
> +#define TGL_DISP_REVID_C0 0x2
> +#define TGL_DISP_REVID_D0 0x3
> +
> +static inline bool
> +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
> +{
> +       const u8 gt2_disp_revid[] = {
> +               TGL_DISP_REVID_A0,
> +               TGL_DISP_REVID_C0,
> +               TGL_DISP_REVID_D0

So we are hardcoding the mapping in the code using the same
information. Why do we even need any of this?
Just define it like this:

#define TGL_DISP_REVID_A0 0x0
#define TGL_DISP_REVID_C0 0x1
#define TGL_DISP_REVID_D0 0x2

Then in the workarounds we continue to use IS_TGL_REVID().  This means
we document it is a "display wa",
but since we don't have a INTEL_DISPLAY_REVID(), IMO it doesn't make
sense to have all this function to do
the mapping.... just make it a compile-time map.

Lucas De Marchi

> +       };
> +       u8 disp_revid;
> +
> +       if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
> +               disp_revid = TGL_DISP_REVID_D0;
> +       else
> +               disp_revid = gt2_disp_revid[INTEL_REVID(p)];
> +
> +       return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid <= until;
> +}
> +
> +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p, since, until)
> +
>  #define IS_LP(dev_priv)        (INTEL_INFO(dev_priv)->is_lp)
>  #define IS_GEN9_LP(dev_priv)   (IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
>  #define IS_GEN9_BC(dev_priv)   (IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 22aa205793e5..49484d5f5f84 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
>                    I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
>
>         /* Wa_1409825376:tgl (pre-prod)*/
> -       if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> +       if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
>                 I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
>                            TGL_VRH_GATING_DIS);
>  }
> --
> 2.25.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds
  2020-02-27  0:02 ` [Intel-gfx] [PATCH v2 01/14] " Lucas De Marchi
@ 2020-02-27  0:42   ` Souza, Jose
  2020-02-27  6:19     ` Lucas De Marchi
  2020-02-27  6:31     ` Jani Nikula
  0 siblings, 2 replies; 24+ messages in thread
From: Souza, Jose @ 2020-02-27  0:42 UTC (permalink / raw)
  To: lucas.de.marchi@gmail.com; +Cc: intel-gfx@lists.freedesktop.org

On Wed, 2020-02-26 at 16:02 -0800, Lucas De Marchi wrote:
> On Tue, Feb 25, 2020 at 5:45 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
> > Spliting GT and display revisions id to correctly apply workarounds
> > because we have some issues that were fixed in display B0 but no
> > hardware was made with B0 stepping, so to keep consistent with
> > BSpec
> > splitting it from GT and adding this adittional handling.
> > 
> > BSpec: 52890
> > BSpec: 44455
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: James Ausmus <james.ausmus@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_power.c    |  2 +-
> >  drivers/gpu/drm/i915/gt/intel_lrc.c           |  2 +-
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 10 +++---
> >  drivers/gpu/drm/i915/i915_drv.h               | 36
> > +++++++++++++++++--
> >  drivers/gpu/drm/i915/intel_pm.c               |  2 +-
> >  5 files changed, 42 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 6e25a1317161..82af963106ab 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct
> > drm_i915_private *dev_priv)
> >         const struct buddy_page_mask *table;
> >         int i;
> > 
> > -       if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> > +       if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0,
> > TGL_DISP_REVID_A0))
> >                 /* Wa_1409767108: tgl */
> >                 table = wa_1409767108_buddy_page_masks;
> >         else
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 39b0125b7143..852981d533a8 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct
> > i915_request *request,
> >                 /*
> >                  * Wa_1604544889:tgl
> >                  */
> > -               if (IS_TGL_REVID(request->i915, TGL_REVID_A0,
> > TGL_REVID_A0)) {
> > +               if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0,
> > TGL_GT_REVID_A0)) {
> >                         flags = 0;
> >                         flags |= PIPE_CONTROL_CS_STALL;
> >                         flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index 887e0dc701f7..bc6114b6dc8f 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct
> > intel_engine_cs *engine,
> >          * the read of FF_MODE2.
> >          */
> >         wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
> > -              IS_TGL_REVID(engine->i915, TGL_REVID_A0,
> > TGL_REVID_A0) ? 0 :
> > -                           FF_MODE2_TDS_TIMER_MASK);
> > +              IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
> > +                              TGL_GT_REVID_A0) ? 0 :
> > FF_MODE2_TDS_TIMER_MASK);
> >  }
> > 
> >  static void
> > @@ -931,13 +931,13 @@ static void
> >  tgl_gt_workarounds_init(struct drm_i915_private *i915, struct
> > i915_wa_list *wal)
> >  {
> >         /* Wa_1409420604:tgl */
> > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
> > TGL_GT_REVID_A0))
> >                 wa_write_or(wal,
> >                             SUBSLICE_UNIT_LEVEL_CLKGATE2,
> >                             CPSSUNIT_CLKGATE_DIS);
> > 
> >         /* Wa_1409180338:tgl */
> > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
> > TGL_GT_REVID_A0))
> >                 wa_write_or(wal,
> >                             SLICE_UNIT_LEVEL_CLKGATE,
> >                             L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> > @@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs
> > *engine, struct i915_wa_list *wal)
> >  {
> >         struct drm_i915_private *i915 = engine->i915;
> > 
> > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
> > TGL_GT_REVID_A0)) {
> >                 /* Wa_1606700617:tgl */
> >                 wa_masked_en(wal,
> >                              GEN9_CS_DEBUG_MODE1,
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index ea13fc0b409b..4fa01f8d3f33 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct
> > drm_i915_private *i915,
> >  #define IS_ICL_REVID(p, since, until) \
> >         (IS_ICELAKE(p) && IS_REVID(p, since, until))
> > 
> > -#define TGL_REVID_A0           0x0
> > +#define TGL_GT_REVID_A0                0x0
> > 
> > -#define IS_TGL_REVID(p, since, until) \
> > +#define IS_TGL_GT_REVID(p, since, until) \
> >         (IS_TIGERLAKE(p) && IS_REVID(p, since, until))
> > 
> > +/*
> > + * revid=0x0 = DISP_REVID_A0
> > + * revid=0x1 = DISP_REVID_C0
> > + * revid=0x2 = DISP_REVID_D0
> > + *
> > + * So ids bellow will not match PCI revid and the function bellow
> > is used.
> > + */
> > +#define TGL_DISP_REVID_A0 0x0
> > +#define TGL_DISP_REVID_B0 0x1
> > +#define TGL_DISP_REVID_C0 0x2
> > +#define TGL_DISP_REVID_D0 0x3
> > +
> > +static inline bool
> > +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
> > +{
> > +       const u8 gt2_disp_revid[] = {
> > +               TGL_DISP_REVID_A0,
> > +               TGL_DISP_REVID_C0,
> > +               TGL_DISP_REVID_D0
> 
> So we are hardcoding the mapping in the code using the same
> information. Why do we even need any of this?
> Just define it like this:
> 
> #define TGL_DISP_REVID_A0 0x0
> #define TGL_DISP_REVID_C0 0x1
> #define TGL_DISP_REVID_D0 0x2

What to do with the workarounds fixed or introduced in display B0?

> 
> Then in the workarounds we continue to use IS_TGL_REVID().  This
> means
> we document it is a "display wa",
> but since we don't have a INTEL_DISPLAY_REVID(), IMO it doesn't make
> sense to have all this function to do
> the mapping.... just make it a compile-time map.
> 
> Lucas De Marchi
> 
> > +       };
> > +       u8 disp_revid;
> > +
> > +       if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
> > +               disp_revid = TGL_DISP_REVID_D0;
> > +       else
> > +               disp_revid = gt2_disp_revid[INTEL_REVID(p)];
> > +
> > +       return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid
> > <= until;
> > +}
> > +
> > +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p,
> > since, until)
> > +
> >  #define IS_LP(dev_priv)        (INTEL_INFO(dev_priv)->is_lp)
> >  #define IS_GEN9_LP(dev_priv)   (IS_GEN(dev_priv, 9) &&
> > IS_LP(dev_priv))
> >  #define IS_GEN9_BC(dev_priv)   (IS_GEN(dev_priv, 9) &&
> > !IS_LP(dev_priv))
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 22aa205793e5..49484d5f5f84 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct
> > drm_i915_private *dev_priv)
> >                    I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
> > 
> >         /* Wa_1409825376:tgl (pre-prod)*/
> > -       if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> > +       if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0,
> > TGL_DISP_REVID_A0))
> >                 I915_WRITE(GEN9_CLKGATE_DIS_3,
> > I915_READ(GEN9_CLKGATE_DIS_3) |
> >                            TGL_VRH_GATING_DIS);
> >  }
> > --
> > 2.25.1
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds
  2020-02-27  0:42   ` Souza, Jose
@ 2020-02-27  6:19     ` Lucas De Marchi
  2020-02-27  6:31     ` Jani Nikula
  1 sibling, 0 replies; 24+ messages in thread
From: Lucas De Marchi @ 2020-02-27  6:19 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org

On Wed, Feb 26, 2020 at 4:43 PM Souza, Jose <jose.souza@intel.com> wrote:
>
> On Wed, 2020-02-26 at 16:02 -0800, Lucas De Marchi wrote:
> > On Tue, Feb 25, 2020 at 5:45 PM José Roberto de Souza
> > <jose.souza@intel.com> wrote:
> > > Spliting GT and display revisions id to correctly apply workarounds
> > > because we have some issues that were fixed in display B0 but no
> > > hardware was made with B0 stepping, so to keep consistent with
> > > BSpec
> > > splitting it from GT and adding this adittional handling.
> > >
> > > BSpec: 52890
> > > BSpec: 44455
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Cc: James Ausmus <james.ausmus@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  .../drm/i915/display/intel_display_power.c    |  2 +-
> > >  drivers/gpu/drm/i915/gt/intel_lrc.c           |  2 +-
> > >  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 10 +++---
> > >  drivers/gpu/drm/i915/i915_drv.h               | 36
> > > +++++++++++++++++--
> > >  drivers/gpu/drm/i915/intel_pm.c               |  2 +-
> > >  5 files changed, 42 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index 6e25a1317161..82af963106ab 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct
> > > drm_i915_private *dev_priv)
> > >         const struct buddy_page_mask *table;
> > >         int i;
> > >
> > > -       if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> > > +       if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0,
> > > TGL_DISP_REVID_A0))
> > >                 /* Wa_1409767108: tgl */
> > >                 table = wa_1409767108_buddy_page_masks;
> > >         else
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > > b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > > index 39b0125b7143..852981d533a8 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > > @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct
> > > i915_request *request,
> > >                 /*
> > >                  * Wa_1604544889:tgl
> > >                  */
> > > -               if (IS_TGL_REVID(request->i915, TGL_REVID_A0,
> > > TGL_REVID_A0)) {
> > > +               if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0,
> > > TGL_GT_REVID_A0)) {
> > >                         flags = 0;
> > >                         flags |= PIPE_CONTROL_CS_STALL;
> > >                         flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > index 887e0dc701f7..bc6114b6dc8f 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct
> > > intel_engine_cs *engine,
> > >          * the read of FF_MODE2.
> > >          */
> > >         wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
> > > -              IS_TGL_REVID(engine->i915, TGL_REVID_A0,
> > > TGL_REVID_A0) ? 0 :
> > > -                           FF_MODE2_TDS_TIMER_MASK);
> > > +              IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
> > > +                              TGL_GT_REVID_A0) ? 0 :
> > > FF_MODE2_TDS_TIMER_MASK);
> > >  }
> > >
> > >  static void
> > > @@ -931,13 +931,13 @@ static void
> > >  tgl_gt_workarounds_init(struct drm_i915_private *i915, struct
> > > i915_wa_list *wal)
> > >  {
> > >         /* Wa_1409420604:tgl */
> > > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
> > > TGL_GT_REVID_A0))
> > >                 wa_write_or(wal,
> > >                             SUBSLICE_UNIT_LEVEL_CLKGATE2,
> > >                             CPSSUNIT_CLKGATE_DIS);
> > >
> > >         /* Wa_1409180338:tgl */
> > > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
> > > TGL_GT_REVID_A0))
> > >                 wa_write_or(wal,
> > >                             SLICE_UNIT_LEVEL_CLKGATE,
> > >                             L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> > > @@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs
> > > *engine, struct i915_wa_list *wal)
> > >  {
> > >         struct drm_i915_private *i915 = engine->i915;
> > >
> > > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> > > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
> > > TGL_GT_REVID_A0)) {
> > >                 /* Wa_1606700617:tgl */
> > >                 wa_masked_en(wal,
> > >                              GEN9_CS_DEBUG_MODE1,
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index ea13fc0b409b..4fa01f8d3f33 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct
> > > drm_i915_private *i915,
> > >  #define IS_ICL_REVID(p, since, until) \
> > >         (IS_ICELAKE(p) && IS_REVID(p, since, until))
> > >
> > > -#define TGL_REVID_A0           0x0
> > > +#define TGL_GT_REVID_A0                0x0
> > >
> > > -#define IS_TGL_REVID(p, since, until) \
> > > +#define IS_TGL_GT_REVID(p, since, until) \
> > >         (IS_TIGERLAKE(p) && IS_REVID(p, since, until))
> > >
> > > +/*
> > > + * revid=0x0 = DISP_REVID_A0
> > > + * revid=0x1 = DISP_REVID_C0
> > > + * revid=0x2 = DISP_REVID_D0
> > > + *
> > > + * So ids bellow will not match PCI revid and the function bellow
> > > is used.
> > > + */
> > > +#define TGL_DISP_REVID_A0 0x0
> > > +#define TGL_DISP_REVID_B0 0x1
> > > +#define TGL_DISP_REVID_C0 0x2
> > > +#define TGL_DISP_REVID_D0 0x3
> > > +
> > > +static inline bool
> > > +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
> > > +{
> > > +       const u8 gt2_disp_revid[] = {
> > > +               TGL_DISP_REVID_A0,
> > > +               TGL_DISP_REVID_C0,
> > > +               TGL_DISP_REVID_D0
> >
> > So we are hardcoding the mapping in the code using the same
> > information. Why do we even need any of this?
> > Just define it like this:
> >
> > #define TGL_DISP_REVID_A0 0x0
> > #define TGL_DISP_REVID_C0 0x1
> > #define TGL_DISP_REVID_D0 0x2
>
> What to do with the workarounds fixed or introduced in display B0?

what would you do if you had display B0?  You only have revid to compare to....
The only possible problem would be if you had display <letter>0 being available
on more than one revids. But then you don't have enough info to decide since you
don't have a display revid.

Lucas De Marchi

>
> >
> > Then in the workarounds we continue to use IS_TGL_REVID().  This
> > means
> > we document it is a "display wa",
> > but since we don't have a INTEL_DISPLAY_REVID(), IMO it doesn't make
> > sense to have all this function to do
> > the mapping.... just make it a compile-time map.
> >
> > Lucas De Marchi
> >
> > > +       };
> > > +       u8 disp_revid;
> > > +
> > > +       if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
> > > +               disp_revid = TGL_DISP_REVID_D0;
> > > +       else
> > > +               disp_revid = gt2_disp_revid[INTEL_REVID(p)];
> > > +
> > > +       return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid
> > > <= until;
> > > +}
> > > +
> > > +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p,
> > > since, until)
> > > +
> > >  #define IS_LP(dev_priv)        (INTEL_INFO(dev_priv)->is_lp)
> > >  #define IS_GEN9_LP(dev_priv)   (IS_GEN(dev_priv, 9) &&
> > > IS_LP(dev_priv))
> > >  #define IS_GEN9_BC(dev_priv)   (IS_GEN(dev_priv, 9) &&
> > > !IS_LP(dev_priv))
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 22aa205793e5..49484d5f5f84 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct
> > > drm_i915_private *dev_priv)
> > >                    I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
> > >
> > >         /* Wa_1409825376:tgl (pre-prod)*/
> > > -       if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> > > +       if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0,
> > > TGL_DISP_REVID_A0))
> > >                 I915_WRITE(GEN9_CLKGATE_DIS_3,
> > > I915_READ(GEN9_CLKGATE_DIS_3) |
> > >                            TGL_VRH_GATING_DIS);
> > >  }
> > > --
> > > 2.25.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >



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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds
  2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
                   ` (16 preceding siblings ...)
  2020-02-27  0:02 ` [Intel-gfx] [PATCH v2 01/14] " Lucas De Marchi
@ 2020-02-27  6:23 ` Jani Nikula
  17 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2020-02-27  6:23 UTC (permalink / raw)
  To: José Roberto de Souza, intel-gfx

On Tue, 25 Feb 2020, José Roberto de Souza <jose.souza@intel.com> wrote:
> Spliting GT and display revisions id to correctly apply workarounds
> because we have some issues that were fixed in display B0 but no
> hardware was made with B0 stepping, so to keep consistent with BSpec
> splitting it from GT and adding this adittional handling.
>
> BSpec: 52890
> BSpec: 44455
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    |  2 +-
>  drivers/gpu/drm/i915/gt/intel_lrc.c           |  2 +-
>  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 10 +++---
>  drivers/gpu/drm/i915/i915_drv.h               | 36 +++++++++++++++++--
>  drivers/gpu/drm/i915/intel_pm.c               |  2 +-
>  5 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 6e25a1317161..82af963106ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
>  	const struct buddy_page_mask *table;
>  	int i;
>  
> -	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
>  		/* Wa_1409767108: tgl */
>  		table = wa_1409767108_buddy_page_masks;
>  	else
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 39b0125b7143..852981d533a8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct i915_request *request,
>  		/*
>  		 * Wa_1604544889:tgl
>  		 */
> -		if (IS_TGL_REVID(request->i915, TGL_REVID_A0, TGL_REVID_A0)) {
> +		if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
>  			flags = 0;
>  			flags |= PIPE_CONTROL_CS_STALL;
>  			flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 887e0dc701f7..bc6114b6dc8f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>  	 * the read of FF_MODE2.
>  	 */
>  	wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
> -	       IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 :
> -			    FF_MODE2_TDS_TIMER_MASK);
> +	       IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
> +			       TGL_GT_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK);
>  }
>  
>  static void
> @@ -931,13 +931,13 @@ static void
>  tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
>  {
>  	/* Wa_1409420604:tgl */
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
>  		wa_write_or(wal,
>  			    SUBSLICE_UNIT_LEVEL_CLKGATE2,
>  			    CPSSUNIT_CLKGATE_DIS);
>  
>  	/* Wa_1409180338:tgl */
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0))
>  		wa_write_or(wal,
>  			    SLICE_UNIT_LEVEL_CLKGATE,
>  			    L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> @@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>  {
>  	struct drm_i915_private *i915 = engine->i915;
>  
> -	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> +	if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0, TGL_GT_REVID_A0)) {
>  		/* Wa_1606700617:tgl */
>  		wa_masked_en(wal,
>  			     GEN9_CS_DEBUG_MODE1,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ea13fc0b409b..4fa01f8d3f33 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define IS_ICL_REVID(p, since, until) \
>  	(IS_ICELAKE(p) && IS_REVID(p, since, until))
>  
> -#define TGL_REVID_A0		0x0
> +#define TGL_GT_REVID_A0		0x0
>  
> -#define IS_TGL_REVID(p, since, until) \
> +#define IS_TGL_GT_REVID(p, since, until) \
>  	(IS_TIGERLAKE(p) && IS_REVID(p, since, until))
>  
> +/*
> + * revid=0x0 = DISP_REVID_A0
> + * revid=0x1 = DISP_REVID_C0
> + * revid=0x2 = DISP_REVID_D0
> + *
> + * So ids bellow will not match PCI revid and the function bellow is used.
> + */
> +#define TGL_DISP_REVID_A0 0x0
> +#define TGL_DISP_REVID_B0 0x1
> +#define TGL_DISP_REVID_C0 0x2
> +#define TGL_DISP_REVID_D0 0x3
> +
> +static inline bool
> +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
> +{
> +	const u8 gt2_disp_revid[] = {
> +		TGL_DISP_REVID_A0,
> +		TGL_DISP_REVID_C0,
> +		TGL_DISP_REVID_D0
> +	};
> +	u8 disp_revid;
> +
> +	if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
> +		disp_revid = TGL_DISP_REVID_D0;
> +	else
> +		disp_revid = gt2_disp_revid[INTEL_REVID(p)];
> +
> +	return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid <= until;
> +}
> +
> +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p, since, until)
> +

IOW, you could just define display steppings in terms of the revids, and
would not have to introduce two separate revid test macros?

#define TGL_DISPLAY_REVID_A0	TGL_REVID_A0
#define TGL_DISPLAY_REVID_B0	TGL_REVID_A0
#define TGL_DISPLAY_REVID_C0	TGL_REVID_B0
#define TGL_DISPLAY_REVID_D0	TGL_REVID_C0

And then you could simply use:

	if (IS_TGL_REVID(i915, TGL_DISPLAY_REVID_A0, TGL_DISPLAY_REVID_B0))

At this point, I don't think there's any reason at all to add the extra
indirection and function, and two separate revid check macros. Because
ultimately it all depends on a single revid, not two independent revids
for GT and display.

BR,
Jani.


>  #define IS_LP(dev_priv)	(INTEL_INFO(dev_priv)->is_lp)
>  #define IS_GEN9_LP(dev_priv)	(IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
>  #define IS_GEN9_BC(dev_priv)	(IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 22aa205793e5..49484d5f5f84 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
>  		   I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
>  
>  	/* Wa_1409825376:tgl (pre-prod)*/
> -	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> +	if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0, TGL_DISP_REVID_A0))
>  		I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
>  			   TGL_VRH_GATING_DIS);
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds
  2020-02-27  0:42   ` Souza, Jose
  2020-02-27  6:19     ` Lucas De Marchi
@ 2020-02-27  6:31     ` Jani Nikula
  1 sibling, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2020-02-27  6:31 UTC (permalink / raw)
  To: Souza, Jose, lucas.de.marchi@gmail.com; +Cc: intel-gfx@lists.freedesktop.org

On Thu, 27 Feb 2020, "Souza, Jose" <jose.souza@intel.com> wrote:
> On Wed, 2020-02-26 at 16:02 -0800, Lucas De Marchi wrote:
>> On Tue, Feb 25, 2020 at 5:45 PM José Roberto de Souza
>> <jose.souza@intel.com> wrote:
>> > Spliting GT and display revisions id to correctly apply workarounds
>> > because we have some issues that were fixed in display B0 but no
>> > hardware was made with B0 stepping, so to keep consistent with
>> > BSpec
>> > splitting it from GT and adding this adittional handling.
>> > 
>> > BSpec: 52890
>> > BSpec: 44455
>> > Cc: Matt Roper <matthew.d.roper@intel.com>
>> > Cc: James Ausmus <james.ausmus@intel.com>
>> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>> > ---
>> >  .../drm/i915/display/intel_display_power.c    |  2 +-
>> >  drivers/gpu/drm/i915/gt/intel_lrc.c           |  2 +-
>> >  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 10 +++---
>> >  drivers/gpu/drm/i915/i915_drv.h               | 36
>> > +++++++++++++++++--
>> >  drivers/gpu/drm/i915/intel_pm.c               |  2 +-
>> >  5 files changed, 42 insertions(+), 10 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
>> > b/drivers/gpu/drm/i915/display/intel_display_power.c
>> > index 6e25a1317161..82af963106ab 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
>> > @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct
>> > drm_i915_private *dev_priv)
>> >         const struct buddy_page_mask *table;
>> >         int i;
>> > 
>> > -       if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
>> > +       if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0,
>> > TGL_DISP_REVID_A0))
>> >                 /* Wa_1409767108: tgl */
>> >                 table = wa_1409767108_buddy_page_masks;
>> >         else
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > index 39b0125b7143..852981d533a8 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct
>> > i915_request *request,
>> >                 /*
>> >                  * Wa_1604544889:tgl
>> >                  */
>> > -               if (IS_TGL_REVID(request->i915, TGL_REVID_A0,
>> > TGL_REVID_A0)) {
>> > +               if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0,
>> > TGL_GT_REVID_A0)) {
>> >                         flags = 0;
>> >                         flags |= PIPE_CONTROL_CS_STALL;
>> >                         flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> > index 887e0dc701f7..bc6114b6dc8f 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> > @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct
>> > intel_engine_cs *engine,
>> >          * the read of FF_MODE2.
>> >          */
>> >         wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
>> > -              IS_TGL_REVID(engine->i915, TGL_REVID_A0,
>> > TGL_REVID_A0) ? 0 :
>> > -                           FF_MODE2_TDS_TIMER_MASK);
>> > +              IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
>> > +                              TGL_GT_REVID_A0) ? 0 :
>> > FF_MODE2_TDS_TIMER_MASK);
>> >  }
>> > 
>> >  static void
>> > @@ -931,13 +931,13 @@ static void
>> >  tgl_gt_workarounds_init(struct drm_i915_private *i915, struct
>> > i915_wa_list *wal)
>> >  {
>> >         /* Wa_1409420604:tgl */
>> > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
>> > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
>> > TGL_GT_REVID_A0))
>> >                 wa_write_or(wal,
>> >                             SUBSLICE_UNIT_LEVEL_CLKGATE2,
>> >                             CPSSUNIT_CLKGATE_DIS);
>> > 
>> >         /* Wa_1409180338:tgl */
>> > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
>> > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
>> > TGL_GT_REVID_A0))
>> >                 wa_write_or(wal,
>> >                             SLICE_UNIT_LEVEL_CLKGATE,
>> >                             L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
>> > @@ -1329,7 +1329,7 @@ rcs_engine_wa_init(struct intel_engine_cs
>> > *engine, struct i915_wa_list *wal)
>> >  {
>> >         struct drm_i915_private *i915 = engine->i915;
>> > 
>> > -       if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
>> > +       if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
>> > TGL_GT_REVID_A0)) {
>> >                 /* Wa_1606700617:tgl */
>> >                 wa_masked_en(wal,
>> >                              GEN9_CS_DEBUG_MODE1,
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> > b/drivers/gpu/drm/i915/i915_drv.h
>> > index ea13fc0b409b..4fa01f8d3f33 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct
>> > drm_i915_private *i915,
>> >  #define IS_ICL_REVID(p, since, until) \
>> >         (IS_ICELAKE(p) && IS_REVID(p, since, until))
>> > 
>> > -#define TGL_REVID_A0           0x0
>> > +#define TGL_GT_REVID_A0                0x0
>> > 
>> > -#define IS_TGL_REVID(p, since, until) \
>> > +#define IS_TGL_GT_REVID(p, since, until) \
>> >         (IS_TIGERLAKE(p) && IS_REVID(p, since, until))
>> > 
>> > +/*
>> > + * revid=0x0 = DISP_REVID_A0
>> > + * revid=0x1 = DISP_REVID_C0
>> > + * revid=0x2 = DISP_REVID_D0
>> > + *
>> > + * So ids bellow will not match PCI revid and the function bellow
>> > is used.
>> > + */
>> > +#define TGL_DISP_REVID_A0 0x0
>> > +#define TGL_DISP_REVID_B0 0x1
>> > +#define TGL_DISP_REVID_C0 0x2
>> > +#define TGL_DISP_REVID_D0 0x3
>> > +
>> > +static inline bool
>> > +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
>> > +{
>> > +       const u8 gt2_disp_revid[] = {
>> > +               TGL_DISP_REVID_A0,
>> > +               TGL_DISP_REVID_C0,
>> > +               TGL_DISP_REVID_D0
>> 
>> So we are hardcoding the mapping in the code using the same
>> information. Why do we even need any of this?
>> Just define it like this:
>> 
>> #define TGL_DISP_REVID_A0 0x0
>> #define TGL_DISP_REVID_C0 0x1
>> #define TGL_DISP_REVID_D0 0x2
>
> What to do with the workarounds fixed or introduced in display B0?

It's all based on a single revid. How does your proposal distinguish
that?

Reading bspec, if you really need to make the distinction, it eerily
seems like you'd have to look at pci ids i.e. use the subplatform
mechanism to hack this. Because, again, there's no separate display
revid.

BR,
Jani.



>
>> 
>> Then in the workarounds we continue to use IS_TGL_REVID().  This
>> means
>> we document it is a "display wa",
>> but since we don't have a INTEL_DISPLAY_REVID(), IMO it doesn't make
>> sense to have all this function to do
>> the mapping.... just make it a compile-time map.
>> 
>> Lucas De Marchi
>> 
>> > +       };
>> > +       u8 disp_revid;
>> > +
>> > +       if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
>> > +               disp_revid = TGL_DISP_REVID_D0;
>> > +       else
>> > +               disp_revid = gt2_disp_revid[INTEL_REVID(p)];
>> > +
>> > +       return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid
>> > <= until;
>> > +}
>> > +
>> > +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p,
>> > since, until)
>> > +
>> >  #define IS_LP(dev_priv)        (INTEL_INFO(dev_priv)->is_lp)
>> >  #define IS_GEN9_LP(dev_priv)   (IS_GEN(dev_priv, 9) &&
>> > IS_LP(dev_priv))
>> >  #define IS_GEN9_BC(dev_priv)   (IS_GEN(dev_priv, 9) &&
>> > !IS_LP(dev_priv))
>> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
>> > b/drivers/gpu/drm/i915/intel_pm.c
>> > index 22aa205793e5..49484d5f5f84 100644
>> > --- a/drivers/gpu/drm/i915/intel_pm.c
>> > +++ b/drivers/gpu/drm/i915/intel_pm.c
>> > @@ -6852,7 +6852,7 @@ static void tgl_init_clock_gating(struct
>> > drm_i915_private *dev_priv)
>> >                    I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
>> > 
>> >         /* Wa_1409825376:tgl (pre-prod)*/
>> > -       if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
>> > +       if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0,
>> > TGL_DISP_REVID_A0))
>> >                 I915_WRITE(GEN9_CLKGATE_DIS_3,
>> > I915_READ(GEN9_CLKGATE_DIS_3) |
>> >                            TGL_VRH_GATING_DIS);
>> >  }
>> > --
>> > 2.25.1
>> > 
>> > _______________________________________________
>> > 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

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2020-02-27  6:31 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-26  1:45 [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds José Roberto de Souza
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 02/14] drm/i915/tgl: Extend Wa_1409825376 stepping José Roberto de Souza
2020-02-26 23:43   ` Radhakrishna Sripada
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 03/14] drm/i915/tgl: Implement Wa_1409804808 José Roberto de Souza
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 04/14] drm/i915/tgl: Implement Wa_1806527549 José Roberto de Souza
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 05/14] drm/i915/tgl: Add Wa_1409085225, Wa_14010229206 José Roberto de Souza
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 06/14] drm/i915/tgl: Extend Wa_1606931601 for all steppings José Roberto de Souza
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 07/14] drm/i915/tgl: Add note to Wa_1607297627 José Roberto de Souza
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 08/14] drm/i915/tgl: Add note about Wa_1607063988 José Roberto de Souza
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 09/14] drm/i915/tgl: Extend Wa_1409767108 to B0 José Roberto de Souza
2020-02-26  1:45 ` [Intel-gfx] [PATCH v2 10/14] drm/i915/tgl: Fix the Wa number of a fix José Roberto de Souza
2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 11/14] drm/i915/tgl: Add note about Wa_1409142259 José Roberto de Souza
2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 12/14] drm/i915/tgl: Restrict Wa_1408615072 to GT A0 stepping José Roberto de Souza
2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 13/14] drm/i915/tgl: Add Wa number to WaAllowPMDepthAndInvocationCountAccessFromUMD José Roberto de Souza
2020-02-26  1:46 ` [Intel-gfx] [PATCH v2 14/14] drm/i915/tgl: Implement Wa_1407901919 José Roberto de Souza
2020-02-26 14:57   ` Chris Wilson
2020-02-26 15:06 ` [Intel-gfx] [PATCH v2 01/14] drm/i915/tgl: Split GT and display workarounds Tvrtko Ursulin
2020-02-26 18:42 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,01/14] " Patchwork
2020-02-26 19:08 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-02-27  0:02 ` [Intel-gfx] [PATCH v2 01/14] " Lucas De Marchi
2020-02-27  0:42   ` Souza, Jose
2020-02-27  6:19     ` Lucas De Marchi
2020-02-27  6:31     ` Jani Nikula
2020-02-27  6:23 ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox