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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
