static u32 *gen12_emit_aux_table_inv(const i915_reg_t inv_reg, u32 *cs) { *cs++ = MI_LOAD_REGISTER_IMM(1); @@ -296,7 +272,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode) if (!HAS_FLAT_CCS(rq->engine->i915)) { aux_inv = rq->engine->mask & ~BIT(BCS0); if (aux_inv)
cmd += 2 * hweight32(aux_inv) + 2;
cmd += 4;@@ -329,14 +305,16 @@ 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;Cool, I didn't know this exists. First Bspec link I found did not mention these register, but second did. That one however (29545) has a worrying "removed by" tag which seems to point to a story suggesting the remapping table might be gone on machines with flat ccs?! Could you double check please?
The variable aux_inv is set only if (!HAS_FLAT_CCS(rq->engine->i915)).
if (rq->engine->class == VIDEO_DECODE_CLASS) {*cs++ = i915_mmio_reg_offset(GEN12_VD0_AUX_NV);} else if (rq->engine->class == VIDEO_ENHANCEMENT_CLASS) {*cs++ = i915_mmio_reg_offset(GEN12_VE0_AUX_NV);} else {GEM_BUG_ON("unknown aux_inv reg\n");*cs++ = i915_mmio_reg_offset(INVALID_MMIO_REG);I'd consider not emitting the LRI if we don't know what to put in unless there is some hidden point to do it?
That's true. I was following the original logic flow here. I think it would be better to check for engine class before setting the variable aux_inv.
}
*cs++ = AUX_INV;diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h index d112ffd56418..2d150eec5c65 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 (1 << 17) #define MI_LRI_FORCE_POSTED (1<<12)
Passing observation - three bits, three flavours of expressing them, sigh...
Haha, REG_BIT(17) it is. The other one causes a CHECK:SPACING, but don't want to touch that in this patch.
#define MI_LOAD_REGISTER_IMM_MAX_REGS (126) #define MI_STORE_REGISTER_MEM MI_INSTR(0x24, 1)