Re: [PATCH 09/27] drm/atomic-helper: roll out commit synchronization

Op 08-06-16 om 14:19 schreef Daniel Vetter:

To facilitate easier reviewing this is split out from the overall nonblocking commit rework. It just rolls out the helper functions and uses them in the main drm_atomic_helper_commit() function to make it clear where in the flow they're used.

The next patch will actually split drm_atomic_helper_commit() into 2 pieces, with the tail being run asynchronously from a worker.

v2: Improve kerneldocs (Maarten).

v3: Don't convert ERESTARTSYS to EINTR (Maarten). Also don't fail if the wait succeed in stall_check - we need to convert that case (it returns the remaining jiffies) to 0 for success.

Tested-by: Tomeu Vizoso tomeu.vizoso@collabora.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Tomeu Vizoso tomeu.vizoso@gmail.com Cc: Daniel Stone daniels@collabora.com Tested-by: Liviu Dudau Liviu.Dudau@arm.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com

---

drivers/gpu/drm/drm_atomic_helper.c | 346 ++++++++++++++++++++++++++++++++++++ include/drm/drm_atomic_helper.h | 7 + 2 files changed, 353 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 326ee34cdba4..63e46827b303 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1155,6 +1155,10 @@ int drm_atomic_helper_commit(struct drm_device *dev, if (nonblock) return -EBUSY;

• ret = drm_atomic_helper_setup_commit(state, nonblock);
• if (ret)

• return ret;
  

• ret = drm_atomic_helper_prepare_planes(dev, state); if (ret) return ret;

@@ -1185,16 +1189,22 @@ int drm_atomic_helper_commit(struct drm_device *dev,

drm_atomic_helper_wait_for_fences(dev, state);

• drm_atomic_helper_wait_for_dependencies(state);

• drm_atomic_helper_commit_modeset_disables(dev, state);

  drm_atomic_helper_commit_planes(dev, state, false);

  drm_atomic_helper_commit_modeset_enables(dev, state);

• drm_atomic_helper_commit_hw_done(state);

• drm_atomic_helper_wait_for_vblanks(dev, state);

  drm_atomic_helper_cleanup_planes(dev, state);

• drm_atomic_helper_commit_cleanup_done(state);

• drm_atomic_state_free(state);

  return 0;

@@ -1239,6 +1249,305 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);

• being displayed.

*/

+static int stall_checks(struct drm_crtc *crtc, bool nonblock) +{

• struct drm_crtc_commit *commit, *stall_commit = NULL;
• bool completed = true;
• int i, ret = 0;
• spin_lock(&crtc->commit_lock);
• i = 0;
• list_for_each_entry(commit, &crtc->commit_list, commit_entry) {

• if (i == 0) {
  

• 	completed = try_wait_for_completion(&commit->flip_done);
  

• 	/* Userspace is not allowed to get ahead of the previous
  

• 	 * commit with nonblocking ones. */
  

• 	if (!completed && nonblock) {
  

• 		spin_unlock(&crtc->commit_lock);
  

• 		return -EBUSY;
  

• 	}
  

• } else if (i == 1) {
  

• 	stall_commit = commit;
  

• 	drm_crtc_commit_get(stall_commit);
  

• } else
  

• 	break;
  

• i++;
  

• }
• spin_unlock(&crtc->commit_lock);
• if (!stall_commit)

• return 0;
  

• /* We don't want to let commits get ahead of cleanup work too much,

• * stalling on 2nd previous commit means triple-buffer won't ever stall.
  

• */
  

• ret = wait_for_completion_interruptible_timeout(&commit->cleanup_done,

• 					10*HZ);
  

• if (ret == 0)

• DRM_ERROR("[CRTC:%d:%s] cleanup_done timed out\n",
  

• 	  crtc->base.id, crtc->name);
  

• drm_crtc_commit_put(stall_commit);
• return ret < 0 ? ret : 0;

+}

+/**

• • drm_atomic_helper_setup_commit - setup possibly nonblocking commit
• • @state: new modeset state to be committed
• • @nonblock: whether nonblocking behavior is requested.
• • This function prepares @state to be used by the atomic helper's support for
• • nonblocking commits. Drivers using the nonblocking commit infrastructure
• • should always call this function from their ->atomic_commit hook.
• • To be able to use this support drivers need to use a few more helper
• • functions. drm_atomic_helper_wait_for_dependencies() must be called before
• • actually committing the hardware state, and for nonblocking commits this call
• • must be placed in the async worker. See also drm_atomic_helper_swap_state()
• • and it's stall parameter, for when a driver's commit hooks look at the
• • ->state pointers of struct &drm_crtc, &drm_plane or &drm_connector directly.
• • Completion of the hardware commit step must be signalled using
• • drm_atomic_helper_commit_hw_done(). After this step the driver is not allowed
• • to read or change any permanent software or hardware modeset state. The only
• • exception is state protected by other means than &drm_modeset_lock locks.
• • Only the free standing @state with pointers to the old state structures can
• • be inspected, e.g. to clean up old buffers using
• • drm_atomic_helper_cleanup_planes().
• • At the very end, before cleaning up @state drivers must call
• • drm_atomic_helper_commit_cleanup_done().
• • This is all implemented by in drm_atomic_helper_commit(), giving drivers a
• • complete and esay-to-use default implementation of the atomic_commit() hook.
• • The tracking of asynchronously executed and still pending commits is done
• • using the core structure &drm_crtc_commit.
• • By default there's no need to clean up resources allocated by this function
• • explicitly: drm_atomic_state_default_clear() will take care of that
• • automatically.
• • Returns:
• • 0 on success. -EBUSY when userspace schedules nonblocking commits too fast,
• • -ENOMEM on allocation failures and -EINTR when a signal is pending.
• */

+int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,

• 		   bool nonblock)
  

