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

["Michał Grzelak" <michal.grzelak@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3785 bytes --]

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?

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. 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
>
>