From: Ray Strode <rstrode@xxxxxxxxxx> A drm atomic commit can be quite slow on some hardware. It can lead to a lengthy queue of commands that need to get processed and waited on before control can go back to user space. If user space is a real-time thread, that delay can have severe consequences, leading to the process getting killed for exceeding rlimits. This commit addresses the problem by always running the slow part of a commit on a workqueue, separated from the task initiating the commit. This change makes the nonblocking and blocking paths work in the same way, and as a result allows the task to sleep and not use up its RLIMIT_RTTIME allocation. Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2861 Signed-off-by: Ray Strode <rstrode@xxxxxxxxxx> --- drivers/gpu/drm/drm_atomic_helper.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 292e38eb6218..1a1e68d98d38 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2028,64 +2028,63 @@ int drm_atomic_helper_commit(struct drm_device *dev, * This is the point of no return - everything below never fails except * when the hw goes bonghits. Which means we can commit the new state on * the software side now. */ ret = drm_atomic_helper_swap_state(state, true); if (ret) goto err; /* * Everything below can be run asynchronously without the need to grab * any modeset locks at all under one condition: It must be guaranteed * that the asynchronous work has either been cancelled (if the driver * supports it, which at least requires that the framebuffers get * cleaned up with drm_atomic_helper_cleanup_planes()) or completed * before the new state gets committed on the software side with * drm_atomic_helper_swap_state(). * * This scheme allows new atomic state updates to be prepared and * checked in parallel to the asynchronous completion of the previous * update. Which is important since compositors need to figure out the * composition of the next frame right after having submitted the * current layout. * * NOTE: Commit work has multiple phases, first hardware commit, then * cleanup. We want them to overlap, hence need system_unbound_wq to * make sure work items don't artificially stall on each another. */ drm_atomic_state_get(state); - if (nonblock) - queue_work(system_unbound_wq, &state->commit_work); - else - commit_tail(state); + queue_work(system_unbound_wq, &state->commit_work); + if (!nonblock) + flush_work(&state->commit_work); return 0; err: drm_atomic_helper_cleanup_planes(dev, state); return ret; } EXPORT_SYMBOL(drm_atomic_helper_commit); /** * DOC: implementing nonblocking commit * * Nonblocking atomic commits should use struct &drm_crtc_commit to sequence * different operations against each another. Locks, especially struct * &drm_modeset_lock, should not be held in worker threads or any other * asynchronous context used to commit the hardware state. * * drm_atomic_helper_commit() implements the recommended sequence for * nonblocking commits, using drm_atomic_helper_setup_commit() internally: * * 1. Run drm_atomic_helper_prepare_planes(). Since this can fail and we * need to propagate out of memory/VRAM errors to userspace, it must be called * synchronously. * * 2. Synchronize with any outstanding nonblocking commit worker threads which * might be affected by the new state update. This is handled by * drm_atomic_helper_setup_commit(). * * Asynchronous workers need to have sufficient parallelism to be able to run * different atomic commits on different CRTCs in parallel. The simplest way to -- 2.41.0
