Re: [RFC 0/8] DRM scheduler kthread_worker for submission latency improvements

[Boris Brezillon <boris.brezillon@collabora.com>]

On Thu, 9 Jul 2026 00:55:17 -0700
Matthew Brost <matthew.brost@intel.com> wrote:

> On Thu, Jul 09, 2026 at 09:25:52AM +0200, Boris Brezillon wrote:
> > On Wed, 8 Jul 2026 13:46:59 -0700
> > Matthew Brost <matthew.brost@intel.com> wrote:
> >   
> > > On Wed, Jul 08, 2026 at 06:01:55PM +0100, Tvrtko Ursulin wrote:  
> > > > 
> > > > On 07/07/2026 08:12, Matthew Brost wrote:    
> > > > > On Mon, Jul 06, 2026 at 03:54:45PM -0700, Matthew Brost wrote:    
> > > > > > On Mon, Jul 06, 2026 at 01:41:01PM +0100, Tvrtko Ursulin wrote:    
> > > > > > > 
> > > > > > > On 03/07/2026 10:22, Matthew Brost wrote:    
> > > > > > > > On Thu, Jul 02, 2026 at 03:37:37PM +0100, Tvrtko Ursulin wrote:    
> > > > > > > > > The problem statement is explained quite well and succinctly at:
> > > > > > > > > https://gitlab.freedesktop.org/panfrost/linux/-/work_items/49
> > > > > > > > > 
> > > > > > > > > Essentially, on a system (over)loaded with a lot of runnable CPU processes, a
> > > > > > > > > high-priority DRM client gets latency injected into the GPU submission path due
> > > > > > > > > to the DRM scheduler use of workqueues.
> > > > > > > > > 
> > > > > > > > > This patch series proposes to replace the workqueues with kthread_work and
> > > > > > > > > priority inheritance to solve this problem.
> > > > > > > > > 
> > > > > > > > > In the above linked issue Chia-I benchmarked the submit latencies which
> > > > > > > > > show a striking improvement:
> > > > > > > > > 
> > > > > > > > > 		median	95%	99%
> > > > > > > > >     before	41us	1.5ms	2.6ms
> > > > > > > > >      after	15us	19us	24us    
> > > > > > > > 
> > > > > > > > Can you give more information on these numbers? e.g., What you ran / how
> > > > > > > > you measured these. It is hard to argue with numbers.    
> > > > > > > 
> > > > > > > I believe Chia-I observed latency on some production hw/sw and then wrote a
> > > > > > > synthetic benchmark to test it more easily. Details are in the above linked
> > > > > > > issue.
> > > > > > >     
> > > > > > 
> > > > > > Thanks, I see this now. Part of the problem, as far as I can tell, is
> > > > > > that fences are signaled from work items rather than directly from IRQ
> > > > > > context.
> > > > > > 
> > > > > > For example:
> > > > > > 
> > > > > > "There is another 2.5 ms of scheduling latency from
> > > > > > panthor_sched_report_fw_events() (running on irq/105-panthor, PID 257)
> > > > > > to process_fw_events_work() (running on kworker/u32:1, PID 62)."
> > > > > > 
> > > > > > I assume this is only addressing the scheduling portion of the latency,
> > > > > > but you also mentioned a 9.5 ms delay for vkQueueWaitIdle(), where part
> > > > > > of the latency appears to be on the signaling side due to the worker
> > > > > > thread.
> > > > > >     
> > > > > > > > > This is obviously really good for preventing compositors from missing frames and    
> > > > > > > > 
> > > > > > > > Modern compositors do not pass render-job fences to draw jobs as input
> > > > > > > > dependencies. On Wayland, this functionality is provided by
> > > > > > > > linux-drm-syncobj-v1 and is enabled by default on Ubuntu 24.10 and
> > > > > > > > later. SurfaceFlinger has also operated this way for quite some time.
> > > > > > > > 
> > > > > > > > The obvious solution for compositors is to submit work directly through
> > > > > > > > the ioctl (i.e., bypass path in sched) when there are no input
> > > > > > > > dependencies, which should be the common case. The main exception is
> > > > > > > > when one of the BOs mapped into the compositor is migrating or otherwise
> > > > > > > > busy (i.e., a BO has a fence in a kernel dma-resv slot).    
> > > > > > > 
> > > > > > > You mean the direct submit RFC you floated some time ago? What was the
> > > > > > > verdict on that one, wasn't it rejected?
> > > > > > >     
> > > > > > 
> > > > > > I had some patches for DRM sched that I never posted. It turned out to
> > > > > > be a little tricky because of some other quirks in DRM sched, but it was
> > > > > > still roughly 100 LoC plus an additional lock.
> > > > > >     
> > > > > 
> > > > > Side note: I quickly rebased DRM DEP here [1] and prototyped some RT
> > > > > solutions.
> > > > > 
> > > > > I added a DRM_DEP_QUEUE_FLAGS_RT flag to internally choose between a
> > > > > workqueue and kthread_work, and to enable FIFO scheduling in [2]. Most
> > > > > of the details are hidden internally, but I had to make a few small
> > > > > changes in Xe to support this on the driver side [3].
> > > > > 
> > > > > Putting aside whether or when DRM DEP will land, if DRM sched really
> > > > > wants FIFO scheduling instead of bypass (the IMO this is somewhat
> > > > > questionable), I think the approach I took in DRM DEP makes a lot more
> > > > > sense. I haven't looked into what changes would be required in DRM
> > > > > sched, though; hopefully it wouldn't be too messy.
> > > > > 
> > > > > Of course, kthreads are now directly exposed to userspace, but this
> > > > > would be limited to privileged userspace with FIFO scheduling
> > > > > capabilities, which seems reasonable. Additionally, this approach does
> > > > > not require the kind of large paradigm shifts proposed by this series.    
> > > > 
> > > > This sounds plausible and TBH I also considered worker duality. If you look
> > > > at my series I wrap everything to drm_sched_work and drm_sched_worker so
> > > > drivers do not even have to know what is the underlying implementation.
> > > >     
> > > 
> > > You could probably abstract this to some extent, but any driver-side
> > > functions that process work on scheduler workers would still require
> > > dedicated function signatures. Perhaps some creative macros could do
> > > something like:
> > > 
> > > DRM_SCHED_DECL_DRIVER_WORK_FUNC(driver_sched, work_func);
> > > DRM_SCHED_INIT_DRIVER_WORK_FUNC(driver_sched, work_func);
> > > 
> > > However, this would only eliminate functions that perform a
> > > `container_of()` conversion or use an `if` statement to select between
> > > workqueues and `kthread_work`.  
> > 
> > Honestly, I'm questioning the viability of such an hybrid design. Not
> > only this makes the code base even harder to reason about (and thus,
> > harder to maintain), but if you think about it, the only reason we got  
> 
> If this is hard to maintain or reason about, then the scheduler is
> broken. It took me roughly 10 minutes to modify a branch with DRM dep
> and Xe to support a hybrid design, and another 20 minutes or so to
> figure out how to run an Xe test with FIFO scheduling :). It shouldn't
> be difficult to do the same in DRM sched either. If it is, that only
> highlights why I think DRM dep is a good idea.
> 
> > here is because we pretended the full/monolithic drm_sched stack was a
> > good fit for this FW-scheduler use case, and the reality is that it's
> > not.
> > 
> > What we need for this FW-scheduler case is a simple per-context FIFO
> > with dep tracking. The KMD can then decide when to pull jobs from
> > this FIFO with its own deferred work model (workqueues, kthread or
> > whatever). All it needs to have is:
> > 
> > - a way to initialize the job and its deps (so basically the
> >   drm_sched_job_xxx() helpers)
> > - a way to queue a job to this FIFO (something like
> >   drm_sched_entity_push_job(), but maybe simpler)
> > - a way to evaluate if the first job in the queue is ready for
> >   execution
> > - a way to register a callback for asynchronous queue/job readiness
> >   re-evaluation. That would basically be called on a dep fence callback
> >   call, and defers the responsibility of how the re-evaluation is done
> >   to the driver. If you're using kthreads, you'll just be issuing a
> >   kthread_queue_work() from this callback. If you want workqueues, you'd
> >   be using queue_work_on(), etc.
> > 
> > The key here, is that the framework no longer decides the deferred work
> > model the driver has to use. This push/pull model makes it so the driver
> > is in control at all times, which also simplifies things like teardown,
> > or rather, make it so drm_sched is not in the way of driver specific
> > teardown sequences.  
> 
> drm_sched teardowns are in fact awful.
> 
> > 
> > I know some of these concepts have been discussed with Philipp and
> > Danilo for the JobQueue implementation they intend to have on the
> > rust-side. I also know that they are not particularly thrilled by this
> > push/pull model, I think they're more going for a solution where jobs
> > are pushed to the driver when they are ready for execution, which
> > requires having their own worker/work to defer the run_job() call, so
> > back to the problem that the framework enforces the execution model.
> >   
> 
> This all sounds nice, but my initial reaction is that you're asking the
> driver to do a lot of things that it will likely get wrong—for example,
> stopping and starting queues, DMA-fence rules,

