Re: [PATCH v2 00/10] drm/i915: Stop the intel_de_wait_custom() abuse

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 06 Nov 2025, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Nov 06, 2025 at 05:43:12PM +0200, Jani Nikula wrote:
>> On Thu, 06 Nov 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Get rid of all the custom fast vs. slow intel_de_wait_custom()
>> > timeouts that have started to spread like a cancer recently.
>> >
>> > The eventual aim is to get rid of the fast vs. slow timeout
>> > stuff entirely and switch over to poll_timeout_us()...
>> 
>> Are you planning on still having intel_de_wait_something_something, or
>> do you mean using poll_timeout_us() directly?
>> 
>> I think I like the intel_de_* wait helpers, but just a coherent small
>> set, and everything beyond that should use poll_timeout_us() instead of
>> adding obscure helpers for one-off cases.
>
> Yeah, I think the helpers are fairly nice for the common stuff so
> I guess we should keep them. But not too many, so probably just
> intel_de_wait_{,for_set,for_clear}_us().
>
> My current branch has both _us() and _ms() versions of those, but
> as discussed in the meeting we should probably just go for _us()
> eventually. I think I'll want to have the _ms() stuff around for
> a while though since it makes some of the mechanical conversions
> easier.

I think that's fine.

> I also haven't quite figured out is what to do about the poll
> interval for poll_timeout_us(). I was thinking of starting with
> something simple like 'interval=max(timeout/8+1,100)' and then
> try to capture some data on how many iterations we end up in
> at least some of the more important places (eg. AUX, PPS, etc).
> I suppose in the worst case we might need custom intervals in
> some places, but I'm hoping some kind of generic approach works
> well enough for most stuff.

I think I pretty much winged it when I was doing the conversions... for
most stuff it's not really super important, as long as it's somewhat
proportional to the timeout.

The downside of poll_timeout_us() is that it's not trivial for us to
change the interval e.g. to some automatic exponential backoff with a
max. I like how __xe_mmio_wait32() has that, but I very much dislike how
it has no cap, and you know how exponential functions behave.

Maybe we could pass a negative as sleep_us to poll_timeout_us() to do
some magic...

BR,
Jani.




-- 
Jani Nikula, Intel