+{

• struct drm_crtc *crtc;
• struct drm_crtc_state *crtc_state;
• struct drm_crtc_commit *commit;
• int i, ret;
• for_each_crtc_in_state(state, crtc, crtc_state, i) {

• commit = kzalloc(sizeof(*commit), GFP_KERNEL);
  

• if (!commit)
  

• 	return -ENOMEM;
  

• init_completion(&commit->flip_done);
  

• init_completion(&commit->hw_done);
  

• init_completion(&commit->cleanup_done);
  

• INIT_LIST_HEAD(&commit->commit_entry);
  

• kref_init(&commit->ref);
  

• commit->crtc = crtc;
  

• state->crtcs[i].commit = commit;
  

• ret = stall_checks(crtc, nonblock);
  

• if (ret)
  

• 	return ret;
  

• /* Drivers only send out events when at least either current or
  

•  * new CRTC state is active. Complete right away if everything
  

•  * stays off. */
  

• if (!crtc->state->active && !crtc_state->active) {
  

• 	complete_all(&commit->flip_done);
  

• 	continue;
  

• }
  

• /* Legacy cursor updates are fully unsynced. */
  

• if (state->legacy_cursor_update) {
  

• 	complete_all(&commit->flip_done);
  

• 	continue;
  

• }
  

• if (!crtc_state->event) {
  

• 	commit->event = kzalloc(sizeof(*commit->event),
  

• 				GFP_KERNEL);
  

• 	if (!commit->event)
  

• 		return -ENOMEM;
  

• 	crtc_state->event = commit->event;
  

• }
  

• crtc_state->event->base.completion = &commit->flip_done;
  

• }
• return 0;

+} +EXPORT_SYMBOL(drm_atomic_helper_setup_commit);

+static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc) +{

• struct drm_crtc_commit *commit;
• int i = 0;
• list_for_each_entry(commit, &crtc->commit_list, commit_entry) {

• /* skip the first entry, that's the current commit */
  

• if (i == 1)
  

• 	return commit;
  

• i++;
  

• }
• return NULL;

+}

+/**

• • drm_atomic_helper_wait_for_dependencies - wait for required preceeding commits
• • @state: new modeset state to be committed
• • This function waits for all preceeding commits that touch the same CRTC as
• • @state to both be committed to the hardware (as signalled by
• • drm_atomic_Helper_commit_hw_done) and executed by the hardware (as signalled

^extra cap here