drm_sched is not doing much for that actually. You can get things wrong
and still not notice, or notice it, but too late. All it does is provide
a set of helpers to stop/start the main scheduler, and people have
copied the pattern that exist in other drivers to get things somewhat
correct.

Sure, the stop/start of your HW queues is something you'll have to do,
but it won't be as complicated as it is today, because the driver is
the one pulling jobs out of the job queue, so if it knows it's stopping
the HW queue, it can stop pulling jobs from there.

As for the fence rules, I'm not too sure what the risk is compared to
what we have today. Ultimately, what guarantees that you do the right
thing is lockdep not complaining where you're in a dma_fence_signalling
section, or drivers signaling the fences at teardown (which can be
automated through helpers too).

> tracking jobs to
> completion,

None of the drm_sched infra helped with that in Panthor: we still have
to track in-flight jobs anyway and do the right thing on a reset when
the context has in-flight jobs. Sure, drm_sched has extra checks, plus
a way to push jobs back to the pending list on resumption, but in case
of Panthor, it didn't solve much, and most of the time, it's actually
in the way. Now, I'm not saying Panthor is the alpha & omega when it
comes to these things, but I'd much rather have a component-base
approach, where drivers can pick the things then need and glue it
together, instead of having this monolithic design that's really meant
to be used for pure HW queues with SW queues on top, and not FW queues.

