[Intel-gfx] [PATCH 1/8] drm/i915/mtl: Define MOCS and PAT tables for MTL

[Matt Roper]

On Mon, Apr 10, 2023 at 08:55:16PM -0700, Yang, Fei wrote:
...
>    >> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
> 
>    >> b/drivers/gpu/drm/i915/gt/intel_gtt.h
> 
>    >> index 69ce55f517f5..b632167eaf2e 100644
> 
>    >> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> 
>    >> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> 
>    >> @@ -88,9 +88,18 @@ typedef u64 gen8_pte_t;
> 
>    >>  #define BYT_PTE_SNOOPED_BY_CPU_CACHES             REG_BIT(2)
> 
>    >>  #define BYT_PTE_WRITEABLE                            REG_BIT(1)
> 
>    >> 
> 
>    >> +#define GEN12_PPGTT_PTE_PAT3    BIT_ULL(62)
> 
>    >>  #define GEN12_PPGTT_PTE_LM          BIT_ULL(11)
> 
>    >> +#define GEN12_PPGTT_PTE_PAT2    BIT_ULL(7)
> 
>    >> +#define GEN12_PPGTT_PTE_NC      BIT_ULL(5)
> 
>    >> +#define GEN12_PPGTT_PTE_PAT1    BIT_ULL(4)
> 
>    >> +#define GEN12_PPGTT_PTE_PAT0    BIT_ULL(3)
> 
>    > 
> 
>    > Which bspec page is this from?  The PPGTT descriptions in
> 
>    > the bspec are kind of hard to track down.
> 
>     
> 
>    [9]https://gfxspecs.intel.com/Predator/Home/Index/53521

The bspec tagging is a bit bizarre in this area, but I don't believe
this page is intended to apply to MTL.  Note that this page is inside a
section specifically listed as "57b VA Support" --- i.e., this general
section is for platforms like PVC rather than MTL.  MTL only has 48b
virtual address space (bspec 55416), so I think one of the pages in the
48b sections is what we should be referencing instead.

If they screwed up and put MTL-specific details only on a PVC-specific
page of the bspec, we should probably file a bspec issue about that to
get it fixed.

> 
>    PAT_Index[2:0] = {PAT, PCD, PWT} = BIT(7, 4, 3)
> 
>    PAT_Index[3] = BIT(62)
> 
>    PAT_Index[4] = BIT(61)
> 
>     
> 
>    > But if these only apply to MTL, why are they labelled as "GEN12?"
> 
>     
> 
>    These apply to GEN12plus.

That's not what your patch is doing; you're using them in a function
that only gets called on MTL.  So the question is whether these
definitions truly applied to older platforms like TGL/RKL/ADL/etc too
(and we need to go back and fix that code), or whether they're
Xe_LPG-specific, in which case the "GEN12_" prefix is incorrect.

Also, handling the MTL-specific PTE encoding later in the series, after
the transition from cache_level to PAT index, might be best since then
you can just implement it correctly at the time the code is introduced;
no need to add the cache_level implementation first (which can't even
use all the bits) just to come back a few patches later and replace it
all with PAT code.

> 
>     
> 
>    >> 
> 
>    >> -#define GEN12_GGTT_PTE_LM           BIT_ULL(1)
> 
>    >> +#define GEN12_GGTT_PTE_LM                         BIT_ULL(1)
> 
>    >> +#define MTL_GGTT_PTE_PAT0                          BIT_ULL(52)
> 
>    >> +#define MTL_GGTT_PTE_PAT1                          BIT_ULL(53)
> 
>    >> +#define GEN12_GGTT_PTE_ADDR_MASK       GENMASK_ULL(45, 12)
> 
>    >> +#define MTL_GGTT_PTE_PAT_MASK                              
>    GENMASK_ULL(53, 52)
> 
>    >> 
> 
>    >>  #define GEN12_PDE_64K BIT(6)
> 
>    >>  #define GEN12_PTE_PS64 BIT(8)
> 
>    >> @@ -147,6 +156,15 @@ typedef u64 gen8_pte_t;  #define GEN8_PDE_IPS_64K
> 
>    >> BIT(11)
> 
>    >>  #define GEN8_PDE_PS_2M   BIT(7)
> 
>    >> 
> 
>    >> +#define MTL_PPAT_L4_CACHE_POLICY_MASK             REG_GENMASK(3, 2)
> 
>    >> +#define MTL_PAT_INDEX_COH_MODE_MASK              REG_GENMASK(1, 0)
> 
>    >> +#define MTL_PPAT_L4_3_UC              
>    REG_FIELD_PREP(MTL_PPAT_L4_CACHE_POLICY_MASK, 3)
> 
>    >> +#define MTL_PPAT_L4_1_WT              
>    REG_FIELD_PREP(MTL_PPAT_L4_CACHE_POLICY_MASK, 1)
> 
>    >> +#define MTL_PPAT_L4_0_WB              
>    REG_FIELD_PREP(MTL_PPAT_L4_CACHE_POLICY_MASK, 0)
> 
>    >> +#define MTL_3_COH_2W     REG_FIELD_PREP(MTL_PAT_INDEX_COH_MODE_MASK,
>    3)
> 
>    >> +#define MTL_2_COH_1W     REG_FIELD_PREP(MTL_PAT_INDEX_COH_MODE_MASK,
>    2)
> 
>    >> +#define MTL_0_COH_NON   REG_FIELD_PREP(MTL_PAT_INDEX_COH_MODE_MASK, 0)
> 
>    > 
> 
>    >The values for these definitions don't seem to be aligned.
> 
>     
> 
>    These are aligned with
>    [10]https://gfxspecs.intel.com/Predator/Home/Index/45101

I mean spacing aligned.  If your tabstops are set to 8, then the values
don't line up visually.


Matt

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation