Re: [PATCH v2 1/6] drm/atomic: Clean up update_output_state.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 24, 2016 at 09:37:28AM +0100, Maarten Lankhorst wrote:
> With the addition of crtc_state->connector_mask other connectors from
> different crtc's aren't needed any more to determine if a crtc has
> connectors, so only call add_affected_connectors on the target crtc.
> This allows a cleanup to first remove all current connectors, then
> add all set->connectors to the target crtc.

The new order of things make sense to me.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

One thing I did notice is that we're not clearing the primary plane
crtc and fb when a crtc gets disabled due to its connector(s) getting
stolen. We do clear them when the crtc is explicitly disabled. Not sure
if that matters for anything, but it does seem a bit inconsistent.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 41 +++++++++++++++----------------------
>  1 file changed, 17 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4da4f2a49078..a0226d4b949a 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1761,28 +1761,18 @@ static int update_output_state(struct drm_atomic_state *state,
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_connector *connector;
>  	struct drm_connector_state *conn_state;
> -	int ret, i, j;
> +	int ret, i;
>  
>  	ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
>  			       state->acquire_ctx);
>  	if (ret)
>  		return ret;
>  
> -	/* First grab all affected connector/crtc states. */
> -	for (i = 0; i < set->num_connectors; i++) {
> -		conn_state = drm_atomic_get_connector_state(state,
> -							    set->connectors[i]);
> -		if (IS_ERR(conn_state))
> -			return PTR_ERR(conn_state);
> -	}
> -
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		ret = drm_atomic_add_affected_connectors(state, crtc);
> -		if (ret)
> -			return ret;
> -	}
> +	/* First disable all connectors on the target crtc. */
> +	ret = drm_atomic_add_affected_connectors(state, set->crtc);
> +	if (ret)
> +		return ret;
>  
> -	/* Then recompute connector->crtc links and crtc enabling state. */
>  	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,
> @@ -1790,16 +1780,19 @@ static int update_output_state(struct drm_atomic_state *state,
>  			if (ret)
>  				return ret;
>  		}
> +	}
>  
> -		for (j = 0; j < set->num_connectors; j++) {
> -			if (set->connectors[j] == connector) {
> -				ret = drm_atomic_set_crtc_for_connector(conn_state,
> -									set->crtc);
> -				if (ret)
> -					return ret;
> -				break;
> -			}
> -		}
> +	/* 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,
> +							    set->connectors[i]);
> +		if (IS_ERR(conn_state))
> +			return PTR_ERR(conn_state);
> +
> +		ret = drm_atomic_set_crtc_for_connector(conn_state,
> +							set->crtc);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -- 
> 2.1.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx