[PATCH 5/6] drm/i915: Improve MCR selection in wa_init_mcr

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Improve selection logic for WaProgramMgsrForL3BankSpecificMmioReads and
WaProgramMgsrForCorrectSliceSpecificMmioReads so instead of just asserting
it tries to find a common index between enabled subslices and L3 banks.

Warning message is logged if no common index can be found but that is not
expected in production parts.

We also switch fls to ffs since it is more readable to select first
enabled entities instead of last.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Michał Winiarski <michal.winiarski at intel.com>
Cc: Stuart Summers <stuart.summers at intel.com>
---
 drivers/gpu/drm/i915/gt/intel_sseu.c        |  4 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 88 +++++++++++----------
 2 files changed, 47 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c
index c12cc476391f..5ee462bd3d8d 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.c
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
@@ -161,12 +161,12 @@ u32 intel_sseu_make_rpcs(struct drm_i915_private *i915,
 u32 intel_calculate_mcr_s_ss_select(struct drm_i915_private *i915)
 {
 	const struct sseu_dev_info *sseu = &RUNTIME_INFO(i915)->sseu;
-	unsigned int slice = fls(sseu->slice_mask) - 1;
+	unsigned int slice = ffs(sseu->slice_mask) - 1;
 	unsigned int subslice;
 	u32 mcr_s_ss_select;
 
 	GEM_BUG_ON(slice >= ARRAY_SIZE(sseu->subslice_mask));
-	subslice = fls(sseu->subslice_mask[slice]);
+	subslice = ffs(sseu->subslice_mask[slice]);
 	GEM_BUG_ON(!subslice);
 	subslice--;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index c9f4fcff4452..cf0d61557422 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -748,9 +748,18 @@ static void
 wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
 	const struct sseu_dev_info *sseu = &RUNTIME_INFO(i915)->sseu;
-	u32 mcr_mask, mcr;
+	unsigned int slice, subslice;
+	u32 l3_en, mcr, mcr_mask;
+
+	GEM_BUG_ON(INTEL_GEN(i915) < 10);
 
 	/*
+	 * WaProgramMgsrForL3BankSpecificMmioReads: cnl,icl
+	 * L3Banks could be fused off in single slice scenario. If that is
+	 * the case, we might need to program MCR select to a valid L3Bank
+	 * by default, to make sure we correctly read certain registers
+	 * later on (in the range 0xB100 - 0xB3FF).
+	 *
 	 * WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl,icl
 	 * Before any MMIO read into slice/subslice specific registers, MCR
 	 * packet control register needs to be programmed to point to any
@@ -760,55 +769,48 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
 	 * are consistent across s/ss in almost all cases. In the rare
 	 * occasions, such as INSTDONE, where this value is dependent
 	 * on s/ss combo, the read should be done with read_subslice_reg.
+	 *
+	 * Since GEN8_MCR_SELECTOR contains dual-purpose bits which select both
+	 * to which subslice, or to which L3 bank, the respective mmio reads
+	 * will go, we have to find a common index which works for both
+	 * accesses.
+	 *
+	 * Case where we cannot find a common index fortunately should not
+	 * happen in production hardware, so we only emit a warning instead of
+	 * implementing something more complex that requires checking the range
+	 * of every MMIO read.
 	 */
-	mcr = intel_calculate_mcr_s_ss_select(i915);
-
-	if (INTEL_GEN(i915) >= 11)
-		mcr_mask = GEN11_MCR_SLICE_MASK | GEN11_MCR_SUBSLICE_MASK;
-	else
-		mcr_mask = GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK;
-
-	wa_write_masked_or(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr);
 
-	/*
-	 * WaProgramMgsrForL3BankSpecificMmioReads: cnl,icl
-	 * L3Banks could be fused off in single slice scenario. If that is
-	 * the case, we might need to program MCR select to a valid L3Bank
-	 * by default, to make sure we correctly read certain registers
-	 * later on (in the range 0xB100 - 0xB3FF).
-	 * This might be incompatible with
-	 * WaProgramMgsrForCorrectSliceSpecificMmioReads.
-	 * Fortunately, this should not happen in production hardware, so
-	 * we only assert that this is the case (instead of implementing
-	 * something more complex that requires checking the range of every
-	 * MMIO read).
-	 */
 	if (INTEL_GEN(i915) >= 10 && is_power_of_2(sseu->slice_mask)) {
-		/*
-		 * GEN8_MCR_SELECTOR contains dual-purpose bits which select
-		 * both to which subslice, or to which L3 bank, the respective
-		 * mmio reads will go.
-		 * Since we have selected one enabled subslice in
-		 * WaProgramMgsrForCorrectSliceSpecificMmioReads, we now
-		 * need to check if the L3 bank of the equal "instance" is also
-		 * enabled.
-		 * If that is not the case we could try to find a number which
-		 * works for both, or going even further, implement a dynamic
-		 * scheme where we switch at before every affected mmio read.
-		 * Fortunately neither seems to be needed at the moment for
-		 * current parts and current driver behaviour.
-		 */
-		unsigned int mcr_ss =
-			BIT((mcr >> GEN11_MCR_SUBSLICE_SHIFT) &
-			    __GEN11_MCR_SUBSLICE_MASK);
-		unsigned int l3_fuse =
+		u32 l3_fuse =
 			intel_uncore_read(&i915->uncore, GEN10_MIRROR_FUSE3) &
 			GEN10_L3BANK_MASK;
-		unsigned int l3_en = ~(l3_fuse << GEN10_L3BANK_PAIR_COUNT |
-				       l3_fuse);
 
-		WARN_ON(!(mcr_ss & l3_en));
+		l3_en = ~(l3_fuse << GEN10_L3BANK_PAIR_COUNT | l3_fuse);
+	} else {
+		l3_en = ~0;
 	}
+
+	slice = ffs(sseu->slice_mask) - 1;
+	GEM_BUG_ON(slice >= ARRAY_SIZE(sseu->subslice_mask));
+	subslice = ffs(l3_en & sseu->subslice_mask[slice]);
+	if (!subslice) {
+		DRM_WARN("No common index found between subslice mask %x and L3 bank mask %x!\n",
+			 sseu->subslice_mask[slice], l3_en);
+		subslice = ffs(l3_en);
+		WARN_ON(!subslice);
+	}
+	subslice--;
+
+	if (INTEL_GEN(i915) >= 11) {
+		mcr = GEN11_MCR_SLICE(slice) | GEN11_MCR_SUBSLICE(subslice);
+		mcr_mask = GEN11_MCR_SLICE_MASK | GEN11_MCR_SUBSLICE_MASK;
+	} else {
+		mcr = GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
+		mcr_mask = GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK;
+	}
+
+	wa_write_masked_or(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr);
 }
 
 static void
-- 
2.20.1