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 <[email protected]> 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/[email protected]?part=1
