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
