[PATCH] drm: Always prepare null framebuffer in transitional helper

[Daniel Vetter]

On Thu, Feb 16, 2017 at 02:44:42PM -0200, Gabriel Krisman Bertazi wrote:
> Despite the documentation claim that cleanup_fb will match prior calls
> to prepare_fb, in case of NULL framebuffers in the transitional helpers,
> the code will skip the call to prepare_fb but not the corresponding
> cleanup_fb call.  This asymmetry in semantics is unnecessarily surprising
> for developers transitioning drivers to atomic model, specially because
> the final atomic handlers don't have the issue - the prepare_fb is
> always called, despite the new state framebuffer being null.
> 
> The only current user of the transitional helper that doesn't take care
> of null framebuffers explicitly inside the prepare_fb hook is
> atmel_hlcdc, so we take special care to make sure we don't break
> anything there.

Oh dear, I noticed all the callers to drm_plane_helper_disable :( What
they really all want to do is shut down the entire display pipe on driver
unload, with drm_atomic_helper_disable_all().

A bit more work to do in a simple cleanup, but care to at least do a small
patch to Documentation/gpu/todo.rst to note it?

Thanks, Daniel

> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman at collabora.co.uk>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +++
>  drivers/gpu/drm/drm_plane_helper.c              | 3 +--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index bd2791c4b002..886ed5d8e304 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -772,6 +772,9 @@ static int atmel_hlcdc_plane_prepare_fb(struct drm_plane *p,
>  			drm_plane_state_to_atmel_hlcdc_plane_state(s);
>  	int ret;
>  
> +	if (!new_state->fb)
> +		return 0;
> +
>  	ret = atmel_hlcdc_layer_update_start(&plane->layer);
>  	if (!ret)
>  		state->prepared = true;
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index 148688fb920a..d8639e46bd2b 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -450,8 +450,7 @@ int drm_plane_helper_commit(struct drm_plane *plane,
>  			goto out;
>  	}
>  
> -	if (plane_funcs->prepare_fb && plane_state->fb &&
> -	    plane_state->fb != old_fb) {
> +	if (plane_funcs->prepare_fb && plane_state->fb != old_fb) {
>  		ret = plane_funcs->prepare_fb(plane,
>  					      plane_state);
>  		if (ret)
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch