Re: [PATCH 07/16] drm/xe/multi_queue: Add support for multi queue dynamic priority change

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

On Fri, Oct 31, 2025 at 11:29:27AM -0700, Niranjana Vishwanathapura wrote:
> Support dynamic priority change for multi queue group queues via
> exec queue set_property ioctl. Issue CGP_SYNC command to GuC through
> the drm scheduler message interface for priority to take effect.
> 
> Signed-off-by: Pallavi Mishra <pallavi.mishra@intel.com>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_exec_queue.c       | 12 ++++-
>  drivers/gpu/drm/xe/xe_exec_queue_types.h |  3 ++
>  drivers/gpu/drm/xe/xe_guc_submit.c       | 56 ++++++++++++++++++++++--
>  3 files changed, 65 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index 0264cab00fd4..98f8f1c7f13b 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -729,9 +729,13 @@ static int exec_queue_set_multi_queue_priority(struct xe_device *xe, struct xe_e
>  	if (XE_IOCTL_DBG(xe, value > XE_MULTI_QUEUE_PRIORITY_HIGH))
>  		return -EINVAL;
>  
> -	q->multi_queue.priority = value;
> +	/* For queue creation time (!q->xef) setting, just store the priority value */
> +	if (!q->xef) {
> +		q->multi_queue.priority = value;
> +		return 0;
> +	}
>  
> -	return 0;
> +	return q->ops->set_multi_queue_priority(q, value);
>  }
>  
>  typedef int (*xe_exec_queue_set_property_fn)(struct xe_device *xe,
> @@ -760,6 +764,10 @@ int xe_exec_queue_set_property_ioctl(struct drm_device *dev, void *data,
>  	if (XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
>  		return -EINVAL;
>  
> +	if (XE_IOCTL_DBG(xe, args->property !=
> +			 DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY))
> +		return -EINVAL;

This check looks to be in the wrong place. There are other valid
properties, e.g., DRM_XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE, etc...

I think CI is complaining about this too.

> +
>  	q = xe_exec_queue_lookup(xef, args->exec_queue_id);
>  	if (XE_IOCTL_DBG(xe, !q))
>  		return -ENOENT;
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> index 964a0e6654c7..dcb55b069ed8 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue_types.h
> +++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> @@ -241,6 +241,9 @@ struct xe_exec_queue_ops {
>  	int (*set_timeslice)(struct xe_exec_queue *q, u32 timeslice_us);
>  	/** @set_preempt_timeout: Set preemption timeout for exec queue */
>  	int (*set_preempt_timeout)(struct xe_exec_queue *q, u32 preempt_timeout_us);
> +	/** @set_multi_queue_priority: Set multi queue priority */
> +	int (*set_multi_queue_priority)(struct xe_exec_queue *q,
> +					enum xe_multi_queue_priority priority);
>  	/**
>  	 * @suspend: Suspend exec queue from executing, allowed to be called
>  	 * multiple times in a row before resume with the caveat that
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 5ec144c1c2dc..426b64ef8d99 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1761,10 +1761,32 @@ static void __guc_exec_queue_process_msg_resume(struct xe_sched_msg *msg)
>  	}
>  }
>  
> -#define CLEANUP		1	/* Non-zero values to catch uninitialized msg */
> -#define SET_SCHED_PROPS	2
> -#define SUSPEND		3
> -#define RESUME		4
> +static void __guc_exec_queue_process_msg_set_multi_queue_priority(struct xe_sched_msg *msg)
> +{
> +	struct xe_exec_queue *q = msg->private_data;
> +
> +	if (guc_exec_queue_allowed_to_change_state(q)) {
> +#define MAX_MULTI_QUEUE_REG_SIZE        (2)
> +		struct xe_guc *guc = exec_queue_to_guc(q);
> +		struct xe_exec_queue_group *group = q->multi_queue.group;
> +		u32 action[MAX_MULTI_QUEUE_REG_SIZE];
> +		int len = 0;
> +
> +		action[len++] = XE_GUC_ACTION_MULTI_QUEUE_CONTEXT_CGP_SYNC;
> +		action[len++] = group->primary->guc->id;
> +#undef MAX_MULTI_QUEUE_REG_SIZE
> +
> +		xe_guc_exec_queue_group_cgp_sync(guc, q, action, len);

Just to confirm - even though multi-group priority isn't part of CGP
rathe the LRC, a CGP_SYNC is needed?

> +	}
> +
> +	kfree(msg);
> +}
> +
> +#define CLEANUP				1	/* Non-zero values to catch uninitialized msg */
> +#define SET_SCHED_PROPS			2
> +#define SUSPEND				3
> +#define RESUME				4
> +#define SET_MULTI_QUEUE_PRIORITY	5
>  #define OPCODE_MASK	0xf
>  #define MSG_LOCKED	BIT(8)
>  #define MSG_HEAD	BIT(9)
> @@ -1788,6 +1810,9 @@ static void guc_exec_queue_process_msg(struct xe_sched_msg *msg)
>  	case RESUME:
>  		__guc_exec_queue_process_msg_resume(msg);
>  		break;
> +	case SET_MULTI_QUEUE_PRIORITY:
> +		__guc_exec_queue_process_msg_set_multi_queue_priority(msg);
> +		break;
>  	default:
>  		XE_WARN_ON("Unknown message type");
>  	}
> @@ -2004,6 +2029,28 @@ static int guc_exec_queue_set_preempt_timeout(struct xe_exec_queue *q,
>  	return 0;
>  }
>  
> +static int guc_exec_queue_set_multi_queue_priority(struct xe_exec_queue *q,
> +						   enum xe_multi_queue_priority priority)
> +{
> +	struct xe_sched_msg *msg;
> +
> +	if (!xe_exec_queue_is_multi_queue(q))
> +		return -EINVAL;

Minor nit, I'd move this check to the caller as
xe_exec_queue_is_multi_queue is available there, then assert it is a
xe_exec_queue_is_multi_queue here.

Matt

> +
> +	if (q->multi_queue.priority == priority ||
> +	    exec_queue_killed_or_banned_or_wedged(q))
> +		return 0;
> +
> +	msg = kmalloc(sizeof(*msg), GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	q->multi_queue.priority = priority;
> +	guc_exec_queue_add_msg(q, msg, SET_MULTI_QUEUE_PRIORITY);
> +
> +	return 0;
> +}
> +
>  static int guc_exec_queue_suspend(struct xe_exec_queue *q)
>  {
>  	struct xe_gpu_scheduler *sched = &q->guc->sched;
> @@ -2095,6 +2142,7 @@ static const struct xe_exec_queue_ops guc_exec_queue_ops = {
>  	.set_priority = guc_exec_queue_set_priority,
>  	.set_timeslice = guc_exec_queue_set_timeslice,
>  	.set_preempt_timeout = guc_exec_queue_set_preempt_timeout,
> +	.set_multi_queue_priority = guc_exec_queue_set_multi_queue_priority,
>  	.suspend = guc_exec_queue_suspend,
>  	.suspend_wait = guc_exec_queue_suspend_wait,
>  	.resume = guc_exec_queue_resume,
> -- 
> 2.43.0
>