dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state.
@ 2017-02-16 14:47 Maarten Lankhorst
  2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Fifth iteration. Instead of trying to convert all drivers straight away,
implement all macros that are required to get state working.

Old situation:
Use obj->state, which can refer to old or new state.
Use drm_atomic_get_(existing_)obj_state, which can refer to new or old state.
Use for_each_obj_in_state, which refers to new or old state.

New situation:

During atomic check:
- Use drm_atomic_get_obj_state to add a object to the atomic state,
  or get the new state.
- Use drm_atomic_get_(old/new)_obj_state to peek at the new/old state,
  without adding the object. This will return NULL if the object is
  not part of the state. For planes and connectors the relevant crtc_state
  is added, so this will work to get the crtc_state from obj_state->crtc
  too, this means not having to write some error handling. 

During atomic commit:
- Do not use drm_atomic_get_obj_state, obj->state or drm_atomic_get_(existing_)obj_state
  any more, replace with drm_atomic_get_old/new_obj_state calls as required.

During both:
- Use for_each_(new,old,oldnew)_obj_in_state to get the old or new state as needed.
  oldnew will be renamed to for_each_obj_in_state after all callers are converted
  to the new api.

When not doing atomic updates:
Look at obj->state for now. I have some patches to fix this but I was asked to
make it return a const state. This breaks a lot of users though so I skipped that
patch in this iteration.

This series will return the correct state regardless of swapping.

Maarten Lankhorst (5):
  drm/atomic: Fix atomic helpers to use the new iterator macros, v2.
  drm/atomic: Make drm_atomic_plane_disabling easier to understand.
  drm/atomic: Add macros to access existing old/new state, v2.
  drm/atomic: Convert get_existing_state callers to get_old/new_state,
    v3.
  drm/blend: Use new atomic iterator macros.

 drivers/gpu/drm/drm_atomic.c            |   6 +-
 drivers/gpu/drm/drm_atomic_helper.c     | 469 ++++++++++++++++----------------
 drivers/gpu/drm/drm_blend.c             |  23 +-
 drivers/gpu/drm/drm_plane_helper.c      |   2 +-
 drivers/gpu/drm/drm_simple_kms_helper.c |   4 +-
 include/drm/drm_atomic.h                | 108 ++++++++
 include/drm/drm_atomic_helper.h         |  26 +-
 7 files changed, 375 insertions(+), 263 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2.
  2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
@ 2017-02-16 14:47 ` Maarten Lankhorst
  2017-03-01  0:49   ` Laurent Pinchart
  2017-02-16 14:47 ` [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand Maarten Lankhorst
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

There are new iterator macros that annotate whether the new or old
state should be used. This is better than using a state that depends on
whether it's called before or after swap. For clarity, also rename the
variables from $obj_state to (old,new)_$obj_state as well.

Changes since v1:
- Use old/new_*_state for variable names as much as possible. (pinchartl)
- Expand commit message.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 431 +++++++++++++++++++-----------------
 1 file changed, 222 insertions(+), 209 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9203f3e933f7..7d432d9a18cf 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -63,14 +63,15 @@
  */
 static void
 drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
+				struct drm_plane_state *old_plane_state,
 				struct drm_plane_state *plane_state,
 				struct drm_plane *plane)
 {
 	struct drm_crtc_state *crtc_state;
 
-	if (plane->state->crtc) {
+	if (old_plane_state->crtc) {
 		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								plane->state->crtc);
+								old_plane_state->crtc);
 
 		if (WARN_ON(!crtc_state))
 			return;
@@ -92,7 +93,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 static int handle_conflicting_encoders(struct drm_atomic_state *state,
 				       bool disable_conflicting_encoders)
 {
-	struct drm_connector_state *conn_state;
+	struct drm_connector_state *new_conn_state;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
 	struct drm_encoder *encoder;
@@ -104,15 +105,15 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 	 * part of the state. If the same encoder is assigned to multiple
 	 * connectors bail out.
 	 */
-	for_each_connector_in_state(state, connector, conn_state, i) {
+	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
 		const struct drm_connector_helper_funcs *funcs = connector->helper_private;
 		struct drm_encoder *new_encoder;
 
-		if (!conn_state->crtc)
+		if (!new_conn_state->crtc)
 			continue;
 
 		if (funcs->atomic_best_encoder)
-			new_encoder = funcs->atomic_best_encoder(connector, conn_state);
+			new_encoder = funcs->atomic_best_encoder(connector, new_conn_state);
 		else if (funcs->best_encoder)
 			new_encoder = funcs->best_encoder(connector);
 		else
@@ -166,20 +167,20 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 			goto out;
 		}
 
-		conn_state = drm_atomic_get_connector_state(state, connector);
-		if (IS_ERR(conn_state)) {
-			ret = PTR_ERR(conn_state);
+		new_conn_state = drm_atomic_get_connector_state(state, connector);
+		if (IS_ERR(new_conn_state)) {
+			ret = PTR_ERR(new_conn_state);
 			goto out;
 		}
 
 		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], disabling [CONNECTOR:%d:%s]\n",
 				 encoder->base.id, encoder->name,
-				 conn_state->crtc->base.id, conn_state->crtc->name,
+				 new_conn_state->crtc->base.id, new_conn_state->crtc->name,
 				 connector->base.id, connector->name);
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state, conn_state->crtc);
+		crtc_state = drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc);
 
-		ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
+		ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL);
 		if (ret)
 			goto out;
 
@@ -245,22 +246,22 @@ steal_encoder(struct drm_atomic_state *state,
 {
 	struct drm_crtc_state *crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
+	struct drm_connector_state *old_connector_state, *new_connector_state;
 	int i;
 
-	for_each_connector_in_state(state, connector, connector_state, i) {
+	for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) {
 		struct drm_crtc *encoder_crtc;
 
-		if (connector_state->best_encoder != encoder)
+		if (new_connector_state->best_encoder != encoder)
 			continue;
 
-		encoder_crtc = connector->state->crtc;
+		encoder_crtc = old_connector_state->crtc;
 
 		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n",
 				 encoder->base.id, encoder->name,
 				 encoder_crtc->base.id, encoder_crtc->name);
 
-		set_best_encoder(state, connector_state, NULL);
+		set_best_encoder(state, new_connector_state, NULL);
 
 		crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
 		crtc_state->connectors_changed = true;
@@ -272,7 +273,8 @@ steal_encoder(struct drm_atomic_state *state,
 static int
 update_connector_routing(struct drm_atomic_state *state,
 			 struct drm_connector *connector,
-			 struct drm_connector_state *connector_state)
+			 struct drm_connector_state *old_connector_state,
+			 struct drm_connector_state *new_connector_state)
 {
 	const struct drm_connector_helper_funcs *funcs;
 	struct drm_encoder *new_encoder;
@@ -282,24 +284,24 @@ update_connector_routing(struct drm_atomic_state *state,
 			 connector->base.id,
 			 connector->name);
 
-	if (connector->state->crtc != connector_state->crtc) {
-		if (connector->state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, connector->state->crtc);
+	if (old_connector_state->crtc != new_connector_state->crtc) {
+		if (old_connector_state->crtc) {
+			crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc);
 			crtc_state->connectors_changed = true;
 		}
 
-		if (connector_state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
+		if (new_connector_state->crtc) {
+			crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
 			crtc_state->connectors_changed = true;
 		}
 	}
 
-	if (!connector_state->crtc) {
+	if (!new_connector_state->crtc) {
 		DRM_DEBUG_ATOMIC("Disabling [CONNECTOR:%d:%s]\n",
 				connector->base.id,
 				connector->name);
 
-		set_best_encoder(state, connector_state, NULL);
+		set_best_encoder(state, new_connector_state, NULL);
 
 		return 0;
 	}
@@ -308,7 +310,7 @@ update_connector_routing(struct drm_atomic_state *state,
 
 	if (funcs->atomic_best_encoder)
 		new_encoder = funcs->atomic_best_encoder(connector,
-							 connector_state);
+							 new_connector_state);
 	else if (funcs->best_encoder)
 		new_encoder = funcs->best_encoder(connector);
 	else
@@ -321,34 +323,34 @@ update_connector_routing(struct drm_atomic_state *state,
 		return -EINVAL;
 	}
 
-	if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) {
+	if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) {
 		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
 				 new_encoder->base.id,
 				 new_encoder->name,
-				 connector_state->crtc->base.id,
-				 connector_state->crtc->name);
+				 new_connector_state->crtc->base.id,
+				 new_connector_state->crtc->name);
 		return -EINVAL;
 	}
 
-	if (new_encoder == connector_state->best_encoder) {
-		set_best_encoder(state, connector_state, new_encoder);
+	if (new_encoder == new_connector_state->best_encoder) {
+		set_best_encoder(state, new_connector_state, new_encoder);
 
 		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] keeps [ENCODER:%d:%s], now on [CRTC:%d:%s]\n",
 				 connector->base.id,
 				 connector->name,
 				 new_encoder->base.id,
 				 new_encoder->name,
-				 connector_state->crtc->base.id,
-				 connector_state->crtc->name);
+				 new_connector_state->crtc->base.id,
+				 new_connector_state->crtc->name);
 
 		return 0;
 	}
 
 	steal_encoder(state, new_encoder);
 
-	set_best_encoder(state, connector_state, new_encoder);
+	set_best_encoder(state, new_connector_state, new_encoder);
 
-	crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
+	crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
 	crtc_state->connectors_changed = true;
 
 	DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n",
@@ -356,8 +358,8 @@ update_connector_routing(struct drm_atomic_state *state,
 			 connector->name,
 			 new_encoder->base.id,
 			 new_encoder->name,
-			 connector_state->crtc->base.id,
-			 connector_state->crtc->name);
+			 new_connector_state->crtc->base.id,
+			 new_connector_state->crtc->name);
 
 	return 0;
 }
@@ -366,57 +368,57 @@ static int
 mode_fixup(struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *conn_state;
+	struct drm_connector_state *new_conn_state;
 	int i;
 	int ret;
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (!crtc_state->mode_changed &&
-		    !crtc_state->connectors_changed)
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (!new_crtc_state->mode_changed &&
+		    !new_crtc_state->connectors_changed)
 			continue;
 
-		drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode);
+		drm_mode_copy(&new_crtc_state->adjusted_mode, &new_crtc_state->mode);
 	}
 
-	for_each_connector_in_state(state, connector, conn_state, i) {
+	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 
-		WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc);
+		WARN_ON(!!new_conn_state->best_encoder != !!new_conn_state->crtc);
 
-		if (!conn_state->crtc || !conn_state->best_encoder)
+		if (!new_conn_state->crtc || !new_conn_state->best_encoder)
 			continue;
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								conn_state->crtc);
+		new_crtc_state =
+			drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc);
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
 		 * it away), so we won't call ->mode_fixup twice.
 		 */
-		encoder = conn_state->best_encoder;
+		encoder = new_conn_state->best_encoder;
 		funcs = encoder->helper_private;
 
-		ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state->mode,
-				&crtc_state->adjusted_mode);
+		ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode,
+				&new_crtc_state->adjusted_mode);
 		if (!ret) {
 			DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
 			return -EINVAL;
 		}
 
 		if (funcs && funcs->atomic_check) {
-			ret = funcs->atomic_check(encoder, crtc_state,
-						  conn_state);
+			ret = funcs->atomic_check(encoder, new_crtc_state,
+						  new_conn_state);
 			if (ret) {
 				DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n",
 						 encoder->base.id, encoder->name);
 				return ret;
 			}
 		} else if (funcs && funcs->mode_fixup) {
-			ret = funcs->mode_fixup(encoder, &crtc_state->mode,
-						&crtc_state->adjusted_mode);
+			ret = funcs->mode_fixup(encoder, &new_crtc_state->mode,
+						&new_crtc_state->adjusted_mode);
 			if (!ret) {
 				DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n",
 						 encoder->base.id, encoder->name);
@@ -425,22 +427,22 @@ mode_fixup(struct drm_atomic_state *state)
 		}
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
-		if (!crtc_state->enable)
+		if (!new_crtc_state->enable)
 			continue;
 
-		if (!crtc_state->mode_changed &&
-		    !crtc_state->connectors_changed)
+		if (!new_crtc_state->mode_changed &&
+		    !new_crtc_state->connectors_changed)
 			continue;
 
 		funcs = crtc->helper_private;
 		if (!funcs->mode_fixup)
 			continue;
 
-		ret = funcs->mode_fixup(crtc, &crtc_state->mode,
-					&crtc_state->adjusted_mode);
+		ret = funcs->mode_fixup(crtc, &new_crtc_state->mode,
+					&new_crtc_state->adjusted_mode);
 		if (!ret) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n",
 					 crtc->base.id, crtc->name);
@@ -487,19 +489,19 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 				struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
+	struct drm_connector_state *old_connector_state, *new_connector_state;
 	int i, ret;
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (!drm_mode_equal(&old_crtc_state->mode, &new_crtc_state->mode)) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode changed\n",
 					 crtc->base.id, crtc->name);
-			crtc_state->mode_changed = true;
+			new_crtc_state->mode_changed = true;
 		}
 
-		if (crtc->state->enable != crtc_state->enable) {
+		if (old_crtc_state->enable != new_crtc_state->enable) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enable changed\n",
 					 crtc->base.id, crtc->name);
 
@@ -511,8 +513,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 			 * The other way around is true as well. enable != 0
 			 * iff connectors are attached and a mode is set.
 			 */
-			crtc_state->mode_changed = true;
-			crtc_state->connectors_changed = true;
+			new_crtc_state->mode_changed = true;
+			new_crtc_state->connectors_changed = true;
 		}
 	}
 
@@ -520,14 +522,15 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 	if (ret)
 		return ret;
 
-	for_each_connector_in_state(state, connector, connector_state, i) {
+	for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) {
 		/*
 		 * This only sets crtc->connectors_changed for routing changes,
 		 * drivers must set crtc->connectors_changed themselves when
 		 * connector properties need to be updated.
 		 */
 		ret = update_connector_routing(state, connector,
-					       connector_state);
+					       old_connector_state,
+					       new_connector_state);
 		if (ret)
 			return ret;
 	}
@@ -538,28 +541,28 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 	 * configuration. This must be done before calling mode_fixup in case a
 	 * crtc only changed its mode but has the same set of connectors.
 	 */
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		bool has_connectors =
-			!!crtc_state->connector_mask;
+			!!new_crtc_state->connector_mask;
 
 		/*
 		 * We must set ->active_changed after walking connectors for
 		 * otherwise an update that only changes active would result in
 		 * a full modeset because update_connector_routing force that.
 		 */
-		if (crtc->state->active != crtc_state->active) {
+		if (old_crtc_state->active != new_crtc_state->active) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active changed\n",
 					 crtc->base.id, crtc->name);
-			crtc_state->active_changed = true;
+			new_crtc_state->active_changed = true;
 		}
 
-		if (!drm_atomic_crtc_needs_modeset(crtc_state))
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
 			continue;
 
 		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] needs all connectors, enable: %c, active: %c\n",
 				 crtc->base.id, crtc->name,
-				 crtc_state->enable ? 'y' : 'n',
-				 crtc_state->active ? 'y' : 'n');
+				 new_crtc_state->enable ? 'y' : 'n',
+				 new_crtc_state->active ? 'y' : 'n');
 
 		ret = drm_atomic_add_affected_connectors(state, crtc);
 		if (ret != 0)
@@ -569,7 +572,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 		if (ret != 0)
 			return ret;
 
-		if (crtc_state->enable != has_connectors) {
+		if (new_crtc_state->enable != has_connectors) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n",
 					 crtc->base.id, crtc->name);
 
@@ -602,22 +605,22 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
 			       struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state, *old_plane_state;
 	int i, ret = 0;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
+	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
 		funcs = plane->helper_private;
 
-		drm_atomic_helper_plane_changed(state, plane_state, plane);
+		drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, plane);
 
 		if (!funcs || !funcs->atomic_check)
 			continue;
 
-		ret = funcs->atomic_check(plane, plane_state);
+		ret = funcs->atomic_check(plane, new_plane_state);
 		if (ret) {
 			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n",
 					 plane->base.id, plane->name);
@@ -625,7 +628,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
 		}
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		funcs = crtc->helper_private;
@@ -633,7 +636,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
 		if (!funcs || !funcs->atomic_check)
 			continue;
 
-		ret = funcs->atomic_check(crtc, crtc_state);
+		ret = funcs->atomic_check(crtc, new_crtc_state);
 		if (ret) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n",
 					 crtc->base.id, crtc->name);
@@ -687,12 +690,12 @@ static void
 disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 {
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	int i;
 
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+	for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 
@@ -729,7 +732,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 
 		/* Right function depends upon target state. */
 		if (funcs) {
-			if (connector->state->crtc && funcs->prepare)
+			if (new_conn_state->crtc && funcs->prepare)
 				funcs->prepare(encoder);
 			else if (funcs->disable)
 				funcs->disable(encoder);
@@ -740,11 +743,11 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 		drm_bridge_post_disable(encoder->bridge);
 	}
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		/* Shut down everything that needs a full modeset. */
-		if (!drm_atomic_crtc_needs_modeset(crtc->state))
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
 			continue;
 
 		if (!old_crtc_state->active)
@@ -757,7 +760,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 
 
 		/* Right function depends upon target state. */
-		if (crtc->state->enable && funcs->prepare)
+		if (new_crtc_state->enable && funcs->prepare)
 			funcs->prepare(crtc);
 		else if (funcs->atomic_disable)
 			funcs->atomic_disable(crtc, old_crtc_state);
@@ -786,13 +789,13 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 					      struct drm_atomic_state *old_state)
 {
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	int i;
 
 	/* clear out existing links and update dpms */
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+	for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) {
 		if (connector->encoder) {
 			WARN_ON(!connector->encoder->crtc);
 
@@ -800,7 +803,7 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 			connector->encoder = NULL;
 		}
 
-		crtc = connector->state->crtc;
+		crtc = new_conn_state->crtc;
 		if ((!crtc && old_conn_state->crtc) ||
 		    (crtc && drm_atomic_crtc_needs_modeset(crtc->state))) {
 			struct drm_property *dpms_prop =
@@ -817,23 +820,23 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 	}
 
 	/* set new links */
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
-		if (!connector->state->crtc)
+	for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
+		if (!new_conn_state->crtc)
 			continue;
 
-		if (WARN_ON(!connector->state->best_encoder))
+		if (WARN_ON(!new_conn_state->best_encoder))
 			continue;
 
-		connector->encoder = connector->state->best_encoder;
-		connector->encoder->crtc = connector->state->crtc;
+		connector->encoder = new_conn_state->best_encoder;
+		connector->encoder->crtc = new_conn_state->crtc;
 	}
 
 	/* set legacy state in the crtc structure */
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		struct drm_plane *primary = crtc->primary;
 
-		crtc->mode = crtc->state->mode;
-		crtc->enabled = crtc->state->enable;
+		crtc->mode = new_crtc_state->mode;
+		crtc->enabled = new_crtc_state->enable;
 
 		if (drm_atomic_get_existing_plane_state(old_state, primary) &&
 		    primary->state->crtc == crtc) {
@@ -841,9 +844,9 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 			crtc->y = primary->state->src_y >> 16;
 		}
 
-		if (crtc->state->enable)
+		if (new_crtc_state->enable)
 			drm_calc_timestamping_constants(crtc,
-							&crtc->state->adjusted_mode);
+							&new_crtc_state->adjusted_mode);
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_update_legacy_modeset_state);
@@ -852,20 +855,20 @@ static void
 crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *new_conn_state;
 	int i;
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
-		if (!crtc->state->mode_changed)
+		if (!new_crtc_state->mode_changed)
 			continue;
 
 		funcs = crtc->helper_private;
 
-		if (crtc->state->enable && funcs->mode_set_nofb) {
+		if (new_crtc_state->enable && funcs->mode_set_nofb) {
 			DRM_DEBUG_ATOMIC("modeset on [CRTC:%d:%s]\n",
 					 crtc->base.id, crtc->name);
 
@@ -873,18 +876,17 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 		}
 	}
 
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+	for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
-		struct drm_crtc_state *new_crtc_state;
 		struct drm_encoder *encoder;
 		struct drm_display_mode *mode, *adjusted_mode;
 
-		if (!connector->state->best_encoder)
+		if (!new_conn_state->best_encoder)
 			continue;
 
-		encoder = connector->state->best_encoder;
+		encoder = new_conn_state->best_encoder;
 		funcs = encoder->helper_private;
-		new_crtc_state = connector->state->crtc->state;
+		new_crtc_state = new_conn_state->crtc->state;
 		mode = &new_crtc_state->mode;
 		adjusted_mode = &new_crtc_state->adjusted_mode;
 
@@ -900,7 +902,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 		 */
 		if (funcs && funcs->atomic_mode_set) {
 			funcs->atomic_mode_set(encoder, new_crtc_state,
-					       connector->state);
+					       new_conn_state);
 		} else if (funcs && funcs->mode_set) {
 			funcs->mode_set(encoder, mode, adjusted_mode);
 		}
@@ -952,24 +954,24 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 					      struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *new_conn_state;
 	int i;
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		/* Need to filter out CRTCs where only planes change. */
-		if (!drm_atomic_crtc_needs_modeset(crtc->state))
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
 			continue;
 
-		if (!crtc->state->active)
+		if (!new_crtc_state->active)
 			continue;
 
 		funcs = crtc->helper_private;
 
-		if (crtc->state->enable) {
+		if (new_crtc_state->enable) {
 			DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n",
 					 crtc->base.id, crtc->name);
 
@@ -980,18 +982,18 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 		}
 	}
 
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+	for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 
-		if (!connector->state->best_encoder)
+		if (!new_conn_state->best_encoder)
 			continue;
 
-		if (!connector->state->crtc->state->active ||
-		    !drm_atomic_crtc_needs_modeset(connector->state->crtc->state))
+		if (!new_conn_state->crtc->state->active ||
+		    !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state))
 			continue;
 
-		encoder = connector->state->best_encoder;
+		encoder = new_conn_state->best_encoder;
 		funcs = encoder->helper_private;
 
 		DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n",
@@ -1041,29 +1043,26 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
 				      bool pre_swap)
 {
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state;
 	int i, ret;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		if (!pre_swap)
-			plane_state = plane->state;
-
-		if (!plane_state->fence)
+	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+		if (!new_plane_state->fence)
 			continue;
 
-		WARN_ON(!plane_state->fb);
+		WARN_ON(!new_plane_state->fb);
 
 		/*
 		 * If waiting for fences pre-swap (ie: nonblock), userspace can
 		 * still interrupt the operation. Instead of blocking until the
 		 * timer expires, make the wait interruptible.
 		 */
-		ret = dma_fence_wait(plane_state->fence, pre_swap);
+		ret = dma_fence_wait(new_plane_state->fence, pre_swap);
 		if (ret)
 			return ret;
 
-		dma_fence_put(plane_state->fence);
-		plane_state->fence = NULL;
+		dma_fence_put(new_plane_state->fence);
+		new_plane_state->fence = NULL;
 	}
 
 	return 0;
@@ -1086,7 +1085,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 		struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	int i, ret;
 	unsigned crtc_mask = 0;
 
@@ -1097,9 +1096,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 	if (old_state->legacy_cursor_update)
 		return;
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
-		struct drm_crtc_state *new_crtc_state = crtc->state;
-
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		if (!new_crtc_state->active || !new_crtc_state->planes_changed)
 			continue;
 
@@ -1111,7 +1108,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 		old_state->crtcs[i].last_vblank_count = drm_crtc_vblank_count(crtc);
 	}
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
 		if (!(crtc_mask & drm_crtc_mask(crtc)))
 			continue;
 
@@ -1420,11 +1417,11 @@ 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_state *old_crtc_state, *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i, ret;
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		commit = kzalloc(sizeof(*commit), GFP_KERNEL);
 		if (!commit)
 			return -ENOMEM;
@@ -1445,7 +1442,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
 		/* 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) {
+		if (!old_crtc_state->active && !new_crtc_state->active) {
 			complete_all(&commit->flip_done);
 			continue;
 		}
@@ -1456,17 +1453,17 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
 			continue;
 		}
 
-		if (!crtc_state->event) {
+		if (!new_crtc_state->event) {
 			commit->event = kzalloc(sizeof(*commit->event),
 						GFP_KERNEL);
 			if (!commit->event)
 				return -ENOMEM;
 
-			crtc_state->event = commit->event;
+			new_crtc_state->event = commit->event;
 		}
 
-		crtc_state->event->base.completion = &commit->flip_done;
-		crtc_state->event->base.completion_release = release_crtc_commit;
+		new_crtc_state->event->base.completion = &commit->flip_done;
+		new_crtc_state->event->base.completion_release = release_crtc_commit;
 		drm_crtc_commit_get(commit);
 	}
 
@@ -1505,12 +1502,12 @@ 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 *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 	long ret;
 
-	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		spin_lock(&crtc->commit_lock);
 		commit = preceeding_commit(crtc);
 		if (commit)
@@ -1557,17 +1554,17 @@ 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 *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 
-	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		commit = old_state->crtcs[i].commit;
 		if (!commit)
 			continue;
 
 		/* backend must have consumed any event by now */
-		WARN_ON(crtc->state->event);
+		WARN_ON(new_crtc_state->event);
 		spin_lock(&crtc->commit_lock);
 		complete_all(&commit->hw_done);
 		spin_unlock(&crtc->commit_lock);
@@ -1589,12 +1586,12 @@ 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 *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 	long ret;
 
-	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		commit = old_state->crtcs[i].commit;
 		if (WARN_ON(!commit))
 			continue;
@@ -1645,16 +1642,16 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state)
 {
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state;
 	int ret, i, j;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
+	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
 		funcs = plane->helper_private;
 
 		if (funcs->prepare_fb) {
-			ret = funcs->prepare_fb(plane, plane_state);
+			ret = funcs->prepare_fb(plane, new_plane_state);
 			if (ret)
 				goto fail;
 		}
@@ -1663,7 +1660,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 	return 0;
 
 fail:
-	for_each_plane_in_state(state, plane, plane_state, j) {
+	for_each_new_plane_in_state(state, plane, new_plane_state, j) {
 		const struct drm_plane_helper_funcs *funcs;
 
 		if (j >= i)
@@ -1672,7 +1669,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
-			funcs->cleanup_fb(plane, plane_state);
+			funcs->cleanup_fb(plane, new_plane_state);
 	}
 
 	return ret;
@@ -1730,14 +1727,14 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 				     uint32_t flags)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *old_plane_state;
+	struct drm_plane_state *old_plane_state, *new_plane_state;
 	int i;
 	bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY;
 	bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET;
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		funcs = crtc->helper_private;
@@ -1745,13 +1742,13 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 		if (!funcs || !funcs->atomic_begin)
 			continue;
 
-		if (active_only && !crtc->state->active)
+		if (active_only && !new_crtc_state->active)
 			continue;
 
 		funcs->atomic_begin(crtc, old_crtc_state);
 	}
 
-	for_each_plane_in_state(old_state, plane, old_plane_state, i) {
+	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 		bool disabling;
 
@@ -1770,7 +1767,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 			 * CRTC to avoid skipping planes being disabled on an
 			 * active CRTC.
 			 */
-			if (!disabling && !plane_crtc_active(plane->state))
+			if (!disabling && !plane_crtc_active(new_plane_state))
 				continue;
 			if (disabling && !plane_crtc_active(old_plane_state))
 				continue;
@@ -1789,12 +1786,12 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 				continue;
 
 			funcs->atomic_disable(plane, old_plane_state);
-		} else if (plane->state->crtc || disabling) {
+		} else if (new_plane_state->crtc || disabling) {
 			funcs->atomic_update(plane, old_plane_state);
 		}
 	}
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		funcs = crtc->helper_private;
@@ -1802,7 +1799,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 		if (!funcs || !funcs->atomic_flush)
 			continue;
 
-		if (active_only && !crtc->state->active)
+		if (active_only && !new_crtc_state->active)
 			continue;
 
 		funcs->atomic_flush(crtc, old_crtc_state);
@@ -1929,11 +1926,21 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 				      struct drm_atomic_state *old_state)
 {
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *old_plane_state, *new_plane_state;
 	int i;
 
-	for_each_plane_in_state(old_state, plane, plane_state, i) {
+	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
+		struct drm_plane_state *plane_state;
+
+		/*
+		 * This might be called before swapping when commit is aborted,
+		 * in which case we have to free the new state.
+		 */
+		if (old_plane_state == plane->state)
+			plane_state = new_plane_state;
+		else
+			plane_state = old_plane_state;
 
 		funcs = plane->helper_private;
 
@@ -1979,15 +1986,15 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 	int i;
 	long ret;
 	struct drm_connector *connector;
-	struct drm_connector_state *conn_state, *old_conn_state;
+	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state, *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state, *old_plane_state;
+	struct drm_plane_state *old_plane_state, *new_plane_state;
 	struct drm_crtc_commit *commit;
 
 	if (stall) {
-		for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		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);
@@ -2007,20 +2014,24 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		}
 	}
 
-	for_each_oldnew_connector_in_state(state, connector, old_conn_state, conn_state, i) {
+	for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_conn_state, i) {
 		WARN_ON(connector->state != old_conn_state);
 
-		connector->state->state = state;
-		swap(state->connectors[i].state, connector->state);
-		connector->state->state = NULL;
+		old_conn_state->state = state;
+		new_conn_state->state = NULL;
+
+		state->connectors[i].state = old_conn_state;
+		connector->state = new_conn_state;
 	}
 
-	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		WARN_ON(crtc->state != old_crtc_state);
 
-		crtc->state->state = state;
-		swap(state->crtcs[i].state, crtc->state);
-		crtc->state->state = NULL;
+		old_crtc_state->state = state;
+		new_crtc_state->state = NULL;
+
+		state->crtcs[i].state = old_crtc_state;
+		crtc->state = new_crtc_state;
 
 		if (state->crtcs[i].commit) {
 			spin_lock(&crtc->commit_lock);
@@ -2032,12 +2043,14 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		}
 	}
 
-	for_each_oldnew_plane_in_state(state, plane, old_plane_state, plane_state, i) {
+	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
 		WARN_ON(plane->state != old_plane_state);
 
-		plane->state->state = state;
-		swap(state->planes[i].state, plane->state);
-		plane->state->state = NULL;
+		old_plane_state->state = state;
+		new_plane_state->state = NULL;
+
+		state->planes[i].state = old_plane_state;
+		plane->state = new_plane_state;
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_swap_state);
@@ -2220,9 +2233,9 @@ static int update_output_state(struct drm_atomic_state *state,
 {
 	struct drm_device *dev = set->crtc->dev;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *conn_state;
+	struct drm_connector_state *new_conn_state;
 	int ret, i;
 
 	ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
@@ -2235,9 +2248,9 @@ static int update_output_state(struct drm_atomic_state *state,
 	if (ret)
 		return ret;
 
-	for_each_connector_in_state(state, connector, conn_state, i) {
-		if (conn_state->crtc == set->crtc) {
-			ret = drm_atomic_set_crtc_for_connector(conn_state,
+	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
+		if (new_conn_state->crtc == set->crtc) {
+			ret = drm_atomic_set_crtc_for_connector(new_conn_state,
 								NULL);
 			if (ret)
 				return ret;
@@ -2246,18 +2259,18 @@ static int update_output_state(struct drm_atomic_state *state,
 
 	/* Then set all connectors from set->connectors on the target crtc */
 	for (i = 0; i < set->num_connectors; i++) {
-		conn_state = drm_atomic_get_connector_state(state,
+		new_conn_state = drm_atomic_get_connector_state(state,
 							    set->connectors[i]);
-		if (IS_ERR(conn_state))
-			return PTR_ERR(conn_state);
+		if (IS_ERR(new_conn_state))
+			return PTR_ERR(new_conn_state);
 
-		ret = drm_atomic_set_crtc_for_connector(conn_state,
+		ret = drm_atomic_set_crtc_for_connector(new_conn_state,
 							set->crtc);
 		if (ret)
 			return ret;
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 		/* Don't update ->enable for the CRTC in the set_config request,
 		 * since a mismatch would indicate a bug in the upper layers.
 		 * The actual modeset code later on will catch any
@@ -2265,13 +2278,13 @@ static int update_output_state(struct drm_atomic_state *state,
 		if (crtc == set->crtc)
 			continue;
 
-		if (!crtc_state->connector_mask) {
-			ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
+		if (!new_crtc_state->connector_mask) {
+			ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
 								NULL);
 			if (ret < 0)
 				return ret;
 
-			crtc_state->active = false;
+			new_crtc_state->active = false;
 		}
 	}
 
@@ -2545,21 +2558,21 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
 {
 	int i;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *conn_state;
+	struct drm_connector_state *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 
 	state->acquire_ctx = ctx;
 
-	for_each_new_plane_in_state(state, plane, plane_state, i)
+	for_each_new_plane_in_state(state, plane, new_plane_state, i)
 		state->planes[i].old_state = plane->state;
 
-	for_each_new_crtc_in_state(state, crtc, crtc_state, i)
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
 		state->crtcs[i].old_state = crtc->state;
 
-	for_each_new_connector_in_state(state, connector, conn_state, i)
+	for_each_new_connector_in_state(state, connector, new_conn_state, i)
 		state->connectors[i].old_state = connector->state;
 
 	return drm_atomic_commit(state);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand.
  2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
  2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst
@ 2017-02-16 14:47 ` Maarten Lankhorst
  2017-03-01  0:53   ` Laurent Pinchart
  2017-02-16 14:47 ` [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2 Maarten Lankhorst
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

This function becomes a lot simpler when having passed both the old and
new state to it. Looking at all callers, it seems that old_plane_state
is never NULL so the check can be dropped.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c |  7 ++++---
 drivers/gpu/drm/drm_plane_helper.c  |  2 +-
 include/drm/drm_atomic_helper.h     | 26 ++++++++------------------
 3 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 7d432d9a18cf..ea544bddc29b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1757,7 +1757,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 		if (!funcs)
 			continue;
 
-		disabling = drm_atomic_plane_disabling(plane, old_plane_state);
+		disabling = drm_atomic_plane_disabling(old_plane_state,
+						       new_plane_state);
 
 		if (active_only) {
 			/*
@@ -1852,11 +1853,11 @@ drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state)
 
 		WARN_ON(plane->state->crtc && plane->state->crtc != crtc);
 
-		if (drm_atomic_plane_disabling(plane, old_plane_state) &&
+		if (drm_atomic_plane_disabling(old_plane_state, plane->state) &&
 		    plane_funcs->atomic_disable)
 			plane_funcs->atomic_disable(plane, old_plane_state);
 		else if (plane->state->crtc ||
-			 drm_atomic_plane_disabling(plane, old_plane_state))
+			 drm_atomic_plane_disabling(old_plane_state, plane->state))
 			plane_funcs->atomic_update(plane, old_plane_state);
 	}
 
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 148688fb920a..f4d70dd5e3e4 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -470,7 +470,7 @@ int drm_plane_helper_commit(struct drm_plane *plane,
 	 * Drivers may optionally implement the ->atomic_disable callback, so
 	 * special-case that here.
 	 */
-	if (drm_atomic_plane_disabling(plane, plane_state) &&
+	if (drm_atomic_plane_disabling(plane_state, plane->state) &&
 	    plane_funcs->atomic_disable)
 		plane_funcs->atomic_disable(plane, plane_state);
 	else
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 9ceda379ce58..dc16274987c7 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -220,10 +220,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 			      __drm_atomic_get_current_plane_state((crtc_state)->state, \
 								   plane)))
 
-/*
+/**
  * drm_atomic_plane_disabling - check whether a plane is being disabled
- * @plane: plane object
- * @old_state: previous atomic state
+ * @old_plane_state: old atomic plane state
+ * @new_plane_state: new atomic plane state
  *
  * Checks the atomic state of a plane to determine whether it's being disabled
  * or not. This also WARNs if it detects an invalid state (both CRTC and FB
@@ -233,28 +233,18 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
  * True if the plane is being disabled, false otherwise.
  */
 static inline bool
-drm_atomic_plane_disabling(struct drm_plane *plane,
-			   struct drm_plane_state *old_state)
+drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state,
+			   struct drm_plane_state *new_plane_state)
 {
 	/*
 	 * When disabling a plane, CRTC and FB should always be NULL together.
 	 * Anything else should be considered a bug in the atomic core, so we
 	 * gently warn about it.
 	 */
-	WARN_ON((plane->state->crtc == NULL && plane->state->fb != NULL) ||
-		(plane->state->crtc != NULL && plane->state->fb == NULL));
+	WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) ||
+		(new_plane_state->crtc != NULL && new_plane_state->fb == NULL));
 
