| Message ID | 20230217185433.2418370-1-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/mtl: Add engine TLB invalidation | expand |
On 17.02.2023 19:54, Matt Roper wrote: > MTL's primary GT can continue to use the same engine TLB invalidation > programming as past Xe_HP-based platforms. However the media GT needs > some special handling: > * Invalidation registers on the media GT are singleton registers > (unlike the primary GT where they are still MCR). > * Since the GSC is now exposed as an engine, there's a new register to > use for TLB invalidation. The offset is identical to the compute > engine offset, but this is expected --- compute engines only exist on > the primary GT while the GSC only exists on the media GT. > * Although there's only a single GSC engine instance, it inexplicably > uses bit 1 to request invalidations rather than bit 0. > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 52 ++++++++++++++++------- > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 + > 2 files changed, 38 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > index f3a91e7f85f7..af8e158fbd84 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > @@ -1166,6 +1166,11 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) > [COPY_ENGINE_CLASS].mcr_reg = XEHP_BLT_TLB_INV_CR, > [COMPUTE_CLASS].mcr_reg = XEHP_COMPCTX_TLB_INV_CR, > }; > + static const union intel_engine_tlb_inv_reg xelpmp_regs[] = { > + [VIDEO_DECODE_CLASS].reg = GEN12_VD_TLB_INV_CR, > + [VIDEO_ENHANCEMENT_CLASS].reg = GEN12_VE_TLB_INV_CR, > + [OTHER_CLASS].reg = XELPMP_GSC_TLB_INV_CR, > + }; > struct drm_i915_private *i915 = engine->i915; > const unsigned int instance = engine->instance; > const unsigned int class = engine->class; > @@ -1185,19 +1190,28 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) > * 12.00 -> 12.50 transition multi cast handling is required too. > */ > > - if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 50) || > - GRAPHICS_VER_FULL(i915) == IP_VER(12, 55)) { > - regs = xehp_regs; > - num = ARRAY_SIZE(xehp_regs); > - } else if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 0) || > - GRAPHICS_VER_FULL(i915) == IP_VER(12, 10)) { > - regs = gen12_regs; > - num = ARRAY_SIZE(gen12_regs); > - } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) { > - regs = gen8_regs; > - num = ARRAY_SIZE(gen8_regs); > - } else if (GRAPHICS_VER(i915) < 8) { > - return 0; > + if (engine->gt->type == GT_MEDIA) { > + if (MEDIA_VER_FULL(i915) == IP_VER(13, 0)) { > + regs = xelpmp_regs; > + num = ARRAY_SIZE(xelpmp_regs); > + } As I understand currently only GEN13 of media can have GT_MEDIA, so fallback to gt_WARN_ONCE below is expected behavior. > + } else { > + if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 71) || 12.71 is not yet present in i915_pci.c, maybe they should be added together, up to you. > + GRAPHICS_VER_FULL(i915) == IP_VER(12, 70) > + GRAPHICS_VER_FULL(i915) == IP_VER(12, 50) || > + GRAPHICS_VER_FULL(i915) == IP_VER(12, 55)) { > + regs = xehp_regs; > + num = ARRAY_SIZE(xehp_regs); > + } else if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 0) || > + GRAPHICS_VER_FULL(i915) == IP_VER(12, 10)) { > + regs = gen12_regs; > + num = ARRAY_SIZE(gen12_regs); > + } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) { > + regs = gen8_regs; > + num = ARRAY_SIZE(gen8_regs); > + } else if (GRAPHICS_VER(i915) < 8) { > + return 0; > + } > } > > if (gt_WARN_ONCE(engine->gt, !num, > @@ -1212,7 +1226,14 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) > > reg = regs[class]; > > - if (regs == gen8_regs && class == VIDEO_DECODE_CLASS && instance == 1) { > + if (class == OTHER_CLASS) { Maybe it would be safer: if (regs == xelpmp_regs && class == OTHER_CLASS) > + /* > + * There's only a single GSC instance, but it uses register bit > + * 1 instead of either 0 or OTHER_GSC_INSTANCE. > + */ > + GEM_WARN_ON(instance != OTHER_GSC_INSTANCE); > + val = 1; > + } else if (regs == gen8_regs && class == VIDEO_DECODE_CLASS && instance == 1) { > reg.reg = GEN8_M2TCR; > val = 0; > } else { > @@ -1228,7 +1249,8 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) > if (GRAPHICS_VER(i915) >= 12 && > (engine->class == VIDEO_DECODE_CLASS || > engine->class == VIDEO_ENHANCEMENT_CLASS || > - engine->class == COMPUTE_CLASS)) > + engine->class == COMPUTE_CLASS || > + engine->class == OTHER_CLASS)) This is little surprise, I would expect non-masked reg for single instance, but it follows bspec, so OK. Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Regards Andrzej > engine->tlb_inv.request = _MASKED_BIT_ENABLE(val); > else > engine->tlb_inv.request = val; > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > index 416976d396ba..423e3e9c564b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > @@ -1090,6 +1090,7 @@ > #define XEHP_BLT_TLB_INV_CR MCR_REG(0xcee4) > #define GEN12_COMPCTX_TLB_INV_CR _MMIO(0xcf04) > #define XEHP_COMPCTX_TLB_INV_CR MCR_REG(0xcf04) > +#define XELPMP_GSC_TLB_INV_CR _MMIO(0xcf04) /* media GT only */ > > #define XEHP_MERT_MOD_CTRL MCR_REG(0xcf28) > #define RENDER_MOD_CTRL MCR_REG(0xcf2c)
On Thu, Feb 23, 2023 at 10:08:51AM +0100, Andrzej Hajda wrote: > On 17.02.2023 19:54, Matt Roper wrote: > > MTL's primary GT can continue to use the same engine TLB invalidation > > programming as past Xe_HP-based platforms. However the media GT needs > > some special handling: > > * Invalidation registers on the media GT are singleton registers > > (unlike the primary GT where they are still MCR). > > * Since the GSC is now exposed as an engine, there's a new register to > > use for TLB invalidation. The offset is identical to the compute > > engine offset, but this is expected --- compute engines only exist on > > the primary GT while the GSC only exists on the media GT. > > * Although there's only a single GSC engine instance, it inexplicably > > uses bit 1 to request invalidations rather than bit 0. > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 52 ++++++++++++++++------- > > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 + > > 2 files changed, 38 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > index f3a91e7f85f7..af8e158fbd84 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > @@ -1166,6 +1166,11 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) > > [COPY_ENGINE_CLASS].mcr_reg = XEHP_BLT_TLB_INV_CR, > > [COMPUTE_CLASS].mcr_reg = XEHP_COMPCTX_TLB_INV_CR, > > }; > > + static const union intel_engine_tlb_inv_reg xelpmp_regs[] = { > > + [VIDEO_DECODE_CLASS].reg = GEN12_VD_TLB_INV_CR, > > + [VIDEO_ENHANCEMENT_CLASS].reg = GEN12_VE_TLB_INV_CR, > > + [OTHER_CLASS].reg = XELPMP_GSC_TLB_INV_CR, > > + }; > > struct drm_i915_private *i915 = engine->i915; > > const unsigned int instance = engine->instance; > > const unsigned int class = engine->class; > > @@ -1185,19 +1190,28 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) > > * 12.00 -> 12.50 transition multi cast handling is required too. > > */ > > - if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 50) || > > - GRAPHICS_VER_FULL(i915) == IP_VER(12, 55)) { > > - regs = xehp_regs; > > - num = ARRAY_SIZE(xehp_regs); > > - } else if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 0) || > > - GRAPHICS_VER_FULL(i915) == IP_VER(12, 10)) { > > - regs = gen12_regs; > > - num = ARRAY_SIZE(gen12_regs); > > - } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) { > > - regs = gen8_regs; > > - num = ARRAY_SIZE(gen8_regs); > > - } else if (GRAPHICS_VER(i915) < 8) { > > - return 0; > > + if (engine->gt->type == GT_MEDIA) { > > + if (MEDIA_VER_FULL(i915) == IP_VER(13, 0)) { > > + regs = xelpmp_regs; > > + num = ARRAY_SIZE(xelpmp_regs); > > + } > > As I understand currently only GEN13 of media can have GT_MEDIA, so fallback > to gt_WARN_ONCE below is expected behavior. "Gen" terminology isn't used anymore, but yes, standalone media is a new feature starting from media version 13. > > > + } else { > > + if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 71) || > > 12.71 is not yet present in i915_pci.c, maybe they should be added together, > up to you. No, i915_pci.c isn't the source of IP versions anymore. Starting with MTL (and continuing with future platforms), the graphics, media, and display IP versions are read out directly from the hardware itself (the GMD_ID registers); they no longer get derived from PCI devid matching. The vestigial 12.70 value in i915_pci.c shouldn't get used anywhere except as a very basic sanity check that the GMD_ID registers are correctly reporting a high enough version. Matt > > > + GRAPHICS_VER_FULL(i915) == IP_VER(12, 70) > + GRAPHICS_VER_FULL(i915) == IP_VER(12, 50) || > > + GRAPHICS_VER_FULL(i915) == IP_VER(12, 55)) { > > + regs = xehp_regs; > > + num = ARRAY_SIZE(xehp_regs); > > + } else if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 0) || > > + GRAPHICS_VER_FULL(i915) == IP_VER(12, 10)) { > > + regs = gen12_regs; > > + num = ARRAY_SIZE(gen12_regs); > > + } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) { > > + regs = gen8_regs; > > + num = ARRAY_SIZE(gen8_regs); > > + } else if (GRAPHICS_VER(i915) < 8) { > > + return 0; > > + } > > } > > if (gt_WARN_ONCE(engine->gt, !num, > > @@ -1212,7 +1226,14 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) > > reg = regs[class]; > > - if (regs == gen8_regs && class == VIDEO_DECODE_CLASS && instance == 1) { > > + if (class == OTHER_CLASS) { > > Maybe it would be safer: > if (regs == xelpmp_regs && class == OTHER_CLASS) > > > > + /* > > + * There's only a single GSC instance, but it uses register bit > > + * 1 instead of either 0 or OTHER_GSC_INSTANCE. > > + */ > > + GEM_WARN_ON(instance != OTHER_GSC_INSTANCE); > > + val = 1; > > + } else if (regs == gen8_regs && class == VIDEO_DECODE_CLASS && instance == 1) { > > reg.reg = GEN8_M2TCR; > > val = 0; > > } else { > > @@ -1228,7 +1249,8 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) > > if (GRAPHICS_VER(i915) >= 12 && > > (engine->class == VIDEO_DECODE_CLASS || > > engine->class == VIDEO_ENHANCEMENT_CLASS || > > - engine->class == COMPUTE_CLASS)) > > + engine->class == COMPUTE_CLASS || > > + engine->class == OTHER_CLASS)) > This is little surprise, I would expect non-masked reg for single instance, > but it follows bspec, so OK. > > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> > > Regards > Andrzej > > > engine->tlb_inv.request = _MASKED_BIT_ENABLE(val); > > else > > engine->tlb_inv.request = val; > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > index 416976d396ba..423e3e9c564b 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > > @@ -1090,6 +1090,7 @@ > > #define XEHP_BLT_TLB_INV_CR MCR_REG(0xcee4) > > #define GEN12_COMPCTX_TLB_INV_CR _MMIO(0xcf04) > > #define XEHP_COMPCTX_TLB_INV_CR MCR_REG(0xcf04) > > +#define XELPMP_GSC_TLB_INV_CR _MMIO(0xcf04) /* media GT only */ > > #define XEHP_MERT_MOD_CTRL MCR_REG(0xcf28) > > #define RENDER_MOD_CTRL MCR_REG(0xcf2c) >
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index f3a91e7f85f7..af8e158fbd84 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -1166,6 +1166,11 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) [COPY_ENGINE_CLASS].mcr_reg = XEHP_BLT_TLB_INV_CR, [COMPUTE_CLASS].mcr_reg = XEHP_COMPCTX_TLB_INV_CR, }; + static const union intel_engine_tlb_inv_reg xelpmp_regs[] = { + [VIDEO_DECODE_CLASS].reg = GEN12_VD_TLB_INV_CR, + [VIDEO_ENHANCEMENT_CLASS].reg = GEN12_VE_TLB_INV_CR, + [OTHER_CLASS].reg = XELPMP_GSC_TLB_INV_CR, + }; struct drm_i915_private *i915 = engine->i915; const unsigned int instance = engine->instance; const unsigned int class = engine->class; @@ -1185,19 +1190,28 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) * 12.00 -> 12.50 transition multi cast handling is required too. */ - if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 50) || - GRAPHICS_VER_FULL(i915) == IP_VER(12, 55)) { - regs = xehp_regs; - num = ARRAY_SIZE(xehp_regs); - } else if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 0) || - GRAPHICS_VER_FULL(i915) == IP_VER(12, 10)) { - regs = gen12_regs; - num = ARRAY_SIZE(gen12_regs); - } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) { - regs = gen8_regs; - num = ARRAY_SIZE(gen8_regs); - } else if (GRAPHICS_VER(i915) < 8) { - return 0; + if (engine->gt->type == GT_MEDIA) { + if (MEDIA_VER_FULL(i915) == IP_VER(13, 0)) { + regs = xelpmp_regs; + num = ARRAY_SIZE(xelpmp_regs); + } + } else { + if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 71) || + GRAPHICS_VER_FULL(i915) == IP_VER(12, 70) || + GRAPHICS_VER_FULL(i915) == IP_VER(12, 50) || + GRAPHICS_VER_FULL(i915) == IP_VER(12, 55)) { + regs = xehp_regs; + num = ARRAY_SIZE(xehp_regs); + } else if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 0) || + GRAPHICS_VER_FULL(i915) == IP_VER(12, 10)) { + regs = gen12_regs; + num = ARRAY_SIZE(gen12_regs); + } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) { + regs = gen8_regs; + num = ARRAY_SIZE(gen8_regs); + } else if (GRAPHICS_VER(i915) < 8) { + return 0; + } } if (gt_WARN_ONCE(engine->gt, !num, @@ -1212,7 +1226,14 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) reg = regs[class]; - if (regs == gen8_regs && class == VIDEO_DECODE_CLASS && instance == 1) { + if (class == OTHER_CLASS) { + /* + * There's only a single GSC instance, but it uses register bit + * 1 instead of either 0 or OTHER_GSC_INSTANCE. + */ + GEM_WARN_ON(instance != OTHER_GSC_INSTANCE); + val = 1; + } else if (regs == gen8_regs && class == VIDEO_DECODE_CLASS && instance == 1) { reg.reg = GEN8_M2TCR; val = 0; } else { @@ -1228,7 +1249,8 @@ static int intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine) if (GRAPHICS_VER(i915) >= 12 && (engine->class == VIDEO_DECODE_CLASS || engine->class == VIDEO_ENHANCEMENT_CLASS || - engine->class == COMPUTE_CLASS)) + engine->class == COMPUTE_CLASS || + engine->class == OTHER_CLASS)) engine->tlb_inv.request = _MASKED_BIT_ENABLE(val); else engine->tlb_inv.request = val; diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index 416976d396ba..423e3e9c564b 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -1090,6 +1090,7 @@ #define XEHP_BLT_TLB_INV_CR MCR_REG(0xcee4) #define GEN12_COMPCTX_TLB_INV_CR _MMIO(0xcf04) #define XEHP_COMPCTX_TLB_INV_CR MCR_REG(0xcf04) +#define XELPMP_GSC_TLB_INV_CR _MMIO(0xcf04) /* media GT only */ #define XEHP_MERT_MOD_CTRL MCR_REG(0xcf28) #define RENDER_MOD_CTRL MCR_REG(0xcf2c)
MTL's primary GT can continue to use the same engine TLB invalidation programming as past Xe_HP-based platforms. However the media GT needs some special handling: * Invalidation registers on the media GT are singleton registers (unlike the primary GT where they are still MCR). * Since the GSC is now exposed as an engine, there's a new register to use for TLB invalidation. The offset is identical to the compute engine offset, but this is expected --- compute engines only exist on the primary GT while the GSC only exists on the media GT. * Although there's only a single GSC engine instance, it inexplicably uses bit 1 to request invalidations rather than bit 0. Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 52 ++++++++++++++++------- drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 + 2 files changed, 38 insertions(+), 15 deletions(-)