All of lore.kernel.org
 help / color / mirror / Atom feed
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: Mon, 6 Jul 2026 15:54:45 -0700	[thread overview]
Message-ID: <akwyNSXO+QspyoBx@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <5963b079-cb44-4cd4-be56-f47bf3f15e0e@igalia.com>

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.

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
> 

  reply	other threads:[~2026-07-06 22:54 UTC|newest]

Thread overview: 23+ 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-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-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-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-03  9:22 ` Matthew Brost
2026-07-06 12:41   ` Tvrtko Ursulin
2026-07-06 22:54     ` Matthew Brost [this message]
2026-07-07  7:12       ` Matthew Brost

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=akwyNSXO+QspyoBx@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.