-	/*
-	 * When using the transitional helpers, old_state may be NULL. If so,
-	 * we know nothing about the current state and have to assume that it
-	 * might be enabled.
-	 *
-	 * When using the atomic helpers, old_state won't be NULL. Therefore
-	 * this check assumes that either the driver will have reconstructed
-	 * the correct state in ->reset() or that the driver will have taken
-	 * appropriate measures to disable all planes.
-	 */
-	return (!old_state || old_state->crtc) && !plane->state->crtc;
+	return old_plane_state->crtc && !new_plane_state->crtc;
 }
 
 #endif /* DRM_ATOMIC_HELPER_H_ */
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2.
  2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
  2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst
  2017-02-16 14:47 ` [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand Maarten Lankhorst
@ 2017-02-16 14:47 ` Maarten Lankhorst
  2017-03-01  0:56   ` Laurent Pinchart
  2017-02-16 14:47 ` [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3 Maarten Lankhorst
  2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst
  4 siblings, 1 reply; 15+ messages in thread
From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

After atomic commit, these macros should be used in place of
get_existing_state. Also after commit get_xx_state should no longer
be used because it may not have the required locks.

The calls to drm_atomic_get_existing_$obj_state should no longer be
used, and converted over to these new calls.

Changes since v1:
- Expand commit message.
- Deprecate get_existing_*_state functions in the documentation.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 include/drm/drm_atomic.h | 108 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index c6f355a970d2..0147a047878d 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -277,6 +277,9 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
  *
  * This function returns the crtc state for the given crtc, or NULL
  * if the crtc is not part of the global atomic state.
+ *
+ * This function is deprecated, @drm_atomic_get_old_crtc_state or
+ * @drm_atomic_get_new_crtc_state should be used instead.
  */
 static inline struct drm_crtc_state *
 drm_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
@@ -286,12 +289,44 @@ drm_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
 }
 
 /**
+ * drm_atomic_get_old_crtc_state - get old crtc state, if it exists
+ * @state: global atomic state object
+ * @crtc: crtc to grab
+ *
+ * This function returns the old crtc state for the given crtc, or
+ * NULL if the crtc is not part of the global atomic state.
+ */
+static inline struct drm_crtc_state *
+drm_atomic_get_old_crtc_state(struct drm_atomic_state *state,
+			      struct drm_crtc *crtc)
+{
+	return state->crtcs[drm_crtc_index(crtc)].old_state;
+}
+/**
+ * drm_atomic_get_new_crtc_state - get new crtc state, if it exists
+ * @state: global atomic state object
+ * @crtc: crtc to grab
+ *
+ * This function returns the new crtc state for the given crtc, or
+ * NULL if the crtc is not part of the global atomic state.
+ */
+static inline struct drm_crtc_state *
+drm_atomic_get_new_crtc_state(struct drm_atomic_state *state,
+			      struct drm_crtc *crtc)
+{
+	return state->crtcs[drm_crtc_index(crtc)].new_state;
+}
+
+/**
  * drm_atomic_get_existing_plane_state - get plane state, if it exists
  * @state: global atomic state object
  * @plane: plane to grab
  *
  * This function returns the plane state for the given plane, or NULL
  * if the plane is not part of the global atomic state.
+ *
+ * This function is deprecated, @drm_atomic_get_old_plane_state or
+ * @drm_atomic_get_new_plane_state should be used instead.
  */
 static inline struct drm_plane_state *
 drm_atomic_get_existing_plane_state(struct drm_atomic_state *state,
@@ -301,12 +336,45 @@ drm_atomic_get_existing_plane_state(struct drm_atomic_state *state,
 }
 
 /**
+ * drm_atomic_get_old_plane_state - get plane state, if it exists
+ * @state: global atomic state object
+ * @plane: plane to grab
+ *
+ * This function returns the old plane state for the given plane, or
+ * NULL if the plane is not part of the global atomic state.
+ */
+static inline struct drm_plane_state *
+drm_atomic_get_old_plane_state(struct drm_atomic_state *state,
+			       struct drm_plane *plane)
+{
+	return state->planes[drm_plane_index(plane)].old_state;
+}
+
+/**
+ * drm_atomic_get_new_plane_state - get plane state, if it exists
+ * @state: global atomic state object
+ * @plane: plane to grab
+ *
+ * This function returns the new plane state for the given plane, or
+ * NULL if the plane is not part of the global atomic state.
+ */
+static inline struct drm_plane_state *
+drm_atomic_get_new_plane_state(struct drm_atomic_state *state,
+			       struct drm_plane *plane)
+{
+	return state->planes[drm_plane_index(plane)].new_state;
+}
+
+/**
  * drm_atomic_get_existing_connector_state - get connector state, if it exists
  * @state: global atomic state object
  * @connector: connector to grab
  *
  * This function returns the connector state for the given connector,
  * or NULL if the connector is not part of the global atomic state.
+ *
+ * This function is deprecated, @drm_atomic_get_old_connector_state or
+ * @drm_atomic_get_new_connector_state should be used instead.
  */
 static inline struct drm_connector_state *
 drm_atomic_get_existing_connector_state(struct drm_atomic_state *state,
@@ -321,6 +389,46 @@ drm_atomic_get_existing_connector_state(struct drm_atomic_state *state,
 }
 
 /**
+ * drm_atomic_get_old_connector_state - get connector state, if it exists
+ * @state: global atomic state object
+ * @connector: connector to grab
+ *
+ * This function returns the old connector state for the given connector,
+ * or NULL if the connector is not part of the global atomic state.
+ */
+static inline struct drm_connector_state *
+drm_atomic_get_old_connector_state(struct drm_atomic_state *state,
+				   struct drm_connector *connector)
+{
+	int index = drm_connector_index(connector);
+
+	if (index >= state->num_connector)
+		return NULL;
+
+	return state->connectors[index].old_state;
+}
+
+/**
+ * drm_atomic_get_new_connector_state - get connector state, if it exists
+ * @state: global atomic state object
+ * @connector: connector to grab
+ *
+ * This function returns the new connector state for the given connector,
+ * or NULL if the connector is not part of the global atomic state.
+ */
+static inline struct drm_connector_state *
+drm_atomic_get_new_connector_state(struct drm_atomic_state *state,
+				   struct drm_connector *connector)
+{
+	int index = drm_connector_index(connector);
+
+	if (index >= state->num_connector)
+		return NULL;
+
+	return state->connectors[index].new_state;
+}
+
+/**
  * __drm_atomic_get_current_plane_state - get current plane state
  * @state: global atomic state object
  * @plane: plane to grab
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3.
  2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
                   ` (2 preceding siblings ...)
  2017-02-16 14:47 ` [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2 Maarten Lankhorst
@ 2017-02-16 14:47 ` Maarten Lankhorst
  2017-03-01  1:04   ` Laurent Pinchart
  2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst
  4 siblings, 1 reply; 15+ messages in thread
From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

This is a straightforward conversion that converts all the users of
get_existing_state in atomic core to use get_old_state or get_new_state

Changes since v1:
- Fix using the wrong state in drm_atomic_helper_update_legacy_modeset_state.
Changes since v2:
- Use the correct state in disable_outputs()

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c            |  6 ++---
 drivers/gpu/drm/drm_atomic_helper.c     | 43 +++++++++++++++++----------------
 drivers/gpu/drm/drm_blend.c             |  3 +--
 drivers/gpu/drm/drm_simple_kms_helper.c |  4 +--
 4 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index afec53832145..c9aac9cbf111 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1358,8 +1358,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
 		return 0;
 
 	if (conn_state->crtc) {
-		crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state,
-								conn_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(conn_state->state,
+							   conn_state->crtc);
 
 		crtc_state->connector_mask &=
 			~(1 << drm_connector_index(conn_state->connector));
@@ -1476,7 +1476,7 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state,
 {
 	struct drm_plane *plane;
 
-	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
+	WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc));
 
 	drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) {
 		struct drm_plane_state *plane_state =
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ea544bddc29b..6296ae6e6aff 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -70,8 +70,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 	struct drm_crtc_state *crtc_state;
 
 	if (old_plane_state->crtc) {
-		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								old_plane_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state,
+							   old_plane_state->crtc);
 
 		if (WARN_ON(!crtc_state))
 			return;
@@ -80,8 +80,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 	}
 
 	if (plane_state->crtc) {
-		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								plane_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc);
 
 		if (WARN_ON(!crtc_state))
 			return;
@@ -150,7 +149,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct drm_crtc_state *crtc_state;
 
-		if (drm_atomic_get_existing_connector_state(state, connector))
+		if (drm_atomic_get_new_connector_state(state, connector))
 			continue;
 
 		encoder = connector->state->best_encoder;
@@ -178,7 +177,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 				 new_conn_state->crtc->base.id, new_conn_state->crtc->name,
 				 connector->base.id, connector->name);
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc);
 
 		ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL);
 		if (ret)
@@ -219,7 +218,7 @@ set_best_encoder(struct drm_atomic_state *state,
 		 */
 		WARN_ON(!crtc && encoder != conn_state->best_encoder);
 		if (crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
 			crtc_state->encoder_mask &=
 				~(1 << drm_encoder_index(conn_state->best_encoder));
@@ -230,7 +229,7 @@ set_best_encoder(struct drm_atomic_state *state,
 		crtc = conn_state->crtc;
 		WARN_ON(!crtc);
 		if (crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
 			crtc_state->encoder_mask |=
 				1 << drm_encoder_index(encoder);
@@ -263,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
 
 		set_best_encoder(state, new_connector_state, NULL);
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state, encoder_crtc);
 		crtc_state->connectors_changed = true;
 
 		return;
@@ -286,12 +285,12 @@ update_connector_routing(struct drm_atomic_state *state,
 
 	if (old_connector_state->crtc != new_connector_state->crtc) {
 		if (old_connector_state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, old_connector_state->crtc);
 			crtc_state->connectors_changed = true;
 		}
 
 		if (new_connector_state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, new_connector_state->crtc);
 			crtc_state->connectors_changed = true;
 		}
 	}
@@ -350,7 +349,7 @@ update_connector_routing(struct drm_atomic_state *state,
 
 	set_best_encoder(state, new_connector_state, new_encoder);
 
-	crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
+	crtc_state = drm_atomic_get_new_crtc_state(state, new_connector_state->crtc);
 	crtc_state->connectors_changed = true;
 
 	DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n",
@@ -392,7 +391,7 @@ mode_fixup(struct drm_atomic_state *state)
 			continue;
 
 		new_crtc_state =
-			drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc);
+			drm_atomic_get_new_crtc_state(state, new_conn_state->crtc);
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
@@ -704,8 +703,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 		if (!old_conn_state->crtc)
 			continue;
 
-		old_crtc_state = drm_atomic_get_existing_crtc_state(old_state,
-								    old_conn_state->crtc);
+		old_crtc_state = drm_atomic_get_old_crtc_state(old_state, old_conn_state->crtc);
 
 		if (!old_crtc_state->active ||
 		    !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
@@ -834,14 +832,17 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 	/* set legacy state in the crtc structure */
 	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		struct drm_plane *primary = crtc->primary;
+		struct drm_plane_state *new_plane_state;
 
 		crtc->mode = new_crtc_state->mode;
 		crtc->enabled = new_crtc_state->enable;
 
-		if (drm_atomic_get_existing_plane_state(old_state, primary) &&
-		    primary->state->crtc == crtc) {
-			crtc->x = primary->state->src_x >> 16;
-			crtc->y = primary->state->src_y >> 16;
+		new_plane_state =
+			drm_atomic_get_new_plane_state(old_state, primary);
+
+		if (new_plane_state && new_plane_state->crtc == crtc) {
+			crtc->x = new_plane_state->src_x >> 16;
+			crtc->y = new_plane_state->src_y >> 16;
 		}
 
 		if (new_crtc_state->enable)
@@ -1843,7 +1844,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state)
 
 	drm_for_each_plane_mask(plane, crtc->dev, plane_mask) {
 		struct drm_plane_state *old_plane_state =
-			drm_atomic_get_existing_plane_state(old_state, plane);
+			drm_atomic_get_old_plane_state(old_state, plane);
 		const struct drm_plane_helper_funcs *plane_funcs;
 
 		plane_funcs = plane->helper_private;
@@ -2914,7 +2915,7 @@ int drm_atomic_helper_page_flip_target(
 	if (ret != 0)
 		goto fail;
 
-	crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	if (WARN_ON(!crtc_state)) {
 		ret = -EINVAL;
 		goto fail;
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 665aafc6ad68..d7053bb67db6 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -387,8 +387,7 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
 		if (!crtc)
 			continue;
 		if (plane->state->zpos != plane_state->zpos) {
-			crtc_state =
-				drm_atomic_get_existing_crtc_state(state, crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 			crtc_state->zpos_changed = true;
 		}
 	}
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 35c5d99296b9..16789faa9291 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -86,8 +86,8 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
 	int ret;
 
 	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
-	crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,
-							&pipe->crtc);
+	crtc_state = drm_atomic_get_new_crtc_state(plane_state->state,
+						   &pipe->crtc);
 	if (crtc_state->enable != !!plane_state->crtc)
 		return -EINVAL; /* plane must match crtc enable state */
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 5/5] drm/blend: Use new atomic iterator macros.
  2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
                   ` (3 preceding siblings ...)
  2017-02-16 14:47 ` [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3 Maarten Lankhorst
@ 2017-02-16 14:47 ` Maarten Lankhorst
  2017-03-01  0:58   ` Laurent Pinchart
  4 siblings, 1 reply; 15+ messages in thread
From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

There are new iterator macros that annotate whether the new or old
state should be used. This is better than using a state that depends on
whether it's called before or after swap. For clarity, also rename the
variables from $obj_state to (old,new)_$obj_state as well.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_blend.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index d7053bb67db6..a0d0d6843288 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -377,26 +377,26 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
 			      struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *old_plane_state, *new_plane_state;
 	int i, ret = 0;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		crtc = plane_state->crtc;
+	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
+		crtc = new_plane_state->crtc;
 		if (!crtc)
 			continue;
-		if (plane->state->zpos != plane_state->zpos) {
-			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
-			crtc_state->zpos_changed = true;
+		if (old_plane_state->zpos != new_plane_state->zpos) {
+			new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+			new_crtc_state->zpos_changed = true;
 		}
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (crtc_state->plane_mask != crtc->state->plane_mask ||
-		    crtc_state->zpos_changed) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (old_crtc_state->plane_mask != new_crtc_state->plane_mask ||
+		    new_crtc_state->zpos_changed) {
 			ret = drm_atomic_helper_crtc_normalize_zpos(crtc,
-								    crtc_state);
+								    new_crtc_state);
 			if (ret)
 				return ret;
 		}
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2.
  2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst
@ 2017-03-01  0:49   ` Laurent Pinchart
  2017-03-01  9:09     ` Maarten Lankhorst
  2017-03-01  9:21     ` [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3 Maarten Lankhorst
  0 siblings, 2 replies; 15+ messages in thread
From: Laurent Pinchart @ 2017-03-01  0:49 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Hi Maarten,

Thank you for the patch.

On Thursday 16 Feb 2017 15:47:06 Maarten Lankhorst wrote:
> There are new iterator macros that annotate whether the new or old
> state should be used. This is better than using a state that depends on
> whether it's called before or after swap. For clarity, also rename the
> variables from $obj_state to (old,new)_$obj_state as well.
> 
> Changes since v1:
> - Use old/new_*_state for variable names as much as possible. (pinchartl)
> - Expand commit message.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 431 +++++++++++++++++----------------
>  1 file changed, 222 insertions(+), 209 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index 9203f3e933f7..7d432d9a18cf
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c

[snip]

> @@ -1929,11 +1926,21 @@ void drm_atomic_helper_cleanup_planes(struct
> drm_device *dev, struct drm_atomic_state *old_state)
>  {
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *old_plane_state, *new_plane_state;
>  	int i;
> 
> -	for_each_plane_in_state(old_state, plane, plane_state, i) {
> +	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state,
> new_plane_state, i) { const struct drm_plane_helper_funcs *funcs;
> +		struct drm_plane_state *plane_state;
> +
> +		/*
> +		 * This might be called before swapping when commit is 
aborted,
> +		 * in which case we have to free the new state.

s/free/cleanup/

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

You will however need to rebase the series on top of the latest drm-misc as it 
conflicts (at compile time) with

commit 40ee6fbef75fe6452dc9e69e6f9f1a2c7808ed67
Author: Manasi Navare <manasi.d.navare@intel.com>
Date:   Fri Dec 16 12:29:06 2016 +0200

    drm: Add a new connector atomic property for link status

> +		 */
> +		if (old_plane_state == plane->state)
> +			plane_state = new_plane_state;
> +		else
> +			plane_state = old_plane_state;
> 
>  		funcs = plane->helper_private;
> 

[snip]

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand.
  2017-02-16 14:47 ` [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand Maarten Lankhorst
@ 2017-03-01  0:53   ` Laurent Pinchart
  0 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2017-03-01  0:53 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Hi Maarten,

Thank you for the patch.

On Thursday 16 Feb 2017 15:47:07 Maarten Lankhorst wrote:
> This function becomes a lot simpler when having passed both the old and
> new state to it. Looking at all callers, it seems that old_plane_state
> is never NULL so the check can be dropped.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_atomic_helper.c |  7 ++++---
>  drivers/gpu/drm/drm_plane_helper.c  |  2 +-
>  include/drm/drm_atomic_helper.h     | 26 ++++++++------------------
>  3 files changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index 7d432d9a18cf..ea544bddc29b
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1757,7 +1757,8 @@ void drm_atomic_helper_commit_planes(struct drm_device
> *dev, if (!funcs)
>  			continue;
> 
> -		disabling = drm_atomic_plane_disabling(plane, 
old_plane_state);
> +		disabling = drm_atomic_plane_disabling(old_plane_state,
> +						       new_plane_state);
> 
>  		if (active_only) {
>  			/*
> @@ -1852,11 +1853,11 @@ drm_atomic_helper_commit_planes_on_crtc(struct
> drm_crtc_state *old_crtc_state)
> 
>  		WARN_ON(plane->state->crtc && plane->state->crtc != crtc);
> 
> -		if (drm_atomic_plane_disabling(plane, old_plane_state) &&
> +		if (drm_atomic_plane_disabling(old_plane_state, plane->state) 
&&
>  		    plane_funcs->atomic_disable)
>  			plane_funcs->atomic_disable(plane, old_plane_state);
>  		else if (plane->state->crtc ||
> -			 drm_atomic_plane_disabling(plane, old_plane_state))
> +			 drm_atomic_plane_disabling(old_plane_state, plane-
>state))
>  			plane_funcs->atomic_update(plane, old_plane_state);
>  	}
> 
> diff --git a/drivers/gpu/drm/drm_plane_helper.c
> b/drivers/gpu/drm/drm_plane_helper.c index 148688fb920a..f4d70dd5e3e4
> 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -470,7 +470,7 @@ int drm_plane_helper_commit(struct drm_plane *plane,
>  	 * Drivers may optionally implement the ->atomic_disable callback, so
>  	 * special-case that here.
>  	 */
> -	if (drm_atomic_plane_disabling(plane, plane_state) &&
> +	if (drm_atomic_plane_disabling(plane_state, plane->state) &&
>  	    plane_funcs->atomic_disable)
>  		plane_funcs->atomic_disable(plane, plane_state);
>  	else
> diff --git a/include/drm/drm_atomic_helper.h
> b/include/drm/drm_atomic_helper.h index 9ceda379ce58..dc16274987c7 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -220,10 +220,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc
> *crtc, __drm_atomic_get_current_plane_state((crtc_state)->state, \ plane)))
> 
> -/*
> +/**
>   * drm_atomic_plane_disabling - check whether a plane is being disabled
> - * @plane: plane object
> - * @old_state: previous atomic state
> + * @old_plane_state: old atomic plane state
> + * @new_plane_state: new atomic plane state
>   *
>   * Checks the atomic state of a plane to determine whether it's being
> disabled * or not. This also WARNs if it detects an invalid state (both
> CRTC and FB @@ -233,28 +233,18 @@ int
> drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, * True if the
> plane is being disabled, false otherwise.
>   */
>  static inline bool
> -drm_atomic_plane_disabling(struct drm_plane *plane,
> -			   struct drm_plane_state *old_state)
> +drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state,
> +			   struct drm_plane_state *new_plane_state)
>  {
>  	/*
>  	 * When disabling a plane, CRTC and FB should always be NULL together.
>  	 * Anything else should be considered a bug in the atomic core, so we
>  	 * gently warn about it.
>  	 */
> -	WARN_ON((plane->state->crtc == NULL && plane->state->fb != NULL) ||
> -		(plane->state->crtc != NULL && plane->state->fb == NULL));
> +	WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) 
||
> +		(new_plane_state->crtc != NULL && new_plane_state->fb == 
NULL));
> 
> -	/*
> -	 * When using the transitional helpers, old_state may be NULL. If so,
> -	 * we know nothing about the current state and have to assume that it
> -	 * might be enabled.
> -	 *
> -	 * When using the atomic helpers, old_state won't be NULL. Therefore
> -	 * this check assumes that either the driver will have reconstructed
> -	 * the correct state in ->reset() or that the driver will have taken
> -	 * appropriate measures to disable all planes.
> -	 */
> -	return (!old_state || old_state->crtc) && !plane->state->crtc;
> +	return old_plane_state->crtc && !new_plane_state->crtc;
>  }
> 
>  #endif /* DRM_ATOMIC_HELPER_H_ */

-- 
Regards,

Laurent Pinchart

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2.
  2017-02-16 14:47 ` [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2 Maarten Lankhorst
@ 2017-03-01  0:56   ` Laurent Pinchart
  0 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2017-03-01  0:56 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Hi Maarten,

Thank you for the patch.

On Thursday 16 Feb 2017 15:47:08 Maarten Lankhorst wrote:
> After atomic commit, these macros should be used in place of
> get_existing_state. Also after commit get_xx_state should no longer
> be used because it may not have the required locks.
> 
> The calls to drm_atomic_get_existing_$obj_state should no longer be
> used, and converted over to these new calls.
> 
> Changes since v1:
> - Expand commit message.
> - Deprecate get_existing_*_state functions in the documentation.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  include/drm/drm_atomic.h | 108 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index c6f355a970d2..0147a047878d 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -277,6 +277,9 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector, *
>   * This function returns the crtc state for the given crtc, or NULL
>   * if the crtc is not part of the global atomic state.
> + *
> + * This function is deprecated, @drm_atomic_get_old_crtc_state or
> + * @drm_atomic_get_new_crtc_state should be used instead.
>   */
>  static inline struct drm_crtc_state *
>  drm_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
> @@ -286,12 +289,44 @@ drm_atomic_get_existing_crtc_state(struct
> drm_atomic_state *state, }
> 
>  /**
> + * drm_atomic_get_old_crtc_state - get old crtc state, if it exists
> + * @state: global atomic state object
> + * @crtc: crtc to grab
> + *
> + * This function returns the old crtc state for the given crtc, or
> + * NULL if the crtc is not part of the global atomic state.
> + */
> +static inline struct drm_crtc_state *
> +drm_atomic_get_old_crtc_state(struct drm_atomic_state *state,
> +			      struct drm_crtc *crtc)
> +{
> +	return state->crtcs[drm_crtc_index(crtc)].old_state;
> +}
> +/**
> + * drm_atomic_get_new_crtc_state - get new crtc state, if it exists
> + * @state: global atomic state object
> + * @crtc: crtc to grab
> + *
> + * This function returns the new crtc state for the given crtc, or
> + * NULL if the crtc is not part of the global atomic state.
> + */
> +static inline struct drm_crtc_state *
> +drm_atomic_get_new_crtc_state(struct drm_atomic_state *state,
> +			      struct drm_crtc *crtc)
> +{
> +	return state->crtcs[drm_crtc_index(crtc)].new_state;
> +}
> +
> +/**
>   * drm_atomic_get_existing_plane_state - get plane state, if it exists
>   * @state: global atomic state object
>   * @plane: plane to grab
>   *
>   * This function returns the plane state for the given plane, or NULL
>   * if the plane is not part of the global atomic state.
> + *
> + * This function is deprecated, @drm_atomic_get_old_plane_state or
> + * @drm_atomic_get_new_plane_state should be used instead.
>   */
>  static inline struct drm_plane_state *
>  drm_atomic_get_existing_plane_state(struct drm_atomic_state *state,
> @@ -301,12 +336,45 @@ drm_atomic_get_existing_plane_state(struct
> drm_atomic_state *state, }
> 
>  /**
> + * drm_atomic_get_old_plane_state - get plane state, if it exists
> + * @state: global atomic state object
> + * @plane: plane to grab
> + *
> + * This function returns the old plane state for the given plane, or
> + * NULL if the plane is not part of the global atomic state.
> + */
> +static inline struct drm_plane_state *
> +drm_atomic_get_old_plane_state(struct drm_atomic_state *state,
> +			       struct drm_plane *plane)
> +{
> +	return state->planes[drm_plane_index(plane)].old_state;
> +}
> +
> +/**
> + * drm_atomic_get_new_plane_state - get plane state, if it exists
> + * @state: global atomic state object
> + * @plane: plane to grab
> + *
> + * This function returns the new plane state for the given plane, or
> + * NULL if the plane is not part of the global atomic state.
> + */
> +static inline struct drm_plane_state *
> +drm_atomic_get_new_plane_state(struct drm_atomic_state *state,
> +			       struct drm_plane *plane)
> +{
> +	return state->planes[drm_plane_index(plane)].new_state;
> +}
> +
> +/**
>   * drm_atomic_get_existing_connector_state - get connector state, if it
> exists * @state: global atomic state object
>   * @connector: connector to grab
>   *
>   * This function returns the connector state for the given connector,
>   * or NULL if the connector is not part of the global atomic state.
> + *
> + * This function is deprecated, @drm_atomic_get_old_connector_state or
> + * @drm_atomic_get_new_connector_state should be used instead.
>   */
>  static inline struct drm_connector_state *
>  drm_atomic_get_existing_connector_state(struct drm_atomic_state *state,
> @@ -321,6 +389,46 @@ drm_atomic_get_existing_connector_state(struct
> drm_atomic_state *state, }
> 
>  /**
> + * drm_atomic_get_old_connector_state - get connector state, if it exists
> + * @state: global atomic state object
> + * @connector: connector to grab
> + *
> + * This function returns the old connector state for the given connector,
> + * or NULL if the connector is not part of the global atomic state.
> + */
> +static inline struct drm_connector_state *
> +drm_atomic_get_old_connector_state(struct drm_atomic_state *state,
> +				   struct drm_connector *connector)
> +{
> +	int index = drm_connector_index(connector);
> +
> +	if (index >= state->num_connector)
> +		return NULL;
> +
> +	return state->connectors[index].old_state;
> +}
> +
> +/**
> + * drm_atomic_get_new_connector_state - get connector state, if it exists
> + * @state: global atomic state object
> + * @connector: connector to grab
> + *
> + * This function returns the new connector state for the given connector,
> + * or NULL if the connector is not part of the global atomic state.
> + */
> +static inline struct drm_connector_state *
> +drm_atomic_get_new_connector_state(struct drm_atomic_state *state,
> +				   struct drm_connector *connector)
> +{
> +	int index = drm_connector_index(connector);
> +
> +	if (index >= state->num_connector)
> +		return NULL;
> +
> +	return state->connectors[index].new_state;
> +}
> +
> +/**
>   * __drm_atomic_get_current_plane_state - get current plane state
>   * @state: global atomic state object
>   * @plane: plane to grab

-- 
Regards,

Laurent Pinchart

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 5/5] drm/blend: Use new atomic iterator macros.
  2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst
@ 2017-03-01  0:58   ` Laurent Pinchart
  0 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2017-03-01  0:58 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Hi Maarten,

Thank you for the patch.

On Thursday 16 Feb 2017 15:47:10 Maarten Lankhorst wrote:
> There are new iterator macros that annotate whether the new or old
> state should be used. This is better than using a state that depends on
> whether it's called before or after swap. For clarity, also rename the
> variables from $obj_state to (old,new)_$obj_state as well.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_blend.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index d7053bb67db6..a0d0d6843288 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -377,26 +377,26 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>  			      struct drm_atomic_state *state)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *old_plane_state, *new_plane_state;
>  	int i, ret = 0;
> 
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> -		crtc = plane_state->crtc;
> +	for_each_oldnew_plane_in_state(state, plane, old_plane_state,
> new_plane_state, i) { +		crtc = new_plane_state->crtc;
>  		if (!crtc)
>  			continue;
> -		if (plane->state->zpos != plane_state->zpos) {
> -			crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
> -			crtc_state->zpos_changed = true;
> +		if (old_plane_state->zpos != new_plane_state->zpos) {
> +			new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
> +			new_crtc_state->zpos_changed = true;
>  		}
>  	}
> 
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		if (crtc_state->plane_mask != crtc->state->plane_mask ||
> -		    crtc_state->zpos_changed) {
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state,
> i) { +		if (old_crtc_state->plane_mask != new_crtc_state-
>plane_mask || +		
>    new_crtc_state->zpos_changed) {
>  			ret = drm_atomic_helper_crtc_normalize_zpos(crtc,
> -								    
crtc_state);
> +								    
new_crtc_state);
>  			if (ret)
>  				return ret;
>  		}

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3.
  2017-02-16 14:47 ` [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3 Maarten Lankhorst
@ 2017-03-01  1:04   ` Laurent Pinchart
  2017-03-01  9:22     ` [PATCH v4.1 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v4 Maarten Lankhorst
  0 siblings, 1 reply; 15+ messages in thread
From: Laurent Pinchart @ 2017-03-01  1:04 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Hi Maarten,

Thank you for the patch.

On Thursday 16 Feb 2017 15:47:09 Maarten Lankhorst wrote:
> This is a straightforward conversion that converts all the users of
> get_existing_state in atomic core to use get_old_state or get_new_state
> 
> Changes since v1:
> - Fix using the wrong state in
> drm_atomic_helper_update_legacy_modeset_state. Changes since v2:
> - Use the correct state in disable_outputs()
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c            |  6 ++---
>  drivers/gpu/drm/drm_atomic_helper.c     | 43 ++++++++++++++----------------
>  drivers/gpu/drm/drm_blend.c             |  3 +--
>  drivers/gpu/drm/drm_simple_kms_helper.c |  4 +--
>  4 files changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index afec53832145..c9aac9cbf111 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1358,8 +1358,8 @@ drm_atomic_set_crtc_for_connector(struct
> drm_connector_state *conn_state, return 0;
> 
>  	if (conn_state->crtc) {
> -		crtc_state = drm_atomic_get_existing_crtc_state(conn_state-
>state,
> -								conn_state-
>crtc);
> +		crtc_state = drm_atomic_get_new_crtc_state(conn_state->state,
> +							   conn_state->crtc);

I believe more *_state -> *_(new|old)_state renames could be useful, but that 
can be addressed as a subsequent patch, so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  		crtc_state->connector_mask &=
>  			~(1 << drm_connector_index(conn_state->connector));
> @@ -1476,7 +1476,7 @@ drm_atomic_add_affected_planes(struct drm_atomic_state
> *state, {
>  	struct drm_plane *plane;
> 
> -	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
> +	WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc));
> 
>  	drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) {
>  		struct drm_plane_state *plane_state =
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index ea544bddc29b..6296ae6e6aff
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -70,8 +70,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> *state, struct drm_crtc_state *crtc_state;
> 
>  	if (old_plane_state->crtc) {
> -		crtc_state = drm_atomic_get_existing_crtc_state(state,
> -								
old_plane_state->crtc);
> +		crtc_state = drm_atomic_get_new_crtc_state(state,
> +							   old_plane_state-
>crtc);
> 
>  		if (WARN_ON(!crtc_state))
>  			return;
> @@ -80,8 +80,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> *state, }
> 
>  	if (plane_state->crtc) {
> -		crtc_state = drm_atomic_get_existing_crtc_state(state,
> -								plane_state-
>crtc);
> +		crtc_state = drm_atomic_get_new_crtc_state(state, plane_state-
>crtc);
> 
>  		if (WARN_ON(!crtc_state))
>  			return;
> @@ -150,7 +149,7 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state, drm_for_each_connector_iter(connector, &conn_iter)
> {
>  		struct drm_crtc_state *crtc_state;
> 
> -		if (drm_atomic_get_existing_connector_state(state, connector))
> +		if (drm_atomic_get_new_connector_state(state, connector))
>  			continue;
> 
>  		encoder = connector->state->best_encoder;
> @@ -178,7 +177,7 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state, new_conn_state->crtc->base.id,
> new_conn_state->crtc->name,
>  				 connector->base.id, connector->name);
> 
> -		crtc_state = drm_atomic_get_existing_crtc_state(state,
> new_conn_state->crtc); +		crtc_state = 
drm_atomic_get_new_crtc_state(state,
> new_conn_state->crtc);
> 
>  		ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL);
>  		if (ret)
> @@ -219,7 +218,7 @@ set_best_encoder(struct drm_atomic_state *state,
>  		 */
>  		WARN_ON(!crtc && encoder != conn_state->best_encoder);
>  		if (crtc) {
> -			crtc_state = drm_atomic_get_existing_crtc_state(state, 
crtc);
> +			crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
> 
>  			crtc_state->encoder_mask &=
>  				~(1 << drm_encoder_index(conn_state-
>best_encoder));
> @@ -230,7 +229,7 @@ set_best_encoder(struct drm_atomic_state *state,
>  		crtc = conn_state->crtc;
>  		WARN_ON(!crtc);
>  		if (crtc) {
> -			crtc_state = drm_atomic_get_existing_crtc_state(state, 
crtc);
> +			crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
> 
>  			crtc_state->encoder_mask |=
>  				1 << drm_encoder_index(encoder);
> @@ -263,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> 
>  		set_best_encoder(state, new_connector_state, NULL);
> 
> -		crtc_state = drm_atomic_get_existing_crtc_state(state, 
encoder_crtc);
> +		crtc_state = drm_atomic_get_new_crtc_state(state, 
encoder_crtc);
>  		crtc_state->connectors_changed = true;
> 
>  		return;
> @@ -286,12 +285,12 @@ update_connector_routing(struct drm_atomic_state
> *state,
> 
>  	if (old_connector_state->crtc != new_connector_state->crtc) {
>  		if (old_connector_state->crtc) {
> -			crtc_state = drm_atomic_get_existing_crtc_state(state,
> old_connector_state->crtc); +			crtc_state =
> drm_atomic_get_new_crtc_state(state, old_connector_state->crtc);
> crtc_state->connectors_changed = true;
>  		}
> 
>  		if (new_connector_state->crtc) {
> -			crtc_state = drm_atomic_get_existing_crtc_state(state,
> new_connector_state->crtc); +			crtc_state =
> drm_atomic_get_new_crtc_state(state, new_connector_state->crtc);
> crtc_state->connectors_changed = true;
>  		}
>  	}
> @@ -350,7 +349,7 @@ update_connector_routing(struct drm_atomic_state *state,
> 
>  	set_best_encoder(state, new_connector_state, new_encoder);
> 
> -	crtc_state = drm_atomic_get_existing_crtc_state(state,
> new_connector_state->crtc); +	crtc_state =
> drm_atomic_get_new_crtc_state(state, new_connector_state->crtc);
> crtc_state->connectors_changed = true;
> 
>  	DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on
> [CRTC:%d:%s]\n", @@ -392,7 +391,7 @@ mode_fixup(struct drm_atomic_state
> *state)
>  			continue;
> 
>  		new_crtc_state =
> -			drm_atomic_get_existing_crtc_state(state, 
new_conn_state->crtc);
> +			drm_atomic_get_new_crtc_state(state, new_conn_state-
>crtc);
> 
>  		/*
>  		 * Each encoder has at most one connector (since we always 
steal
> @@ -704,8 +703,7 @@ disable_outputs(struct drm_device *dev, struct
> drm_atomic_state *old_state) if (!old_conn_state->crtc)
>  			continue;
> 
> -		old_crtc_state = drm_atomic_get_existing_crtc_state(old_state,
> -								    
old_conn_state->crtc);
> +		old_crtc_state = drm_atomic_get_old_crtc_state(old_state,
> old_conn_state->crtc);
> 
>  		if (!old_crtc_state->active ||
>  		    !drm_atomic_crtc_needs_modeset(old_conn_state->crtc-
>state))
> @@ -834,14 +832,17 @@ drm_atomic_helper_update_legacy_modeset_state(struct
> drm_device *dev, /* set legacy state in the crtc structure */
>  	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
>  		struct drm_plane *primary = crtc->primary;
> +		struct drm_plane_state *new_plane_state;
> 
>  		crtc->mode = new_crtc_state->mode;
>  		crtc->enabled = new_crtc_state->enable;
> 
> -		if (drm_atomic_get_existing_plane_state(old_state, primary) &&
> -		    primary->state->crtc == crtc) {
> -			crtc->x = primary->state->src_x >> 16;
> -			crtc->y = primary->state->src_y >> 16;
> +		new_plane_state =
> +			drm_atomic_get_new_plane_state(old_state, primary);
> +
> +		if (new_plane_state && new_plane_state->crtc == crtc) {
> +			crtc->x = new_plane_state->src_x >> 16;
> +			crtc->y = new_plane_state->src_y >> 16;
>  		}
> 
>  		if (new_crtc_state->enable)
> @@ -1843,7 +1844,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct
> drm_crtc_state *old_crtc_state)
> 
>  	drm_for_each_plane_mask(plane, crtc->dev, plane_mask) {
>  		struct drm_plane_state *old_plane_state =
> -			drm_atomic_get_existing_plane_state(old_state, plane);
> +			drm_atomic_get_old_plane_state(old_state, plane);
>  		const struct drm_plane_helper_funcs *plane_funcs;
> 
>  		plane_funcs = plane->helper_private;
> @@ -2914,7 +2915,7 @@ int drm_atomic_helper_page_flip_target(
>  	if (ret != 0)
>  		goto fail;
> 
> -	crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>  	if (WARN_ON(!crtc_state)) {
>  		ret = -EINVAL;
>  		goto fail;
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 665aafc6ad68..d7053bb67db6 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -387,8 +387,7 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>  		if (!crtc)
>  			continue;
>  		if (plane->state->zpos != plane_state->zpos) {
> -			crtc_state =
> -				drm_atomic_get_existing_crtc_state(state, 
crtc);
> +			crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
>  			crtc_state->zpos_changed = true;
>  		}
>  	}
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
> b/drivers/gpu/drm/drm_simple_kms_helper.c index 35c5d99296b9..16789faa9291
> 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -86,8 +86,8 @@ static int drm_simple_kms_plane_atomic_check(struct
> drm_plane *plane, int ret;
> 
>  	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
> -	crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,
> -							&pipe->crtc);
> +	crtc_state = drm_atomic_get_new_crtc_state(plane_state->state,
> +						   &pipe->crtc);
>  	if (crtc_state->enable != !!plane_state->crtc)
>  		return -EINVAL; /* plane must match crtc enable state */

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2.
  2017-03-01  0:49   ` Laurent Pinchart
@ 2017-03-01  9:09     ` Maarten Lankhorst
  2017-03-01  9:21     ` [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3 Maarten Lankhorst
  1 sibling, 0 replies; 15+ messages in thread
From: Maarten Lankhorst @ 2017-03-01  9:09 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel; +Cc: intel-gfx

Op 01-03-17 om 01:49 schreef Laurent Pinchart:
> Hi Maarten,
>
> Thank you for the patch.
>
> On Thursday 16 Feb 2017 15:47:06 Maarten Lankhorst wrote:
>> There are new iterator macros that annotate whether the new or old
>> state should be used. This is better than using a state that depends on
>> whether it's called before or after swap. For clarity, also rename the
>> variables from $obj_state to (old,new)_$obj_state as well.
>>
>> Changes since v1:
>> - Use old/new_*_state for variable names as much as possible. (pinchartl)
>> - Expand commit message.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic_helper.c | 431 +++++++++++++++++----------------
>>  1 file changed, 222 insertions(+), 209 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> b/drivers/gpu/drm/drm_atomic_helper.c index 9203f3e933f7..7d432d9a18cf
>> 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> [snip]
>
>> @@ -1929,11 +1926,21 @@ void drm_atomic_helper_cleanup_planes(struct
>> drm_device *dev, struct drm_atomic_state *old_state)
>>  {
>>  	struct drm_plane *plane;
>> -	struct drm_plane_state *plane_state;
>> +	struct drm_plane_state *old_plane_state, *new_plane_state;
>>  	int i;
>>
>> -	for_each_plane_in_state(old_state, plane, plane_state, i) {
>> +	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state,
>> new_plane_state, i) { const struct drm_plane_helper_funcs *funcs;
>> +		struct drm_plane_state *plane_state;
>> +
>> +		/*
>> +		 * This might be called before swapping when commit is 
> aborted,
>> +		 * in which case we have to free the new state.
> s/free/cleanup/
>
> Apart from that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> You will however need to rebase the series on top of the latest drm-misc as it 
> conflicts (at compile time) with
Yeah I noticed, patch 1 and 5 are affected, will resend those.

Thanks for the review,
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3.
  2017-03-01  0:49   ` Laurent Pinchart
  2017-03-01  9:09     ` Maarten Lankhorst
@ 2017-03-01  9:21     ` Maarten Lankhorst
  2017-03-01 10:06       ` Laurent Pinchart
  1 sibling, 1 reply; 15+ messages in thread
From: Maarten Lankhorst @ 2017-03-01  9:21 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel; +Cc: intel-gfx

There are new iterator macros that annotate whether the new or old
state should be used. This is better than using a state that depends on
whether it's called before or after swap. For clarity, also rename the
variables from $obj_state to (old,new)_$obj_state as well.

Changes since v1:
- Use old/new_*_state for variable names as much as possible. (pinchartl)
- Expand commit message.
Changes since v2:
- Rebase on top of link training patches.
- free -> cleanup (pinchartl)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 446 +++++++++++++++++++-----------------
 1 file changed, 230 insertions(+), 216 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 6b12396f718b..aa1e7609b024 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -63,14 +63,15 @@
  */
 static void
 drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
+				struct drm_plane_state *old_plane_state,
 				struct drm_plane_state *plane_state,
 				struct drm_plane *plane)
 {
 	struct drm_crtc_state *crtc_state;
 
-	if (plane->state->crtc) {
+	if (old_plane_state->crtc) {
 		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								plane->state->crtc);
+								old_plane_state->crtc);
 
 		if (WARN_ON(!crtc_state))
 			return;
@@ -92,7 +93,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 static int handle_conflicting_encoders(struct drm_atomic_state *state,
 				       bool disable_conflicting_encoders)
 {
-	struct drm_connector_state *conn_state;
+	struct drm_connector_state *new_conn_state;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
 	struct drm_encoder *encoder;
@@ -104,15 +105,15 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 	 * part of the state. If the same encoder is assigned to multiple
 	 * connectors bail out.
 	 */
-	for_each_connector_in_state(state, connector, conn_state, i) {
+	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
 		const struct drm_connector_helper_funcs *funcs = connector->helper_private;
 		struct drm_encoder *new_encoder;
 
-		if (!conn_state->crtc)
+		if (!new_conn_state->crtc)
 			continue;
 
 		if (funcs->atomic_best_encoder)
-			new_encoder = funcs->atomic_best_encoder(connector, conn_state);
+			new_encoder = funcs->atomic_best_encoder(connector, new_conn_state);
 		else if (funcs->best_encoder)
 			new_encoder = funcs->best_encoder(connector);
 		else
@@ -166,20 +167,20 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 			goto out;
 		}
 
-		conn_state = drm_atomic_get_connector_state(state, connector);
-		if (IS_ERR(conn_state)) {
-			ret = PTR_ERR(conn_state);
+		new_conn_state = drm_atomic_get_connector_state(state, connector);
+		if (IS_ERR(new_conn_state)) {
+			ret = PTR_ERR(new_conn_state);
 			goto out;
 		}
 
 		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], disabling [CONNECTOR:%d:%s]\n",
 				 encoder->base.id, encoder->name,
-				 conn_state->crtc->base.id, conn_state->crtc->name,
+				 new_conn_state->crtc->base.id, new_conn_state->crtc->name,
 				 connector->base.id, connector->name);
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state, conn_state->crtc);
+		crtc_state = drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc);
 
-		ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
+		ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL);
 		if (ret)
 			goto out;
 
@@ -245,22 +246,22 @@ steal_encoder(struct drm_atomic_state *state,
 {
 	struct drm_crtc_state *crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
+	struct drm_connector_state *old_connector_state, *new_connector_state;
 	int i;
 
-	for_each_connector_in_state(state, connector, connector_state, i) {
+	for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) {
 		struct drm_crtc *encoder_crtc;
 
-		if (connector_state->best_encoder != encoder)
+		if (new_connector_state->best_encoder != encoder)
 			continue;
 
-		encoder_crtc = connector->state->crtc;
+		encoder_crtc = old_connector_state->crtc;
 
 		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n",
 				 encoder->base.id, encoder->name,
 				 encoder_crtc->base.id, encoder_crtc->name);
 
-		set_best_encoder(state, connector_state, NULL);
+		set_best_encoder(state, new_connector_state, NULL);
 
 		crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
 		crtc_state->connectors_changed = true;
@@ -272,7 +273,8 @@ steal_encoder(struct drm_atomic_state *state,
 static int
 update_connector_routing(struct drm_atomic_state *state,
 			 struct drm_connector *connector,
-			 struct drm_connector_state *connector_state)
+			 struct drm_connector_state *old_connector_state,
+			 struct drm_connector_state *new_connector_state)
 {
 	const struct drm_connector_helper_funcs *funcs;
 	struct drm_encoder *new_encoder;
@@ -282,24 +284,24 @@ update_connector_routing(struct drm_atomic_state *state,
 			 connector->base.id,
 			 connector->name);
 
-	if (connector->state->crtc != connector_state->crtc) {
-		if (connector->state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, connector->state->crtc);
+	if (old_connector_state->crtc != new_connector_state->crtc) {
+		if (old_connector_state->crtc) {
+			crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc);
 			crtc_state->connectors_changed = true;
 		}
 
-		if (connector_state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
+		if (new_connector_state->crtc) {
+			crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
 			crtc_state->connectors_changed = true;
 		}
 	}
 
-	if (!connector_state->crtc) {
+	if (!new_connector_state->crtc) {
 		DRM_DEBUG_ATOMIC("Disabling [CONNECTOR:%d:%s]\n",
 				connector->base.id,
 				connector->name);
 
-		set_best_encoder(state, connector_state, NULL);
+		set_best_encoder(state, new_connector_state, NULL);
 
 		return 0;
 	}
@@ -308,7 +310,7 @@ update_connector_routing(struct drm_atomic_state *state,
 
 	if (funcs->atomic_best_encoder)
 		new_encoder = funcs->atomic_best_encoder(connector,
-							 connector_state);
+							 new_connector_state);
 	else if (funcs->best_encoder)
 		new_encoder = funcs->best_encoder(connector);
 	else
@@ -321,34 +323,34 @@ update_connector_routing(struct drm_atomic_state *state,
 		return -EINVAL;
 	}
 
-	if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) {
+	if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) {
 		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
 				 new_encoder->base.id,
 				 new_encoder->name,
-				 connector_state->crtc->base.id,
-				 connector_state->crtc->name);
+				 new_connector_state->crtc->base.id,
+				 new_connector_state->crtc->name);
 		return -EINVAL;
 	}
 
-	if (new_encoder == connector_state->best_encoder) {
-		set_best_encoder(state, connector_state, new_encoder);
+	if (new_encoder == new_connector_state->best_encoder) {
+		set_best_encoder(state, new_connector_state, new_encoder);
 
 		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] keeps [ENCODER:%d:%s], now on [CRTC:%d:%s]\n",
 				 connector->base.id,
 				 connector->name,
 				 new_encoder->base.id,
 				 new_encoder->name,
-				 connector_state->crtc->base.id,
-				 connector_state->crtc->name);
+				 new_connector_state->crtc->base.id,
+				 new_connector_state->crtc->name);
 
 		return 0;
 	}
 
 	steal_encoder(state, new_encoder);
 
-	set_best_encoder(state, connector_state, new_encoder);
+	set_best_encoder(state, new_connector_state, new_encoder);
 
-	crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
+	crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
 	crtc_state->connectors_changed = true;
 
 	DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n",
@@ -356,8 +358,8 @@ update_connector_routing(struct drm_atomic_state *state,
 			 connector->name,
 			 new_encoder->base.id,
 			 new_encoder->name,
-			 connector_state->crtc->base.id,
-			 connector_state->crtc->name);
+			 new_connector_state->crtc->base.id,
+			 new_connector_state->crtc->name);
 
 	return 0;
 }
@@ -366,57 +368,57 @@ static int
 mode_fixup(struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *conn_state;
+	struct drm_connector_state *new_conn_state;
 	int i;
 	int ret;
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (!crtc_state->mode_changed &&
-		    !crtc_state->connectors_changed)
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (!new_crtc_state->mode_changed &&
+		    !new_crtc_state->connectors_changed)
 			continue;
 
-		drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode);
+		drm_mode_copy(&new_crtc_state->adjusted_mode, &new_crtc_state->mode);
 	}
 
-	for_each_connector_in_state(state, connector, conn_state, i) {
+	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 
-		WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc);
+		WARN_ON(!!new_conn_state->best_encoder != !!new_conn_state->crtc);
 
-		if (!conn_state->crtc || !conn_state->best_encoder)
+		if (!new_conn_state->crtc || !new_conn_state->best_encoder)
 			continue;
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								conn_state->crtc);
+		new_crtc_state =
+			drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc);
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
 		 * it away), so we won't call ->mode_fixup twice.
 		 */
-		encoder = conn_state->best_encoder;
+		encoder = new_conn_state->best_encoder;
 		funcs = encoder->helper_private;
 
-		ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state->mode,
-				&crtc_state->adjusted_mode);
+		ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode,
+				&new_crtc_state->adjusted_mode);
 		if (!ret) {
 			DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
 			return -EINVAL;
 		}
 
 		if (funcs && funcs->atomic_check) {
-			ret = funcs->atomic_check(encoder, crtc_state,
-						  conn_state);
+			ret = funcs->atomic_check(encoder, new_crtc_state,
+						  new_conn_state);
 			if (ret) {
 				DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n",
 						 encoder->base.id, encoder->name);
 				return ret;
 			}
 		} else if (funcs && funcs->mode_fixup) {
-			ret = funcs->mode_fixup(encoder, &crtc_state->mode,
-						&crtc_state->adjusted_mode);
+			ret = funcs->mode_fixup(encoder, &new_crtc_state->mode,
+						&new_crtc_state->adjusted_mode);
 			if (!ret) {
 				DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n",
 						 encoder->base.id, encoder->name);
@@ -425,22 +427,22 @@ mode_fixup(struct drm_atomic_state *state)
 		}
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
-		if (!crtc_state->enable)
+		if (!new_crtc_state->enable)
 			continue;
 
-		if (!crtc_state->mode_changed &&
-		    !crtc_state->connectors_changed)
+		if (!new_crtc_state->mode_changed &&
+		    !new_crtc_state->connectors_changed)
 			continue;
 
 		funcs = crtc->helper_private;
 		if (!funcs->mode_fixup)
 			continue;
 
-		ret = funcs->mode_fixup(crtc, &crtc_state->mode,
-					&crtc_state->adjusted_mode);
+		ret = funcs->mode_fixup(crtc, &new_crtc_state->mode,
+					&new_crtc_state->adjusted_mode);
 		if (!ret) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n",
 					 crtc->base.id, crtc->name);
@@ -487,19 +489,19 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 				struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
+	struct drm_connector_state *old_connector_state, *new_connector_state;
 	int i, ret;
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (!drm_mode_equal(&old_crtc_state->mode, &new_crtc_state->mode)) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode changed\n",
 					 crtc->base.id, crtc->name);
-			crtc_state->mode_changed = true;
+			new_crtc_state->mode_changed = true;
 		}
 
-		if (crtc->state->enable != crtc_state->enable) {
+		if (old_crtc_state->enable != new_crtc_state->enable) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enable changed\n",
 					 crtc->base.id, crtc->name);
 
@@ -511,8 +513,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 			 * The other way around is true as well. enable != 0
 			 * iff connectors are attached and a mode is set.
 			 */
