Re: [PATCH v12] drm/i915/dg2: Add per-client control for Wa_22013059131

[sashiko-bot@kernel.org]

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 <jia.yao@intel.com>

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/20260930171334.3636-1-jia.yao@intel.com?part=1