On Mon, Apr 23, 2018 at 09:12:46AM -0700, 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.
> v9:
>  - Fixed label location. (Oscar)
> v10:
>  - Improved comments and replaced magical number. (Oscar)
> 
> Cc: Oscar Mateo <[email protected]>
> Cc: Michel Thierry <[email protected]>
> Cc: Joonas Lahtinen <[email protected]>
> Cc: Chris Wilson <[email protected]>
> Cc: Mika Kuoppala <[email protected]>
> Cc: Tvrtko Ursulin <[email protected]>
> Signed-off-by: Yunwei Zhang <[email protected]>
> Reviewed-by: Oscar Mateo <[email protected]>

I confess that I got lost on this thread, so please
accept my apologies in advance if I'm missing something here.

But I don't know anymore:

- if this series has 2 or 3 patches.
- which of the patches rv-b by Oscar are still valid
- if they are passing cleaning on CI.

So, my suggestion is to start a new series from scratch.
(resend all without any in-reply-to)

But please double check with Oscar if his rv-b should stay
on the new series.

Thanks,
Rodrigo.


> ---
>  drivers/gpu/drm/i915/i915_reg.h          |  4 ++++
>  drivers/gpu/drm/i915/intel_device_info.c | 34 
> ++++++++++++++++++++++++++++++++
>  2 files changed, 38 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 d917c9b..44ca90a 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -729,6 +729,40 @@ static void sanitize_mcr(struct intel_device_info *info)
>       u32 slice = fls(info->sseu.slice_mask);
>       u32 subslice = fls(info->sseu.subslice_mask[slice]);
>  
> +     /*
> +      * WaProgramMgsrForL3BankSpecificMmioReads: cnl,icl
> +      * L3Banks could be fused off in single slice scenario. If that is
> +      * the case, we might need to program MCR select to a valid L3Bank
> +      * by default, to make sure we correctly read certain registers
> +      * later on (in the range 0xB100 - 0xB3FF).
> +      * This might be incompatible with
> +      * WaProgramMgsrForCorrectSliceSpecificMmioReads.
> +      * Fortunately, this should not happen in production hardware, so
> +      * we only assert that this is the case (instead of implementing
> +      * something more complex that requires checking the range of every
> +      * MMIO read).
> +      */
> +     if (INTEL_GEN(dev_priv) >= 10 &&
> +         is_power_of_2(info->sseu.slice_mask)) {
> +             /*
> +              * 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 >>
> +                                GEN10_L3BANK_PAIR_COUNT) &
> +                                GEN10_L3BANK_MASK;
> +             u8 disabled_mask = fuse3 & GEN10_L3BANK_MASK;
> +
> +             /*
> +              * Production silicon should have matched L3Bank and
> +              * subslice enabled
> +              */
> +             WARN_ON((enabled_mask & disabled_mask) != enabled_mask);
> +     }
> +
>       if (INTEL_GEN(dev_priv) >= 11) {
>               mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK |
>                                         GEN11_MCR_SUBSLICE_MASK;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to