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

[Daniel Vetter]

On Sun, Sep 13, 2015 at 11:33:26PM +0300, Laurent Pinchart wrote:
> 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.

I've pulled this one into drm-misc and I plan to send out a pull request
to Dave (now that -rc1 is out) next week sometimes. Then you can just
rebase the rcar parts on top of drm-next.

Thanks, Daniel

> 
> 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
> 

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