Re: [PATCH] drm/atomic: Don't set crtc_state->enable manually

From: Daniel Vetter <daniel@ffwll.ch>
To: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Daniel Stone <daniels@collabora.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/atomic: Don't set crtc_state->enable manually
Date: Fri, 19 Jun 2015 18:01:35 +0200	[thread overview]
Message-ID: <20150619160135.GH25769@phenom.ffwll.local> (raw)
In-Reply-To: <1434721315-4022-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

On Fri, Jun 19, 2015 at 04:41:55PM +0300, Laurent Pinchart wrote:
> The enable field needs to be kept in sync with the mode_blob field. Call
> drm_atomic_set_mode_prop_for_crtc() instead of setting enable to false
> in order to dereference the mode blob correctly.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Another option would be to open-code the mode blob unreference in
> update_output_state(). I'm not sure what's best.
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 536ae4da4665..64c75af1c088 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1554,6 +1554,8 @@ static int update_output_state(struct drm_atomic_state *state,
>  	}
>  
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		unsigned int num_connectors;
> +
>  		/* 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
> @@ -1561,10 +1563,12 @@ static int update_output_state(struct drm_atomic_state *state,
>  		if (crtc == set->crtc)
>  			continue;
>  
> -		crtc_state->enable =
> -			drm_atomic_connectors_for_crtc(state, crtc);
> -		if (!crtc_state->enable)
> +		num_connectors = drm_atomic_connectors_for_crtc(state, crtc);
> +		if (!num_connectors) {
> +			ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
> +								NULL);

ret seems lost here because of the return 0; below.
-Daniel

>  			crtc_state->active = false;
> +		}
>  	}
>  
>  	return 0;
> -- 
> Regards,
> 
> Laurent Pinchart
> 

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