Commit 163bcc2c authored by Maarten Lankhorst's avatar Maarten Lankhorst
Browse files

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: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20170904150456.31049-1-maarten.lankhorst@linux.intel.com
[mlankhorst: preceeding -> preceding (checkpatch)]
parent f46640b9

drivers/gpu/drm/drm_atomic.c

+0 −7
Original line number Diff line number Diff line
@@ -163,13 +163,6 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) 
		crtc->funcs->atomic_destroy_state(crtc,

						  state->crtcs[i].state);



		if (state->crtcs[i].commit) {

			kfree(state->crtcs[i].commit->event);

			state->crtcs[i].commit->event = NULL;

			drm_crtc_commit_put(state->crtcs[i].commit);

		}



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

		state->crtcs[i].ptr = NULL;

		state->crtcs[i].state = NULL;

	}

drivers/gpu/drm/drm_atomic_helper.c

+35 −47
Original line number Diff line number Diff line
@@ -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);

include/drm/drm_atomic.h

+0 −1
Original line number Diff line number Diff line
@@ -144,7 +144,6 @@ struct __drm_planes_state { 
struct __drm_crtcs_state {

	struct drm_crtc *ptr;

	struct drm_crtc_state *state, *old_state, *new_state;

	struct drm_crtc_commit *commit;

	s32 __user *out_fence_ptr;

	unsigned last_vblank_count;

};

include/drm/drm_crtc.h

+19 −4
Original line number Diff line number Diff line
@@ -253,6 +253,15 @@ struct drm_crtc_state { 
	 */

	struct drm_pending_vblank_event *event;



	/**

	 * @commit:

	 *

	 * This tracks how the commit for this update proceeds through the

	 * various phases. This is never cleared, except when we destroy the

	 * state, so that subsequent commits can synchronize with previous ones.

	 */

	struct drm_crtc_commit *commit;



	struct drm_atomic_state *state;

};
@@ -808,10 +817,16 @@ struct drm_crtc { 
	 * @commit_list:

	 *

	 * List of &drm_crtc_commit structures tracking pending commits.

	 * Protected by @commit_lock. This list doesn't hold its own full

	 * reference, but burrows it from the ongoing commit. Commit entries

	 * must be removed from this list once the commit is fully completed,

	 * but before it's correspoding &drm_atomic_state gets destroyed.

	 * Protected by @commit_lock. This list holds its own full reference,

	 * as does the ongoing commit.

	 *

	 * "Note that the commit for a state change is also tracked in

	 * &drm_crtc_state.commit. For accessing the immediately preceding

	 * commit in an atomic update it is recommended to just use that

	 * pointer in the old CRTC state, since accessing that doesn't need

	 * any locking or list-walking. @commit_list should only be used to

	 * stall for framebuffer cleanup that's signalled through

	 * &drm_crtc_commit.cleanup_done."

	 */

	struct list_head commit_list;

CRA Git | Maintained and supported by SUSTech CRA and CCSE