RE: [PATCH v1] drm/i915/aux: use polling when irqs are unavailable

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

On Sun, 29 Mar 2026, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Grzelak, Michal <michal.grzelak@intel.com>
>> Sent: Saturday, March 28, 2026 9:22 PM
>> To: Murthy, Arun R <arun.r.murthy@intel.com>
>> Cc: Grzelak, Michal <michal.grzelak@intel.com>; intel-xe@lists.freedesktop.org;
>> intel-gfx@lists.freedesktop.org; Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Subject: RE: [PATCH v1] drm/i915/aux: use polling when irqs are unavailable
>> 
>> On Tue, 24 Mar 2026, Murthy, Arun R wrote:
>> >
>> >> -----Original Message-----
>> >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>> >> Of Michał Grzelak
>> >> Sent: Tuesday, February 10, 2026 4:50 PM
>> >> To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
>> >> Cc: Grzelak, Michal <michal.grzelak@intel.com>; Ville Syrjälä
>> >> <ville.syrjala@linux.intel.com>
>> >> Subject: [PATCH v1] drm/i915/aux: use polling when irqs are
>> >> unavailable
>> >>
>> >> PTL with physically disconnected display was observed to have 40s
>> >> longer execution time when testing
>> xe_fault_injection@xe_guc_mmio_send_recv.
>> >> The issue has not been seen when reverting commit 40a9f77a28fa
>> >> ("Revert
>> >> "drm/i915/dp: change aux_ctl reg read to polling read"").
>> >>
>> >> Apparently the configuration suffers from not having AUX enabled when
>> >> using interrupts. One probable cause can be xe enabling interrupts
>> >> too
>> >> late: interrupts need memory allocations which currently can't be
>> >> done before the display FB takeover is done.
>> >>
>> >> As for now, use polling for AUX in case interrupts are unavailable.
>> >>
>> >> Fixes: 40a9f77a28fa ("Revert "drm/i915/dp: change aux_ctl reg read to
>> >> polling
>> >> read"")
>> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_dp_aux.c | 20
>> >> ++++++++++++++++----
>> >>  1 file changed, 16 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> >> b/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> >> index b20ec3e589fad..9c9b6410366d5 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> >> @@ -12,6 +12,7 @@
>> >>  #include "intel_dp.h"
>> >>  #include "intel_dp_aux.h"
>> >>  #include "intel_dp_aux_regs.h"
>> >> +#include "intel_parent.h"
>> >>  #include "intel_pps.h"
>> >>  #include "intel_quirks.h"
>> >>  #include "intel_tc.h"
>> >> @@ -60,18 +61,29 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>> >>  	struct intel_display *display = to_intel_display(intel_dp);
>> >>  	i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
>> >>  	const unsigned int timeout_ms = 10;
>> >> +	bool done = true;
>> >>  	u32 status;
>> >> -	bool done;
>> >> +	int ret;
>> >>
>> >> +	if (intel_parent_irq_enabled(display)) {
>> >>  #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));
>> >> +		done = wait_event_timeout(display->gmbus.wait_queue, C,
>> >> +
>> > Wonder if this is a corner/error case, as to how interrupts are disabled.
>> > Rather I feel should find out why interrupts are being disabled, if this would
>> be a valid scenario then in most of the places where wait_event_timeout() is
>> used in drm we should check if parent_irq is enabled! In that case, can this be a
>> workaround ?
>> 
>> I'm not sure I get what you mean. Can you elaborate more on what you would
>> like to see?
>> 
> Having interrupts disabled for such a long time is an issue and will have to identify that.
> This work around would definitely help in those scenarios and a good to have in driver.
>
>> My suspicion is that you are asking to
>> s/wait_event_timeout()/<this patch> in the whole drm directory. If that's the
>> case, I don't know if it is really neccessary since it is AUX-only related issue.
> Why is it only AUX related. For AUX Tx we are relaying on interrupts and getting this interrupt is the issue over here due to parent interrupt being disabled.
> Similarly in other places where waitqueues are used in i915, can have this WA to make the driver more robust.

Please analyze and root cause before even suggesting changes like that.

BR,
Jani.


>
> Thanks and Regards,
> Arun R Murthy
> -------------------- 
>
>> Could you put some rationale behind it?
>> 
>> BR,
>> Michał
>> 
>> >
>> > Thanks and Regards,
>> > Arun R Murthy
>> > --------------------
>> >> msecs_to_jiffies_timeout(timeout_ms));
>> >> +
>> >> +#undef C
>> >> +	} else {
>> >> +		ret = intel_de_wait_ms(display, ch_ctl,
>> >> +				       DP_AUX_CH_CTL_SEND_BUSY, 0,
>> >> +				       timeout_ms, &status);
>> >> +
>> >> +		if (ret == -ETIMEDOUT)
>> >> +			done = false;
>> >> +	}
>> >>
>> >>  	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.45.2
>> >
>> >

-- 
Jani Nikula, Intel