[Intel-gfx] [PATCH 0/4] Steer multicast register workaround verification

[Intel-gfx] [PATCH 0/4] Steer multicast register workaround verification

[Matt Roper]

We're seeing some CI errors indicating that a workaround did not apply
properly on EHL/JSL.  The workaround in question is updating a multicast
register, the failures are only seen on specific CI machines, and the
failures only seem to happen on resets and such rather than on initial
driver load.  It seems likely that the culprit here is failure to steer
the multicast register readback on a SKU that has slice0 / subslice0
fused off.

This series makes a couple changes:
 * Workaround verification will explicitly steer MCR registers by
   calling read_subslice_reg rather than a regular read.
 * New multicast ranges are added for gen11 and gen12.  Sadly this
   information is still missing from the bspec (just like the updated
   forcewake tables).  The hardware guys have given us a spreadsheet
   with both the forcewake and the multicast information while they work
   on getting the spec properly updated, so that's where the new ranges
   come from.

In addition to MCR and forcewake, there's supposed to be some more bspec
updates coming soon that deal with steering (i.e., different MCR ranges
should actually be using different registers to steer rather than just
the 0xFDC register we're familiar with); I don't have the full details
on that yet, so those updates will have to wait until we actually have
an updated spec. 

References: https://gitlab.freedesktop.org/drm/intel/issues/1222

Matt Roper (4):
  drm/i915: Setup multicast register steering for all gen >= 10
  drm/i915: Steer multicast register readback in wa_verify
  drm/i915: Don't skip verification of MCR engine workarounds
  drm/i915: Add MCR ranges for gen11 and gen12

 drivers/gpu/drm/i915/gt/intel_engine.h        |   3 +
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  17 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c   | 146 ++++++++++++------
 .../gpu/drm/i915/gt/intel_workarounds_types.h |   2 +
 4 files changed, 110 insertions(+), 58 deletions(-)

-- 
2.24.1

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

[Intel-gfx] [PATCH 1/4] drm/i915: Setup multicast register steering for all gen >= 10

[Matt Roper]

Steering of multicast registers for workarounds is needed on all
platforms gen10 and above.  Move the wa_init_mcr() call to the
higher-level gt_init_workarounds() rather than re-calling it for each
individual platform.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index adddc5c93b48..4a255de13394 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -870,8 +870,6 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
 static void
 cnl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
-	wa_init_mcr(i915, wal);
-
 	/* WaDisableI2mCycleOnWRPort:cnl (pre-prod) */
 	if (IS_CNL_REVID(i915, CNL_REVID_B0, CNL_REVID_B0))
 		wa_write_or(wal,
@@ -887,8 +885,6 @@ cnl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 static void
 icl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
-	wa_init_mcr(i915, wal);
-
 	/* WaInPlaceDecompressionHang:icl */
 	wa_write_or(wal,
 		    GEN9_GAMT_ECO_REG_RW_IA,
@@ -943,8 +939,6 @@ icl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 static void
 tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
-	wa_init_mcr(i915, wal);
-
 	/* Wa_1409420604:tgl */
 	if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
 		wa_write_or(wal,
@@ -961,6 +955,9 @@ tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 static void
 gt_init_workarounds(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
+	if (INTEL_GEN(i915) >= 10)
+		wa_init_mcr(i915, wal);
+
 	if (IS_GEN(i915, 12))
 		tgl_gt_workarounds_init(i915, wal);
 	else if (IS_GEN(i915, 11))
-- 
2.24.1

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

[Intel-gfx] [PATCH 2/4] drm/i915: Steer multicast register readback in wa_verify

[Matt Roper]

Reads of multicast registers give the value for slice/subslice 0 by
default unless we manually steer them to a specific slice/subslice.  If
slice/subslice 0 are fused off in hardware, we'll always read back a
value of 0 rather than the value we wrote into the multicast register.
Although wa_init_mcr() sets up some initial steering to a valid
slice/subslice at startup, we appear to later lose that steering setting
in some cases.  Let's handle this more explicitly and have wa_verify use
intel_read_subslice_reg() to directly steer the readback of workaround
registers in an MCR range to a valid slice/subslice.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine.h        |  3 +
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     | 17 ++--
 drivers/gpu/drm/i915/gt/intel_workarounds.c   | 98 ++++++++++++-------
 .../gpu/drm/i915/gt/intel_workarounds_types.h |  2 +
 4 files changed, 73 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index d10e52ff059f..54d1fa233a8a 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -214,6 +214,9 @@ int intel_ring_submission_setup(struct intel_engine_cs *engine);
 int intel_engine_stop_cs(struct intel_engine_cs *engine);
 void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine);
 
+u32 intel_read_subslice_reg(struct intel_uncore *uncore,
+			    int slice, int subslice, i915_reg_t reg);
+
 void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask);
 
 u64 intel_engine_get_active_head(const struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index c9e46c5ced43..3ee17aa9928e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -951,12 +951,11 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
 	}
 }
 
-static u32
-read_subslice_reg(const struct intel_engine_cs *engine,
-		  int slice, int subslice, i915_reg_t reg)
+u32
+intel_read_subslice_reg(struct intel_uncore *uncore,
+			int slice, int subslice, i915_reg_t reg)
 {
-	struct drm_i915_private *i915 = engine->i915;
-	struct intel_uncore *uncore = engine->uncore;
+	struct drm_i915_private *i915 = uncore->i915;
 	u32 mcr_mask, mcr_ss, mcr, old_mcr, val;
 	enum forcewake_domains fw_domains;
 
@@ -1027,11 +1026,11 @@ void intel_engine_get_instdone(const struct intel_engine_cs *engine,
 		}
 		for_each_instdone_slice_subslice(i915, sseu, slice, subslice) {
 			instdone->sampler[slice][subslice] =
-				read_subslice_reg(engine, slice, subslice,
-						  GEN7_SAMPLER_INSTDONE);
+				intel_read_subslice_reg(uncore, slice, subslice,
+							GEN7_SAMPLER_INSTDONE);
 			instdone->row[slice][subslice] =
-				read_subslice_reg(engine, slice, subslice,
-						  GEN7_ROW_INSTDONE);
+				intel_read_subslice_reg(uncore, slice, subslice,
+							GEN7_ROW_INSTDONE);
 		}
 		break;
 	case 7:
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 4a255de13394..73a3937c689b 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -56,6 +56,7 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name, const char
 {
 	wal->name = name;
 	wal->engine_name = engine_name;
+	wal->mcrslice = wal->mcrss = 0;
 }
 
 #define WA_LIST_CHUNK (1 << 4)
@@ -863,6 +864,8 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
 	}
 
 	drm_dbg(&i915->drm, "MCR slice/subslice = %x\n", mcr);
+	wal->mcrslice = slice;
+	wal->mcrss = subslice;
 
 	wa_write_masked_or(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr);
 }
@@ -1006,7 +1009,8 @@ wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal)
 }
 
 static bool
-wa_verify(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
+wa_verify(struct drm_i915_private *i915, const struct i915_wa *wa, u32 cur,
+	  const char *name, const char *from)
 {
 	if ((cur ^ wa->set) & wa->read) {
 		DRM_ERROR("%s workaround lost on %s! (%x=%x/%x, expected %x)\n",
@@ -1019,6 +1023,38 @@ wa_verify(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
 	return true;
 }
 
+static const struct {
+	u32 start;
+	u32 end;
+} mcr_ranges_gen8[] = {
+	{ .start = 0x5500, .end = 0x55ff },
+	{ .start = 0x7000, .end = 0x7fff },
+	{ .start = 0x9400, .end = 0x97ff },
+	{ .start = 0xb000, .end = 0xb3ff },
+	{ .start = 0xe000, .end = 0xe7ff },
+	{},
+};
+
+static bool mcr_range(struct drm_i915_private *i915, u32 offset)
+{
+	int i;
+
+	if (INTEL_GEN(i915) < 8)
+		return false;
+
+	/*
+	 * Registers in these ranges are affected by the MCR selector
+	 * which only controls CPU initiated MMIO. Routing does not
+	 * work for CS access so we cannot verify them on this path.
+	 */
+	for (i = 0; mcr_ranges_gen8[i].start; i++)
+		if (offset >= mcr_ranges_gen8[i].start &&
+		    offset <= mcr_ranges_gen8[i].end)
+			return true;
+
+	return false;
+}
+
 static void
 wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal)
 {
@@ -1040,10 +1076,28 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal)
 			intel_uncore_rmw_fw(uncore, wa->reg, wa->clr, wa->set);
 		else
 			intel_uncore_write_fw(uncore, wa->reg, wa->set);
-		if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
-			wa_verify(wa,
-				  intel_uncore_read_fw(uncore, wa->reg),
+		if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) {
+			u32 readout;
+
+			/*
+			 * Reads of multicast registers default to slice 0,
+			 * subslice 0 unless we steer them to a specific
+			 * slice/subslice instance.  If the first slice/subslice
+			 * is fused off, regular reads will come back as 0's
+			 * rather than the value we applied.
+			 */
+			if (mcr_range(uncore->i915,
+				      i915_mmio_reg_offset(wa->reg)))
+				readout = intel_read_subslice_reg(uncore,
+								  wal->mcrslice,
+								  wal->mcrss,
+								  wa->reg);
+			else
+				readout = intel_uncore_read_fw(uncore, wa->reg);
+
+			wa_verify(uncore->i915, wa, readout,
 				  wal->name, "application");
+		}
 	}
 
 	intel_uncore_forcewake_put__locked(uncore, fw);
@@ -1064,7 +1118,7 @@ static bool wa_list_verify(struct intel_uncore *uncore,
 	bool ok = true;
 
 	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
-		ok &= wa_verify(wa,
+		ok &= wa_verify(uncore->i915, wa,
 				intel_uncore_read(uncore, wa->reg),
 				wal->name, from);
 
@@ -1665,38 +1719,6 @@ create_scratch(struct i915_address_space *vm, int count)
 	return ERR_PTR(err);
 }
 
-static const struct {
-	u32 start;
-	u32 end;
-} mcr_ranges_gen8[] = {
-	{ .start = 0x5500, .end = 0x55ff },
-	{ .start = 0x7000, .end = 0x7fff },
-	{ .start = 0x9400, .end = 0x97ff },
-	{ .start = 0xb000, .end = 0xb3ff },
-	{ .start = 0xe000, .end = 0xe7ff },
-	{},
-};
-
-static bool mcr_range(struct drm_i915_private *i915, u32 offset)
-{
-	int i;
-
-	if (INTEL_GEN(i915) < 8)
-		return false;
-
-	/*
-	 * Registers in these ranges are affected by the MCR selector
-	 * which only controls CPU initiated MMIO. Routing does not
-	 * work for CS access so we cannot verify them on this path.
-	 */
-	for (i = 0; mcr_ranges_gen8[i].start; i++)
-		if (offset >= mcr_ranges_gen8[i].start &&
-		    offset <= mcr_ranges_gen8[i].end)
-			return true;
-
-	return false;
-}
-
 static int
 wa_list_srm(struct i915_request *rq,
 	    const struct i915_wa_list *wal,
@@ -1794,7 +1816,7 @@ static int engine_wa_list_verify(struct intel_context *ce,
 		if (mcr_range(rq->i915, i915_mmio_reg_offset(wa->reg)))
 			continue;
 
-		if (!wa_verify(wa, results[i], wal->name, from))
+		if (!wa_verify(rq->i915, wa, results[i], wal->name, from))
 			err = -ENXIO;
 	}
 
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
index d166a7145720..a881ea470aa4 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
@@ -24,6 +24,8 @@ struct i915_wa_list {
 	struct i915_wa	*list;
 	unsigned int	count;
 	unsigned int	wa_count;
+	u32		mcrslice;
+	u32		mcrss;
 };
 
 #endif /* __INTEL_WORKAROUNDS_TYPES_H__ */
-- 
2.24.1

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

[Intel-gfx] [PATCH 3/4] drm/i915: Don't skip verification of MCR engine workarounds

[Matt Roper]

Now that we manually steer multicast register reads during workaround
verification, it should be safe to verify these ones too.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 73a3937c689b..d1b7a445f2da 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1812,13 +1812,9 @@ static int engine_wa_list_verify(struct intel_context *ce,
 	}
 
 	err = 0;
-	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
-		if (mcr_range(rq->i915, i915_mmio_reg_offset(wa->reg)))
-			continue;
-
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
 		if (!wa_verify(rq->i915, wa, results[i], wal->name, from))
 			err = -ENXIO;
-	}
 
 	i915_gem_object_unpin_map(vma->obj);
 
-- 
2.24.1

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

[Intel-gfx] [PATCH 4/4] drm/i915: Add MCR ranges for gen11 and gen12

[Matt Roper]

The multicast register ranges are slightly different for gen11 and gen12
than the table we have for gen8.  This information never got updated in
the bspec, so this patch is based on a spreadsheet provided by the
hardware team while they work on getting the official documentation
updated.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 45 ++++++++++++++++++---
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index d1b7a445f2da..cd71ff347221 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1023,10 +1023,12 @@ wa_verify(struct drm_i915_private *i915, const struct i915_wa *wa, u32 cur,
 	return true;
 }
 
-static const struct {
+struct mcr_range {
 	u32 start;
 	u32 end;
-} mcr_ranges_gen8[] = {
+};
+
+static const struct mcr_range mcr_ranges_gen8[] = {
 	{ .start = 0x5500, .end = 0x55ff },
 	{ .start = 0x7000, .end = 0x7fff },
 	{ .start = 0x9400, .end = 0x97ff },
@@ -1035,11 +1037,42 @@ static const struct {
 	{},
 };
 
+static const struct mcr_range mcr_ranges_gen11[] = {
+	{ .start = 0x5500,  .end = 0x55ff },
+	{ .start = 0x7000,  .end = 0x7fff },
+	{ .start = 0x8140,  .end = 0x815f },
+	{ .start = 0x8c00,  .end = 0x8cff },
+	{ .start = 0x94d0,  .end = 0x955f },
+	{ .start = 0xb000,  .end = 0xb3ff },
+	{ .start = 0xdf00,  .end = 0xe8ff },
+	{ .start = 0x24400, .end = 0x24fff },
+	{},
+};
+
+static const struct mcr_range mcr_ranges_gen12[] = {
+	{ .start = 0xb00,   .end = 0xbff },
+	{ .start = 0x1000,  .end = 0x1fff },
+	{ .start = 0x8150,  .end = 0x815f },
+	{ .start = 0x8700,  .end = 0x87ff },
+	{ .start = 0x9520,  .end = 0x955f },
+	{ .start = 0xb100,  .end = 0xb3ff },
+	{ .start = 0xde80,  .end = 0xe8ff },
+	{ .start = 0x24a00, .end = 0x24a7f },
+	{},
+};
+
 static bool mcr_range(struct drm_i915_private *i915, u32 offset)
 {
+	const struct mcr_range *range_list;
 	int i;
 
-	if (INTEL_GEN(i915) < 8)
+	if (INTEL_GEN(i915) >= 12)
+		range_list = mcr_ranges_gen12;
+	else if (INTEL_GEN(i915) >= 11)
+		range_list = mcr_ranges_gen11;
+	else if (INTEL_GEN(i915) >= 8)
+		range_list = mcr_ranges_gen8;
+	else
 		return false;
 
 	/*
@@ -1047,9 +1080,9 @@ static bool mcr_range(struct drm_i915_private *i915, u32 offset)
 	 * which only controls CPU initiated MMIO. Routing does not
 	 * work for CS access so we cannot verify them on this path.
 	 */
-	for (i = 0; mcr_ranges_gen8[i].start; i++)
-		if (offset >= mcr_ranges_gen8[i].start &&
-		    offset <= mcr_ranges_gen8[i].end)
+	for (i = 0; range_list[i].start; i++)
+		if (offset >= range_list[i].start &&
+		    offset <= range_list[i].end)
 			return true;
 
 	return false;
-- 
2.24.1

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Steer multicast register workaround verification

[Patchwork]

== Series Details ==

Series: Steer multicast register workaround verification
URL   : https://patchwork.freedesktop.org/series/76792/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c89d34e99992 drm/i915: Setup multicast register steering for all gen >= 10
4759117a6337 drm/i915: Steer multicast register readback in wa_verify
-:77: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#77: FILE: drivers/gpu/drm/i915/gt/intel_workarounds.c:59:
+	wal->mcrslice = wal->mcrss = 0;

total: 0 errors, 0 warnings, 1 checks, 195 lines checked
1d656b938337 drm/i915: Don't skip verification of MCR engine workarounds
d348e98bdcf9 drm/i915: Add MCR ranges for gen11 and gen12

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for Steer multicast register workaround verification

[Patchwork]

== Series Details ==

Series: Steer multicast register workaround verification
URL   : https://patchwork.freedesktop.org/series/76792/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8403 -> Patchwork_17534
====================================================

Summary
-------

  **FAILURE**

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@workarounds:
    - fi-bsw-n3050:       [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8403/fi-bsw-n3050/igt@i915_selftest@live@workarounds.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17534/fi-bsw-n3050/igt@i915_selftest@live@workarounds.html
    - fi-bsw-kefka:       [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8403/fi-bsw-kefka/igt@i915_selftest@live@workarounds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17534/fi-bsw-kefka/igt@i915_selftest@live@workarounds.html
    - fi-bsw-nick:        [PASS][5] -> [DMESG-FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8403/fi-bsw-nick/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17534/fi-bsw-nick/igt@i915_selftest@live@workarounds.html
    - fi-bdw-5557u:       [PASS][7] -> [DMESG-FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8403/fi-bdw-5557u/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17534/fi-bdw-5557u/igt@i915_selftest@live@workarounds.html

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@i915_selftest@live@gt_engines:
    - fi-bwr-2160:        [INCOMPLETE][9] ([i915#489]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8403/fi-bwr-2160/igt@i915_selftest@live@gt_engines.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17534/fi-bwr-2160/igt@i915_selftest@live@gt_engines.html

  
  [i915#489]: https://gitlab.freedesktop.org/drm/intel/issues/489


Participating hosts (51 -> 17)
------------------------------

  ERROR: It appears as if the changes made in Patchwork_17534 prevented too many machines from booting.

  Missing    (34): fi-kbl-soraka fi-icl-u2 fi-apl-guc fi-icl-y fi-skl-lmem fi-icl-guc fi-icl-dsi fi-skl-6600u fi-cml-u2 fi-bxt-dsi fi-tgl-u fi-cml-s fi-glk-dsi fi-kbl-7500u fi-ctg-p8600 fi-kbl-7560u fi-skl-6700k2 fi-kbl-r fi-ilk-m540 fi-ehl-1 fi-tgl-dsi fi-skl-guc fi-cfl-8700k fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-cfl-guc fi-kbl-guc fi-whl-u fi-kbl-x1275 fi-cfl-8109u fi-kbl-8809g fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8403 -> Patchwork_17534

  CI-20190529: 20190529
  CI_DRM_8403: 09978e99929f6e5acfe1e959f6499a134f210887 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5619: 94de923ca8d4cc8f532b8062d87aaad9da6ef956 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17534: d348e98bdcf92355b0c475f858e733c87ff2c69a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d348e98bdcf9 drm/i915: Add MCR ranges for gen11 and gen12
1d656b938337 drm/i915: Don't skip verification of MCR engine workarounds
4759117a6337 drm/i915: Steer multicast register readback in wa_verify
c89d34e99992 drm/i915: Setup multicast register steering for all gen >= 10

== Logs ==

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

Re: [Intel-gfx] [PATCH 0/4] Steer multicast register workaround verification

[Tvrtko Ursulin]

Hi,

On 01/05/2020 00:15, Matt Roper wrote:
> We're seeing some CI errors indicating that a workaround did not apply
> properly on EHL/JSL.  The workaround in question is updating a multicast
> register, the failures are only seen on specific CI machines, and the
> failures only seem to happen on resets and such rather than on initial
> driver load.  It seems likely that the culprit here is failure to steer
> the multicast register readback on a SKU that has slice0 / subslice0
> fused off.
> 
> This series makes a couple changes:
>   * Workaround verification will explicitly steer MCR registers by
>     calling read_subslice_reg rather than a regular read.
>   * New multicast ranges are added for gen11 and gen12.  Sadly this
>     information is still missing from the bspec (just like the updated
>     forcewake tables).  The hardware guys have given us a spreadsheet
>     with both the forcewake and the multicast information while they work
>     on getting the spec properly updated, so that's where the new ranges
>     come from.

I think there are multiple things here. To begin with, newly discovered 
ranges are of course a savior.

But I am not sure about the approach of using intel_read_subslice_reg in 
wa_verify. It is one suspicion that 0xfdc is lost on reset, but we do 
reprogram it afterwards don't we? And since it is the first register in 
the list it is supposed to be in place before the rest of verification 
runs, no?

A year or two I tried figuring this for Icelake and failed, but AFAIR 
(maybe my experiments can be found somewhere on trybot patchwork), I 
even tried both applying the affected ones via unicast (for each ss, or 
l3 where applicable) and also verifying a single register in all enabled 
ss. AFAIR there were still some issues there. Granted my memory could be 
leaky.. But I think this multiple write/verify could still be useful.

(Now that I think about it, I think that the problem area back when I 
experiementing with it was more suspend/resume.. hm..)

My main concern is that with current code we effectively have, after reset:

intel_gt_apply_workarounds:
    program 0xfdc
    program the rest of wa
verify_wa
    do reads using configured 0xfdc

So MCR should be correct. This series seems to be doing:

intel_gt_apply_workarounds:
    program 0xfdc
	* store ss used for MCR configuration
    program the rest of wa
verify_wa
    Do reads but reconfigure 0xfdc before every register in range,
    but to the same value as in initial configuration.

Is this correct? Is the thinking then simply writing the same value to 
0xfdc multiple times fixes things?

Regards,

Tvrtko

P.S. Update, found the experiments, listing some of them:

https://patchwork.freedesktop.org/series/64183/
https://patchwork.freedesktop.org/series/64013/

It reminded me that there were some unexplained issues with regards of 
where I used ffs or fls for finding the valid common MCR setting between 
L3 and SSEU. I think we use a different one than Windows but ours works 
better for our verification, empirically at least. Usual disclaimer 
about my leaky memory applies here.

> In addition to MCR and forcewake, there's supposed to be some more bspec
> updates coming soon that deal with steering (i.e., different MCR ranges
> should actually be using different registers to steer rather than just
> the 0xFDC register we're familiar with); I don't have the full details
> on that yet, so those updates will have to wait until we actually have
> an updated spec.
> 
> References: https://gitlab.freedesktop.org/drm/intel/issues/1222
> 
> Matt Roper (4):
>    drm/i915: Setup multicast register steering for all gen >= 10
>    drm/i915: Steer multicast register readback in wa_verify
>    drm/i915: Don't skip verification of MCR engine workarounds
>    drm/i915: Add MCR ranges for gen11 and gen12
> 
>   drivers/gpu/drm/i915/gt/intel_engine.h        |   3 +
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  17 +-
>   drivers/gpu/drm/i915/gt/intel_workarounds.c   | 146 ++++++++++++------
>   .../gpu/drm/i915/gt/intel_workarounds_types.h |   2 +
>   4 files changed, 110 insertions(+), 58 deletions(-)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/4] Steer multicast register workaround verification

[Matt Roper]

On Fri, May 01, 2020 at 09:01:42AM +0100, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 01/05/2020 00:15, Matt Roper wrote:
> > We're seeing some CI errors indicating that a workaround did not apply
> > properly on EHL/JSL.  The workaround in question is updating a multicast
> > register, the failures are only seen on specific CI machines, and the
> > failures only seem to happen on resets and such rather than on initial
> > driver load.  It seems likely that the culprit here is failure to steer
> > the multicast register readback on a SKU that has slice0 / subslice0
> > fused off.
> > 
> > This series makes a couple changes:
> >   * Workaround verification will explicitly steer MCR registers by
> >     calling read_subslice_reg rather than a regular read.
> >   * New multicast ranges are added for gen11 and gen12.  Sadly this
> >     information is still missing from the bspec (just like the updated
> >     forcewake tables).  The hardware guys have given us a spreadsheet
> >     with both the forcewake and the multicast information while they work
> >     on getting the spec properly updated, so that's where the new ranges
> >     come from.
> 
> I think there are multiple things here. To begin with, newly discovered
> ranges are of course a savior.
> 
> But I am not sure about the approach of using intel_read_subslice_reg in
> wa_verify. It is one suspicion that 0xfdc is lost on reset, but we do
> reprogram it afterwards don't we? And since it is the first register in the
> list it is supposed to be in place before the rest of verification runs, no?
> 
> A year or two I tried figuring this for Icelake and failed, but AFAIR (maybe
> my experiments can be found somewhere on trybot patchwork), I even tried
> both applying the affected ones via unicast (for each ss, or l3 where
> applicable) and also verifying a single register in all enabled ss. AFAIR
> there were still some issues there. Granted my memory could be leaky.. But I
> think this multiple write/verify could still be useful.
> 
> (Now that I think about it, I think that the problem area back when I
> experiementing with it was more suspend/resume.. hm..)
> 
> My main concern is that with current code we effectively have, after reset:
> 
> intel_gt_apply_workarounds:
>    program 0xfdc
>    program the rest of wa
> verify_wa
>    do reads using configured 0xfdc
> 
> So MCR should be correct. This series seems to be doing:
> 
> intel_gt_apply_workarounds:
>    program 0xfdc
> 	* store ss used for MCR configuration
>    program the rest of wa
> verify_wa
>    Do reads but reconfigure 0xfdc before every register in range,
>    but to the same value as in initial configuration.
> 
> Is this correct? Is the thinking then simply writing the same value to 0xfdc
> multiple times fixes things?

We stick the MCR steering at the beginning of the GT workaround list,
but the workaround that CI is complaining about is one of the RCS engine
workarounds, which is on a separate WA list.  At startup I think the RCS
workarounds are applied shortly after the GT workarounds, so the
steering is still in place, but on things like engine resets and such
that might not be the case.

A simpler approach might be to just stick the steering settings at the
front of the RCS workaround list, just as we do for the GT list.  That
was actually what I intended to try initially, but I started thinking
that we might want to handle registers invidually once we get the gen12
details about different steering registers for different ranges.  But I
guess we should probably hold off on that until we actually have all the
details documented and just keep things simple for now.

I'll send a v2 of this series later today that just sticks the steering
settings at the front of the RCS engine's WA list instead of trying to
individually steer each register.  That's probably a simpler and cleaner
approach for now.


Matt


> 
> Regards,
> 
> Tvrtko
> 
> P.S. Update, found the experiments, listing some of them:
> 
> https://patchwork.freedesktop.org/series/64183/
> https://patchwork.freedesktop.org/series/64013/
> 
> It reminded me that there were some unexplained issues with regards of where
> I used ffs or fls for finding the valid common MCR setting between L3 and
> SSEU. I think we use a different one than Windows but ours works better for
> our verification, empirically at least. Usual disclaimer about my leaky
> memory applies here.
> 
> > In addition to MCR and forcewake, there's supposed to be some more bspec
> > updates coming soon that deal with steering (i.e., different MCR ranges
> > should actually be using different registers to steer rather than just
> > the 0xFDC register we're familiar with); I don't have the full details
> > on that yet, so those updates will have to wait until we actually have
> > an updated spec.
> > 
> > References: https://gitlab.freedesktop.org/drm/intel/issues/1222
> > 
> > Matt Roper (4):
> >    drm/i915: Setup multicast register steering for all gen >= 10
> >    drm/i915: Steer multicast register readback in wa_verify
> >    drm/i915: Don't skip verification of MCR engine workarounds
> >    drm/i915: Add MCR ranges for gen11 and gen12
> > 
> >   drivers/gpu/drm/i915/gt/intel_engine.h        |   3 +
> >   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  17 +-
> >   drivers/gpu/drm/i915/gt/intel_workarounds.c   | 146 ++++++++++++------
> >   .../gpu/drm/i915/gt/intel_workarounds_types.h |   2 +
> >   4 files changed, 110 insertions(+), 58 deletions(-)
> > 

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