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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

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.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Arvind Yadav <arvind.yadav@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

> +	 *
> +	 * 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
>