On 4/17/2018 2:05 PM, Yunwei Zhang wrote:
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)
v7:
- Rebased.
v8:
- Reviewed by Oscar
- Store default MCR value instead of calculate on the run. (Oscar)
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
Signed-off-by: Yunwei Zhang <yunwei.zhang@xxxxxxxxx>
Reviewed-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_device_info.c | 33 ++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_device_info.h | 3 +++
drivers/gpu/drm/i915/intel_engine_cs.c | 14 +++++++++++---
3 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index a32ba72..2243a23 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -719,6 +719,36 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
return 0;
}
+static void wa_init_mcr(struct intel_device_info *info)
mcr_sanitize?
+{
+ struct drm_i915_private *dev_priv =
+ container_of(info, struct drm_i915_private, info);
+ u32 mcr;
+ u32 mcr_slice_subslice_mask;
+ u32 mcr_slice_subslice_select;
+ u32 slice = fls(info->sseu.slice_mask);
+ u32 subslice = fls(info->sseu.subslice_mask[slice]);
+
+ if (INTEL_GEN(dev_priv) >= 11) {
+ mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK |
+ GEN11_MCR_SUBSLICE_MASK;
+ mcr_slice_subslice_select = GEN11_MCR_SLICE(slice) |
+ GEN11_MCR_SUBSLICE(subslice);
+ } else {
+ mcr_slice_subslice_mask = GEN8_MCR_SLICE_MASK |
+ GEN8_MCR_SUBSLICE_MASK;
+ mcr_slice_subslice_select = GEN8_MCR_SLICE(slice) |
+ GEN8_MCR_SUBSLICE(subslice);
+ }
+
+ mcr = I915_READ(GEN8_MCR_SELECTOR);
+ mcr &= ~mcr_slice_subslice_mask;
Until here you are not applying any WA, only sanitizing what the MCR
contains. The real WA is in the two following lines. That's where the
WaProgramMgsrForCorrectSliceSpecificMmioReads label should be (and maybe
a small comment noting that we are selecting a kind of random
slice/subslice combination to make sure MMIO reads in a certaing range
are valid).
+ if (INTEL_GEN(dev_priv) >= 10)
+ mcr |= mcr_slice_subslice_select;
+
+ info->mcr = mcr;
And now you also want to write the HW register back, otherwise you are
not applying the WA!
+}
+
/**
* intel_device_info_runtime_init - initialize runtime info
* @info: intel device info struct
@@ -851,6 +881,9 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
else if (INTEL_INFO(dev_priv)->gen >= 11)
gen11_sseu_info_init(dev_priv);
+ /* WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl,icl */
+ wa_init_mcr(info);
+
/* Initialize command stream timestamp frequency */
info->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
}
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 933e316..5449a15 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -176,6 +176,9 @@ struct intel_device_info {
/* Slice/subslice/EU info */
struct sseu_dev_info sseu;
+ /* MCR packet control */
+ u32 mcr;
Use a better name for this, like default_mcr. I'll explain why in a
second...
+
u32 cs_timestamp_frequency_khz;
struct color_luts {
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 1a83707..08798f2 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -831,18 +831,26 @@ 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.
+ * before GEN10 or to a enabled s/ss post GEN10 after reading out the
+ * registers.
*/
- WARN_ON_ONCE(mcr & mcr_slice_subslice_mask);
+ WARN_ON_ONCE(mcr != dev_priv->info.mcr);
... if you call it default_mcr, you can skip the comment completely.
It's very clear you are just making sure the MCR has the expected value.
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,icl
No need for this label here, because this is not were the WA should be
applied!! Hopefully, you already applied it before now.
+ * expects mcr to be programed to a enabled slice/subslice pair
+ * before any MMIO read into slice/subslice register
+ */
+ mcr = dev_priv->info.mcr;
Same here: with a better naming, it's very clear you are just setting
the MCR to the default value, so there is no need for any extra comment
(and remove the following blank line, to show this goes together with
the I915_WRITE_FW).
+
I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
