[Intel-gfx] [PATCH v6 1/2] drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads

[Zhang, Yunwei]

Hi All,

I see the latest BAT test failed but the only thing I changed in this 
new patchset is comment. It should be false alarm, not sure if this is 
halting the further review. Please see if the code needs more change.

Thanks,

Yunwei


On 3/29/2018 8:44 AM, 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)
> v6:
>   - Updated the comment that conflict with the patch. (Chris)
>
> Cc: Oscar Mateo <oscar.mateo at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> Signed-off-by: Yunwei Zhang <yunwei.zhang at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_engine_cs.c | 42 +++++++++++++++++++++++++++++++---
>   1 file changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 12486d8..4c50bee 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,30 @@ 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(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
> +	 */
> +	if (INTEL_GEN(dev_priv) < 10)
> +		mcr &= ~mcr_slice_subslice_mask;
> +	else
> +		mcr = calculate_mcr(dev_priv, mcr);
> +
>   	I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
>   
>   	intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
> @@ -1307,6 +1340,9 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine)
>   	struct drm_i915_private *dev_priv = engine->i915;
>   	int ret;
>   
> +	/* WaProgramMgsrForCorrectSliceSpecificMmioReads: cnl */
> +	wa_init_mcr(dev_priv);
> +
>   	/* WaDisableI2mCycleOnWRPort:cnl (pre-prod) */
>   	if (IS_CNL_REVID(dev_priv, CNL_REVID_B0, CNL_REVID_B0))
>   		I915_WRITE(GAMT_CHKN_BIT_REG,