On Tue, 01 Jul 2025, Lucas De Marchi <[email protected]> wrote: > On Tue, Jul 01, 2025 at 12:28:41PM +0300, Jani Nikula wrote: >>On Mon, 30 Jun 2025, Dibin Moolakadan Subrahmanian >><[email protected]> wrote: >>> The current wait_panel_on() uses intel_de_wait() with a long timeout >>> (5000ms), which is suboptimal on Xe platforms where the underlying >>> xe_mmio_wait32() employs an exponential backoff strategy. This leads >>> to unnecessary delays during resume or power-on when the panel becomes >>> ready earlier than the full timeout. >>> >>> This patch splits the total wait time into two pases >>> - First wait for the typical panel-on time(180ms) >>> - If panel is not ready , continue polling in short 20ms intervals >>> until the maximum timeout (5000ms) is reached >> >>I'm *very* reluctant to merge any new custom wait hacks. I'm in the >>process of *removing* a plethora of them [1][2][3]. > > good riddance
Yay! >> >>I think the question is, should xe_mmio_wait32() (and the i915 >>counterpart) and the intel_de_wait*() functions be migrated to an >>interface similar to read_poll_timeout(), i.e. provide sleep and timeout >>separately, no exponential backoff. And implement the waits using >>read_poll_timeout(), or whatever Ville ends up with [4]. > > I saw your patch series and I'm eagerly waiting it to either propagate > it in xe or have someone merge such a patch. I'm not sure about > removing the exponential backoff is a good thing overall, but if it's > needed then it needs to be justified to add a new function to pair with > read_poll_timeout(), not a custom driver function. While I'm negative about the patch at hand, the underlying problem is very real. I think at the very least the exponential sleep backoff needs an upper bound that's relative to the timeout. Maybe 10-25% of timeout? With the example case of 5 second timeout, the exponential backoff starting from 10 us leads to a dozen polls before reaching 100 ms elapsed time, but then polls at approximately 1 s, 2 s, 4 s, and 8 s elapsed time. Longer timeouts are of course rare, but they exhibit increasingly worse behaviour. So if what we're waiting takes 2.1 seconds, the next check will be about 2 seconds later. Similarly, if it takes 4.1 seconds, the next check will be about 4 seconds later, in this case exceeding the timeout by 3 seconds. Anyway, if xe_mmio_wait32() remains as it is, with read_poll_timeout() it's trivial to do the wait in the intel_de_*() macros, in display side, with sleeps and timeouts defined in display. Because for most things the really quick fast polls are useless in display. BR, Jani. -- Jani Nikula, Intel
