Re: [RFC 4/8] drm/xe: Convert to per gt scheduler workers

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

On Thu, Jul 09, 2026 at 12:35:43PM +0100, Tvrtko Ursulin wrote:
> 
> On 08/07/2026 20:32, Matthew Brost wrote:
> > On Wed, Jul 08, 2026 at 05:35:46PM +0100, Tvrtko Ursulin wrote:
> > > 
> > > On 07/07/2026 00:14, Matthew Brost wrote:
> > > > On Mon, Jul 06, 2026 at 01:27:46PM +0100, Tvrtko Ursulin wrote:
> > > > > 
> > > > > 
> > > > > On 03/07/2026 10:06, Matthew Brost wrote:
> > > > > > On Thu, Jul 02, 2026 at 03:37:41PM +0100, Tvrtko Ursulin wrote:
> > > > > > > As the submission side of xe is serialized by the global GuC CT lock, we
> > > > 
> > > > Profiling shows that, without backpressure on the firmware interface
> > > > channels, guc_send_ct throughput is about 4 H2G messages per
> > > > microsecond. As such, the serialization here is very likely just a
> > > > spin-wait on a short critical section.
> > > > 
> > > > The work items also perform work outside the CT lock, but in practice,
> > > > for the non-waiting CT-lock cases, it may not matter whether we use a
> > > > single workqueue or multiple workqueues give the slowest point is likely
> > > > the GuC dequeuing H2G.
> > > 
> > > Yes, and how fast is the GuC MCU vs how many main CPU threads is realistic
> > > to spawn to feed it. The number is maybe not one, but maybe it is also not
> > > that many more. I know we got to this point because it was the easy way, and
> > > I think it is also okay to do something completely different like split the
> > > scheduler better or write a new one. Although both those options need buy-in
> > > from the DRM scheduler and DRM maintainers. End goal is simply to provide a
> > > smooth UI to users.
> > > 
> > > > We're also looking at doing ULLS submission in the KMD, in which case
> > > > there would be no serialization at all. Likewise, if we created
> > > > doorbells, we would not need to serialize either.
> > > > 
> > > > This series seems like backing Xe design into a corner for a problem
> > > > that compositors / RT submissions have a pretty reasoable different
> > > > option (bypass). More below.
> > > > 
> > > > > > > can easily afford to create our own per gt work queues. Whereas before
> > > > > > > kernel could create up to the number of CPUs threads, we now create one
> > > > > > > per GT.
> > > > > > > 
> > > > > > 
> > > > > > This breaks down for any execution queue that waits in a scheduler work
> > > > > > item for any reason. Long-running preemption queues do this for
> > > > > > time-slicing, multi-queue registration, and possibly a few other cases.
> > > > > > 
> > > > > > I've looked at this before and rough divide is this works for 3D but not
> > > > > > long running.
> > > > > 
> > > > > Hmmm curious. Could you expand a bit more on this?
> > > > > 
> > > > > Where do those wait, in run job or prepare job? And on what ie. where is the
> > > > > cross-dependency?
> > > > 
> > > > I think run_job can wait on multi-q syncs if a sync is outstanding for
> > > > the group.
> > > 
> > > Not sure I understand. A synchronous wait in the run_job callback on
> > > something not yet submitted? Because if it is already submitted it would be
> > > ahead in the queue so it wouldn't hang, no?
> > > 
> > 
> > It is waiting on a firmware control message as part of the multi-Q
> > registration process to acknowledge completion. Typically, this is an
> > asynchronous fire-and-forget command, but only one of these can be
> > outstanding at a time because the submission state machine can track
> > only a single instance. As a result, if two multi-Q syncs arrive
> > back-to-back for any reason, the second must wait for the first to
> > complete beforing issuing.
> 
> So first one has been issued from the first run_job. Second run_job arrives
> and waits for the firmware to finish processing the first one. I still don't
> see the problem. Is the completion of the first multi-Q registration queued

If this happens on a GT-ordered workqueue, a single queue waiting will
block every other queue on the GT from making progress.

A single GT-ordered workqueue also limits future possibilities, because
anything running on the scheduler workqueue is effectively forbidden
from blocking. That's a paradigm shift I don't want to introduce.

Matt 

> for processing in the same scheduler submit workqueue?
> 
> > We actually do this in a few other places as well with various GuC
> > messages. Could the state machine be extended to track more than one
> > message per class? Sure. Would that be significantly more complicated
> > than simply waiting or sleeping? Absolutely.
> > 
> > This state is used to rebuild the GuC backend after various global
> > events, such as power-management events, global resets, and VF
> > migration, so it must be 100% accurate to ensure proper recovery from
> > such events. This is also why all firmware control messages are issued
> > from either `run_job` or "alien" work items. After a queue is stopped,
> > these global events have a stable snapshot of the queue state, allowing
> > them to take the correct actions to rebuild the backend.
> 
> Ah so the answer to the above is yes?
> 
> submit_wq:
> 
> run_job A: queues multi-Q registration and submits job A
> run_job B: waits for multi-Q registration queue to be empty
> 
> <Irq arrives>:
> multi-Q registration complete
>   -> schedules multi-Q queue emptying action on the same submit_wq
> 
> submit_wq:
> 
> run_job B: still waits for multi-Q registration queue to be empty
>            submits own multi-Q registration
>            submits job B
> multi-Q  : mark queue as available -> does not get to run ever
> 
> Is that fundamentally it? If my patch breaks it why it is not broken even
> today, if it is the same ordered worker?
> 
> This family of functions:?
> 
> static void xe_sched_process_msg_queue(struct xe_gpu_scheduler *sched)
> {
> 	if (!drm_sched_is_stopped(&sched->base))
> 		queue_work(sched->base.submit_wq, &sched->work_process_msg);
> }
> 
> Looks the same submit_wq as for run_job. Which is DRM scheduler owned so
> single threaded.
> 
> Regards,
> 
> Tvrtko
> 
> > > > > Is it related to the alien work items xe "injects" onto the scheduler
> > > > > workers?
> > > > 
> > > > Preempt fences inject suspend/resume messages into the scheduler queue
> > > > to toggle scheduling state and wait for a suspend timeslice.
> > > > Additionally, if the state machine that tracks asynchronous firmware
> > > > commands is in an unexpected state, we wait for the firmware before
> > > > issuing another command.
> > > > 
> > > > As a general principle, we've accepted that it is okay to sleep because
> > > > this runs on a dedicated workqueue. We could rework that, but I'd prefer
> > > > not to, as it simplifies the code considerably.
> > > > 
> > > > Matt
> > > > 
> > > > > 
> > > > > Regards,
> > > > > 
> > > > > Tvrtko
> > > > > 
> > > > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> > > > > > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> > > > > > > ---
> > > > > > >     drivers/gpu/drm/xe/xe_gt.c         |  7 +++++++
> > > > > > >     drivers/gpu/drm/xe/xe_gt_types.h   |  3 +++
> > > > > > >     drivers/gpu/drm/xe/xe_guc_submit.c | 11 +++++++----
> > > > > > >     3 files changed, 17 insertions(+), 4 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > > > > > > index 783eb6d631b5..fb2db9eff341 100644
> > > > > > > --- a/drivers/gpu/drm/xe/xe_gt.c
> > > > > > > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > > > > > > @@ -706,6 +706,8 @@ static void xe_gt_fini(void *arg)
> > > > > > >     	for (i = 0; i < XE_ENGINE_CLASS_MAX; ++i)
> > > > > > >     		xe_hw_fence_irq_finish(&gt->fence_irq[i]);
> > > > > > > +	destroy_workqueue(gt->submit_wq);
> > > > > > > +
> > > > > > >     	xe_gt_disable_host_l2_vram(gt);
> > > > > > >     }
> > > > > > > @@ -716,6 +718,11 @@ int xe_gt_init(struct xe_gt *gt)
> > > > > > >     	INIT_WORK(&gt->reset.worker, gt_reset_worker);
> > > > > > > +	gt->submit_wq = alloc_ordered_workqueue("xe-submit-gt%u",
> > > > > > > +						WQ_MEM_RECLAIM, gt->info.id);
> > > > > > > +	if (IS_ERR(gt->submit_wq))
> > > > > > > +		return PTR_ERR(gt->submit_wq);
> > > > > > > +
> > > > > > >     	for (i = 0; i < XE_ENGINE_CLASS_MAX; ++i) {
> > > > > > >     		gt->ring_ops[i] = xe_ring_ops_get(gt, i);
> > > > > > >     		xe_hw_fence_irq_init(&gt->fence_irq[i]);
> > > > > > > diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> > > > > > > index e5588c88800a..175c42672546 100644
> > > > > > > --- a/drivers/gpu/drm/xe/xe_gt_types.h
> > > > > > > +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> > > > > > > @@ -248,6 +248,9 @@ struct xe_gt {
> > > > > > >     	/** @exec_queue_ops: submission backend exec queue operations */
> > > > > > >     	const struct xe_exec_queue_ops *exec_queue_ops;
> > > > > > > +	/** @submit_wq: ... */
> > > > > > > +	struct workqueue_struct *submit_wq;
> > > > > > > +
> > > > > > >     	/**
> > > > > > >     	 * @ring_ops: ring operations for this hw engine (1 per engine class)
> > > > > > >     	 */
> > > > > > > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> > > > > > > index 9458bf477fa6..858f84dc8fed 100644
> > > > > > > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > > > > > > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > > > > > > @@ -1928,11 +1928,12 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
> > > > > > >     	struct xe_gpu_scheduler *sched;
> > > > > > >     	struct xe_guc *guc = exec_queue_to_guc(q);
> > > > > > >     	struct workqueue_struct *submit_wq = NULL;
> > > > > > > +	struct xe_gt *gt = guc_to_gt(guc);
> > > > > > >     	struct xe_guc_exec_queue *ge;
> > > > > > >     	long timeout;
> > > > > > >     	int err, i;
> > > > > > > -	xe_gt_assert(guc_to_gt(guc), xe_device_uc_enabled(guc_to_xe(guc)));
> > > > > > > +	xe_gt_assert(gt, xe_device_uc_enabled(guc_to_xe(guc)));
> > > > > > >     	ge = kzalloc_obj(*ge);
> > > > > > >     	if (!ge)
> > > > > > > @@ -1964,12 +1965,14 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
> > > > > > >     		struct xe_exec_queue *primary = xe_exec_queue_multi_queue_primary(q);
> > > > > > >     		submit_wq = primary->guc->sched.base.submit_wq;
> > > > > > > +	} else {
> > > > > > > +		submit_wq = gt->submit_wq;
> > > > > > >     	}
> > > > > > >     	err = xe_sched_init(&ge->sched, &drm_sched_ops, &xe_sched_ops,
> > > > > > > -			    submit_wq, xe_lrc_ring_size() / MAX_JOB_SIZE_BYTES, 64,
> > > > > > > -			    timeout, guc_to_gt(guc)->ordered_wq, NULL,
> > > > > > > -			    q->name, gt_to_xe(q->gt)->drm.dev);
> > > > > > > +			    submit_wq, xe_lrc_ring_size() / MAX_JOB_SIZE_BYTES,
> > > > > > > +			    64, timeout, gt->ordered_wq, NULL, q->name,
> > > > > > > +			    gt_to_xe(gt)->drm.dev);
> > > > > > >     	if (err)
> > > > > > >     		goto err_release_id;
> > > > > > > -- 
> > > > > > > 2.54.0
> > > > > > > 
> > > > > 
> > > 
>