On Thu, 03 Jul 2025, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > 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. ;) BR, Jani. [1] https://lore.kernel.org/r/[email protected] > > Cc: Jani Nikula <[email protected]> > Cc: Lucas De Marchi <[email protected]> > Cc: Dibin Moolakadan Subrahmanian <[email protected]> > Cc: Imre Deak <[email protected]> > Cc: David Laight <[email protected]> > Cc: Geert Uytterhoeven <[email protected]> > Cc: Matt Wagantall <[email protected]> > Cc: Dejin Zheng <[email protected]> > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/i915_utils.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_utils.h > b/drivers/gpu/drm/i915/i915_utils.h > index f7fb40cfdb70..8509d1de1901 100644 > --- a/drivers/gpu/drm/i915/i915_utils.h > +++ b/drivers/gpu/drm/i915/i915_utils.h > @@ -32,6 +32,7 @@ > #include <linux/types.h> > #include <linux/workqueue.h> > #include <linux/sched/clock.h> > +#include <linux/iopoll.h> > > #ifdef CONFIG_X86 > #include <asm/hypervisor.h> > @@ -238,7 +239,7 @@ wait_remaining_ms_from_jiffies(unsigned long > timestamp_jiffies, int to_wait_ms) > * timeout could be due to preemption or similar and we've never had a > chance to > * check the condition before the timeout. > */ > -#define __wait_for(OP, COND, US, Wmin, Wmax) ({ \ > +#define __wait_for_old(OP, COND, US, Wmin, Wmax) ({ \ > const ktime_t end__ = ktime_add_ns(ktime_get_raw(), 1000ll * (US)); \ > long wait__ = (Wmin); /* recommended min for usleep is 10 us */ \ > int ret__; \ > @@ -263,6 +264,8 @@ wait_remaining_ms_from_jiffies(unsigned long > timestamp_jiffies, int to_wait_ms) > ret__; \ > }) > > +#define __wait_for(OP, COND, US, Wmin, Wmax) \ > + poll_timeout_us(OP, COND, (Wmin), (US), false) > #define _wait_for(COND, US, Wmin, Wmax) __wait_for(, (COND), (US), > (Wmin), \ > (Wmax)) > #define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000) -- Jani Nikula, Intel