-			crtc_state->mode_changed = true;
-			crtc_state->connectors_changed = true;
+			new_crtc_state->mode_changed = true;
+			new_crtc_state->connectors_changed = true;
 		}
 	}
 
@@ -520,22 +522,23 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 	if (ret)
 		return ret;
 
-	for_each_connector_in_state(state, connector, connector_state, i) {
+	for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) {
 		/*
 		 * This only sets crtc->connectors_changed for routing changes,
 		 * drivers must set crtc->connectors_changed themselves when
 		 * connector properties need to be updated.
 		 */
 		ret = update_connector_routing(state, connector,
-					       connector_state);
+					       old_connector_state,
+					       new_connector_state);
 		if (ret)
 			return ret;
-		if (connector->state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state,
-									connector->state->crtc);
-			if (connector->state->link_status !=
-			    connector_state->link_status)
-				crtc_state->connectors_changed = true;
+		if (old_connector_state->crtc) {
+			new_crtc_state = drm_atomic_get_existing_crtc_state(state,
+									    old_connector_state->crtc);
+			if (old_connector_state->link_status !=
+			    new_connector_state->link_status)
+				new_crtc_state->connectors_changed = true;
 		}
 	}
 
@@ -545,28 +548,28 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 	 * configuration. This must be done before calling mode_fixup in case a
 	 * crtc only changed its mode but has the same set of connectors.
 	 */
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		bool has_connectors =
-			!!crtc_state->connector_mask;
+			!!new_crtc_state->connector_mask;
 
 		/*
 		 * We must set ->active_changed after walking connectors for
 		 * otherwise an update that only changes active would result in
 		 * a full modeset because update_connector_routing force that.
 		 */
