Re: [PATCH 2/2] drm/xe: Add wa_14025941587 to xe2, xe3 and xe3p platforms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Tick quantization rounding up causes `idledly` to exceed `maxcnt`, triggering spurious kernel warnings and WA invariant violations.
--

commit d777113f52ced7ebda03cbb9155258701e427a45
Author: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>

drm/xe: Add wa_14025941587 to xe2, xe3 and xe3p platforms

This commit avoids programming the IDLEDLY timer to less than 5 microseconds.
It applies wa_14025941587 to Graphics Versions 20.01 to 35.11 and Media
Versions 13.01 to 35.03.

> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
> index c86890e09b555..37cd7fe38c2b5 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
[ ... ]
> @@ -588,33 +588,78 @@ static void hw_engine_init_early(struct xe_gt *gt, struct xe_hw_engine *hwe,
>  static void adjust_idledly(struct xe_hw_engine *hwe)
>  {
>  	struct xe_gt *gt = hwe->gt;
[ ... ]
> +		if (XE_GT_WA(gt, 16023105232) &&
> +		    xe_gt_WARN_ON(gt, idledly >= maxcnt || inhibit_switch)) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Will this warning spuriously fire on subsequent reads?

If maxcnt is unprogrammed (0), the two workarounds conflict entirely (one
forcing 5000, the other forcing 0), which would cause a WARN_ON and clamp
cycle on every reset.

Additionally, if maxcnt tightly bounds the 5000ns minimum, the tick
quantization rounding up below can push the delay time above maxcnt. On the
next engine reset or resume, this triggers this xe_gt_WARN_ON() and forces a
clamp, causing idledly to oscillate and violate the invariants of
Wa_16023105232.

> +			/* maxcnt may be 0 if IDLE_WAIT_TIME is unprogrammed. */
> +			idledly = maxcnt ? maxcnt - 1 : 0;
> +			clamped_below_maxcnt = true;
> +			wa_applied = true;
> +			/* inhibit_switch override to keep
> +			 * semantics of Wa_16023105232
> +			 */
> +			if (inhibit_switch)
> +				idledly_reg_val &= ~INHIBIT_SWITCH_UNTIL_PREEMPTED;
> +		}
> +
> +		if (wa_applied) {
> +			u32 idledly_ticks;
> +
> +			/*
> +			 * Wa_16023105232 requires idledly < maxcnt, so floor
> +			 * that clamp; otherwise round up to guarantee the
> +			 * Wa_14025941587 minimum survives tick quantization.
> +			 */
> +			if (clamped_below_maxcnt)
> +				idledly_ticks = DIV_ROUND_DOWN_ULL((u64)idledly * 1000,
> +								   idledly_units_ps);
> +			else
> +				idledly_ticks = DIV_ROUND_UP_ULL((u64)idledly * 1000,
> +								 idledly_units_ps);
                                                                 ^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Does this ceiling division push the delay time above the maxcnt boundary?

When maxcnt is close to the 5000ns minimum (e.g. 5120ns), rounding up idledly
to ticks produces a delay (e.g. 5417ns) that exceeds maxcnt, storing a value
that will fail the Wa_16023105232 check on the next read.

> +
> +			idledly_reg_val &= ~IDLE_DELAY;
> +			idledly_reg_val |= REG_FIELD_PREP(IDLE_DELAY, idledly_ticks);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909161844.125147-1-tilak.tirumalesh.tangudu@intel.com?part=2