(+ intel-gfx for real this time)
Quoting Joonas Lahtinen (2026-07-01 15:04:57) > (+ intel-gfx, Matt and Tvrtko) > > Quoting [email protected] (2026-07-01 01:53:49) > > Thank you for your contribution! Sashiko AI review found 1 potential > > issue(s) to consider: > > - [High] Dynamically modifying the globally shared MCR register > > LSC_CHICKEN_BIT_0 per-context causes a race condition when contexts execute > > concurrently on different engines, leading to L3 cache state corruption and > > GPU hangs. > > -- > > > > commit aa5554c571fd6ea50b4b1d107d0ba27eeae18a7c > > Author: Jia Yao <[email protected]> > > > > drm/i915/dg2: Add per-context control for Wa_22013059131 > > > > This commit introduces a per-context opt-out for Wa_22013059131 on DG2-G11 > > platforms, allowing userspace to manage LSC_CHICKEN_BIT_0 bit 15 itself. > > It applies this workaround conditionally during indirect context batchbuffer > > execution rather than globally at engine init. > > > > Link: https://github.com/intel/compute-runtime/pull/919 > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c > > > b/drivers/gpu/drm/i915/gt/intel_lrc.c > > > index 147d22907960b..a35601e6f1e90 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > > > > [ ... ] > > > > > @@ -1348,6 +1348,37 @@ gen12_invalidate_state_cache(u32 *cs) > > > return cs; > > > } > > > > > > +static u32 * > > > +dg2_g11_emit_wa_22013059131(const struct intel_context *ce, u32 *cs) > > > +{ > > > + /* > > > + * While re-writing LSC_CHICKEN_BIT_0 for Wa_22013059131, the > > > + * other bits of the register will also get overwritten. The > > > + * hardware default for all other bits is 0, but any workarounds > > > + * that adjust the other bits in the lower dword of the register > > > + * also need to be re-applied here. At the moment that's just > > > + * Wa_22014226127, which is always set for DG2-G11 platforms. > > > + */ > > > + u32 val = DISABLE_D8_D16_COASLESCE; > > > + > > > + /* > > > + * Wa_22013059131: only set FORCE_1_SUB_MESSAGE_PER_FRAGMENT for > > > + * userspace contexts that have not opted out. Kernel-internal > > > + * contexts (gem_context == NULL) never run shader workloads that > > > + * require this workaround, so skip them unconditionally. > > > + */ > > > + if (rcu_access_pointer(ce->gem_context) && > > > + !test_bit(CONTEXT_WA_22013059131, &ce->flags)) { > > > + val |= FORCE_1_SUB_MESSAGE_PER_FRAGMENT; > > > + } > > > + > > > > [Severity: High] > > Does dynamically modifying LSC_CHICKEN_BIT_0 per-context cause a race > > condition > > when contexts execute concurrently on different engines? > > > > LSC_CHICKEN_BIT_0 is a globally shared MCR register affecting the L3 cache, > > which is shared across all render and compute engines on DG2. Modifying it > > via > > MI_LOAD_REGISTER_IMM broadcasts the change to all L3 banks. > > > > If Context A (which requires the workaround) runs on RCS0, and Context B > > (which > > opts out) switches in on CCS0, CCS0's context switch executes this indirect > > batchbuffer and clears bit 15 globally. This could dynamically alter the > > state > > for Context A on RCS0, potentially disabling the workaround and causing a > > GPU > > hang. > > The fact that we expect compute and Mesa to align across their own > contexts sharing single address space today is one thing, but I think > we should be enforcing that. Currently nothing preventing RCS and CCS > sharing one address space from fighting over the variable. > > This makes me doubt if modelling this as a per-context variable was > the right choice. > > Maybe we should instead be making it DRM client wide property and > inheriting it to all contexts? > > Regards, Joonas > > > > > > + *cs++ = MI_LOAD_REGISTER_IMM(1); > > > + *cs++ = i915_mmio_reg_offset(LSC_CHICKEN_BIT_0); > > > + *cs++ = val; > > > + > > > + return cs; > > > +} > > > > -- > > Sashiko AI review ยท > > https://sashiko.dev/#/patchset/[email protected]?part=1
