On 18/03/2022 18:08, fei.yang@intel.com wrote:

From: Fei Yang fei.yang@intel.com

GPU hangs have been observed when multiple engines write to the same aux_inv register at the same time. To avoid this each engine should only invalidate its own auxiliary table. The function gen12_emit_flush_xcs() currently invalidate the auxiliary table for all engines because the rq->engine is not necessarily the engine eventually carrying out the request, and potentially the engine could even be a virtual one (with engine->instance being -1). With the MMIO remap feature, we can actually set bit 17 of MI_LRI instruction and let the hardware to figure out the local aux_inv register at runtime to avoid invalidating auxiliary table for all engines.

Bspec: 45728

Cc: Stuart Summers stuart.summers@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Chris Wilson chris.p.wilson@intel.com Signed-off-by: Fei Yang fei.yang@intel.com

---

drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 44 +++++--------------- drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 1 + 2 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 36148887c699..6e83ac06aaf6 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -165,30 +165,6 @@ static u32 preparser_disable(bool state) return MI_ARB_CHECK | 1 << 8 | state; }

-static i915_reg_t aux_inv_reg(const struct intel_engine_cs *engine) -{

• static const i915_reg_t vd[] = {

• GEN12_VD0_AUX_NV,
  

• GEN12_VD1_AUX_NV,
  

• GEN12_VD2_AUX_NV,
  

• GEN12_VD3_AUX_NV,
  

• };
• static const i915_reg_t ve[] = {

• GEN12_VE0_AUX_NV,
  

• GEN12_VE1_AUX_NV,
  

• };
• if (engine->class == VIDEO_DECODE_CLASS)

• return vd[engine->instance];
  

• if (engine->class == VIDEO_ENHANCEMENT_CLASS)

• return ve[engine->instance];
  

• GEM_BUG_ON("unknown aux_inv reg\n");
• return INVALID_MMIO_REG;

-}

• static u32 *gen12_emit_aux_table_inv(const i915_reg_t inv_reg, u32 *cs) { *cs++ = MI_LOAD_REGISTER_IMM(1);

@@ -293,10 +269,12 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode) if (mode & EMIT_INVALIDATE) { cmd += 2;

• if (!HAS_FLAT_CCS(rq->engine->i915)) {
  

• if (!HAS_FLAT_CCS(rq->engine->i915) &&
  

•     (rq->engine->class == VIDEO_DECODE_CLASS ||
  

•      rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) {
  aux_inv = rq->engine->mask & ~BIT(BCS0);
  if (aux_inv)
  

• 		cmd += 2 * hweight32(aux_inv) + 2;
  

• 		cmd += 4;
  

  } }

@@ -329,14 +307,12 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode) *cs++ = 0; /* value */

if (aux_inv) { /* hsdes: 1809175790 */

• struct intel_engine_cs *engine;
  

• unsigned int tmp;
  

• *cs++ = MI_LOAD_REGISTER_IMM(hweight32(aux_inv));
  

• for_each_engine_masked(engine, rq->engine->gt, aux_inv, tmp) {
  

• 	*cs++ = i915_mmio_reg_offset(aux_inv_reg(engine));
  

• 	*cs++ = AUX_INV;
  

• }
  

• *cs++ = MI_LOAD_REGISTER_IMM(1) | MI_LRI_MMIO_REMAP_EN;
  

• if (rq->engine->class == VIDEO_DECODE_CLASS)
  

• 	*cs++ = i915_mmio_reg_offset(GEN12_VD0_AUX_NV);
  

• else
  

• 	*cs++ = i915_mmio_reg_offset(GEN12_VE0_AUX_NV);
  

• *cs++ = AUX_INV;
  

  *cs++ = MI_NOOP; }

diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h index d112ffd56418..4243be030bc1 100644 --- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h +++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h @@ -144,6 +144,7 @@ #define MI_LOAD_REGISTER_IMM(x) MI_INSTR(0x22, 2*(x)-1) /* Gen11+. addr = base + (ctx_restore ? offset & GENMASK(12,2) : offset) */ #define MI_LRI_LRM_CS_MMIO REG_BIT(19) +#define MI_LRI_MMIO_REMAP_EN REG_BIT(17) #define MI_LRI_FORCE_POSTED (1<<12) #define MI_LOAD_REGISTER_IMM_MAX_REGS (126) #define MI_STORE_REGISTER_MEM MI_INSTR(0x24, 1)

LGTM.

Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@intel.com

Affects igpus from TGL onwards? If so:

Cc: stable@vger.kernel.org # v5.7+

?

Backporting might end up fun..

Regards,

Tvrtko