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

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

On Tue, 2026-09-15 at 18:51 -0400, Vivi, Rodrigo wrote:
> On Tue, Sep 15, 2026 at 04:00:54PM +0000, Teres Alexis, Alan Previn wrote:
> > On Tue, 2026-09-15 at 10:34 +0300, Nikula, Jani wrote:
> > > On Mon, 14 Sep 2026, Alan Previn <alan.previn.teres.alexis@intel.com> wrote:
> > alan:snip
> > > >  int xe_mmio_wait32(struct xe_mmio *mmio, struct xe_reg reg, u32 mask, u32 val, u32 timeout_us,
> > > > -		   u32 *out_val, bool atomic)
> > > > +		   u32 *out_val)
> > > >  {
> > > > -	return __xe_mmio_wait32(mmio, reg, mask, val, timeout_us, out_val, atomic, true);
> > > > +	u32 read;
> > > > +	int ret;
> > > > +
> > > > +	ret = poll_timeout_us(read = xe_mmio_read32(mmio, reg), (read & mask) == val,
> > > > +			      10, timeout_us, false);
> > > 
> > > You probably do need to let the callers pass in the wait too. 10 us wait
> > > with a long timeout is going to be pretty bad.
> 
> agreed
> 
> > > 
> > > 
> > alan: okay - perhaps i can make every caller pass in a polling-wait thats a fraction of their wait time.
> > (as a starting point since i dont know what's the expected behavior of every caller).
> > so perhaps something like "timeout_us << 4" (i.e. 1/16th) but pass in 10 us if its anything smaller than that
> > (i.e. smaller than 16 usec).
> 
> I think we might be complicating this too much..
> 
> what about something simpler like:
> 
> #define XE_MMIO_WAIT_MAX_BACKOFF_US   1000
> 
> ...
> -             wait <<= 1;
> +             wait = min_t(s64, wait << 1, XE_MMIO_WAIT_MAX_BACKOFF_US);
> 
> 

alan: i dont understand your this comment on the increasing the "wait by x2" in the loop after agreeing with Jani on the earlier statement.
Some historical context:

 1. current baseline code it stands to day IS in violation of linux rules for how to use those sleep/delay functions
 2. my initial revs on fixing this was to minimize the changes so its not complicated by simply fixing the code in place.
 3. Jani said we really should use the proper linux kernel helpers: poll_timeout_us / poll_timeout_us_atomic.
 4. Those helpers have the "timeout_us" and the "intra-loop-wait-us" period. I hardcoded to 10 usec.
	- Jani said i should not hardcode and ensure all up-the-stack callers of the xe_mmio_wait32 function passes in the intra-wait-loop
value. 
	- Then you (Rodrigo) agreed with his request but go on to propose going back to exponential 2x intra-wait-loop.
		- but that contradicts Jani's request u agreed to and also that means we implement the intra-wait-loop? (i.e. dont use the
proper linux helper?)

 ...alan


> Also the Fixes tag is not the right one... the bug was there before...
> 
> > 
> > ...alan