[PATCH 05/14] drm/crtc_helper: Disable and reenable primary plane in drm_helper_crtc_mode_set

[Daniel Vetter]

On Tue, May 24, 2016 at 06:10:44PM +0800, Liu Ying wrote:
> Since CRTC has already been disabled in crtc_funcs->prepare(), it doesn't hurt
> to disable the primary plane in drm_helper_crtc_mode_set() before enabling it
> in drm_helper_crtc_mode_set_base().  This makes those who reject active plane
> update in plane_funcs->atomic_check() happy.
> 
> Signed-off-by: Liu Ying <gnuiyl at gmail.com>
> ---
>  drivers/gpu/drm/drm_crtc_helper.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 79555d2..7fabcd7 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -1013,6 +1013,15 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod
>  			goto out;
>  	}
>  
> +	/*
> +	 * It doesn't hurt to disable primary plane here since crtc is off
> +	 * and we'll enable it again in drm_helper_crtc_mode_set_base()
> +	 * below soon.
> +	 */
> +	ret = drm_plane_helper_disable(crtc->primary);
> +	if (ret)
> +		goto out;
> +

Ok, so there's no problem with mixing up legacy and transitional helpers
here. It's still not how it's supposed to work though. All the helpers
(legacy, transitional & atomic) assume that when the helpers call into
crtc->disable hooks, the driver will also disable all the scanout engines.
The helpers will _not_ do that for you, it's the driver duties.

For atomic we have a helper function which you can call at the right
place, drm_atomic_helper_disable_planes_on_crtc(). Unfortunately it looks
like no one ever used it, so probably a bunch of bugs in drivers because
of this. Explicitly disabling the plane when the crtc is shut down might
confused/break drivers which get this right, hence why we can't have this
in the helper libraries.

In short: You need to fix this in your driver, by manually disabling the
plane at a suitable place first. Most likely that's in the crtc->prepare
hook (since you're still transitioning).
-Daniel

>  	swap(crtc->state, crtc_state);
>  
>  	crtc_funcs->mode_set_nofb(crtc);
> -- 
> 2.7.4
> 
> _______________________________________________
> 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