Re: [PATCH 5/6] drm/atomic: use connector references (v2)

From: Daniel Vetter <daniel@ffwll.ch>
To: Dave Airlie <airlied@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 5/6] drm/atomic: use connector references (v2)
Date: Tue, 3 May 2016 10:03:46 +0200	[thread overview]
Message-ID: <20160503080346.GD14148@phenom.ffwll.local> (raw)
In-Reply-To: <1462249706-4759-5-git-send-email-airlied@gmail.com>

On Tue, May 03, 2016 at 02:28:25PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> Take a reference when setting a crtc on a connecter,
> also take one when duplicating if a crtc is set,
> and drop one on destroy if a crtc is set.
> 
> v2: take Daniel Stone's advice and simplify the
> ref/unref dances, also take care of NULL as connector
> to state reset.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_atomic.c        | 4 ++++
>  drivers/gpu/drm/drm_atomic_helper.c | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 9d5e3c8..91cae88 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1160,6 +1160,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
>  {
>  	struct drm_crtc_state *crtc_state;
>  
> +	if (crtc)
> +		drm_connector_reference(conn_state->connector);
>  	if (conn_state->crtc && conn_state->crtc != crtc) {
>  		crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state,
>  								conn_state->crtc);
> @@ -1177,6 +1179,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
>  			1 << drm_connector_index(conn_state->connector);
>  	}
>  
> +	if (conn_state->crtc)
> +		drm_connector_unreference(conn_state->connector);
>  	conn_state->crtc = crtc;
>  
>  	if (crtc)
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index d25abce..f3e2642 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2762,6 +2762,8 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
>  					    struct drm_connector_state *state)
>  {
>  	memcpy(state, connector->state, sizeof(*state));
> +	if (state->crtc)
> +		drm_connector_reference(connector);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
>  
> @@ -2889,6 +2891,9 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>  	 * state will automatically do the right thing if code is ever added
>  	 * to this function.
>  	 */
> +	if (connector && state->crtc) {
> +		drm_connector_unreference(state->connector);
> +	}

Bikeshed: no need for {} and you don't need to check that connector is
NULL either. Tbh all the destroy_state helpers don't really need their
object argument, only the state. Cleaning that up is somewhere on my todo.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
>  
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx