Re: [PATCH 4/4] DO-NOT-MERGE: drm/i915: Use poll_timeout_us()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jul 03, 2025 at 03:12:39PM +0300, Jani Nikula wrote:
> On Thu, 03 Jul 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make sure poll_timeout_us() works by using it in i915
> > instead of the custom __wait_for().
> >
> > Remaining difference between two:
> >                | poll_timeout_us() | __wait_for()
> > ---------------------------------------------------
> > backoff        | fixed interval    | exponential
> > usleep_range() | N/4+1 to N        | N to N*2
> > clock          | MONOTONIC         | MONOTONIC_RAW
> >
> > Just a test hack for now, proper conversion probably
> > needs actual thought.
> 
> Agreed.
> 
> I feel pretty strongly about converting everything to use
> poll_timeout_us() and poll_timeout_us_atomic() directly. I think the
> plethora of wait_for variants in i915_utils.h is more confusing than
> helpful (even if some of them are supposed to be "simpler"
> alternatives). I also think the separate atomic variant is better than
> magically deciding that based on delay length.
> 
> I'm also not all that convinced about the exponential wait. Not all of
> the wait_for versions use it, and then it needs to have a max wait
> anyway (we have an issue with xe not having that [1]). I believe callers
> can decide on a sleep length that is appropriate for the timeout, case
> by case, and gut feeling says it's probably fine. ;)

Yeah, we've not really done any work to justify the polling interval/backoff
strategy. At some point it would be nice to collect some statistics to see
what the typical wait durations are, and then perhaps tune the polling
interval on a case by case basis to be at least somewhat optimal (short
enough to not cause significant delays, but long enough to avoid excessive
polling).

-- 
Ville Syrjälä
Intel