Re: [Intel-gfx] [PATCH v3 6/7] drm/atomic: Clean up steal_encoder, v2.

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

On Thu, Mar 03, 2016 at 10:17:41AM +0100, Maarten Lankhorst wrote:
> Now that only encoders can be stolen that are part of the state
> steal_encoder no longer needs to inspect all connectors,
> just those that are part of the atomic state.
> 
> Changes since v1:
> - Change return value to void, can no longer fail.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 25 +++++--------------------
>  1 file changed, 5 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index bb60148c5c8d..2395201eb7ab 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -227,25 +227,18 @@ set_best_encoder(struct drm_atomic_state *state,
>  	conn_state->best_encoder = encoder;
>  }
>  
> -static int
> +static void
>  steal_encoder(struct drm_atomic_state *state,
>  	      struct drm_encoder *encoder)
>  {
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_connector *connector;
>  	struct drm_connector_state *connector_state;
> +	int i;
>  
> -	drm_for_each_connector(connector, state->dev) {
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		struct drm_crtc *encoder_crtc;
>  
> -		if (connector->state->best_encoder != encoder)
> -			continue;
> -
> -		connector_state = drm_atomic_get_connector_state(state,
> -								 connector);
> -		if (IS_ERR(connector_state))
> -			return PTR_ERR(connector_state);
> -
>  		if (connector_state->best_encoder != encoder)
>  			continue;
>  
> @@ -260,10 +253,8 @@ steal_encoder(struct drm_atomic_state *state,
>  		crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
>  		crtc_state->connectors_changed = true;
>  
> -		return 0;
> +		return;

I was wondering if we should add some kind of sanity check here (or
maybe better do it afterwards across the entire device state?) to
make sure the same encoder didn't end up used multiple times?

Anyway, patch is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	}
> -
> -	return 0;
>  }
>  
>  static int
> @@ -343,13 +334,7 @@ update_connector_routing(struct drm_atomic_state *state,
>  		return 0;
>  	}
>  
> -	ret = steal_encoder(state, new_encoder);
> -	if (ret) {
> -		DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
> -				 connector->base.id,
> -				 connector->name);
> -		return ret;
> -	}
> +	steal_encoder(state, new_encoder);
>  
>  	if (WARN_ON(!connector_state->crtc))
>  		return -EINVAL;
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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