| Message ID | 1524088912-18161-1-git-send-email-yunwei.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 4/18/2018 3:01 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) > v9: > - Changed naming and label fixes. (Oscar) > - Store only the selector instead of whole MCR. (Oscar) > v10: > - Improved comments, naming and line breaknig. (Oscar) > v11: > - Moved the comment to most relavent block. (Oscar) > > Cc: Oscar Mateo <oscar.mateo@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Signed-off-by: Yunwei Zhang <yunwei.zhang@intel.com> > Reviewed-by: Oscar Mateo <oscar.mateo@intel.com> > --- > drivers/gpu/drm/i915/intel_device_info.c | 47 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_device_info.h | 3 ++ > drivers/gpu/drm/i915/intel_engine_cs.c | 10 +++---- > 3 files changed, 55 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index a32ba72..ea62d45 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -719,6 +719,51 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv) > return 0; > } > > + > +static void sanitize_mcr(struct intel_device_info *info) > +{ > + 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; > + > + /* > + * WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl,icl > + * Before any MMIO read into slice/subslice specific registers, MCR > + * packet control register needs to be programmed to point to any > + * enabled s/ss pair. Otherwise, incorrect values will be returned. > + * This means each subsequent MMIO read will be forwarded to an > + * specific s/ss combination, but this is OK since these registers > + * 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. > + */ > + if (INTEL_GEN(dev_priv) >= 10) > + mcr |= mcr_slice_subslice_select; > + > + I915_WRITE(GEN8_MCR_SELECTOR, mcr); > + > + info->default_mcr_s_ss_select = mcr_slice_subslice_select; Sorry: forget my r-b, this is broken. For GENs that do not need the WA, info->default_mcr_s_ss_select should be 0 > +} > + > /** > * intel_device_info_runtime_init - initialize runtime info > * @info: intel device info struct > @@ -851,6 +896,8 @@ 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); > > + sanitize_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..2feccee 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; > > + /* default selected slice/subslice in MCR packet control */ > + u32 default_mcr_s_ss_select; > + > 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..2b24277 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -831,11 +831,9 @@ 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((mcr & mcr_slice_subslice_mask) != > + dev_priv->info.default_mcr_s_ss_select); > mcr &= ~mcr_slice_subslice_mask; > mcr |= mcr_slice_subslice_select; > I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); > @@ -843,6 +841,8 @@ read_subslice_reg(struct drm_i915_private *dev_priv, int slice, > ret = I915_READ_FW(reg); > > mcr &= ~mcr_slice_subslice_mask; > + mcr |= dev_priv->info.default_mcr_s_ss_select; > + > I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); > > intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index a32ba72..ea62d45 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -719,6 +719,51 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv) return 0; } + +static void sanitize_mcr(struct intel_device_info *info) +{ + 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; + + /* + * WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl,icl + * Before any MMIO read into slice/subslice specific registers, MCR + * packet control register needs to be programmed to point to any + * enabled s/ss pair. Otherwise, incorrect values will be returned. + * This means each subsequent MMIO read will be forwarded to an + * specific s/ss combination, but this is OK since these registers + * 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. + */ + if (INTEL_GEN(dev_priv) >= 10) + mcr |= mcr_slice_subslice_select; + + I915_WRITE(GEN8_MCR_SELECTOR, mcr); + + info->default_mcr_s_ss_select = mcr_slice_subslice_select; +} + /** * intel_device_info_runtime_init - initialize runtime info * @info: intel device info struct @@ -851,6 +896,8 @@ 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); + sanitize_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..2feccee 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; + /* default selected slice/subslice in MCR packet control */ + u32 default_mcr_s_ss_select; + 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..2b24277 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -831,11 +831,9 @@ 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((mcr & mcr_slice_subslice_mask) != + dev_priv->info.default_mcr_s_ss_select); mcr &= ~mcr_slice_subslice_mask; mcr |= mcr_slice_subslice_select; I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); @@ -843,6 +841,8 @@ read_subslice_reg(struct drm_i915_private *dev_priv, int slice, ret = I915_READ_FW(reg); mcr &= ~mcr_slice_subslice_mask; + mcr |= dev_priv->info.default_mcr_s_ss_select; + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr); intel_uncore_forcewake_put__locked(dev_priv, fw_domains);