On 3/27/2018 3:27 PM, Chris Wilson wrote:
Quoting Yunwei Zhang (2018-03-27 23:14:16)
WaProgramMgsrForCorrectSliceSpecificMmioReads dictate that before any MMIO
read into Slice/Subslice specific registers, MCR packet control
register(0xFDC) needs to be programmed to point to any enabled
slice/subslice pair. Otherwise, incorrect value will be returned.

However, that means each subsequent MMIO read will be forwarded to a
specific slice/subslice combination as read is unicast. This is OK since
slice/subslice specific register values are consistent in almost all cases
across slice/subslice. There are rare occasions such as INSTDONE that this
value will be dependent on slice/subslice combo, in such cases, we need to
program 0xFDC and recover this after. This is already covered by
read_subslice_reg.

Also, 0xFDC will lose its information after TDR/engine reset/power state
change.

References: HSD#1405586840, BSID#0575

v2:
  - use fls() instead of find_last_bit() (Chris)
  - added INTEL_SSEU to extract sseu from device info. (Chris)
v3:
  - rebase on latest tip
v5:
  - Added references (Mika)
  - Change the ordered of passing arguments and etc. (Ursulin)

Cc: Oscar Mateo <oscar.ma...@intel.com>
Cc: Michel Thierry <michel.thie...@intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
Signed-off-by: Yunwei Zhang <yunwei.zh...@intel.com>
---
  drivers/gpu/drm/i915/intel_engine_cs.c | 39 ++++++++++++++++++++++++++++++++--
  1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index de09fa4..4c78d1e 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -796,6 +796,27 @@ const char *i915_cache_level_str(struct drm_i915_private 
*i915, int type)
         }
  }
+static u32 calculate_mcr(struct drm_i915_private *dev_priv, u32 mcr)
+{
+       const struct sseu_dev_info *sseu = &(INTEL_INFO(dev_priv)->sseu);
+       u32 slice = fls(sseu->slice_mask);
+       u32 subslice = fls(sseu->subslice_mask[slice]);
+
+       mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
+       mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
+
+       return mcr;
+}
+
+static void wa_init_mcr(struct drm_i915_private *dev_priv)
+{
+       u32 mcr;
+
+       mcr = I915_READ(GEN8_MCR_SELECTOR);
+       mcr = calculate_mcr(dev_priv, mcr);
+       I915_WRITE(GEN8_MCR_SELECTOR, mcr);
+}
+
  static inline uint32_t
  read_subslice_reg(struct drm_i915_private *dev_priv, int slice,
                   int subslice, i915_reg_t reg)
@@ -828,18 +849,29 @@ read_subslice_reg(struct drm_i915_private *dev_priv, int 
slice,
         intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
mcr = I915_READ_FW(GEN8_MCR_SELECTOR);
+
         /*
          * The HW expects the slice and sublice selectors to be reset to 0
          * after reading out the registers.
          */
-       WARN_ON_ONCE(mcr & mcr_slice_subslice_mask);
+       WARN_ON_ONCE(INTEL_GEN(dev_priv) < 10 &&
+                    (mcr & mcr_slice_subslice_mask));
         mcr &= ~mcr_slice_subslice_mask;
         mcr |= mcr_slice_subslice_select;
         I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
ret = I915_READ_FW(reg); - mcr &= ~mcr_slice_subslice_mask;
+       /*
+        * WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl
+        * expects mcr to be programed to a enabled slice/subslice pair
+        * before any MMIO read into slice/subslice register
+        */
So the read was above, where we did set the subslice_select
appropriately. Here we are resetting back to 0 *after* the read, as the
comment before indicates.

So what are you trying to accomplish with this patch? Other than leaving
the code in conflict with itself.
-Chris
Hi Chris,

The comment mentioned 0xFDC needs to be reset to 0 was before this WA was introduced, in later HW, this WA requires 0xFDC to be programmed to a enabled slice/subslice.

What this patch does it to initialize 0xFDC once at the initialization (also it will be called after engine reset/TDR/coming out of c6) and make sure every time it is changed, it will be reprogrammed to a enabled slice/subslice so that a MMIO read will get the correct value. read_subslice_reg changes the 0xFDC value and if it is set to 0, it will cause MMIO read to return invalid value for s/ss specific registers.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to