Re: [PATCH 08/13] drm/xe/irq: Serialize IRQ suspend and resume

[Yadav, Arvind]

On 01-09-2026 02:36, Rodrigo Vivi wrote:
On Thu, Aug 27, 2026 at 03:47:56PM +0530, Arvind Yadav wrote:
Wedge isolation suspends interrupts from a worker. This can race with
PM resume and allow resume to enable interrupts after isolation has
disabled them.

Add a managed mutex around IRQ suspend and resume. Check the wedged state
while holding the mutex so either ordering leaves interrupts disabled.

Cc: Matthew Brost <matthew.br...@intel.com>
Cc: Thomas Hellström <thomas.hellst...@linux.intel.com>
Cc: Himal Prasad Ghimiray <himal.prasad.ghimi...@intel.com>
Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Arvind Yadav <arvind.ya...@intel.com>
---
  drivers/gpu/drm/xe/xe_device_types.h |  4 ++++
  drivers/gpu/drm/xe/xe_irq.c          | 26 +++++++++++++++++++++++---
  2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_device_types.h 
b/drivers/gpu/drm/xe/xe_device_types.h
index 43a86564adf0..dda4d9919ca6 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -6,6 +6,7 @@
  #ifndef _XE_DEVICE_TYPES_H_
  #define _XE_DEVICE_TYPES_H_
  
+#include <linux/mutex.h>
  #include <linux/pci.h>
  #include <linux/srcu.h>
  
@@ -266,6 +267,9 @@ struct xe_device {
                /** @irq.lock: lock for processing irq's on this device */
                spinlock_t lock;
  
+		/** @irq.pm_lock: Serializes IRQ suspend and resume */
+               struct mutex pm_lock;
+
                /** @irq.enabled: interrupts enabled on this device */
                atomic_t enabled;
  
diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index d314993b14a1..9cdcb16f3ca8 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -797,8 +797,14 @@ static void irq_uninstall(void *arg)
  
  int xe_irq_init(struct xe_device *xe)
  {
+       int err;
+
        spin_lock_init(&xe->irq.lock);
  
+	err = drmm_mutex_init(&xe->drm, &xe->irq.pm_lock);
+       if (err)
+               return err;
+
        return xe_irq_msix_init(xe);
  }
  
@@ -843,6 +849,8 @@ static void xe_irq_msi_synchronize_irq(struct xe_device *xe)
  
  void xe_irq_suspend(struct xe_device *xe)
  {
+       mutex_lock(&xe->irq.pm_lock);
+
        atomic_set(&xe->irq.enabled, 0); /* no new irqs */
  
  	/* flush irqs */
@@ -851,6 +859,8 @@ void xe_irq_suspend(struct xe_device *xe)
        else
                xe_irq_msi_synchronize_irq(xe);
        xe_irq_reset(xe); /* turn irqs off */
+
+       mutex_unlock(&xe->irq.pm_lock);
  }
  
  void xe_irq_resume(struct xe_device *xe)
@@ -858,10 +868,17 @@ void xe_irq_resume(struct xe_device *xe)
        struct xe_gt *gt;
        int id;
  
+	mutex_lock(&xe->irq.pm_lock);
+
+       if (xe_device_wedged(xe))
+               goto out_unlock;
+
        /*
-        * lock not needed:
-        * 1. no irq will arrive before the postinstall
-        * 2. display is not yet resumed
+        * pm_lock serializes resume against wedge isolation.
if we are using mutex to serialize code we are doing it wrong.
worth reading:  https://blog.ffwll.ch/2022/08/locking-hierarchy.html


Thanks for pointing this out.  The mutex protects the state around 
irq.enabled, not the suspend/resume functions.
I guess the current mutex name and commit message do not make this 
clear. So I will  rename it to state_lock and update the comments and 
commit message to make this clearer.

Thanks,
Arvind


+        *
+        * irq.lock is not needed because:
+        * 1. no IRQ arrives before postinstall;
+        * 2. display has not been resumed yet.
         */
        atomic_set(&xe->irq.enabled, 1);
        xe_irq_reset(xe);
@@ -869,6 +886,9 @@ void xe_irq_resume(struct xe_device *xe)
  
  	for_each_gt(gt, xe, id)
                xe_irq_enable_hwe(gt);
+
+out_unlock:
+       mutex_unlock(&xe->irq.pm_lock);
  }
  
  /* MSI-X related definitions and functions below. */
--
2.43.0