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.ursu...@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
Signed-off-by: Matt Roper <matthew.d.ro...@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.ha...@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)

Reply via email to