[PATCH] drm/atomic-helper: Don't skip plane disabling on active CRTC

[Laurent Pinchart]

Hi Daniel,

I've successfully tested the following patch with the rcar-du-drm driver. I'm 
not sure who else passes a true active_only argument to 
drm_atomic_helper_commit_planes() out-of-tree so I don't know who you would 
like to received a Tested-by tag from, but I'll need both your base patch and 
this fix for my next rcar-du-drm pull request.

On Friday 11 September 2015 00:07:19 Laurent Pinchart wrote:
> Since commit "drm/atomic-helper: Add option to update planes only on
> active crtc" the drm_atomic_helper_commit_planes() function accepts an
> active_only argument to skip updating planes when the associated CRTC is
> inactive. Planes being disabled on an active CRTC are incorrectly
> considered as associated with an inactive CRTC and are thus skipped,
> preventing any plane disabling update from reaching drivers.
> 
> Fix it by checking the state of the CRTC stored in the old plane state
> for planes being disabled.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> Hi Daniel,
> 
> This fixes the patch mentioned in the commit message. Please feel free to
> squash the fix into the original patch if it's not too late, or take it in
> your branch otherwise (in which case a Fixes: line might be appropriate).
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index a067ddb1b443..0aa19fa0a5d5
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1201,23 +1201,35 @@ void drm_atomic_helper_commit_planes(struct
> drm_device *dev,
> 
>  	for_each_plane_in_state(old_state, plane, old_plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> +		bool disabling;
> 
>  		funcs = plane->helper_private;
> 
>  		if (!funcs)
>  			continue;
> 
> -		if (active_only && !plane_crtc_active(plane->state))
> -			continue;
> +		disabling = drm_atomic_plane_disabling(plane, old_plane_state);
> +
> +		if (active_only) {
> +			/*
> +			 * Skip planes related to inactive CRTCs. If the plane
> +			 * is enabled use the state of the current CRTC. If the
> +			 * plane is being disabled use the state of the old
> +			 * CRTC to avoid skipping planes being disabled on an
> +			 * active CRTC.
> +			 */
> +			if (!disabling && !plane_crtc_active(plane->state))
> +				continue;
> +			if (disabling && !plane_crtc_active(old_plane_state))
> +				continue;
> +		}
> 
>  		/*
>  		 * Special-case disabling the plane if drivers support it.
>  		 */
> -		if (drm_atomic_plane_disabling(plane, old_plane_state) &&
> -		    funcs->atomic_disable)
> +		if (disabling && funcs->atomic_disable)
>  			funcs->atomic_disable(plane, old_plane_state);
> -		else if (plane->state->crtc ||
> -			 drm_atomic_plane_disabling(plane, old_plane_state))
> +		else if (plane->state->crtc || disabling)
>  			funcs->atomic_update(plane, old_plane_state);
>  	}

-- 
Regards,

Laurent Pinchart