From: Ray Strode <rstr...@redhat.com>

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 <rstr...@redhat.com>
---
 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

Reply via email to