Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/i915/dg2: kernel contexts clear Wa_22013059131 in LSC_CHICKEN_BIT_0 --
commit 4fe69a30e73450cf67244b9e1360470b774fd0a5 Author: Jia Yao <[email protected]> drm/i915/dg2: Add per-client control for Wa_22013059131 Wa_22013059131 sets FORCE_1_SUB_MESSAGE_PER_FRAGMENT in LSC_CHICKEN_BIT_0 at engine init, but this is known to cause GPU hangs in certain workloads. Add I915_CONTEXT_PARAM_WA_22013059131 so userspace that handles the workaround itself (e.g. by limiting SLM size) can set it to 1 to let the kernel know bit 15 programming is not needed. Link: https://github.com/intel/compute-runtime/pull/919 > diff --git a/drivers/gpu/drm/i915/gt/intel_context.c > b/drivers/gpu/drm/i915/gt/intel_context.c > --- a/drivers/gpu/drm/i915/gt/intel_context.c > +++ b/drivers/gpu/drm/i915/gt/intel_context.c > @@ -80,6 +81,7 @@ int intel_context_alloc_state(struct intel_context *ce) > if (ctx->client) > i915_drm_client_add_context_objects(ctx->client, > ce); > + lrc_latch_wa_22013059131(ce, ctx); [Severity: Medium] Since lrc_latch_wa_22013059131() in intel_context_alloc_state() is only called when ctx is not NULL, what happens for kernel-internal contexts where ce->gem_context is NULL? If the latch function is skipped, CONTEXT_WA_22013059131_APPLY remains unset (0). Does this mean the kernel context will execute with the FORCE_1_SUB_MESSAGE_PER_FRAGMENT workaround disabled? > i915_gem_context_put(ctx); > } > } > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c > b/drivers/gpu/drm/i915/gt/intel_lrc.c > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c [ ... ] > +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; > + > + if (test_bit(CONTEXT_WA_22013059131_APPLY, &ce->flags)) > + val |= FORCE_1_SUB_MESSAGE_PER_FRAGMENT; > + > + *cs++ = MI_LOAD_REGISTER_IMM(1); > + *cs++ = i915_mmio_reg_offset(LSC_CHICKEN_BIT_0); > + *cs++ = val; [Severity: Medium] If CONTEXT_WA_22013059131_APPLY is unset for kernel contexts, this will unconditionally emit a write that clears FORCE_1_SUB_MESSAGE_PER_FRAGMENT in the hardware register. The commit message indicates that "Kernel-internal context will not change workaround settings", but it appears this code clears the workaround during kernel context execution. Could this expose the kernel context to the GPU hangs the workaround is meant to prevent? > + > + return cs; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
