<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <div class="WordSection1"> >Subject: Re: [Intel-gfx] [PATCH 1/8] drm/i915/mtl: Define MOCS and PAT tables for MTL<o:p></o:p> ><o:p> </o:p> > On Fri, Apr 07, 2023 at 12:12:29AM -0700, <a href="mailto:fei.yang@intel.com"> fei.yang@intel.com</a> wrote:<o:p></o:p> >> From: Fei Yang <a href="mailto:fei.yang@intel.com">fei.yang@intel.com</a><o:p></o:p> >> <o:p></o:p> >> On MTL, GT can no longer allocate on LLC - only the CPU can.<o:p></o:p> >> This, along with addition of support for ADM/L4 cache calls a MOCS/PAT <o:p></o:p> >> table update. Also defines PTE encode functions for MTL as it has <o:p></o:p> >> different PAT index definition than previous platforms.<o:p></o:p> ><o:p> </o:p> > It might be best to keep the PTE encoding as a separate patch from the<o:p></o:p> > MOCS/PAT tables. It's a different enough topic that it probably deserves<o:p></o:p> > a patch of its own.<o:p></o:p> <o:p> </o:p> Will update in the next revision.<o:p></o:p> <o:p> </o:p> >> <o:p></o:p> >> BSpec: 44509, 45101, 44235<o:p></o:p> >> <o:p></o:p> >> Cc: Matt Roper <a href="mailto:matthew.d.roper@intel.com"> matthew.d.roper@intel.com</a><o:p></o:p> >> Cc: Lucas De Marchi <a href="mailto:lucas.demarchi@intel.com"> lucas.demarchi@intel.com</a><o:p></o:p> >> Signed-off-by: Madhumitha Tolakanahalli Pradeep <o:p></o:p> >> <a href="mailto:madhumitha.tolakanahalli.pradeep@intel.com"> madhumitha.tolakanahalli.pradeep@intel.com</a><o:p></o:p> >> Signed-off-by: Aravind Iddamsetty <a href="mailto:aravind.iddamsetty@intel.com"> aravind.iddamsetty@intel.com</a><o:p></o:p> >> Signed-off-by: Fei Yang <a href="mailto:fei.yang@intel.com"> fei.yang@intel.com</a><o:p></o:p> >> ---<o:p></o:p> >> drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 28 +++++++++<o:p></o:p> >> drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 3 +<o:p></o:p> >> drivers/gpu/drm/i915/gt/intel_ggtt.c | 27 +++++++++<o:p></o:p> >> drivers/gpu/drm/i915/gt/intel_gtt.c | 23 +++++++-<o:p></o:p> >> drivers/gpu/drm/i915/gt/intel_gtt.h | 20 ++++++-<o:p></o:p> >> drivers/gpu/drm/i915/gt/intel_mocs.c | 76 +++++++++++++++++++++++--<o:p></o:p> >> drivers/gpu/drm/i915/gt/selftest_mocs.c | 2 +-<o:p></o:p> >> drivers/gpu/drm/i915/i915_pci.c | 1 +<o:p></o:p> >> 8 files changed, 173 insertions(+), 7 deletions(-)<o:p></o:p> >> <o:p></o:p> >> diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c <o:p></o:p> >> b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c<o:p></o:p> >> index 4daaa6f55668..df4073d32114 100644<o:p></o:p> >> --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c<o:p></o:p> >> +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c<o:p></o:p> >> @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr,<o:p></o:p> >> return pte;<o:p></o:p> >> }<o:p></o:p> >> <o:p></o:p> >> +static u64 mtl_pte_encode(dma_addr_t addr,<o:p></o:p> >> + enum i915_cache_level level,<o:p></o:p> >> + u32 flags)<o:p></o:p> >> +{<o:p></o:p> >> + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW;<o:p></o:p> >> +<o:p></o:p> >> + if (unlikely(flags & PTE_READ_ONLY))<o:p></o:p> >> + pte &= ~GEN8_PAGE_RW;<o:p></o:p> >> +<o:p></o:p> >> + if (flags & PTE_LM)<o:p></o:p> >> + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC;<o:p></o:p> >> +<o:p></o:p> >> + switch (level) {<o:p></o:p> >> + case I915_CACHE_NONE:<o:p></o:p> >> + pte |= GEN12_PPGTT_PTE_PAT1;<o:p></o:p> >> + break;<o:p></o:p> >> + case I915_CACHE_LLC:<o:p></o:p> >> + case I915_CACHE_L3_LLC:<o:p></o:p> >> + pte |= GEN12_PPGTT_PTE_PAT0 | GEN12_PPGTT_PTE_PAT1;<o:p></o:p> >> + break;<o:p></o:p> >> + case I915_CACHE_WT:<o:p></o:p> >> + pte |= GEN12_PPGTT_PTE_PAT0;<o:p></o:p> >> + break;<o:p></o:p> >> + }<o:p></o:p> >> +<o:p></o:p> >> + return pte;<o:p></o:p> >> +}<o:p></o:p> >> +<o:p></o:p> >> static void gen8_ppgtt_notify_vgt(struct i915_ppgtt *ppgtt, bool <o:p></o:p> >> create) {<o:p></o:p> >> struct drm_i915_private *i915 = ppgtt->vm.i915; diff --git <o:p></o:p> >> a/drivers/gpu/drm/i915/gt/gen8_ppgtt.h <o:p></o:p> >> b/drivers/gpu/drm/i915/gt/gen8_ppgtt.h<o:p></o:p> >> index f541d19264b4..6b8ce7f4d25a 100644<o:p></o:p> >> --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.h<o:p></o:p> >> +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.h<o:p></o:p> >> @@ -18,5 +18,8 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt <o:p></o:p> >> *gt,<o:p></o:p> >> u64 gen8_ggtt_pte_encode(dma_addr_t addr,<o:p></o:p> >> enum i915_cache_level level,<o:p></o:p> >> u32 flags);<o:p></o:p> >> +u64 mtl_ggtt_pte_encode(dma_addr_t addr,<o:p></o:p> >> + unsigned int pat_index,<o:p></o:p> >> + u32 flags);<o:p></o:p> >> <o:p></o:p> >> #endif<o:p></o:p> >> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c <o:p></o:p> >> b/drivers/gpu/drm/i915/gt/intel_ggtt.c<o:p></o:p> >> index 3c7f1ed92f5b..4a16bfcde1de 100644<o:p></o:p> >> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c<o:p></o:p> >> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c<o:p></o:p> >> @@ -220,6 +220,33 @@ static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)<o:p></o:p> >> }<o:p></o:p> >> }<o:p></o:p> >> <o:p></o:p> >> +u64 mtl_ggtt_pte_encode(dma_addr_t addr,<o:p></o:p> >> + enum i915_cache_level level,<o:p></o:p> >> + u32 flags)<o:p></o:p> >> +{<o:p></o:p> >> + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT;<o:p></o:p> >> +<o:p></o:p> >> + GEM_BUG_ON(addr & ~GEN12_GGTT_PTE_ADDR_MASK);<o:p></o:p> >> +<o:p></o:p> >> + if (flags & PTE_LM)<o:p></o:p> >> + pte |= GEN12_GGTT_PTE_LM;<o:p></o:p> >> +<o:p></o:p> >> + switch (level) {<o:p></o:p> >> + case I915_CACHE_NONE:<o:p></o:p> >> + pte |= MTL_GGTT_PTE_PAT1;<o:p></o:p> >> + break;<o:p></o:p> >> + case I915_CACHE_LLC:<o:p></o:p> >> + case I915_CACHE_L3_LLC:<o:p></o:p> >> + pte |= MTL_GGTT_PTE_PAT0 | MTL_GGTT_PTE_PAT1;<o:p></o:p> >> + break;<o:p></o:p> >> + case I915_CACHE_WT:<o:p></o:p> >> + pte |= MTL_GGTT_PTE_PAT0;<o:p></o:p> >> + break;<o:p></o:p> >> + }<o:p></o:p> >> +<o:p></o:p> >> + return pte;<o:p></o:p> >> +}<o:p></o:p> >> +<o:p></o:p> >> u64 gen8_ggtt_pte_encode(dma_addr_t addr,<o:p></o:p> >> enum i915_cache_level level,<o:p></o:p> >> u32 flags)<o:p></o:p> >> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c <o:p></o:p> >> b/drivers/gpu/drm/i915/gt/intel_gtt.c<o:p></o:p> >> index 4f436ba7a3c8..1e1b34e22cf5 100644<o:p></o:p> >> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c<o:p></o:p> >> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c<o:p></o:p> >> @@ -468,6 +468,25 @@ void gtt_write_workarounds(struct intel_gt *gt)<o:p></o:p> >> }<o:p></o:p> >> }<o:p></o:p> >> <o:p></o:p> >> +static void mtl_setup_private_ppat(struct intel_uncore *uncore) {<o:p></o:p> >> + intel_uncore_write(uncore, GEN12_PAT_INDEX(0),<o:p></o:p> >> + MTL_PPAT_L4_0_WB);<o:p></o:p> >> + intel_uncore_write(uncore, GEN12_PAT_INDEX(1),<o:p></o:p> >> + MTL_PPAT_L4_1_WT);<o:p></o:p> >> + intel_uncore_write(uncore, GEN12_PAT_INDEX(2),<o:p></o:p> >> + MTL_PPAT_L4_3_UC);<o:p></o:p> >> + intel_uncore_write(uncore, GEN12_PAT_INDEX(3),<o:p></o:p> >> + MTL_PPAT_L4_0_WB | MTL_2_COH_1W);<o:p></o:p> >> + intel_uncore_write(uncore, GEN12_PAT_INDEX(4),<o:p></o:p> >> + MTL_PPAT_L4_0_WB | MTL_3_COH_2W);<o:p></o:p> ><o:p> </o:p> > These registers are multicast registers on the primary GT. We need<o:p></o:p> > to use XEHP_PAT_INDEX and the MCR-aware functions on the primary GT.<o:p></o:p> <o:p> </o:p> The multicast change has been merged to a different branch and not<o:p></o:p> validated extensively on MTL so far. Will post to mailing list after<o:p></o:p> proper validation.<o:p></o:p> <o:p> </o:p> > Although we're not programming the higher PAT entries on MTL (yet),<o:p></o:p> > there's also a jump in the MMIO offsets that we should incorporate<o:p></o:p> > into the PAT_INDEX macro to future-proof the code.<o:p></o:p> ><o:p> </o:p> >> +<o:p></o:p> >> + /*<o:p></o:p> >> + * Remaining PAT entries are left at the hardware-default<o:p></o:p> >> + * fully-cached setting<o:p></o:p> >> + */<o:p></o:p> >> +}<o:p></o:p> >> +<o:p></o:p> >> static void tgl_setup_private_ppat(struct intel_uncore *uncore) {<o:p></o:p> >> /* TGL doesn't support LLC or AGE settings */ @@ -603,7 +622,9 @@ <o:p></o:p> >> void setup_private_pat(struct intel_gt *gt)<o:p></o:p> >> <o:p></o:p> >> GEM_BUG_ON(GRAPHICS_VER(i915) < 8);<o:p></o:p> >> <o:p></o:p> >> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))<o:p></o:p> >> + if (IS_METEORLAKE(i915))<o:p></o:p> >> + mtl_setup_private_ppat(uncore);<o:p></o:p> ><o:p> </o:p> > We need to handle the primary GT and media GT separately since the<o:p></o:p> > media GT registers are unicast whereas primary are multicast.<o:p></o:p> <o:p> </o:p> Same as above.<o:p></o:p> <o:p> </o:p> >See<o:p></o:p> > <a href="https://gitlab.freedesktop.org/drm/xe/kernel/-/blob/drm-xe-next/drivers/gpu/drm/xe/xe_pat.c"> https://gitlab.freedesktop.org/drm/xe/kernel/-/blob/drm-xe-next/drivers/gpu/drm/xe/xe_pat.c</a><o:p></o:p> > for how this was handled in the Xe driver.<o:p></o:p> ><o:p> </o:p> >> + else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))<o:p></o:p> >> xehp_setup_private_ppat(gt);<o:p></o:p> >> else if (GRAPHICS_VER(i915) >= 12)<o:p></o:p> >> tgl_setup_private_ppat(uncore);<o:p></o:p> >> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h <o:p></o:p> >> b/drivers/gpu/drm/i915/gt/intel_gtt.h<o:p></o:p> >> index 69ce55f517f5..b632167eaf2e 100644<o:p></o:p> >> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h<o:p></o:p> >> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h<o:p></o:p> >> @@ -88,9 +88,18 @@ typedef u64 gen8_pte_t;<o:p></o:p> >> #define BYT_PTE_SNOOPED_BY_CPU_CACHES REG_BIT(2)<o:p></o:p> >> #define BYT_PTE_WRITEABLE REG_BIT(1)<o:p></o:p> >> <o:p></o:p> >> +#define GEN12_PPGTT_PTE_PAT3 BIT_ULL(62)<o:p></o:p> >> #define GEN12_PPGTT_PTE_LM BIT_ULL(11)<o:p></o:p> >> +#define GEN12_PPGTT_PTE_PAT2 BIT_ULL(7)<o:p></o:p> >> +#define GEN12_PPGTT_PTE_NC BIT_ULL(5)<o:p></o:p> >> +#define GEN12_PPGTT_PTE_PAT1 BIT_ULL(4)<o:p></o:p> >> +#define GEN12_PPGTT_PTE_PAT0 BIT_ULL(3)<o:p></o:p> ><o:p> </o:p> > Which bspec page is this from? The PPGTT descriptions in<o:p></o:p> > the bspec are kind of hard to track down.<o:p></o:p> <o:p> </o:p> <a href="https://gfxspecs.intel.com/Predator/Home/Index/53521">https://gfxspecs.intel.com/Predator/Home/Index/53521</a><o:p></o:p> PAT_Index[2:0] = {PAT, PCD, PWT} = BIT(7, 4, 3)<o:p></o:p> PAT_Index[3] = BIT(62)<o:p></o:p> PAT_Index[4] = BIT(61)<o:p></o:p> <o:p> </o:p> > But if these only apply to MTL, why are they labelled as "GEN12?"<o:p></o:p> <o:p> </o:p> These apply to GEN12plus.<o:p></o:p> <o:p> </o:p> >> <o:p></o:p> >> -#define GEN12_GGTT_PTE_LM BIT_ULL(1)<o:p></o:p> >> +#define GEN12_GGTT_PTE_LM BIT_ULL(1)<o:p></o:p> >> +#define MTL_GGTT_PTE_PAT0 BIT_ULL(52)<o:p></o:p> >> +#define MTL_GGTT_PTE_PAT1 BIT_ULL(53)<o:p></o:p> >> +#define GEN12_GGTT_PTE_ADDR_MASK GENMASK_ULL(45, 12)<o:p></o:p> >> +#define MTL_GGTT_PTE_PAT_MASK GENMASK_ULL(53, 52)<o:p></o:p> >> <o:p></o:p> >> #define GEN12_PDE_64K BIT(6)<o:p></o:p> >> #define GEN12_PTE_PS64 BIT(8)<o:p></o:p> >> @@ -147,6 +156,15 @@ typedef u64 gen8_pte_t; #define GEN8_PDE_IPS_64K <o:p></o:p> >> BIT(11)<o:p></o:p> >> #define GEN8_PDE_PS_2M BIT(7)<o:p></o:p> >> <o:p></o:p> >> +#define MTL_PPAT_L4_CACHE_POLICY_MASK REG_GENMASK(3, 2)<o:p></o:p> >> +#define MTL_PAT_INDEX_COH_MODE_MASK REG_GENMASK(1, 0)<o:p></o:p> >> +#define MTL_PPAT_L4_3_UC REG_FIELD_PREP(MTL_PPAT_L4_CACHE_POLICY_MASK, 3)<o:p></o:p> >> +#define MTL_PPAT_L4_1_WT REG_FIELD_PREP(MTL_PPAT_L4_CACHE_POLICY_MASK, 1)<o:p></o:p> >> +#define MTL_PPAT_L4_0_WB REG_FIELD_PREP(MTL_PPAT_L4_CACHE_POLICY_MASK, 0)<o:p></o:p> >> +#define MTL_3_COH_2W REG_FIELD_PREP(MTL_PAT_INDEX_COH_MODE_MASK, 3)<o:p></o:p> >> +#define MTL_2_COH_1W REG_FIELD_PREP(MTL_PAT_INDEX_COH_MODE_MASK, 2)<o:p></o:p> >> +#define MTL_0_COH_NON REG_FIELD_PREP(MTL_PAT_INDEX_COH_MODE_MASK, 0)<o:p></o:p> ><o:p> </o:p> >The values for these definitions don't seem to be aligned.<o:p></o:p> <o:p> </o:p> These are aligned with <a href="https://gfxspecs.intel.com/Predator/Home/Index/45101"> https://gfxspecs.intel.com/Predator/Home/Index/45101</a><o:p></o:p> <o:p> </o:p> >> +<o:p></o:p> >> enum i915_cache_level;<o:p></o:p> >> <o:p></o:p> >> struct drm_i915_gem_object;<o:p></o:p> >> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c <o:p></o:p> >> b/drivers/gpu/drm/i915/gt/intel_mocs.c<o:p></o:p> >> index 69b489e8dfed..89570f137b2c 100644<o:p></o:p> >> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c<o:p></o:p> >> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c<o:p></o:p> >> @@ -40,6 +40,10 @@ struct drm_i915_mocs_table {<o:p></o:p> >> #define LE_COS(value) ((value) << 15)<o:p></o:p> >> #define LE_SSE(value) ((value) << 17)<o:p></o:p> >> <o:p></o:p> >> +/* Defines for the tables (GLOB_MOCS_0 - GLOB_MOCS_16) */<o:p></o:p> >> +#define _L4_CACHEABILITY(value) ((value) << 2)<o:p></o:p> ><o:p> </o:p> > We should use REG_GENMASK + REG_FIELD_PREP for new code, which will help<o:p></o:p> > ensure that we don't try to pack a value into a field that's too big to fit.<o:p></o:p> ><o:p> </o:p> >> +#define IG_PAT(value) ((value) << 8)<o:p></o:p> <o:p> </o:p> This is used in defining MOCS table. I think it's easier to read with IG_PAT(0)<o:p></o:p> and IG_PAT(1), than with and without IG_PAT.<o:p></o:p> <o:p> </o:p> > Do we need this one parameterized? It's just a boolean flag, so<o:p></o:p> ><o:p> </o:p> > #define IG_PAT REG_BIT(8)<o:p></o:p> ><o:p> </o:p> > would be fine (and then don't include the flag on the entries where it isn't set.<o:p></o:p> ><o:p> </o:p> >> +<o:p></o:p> >> /* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */<o:p></o:p> >> #define L3_ESC(value) ((value) << 0)<o:p></o:p> >> #define L3_SCC(value) ((value) << 1)<o:p></o:p> >> @@ -50,6 +54,7 @@ struct drm_i915_mocs_table {<o:p></o:p> >> /* Helper defines */<o:p></o:p> >> #define GEN9_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but configured. */<o:p></o:p> >> #define PVC_NUM_MOCS_ENTRIES 3<o:p></o:p> >> +#define MTL_NUM_MOCS_ENTRIES 16<o:p></o:p> >> <o:p></o:p> >> /* (e)LLC caching options */<o:p></o:p> >> /*<o:p></o:p> >> @@ -73,6 +78,12 @@ struct drm_i915_mocs_table {<o:p></o:p> >> #define L3_2_RESERVED _L3_CACHEABILITY(2)<o:p></o:p> >> #define L3_3_WB _L3_CACHEABILITY(3)<o:p></o:p> >> <o:p></o:p> >> +/* L4 caching options */<o:p></o:p> >> +#define L4_0_WB _L4_CACHEABILITY(0)<o:p></o:p> >> +#define L4_1_WT _L4_CACHEABILITY(1)<o:p></o:p> >> +#define L4_2_RESERVED _L4_CACHEABILITY(2)<o:p></o:p> ><o:p> </o:p> > This definition is unnecessary and unused.<o:p></o:p> ><o:p> </o:p> >> +#define L4_3_UC _L4_CACHEABILITY(3)<o:p></o:p> >> +<o:p></o:p> >> #define MOCS_ENTRY(__idx, __control_value, __l3cc_value) \<o:p></o:p> >> [__idx] = { \<o:p></o:p> >> .control_value = __control_value, \ @@ -416,6 +427,57 @@ static <o:p></o:p> >> const struct drm_i915_mocs_entry pvc_mocs_table[] = {<o:p></o:p> >> MOCS_ENTRY(2, 0, L3_3_WB),<o:p></o:p> >> };<o:p></o:p> >> <o:p></o:p> >> +static const struct drm_i915_mocs_entry mtl_mocs_table[] = {<o:p></o:p> ><o:p> </o:p> > The entries below with WB L4 cache don't include L4_0_WB. That works<o:p></o:p> > out okay since the value is 0, but it seems like it would still be worth<o:p></o:p> > including it, just to make the table meaning more clear.<o:p></o:p> ><o:p> </o:p> > Conversely, I'd drop the "IG_PAT(0)" on entry 0 since that's just a simple<o:p></o:p> > boolean flag, so the "0" there is easier to misread than just not including<o:p></o:p> > it at all.<o:p></o:p> ><o:p> </o:p> >> + /* Error - Reserved for Non-Use */<o:p></o:p> >> + MOCS_ENTRY(0,<o:p></o:p> >> + IG_PAT(0),<o:p></o:p> >> + L3_LKUP(1) | L3_3_WB),<o:p></o:p> >> + /* Cached - L3 + L4 */<o:p></o:p> >> + MOCS_ENTRY(1,<o:p></o:p> >> + IG_PAT(1),<o:p></o:p> >> + L3_LKUP(1) | L3_3_WB),<o:p></o:p> >> + /* L4 - GO:L3 */<o:p></o:p> >> + MOCS_ENTRY(2,<o:p></o:p> >> + IG_PAT(1),<o:p></o:p> >> + L3_LKUP(1) | L3_1_UC),<o:p></o:p> >> + /* Uncached - GO:L3 */<o:p></o:p> >> + MOCS_ENTRY(3,<o:p></o:p> >> + IG_PAT(1) | L4_3_UC,<o:p></o:p> >> + L3_LKUP(1) | L3_1_UC),<o:p></o:p> >> + /* L4 - GO:Mem */<o:p></o:p> >> + MOCS_ENTRY(4,<o:p></o:p> >> + IG_PAT(1),<o:p></o:p> >> + L3_LKUP(1) | L3_GLBGO(1) | L3_1_UC),<o:p></o:p> >> + /* Uncached - GO:Mem */<o:p></o:p> >> + MOCS_ENTRY(5,<o:p></o:p> >> + IG_PAT(1) | L4_3_UC,<o:p></o:p> >> + L3_LKUP(1) | L3_GLBGO(1) | L3_1_UC),<o:p></o:p> >> + /* L4 - L3:NoLKUP; GO:L3 */<o:p></o:p> >> + MOCS_ENTRY(6,<o:p></o:p> >> + IG_PAT(1),<o:p></o:p> >> + L3_1_UC),<o:p></o:p> >> + /* Uncached - L3:NoLKUP; GO:L3 */<o:p></o:p> >> + MOCS_ENTRY(7,<o:p></o:p> >> + IG_PAT(1) | L4_3_UC,<o:p></o:p> >> + L3_1_UC),<o:p></o:p> >> + /* L4 - L3:NoLKUP; GO:Mem */<o:p></o:p> >> + MOCS_ENTRY(8,<o:p></o:p> >> + IG_PAT(1),<o:p></o:p> >> + L3_GLBGO(1) | L3_1_UC),<o:p></o:p> >> + /* Uncached - L3:NoLKUP; GO:Mem */<o:p></o:p> >> + MOCS_ENTRY(9,<o:p></o:p> >> + IG_PAT(1) | L4_3_UC,<o:p></o:p> >> + L3_GLBGO(1) | L3_1_UC),<o:p></o:p> >> + /* Display - L3; L4:WT */<o:p></o:p> >> + MOCS_ENTRY(14,<o:p></o:p> >> + IG_PAT(1) | L4_1_WT,<o:p></o:p> >> + L3_LKUP(1) | L3_3_WB),<o:p></o:p> >> + /* CCS - Non-Displayable */<o:p></o:p> >> + MOCS_ENTRY(15,<o:p></o:p> >> + IG_PAT(1),<o:p></o:p> >> + L3_GLBGO(1) | L3_1_UC),<o:p></o:p> >> +};<o:p></o:p> >> +<o:p></o:p> >> enum {<o:p></o:p> >> HAS_GLOBAL_MOCS = BIT(0),<o:p></o:p> >> HAS_ENGINE_MOCS = BIT(1),<o:p></o:p> >> @@ -445,7 +507,13 @@ static unsigned int get_mocs_settings(const struct drm_i915_private *i915,<o:p></o:p> >> memset(table, 0, sizeof(struct drm_i915_mocs_table));<o:p></o:p> >> <o:p></o:p> >> table->unused_entries_index = I915_MOCS_PTE;<o:p></o:p> >> - if (IS_PONTEVECCHIO(i915)) {<o:p></o:p> >> + if (IS_METEORLAKE(i915)) {<o:p></o:p> >> + table->size = ARRAY_SIZE(mtl_mocs_table);<o:p></o:p> >> + table->table = mtl_mocs_table;<o:p></o:p> >> + table->n_entries = MTL_NUM_MOCS_ENTRIES;<o:p></o:p> >> + table->uc_index = 9;<o:p></o:p> >> + table->unused_entries_index = 1;<o:p></o:p> >> + } else if (IS_PONTEVECCHIO(i915)) {<o:p></o:p> >> table->size = ARRAY_SIZE(pvc_mocs_table);<o:p></o:p> >> table->table = pvc_mocs_table;<o:p></o:p> >> table->n_entries = PVC_NUM_MOCS_ENTRIES; @@ -646,9 +714,9 @@ void <o:p></o:p> >> intel_mocs_init_engine(struct intel_engine_cs *engine)<o:p></o:p> >> init_l3cc_table(engine->gt, &table); }<o:p></o:p> >> <o:p></o:p> >> -static u32 global_mocs_offset(void)<o:p></o:p> >> +static u32 global_mocs_offset(struct intel_gt *gt)<o:p></o:p> >> {<o:p></o:p> >> - return i915_mmio_reg_offset(GEN12_GLOBAL_MOCS(0));<o:p></o:p> >> + return i915_mmio_reg_offset(GEN12_GLOBAL_MOCS(0)) + <o:p></o:p> >> +gt->uncore->gsi_offset;<o:p></o:p> ><o:p> </o:p> > Isn't this going to double-apply the GSI offset in __init_mocs_table?<o:p></o:p> <o:p> </o:p> Good point. Not sure if there are test cases to validate this, but do need<o:p></o:p> to double check.<o:p></o:p> <o:p> </o:p> > We do need to handle the offset manually in read_mocs_table() since<o:p></o:p> > those reads are done with an MI_STORE_REGISTER_MEM instruction.<o:p></o:p> ><o:p> </o:p> > Handling the GSI offset properly deserves its own patch with a dedicated<o:p></o:p> > commit message explaining the details.<o:p></o:p> ><o:p> </o:p> >> }<o:p></o:p> >> <o:p></o:p> >> void intel_set_mocs_index(struct intel_gt *gt) @@ -671,7 +739,7 @@ <o:p></o:p> >> void intel_mocs_init(struct intel_gt *gt)<o:p></o:p> >> */<o:p></o:p> >> flags = get_mocs_settings(gt->i915, &table);<o:p></o:p> >> if (flags & HAS_GLOBAL_MOCS)<o:p></o:p> >> - __init_mocs_table(gt->uncore, &table, global_mocs_offset());<o:p></o:p> >> + __init_mocs_table(gt->uncore, &table, global_mocs_offset(gt));<o:p></o:p> >> <o:p></o:p> >> /*<o:p></o:p> >> * Initialize the L3CC table as part of mocs initalization to make <o:p></o:p> >> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c <o:p> </o:p> >> b/drivers/gpu/drm/i915/gt/selftest_mocs.c<o:p></o:p> >> index ca009a6a13bd..730796346514 100644<o:p></o:p> >> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c<o:p></o:p> >> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c<o:p></o:p> >> @@ -137,7 +137,7 @@ static int read_mocs_table(struct i915_request *rq,<o:p></o:p> >> return 0;<o:p></o:p> >> <o:p></o:p> >> if (HAS_GLOBAL_MOCS_REGISTERS(rq->engine->i915))<o:p></o:p> >> - addr = global_mocs_offset();<o:p></o:p> >> + addr = global_mocs_offset(rq->engine->gt);<o:p></o:p> >> else<o:p></o:p> >> addr = mocs_offset(rq->engine);<o:p></o:p> >> <o:p></o:p> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c <o:p></o:p> >> b/drivers/gpu/drm/i915/i915_pci.c index cddb6e197972..025d32c0b161 <o:p></o:p> >> 100644<o:p></o:p> >> --- a/drivers/gpu/drm/i915/i915_pci.c<o:p></o:p> >> +++ b/drivers/gpu/drm/i915/i915_pci.c<o:p></o:p> >> @@ -1146,6 +1146,7 @@ static const struct intel_device_info mtl_info = {<o:p></o:p> >> .has_flat_ccs = 0,<o:p></o:p> >> .has_gmd_id = 1,<o:p></o:p> >> .has_guc_deprivilege = 1,<o:p></o:p> >> + .has_llc = 0,<o:p></o:p> ><o:p> </o:p> > This should also be in its own patch since it isn't directly related to the<o:p></o:p> > MOCS / PAT tables.<o:p></o:p> <o:p> </o:p> Will update in the next revision.<o:p></o:p> <o:p> </o:p> > Matt<o:p></o:p> ><o:p> </o:p> >> .has_mslice_steering = 0,<o:p></o:p> >> .has_snoop = 1,<o:p></o:p> >> .__runtime.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM,<o:p></o:p> >> --<o:p></o:p> >> 2.25.1<o:p></o:p> >> <o:p></o:p> ><o:p> </o:p> > -- <o:p></o:p> > Matt Roper<o:p></o:p> > Graphics Software Engineer<o:p></o:p> > Linux GPU Platform Enablement<o:p></o:p> > Intel Corporation<o:p></o:p> ><o:p> </o:p> </div> </body> </html>