On Tue, Nov 11, 2025 at 10:01:14AM +0200, Jani Nikula wrote: > On Mon, 10 Nov 2025, Ville Syrjala <[email protected]> wrote: > > From: Ville Syrjälä <[email protected]> > > > > Clean up the register polling stuff: > > - rename the current wait stuff to > > intel_de_wait_{,for_set,for_clear}_ms() > > - introduce intel_de_wait_{,for_set,for_clear}_us() > > - nuke intel_de_wait_custom() > > - change the wakelock stuff to use _fw() instead of > > hand rolling yet another level of register accessors > > - a few other minor cleanups > > > > After this it should be fairly easy to switch over to > > poll_timeout_us(). > > Overall the series looks fine. > > Acked-by: Jani Nikula <[email protected]> > > since Suraj already did the detailed review. > > One questions remains unanswered, and I guess I'll have to wait for > follow-up to see the answer. I really *really* dislike how the i915 > variants are somewhat ambiguously atomic, i.e. atomic when slow timeout > is 0, and the xe wrappers replicate that behaviour. xe_mmio_wait32() has > atomic as parameter. > > I would like the atomic variants to be explicit, and separate. Similar > to poll_timeout_us() and poll_timeout_us_atomic(). You immediately know > from the call whether you're doing the right thing or not. And that > really should not directly depend on the timeout length. Since you plan > on using the generic poll helpers, I presume this will propagate to the > register polling helpers. > > Since we do a lot of non-atomic millisecond waits, I guess it's worth > having the _ms variants, although like I said before, I'd kind of like > going for _us all over the place. But no big deal, in the end the _ms > variants can be trivial wrappers on the non-atomic _us ones.
At the end of this series we have intel_de_wait_fw_us_atomic() which is the only thing that anyone should call from atomic context. Granted the _us() stuff all still work in atomic context due to the underlying uncore implementation. But when we switch to poll_timeout_us() I plan to make all the other stuff unusable from atomic contexts. Ideally we probably shouldn't even have intel_de_wait_fw_us_atomic(). I think the wakelock code is the only place that needs it, due to the spinlock presumably. I haven't checked if we could just eg. make that a mutex and use the non-atomic thing there as well. -- Ville Syrjälä Intel
