[PATCH] drm/xe/display: Avoid dig_port work during suspend

[Dibin Moolakadan Subrahmanian]

On 15-07-2025 15:10, Imre Deak wrote:
> On Tue, Jul 15, 2025 at 11:22:19AM +0530, Dibin Moolakadan Subrahmanian wrote:
>>   It has been observed that during `xe_display_pm_suspend()` execution,
>>   an HPD interrupt can still be triggered, resulting in `dig_port_work`
>>   being scheduled. The issue arises when this work executes after
>>   `xe_display_pm_suspend_late()`, by which time the display is fully
>>   suspended.
>>
>>   This can lead to errors such as "DC state mismatch", as the dig_port
>>   work accesses display resources that are no longer available or
>>   powered.
>>
>>   To address this, introduce a new `ignore_dig_port` flag in the
>>   hotplug in structure. This flag is checked in the interrupt handler to
>>   prevent queuing of `dig_port_work` while the system is mid-suspend.
>>   This behavior is consistent with the existing approach of suppressing
>>   hotplug_work during suspend.
>>
>> Signed-off-by: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian at intel.com>
>> ---
>>   .../gpu/drm/i915/display/intel_display_core.h |  3 +++
>>   drivers/gpu/drm/i915/display/intel_hotplug.c  | 22 ++++++++++++++++++-
>>   drivers/gpu/drm/i915/display/intel_hotplug.h  |  2 ++
>>   drivers/gpu/drm/xe/display/xe_display.c       |  4 ++++
>>   4 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
>> index 8c226406c5cd..376682c53798 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
>> @@ -209,6 +209,9 @@ struct intel_hotplug {
>>   	 * cue to ignore the long HPDs and can be set / unset using debugfs.
>>   	 */
>>   	bool ignore_long_hpd;
>> +
>> +	/* Flag to ignore dig_port work , used in suspend*/
>> +	bool ignore_dig_port;
>>   };
>>   
>>   struct intel_vbt_data {
>> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
>> index 265aa97fcc75..b2891b7c3205 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
>> @@ -223,6 +223,26 @@ queue_detection_work(struct intel_display *display, struct work_struct *work)
>>   	return queue_work(display->wq.unordered, work);
>>   }
>>   
>> +void intel_hpd_ignore_dig_port_work(struct intel_display *display, bool value)
>> +{
>> +	if (!HAS_DISPLAY(display))
>> +		return;
>> +
>> +	spin_lock_irq(&display->irq.lock);
>> +	display->hotplug.ignore_dig_port = value;
>> +	spin_unlock_irq(&display->irq.lock);
>> +}
>> +
>> +bool intel_hpd_can_queue_dig_port(struct intel_display *display)
>> +{
>> +	if (!HAS_DISPLAY(display))
>> +		return FALSE;
>> +
>> +	lockdep_assert_held(&display->irq.lock);
>> +
>> +	return !display->hotplug.ignore_dig_port;
>> +}
>> +
>>   static void
>>   intel_hpd_irq_storm_switch_to_polling(struct intel_display *display)
>>   {
>> @@ -691,7 +711,7 @@ void intel_hpd_irq_handler(struct intel_display *display,
>>   	 * queue for otherwise the flush_work in the pageflip code will
>>   	 * deadlock.
>>   	 */
>> -	if (queue_dig)
>> +	if (queue_dig && intel_hpd_can_queue_dig_port(display))
>>   		queue_work(display->hotplug.dp_wq, &display->hotplug.dig_port_work);
>>   	if (queue_hp)
>>   		queue_delayed_detection_work(display,
>> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
>> index edc41c9d3d65..9dc40ec7074c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
>> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
>> @@ -34,5 +34,7 @@ void intel_hpd_debugfs_register(struct intel_display *display);
>>   void intel_hpd_enable_detection_work(struct intel_display *display);
>>   void intel_hpd_disable_detection_work(struct intel_display *display);
>>   bool intel_hpd_schedule_detection(struct intel_display *display);
>> +void intel_hpd_ignore_dig_port_work(struct intel_display *display, bool value);
>> +bool intel_hpd_can_queue_dig_port(struct intel_display *display);
>>   
>>   #endif /* __INTEL_HOTPLUG_H__ */
>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
>> index e2e0771cf274..2db71bd07c9f 100644
>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>> @@ -342,6 +342,8 @@ void xe_display_pm_suspend(struct xe_device *xe)
>>   
>>   	intel_hpd_cancel_work(display);
>>   
>> +	intel_hpd_ignore_dig_port_work(display, 1);
>> +
> The actual problem is that HPD IRQs are disabled too late in xe, this
> should happen already before intel_hpd_cancel_work() is called.

You're right — the HPD IRQs appear to be disabled only later via xe_irq_suspend(xe),
which is not early enough to prevent unwanted behavior during suspend.

But unlike the HPD IRQ enable path, which uses function pointers from
struct intel_hotplug_funcs, there doesn't appear to be a hook to disable IRQs early.

The proposed approach mirrors how we're already handling hotplug_work,
which is gated by the detection_work_enabled flag. This flag is cleared during suspend
by intel_display_driver_disable_user_access().

Regards,
Dibin

>>   	if (has_display(xe)) {
>>   		intel_display_driver_suspend_access(display);
>>   		intel_encoder_suspend_all(display);
>> @@ -469,6 +471,8 @@ void xe_display_pm_resume(struct xe_device *xe)
>>   	if (has_display(xe))
>>   		intel_display_driver_resume_access(display);
>>   
>> +	intel_hpd_ignore_dig_port_work(display, 0);
>> +
>>   	intel_hpd_init(display);
>>   
>>   	if (has_display(xe)) {
>> -- 
>> 2.43.0
>>