Re: [PATCH 05/19] drm/atomic: Make add_affected_connectors look at crtc_state.

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

On Mon, Oct 17, 2016 at 02:37:04PM +0200, Maarten Lankhorst wrote:
> This kills another dereference of connector->state. connector_mask
> holds all unchanged connectors at least and any changed connectors
> are already in state anyway.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 120775fcfb70..1915ec44f7eb 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1230,8 +1230,13 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
>  	struct drm_mode_config *config = &state->dev->mode_config;
>  	struct drm_connector *connector;
>  	struct drm_connector_state *conn_state;
> +	struct drm_crtc_state *crtc_state;
>  	int ret;
>  
> +	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +	if (IS_ERR(crtc_state))
> +		return PTR_ERR(crtc_state);
> +
>  	ret = drm_modeset_lock(&config->connection_mutex, state->acquire_ctx);
>  	if (ret)
>  		return ret;
> @@ -1240,11 +1245,11 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
>  			 crtc->base.id, crtc->name, state);
>  
>  	/*
> -	 * Changed connectors are already in @state, so only need to look at the
> -	 * current configuration.
> +	 * Changed connectors are already in @state, so only need to look
> +	 * at the connector_mask in crtc_state.
>  	 */
>  	drm_for_each_connector(connector, state->dev) {
> -		if (connector->state->crtc != crtc)
> +		if (!(crtc_state->connector_mask & (1 << drm_connector_index(connector))))
>  			continue;

So this being the new crtc state, connector_mask will include all newly enabled
connectors (these will have been already added to the top level state), and
also any connector that was already enabled and isn't getting disabled (these
are the ones we need to explicitly add to the top level state here).

/me wishes the top level state would be renamed to drm_atomic_transaction or something...

Any connector that is getting disabled will already have been added to
the top level state as well, so them not being included in the new crtc
state's connectors_mask is fine.

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

>  
>  		conn_state = drm_atomic_get_connector_state(state, connector);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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