Re: [PATCH] drm/xe/mmio: Fix xe_mmio_wait32() to honor delay/sleep maximums

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/xe: udelay capped at 5us due to MAX_UDELAY_MS unit mismatch
- [Medium] drm/xe: commit message claims DIV_ROUND_UP helper but code is missing
--

commit cceb24ccd008b44c48532606a8d174234bb68933
Author: Alan Previn <alan.previn.teres.alexis@intel.com>

drm/xe/mmio: Fix xe_mmio_wait32() to honor delay/sleep maximums

This commit modifies __xe_mmio_wait32() to cap delay values to avoid
intolerably large waits. It picks the appropriate sleep or delay helpers
based on atomic context and wait duration.

>     Check for overflow in udelay/usleep_range use in __xe_mmio_wait32
>     and pick the correct helper according to the wait time and atomic.
>     Implement a similar helper to replace DIV_ROUND_UP for 32-bit CPUs.

[Severity: Medium]
The commit message explicitly states "Implement a similar helper to replace
DIV_ROUND_UP for 32-bit CPUs", but the patch does not contain any such
implementation in __xe_mmio_wait32() or elsewhere.

Was this code intended to be included, or is the commit message outdated from
previous iterations of this patch?

> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index 7fa18dfcb5a28..c7c37e59dab05 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -350,10 +350,11 @@ static int __xe_mmio_wait32(struct xe_mmio *mmio, struct xe_reg reg, u32 mask, u
>  			wait = ktime_us_delta(end, cur);
>  
>  		if (atomic)
> -			udelay(wait);
> +			udelay(min_t(s64, wait, MAX_UDELAY_MS));

[Severity: Medium]
Is there a unit mismatch here in __xe_mmio_wait32() between wait and
MAX_UDELAY_MS?

The wait variable is calculated in microseconds, but MAX_UDELAY_MS is a
constant representing milliseconds. By using min_t() with MAX_UDELAY_MS
directly, the udelay() argument appears to be incorrectly capped at a few
microseconds instead of the intended millisecond equivalent (e.g.,
MAX_UDELAY_MS * 1000).

If the wait grows beyond a few microseconds, could this unit mismatch
completely defeat the exponential backoff for atomic MMIO waits, causing the
CPU to poll the MMIO bus indefinitely at small intervals instead of backing
off?

>  		else
>  			usleep_range(wait, wait << 1);
> -		wait <<= 1;
> +#define __XE_MMIO_WAIT_MAX_BACKOFF_100MS (100 * USEC_PER_MSEC)
> +		wait = min_t(s64, wait << 1, __XE_MMIO_WAIT_MAX_BACKOFF_100MS);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929054444.1933871-2-alan.previn.teres.alexis@intel.com?part=1