-		if (crtc->state->active != crtc_state->active) {
+		if (old_crtc_state->active != new_crtc_state->active) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active changed\n",
 					 crtc->base.id, crtc->name);
-			crtc_state->active_changed = true;
+			new_crtc_state->active_changed = true;
 		}
 
-		if (!drm_atomic_crtc_needs_modeset(crtc_state))
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
 			continue;
 
 		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] needs all connectors, enable: %c, active: %c\n",
 				 crtc->base.id, crtc->name,
-				 crtc_state->enable ? 'y' : 'n',
-				 crtc_state->active ? 'y' : 'n');
+				 new_crtc_state->enable ? 'y' : 'n',
+				 new_crtc_state->active ? 'y' : 'n');
 
 		ret = drm_atomic_add_affected_connectors(state, crtc);
 		if (ret != 0)
@@ -576,7 +579,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 		if (ret != 0)
 			return ret;
 
-		if (crtc_state->enable != has_connectors) {
+		if (new_crtc_state->enable != has_connectors) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n",
 					 crtc->base.id, crtc->name);
 
@@ -609,22 +612,22 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
 			       struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state, *old_plane_state;
 	int i, ret = 0;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
+	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
 		funcs = plane->helper_private;
 
-		drm_atomic_helper_plane_changed(state, plane_state, plane);
+		drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, plane);
 
 		if (!funcs || !funcs->atomic_check)
 			continue;
 
-		ret = funcs->atomic_check(plane, plane_state);
+		ret = funcs->atomic_check(plane, new_plane_state);
 		if (ret) {
 			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n",
 					 plane->base.id, plane->name);
@@ -632,7 +635,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
 		}
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		funcs = crtc->helper_private;
@@ -640,7 +643,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
 		if (!funcs || !funcs->atomic_check)
 			continue;
 
-		ret = funcs->atomic_check(crtc, crtc_state);
+		ret = funcs->atomic_check(crtc, new_crtc_state);
 		if (ret) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n",
 					 crtc->base.id, crtc->name);
@@ -694,12 +697,12 @@ static void
 disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 {
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	int i;
 
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+	for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 
@@ -736,7 +739,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 
 		/* Right function depends upon target state. */
 		if (funcs) {
-			if (connector->state->crtc && funcs->prepare)
+			if (new_conn_state->crtc && funcs->prepare)
 				funcs->prepare(encoder);
 			else if (funcs->disable)
 				funcs->disable(encoder);
@@ -747,11 +750,11 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 		drm_bridge_post_disable(encoder->bridge);
 	}
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		/* Shut down everything that needs a full modeset. */
-		if (!drm_atomic_crtc_needs_modeset(crtc->state))
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
 			continue;
 
 		if (!old_crtc_state->active)
@@ -764,7 +767,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 
 
 		/* Right function depends upon target state. */
-		if (crtc->state->enable && funcs->prepare)
+		if (new_crtc_state->enable && funcs->prepare)
 			funcs->prepare(crtc);
 		else if (funcs->atomic_disable)
 			funcs->atomic_disable(crtc, old_crtc_state);
@@ -793,13 +796,13 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 					      struct drm_atomic_state *old_state)
 {
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	int i;
 
 	/* clear out existing links and update dpms */
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+	for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) {
 		if (connector->encoder) {
 			WARN_ON(!connector->encoder->crtc);
 
@@ -807,7 +810,7 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 			connector->encoder = NULL;
 		}
 
-		crtc = connector->state->crtc;
+		crtc = new_conn_state->crtc;
 		if ((!crtc && old_conn_state->crtc) ||
 		    (crtc && drm_atomic_crtc_needs_modeset(crtc->state))) {
 			struct drm_property *dpms_prop =
@@ -824,23 +827,23 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 	}
 
 	/* set new links */
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
-		if (!connector->state->crtc)
+	for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
+		if (!new_conn_state->crtc)
 			continue;
 
-		if (WARN_ON(!connector->state->best_encoder))
+		if (WARN_ON(!new_conn_state->best_encoder))
 			continue;
 
-		connector->encoder = connector->state->best_encoder;
-		connector->encoder->crtc = connector->state->crtc;
+		connector->encoder = new_conn_state->best_encoder;
+		connector->encoder->crtc = new_conn_state->crtc;
 	}
 
 	/* set legacy state in the crtc structure */
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		struct drm_plane *primary = crtc->primary;
 
-		crtc->mode = crtc->state->mode;
-		crtc->enabled = crtc->state->enable;
+		crtc->mode = new_crtc_state->mode;
+		crtc->enabled = new_crtc_state->enable;
 
 		if (drm_atomic_get_existing_plane_state(old_state, primary) &&
 		    primary->state->crtc == crtc) {
@@ -848,9 +851,9 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 			crtc->y = primary->state->src_y >> 16;
 		}
 
-		if (crtc->state->enable)
+		if (new_crtc_state->enable)
 			drm_calc_timestamping_constants(crtc,
-							&crtc->state->adjusted_mode);
+							&new_crtc_state->adjusted_mode);
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_update_legacy_modeset_state);
@@ -859,20 +862,20 @@ static void
 crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *new_conn_state;
 	int i;
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
-		if (!crtc->state->mode_changed)
+		if (!new_crtc_state->mode_changed)
 			continue;
 
 		funcs = crtc->helper_private;
 
-		if (crtc->state->enable && funcs->mode_set_nofb) {
+		if (new_crtc_state->enable && funcs->mode_set_nofb) {
 			DRM_DEBUG_ATOMIC("modeset on [CRTC:%d:%s]\n",
 					 crtc->base.id, crtc->name);
 
@@ -880,18 +883,17 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 		}
 	}
 
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+	for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
-		struct drm_crtc_state *new_crtc_state;
 		struct drm_encoder *encoder;
 		struct drm_display_mode *mode, *adjusted_mode;
 
-		if (!connector->state->best_encoder)
+		if (!new_conn_state->best_encoder)
 			continue;
 
-		encoder = connector->state->best_encoder;
+		encoder = new_conn_state->best_encoder;
 		funcs = encoder->helper_private;
-		new_crtc_state = connector->state->crtc->state;
+		new_crtc_state = new_conn_state->crtc->state;
 		mode = &new_crtc_state->mode;
 		adjusted_mode = &new_crtc_state->adjusted_mode;
 
@@ -907,7 +909,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 		 */
 		if (funcs && funcs->atomic_mode_set) {
 			funcs->atomic_mode_set(encoder, new_crtc_state,
-					       connector->state);
+					       new_conn_state);
 		} else if (funcs && funcs->mode_set) {
 			funcs->mode_set(encoder, mode, adjusted_mode);
 		}
@@ -959,24 +961,24 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 					      struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *new_conn_state;
 	int i;
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		/* Need to filter out CRTCs where only planes change. */
-		if (!drm_atomic_crtc_needs_modeset(crtc->state))
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
 			continue;
 
-		if (!crtc->state->active)
+		if (!new_crtc_state->active)
 			continue;
 
 		funcs = crtc->helper_private;
 
-		if (crtc->state->enable) {
+		if (new_crtc_state->enable) {
 			DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n",
 					 crtc->base.id, crtc->name);
 
@@ -987,18 +989,18 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 		}
 	}
 
-	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+	for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 
-		if (!connector->state->best_encoder)
+		if (!new_conn_state->best_encoder)
 			continue;
 
-		if (!connector->state->crtc->state->active ||
-		    !drm_atomic_crtc_needs_modeset(connector->state->crtc->state))
+		if (!new_conn_state->crtc->state->active ||
+		    !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state))
 			continue;
 
-		encoder = connector->state->best_encoder;
+		encoder = new_conn_state->best_encoder;
 		funcs = encoder->helper_private;
 
 		DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n",
@@ -1048,29 +1050,26 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
 				      bool pre_swap)
 {
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state;
 	int i, ret;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		if (!pre_swap)
-			plane_state = plane->state;
-
-		if (!plane_state->fence)
+	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+		if (!new_plane_state->fence)
 			continue;
 
-		WARN_ON(!plane_state->fb);
+		WARN_ON(!new_plane_state->fb);
 
 		/*
 		 * If waiting for fences pre-swap (ie: nonblock), userspace can
 		 * still interrupt the operation. Instead of blocking until the
 		 * timer expires, make the wait interruptible.
 		 */
-		ret = dma_fence_wait(plane_state->fence, pre_swap);
+		ret = dma_fence_wait(new_plane_state->fence, pre_swap);
 		if (ret)
 			return ret;
 
-		dma_fence_put(plane_state->fence);
-		plane_state->fence = NULL;
+		dma_fence_put(new_plane_state->fence);
+		new_plane_state->fence = NULL;
 	}
 
 	return 0;
