[Intel-gfx] [PATCH v8 2/2] drm/i915: Implement WaProgramMgsrForL3BankSpecificMmioReads

[Oscar Mateo]

On 4/17/2018 2:05 PM, Yunwei Zhang wrote:
> L3Bank could be fused off in hardware for debug purpose, and it
> is possible that subslice is enabled while its corresponding L3Bank pairs
> are disabled. In such case, if MCR packet control register(0xFDC) is
> programed to point to a disabled bank pair, a MMIO read into L3Bank range
> will return 0 instead of correct values.
>
> However, this is not going to be the case in any production silicon.
> Therefore, we only check at initialization and issue a warning should
> this really happen.
>
> References: HSDES#1405586840
>
> v2:
>   - use fls instead of find_last_bit (Chris)
>   - use is_power_of_2() instead of counting bit set (Chris)
> v3:
>   - rebase on latest tip
> v5:
>   - Added references (Mika)
>   - Move local variable into scope where they are used (Ursulin)
>   - use a new local variable to reduce long line of code (Ursulin)
> v6:
>   - Some coding style and use more local variables for clearer
>     logic (Ursulin)
> v7:
>   - Rebased.
> v8:
>   - Reviewed by Oscar.
>
> 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>
> Reviewed-by: Oscar Mateo <oscar.mateo at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h          |  4 ++++
>   drivers/gpu/drm/i915/intel_device_info.c | 23 +++++++++++++++++++++++
>   2 files changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fb10602..6c9c01b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2709,6 +2709,10 @@ enum i915_power_well_id {
>   #define   GEN10_F2_SS_DIS_SHIFT		18
>   #define   GEN10_F2_SS_DIS_MASK		(0xf << GEN10_F2_SS_DIS_SHIFT)
>   
> +#define	GEN10_MIRROR_FUSE3		_MMIO(0x9118)
> +#define GEN10_L3BANK_PAIR_COUNT     4
> +#define GEN10_L3BANK_MASK   0x0F
> +
>   #define GEN8_EU_DISABLE0		_MMIO(0x9134)
>   #define   GEN8_EU_DIS0_S0_MASK		0xffffff
>   #define   GEN8_EU_DIS0_S1_SHIFT		24
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 2243a23..5a013fa 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -729,6 +729,29 @@ static void wa_init_mcr(struct intel_device_info *info)
>   	u32 slice = fls(info->sseu.slice_mask);
>   	u32 subslice = fls(info->sseu.subslice_mask[slice]);
>   
> +	/*
> +	 * L3Banks could be fused off in single slice scenario, however, if
> +	 * more than one slice is enabled, this should not happen.
> +	 */
> +	if (is_power_of_2(info->sseu.slice_mask)) {
> +		/*
> +		 * WaProgramMgsrForL3BankSpecificMmioReads:
> +		 * read FUSE3 for enabled L3 Bank IDs, if L3 Bank matches
> +		 * enabled subslice, no need to redirect MCR packet
> +		 */
> +		u32 fuse3 = I915_READ(GEN10_MIRROR_FUSE3);
> +		u8 ss_mask = info->sseu.subslice_mask[slice];
> +
> +		u8 enabled_mask = (ss_mask | ss_mask >> 4) & 0xf;
> +		u8 disabled_mask = fuse3 & 0xf;
> +
> +		/*
> +		 * Production silicon should have matched L3Bank and
> +		 * subslice enabled
> +		 */
> +		WARN_ON((enabled_mask & disabled_mask) != enabled_mask);
> +	}
> +

You have lost the WaProgramMgsrForL3BankSpecificMmioReads label here. 
Also, this check only makes sense for platforms where the WA is needed 
(Gen10+)

>   	if (INTEL_GEN(dev_priv) >= 11) {
>   		mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK |
>   					  GEN11_MCR_SUBSLICE_MASK;