drm/atomic: Move drm_crtc_commit to drm_crtc_state, v4.

Most code only cares about the current commit or previous commit.
Fortuantely we already have a place to track those. Move it to
drm_crtc_state where it belongs. :)

The per-crtc commit_list is kept for places where we have to look
deeper than the current or previous commit for checking whether to stall
on unpin. This is used in drm_atomic_helper_setup_commit and
intel_has_pending_fb_unpin.

Changes since v1:
- Update kerneldoc for drm_crtc.commit_list. (danvet)
Changes since v2:
- Remove drm_atomic_helper_async_check hunk. (pinchartl)
Changes since v3:
- Fix use-after-free in drm_atomic_helper_commit_cleanup_done().

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20170904150456.31049-1-maarten.lankhorst@linux.intel.com
[mlankhorst: preceeding -> preceding (checkpatch)]

1 files changed, 35 insertions, 47 deletions

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 94ad11f..075b0b3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1262,12 +1262,12 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
 void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev,
 					  struct drm_atomic_state *old_state)
 {
-	struct drm_crtc_state *unused;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc *crtc;
 	int i;
 
-	for_each_new_crtc_in_state(old_state, crtc, unused, i) {
-		struct drm_crtc_commit *commit = old_state->crtcs[i].commit;
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
+		struct drm_crtc_commit *commit = new_crtc_state->commit;
 		int ret;
 
 		if (!commit)
@@ -1730,7 +1730,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
 		kref_init(&commit->ref);
 		commit->crtc = crtc;
 
-		state->crtcs[i].commit = commit;
+		new_crtc_state->commit = commit;
 
 		ret = stall_checks(crtc, nonblock);
 		if (ret)
@@ -1768,22 +1768,6 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
 }
 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
  * @old_state: atomic state object with old state structures
@@ -1799,17 +1783,13 @@ static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc)
 void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *new_crtc_state;
+	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 	long ret;
 
-	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
-		spin_lock(&crtc->commit_lock);
-		commit = preceeding_commit(crtc);
-		if (commit)
-			drm_crtc_commit_get(commit);
-		spin_unlock(&crtc->commit_lock);
+	for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+		commit = old_crtc_state->commit;
 
 		if (!commit)
 			continue;
@@ -1827,8 +1807,6 @@ void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state)
 		if (ret == 0)
 			DRM_ERROR("[CRTC:%d:%s] flip_done timed out\n",
 				  crtc->base.id, crtc->name);
-
-		drm_crtc_commit_put(commit);
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies);
@@ -1851,15 +1829,25 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies);
 void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *new_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 
-	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
-		commit = old_state->crtcs[i].commit;
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
+		commit = new_crtc_state->commit;
 		if (!commit)
 			continue;
 
+		/*
+		 * copy new_crtc_state->commit to old_crtc_state->commit,
+		 * it's unsafe to touch new_crtc_state after hw_done,
+		 * but we still need to do so in cleanup_done().
+		 */
+		if (old_crtc_state->commit)
+			drm_crtc_commit_put(old_crtc_state->commit);
+
+		old_crtc_state->commit = drm_crtc_commit_get(commit);
+
 		/* backend must have consumed any event by now */
 		WARN_ON(new_crtc_state->event);
 		complete_all(&commit->hw_done);
@@ -1881,13 +1869,13 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_hw_done);
 void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *new_crtc_state;
+	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 	long ret;
 
-	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
-		commit = old_state->crtcs[i].commit;
+	for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+		commit = old_crtc_state->commit;
 		if (WARN_ON(!commit))
 			continue;
 
@@ -2293,20 +2281,13 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 	struct drm_private_state *old_obj_state, *new_obj_state;
 
 	if (stall) {
-		for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
-			spin_lock(&crtc->commit_lock);
-			commit = list_first_entry_or_null(&crtc->commit_list,
-					struct drm_crtc_commit, commit_entry);
-			if (commit)
-				drm_crtc_commit_get(commit);
-			spin_unlock(&crtc->commit_lock);
+		for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
+			commit = old_crtc_state->commit;
 
 			if (!commit)
 				continue;
 
 			ret = wait_for_completion_interruptible(&commit->hw_done);
-			drm_crtc_commit_put(commit);
-
 			if (ret)
 				return ret;
 		}
@@ -2331,13 +2312,13 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		state->crtcs[i].state = old_crtc_state;
 		crtc->state = new_crtc_state;
 
-		if (state->crtcs[i].commit) {
+		if (new_crtc_state->commit) {
 			spin_lock(&crtc->commit_lock);
-			list_add(&state->crtcs[i].commit->commit_entry,
+			list_add(&new_crtc_state->commit->commit_entry,
 				 &crtc->commit_list);
 			spin_unlock(&crtc->commit_lock);
 
-			state->crtcs[i].commit->event = NULL;
+			new_crtc_state->commit->event = NULL;
 		}
 	}
 
@@ -3171,6 +3152,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 	state->connectors_changed = false;
 	state->color_mgmt_changed = false;
 	state->zpos_changed = false;
+	state->commit = NULL;
 	state->event = NULL;
 	state->pageflip_flags = 0;
 }
@@ -3209,6 +3191,12 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
  */
 void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
 {
+	if (state->commit) {
+		kfree(state->commit->event);
+		state->commit->event = NULL;
+		drm_crtc_commit_put(state->commit);
+	}
+
 	drm_property_blob_put(state->mode_blob);
 	drm_property_blob_put(state->degamma_lut);
 	drm_property_blob_put(state->ctm);