Re: [PATCH v1 2/4] drm/xe/sysctrl: Add system controller interrupt handler

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Jan 16, 2026 at 03:03:31PM +0530, Raag Jadav wrote:
> Add system controller interrupt handler which is denoted by 11th bit in
> GFX master interrupt register. While at it, add ordered workqueue for
> scheduling system controller work.
> 
> Co-developed-by: Soham Purkait <soham.purkait@intel.com>
> Signed-off-by: Soham Purkait <soham.purkait@intel.com>
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_irq_regs.h |  1 +
>  drivers/gpu/drm/xe/xe_irq.c           |  2 ++
>  drivers/gpu/drm/xe/xe_sysctrl.c       | 39 +++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_sysctrl.h       |  3 +++
>  drivers/gpu/drm/xe/xe_sysctrl_types.h |  7 +++++
>  5 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_irq_regs.h b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> index 9d74f454d3ff..1d6b976c4de0 100644
> --- a/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> @@ -22,6 +22,7 @@
>  #define   DISPLAY_IRQ				REG_BIT(16)
>  #define   SOC_H2DMEMINT_IRQ			REG_BIT(13)
>  #define   I2C_IRQ				REG_BIT(12)
> +#define   SYSCTRL_IRQ				REG_BIT(11)
>  #define   GT_DW_IRQ(x)				REG_BIT(x)
>  
>  /*
> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> index 7560a45f7f64..9e49e2241da4 100644
> --- a/drivers/gpu/drm/xe/xe_irq.c
> +++ b/drivers/gpu/drm/xe/xe_irq.c
> @@ -24,6 +24,7 @@
>  #include "xe_mmio.h"
>  #include "xe_pxp.h"
>  #include "xe_sriov.h"
> +#include "xe_sysctrl.h"
>  #include "xe_tile.h"
>  
>  /*
> @@ -525,6 +526,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
>  				xe_heci_csc_irq_handler(xe, master_ctl);
>  			xe_display_irq_handler(xe, master_ctl);
>  			xe_i2c_irq_handler(xe, master_ctl);
> +			xe_sysctrl_irq_handler(xe, master_ctl);
>  			xe_mert_irq_handler(xe, master_ctl);
>  			gu_misc_iir = gu_misc_irq_ack(xe, master_ctl);
>  		}
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c b/drivers/gpu/drm/xe/xe_sysctrl.c
> index 8daab7703247..1d78916dd6ad 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl.c
> @@ -7,6 +7,7 @@
>  #include <linux/device.h>
>  #include <linux/mutex.h>
>  
> +#include "regs/xe_irq_regs.h"
>  #include "regs/xe_sysctrl_regs.h"
>  #include "xe_device.h"
>  #include "xe_printk.h"
> @@ -27,9 +28,17 @@
>   * with the System Controller through the mailbox.
>   */
>  
> +static void xe_sysctrl_work(struct work_struct *work)
> +{
> +}
> +
>  static void xe_sysctrl_fini(void *arg)
>  {
>  	struct xe_device *xe = arg;
> +	struct xe_sysctrl *sc = &xe->sc;
> +
> +	cancel_work_sync(&sc->work);
> +	destroy_workqueue(sc->wq);
>  
>  	xe->soc_remapper.set_sysctrl_region(xe, 0);
>  }
> @@ -56,6 +65,14 @@ int xe_sysctrl_init(struct xe_device *xe)
>  
>  	xe->soc_remapper.set_sysctrl_region(xe, SYSCTRL_MAILBOX_INDEX);
>  
> +	INIT_WORK(&sc->work, xe_sysctrl_work);
> +
> +	sc->wq = alloc_ordered_workqueue("sysctrl-ordered-wq", 0);

We do have drmm_alloc_ordered_workqueue, so you don’t need to manually
clean up the WQ. We don’t use it as widely as we should in Xe, as it was
added after we merged, but we should use that helper going forward.

I do have a question—does this require a dedicated WQ though? i.e.,
would a system WQ work here? We allocate a lot of WQs in Xe, but
typically that’s done when we really care about ordering. For example,
we may have multiple work_items scheduled on a WQ but want only one of
them executing at any given time, so we schedule them on a single
dedicated WQ. Here you only have one work_item, so I believe you can
probably just use one of the system WQs.

Matt 

> +	if (!sc->wq) {
> +		ret = -ENOMEM;
> +		goto err_sysctrl_fini;
> +	}
> +
>  	ret = devm_add_action_or_reset(xe->drm.dev, xe_sysctrl_fini, xe);
>  	if (ret)
>  		return ret;
> @@ -67,4 +84,26 @@ int xe_sysctrl_init(struct xe_device *xe)
>  	xe_sysctrl_mailbox_init(sc);
>  
>  	return 0;
> +
> +err_sysctrl_fini:
> +	xe_sysctrl_fini(xe);
> +	return ret;
> +}
> +
> +/**
> + * xe_sysctrl_irq_handler: Handler for System Controller interrupts
> + * @xe: xe device instance
> + * @master_ctl: interrupt register
> + *
> + * Handle interrupts generated by System Controller.
> + */
> +void xe_sysctrl_irq_handler(struct xe_device *xe, u32 master_ctl)
> +{
> +	struct xe_sysctrl *sc = &xe->sc;
> +
> +	if (!xe->info.has_sysctrl)
> +		return;
> +
> +	if (master_ctl & SYSCTRL_IRQ)
> +		queue_work(sc->wq, &sc->work);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h b/drivers/gpu/drm/xe/xe_sysctrl.h
> index ee7826fe4c98..5919310b9db9 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl.h
> +++ b/drivers/gpu/drm/xe/xe_sysctrl.h
> @@ -6,8 +6,11 @@
>  #ifndef _XE_SYSCTRL_H_
>  #define _XE_SYSCTRL_H_
>  
> +#include <linux/types.h>
> +
>  struct xe_device;
>  
>  int xe_sysctrl_init(struct xe_device *xe);
> +void xe_sysctrl_irq_handler(struct xe_device *xe, u32 master_ctl);
>  
>  #endif /* _XE_SYSCTRL_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_types.h b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> index 88a34967688b..14fc80dfee6e 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_types.h
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> @@ -8,6 +8,7 @@
>  
>  #include <linux/mutex.h>
>  #include <linux/types.h>
> +#include <linux/workqueue_types.h>
>  
>  /**
>   * struct xe_sysctrl - System Controller driver context
> @@ -18,6 +19,12 @@ struct xe_sysctrl {
>  
>  	/** @phase_bit: MKHI message boundary phase toggle bit */
>  	u32 phase_bit;
> +
> +	/** @wq: Queue for sysctrl work */
> +	struct workqueue_struct *wq;
> +
> +	/** @work: Worker for pending events */
> +	struct work_struct work;
>  };
>  
>  #endif /* _XE_SYSCTRL_TYPES_H_ */
> -- 
> 2.43.0
>