Re: [CI 4/4] drm/xe/uapi: Support requesting unique MSI-X for exec queue

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

On Fri, Dec 13, 2024 at 09:03:53AM +0200, Ilia Levi wrote:
> From: Dani Liberman <dliberman@habana.ai>
> 
> Unique MSI-X per exec queue will improve the performance of the
> IRQ handler. In case no MSI-X is available, the uAPI will return
> -EBUSY error and the user would be able to execute the uAPI again
> without the flag (fallback to default MSI-X).

Patches 1, 2 and 3 pushed to drm-xe-next. This patch here is on
hold while we don't have the open source userspace for that ready

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements

Thanks,
Rodrigo.

> 
> Co-developed-by: Ilia Levi <ilia.levi@intel.com>
> Signed-off-by: Ilia Levi <ilia.levi@intel.com>
> Signed-off-by: Dani Liberman <dliberman@habana.ai>
> Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_exec_queue.c       | 57 +++++++++++++++++++++---
>  drivers/gpu/drm/xe/xe_exec_queue_types.h |  2 +
>  drivers/gpu/drm/xe/xe_irq.c              | 18 ++++++++
>  drivers/gpu/drm/xe/xe_irq.h              |  1 +
>  include/uapi/drm/xe_drm.h                |  8 +++-
>  5 files changed, 80 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index 9c94be571900..35ef75f9919f 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -12,6 +12,7 @@
>  #include <uapi/drm/xe_drm.h>
>  
>  #include "xe_device.h"
> +#include "xe_drv.h"
>  #include "xe_gt.h"
>  #include "xe_hw_engine_class_sysfs.h"
>  #include "xe_hw_engine_group.h"
> @@ -35,8 +36,46 @@ enum xe_exec_queue_sched_prop {
>  static int exec_queue_user_extensions(struct xe_device *xe, struct xe_exec_queue *q,
>  				      u64 extensions, int ext_number);
>  
> +static int xe_exec_queue_msix_init(struct xe_device *xe, struct xe_exec_queue *q, bool unique_msix)
> +{
> +	u16 msix;
> +	int err;
> +
> +	if (!xe_device_has_msix(xe))
> +		return 0;
> +
> +	if (!unique_msix) {
> +		q->msix_vec = XE_IRQ_DEFAULT_MSIX;
> +		return 0;
> +	}
> +
> +	err = xe_irq_msix_request_irq(xe, xe_irq_msix_hwe_handler, q,
> +				      DRIVER_NAME "-exec-queue", true, &msix);
> +	if (err < 0) {
> +		drm_dbg(&xe->drm, "Can't allocate unique MSI-X to exec queue (%d)\n", err);
> +		return err;
> +	}
> +
> +	q->msix_vec = msix;
> +
> +	return 0;
> +}
> +
> +static void xe_exec_queue_msix_fini(struct xe_exec_queue *q)
> +{
> +	struct xe_device *xe = gt_to_xe(q->gt);
> +
> +	if (!xe_device_has_msix(xe))
> +		return;
> +
> +	if (q->msix_vec && q->msix_vec != XE_IRQ_DEFAULT_MSIX)
> +		xe_irq_msix_free_irq(xe, q->msix_vec);
> +}
> +
>  static void __xe_exec_queue_free(struct xe_exec_queue *q)
>  {
> +	xe_exec_queue_msix_fini(q);
> +
>  	if (q->vm)
>  		xe_vm_put(q->vm);
>  
> @@ -69,7 +108,12 @@ static struct xe_exec_queue *__xe_exec_queue_alloc(struct xe_device *xe,
>  	q->gt = gt;
>  	q->class = hwe->class;
>  	q->width = width;
> -	q->msix_vec = XE_IRQ_DEFAULT_MSIX;
> +	err = xe_exec_queue_msix_init(xe, q, flags & EXEC_QUEUE_FLAG_UNIQUE_MSIX);
> +	if (err) {
> +		kfree(q);
> +		return ERR_PTR(err);
> +	}
> +
>  	q->logical_mask = logical_mask;
>  	q->fence_irq = &gt->fence_irq[hwe->class];
>  	q->ring_ops = gt->ring_ops[hwe->class];
> @@ -547,13 +591,13 @@ int xe_exec_queue_create_ioctl(struct drm_device *dev, void *data,
>  	struct xe_gt *gt;
>  	struct xe_tile *tile;
>  	struct xe_exec_queue *q = NULL;
> +	u32 flags = 0;
>  	u32 logical_mask;
>  	u32 id;
>  	u32 len;
>  	int err;
>  
> -	if (XE_IOCTL_DBG(xe, args->flags) ||
> -	    XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
> +	if (XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
>  		return -EINVAL;
>  
>  	len = args->width * args->num_placements;
> @@ -569,6 +613,9 @@ int xe_exec_queue_create_ioctl(struct drm_device *dev, void *data,
>  	if (XE_IOCTL_DBG(xe, eci[0].gt_id >= xe->info.gt_count))
>  		return -EINVAL;
>  
> +	if (args->flags & DRM_XE_EXEC_QUEUE_CREATE_FLAG_UNIQUE_INTERRUPT_HINT)
> +		flags |= EXEC_QUEUE_FLAG_UNIQUE_MSIX;
> +
>  	if (eci[0].engine_class == DRM_XE_ENGINE_CLASS_VM_BIND) {
>  		if (XE_IOCTL_DBG(xe, args->width != 1) ||
>  		    XE_IOCTL_DBG(xe, args->num_placements != 1) ||
> @@ -577,8 +624,8 @@ int xe_exec_queue_create_ioctl(struct drm_device *dev, void *data,
>  
>  		for_each_tile(tile, xe, id) {
>  			struct xe_exec_queue *new;
> -			u32 flags = EXEC_QUEUE_FLAG_VM;
>  
> +			flags |= EXEC_QUEUE_FLAG_VM;
>  			if (id)
>  				flags |= EXEC_QUEUE_FLAG_BIND_ENGINE_CHILD;
>  
> @@ -625,7 +672,7 @@ int xe_exec_queue_create_ioctl(struct drm_device *dev, void *data,
>  		}
>  
>  		q = xe_exec_queue_create(xe, vm, logical_mask,
> -					 args->width, hwe, 0,
> +					 args->width, hwe, flags,
>  					 args->extensions);
>  		up_read(&vm->lock);
>  		xe_vm_put(vm);
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> index eec8f9935a58..6e419b572000 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue_types.h
> +++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> @@ -85,6 +85,8 @@ struct xe_exec_queue {
>  #define EXEC_QUEUE_FLAG_BIND_ENGINE_CHILD	BIT(3)
>  /* kernel exec_queue only, set priority to highest level */
>  #define EXEC_QUEUE_FLAG_HIGH_PRIORITY		BIT(4)
> +/* queue with unique msix interrupt */
> +#define EXEC_QUEUE_FLAG_UNIQUE_MSIX		BIT(5)
>  
>  	/**
>  	 * @flags: flags for this exec queue, should statically setup aside from ban
> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> index 32f5a67a917b..52829771c89b 100644
> --- a/drivers/gpu/drm/xe/xe_irq.c
> +++ b/drivers/gpu/drm/xe/xe_irq.c
> @@ -14,6 +14,7 @@
>  #include "regs/xe_irq_regs.h"
>  #include "xe_device.h"
>  #include "xe_drv.h"
> +#include "xe_exec_queue_types.h"
>  #include "xe_gsc_proxy.h"
>  #include "xe_gt.h"
>  #include "xe_guc.h"
> @@ -880,6 +881,23 @@ static irqreturn_t xe_irq_msix_default_hwe_handler(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> +/*
> + * In MSI-X mode command streamers raise an interrupt only as a result of
> + * MI_USER_INTERRUPT and MI_FLUSH_DW_NOTIFY commands.
> + */
> +irqreturn_t xe_irq_msix_hwe_handler(int irq, void *arg)
> +{
> +	struct xe_exec_queue *q = arg;
> +	struct xe_tile *tile = gt_to_tile(q->hwe->gt);
> +
> +	if (!atomic_read(&tile->xe->irq.enabled))
> +		return IRQ_NONE;
> +
> +	xe_memirq_hwe_handler(&tile->memirq, q->hwe);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static int xe_irq_msix_alloc_vector(struct xe_device *xe, void *irq_buf,
>  				    bool dynamic_msix, u16 *msix)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_irq.h b/drivers/gpu/drm/xe/xe_irq.h
> index a28bd577ba52..47058e2d9d36 100644
> --- a/drivers/gpu/drm/xe/xe_irq.h
> +++ b/drivers/gpu/drm/xe/xe_irq.h
> @@ -19,6 +19,7 @@ int xe_irq_install(struct xe_device *xe);
>  void xe_irq_suspend(struct xe_device *xe);
>  void xe_irq_resume(struct xe_device *xe);
>  void xe_irq_enable_hwe(struct xe_gt *gt);
> +irqreturn_t xe_irq_msix_hwe_handler(int irq, void *arg);
>  int xe_irq_msix_request_irq(struct xe_device *xe, irq_handler_t handler, void *irq_buf,
>  			    const char *name, bool dynamic_msix, u16 *msix);
>  void xe_irq_msix_free_irq(struct xe_device *xe, u16 msix);
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 0383b52cbd86..d5e824355a5b 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -1123,7 +1123,13 @@ struct drm_xe_exec_queue_create {
>  	/** @vm_id: VM to use for this exec queue */
>  	__u32 vm_id;
>  
> -	/** @flags: MBZ */
> +	/*
> +	 * When creating exec queue in MSIX platforms, the user can request a unique MSIX interrupt
> +	 * for the irq handler. If there is no available MSIX, -EBUSY will be returned.
> +	 */
> +#define	DRM_XE_EXEC_QUEUE_CREATE_FLAG_UNIQUE_INTERRUPT_HINT (0x1 << 0)
> +
> +	/** @flags: create queue flags */
>  	__u32 flags;
>  
>  	/** @exec_queue_id: Returned exec queue ID */
> -- 
> 2.43.2
>