From: Matthew Brost <matthew.brost@intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: <dri-devel@lists.freedesktop.org>,
Boris Brezillon <boris.brezillon@collabora.com>,
Steven Price <steven.price@arm.com>,
"Liviu Dudau" <liviu.dudau@arm.com>,
Chia-I Wu <olvaffe@gmail.com>, Danilo Krummrich <dakr@kernel.org>,
Philipp Stanner <phasta@kernel.org>, <kernel-dev@igalia.com>
Subject: Re: [RFC 0/8] DRM scheduler kthread_worker for submission latency improvements
Date: Wed, 8 Jul 2026 15:39:55 -0700 [thread overview]
Message-ID: <ak7Ru+STL9knR1xS@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <ak63Qzqxn1k9ezEd@gsse-cloud1.jf.intel.com>
On Wed, Jul 08, 2026 at 01:46:59PM -0700, Matthew Brost 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`.
>
> > It is definitely true this solution would simplify the implementation, as
>
> Yea, I think when I pushed back supporting kthreads + work queues, I
> wasn't aware of kthread_work which sematicly matches work queues. Given
> the same semantics supporting both seems trivial.
>
> > you say, problem of thread explosion is limited to RT tasks so basically a
> > non-issue. And the need to create own worker pool implementation also goes
> > away.
> >
> > Why I was unsure about deciding at drm_sched_init time is a) runtime
> > priority changes, but perhaps that is not such a big deal and we could live
>
> Xe only allow static priorities on queues, unsure about other drivers.
>
Sorry for the double reply. Even dynamic priorities are likely workable.
If you stop a scheduler, you should be able to reinitialize it in a
different mode (e.g., switch between a workqueue and `kthread_work`)
and then restart it.
I think DRM sched would need slightly stronger stop semantics, though—
for example, a lock around the "stopped" state and work queuing—but it
seems possible if absolutely necessary.
Matt
> Matt
>
> > with a static setup, and b) the priority inversion problems relating to
> > other workqueues used by various drivers (like the panthor issue Boris
> > raised).
> >
> > Or we can cap the number of workqueues to some reasonable number and then
> > Tejun maybe can give us RT workers. There are many options on the table so
> > lets see how to discussion develops.
> >
> > Regards,
> >
> > Tvrtko
> >
> > > [1] https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commits/drm-dep-rebase-7-6
> > > [2] https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/fdc38e4acefc9327637fd818b5b91fd6b2a6198f?file_path=include%2Fdrm%2Fdrm_dep.h#line_99bf000ae_A178
> > > [3] https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commit/f1483af15747a2d5d73262d2cdfc9b616230c5d9
> > >
> > > > DRM dep has this built in without any of that complexity. I'm hoping to
> > > > get around to rebasing it soon, but of course there's always something
> > > > else to work on. Sooner or later, it will get rebased, and Xe will move
> > > > over to it given the latency and power-saving benefits I observed. Given
> > > > power savings, I'd think drivers for ARM based phones would be pretty
> > > > keen on moving over too.
> > > >
> > > > From an architecture point of view, bypass is the ultimate win because
> > > > the context switch is completely avoided. Again, this is primarily for
> > > > compositor use cases, since they do not wait on fences.
> > > >
> > > > Also, in general, if Panthor selects PANTHOR_GROUP_PRIORITY_REALTIME, it
> > > > would be very odd to pass in fences. The RT priority then depends on
> > > > other work completing before the RT job can be scheduled, which
> > > > seemingly defeats the purpose of having RT priority in the first place.
> > > >
> > > > > > > similar. Another good quote for the above issue, explaining the consequence of
> > > > > > > CPU starvation of the submit path, is this:
> > > > > > >
> > > > > > > """
> > > > > > > As a result, vkQueueWaitIdle blocks for 9.5ms for a gpu job that takes 4.5ms
> > > > > > > gpu time.
> > > > > > > """
> > > > > >
> > > > > > More details here?
> > > > >
> > > > > This just illustrates how long can the workqueue wait for it's slice on the
> > > > > CPU if the cores are loaded with other tasks. In other words, how long since
> > > > > job is runnable to it actually being passed to the GPU.
> > > > >
> > > >
> > > > Yes. I do wonder if this is an Android quirk, though. While working on
> > > > some MM-related Android issues, I noticed it has quite a few quirks.
> > > >
> > > > I've done a lot of profiling around workqueues in Xe (submission, page
> > > > faults, resuming after preempt fences, etc.), and I've seen latency
> > > > spikes of perhaps 10–20 µs, but never anything on the millisecond scale
> > > > on Linux.
> > > >
> > > > I also recently fixed a workqueue bug [1] that showed up fairly often on
> > > > Android. In that case, workqueues could stop scheduling under the right
> > > > conditions. If I recall correctly, a flush_work() could get the work
> > > > item unstuck. It might be worth looking into whether that helps here.
> > > >
> > > > [1] https://patchwork.freedesktop.org/series/164199/
> > > >
> > > > > > > DRM scheduler was originally using kthreads but was converted workqueues due
> > > > > > > desire by xe to create thousands of schedulers. This series also questions
> > > > > > > whether that was needed, given how the submission is serialized by a device
> > > > > > > global lock (per GT, so almost device global). Panthor has a similar situation;
> > > > > > > hence the series contains two patches to move those two to a setup which matches
> > > > > > > the design of those drivers.
> > > > > > >
> > > > > > > Other drivers, like for example amdgpu, v3d, etnaviv etc, which use the
> > > > > > > scheduler as a hardware scheduler, where number of instances follow the number
> > > > > > > of hardware blocks instead the number of userspace contexts, are completely
> > > > > > > fine.
> > > > > > >
> > > > > > > There are use cases however which do currently track the number of userspace
> > > > > > > contexts and which do allow for more parallelism. For those a straight
> > > > > > > kthread_work conversion would be a problem due an explosion in number of
> > > > > > > threads.
> > > > > > >
> > > > > > > The most direct example is panthor VM bind queue which creates a scheduler per
> > > > > > > userspace context and relies on work queue concurrency management to keep the
> > > > > > > number of threads in check.
> > > > > > >
> > > > > > > This creates a challenge for the kthread_work conversion. To solve which I for
> > > > > > > now opted to create a trivial round-robin thread pool. For the RFC this is
> > > > > > > limited to four CPU threads and is something which will need to be discussed.
> > > > > >
> > > > > > 4 CPU threads per device, per drm sched module?
> > > > >
> > > > > 4 CPU threads per drm_sched_create_concurrent_worker(). So depends on the
> > > >
> > > > Ugh, that is a pretty ugly API.
> > > >
> > > > Matt
> > > >
> > > > > driver how it wants to use it. For xe there are no usages of that, albeit
> > > > > you say single thread is not workable, we can discuss that in the other
> > > > > sub-thread. For panthor this RFC uses that flavour of the worker for the VM
> > > > > bind queues and in that case it is 4 CPU threads per device which handle VM
> > > > > bind requests from all userspace clients.
> > > > >
> > > > > Regards,
> > > > >
> > > > > Tvrtko
> > > > >
> > > > > >
> > > > > > I have more questions but let's get some clarification first.
> > > > > >
> > > > > > Matt
> > > > > >
> > > > > > > Ie. how much parallelsim those really need. The true answer is somewhere between
> > > > > > > "at most the number of active userspace contexts and the number of CPU cores".
> > > > > > > Or it could be less than that, since after all, VM BIND parallelism is
> > > > > > > eventually going to choke on a narrower gate of actual GPU execution. We could
> > > > > > > also allow drivers to pick their number.
> > > > > > >
> > > > > > > In terms of how I implemented priority inheritance, the most important
> > > > > > > characteristic is that it is temporary. As many userspace clients may be
> > > > > > > submitting to a single DRM scheduler instance, a generic solution is to only
> > > > > > > elevate the submission worker priority while there are active high priority
> > > > > > > submitters. The mechanism is light weight and has a hysteresis built in to avoid
> > > > > > > frequent scheduler operations.
> > > > > > >
> > > > > > > That's pretty much it for now apart for an important detail that this RFC will
> > > > > > > not build for all drivers! Out of those which directly use the DRM scheduler
> > > > > > > APIs changed, I converted only panthor and xe. Amdgpu will also work by the way.
> > > > > > > Others I have not tried to build.
> > > > > > >
> > > > > > > Cc: Boris Brezillon <boris.brezillon@collabora.com>
> > > > > > > Cc: Steven Price <steven.price@arm.com>
> > > > > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > > > > Cc: Chia-I Wu <olvaffe@gmail.com>
> > > > > > > Cc: Danilo Krummrich <dakr@kernel.org>
> > > > > > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > > > > > Cc: Philipp Stanner <phasta@kernel.org>
> > > > > > >
> > > > > > > Tvrtko Ursulin (8):
> > > > > > > drm/panthor: Remove redundant drm_sched_job_cleanup() from the
> > > > > > > .free_job callback
> > > > > > > drm/panthor: Use separate workqueue for DRM scheduler
> > > > > > > drm/sched: Use generic naming for workqueue helpers
> > > > > > > drm/xe: Convert to per gt scheduler workers
> > > > > > > drm: Wrap DRM scheduler worker in own abstraction
> > > > > > > drm/sched: Convert the scheduler job submission to kthread_worker
> > > > > > > drm/sched: Add ability to change drm_sched_worker priority
> > > > > > > drm/sched: Notify worker of the entity submission priority
> > > > > > >
> > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 8 +-
> > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 +-
> > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 4 +-
> > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 2 +-
> > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 8 +-
> > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 8 +-
> > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c | 2 +-
> > > > > > > drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c | 4 +-
> > > > > > > drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c | 4 +-
> > > > > > > drivers/gpu/drm/msm/adreno/adreno_device.c | 4 +-
> > > > > > > drivers/gpu/drm/panthor/panthor_mmu.c | 20 +-
> > > > > > > drivers/gpu/drm/panthor/panthor_sched.c | 23 ++-
> > > > > > > drivers/gpu/drm/scheduler/sched_entity.c | 3 +
> > > > > > > drivers/gpu/drm/scheduler/sched_main.c | 168 +++++++++++-----
> > > > > > > drivers/gpu/drm/scheduler/sched_rq.c | 202 ++++++++++++++++++++
> > > > > > > drivers/gpu/drm/xe/xe_dep_scheduler.c | 6 +-
> > > > > > > drivers/gpu/drm/xe/xe_dep_scheduler.h | 6 +-
> > > > > > > drivers/gpu/drm/xe/xe_exec_queue.c | 6 +-
> > > > > > > drivers/gpu/drm/xe/xe_gpu_scheduler.c | 21 +-
> > > > > > > drivers/gpu/drm/xe/xe_gpu_scheduler.h | 2 +-
> > > > > > > drivers/gpu/drm/xe/xe_gpu_scheduler_types.h | 2 +-
> > > > > > > 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 | 18 +-
> > > > > > > drivers/gpu/drm/xe/xe_tlb_inval.c | 14 +-
> > > > > > > drivers/gpu/drm/xe/xe_tlb_inval_types.h | 5 +-
> > > > > > > include/drm/gpu_scheduler.h | 131 ++++++++++++-
> > > > > > > 27 files changed, 551 insertions(+), 134 deletions(-)
> > > > > > >
> > > > > > > --
> > > > > > > 2.54.0
> > > > >
> >
next prev parent reply other threads:[~2026-07-08 22:40 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-02 14:37 [RFC 0/8] DRM scheduler kthread_worker for submission latency improvements Tvrtko Ursulin
2026-07-02 14:37 ` [RFC 1/8] drm/panthor: Remove redundant drm_sched_job_cleanup() from the .free_job callback Tvrtko Ursulin
2026-07-03 15:00 ` Steven Price
2026-07-09 10:05 ` Tvrtko Ursulin
2026-07-09 10:15 ` Boris Brezillon
2026-07-09 12:16 ` Tvrtko Ursulin
2026-07-02 14:37 ` [RFC 2/8] drm/panthor: Use separate workqueue for DRM scheduler Tvrtko Ursulin
2026-07-02 15:31 ` Boris Brezillon
2026-07-06 12:03 ` Tvrtko Ursulin
2026-07-06 14:18 ` Boris Brezillon
2026-07-08 16:47 ` Tvrtko Ursulin
2026-07-09 6:45 ` Boris Brezillon
2026-07-09 10:56 ` Tvrtko Ursulin
2026-07-09 12:19 ` Boris Brezillon
2026-07-09 14:33 ` Tvrtko Ursulin
2026-07-09 15:24 ` Boris Brezillon
2026-07-02 14:37 ` [RFC 3/8] drm/sched: Use generic naming for workqueue helpers Tvrtko Ursulin
2026-07-02 14:37 ` [RFC 4/8] drm/xe: Convert to per gt scheduler workers Tvrtko Ursulin
2026-07-03 9:06 ` Matthew Brost
2026-07-06 12:27 ` Tvrtko Ursulin
2026-07-06 23:14 ` Matthew Brost
2026-07-08 16:35 ` Tvrtko Ursulin
2026-07-08 19:32 ` Matthew Brost
2026-07-09 11:35 ` Tvrtko Ursulin
2026-07-09 11:45 ` Matthew Brost
2026-07-09 12:05 ` Tvrtko Ursulin
2026-07-02 14:37 ` [RFC 5/8] drm: Wrap DRM scheduler worker in own abstraction Tvrtko Ursulin
2026-07-02 14:37 ` [RFC 6/8] drm/sched: Convert the scheduler job submission to kthread_worker Tvrtko Ursulin
2026-07-02 14:37 ` [RFC 7/8] drm/sched: Add ability to change drm_sched_worker priority Tvrtko Ursulin
2026-07-02 14:37 ` [RFC 8/8] drm/sched: Notify worker of the entity submission priority Tvrtko Ursulin
2026-07-03 8:52 ` [RFC 0/8] DRM scheduler kthread_worker for submission latency improvements Philipp Stanner
2026-07-06 12:20 ` Tvrtko Ursulin
2026-07-06 19:36 ` Tejun Heo
2026-07-08 16:24 ` Tvrtko Ursulin
2026-07-09 8:28 ` Tvrtko Ursulin
2026-07-10 12:29 ` Tvrtko Ursulin
2026-07-10 16:11 ` Tejun Heo
2026-07-10 18:29 ` Matthew Brost
2026-07-03 9:22 ` Matthew Brost
2026-07-06 12:41 ` Tvrtko Ursulin
2026-07-06 22:54 ` Matthew Brost
2026-07-07 7:12 ` Matthew Brost
2026-07-08 17:01 ` Tvrtko Ursulin
2026-07-08 20:46 ` Matthew Brost
2026-07-08 22:39 ` Matthew Brost [this message]
2026-07-09 6:52 ` Boris Brezillon
2026-07-09 7:25 ` Boris Brezillon
2026-07-09 7:55 ` Matthew Brost
2026-07-09 10:08 ` Boris Brezillon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ak7Ru+STL9knR1xS@gsse-cloud1.jf.intel.com \
--to=matthew.brost@intel.com \
--cc=boris.brezillon@collabora.com \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=kernel-dev@igalia.com \
--cc=liviu.dudau@arm.com \
--cc=olvaffe@gmail.com \
--cc=phasta@kernel.org \
--cc=steven.price@arm.com \
--cc=tvrtko.ursulin@igalia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox