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

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

On Mon, Nov 03, 2025 at 10:14:28AM -0800, Niranjana Vishwanathapura wrote:
> On Sat, Nov 01, 2025 at 04:41:58PM -0700, Matthew Brost wrote:
> > 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;
> > > +	}
> > 
> > I also don't love this check here as if exec queue creation order
> > changes, this code breaks. I'm pretty sure you can just delete this and
> > send on the message to the backend given
> > guc_exec_queue_allowed_to_change_state check will change the backend op to
> > a NOP.
> > 
> 
> The multi_queue_priority propertly can be set either during queue
> creation time (via multi_queue_priority extension) or later during
> set_property ioctl. This function gets called in both cases.
> 
> When this function gets called during the queue creation time (during user
> extension parsing), the guc initializaiton of the queue is not yet done
> (ie., q->ops->init() is not called yet). So, we shouldn't call the
> below q->ops->set_multi_queue_priority() function. That is what the
> above code is handling.
> 

I thinking thevfunc '->init' would have been called by the time the
extension was parsed, but I guess it isn't. I still don't love tying
this to q->xref was would be set that eariler but I guess this ok for
now.

Matt

> Niranjana
> 
> > Matt
> > 
> > > 
> > > -	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;
> > > +
> > >  	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);
> > > +	}
> > > +
> > > +	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;
> > > +
> > > +	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
> > >