> handling timeouts, managing object lifetimes,

Yet again, lifetime would benefit from this split I think: as soon as
the job leaves the job queue (FIFO), ownership is transferred to the
driver, which makes things a lot easier to reason about than the
current "job is passed to the driver, but it's still owned by the
scheduler" and you to do shenanigans on a pre/post reset. All of a
sudden, this decision to re-issue jobs or not is left to the driver,
which, when it does, passes ownership back to the job queue.

Same for timeouts, it's up to the driver to decide at which level it's
placed, which would prevent the spurious timeouts issue we had in
Panthor, because GPU contexts are not always resident when jobs are
passed to the driver (second layer of scheduling there).

> and
> implementing teardown flows.

Teardown flow is probably where this new model would be the most
beneficial, because the job queue is just a passive component, so, as
soon as the driver decides that it wants to stop pulling jobs from it,
it happens, period. Now, there's of course other components you have to
stop to guarantee that in-flight jobs are either fully killed, or land
before the rest of the teardown happens. It's another case where I
didn't find drm_sched particularly useful, because you can get things
wrong still, and all it adds is extra churn because things can still be
pushed while you're in the process of declaring the HW queue dead.

> Consequently, DRM sched has historically
> gotten many of these things wrong as well, although the concepts you
> want a scheduler to provide are there—they're just implemented poorly.

There might be a bit of that, but there's also a lot of FW-scheduling
is not the same as global-HW-queue-scheduling. There's basically a whole
layer that's gone, because the queue is no longer global, but instead
per GPU-context. So basically, all the heavy-lifted logic in
drm_sched_main, and the teardown/pause/resume complexity is gone,
because when you're touching a queue, it has no impact on other queues.

> 
> Another major problem with what you're suggesting is that you
> essentially have to accept that a ready job may be pushed to the driver
> from IRQ context, since fences can signal from IRQ context.

That's a case for proper documentation, I think. Something along the
lines of "Don't ever do anything from the ::evaluate() callback other
than queuing a re-evaluation through some deferral mechanism, like
workqueue or kthread_worker". Lockdep should do the rest. Providing
default helpers for kthread_work/work_struct would certainly point
users in the right direction.

> That's a
> massive can of worms, and I know firsthand what a terrible idea it can
> be because i915 did exactly that.

I'd argue that, as it is today, drm_sched is already a massive can of
worms for these FW-scheduler use cases, otherwise we wouldn't be having
this discussion :P. I'm not sure getting rid of it in favor of
something simpler/modular would be significantly worse, especially not
if we can provide semi-generic components that can be pulled by drivers
to leverage some of the tricky aspects you mentioned (per-HW/FW-queue
timeout, semi-automatic dma_fence_signalling annotation in
work_item/kthread_work implems, ...). That's basically the things we've
been discussing for months with Philipp and Danilo.