Skip to content

Commit 28cf243

Browse files
author
Lucas De Marchi
committed
drm/i915/gt: Fix context workarounds with non-masked regs
Most of the context workarounds tweak masked registers, but not all. For masked registers, when writing the value it's sufficient to just write the wa->set_bits since that will take care of both the clr and set bits as well as not overwriting other bits. However there are some workarounds, the registers are non-masked. Up until now the driver was simply emitting a MI_LOAD_REGISTER_IMM with the set_bits to program the register via the GPU in the WA bb. This has the side effect of overwriting the content of the register outside of bits that should be set and also doesn't handle the bits that should be cleared. Kenneth reported that on DG2, mesa was seeing a weird behavior due to the kernel programming of L3SQCREG5 in dg2_ctx_gt_tuning_init(). With the GPU idle, that register could be read via intel_reg as 0x00e001ff, but during a 3D workload it would change to 0x0000007f. So the programming of that tuning was affecting more than the bits in L3_PWM_TIMER_INIT_VAL_MASK. Matt Roper noticed the lack of rmw for the context workarounds due to the use of MI_LOAD_REGISTER_IMM. So, for registers that are not masked, read its value via mmio, modify and then set it in the buffer to be written by the GPU. This should take care in a simple way of programming just the bits required by the tuning/workaround. If in future there are registers that involved that can't be read by the CPU, a more complex approach may be required like a) issuing additional instructions to read and modify; or b) scan the golden context and patch it in place before saving it; or something else. But for now this should suffice. Scanning the context workarounds for all platforms, these are the impacted ones with the respective registers mtl: DRAW_WATERMARK mtl/dg2: XEHP_L3SQCREG5, XEHP_FF_MODE2 ICL has some non-masked registers in the context workarounds: GEN8_L3CNTLREG, IVB_FBC_RT_BASE and VB_FBC_RT_BASE_UPPER, but there shouldn't be an impact. The first is already being manually read and the other 2 are intentionally overwriting the entire register. Same reasoning applies to GEN12_FF_MODE2: the WA is intentionally overwriting all the bits to avoid a read-modify-write. v2: Reword commit message wrt GEN12_FF_MODE2 and the changed behavior on preparatory patches. v3: Also skip reading if clear|set bits covers everything Cc: Kenneth Graunke <kenneth@whitecape.org> Cc: Matt Roper <matthew.d.roper@intel.com> Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23783#note_1968971 Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> Link: https://patchwork.freedesktop.org/patch/msgid/20230630203509.1635216-4-lucas.demarchi@intel.com
1 parent e8f7df1 commit 28cf243

1 file changed

Lines changed: 27 additions & 1 deletion

File tree

drivers/gpu/drm/i915/gt/intel_workarounds.c

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,9 @@ void intel_engine_init_ctx_wa(struct intel_engine_cs *engine)
986986
int intel_engine_emit_ctx_wa(struct i915_request *rq)
987987
{
988988
struct i915_wa_list *wal = &rq->engine->ctx_wa_list;
989+
struct intel_uncore *uncore = rq->engine->uncore;
990+
enum forcewake_domains fw;
991+
unsigned long flags;
989992
struct i915_wa *wa;
990993
unsigned int i;
991994
u32 *cs;
@@ -1002,13 +1005,36 @@ int intel_engine_emit_ctx_wa(struct i915_request *rq)
10021005
if (IS_ERR(cs))
10031006
return PTR_ERR(cs);
10041007

1008+
fw = wal_get_fw_for_rmw(uncore, wal);
1009+
1010+
intel_gt_mcr_lock(wal->gt, &flags);
1011+
spin_lock(&uncore->lock);
1012+
intel_uncore_forcewake_get__locked(uncore, fw);
1013+
10051014
*cs++ = MI_LOAD_REGISTER_IMM(wal->count);
10061015
for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
1016+
u32 val;
1017+
1018+
/* Skip reading the register if it's not really needed */
1019+
if (wa->masked_reg || (wa->clr | wa->set) == U32_MAX) {
1020+
val = wa->set;
1021+
} else {
1022+
val = wa->is_mcr ?
1023+
intel_gt_mcr_read_any_fw(wal->gt, wa->mcr_reg) :
1024+
intel_uncore_read_fw(uncore, wa->reg);
1025+
val &= ~wa->clr;
1026+
val |= wa->set;
1027+
}
1028+
10071029
*cs++ = i915_mmio_reg_offset(wa->reg);
1008-
*cs++ = wa->set;
1030+
*cs++ = val;
10091031
}
10101032
*cs++ = MI_NOOP;
10111033

1034+
intel_uncore_forcewake_put__locked(uncore, fw);
1035+
spin_unlock(&uncore->lock);
1036+
intel_gt_mcr_unlock(wal->gt, flags);
1037+
10121038
intel_ring_advance(rq, cs);
10131039

10141040
ret = rq->engine->emit_flush(rq, EMIT_BARRIER);

0 commit comments

Comments
 (0)