All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3.
Date: Wed, 01 Mar 2017 12:06:13 +0200	[thread overview]
Message-ID: <5214774.8laWGkfifG@avalon> (raw)
In-Reply-To: <aafa0d4d-474d-441f-3685-fa6c042ef37e@linux.intel.com>

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

  reply	other threads:[~2017-03-01 10:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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
2017-02-16 19:22 ` ✓ Fi.CI.BAT: success for drm/atomic: Add accessor macros for all atomic state. (rev5) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5214774.8laWGkfifG@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.