On Thu, 20 Nov 2025, "Murthy, Arun R" <[email protected]> wrote: >> -----Original Message----- >> From: Intel-gfx <[email protected]> On Behalf Of Ville >> Syrjala >> Sent: Thursday, November 20, 2025 12:23 AM >> To: [email protected] >> Cc: [email protected] >> Subject: [PATCH 2/2] Revert "drm/i915/dp: change aux_ctl reg read to polling >> read" >> >> From: Ville Syrjälä <[email protected]> >> >> This reverts commit 5a9b0c7418448ed3766f61ba0a71d08f259c3181. >> >> The switch from AUX interrupts to pollign was very hand-wavy. >> Yes, there have been some situations in CI on a few platforms where the AUX >> hardware seemingly forgets to signal the timeout, but those have been >> happening after we switched to polling as well. So I don't think we have any >> conclusive evidence that polling actually helps here. >> >> Someone really should root cause the actual problem, and see if there is a >> proper workaround we could implemnt (eg. disabling clock gating/etc.). In the >> meantime just go back to using the interrupt for AUX completion. >> >> If the hardware fails to signal the timeout we will just hit the >> wait_event_timeout() software timeout instead. I suppose we could try to tune >> the software timeout to more closely match the expected hardware timeout. >> Might need to use >> wait_event_hrtimeout() or something to avoid jiffies granularity issues... >> >> The AUX polling is also a hinderance towards using poll_timeout_us() because >> we have a very long timeout, but would need a fairly short polling interval >> to >> keep AUX transfer reasonably fast. Someone would need to come up with good >> numbers in a somewhat scientific way. >> > Upon multiple rounds of validation based on the results polling had > improvements when compared with the interrupt mechanism. We can optimize more > by using poll_timeout, I am afraid that reverting back to interrupts may end > up with more failures.
I'm not sure the issues were root caused properly. BR, Jani. > > Thanks and Regards, > Arun R Murthy > -------------------- >> Signed-off-by: Ville Syrjälä <[email protected]> >> --- >> drivers/gpu/drm/i915/display/intel_dp_aux.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c >> b/drivers/gpu/drm/i915/display/intel_dp_aux.c >> index 809799f63e32..d1a93e4a59b5 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c >> @@ -6,6 +6,7 @@ >> #include <drm/drm_print.h> >> >> #include "intel_de.h" >> +#include "intel_display_jiffies.h" >> #include "intel_display_types.h" >> #include "intel_display_utils.h" >> #include "intel_dp.h" >> @@ -60,16 +61,17 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp) >> i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp); >> const unsigned int timeout_ms = 10; >> u32 status; >> - int ret; >> + bool done; >> >> - ret = intel_de_wait_ms(display, ch_ctl, >> - DP_AUX_CH_CTL_SEND_BUSY, 0, >> - timeout_ms, &status); >> +#define C (((status = intel_de_read_notrace(display, ch_ctl)) & >> DP_AUX_CH_CTL_SEND_BUSY) == 0) >> + done = wait_event_timeout(display->gmbus.wait_queue, C, >> + msecs_to_jiffies_timeout(timeout_ms)); >> >> - if (ret == -ETIMEDOUT) >> + if (!done) >> drm_err(display->drm, >> "%s: did not complete or timeout within %ums (status >> 0x%08x)\n", >> intel_dp->aux.name, timeout_ms, status); >> +#undef C >> >> return status; >> } >> -- >> 2.49.1 > -- Jani Nikula, Intel