@@ -1093,7 +1092,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 		struct drm_atomic_state *old_state)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	int i, ret;
 	unsigned crtc_mask = 0;
 
@@ -1104,9 +1103,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 	if (old_state->legacy_cursor_update)
 		return;
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
-		struct drm_crtc_state *new_crtc_state = crtc->state;
-
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		if (!new_crtc_state->active || !new_crtc_state->planes_changed)
 			continue;
 
@@ -1118,7 +1115,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 		old_state->crtcs[i].last_vblank_count = drm_crtc_vblank_count(crtc);
 	}
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
 		if (!(crtc_mask & drm_crtc_mask(crtc)))
 			continue;
 
@@ -1427,11 +1424,11 @@ 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_state *old_crtc_state, *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i, ret;
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		commit = kzalloc(sizeof(*commit), GFP_KERNEL);
 		if (!commit)
 			return -ENOMEM;
@@ -1452,7 +1449,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
 		/* 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) {
+		if (!old_crtc_state->active && !new_crtc_state->active) {
 			complete_all(&commit->flip_done);
 			continue;
 		}
@@ -1463,17 +1460,17 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
 			continue;
 		}
 
-		if (!crtc_state->event) {
+		if (!new_crtc_state->event) {
 			commit->event = kzalloc(sizeof(*commit->event),
 						GFP_KERNEL);
 			if (!commit->event)
 				return -ENOMEM;
 
-			crtc_state->event = commit->event;
+			new_crtc_state->event = commit->event;
 		}
 
-		crtc_state->event->base.completion = &commit->flip_done;
-		crtc_state->event->base.completion_release = release_crtc_commit;
+		new_crtc_state->event->base.completion = &commit->flip_done;
+		new_crtc_state->event->base.completion_release = release_crtc_commit;
 		drm_crtc_commit_get(commit);
 	}
 
@@ -1512,12 +1509,12 @@ 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 *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 	long ret;
 
-	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		spin_lock(&crtc->commit_lock);
 		commit = preceeding_commit(crtc);
 		if (commit)
@@ -1564,17 +1561,17 @@ 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 *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 
-	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		commit = old_state->crtcs[i].commit;
 		if (!commit)
 			continue;
 
 		/* backend must have consumed any event by now */
-		WARN_ON(crtc->state->event);
+		WARN_ON(new_crtc_state->event);
 		spin_lock(&crtc->commit_lock);
 		complete_all(&commit->hw_done);
 		spin_unlock(&crtc->commit_lock);
@@ -1596,12 +1593,12 @@ 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 *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc_commit *commit;
 	int i;
 	long ret;
 
-	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		commit = old_state->crtcs[i].commit;
 		if (WARN_ON(!commit))
 			continue;
@@ -1652,16 +1649,16 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state)
 {
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state;
 	int ret, i, j;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
+	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
 		funcs = plane->helper_private;
 
 		if (funcs->prepare_fb) {
-			ret = funcs->prepare_fb(plane, plane_state);
+			ret = funcs->prepare_fb(plane, new_plane_state);
 			if (ret)
 				goto fail;
 		}
@@ -1670,7 +1667,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 	return 0;
 
 fail:
-	for_each_plane_in_state(state, plane, plane_state, j) {
+	for_each_new_plane_in_state(state, plane, new_plane_state, j) {
 		const struct drm_plane_helper_funcs *funcs;
 
 		if (j >= i)
@@ -1679,7 +1676,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
-			funcs->cleanup_fb(plane, plane_state);
+			funcs->cleanup_fb(plane, new_plane_state);
 	}
 
 	return ret;
@@ -1737,14 +1734,14 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 				     uint32_t flags)
 {
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *old_plane_state;
+	struct drm_plane_state *old_plane_state, *new_plane_state;
 	int i;
 	bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY;
 	bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET;
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		funcs = crtc->helper_private;
@@ -1752,13 +1749,13 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 		if (!funcs || !funcs->atomic_begin)
 			continue;
 
-		if (active_only && !crtc->state->active)
+		if (active_only && !new_crtc_state->active)
 			continue;
 
 		funcs->atomic_begin(crtc, old_crtc_state);
 	}
 
-	for_each_plane_in_state(old_state, plane, old_plane_state, i) {
+	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 		bool disabling;
 
@@ -1777,7 +1774,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 			 * CRTC to avoid skipping planes being disabled on an
 			 * active CRTC.
 			 */
-			if (!disabling && !plane_crtc_active(plane->state))
+			if (!disabling && !plane_crtc_active(new_plane_state))
 				continue;
 			if (disabling && !plane_crtc_active(old_plane_state))
 				continue;
@@ -1796,12 +1793,12 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 				continue;
 
 			funcs->atomic_disable(plane, old_plane_state);
-		} else if (plane->state->crtc || disabling) {
+		} else if (new_plane_state->crtc || disabling) {
 			funcs->atomic_update(plane, old_plane_state);
 		}
 	}
 
-	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		funcs = crtc->helper_private;
@@ -1809,7 +1806,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 		if (!funcs || !funcs->atomic_flush)
 			continue;
 
-		if (active_only && !crtc->state->active)
+		if (active_only && !new_crtc_state->active)
 			continue;
 
 		funcs->atomic_flush(crtc, old_crtc_state);
@@ -1936,11 +1933,21 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 				      struct drm_atomic_state *old_state)
 {
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *old_plane_state, *new_plane_state;
 	int i;
 
-	for_each_plane_in_state(old_state, plane, plane_state, i) {
+	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
+		struct drm_plane_state *plane_state;
+
+		/*
+		 * This might be called before swapping when commit is aborted,
+		 * in which case we have to cleanup the new state.
+		 */
+		if (old_plane_state == plane->state)
+			plane_state = new_plane_state;
+		else
+			plane_state = old_plane_state;
 
 		funcs = plane->helper_private;
 
@@ -1986,15 +1993,15 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 	int i;
 	long ret;
 	struct drm_connector *connector;
-	struct drm_connector_state *conn_state, *old_conn_state;
+	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state, *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state, *old_plane_state;
+	struct drm_plane_state *old_plane_state, *new_plane_state;
 	struct drm_crtc_commit *commit;
 
 	if (stall) {
-		for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		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);
@@ -2014,20 +2021,24 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		}
 	}
 
-	for_each_oldnew_connector_in_state(state, connector, old_conn_state, conn_state, i) {
+	for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_conn_state, i) {
 		WARN_ON(connector->state != old_conn_state);
 
-		connector->state->state = state;
-		swap(state->connectors[i].state, connector->state);
-		connector->state->state = NULL;
+		old_conn_state->state = state;
+		new_conn_state->state = NULL;
+
+		state->connectors[i].state = old_conn_state;
+		connector->state = new_conn_state;
 	}
 
-	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		WARN_ON(crtc->state != old_crtc_state);
 
-		crtc->state->state = state;
-		swap(state->crtcs[i].state, crtc->state);
-		crtc->state->state = NULL;
+		old_crtc_state->state = state;
+		new_crtc_state->state = NULL;
+
+		state->crtcs[i].state = old_crtc_state;
+		crtc->state = new_crtc_state;
 
 		if (state->crtcs[i].commit) {
 			spin_lock(&crtc->commit_lock);
@@ -2039,12 +2050,14 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		}
 	}
 
