On 15-07-2025 20:55, Imre Deak wrote:
On Tue, Jul 15, 2025 at 06:38:15PM +0530, Dibin Moolakadan Subrahmanian wrote:
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
<[email protected]>
---
.../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().
The work should not run after intel_hpd_cancel_work() returns. To ensure
that it's the IRQs scheduling the work which should be disabled, before
intel_hpd_cancel_work() is called. For now, calling intel_hpd_block() on
all the encoders would be enough for that. intel_hpd_cancel_work() will
see then blocked IRQs, so the WARN about that should be removed from it.
Thanks for the suggestion , I will modify the patch accordingly.
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