-	for_each_oldnew_plane_in_state(state, plane, old_plane_state, plane_state, i) {
+	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
 		WARN_ON(plane->state != old_plane_state);
 
-		plane->state->state = state;
-		swap(state->planes[i].state, plane->state);
-		plane->state->state = NULL;
+		old_plane_state->state = state;
+		new_plane_state->state = NULL;
+
+		state->planes[i].state = old_plane_state;
+		plane->state = new_plane_state;
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_swap_state);
@@ -2227,9 +2240,9 @@ static int update_output_state(struct drm_atomic_state *state,
 {
 	struct drm_device *dev = set->crtc->dev;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *conn_state;
+	struct drm_connector_state *new_conn_state;
 	int ret, i;
 
 	ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
@@ -2242,31 +2255,32 @@ static int update_output_state(struct drm_atomic_state *state,
 	if (ret)
 		return ret;
 
-	for_each_connector_in_state(state, connector, conn_state, i) {
-		if (conn_state->crtc == set->crtc) {
-			ret = drm_atomic_set_crtc_for_connector(conn_state,
+	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
+		if (new_conn_state->crtc == set->crtc) {
+			ret = drm_atomic_set_crtc_for_connector(new_conn_state,
 								NULL);
 			if (ret)
 				return ret;
+
 			/* Make sure legacy setCrtc always re-trains */
-			conn_state->link_status = DRM_LINK_STATUS_GOOD;
+			new_conn_state->link_status = DRM_LINK_STATUS_GOOD;
 		}
 	}
 
 	/* Then set all connectors from set->connectors on the target crtc */
 	for (i = 0; i < set->num_connectors; i++) {
-		conn_state = drm_atomic_get_connector_state(state,
+		new_conn_state = drm_atomic_get_connector_state(state,
 							    set->connectors[i]);
-		if (IS_ERR(conn_state))
-			return PTR_ERR(conn_state);
+		if (IS_ERR(new_conn_state))
+			return PTR_ERR(new_conn_state);
 
-		ret = drm_atomic_set_crtc_for_connector(conn_state,
+		ret = drm_atomic_set_crtc_for_connector(new_conn_state,
 							set->crtc);
 		if (ret)
 			return ret;
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 		/* Don't update ->enable for the CRTC in the set_config request,
 		 * since a mismatch would indicate a bug in the upper layers.
 		 * The actual modeset code later on will catch any
@@ -2274,13 +2288,13 @@ static int update_output_state(struct drm_atomic_state *state,
 		if (crtc == set->crtc)
 			continue;
 
-		if (!crtc_state->connector_mask) {
-			ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
+		if (!new_crtc_state->connector_mask) {
+			ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
 								NULL);
 			if (ret < 0)
 				return ret;
 
-			crtc_state->active = false;
+			new_crtc_state->active = false;
 		}
 	}
 
@@ -2583,21 +2597,21 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
 {
 	int i;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *conn_state;
+	struct drm_connector_state *new_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 
 	state->acquire_ctx = ctx;
 
-	for_each_new_plane_in_state(state, plane, plane_state, i)
+	for_each_new_plane_in_state(state, plane, new_plane_state, i)
 		state->planes[i].old_state = plane->state;
 
-	for_each_new_crtc_in_state(state, crtc, crtc_state, i)
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
 		state->crtcs[i].old_state = crtc->state;
 
-	for_each_new_connector_in_state(state, connector, conn_state, i)
+	for_each_new_connector_in_state(state, connector, new_conn_state, i)
 		state->connectors[i].old_state = connector->state;
 
 	return drm_atomic_commit(state);
-- 
2.7.4


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4.1 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v4.
  2017-03-01  1:04   ` Laurent Pinchart
@ 2017-03-01  9:22     ` Maarten Lankhorst
  0 siblings, 0 replies; 15+ messages in thread
From: Maarten Lankhorst @ 2017-03-01  9:22 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel; +Cc: intel-gfx

This is a straightforward conversion that converts all the users of
get_existing_state in atomic core to use get_old_state or get_new_state

Changes since v1:
- Fix using the wrong state in drm_atomic_helper_update_legacy_modeset_state.
Changes since v2:
- Use the correct state in disable_outputs()
Changes since v3:
- Rebase for link status training.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c            |  6 ++---
 drivers/gpu/drm/drm_atomic_helper.c     | 47 +++++++++++++++++----------------
 drivers/gpu/drm/drm_blend.c             |  3 +--
 drivers/gpu/drm/drm_simple_kms_helper.c |  4 +--
 4 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 236d947011f9..9b892af7811a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1374,8 +1374,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
 		return 0;
 
 	if (conn_state->crtc) {
-		crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state,
-								conn_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(conn_state->state,
+							   conn_state->crtc);
 
 		crtc_state->connector_mask &=
 			~(1 << drm_connector_index(conn_state->connector));
@@ -1492,7 +1492,7 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state,
 {
 	struct drm_plane *plane;
 
-	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
+	WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc));
 
 	drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) {
 		struct drm_plane_state *plane_state =
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index bc96d31bd388..4e26b73bb0d5 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -70,8 +70,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 	struct drm_crtc_state *crtc_state;
 
 	if (old_plane_state->crtc) {
-		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								old_plane_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state,
+							   old_plane_state->crtc);
 
 		if (WARN_ON(!crtc_state))
 			return;
@@ -80,8 +80,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 	}
 
 	if (plane_state->crtc) {
-		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								plane_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc);
 
 		if (WARN_ON(!crtc_state))
 			return;
@@ -150,7 +149,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct drm_crtc_state *crtc_state;
 
-		if (drm_atomic_get_existing_connector_state(state, connector))
+		if (drm_atomic_get_new_connector_state(state, connector))
 			continue;
 
 		encoder = connector->state->best_encoder;
@@ -178,7 +177,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 				 new_conn_state->crtc->base.id, new_conn_state->crtc->name,
 				 connector->base.id, connector->name);
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc);
 
 		ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL);
 		if (ret)
@@ -219,7 +218,7 @@ set_best_encoder(struct drm_atomic_state *state,
 		 */
 		WARN_ON(!crtc && encoder != conn_state->best_encoder);
 		if (crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
 			crtc_state->encoder_mask &=
 				~(1 << drm_encoder_index(conn_state->best_encoder));
@@ -230,7 +229,7 @@ set_best_encoder(struct drm_atomic_state *state,
 		crtc = conn_state->crtc;
 		WARN_ON(!crtc);
 		if (crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
 			crtc_state->encoder_mask |=
 				1 << drm_encoder_index(encoder);
@@ -263,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
 
 		set_best_encoder(state, new_connector_state, NULL);
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state, encoder_crtc);
 		crtc_state->connectors_changed = true;
 
 		return;
@@ -286,12 +285,12 @@ update_connector_routing(struct drm_atomic_state *state,
 
 	if (old_connector_state->crtc != new_connector_state->crtc) {
 		if (old_connector_state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, old_connector_state->crtc);
 			crtc_state->connectors_changed = true;
 		}
 
 		if (new_connector_state->crtc) {
-			crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, new_connector_state->crtc);
 			crtc_state->connectors_changed = true;
 		}
 	}
@@ -350,7 +349,7 @@ update_connector_routing(struct drm_atomic_state *state,
 
 	set_best_encoder(state, new_connector_state, new_encoder);
 
-	crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
+	crtc_state = drm_atomic_get_new_crtc_state(state, new_connector_state->crtc);
 	crtc_state->connectors_changed = true;
 
 	DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n",
@@ -392,7 +391,7 @@ mode_fixup(struct drm_atomic_state *state)
 			continue;
 
 		new_crtc_state =
-			drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc);
+			drm_atomic_get_new_crtc_state(state, new_conn_state->crtc);
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
@@ -534,8 +533,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 		if (ret)
 			return ret;
 		if (old_connector_state->crtc) {
-			new_crtc_state = drm_atomic_get_existing_crtc_state(state,
-									    old_connector_state->crtc);
+			new_crtc_state = drm_atomic_get_new_crtc_state(state,
+								       old_connector_state->crtc);
 			if (old_connector_state->link_status !=
 			    new_connector_state->link_status)
 				new_crtc_state->connectors_changed = true;
@@ -711,8 +710,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 		if (!old_conn_state->crtc)
 			continue;
 
-		old_crtc_state = drm_atomic_get_existing_crtc_state(old_state,
-								    old_conn_state->crtc);
+		old_crtc_state = drm_atomic_get_old_crtc_state(old_state, old_conn_state->crtc);
 
 		if (!old_crtc_state->active ||
 		    !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
@@ -841,14 +839,17 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 	/* set legacy state in the crtc structure */
 	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
 		struct drm_plane *primary = crtc->primary;
+		struct drm_plane_state *new_plane_state;
 
 		crtc->mode = new_crtc_state->mode;
 		crtc->enabled = new_crtc_state->enable;
 
-		if (drm_atomic_get_existing_plane_state(old_state, primary) &&
-		    primary->state->crtc == crtc) {
-			crtc->x = primary->state->src_x >> 16;
-			crtc->y = primary->state->src_y >> 16;
+		new_plane_state =
+			drm_atomic_get_new_plane_state(old_state, primary);
+
+		if (new_plane_state && new_plane_state->crtc == crtc) {
+			crtc->x = new_plane_state->src_x >> 16;
+			crtc->y = new_plane_state->src_y >> 16;
 		}
 
 		if (new_crtc_state->enable)
@@ -1850,7 +1851,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state)
 
 	drm_for_each_plane_mask(plane, crtc->dev, plane_mask) {
 		struct drm_plane_state *old_plane_state =
-			drm_atomic_get_existing_plane_state(old_state, plane);
+			drm_atomic_get_old_plane_state(old_state, plane);
 		const struct drm_plane_helper_funcs *plane_funcs;
 
 		plane_funcs = plane->helper_private;
@@ -2953,7 +2954,7 @@ int drm_atomic_helper_page_flip_target(
 	if (ret != 0)
 		goto fail;
 
-	crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	if (WARN_ON(!crtc_state)) {
 		ret = -EINVAL;
 		goto fail;
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 665aafc6ad68..d7053bb67db6 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -387,8 +387,7 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
 		if (!crtc)
 			continue;
 		if (plane->state->zpos != plane_state->zpos) {
-			crtc_state =
-				drm_atomic_get_existing_crtc_state(state, crtc);
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 			crtc_state->zpos_changed = true;
 		}
 	}
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 35c5d99296b9..16789faa9291 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -86,8 +86,8 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
 	int ret;
 
 	pipe = container_of(plane, struct drm_simple_display_pipe, plane);
-	crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,
-							&pipe->crtc);
+	crtc_state = drm_atomic_get_new_crtc_state(plane_state->state,
+						   &pipe->crtc);
 	if (crtc_state->enable != !!plane_state->crtc)
 		return -EINVAL; /* plane must match crtc enable state */
 
-- 
2.7.4


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3.
  2017-03-01  9:21     ` [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3 Maarten Lankhorst
@ 2017-03-01 10:06       ` Laurent Pinchart
  0 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2017-03-01 10:06 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel

Hi Maarten,

Thank you for the resend.

For the whole series,

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

On Wednesday 01 Mar 2017 10:21:26 Maarten Lankhorst wrote:
> There are new iterator macros that annotate whether the new or old
> state should be used. This is better than using a state that depends on
> whether it's called before or after swap. For clarity, also rename the
> variables from $obj_state to (old,new)_$obj_state as well.
> 
> Changes since v1:
> - Use old/new_*_state for variable names as much as possible. (pinchartl)
> - Expand commit message.
> Changes since v2:
> - Rebase on top of link training patches.
> - free -> cleanup (pinchartl)
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 446 ++++++++++++++++-----------------
>  1 file changed, 230 insertions(+), 216 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index 6b12396f718b..aa1e7609b024
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -63,14 +63,15 @@
>   */
>  static void
>  drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
> +				struct drm_plane_state *old_plane_state,
>  				struct drm_plane_state *plane_state,
>  				struct drm_plane *plane)
>  {
>  	struct drm_crtc_state *crtc_state;
> 
> -	if (plane->state->crtc) {
> +	if (old_plane_state->crtc) {
>  		crtc_state = drm_atomic_get_existing_crtc_state(state,
> -								plane->state-
>crtc);
> +								
old_plane_state->crtc);
> 
>  		if (WARN_ON(!crtc_state))
>  			return;
> @@ -92,7 +93,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> *state, static int handle_conflicting_encoders(struct drm_atomic_state
> *state, bool disable_conflicting_encoders)
>  {
> -	struct drm_connector_state *conn_state;
> +	struct drm_connector_state *new_conn_state;
>  	struct drm_connector *connector;
>  	struct drm_connector_list_iter conn_iter;
>  	struct drm_encoder *encoder;
> @@ -104,15 +105,15 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state, * part of the state. If the same encoder is
> assigned to multiple * connectors bail out.
>  	 */
> -	for_each_connector_in_state(state, connector, conn_state, i) {
> +	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
>  		const struct drm_connector_helper_funcs *funcs =
> connector->helper_private; struct drm_encoder *new_encoder;
> 
> -		if (!conn_state->crtc)
> +		if (!new_conn_state->crtc)
>  			continue;
> 
>  		if (funcs->atomic_best_encoder)
> -			new_encoder = funcs->atomic_best_encoder(connector, 
conn_state);
> +			new_encoder = funcs->atomic_best_encoder(connector, 
new_conn_state);
>  		else if (funcs->best_encoder)
>  			new_encoder = funcs->best_encoder(connector);
>  		else
> @@ -166,20 +167,20 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state, goto out;
>  		}
> 
> -		conn_state = drm_atomic_get_connector_state(state, connector);
> -		if (IS_ERR(conn_state)) {
> -			ret = PTR_ERR(conn_state);
> +		new_conn_state = drm_atomic_get_connector_state(state, 
connector);
> +		if (IS_ERR(new_conn_state)) {
> +			ret = PTR_ERR(new_conn_state);
>  			goto out;
>  		}
> 
>  		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], 
disabling
> [CONNECTOR:%d:%s]\n", encoder->base.id, encoder->name,
> -				 conn_state->crtc->base.id, conn_state->crtc-
>name,
> +				 new_conn_state->crtc->base.id, 
new_conn_state->crtc->name,
>  				 connector->base.id, connector->name);
> 
> -		crtc_state = drm_atomic_get_existing_crtc_state(state, 
conn_state->crtc);
> +		crtc_state = drm_atomic_get_existing_crtc_state(state,
> new_conn_state->crtc);
> 
> -		ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
> +		ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL);
>  		if (ret)
>  			goto out;
> 
> @@ -245,22 +246,22 @@ steal_encoder(struct drm_atomic_state *state,
>  {
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *connector_state;
> +	struct drm_connector_state *old_connector_state, *new_connector_state;
>  	int i;
> 
> -	for_each_connector_in_state(state, connector, connector_state, i) {
> +	for_each_oldnew_connector_in_state(state, connector, 
old_connector_state,
> new_connector_state, i) { struct drm_crtc *encoder_crtc;
> 
> -		if (connector_state->best_encoder != encoder)
> +		if (new_connector_state->best_encoder != encoder)
>  			continue;
> 
> -		encoder_crtc = connector->state->crtc;
> +		encoder_crtc = old_connector_state->crtc;
> 
>  		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], 
stealing it\n",
> encoder->base.id, encoder->name,
>  				 encoder_crtc->base.id, encoder_crtc->name);
> 
> -		set_best_encoder(state, connector_state, NULL);
> +		set_best_encoder(state, new_connector_state, NULL);
> 
>  		crtc_state = drm_atomic_get_existing_crtc_state(state, 
encoder_crtc);
>  		crtc_state->connectors_changed = true;
> @@ -272,7 +273,8 @@ steal_encoder(struct drm_atomic_state *state,
>  static int
>  update_connector_routing(struct drm_atomic_state *state,
>  			 struct drm_connector *connector,
> -			 struct drm_connector_state *connector_state)
> +			 struct drm_connector_state *old_connector_state,
> +			 struct drm_connector_state *new_connector_state)
>  {
>  	const struct drm_connector_helper_funcs *funcs;
>  	struct drm_encoder *new_encoder;
> @@ -282,24 +284,24 @@ update_connector_routing(struct drm_atomic_state
> *state, connector->base.id,
>  			 connector->name);
> 
> -	if (connector->state->crtc != connector_state->crtc) {
> -		if (connector->state->crtc) {
> -			crtc_state = drm_atomic_get_existing_crtc_state(state,
> connector->state->crtc); +	if (old_connector_state->crtc !=
> new_connector_state->crtc) {
> +		if (old_connector_state->crtc) {
> +			crtc_state = drm_atomic_get_existing_crtc_state(state,
> old_connector_state->crtc); crtc_state->connectors_changed = true;
>  		}
> 
> -		if (connector_state->crtc) {
> -			crtc_state = drm_atomic_get_existing_crtc_state(state,
> connector_state->crtc); +		if (new_connector_state->crtc) {
> +			crtc_state = drm_atomic_get_existing_crtc_state(state,
> new_connector_state->crtc); crtc_state->connectors_changed = true;
>  		}
>  	}
> 
> -	if (!connector_state->crtc) {
> +	if (!new_connector_state->crtc) {
>  		DRM_DEBUG_ATOMIC("Disabling [CONNECTOR:%d:%s]\n",
>  				connector->base.id,
>  				connector->name);
> 
> -		set_best_encoder(state, connector_state, NULL);
> +		set_best_encoder(state, new_connector_state, NULL);
> 
>  		return 0;
>  	}
> @@ -308,7 +310,7 @@ update_connector_routing(struct drm_atomic_state *state,
> 
>  	if (funcs->atomic_best_encoder)
>  		new_encoder = funcs->atomic_best_encoder(connector,
> -							 connector_state);
> +							 new_connector_state);
>  	else if (funcs->best_encoder)
>  		new_encoder = funcs->best_encoder(connector);
>  	else
> @@ -321,34 +323,34 @@ update_connector_routing(struct drm_atomic_state
> *state, return -EINVAL;
>  	}
> 
> -	if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) {
> +	if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) {
>  		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:
%s]\n",
>  				 new_encoder->base.id,
>  				 new_encoder->name,
> -				 connector_state->crtc->base.id,
> -				 connector_state->crtc->name);
> +				 new_connector_state->crtc->base.id,
> +				 new_connector_state->crtc->name);
>  		return -EINVAL;
>  	}
> 
> -	if (new_encoder == connector_state->best_encoder) {
> -		set_best_encoder(state, connector_state, new_encoder);
> +	if (new_encoder == new_connector_state->best_encoder) {
> +		set_best_encoder(state, new_connector_state, new_encoder);
> 
>  		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] keeps [ENCODER:%d:%s], now 
on
> [CRTC:%d:%s]\n", connector->base.id,
>  				 connector->name,
>  				 new_encoder->base.id,
>  				 new_encoder->name,
> -				 connector_state->crtc->base.id,
> -				 connector_state->crtc->name);
> +				 new_connector_state->crtc->base.id,
> +				 new_connector_state->crtc->name);
> 
>  		return 0;
>  	}
> 
>  	steal_encoder(state, new_encoder);
> 
> -	set_best_encoder(state, connector_state, new_encoder);
> +	set_best_encoder(state, new_connector_state, new_encoder);
> 
> -	crtc_state = drm_atomic_get_existing_crtc_state(state,
> connector_state->crtc); +	crtc_state =
> drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc);
> crtc_state->connectors_changed = true;
> 
>  	DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on
> [CRTC:%d:%s]\n", @@ -356,8 +358,8 @@ update_connector_routing(struct
> drm_atomic_state *state, connector->name,
>  			 new_encoder->base.id,
>  			 new_encoder->name,
> -			 connector_state->crtc->base.id,
> -			 connector_state->crtc->name);
> +			 new_connector_state->crtc->base.id,
> +			 new_connector_state->crtc->name);
> 
>  	return 0;
>  }
> @@ -366,57 +368,57 @@ static int
>  mode_fixup(struct drm_atomic_state *state)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *conn_state;
> +	struct drm_connector_state *new_conn_state;
>  	int i;
>  	int ret;
> 
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		if (!crtc_state->mode_changed &&
> -		    !crtc_state->connectors_changed)
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (!new_crtc_state->mode_changed &&
> +		    !new_crtc_state->connectors_changed)
>  			continue;
> 
> -		drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode);
> +		drm_mode_copy(&new_crtc_state->adjusted_mode, &new_crtc_state-
>mode);
>  	}
> 
> -	for_each_connector_in_state(state, connector, conn_state, i) {
> +	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
>  		const struct drm_encoder_helper_funcs *funcs;
>  		struct drm_encoder *encoder;
> 
> -		WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc);
> +		WARN_ON(!!new_conn_state->best_encoder != !!new_conn_state-
>crtc);
> 
> -		if (!conn_state->crtc || !conn_state->best_encoder)
> +		if (!new_conn_state->crtc || !new_conn_state->best_encoder)
>  			continue;
> 
> -		crtc_state = drm_atomic_get_existing_crtc_state(state,
> -								conn_state-
>crtc);
> +		new_crtc_state =
> +			drm_atomic_get_existing_crtc_state(state, 
new_conn_state->crtc);
> 
>  		/*
>  		 * Each encoder has at most one connector (since we always 
steal
>  		 * it away), so we won't call ->mode_fixup twice.
>  		 */
> -		encoder = conn_state->best_encoder;
> +		encoder = new_conn_state->best_encoder;
>  		funcs = encoder->helper_private;
> 
> -		ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state-
>mode,
> -				&crtc_state->adjusted_mode);
> +		ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state-
>mode,
> +				&new_crtc_state->adjusted_mode);
>  		if (!ret) {
>  			DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
>  			return -EINVAL;
>  		}
> 
>  		if (funcs && funcs->atomic_check) {
> -			ret = funcs->atomic_check(encoder, crtc_state,
> -						  conn_state);
> +			ret = funcs->atomic_check(encoder, new_crtc_state,
> +						  new_conn_state);
>  			if (ret) {
>  				DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check 
failed\n",
>  						 encoder->base.id, encoder-
>name);
>  				return ret;
>  			}
>  		} else if (funcs && funcs->mode_fixup) {
> -			ret = funcs->mode_fixup(encoder, &crtc_state->mode,
> -						&crtc_state->adjusted_mode);
> +			ret = funcs->mode_fixup(encoder, &new_crtc_state-
>mode,
> +						&new_crtc_state-
>adjusted_mode);
>  			if (!ret) {
>  				DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup 
failed\n",
>  						 encoder->base.id, encoder-
>name);
> @@ -425,22 +427,22 @@ mode_fixup(struct drm_atomic_state *state)
>  		}
>  	}
> 
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
> 
> -		if (!crtc_state->enable)
> +		if (!new_crtc_state->enable)
>  			continue;
> 
> -		if (!crtc_state->mode_changed &&
> -		    !crtc_state->connectors_changed)
> +		if (!new_crtc_state->mode_changed &&
> +		    !new_crtc_state->connectors_changed)
>  			continue;
> 
>  		funcs = crtc->helper_private;
>  		if (!funcs->mode_fixup)
>  			continue;
> 
> -		ret = funcs->mode_fixup(crtc, &crtc_state->mode,
> -					&crtc_state->adjusted_mode);
> +		ret = funcs->mode_fixup(crtc, &new_crtc_state->mode,
> +					&new_crtc_state->adjusted_mode);
>  		if (!ret) {
>  			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n",
>  					 crtc->base.id, crtc->name);
> @@ -487,19 +489,19 @@ drm_atomic_helper_check_modeset(struct drm_device
> *dev, struct drm_atomic_state *state)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *connector_state;
> +	struct drm_connector_state *old_connector_state, *new_connector_state;
>  	int i, ret;
> 
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) {
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state,
> i) { +		if (!drm_mode_equal(&old_crtc_state->mode, 
&new_crtc_state->mode))
> { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode changed\n",
>  					 crtc->base.id, crtc->name);
> -			crtc_state->mode_changed = true;
> +			new_crtc_state->mode_changed = true;
>  		}
> 
> -		if (crtc->state->enable != crtc_state->enable) {
> +		if (old_crtc_state->enable != new_crtc_state->enable) {
>  			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enable changed\n",
>  					 crtc->base.id, crtc->name);
> 
> @@ -511,8 +513,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  			 * The other way around is true as well. enable != 0
>  			 * iff connectors are attached and a mode is set.
>  			 */
> -			crtc_state->mode_changed = true;
> -			crtc_state->connectors_changed = true;
> +			new_crtc_state->mode_changed = true;
> +			new_crtc_state->connectors_changed = true;
>  		}
>  	}
> 
> @@ -520,22 +522,23 @@ drm_atomic_helper_check_modeset(struct drm_device
> *dev, if (ret)
>  		return ret;
> 
> -	for_each_connector_in_state(state, connector, connector_state, i) {
> +	for_each_oldnew_connector_in_state(state, connector, 
old_connector_state,
> new_connector_state, i) { /*
>  		 * This only sets crtc->connectors_changed for routing 
changes,
>  		 * drivers must set crtc->connectors_changed themselves when
>  		 * connector properties need to be updated.
>  		 */
>  		ret = update_connector_routing(state, connector,
> -					       connector_state);
> +					       old_connector_state,
> +					       new_connector_state);
>  		if (ret)
>  			return ret;
> -		if (connector->state->crtc) {
> -			crtc_state = drm_atomic_get_existing_crtc_state(state,
> -									
connector->state->crtc);
> -			if (connector->state->link_status !=
> -			    connector_state->link_status)
> -				crtc_state->connectors_changed = true;
> +		if (old_connector_state->crtc) {
> +			new_crtc_state = 
drm_atomic_get_existing_crtc_state(state,
> +									    
old_connector_state->crtc);
> +			if (old_connector_state->link_status !=
> +			    new_connector_state->link_status)
> +				new_crtc_state->connectors_changed = true;
>  		}
>  	}
> 
> @@ -545,28 +548,28 @@ drm_atomic_helper_check_modeset(struct drm_device
> *dev, * configuration. This must be done before calling mode_fixup in case
> a * crtc only changed its mode but has the same set of connectors. */
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state,
> i) { bool has_connectors =
> -			!!crtc_state->connector_mask;
> +			!!new_crtc_state->connector_mask;
> 
>  		/*
>  		 * We must set ->active_changed after walking connectors for
>  		 * otherwise an update that only changes active would result 
in
>  		 * a full modeset because update_connector_routing force that.
>  		 */
> -		if (crtc->state->active != crtc_state->active) {
> +		if (old_crtc_state->active != new_crtc_state->active) {
>  			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active changed\n",
>  					 crtc->base.id, crtc->name);
> -			crtc_state->active_changed = true;
> +			new_crtc_state->active_changed = true;
>  		}
> 
> -		if (!drm_atomic_crtc_needs_modeset(crtc_state))
> +		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
>  			continue;
> 
>  		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] needs all connectors, enable: 
%c, active:
> %c\n", crtc->base.id, crtc->name,
> -				 crtc_state->enable ? 'y' : 'n',
> -				 crtc_state->active ? 'y' : 'n');
> +				 new_crtc_state->enable ? 'y' : 'n',
> +				 new_crtc_state->active ? 'y' : 'n');
> 
>  		ret = drm_atomic_add_affected_connectors(state, crtc);
>  		if (ret != 0)
> @@ -576,7 +579,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  		if (ret != 0)
>  			return ret;
> 
> -		if (crtc_state->enable != has_connectors) {
> +		if (new_crtc_state->enable != has_connectors) {
>  			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors 
mismatch\n",
>  					 crtc->base.id, crtc->name);
> 
> @@ -609,22 +612,22 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> struct drm_atomic_state *state)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *new_plane_state, *old_plane_state;
>  	int i, ret = 0;
> 
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> +	for_each_oldnew_plane_in_state(state, plane, old_plane_state,
> new_plane_state, i) { const struct drm_plane_helper_funcs *funcs;
> 
>  		funcs = plane->helper_private;
> 
> -		drm_atomic_helper_plane_changed(state, plane_state, plane);
> +		drm_atomic_helper_plane_changed(state, old_plane_state, 
new_plane_state,
> plane);
> 
>  		if (!funcs || !funcs->atomic_check)
>  			continue;
> 
> -		ret = funcs->atomic_check(plane, plane_state);
> +		ret = funcs->atomic_check(plane, new_plane_state);
>  		if (ret) {
>  			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check 
failed\n",
>  					 plane->base.id, plane->name);
> @@ -632,7 +635,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>  		}
>  	}
> 
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
> 
>  		funcs = crtc->helper_private;
> @@ -640,7 +643,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>  		if (!funcs || !funcs->atomic_check)
>  			continue;
> 
> -		ret = funcs->atomic_check(crtc, crtc_state);
> +		ret = funcs->atomic_check(crtc, new_crtc_state);
>  		if (ret) {
>  			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check 
failed\n",
>  					 crtc->base.id, crtc->name);
> @@ -694,12 +697,12 @@ static void
>  disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> {
>  	struct drm_connector *connector;
> -	struct drm_connector_state *old_conn_state;
> +	struct drm_connector_state *old_conn_state, *new_conn_state;
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *old_crtc_state;
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	int i;
> 
> -	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
> +	for_each_oldnew_connector_in_state(old_state, connector, 
old_conn_state,
> new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs;
>  		struct drm_encoder *encoder;
> 
> @@ -736,7 +739,7 @@ disable_outputs(struct drm_device *dev, struct
> drm_atomic_state *old_state)
> 
>  		/* Right function depends upon target state. */
>  		if (funcs) {
> -			if (connector->state->crtc && funcs->prepare)
> +			if (new_conn_state->crtc && funcs->prepare)
>  				funcs->prepare(encoder);
>  			else if (funcs->disable)
>  				funcs->disable(encoder);
> @@ -747,11 +750,11 @@ disable_outputs(struct drm_device *dev, struct
> drm_atomic_state *old_state) drm_bridge_post_disable(encoder->bridge);
>  	}
> 
> -	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs;
> 
>  		/* Shut down everything that needs a full modeset. */
> -		if (!drm_atomic_crtc_needs_modeset(crtc->state))
> +		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
>  			continue;
> 
>  		if (!old_crtc_state->active)
> @@ -764,7 +767,7 @@ disable_outputs(struct drm_device *dev, struct
> drm_atomic_state *old_state)
> 
> 
>  		/* Right function depends upon target state. */
> -		if (crtc->state->enable && funcs->prepare)
> +		if (new_crtc_state->enable && funcs->prepare)
>  			funcs->prepare(crtc);
>  		else if (funcs->atomic_disable)
>  			funcs->atomic_disable(crtc, old_crtc_state);
> @@ -793,13 +796,13 @@ drm_atomic_helper_update_legacy_modeset_state(struct
> drm_device *dev, struct drm_atomic_state *old_state)
>  {
>  	struct drm_connector *connector;
> -	struct drm_connector_state *old_conn_state;
> +	struct drm_connector_state *old_conn_state, *new_conn_state;
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *old_crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	int i;
> 
>  	/* clear out existing links and update dpms */
> -	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
> +	for_each_oldnew_connector_in_state(old_state, connector, 
old_conn_state,
> new_conn_state, i) { if (connector->encoder) {
>  			WARN_ON(!connector->encoder->crtc);
> 
> @@ -807,7 +810,7 @@ drm_atomic_helper_update_legacy_modeset_state(struct
> drm_device *dev, connector->encoder = NULL;
>  		}
> 
> -		crtc = connector->state->crtc;
> +		crtc = new_conn_state->crtc;
>  		if ((!crtc && old_conn_state->crtc) ||
>  		    (crtc && drm_atomic_crtc_needs_modeset(crtc->state))) {
>  			struct drm_property *dpms_prop =
> @@ -824,23 +827,23 @@ drm_atomic_helper_update_legacy_modeset_state(struct
> drm_device *dev, }
> 
>  	/* set new links */
> -	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
> -		if (!connector->state->crtc)
> +	for_each_new_connector_in_state(old_state, connector, new_conn_state, 
i) {
> +		if (!new_conn_state->crtc)
>  			continue;
> 
> -		if (WARN_ON(!connector->state->best_encoder))
> +		if (WARN_ON(!new_conn_state->best_encoder))
>  			continue;
> 
> -		connector->encoder = connector->state->best_encoder;
> -		connector->encoder->crtc = connector->state->crtc;
> +		connector->encoder = new_conn_state->best_encoder;
> +		connector->encoder->crtc = new_conn_state->crtc;
>  	}
> 
>  	/* set legacy state in the crtc structure */
> -	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
>  		struct drm_plane *primary = crtc->primary;
> 
> -		crtc->mode = crtc->state->mode;
> -		crtc->enabled = crtc->state->enable;
> +		crtc->mode = new_crtc_state->mode;
> +		crtc->enabled = new_crtc_state->enable;
> 
>  		if (drm_atomic_get_existing_plane_state(old_state, primary) &&
>  		    primary->state->crtc == crtc) {
> @@ -848,9 +851,9 @@ drm_atomic_helper_update_legacy_modeset_state(struct
> drm_device *dev, crtc->y = primary->state->src_y >> 16;
>  		}
> 
> -		if (crtc->state->enable)
> +		if (new_crtc_state->enable)
>  			drm_calc_timestamping_constants(crtc,
> -							&crtc->state-
>adjusted_mode);
> +							&new_crtc_state-
>adjusted_mode);
>  	}
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_update_legacy_modeset_state);
> @@ -859,20 +862,20 @@ static void
>  crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *old_crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *old_conn_state;
> +	struct drm_connector_state *new_conn_state;
>  	int i;
> 
> -	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
> 
> -		if (!crtc->state->mode_changed)
> +		if (!new_crtc_state->mode_changed)
>  			continue;
> 
>  		funcs = crtc->helper_private;
> 
> -		if (crtc->state->enable && funcs->mode_set_nofb) {
> +		if (new_crtc_state->enable && funcs->mode_set_nofb) {
>  			DRM_DEBUG_ATOMIC("modeset on [CRTC:%d:%s]\n",
>  					 crtc->base.id, crtc->name);
> 
> @@ -880,18 +883,17 @@ crtc_set_mode(struct drm_device *dev, struct
> drm_atomic_state *old_state) }
>  	}
> 
> -	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
> +	for_each_new_connector_in_state(old_state, connector, new_conn_state, 
i) {
> const struct drm_encoder_helper_funcs *funcs;
> -		struct drm_crtc_state *new_crtc_state;
>  		struct drm_encoder *encoder;
>  		struct drm_display_mode *mode, *adjusted_mode;
> 
> -		if (!connector->state->best_encoder)
> +		if (!new_conn_state->best_encoder)
>  			continue;
> 
> -		encoder = connector->state->best_encoder;
> +		encoder = new_conn_state->best_encoder;
>  		funcs = encoder->helper_private;
> -		new_crtc_state = connector->state->crtc->state;
> +		new_crtc_state = new_conn_state->crtc->state;
>  		mode = &new_crtc_state->mode;
>  		adjusted_mode = &new_crtc_state->adjusted_mode;
> 
> @@ -907,7 +909,7 @@ crtc_set_mode(struct drm_device *dev, struct
> drm_atomic_state *old_state) */
>  		if (funcs && funcs->atomic_mode_set) {
>  			funcs->atomic_mode_set(encoder, new_crtc_state,
> -					       connector->state);
> +					       new_conn_state);
>  		} else if (funcs && funcs->mode_set) {
>  			funcs->mode_set(encoder, mode, adjusted_mode);
>  		}
> @@ -959,24 +961,24 @@ void drm_atomic_helper_commit_modeset_enables(struct
> drm_device *dev, struct drm_atomic_state *old_state)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *old_crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *old_conn_state;
> +	struct drm_connector_state *new_conn_state;
>  	int i;
> 
> -	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
> 
>  		/* Need to filter out CRTCs where only planes change. */
> -		if (!drm_atomic_crtc_needs_modeset(crtc->state))
> +		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
>  			continue;
> 
> -		if (!crtc->state->active)
> +		if (!new_crtc_state->active)
>  			continue;
> 
>  		funcs = crtc->helper_private;
> 
> -		if (crtc->state->enable) {
> +		if (new_crtc_state->enable) {
>  			DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n",
>  					 crtc->base.id, crtc->name);
> 
> @@ -987,18 +989,18 @@ void drm_atomic_helper_commit_modeset_enables(struct
> drm_device *dev, }
>  	}
> 
> -	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
> +	for_each_new_connector_in_state(old_state, connector, new_conn_state, 
i) {
> const struct drm_encoder_helper_funcs *funcs;
>  		struct drm_encoder *encoder;
> 
> -		if (!connector->state->best_encoder)
> +		if (!new_conn_state->best_encoder)
>  			continue;
> 
> -		if (!connector->state->crtc->state->active ||
> -		    !drm_atomic_crtc_needs_modeset(connector->state->crtc-
>state))
> +		if (!new_conn_state->crtc->state->active ||
> +		    !drm_atomic_crtc_needs_modeset(new_conn_state->crtc-
>state))
>  			continue;
> 
> -		encoder = connector->state->best_encoder;
> +		encoder = new_conn_state->best_encoder;
>  		funcs = encoder->helper_private;
> 
>  		DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n",
> @@ -1048,29 +1050,26 @@ int drm_atomic_helper_wait_for_fences(struct
> drm_device *dev, bool pre_swap)
>  {
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *new_plane_state;
>  	int i, ret;
> 
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> -		if (!pre_swap)
> -			plane_state = plane->state;
> -
> -		if (!plane_state->fence)
> +	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
> +		if (!new_plane_state->fence)
>  			continue;
> 
> -		WARN_ON(!plane_state->fb);
> +		WARN_ON(!new_plane_state->fb);
> 
>  		/*
>  		 * If waiting for fences pre-swap (ie: nonblock), userspace 
can
>  		 * still interrupt the operation. Instead of blocking until 
the
>  		 * timer expires, make the wait interruptible.
>  		 */
> -		ret = dma_fence_wait(plane_state->fence, pre_swap);
> +		ret = dma_fence_wait(new_plane_state->fence, pre_swap);
>  		if (ret)
>  			return ret;
> 
> -		dma_fence_put(plane_state->fence);
> -		plane_state->fence = NULL;
> +		dma_fence_put(new_plane_state->fence);
> +		new_plane_state->fence = NULL;
>  	}
> 
>  	return 0;
> @@ -1093,7 +1092,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device
> *dev, struct drm_atomic_state *old_state)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *old_crtc_state;
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	int i, ret;
>  	unsigned crtc_mask = 0;
> 
> @@ -1104,9 +1103,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device
> *dev, if (old_state->legacy_cursor_update)
>  		return;
> 
> -	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> -		struct drm_crtc_state *new_crtc_state = crtc->state;
> -
> +	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> new_crtc_state, i) { if (!new_crtc_state->active ||
> !new_crtc_state->planes_changed) continue;
> 
> @@ -1118,7 +1115,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device
> *dev, old_state->crtcs[i].last_vblank_count = drm_crtc_vblank_count(crtc);
> }
> 
> -	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +	for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
>  		if (!(crtc_mask & drm_crtc_mask(crtc)))
>  			continue;
> 
> @@ -1427,11 +1424,11 @@ 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_state *old_crtc_state, *new_crtc_state;
>  	struct drm_crtc_commit *commit;
>  	int i, ret;
> 
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state,
> i) { commit = kzalloc(sizeof(*commit), GFP_KERNEL);
>  		if (!commit)
>  			return -ENOMEM;
> @@ -1452,7 +1449,7 @@ int drm_atomic_helper_setup_commit(struct
> drm_atomic_state *state, /* 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) {
> +		if (!old_crtc_state->active && !new_crtc_state->active) {
>  			complete_all(&commit->flip_done);
>  			continue;
>  		}
> @@ -1463,17 +1460,17 @@ int drm_atomic_helper_setup_commit(struct
> drm_atomic_state *state, continue;
>  		}
> 
> -		if (!crtc_state->event) {
> +		if (!new_crtc_state->event) {
>  			commit->event = kzalloc(sizeof(*commit->event),
>  						GFP_KERNEL);
>  			if (!commit->event)
>  				return -ENOMEM;
> 
> -			crtc_state->event = commit->event;
> +			new_crtc_state->event = commit->event;
>  		}
> 
> -		crtc_state->event->base.completion = &commit->flip_done;
> -		crtc_state->event->base.completion_release = 
release_crtc_commit;
> +		new_crtc_state->event->base.completion = &commit->flip_done;
> +		new_crtc_state->event->base.completion_release = 
release_crtc_commit;
>  		drm_crtc_commit_get(commit);
>  	}
> 
> @@ -1512,12 +1509,12 @@ 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 *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_crtc_commit *commit;
>  	int i;
>  	long ret;
> 
> -	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
> +	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
>  		spin_lock(&crtc->commit_lock);
>  		commit = preceeding_commit(crtc);
>  		if (commit)
> @@ -1564,17 +1561,17 @@
> 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 *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_crtc_commit *commit;
>  	int i;
> 
> -	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
> +	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
>  		commit = old_state->crtcs[i].commit;
>  		if (!commit)
>  			continue;
> 
>  		/* backend must have consumed any event by now */
> -		WARN_ON(crtc->state->event);
> +		WARN_ON(new_crtc_state->event);
>  		spin_lock(&crtc->commit_lock);
>  		complete_all(&commit->hw_done);
>  		spin_unlock(&crtc->commit_lock);
> @@ -1596,12 +1593,12 @@ 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 *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_crtc_commit *commit;
>  	int i;
>  	long ret;
> 
> -	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
> +	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
>  		commit = old_state->crtcs[i].commit;
>  		if (WARN_ON(!commit))
>  			continue;
> @@ -1652,16 +1649,16 @@ int drm_atomic_helper_prepare_planes(struct
> drm_device *dev, struct drm_atomic_state *state)
>  {
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *new_plane_state;
>  	int ret, i, j;
> 
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> +	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> 
>  		funcs = plane->helper_private;
> 
>  		if (funcs->prepare_fb) {
> -			ret = funcs->prepare_fb(plane, plane_state);
> +			ret = funcs->prepare_fb(plane, new_plane_state);
>  			if (ret)
>  				goto fail;
>  		}
> @@ -1670,7 +1667,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device
> *dev, return 0;
> 
>  fail:
> -	for_each_plane_in_state(state, plane, plane_state, j) {
> +	for_each_new_plane_in_state(state, plane, new_plane_state, j) {
>  		const struct drm_plane_helper_funcs *funcs;
> 
>  		if (j >= i)
> @@ -1679,7 +1676,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device
> *dev, funcs = plane->helper_private;
> 
>  		if (funcs->cleanup_fb)
> -			funcs->cleanup_fb(plane, plane_state);
> +			funcs->cleanup_fb(plane, new_plane_state);
>  	}
> 
>  	return ret;
> @@ -1737,14 +1734,14 @@ void drm_atomic_helper_commit_planes(struct
> drm_device *dev, uint32_t flags)
>  {
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *old_crtc_state;
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct drm_plane *plane;
> -	struct drm_plane_state *old_plane_state;
> +	struct drm_plane_state *old_plane_state, *new_plane_state;
>  	int i;
>  	bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY;
>  	bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET;
> 
> -	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs;
> 
>  		funcs = crtc->helper_private;
> @@ -1752,13 +1749,13 @@ void drm_atomic_helper_commit_planes(struct
> drm_device *dev, if (!funcs || !funcs->atomic_begin)
>  			continue;
> 
> -		if (active_only && !crtc->state->active)
> +		if (active_only && !new_crtc_state->active)
>  			continue;
> 
>  		funcs->atomic_begin(crtc, old_crtc_state);
>  	}
> 
> -	for_each_plane_in_state(old_state, plane, old_plane_state, i) {
> +	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state,
> new_plane_state, i) { const struct drm_plane_helper_funcs *funcs;
>  		bool disabling;
> 
> @@ -1777,7 +1774,7 @@ void drm_atomic_helper_commit_planes(struct drm_device
> *dev, * CRTC to avoid skipping planes being disabled on an
>  			 * active CRTC.
>  			 */
> -			if (!disabling && !plane_crtc_active(plane->state))
> +			if (!disabling && !plane_crtc_active(new_plane_state))
>  				continue;
>  			if (disabling && !plane_crtc_active(old_plane_state))
>  				continue;
> @@ -1796,12 +1793,12 @@ void drm_atomic_helper_commit_planes(struct
> drm_device *dev, continue;
> 
>  			funcs->atomic_disable(plane, old_plane_state);
> -		} else if (plane->state->crtc || disabling) {
> +		} else if (new_plane_state->crtc || disabling) {
>  			funcs->atomic_update(plane, old_plane_state);
>  		}
>  	}
> 
> -	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs;
> 
>  		funcs = crtc->helper_private;
> @@ -1809,7 +1806,7 @@ void drm_atomic_helper_commit_planes(struct drm_device
> *dev, if (!funcs || !funcs->atomic_flush)
>  			continue;
> 
> -		if (active_only && !crtc->state->active)
> +		if (active_only && !new_crtc_state->active)
>  			continue;
> 
>  		funcs->atomic_flush(crtc, old_crtc_state);
> @@ -1936,11 +1933,21 @@ void drm_atomic_helper_cleanup_planes(struct
> drm_device *dev, struct drm_atomic_state *old_state)
>  {
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *old_plane_state, *new_plane_state;
>  	int i;
> 
> -	for_each_plane_in_state(old_state, plane, plane_state, i) {
> +	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state,
> new_plane_state, i) { const struct drm_plane_helper_funcs *funcs;
> +		struct drm_plane_state *plane_state;
> +
> +		/*
> +		 * This might be called before swapping when commit is 
aborted,
> +		 * in which case we have to cleanup the new state.
> +		 */
> +		if (old_plane_state == plane->state)
> +			plane_state = new_plane_state;
> +		else
> +			plane_state = old_plane_state;
> 
>  		funcs = plane->helper_private;
> 
> @@ -1986,15 +1993,15 @@ void drm_atomic_helper_swap_state(struct
> drm_atomic_state *state, int i;
>  	long ret;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *conn_state, *old_conn_state;
> +	struct drm_connector_state *old_conn_state, *new_conn_state;
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state, *old_crtc_state;
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state, *old_plane_state;
> +	struct drm_plane_state *old_plane_state, *new_plane_state;
>  	struct drm_crtc_commit *commit;
> 
>  	if (stall) {
> -		for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		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);
> @@ -2014,20 +2021,24 @@ void drm_atomic_helper_swap_state(struct
> drm_atomic_state *state, }
>  	}
> 
> -	for_each_oldnew_connector_in_state(state, connector, old_conn_state,
> conn_state, i) { +	for_each_oldnew_connector_in_state(state, connector,
> old_conn_state, new_conn_state, i) { WARN_ON(connector->state !=
> old_conn_state);
> 
> -		connector->state->state = state;
> -		swap(state->connectors[i].state, connector->state);
> -		connector->state->state = NULL;
> +		old_conn_state->state = state;
> +		new_conn_state->state = NULL;
> +
> +		state->connectors[i].state = old_conn_state;
> +		connector->state = new_conn_state;
>  	}
> 
> -	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, 
i)
> { +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) { WARN_ON(crtc->state != old_crtc_state);
> 
> -		crtc->state->state = state;
> -		swap(state->crtcs[i].state, crtc->state);
> -		crtc->state->state = NULL;
> +		old_crtc_state->state = state;
> +		new_crtc_state->state = NULL;
> +
> +		state->crtcs[i].state = old_crtc_state;
> +		crtc->state = new_crtc_state;
> 
>  		if (state->crtcs[i].commit) {
>  			spin_lock(&crtc->commit_lock);
> @@ -2039,12 +2050,14 @@ void drm_atomic_helper_swap_state(struct
> drm_atomic_state *state, }
>  	}
> 
> -	for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
plane_state,
> i) { +	for_each_oldnew_plane_in_state(state, plane, old_plane_state,
> new_plane_state, i) { WARN_ON(plane->state != old_plane_state);
> 
> -		plane->state->state = state;
> -		swap(state->planes[i].state, plane->state);
> -		plane->state->state = NULL;
> +		old_plane_state->state = state;
> +		new_plane_state->state = NULL;
> +
> +		state->planes[i].state = old_plane_state;
> +		plane->state = new_plane_state;
>  	}
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_swap_state);
> @@ -2227,9 +2240,9 @@ static int update_output_state(struct drm_atomic_state
> *state, {
>  	struct drm_device *dev = set->crtc->dev;
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *conn_state;
> +	struct drm_connector_state *new_conn_state;
>  	int ret, i;
> 
>  	ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
> @@ -2242,31 +2255,32 @@ static int update_output_state(struct
> drm_atomic_state *state, if (ret)
>  		return ret;
> 
> -	for_each_connector_in_state(state, connector, conn_state, i) {
> -		if (conn_state->crtc == set->crtc) {
> -			ret = drm_atomic_set_crtc_for_connector(conn_state,
> +	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
> +		if (new_conn_state->crtc == set->crtc) {
> +			ret = 
drm_atomic_set_crtc_for_connector(new_conn_state,
>  								NULL);
>  			if (ret)
>  				return ret;
> +
>  			/* Make sure legacy setCrtc always re-trains */
> -			conn_state->link_status = DRM_LINK_STATUS_GOOD;
> +			new_conn_state->link_status = DRM_LINK_STATUS_GOOD;
>  		}
>  	}
> 
>  	/* Then set all connectors from set->connectors on the target crtc */
>  	for (i = 0; i < set->num_connectors; i++) {
> -		conn_state = drm_atomic_get_connector_state(state,
> +		new_conn_state = drm_atomic_get_connector_state(state,
>  							    set-
>connectors[i]);
> -		if (IS_ERR(conn_state))
> -			return PTR_ERR(conn_state);
> +		if (IS_ERR(new_conn_state))
> +			return PTR_ERR(new_conn_state);
> 
> -		ret = drm_atomic_set_crtc_for_connector(conn_state,
> +		ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>  							set->crtc);
>  		if (ret)
>  			return ret;
>  	}
> 
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		/* Don't update ->enable for the CRTC in the set_config 
request,
>  		 * since a mismatch would indicate a bug in the upper layers.
>  		 * The actual modeset code later on will catch any
> @@ -2274,13 +2288,13 @@ static int update_output_state(struct
> drm_atomic_state *state, if (crtc == set->crtc)
>  			continue;
> 
> -		if (!crtc_state->connector_mask) {
> -			ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
> +		if (!new_crtc_state->connector_mask) {
> +			ret = 
drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
>  								NULL);
>  			if (ret < 0)
>  				return ret;
> 
> -			crtc_state->active = false;
> +			new_crtc_state->active = false;
>  		}
>  	}
> 
> @@ -2583,21 +2597,21 @@ int drm_atomic_helper_commit_duplicated_state(struct
> drm_atomic_state *state, {
>  	int i;
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *new_plane_state;
>  	struct drm_connector *connector;
> -	struct drm_connector_state *conn_state;
> +	struct drm_connector_state *new_conn_state;
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
> 
>  	state->acquire_ctx = ctx;
> 
> -	for_each_new_plane_in_state(state, plane, plane_state, i)
> +	for_each_new_plane_in_state(state, plane, new_plane_state, i)
>  		state->planes[i].old_state = plane->state;
> 
> -	for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
>  		state->crtcs[i].old_state = crtc->state;
> 
> -	for_each_new_connector_in_state(state, connector, conn_state, i)
> +	for_each_new_connector_in_state(state, connector, new_conn_state, i)
>  		state->connectors[i].old_state = connector->state;
> 
>  	return drm_atomic_commit(state);

-- 
Regards,

Laurent Pinchart

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-03-01 10:06 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst
2017-03-01  0:49   ` Laurent Pinchart
2017-03-01  9:09     ` Maarten Lankhorst
2017-03-01  9:21     ` [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3 Maarten Lankhorst
2017-03-01 10:06       ` Laurent Pinchart
2017-02-16 14:47 ` [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand Maarten Lankhorst
2017-03-01  0:53   ` Laurent Pinchart
2017-02-16 14:47 ` [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2 Maarten Lankhorst
2017-03-01  0:56   ` Laurent Pinchart
2017-02-16 14:47 ` [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3 Maarten Lankhorst
2017-03-01  1:04   ` Laurent Pinchart
2017-03-01  9:22     ` [PATCH v4.1 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v4 Maarten Lankhorst
2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst
2017-03-01  0:58   ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).