On Fri, Aug 19, 2016 at 05:36:59PM +0800, Liu Ying wrote:
> We don't support configuring active plane on-the-fly for imx-drm.
> The relevant CRTC should be disabled before the plane configuration.
> Of course, the plane itself should be disabled as well.
>
> This patch adds active plane reconfiguration support by forcing CRTC
> mode change in plane's ->atomic_check callback and adding disable
> operation for all appropriate active planes(when necessary) in CRTC's
> ->atomic_disable callback. The atomic core would reenabled the
> affected CRTC and planes at atomic commit stage.
>
> Suggested-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Philipp Zabel <p.zabel at pengutronix.de>
> Cc: David Airlie <airlied at linux.ie>
> Cc: Russell King <linux at armlinux.org.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Peter Senna Tschudin <peter.senna at gmail.com>
> Cc: Lucas Stach <l.stach at pengutronix.de>
> Signed-off-by: Liu Ying <gnuiyl at gmail.com>
> ---
> v2->v3:
> * Disable all appropriate affected planes(when necessary) in CRTC's
> ->atomic_disable callback, but not in each plane's ->atomic_update callback,
> as suggested by Daniel Vetter.
> * +Cc Lucas Stach, as he tested the patch v2.
>
> v1->v2:
> * Do not reject reconfiguring an active overlay plane.
>
> drivers/gpu/drm/imx/imx-drm-core.c | 26 +++++++++++++++++++++++++-
> drivers/gpu/drm/imx/ipuv3-crtc.c | 26 ++++++++++++++++++++++++++
> drivers/gpu/drm/imx/ipuv3-plane.c | 21 ++++++++++++++-------
> 3 files changed, 65 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c
> b/drivers/gpu/drm/imx/imx-drm-core.c
> index 438bac8..d61b048 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -146,10 +146,34 @@ static void imx_drm_output_poll_changed(struct
> drm_device *drm)
> drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
> }
>
> +static int imx_drm_atomic_check(struct drm_device *dev,
> + struct drm_atomic_state *state)
> +{
> + int ret;
> +
> + ret = drm_atomic_helper_check_modeset(dev, state);
> + if (ret)
> + return ret;
> +
> + ret = drm_atomic_helper_check_planes(dev, state);
> + if (ret)
> + return ret;
> +
> + /*
> + * Check modeset again in case crtc_state->mode_changed is
> + * updated in plane's ->atomic_check callback.
> + */
> + ret = drm_atomic_helper_check_modeset(dev, state);
> + if (ret)
> + return ret;
> +
> + return ret;
> +}
> +
> static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
> .fb_create = drm_fb_cma_create,
> .output_poll_changed = imx_drm_output_poll_changed,
> - .atomic_check = drm_atomic_helper_check,
> + .atomic_check = imx_drm_atomic_check,
> .atomic_commit = drm_atomic_helper_commit,
> };
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c
> b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 83c46bd..0779eed 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -76,6 +76,32 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
> crtc->state->event = NULL;
> }
> spin_unlock_irq(&crtc->dev->event_lock);
> +
> + /*
> + * The relevant plane's ->atomic_check callback may set
> + * crtc_state->mode_changed to be true when the active
> + * plane needs to be reconfigured. In this case and only
> + * this case, active_changed is false - we disable all the
> + * appropriate active planes here.
> + */
> + if (!crtc->state->active_changed) {
> + struct drm_plane *plane;
> +
> + drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) {
> + const struct drm_plane_helper_funcs *plane_funcs =
> + plane->helper_private;
> +
> + /*
> + * Filter out the plane which is explicitly required
> + * to be disabled by the user via atomic commit
> + * so that it won't be accidentally disabled twice.
> + */
> + if (!plane->state->crtc)
> + continue;
I think the better place would be to do the filtering in commit_planes(),
not here. And would be great to include your patch to fix up
disable_planes_on_crtc(), too.
-Daniel
> +
> + plane_funcs->atomic_disable(plane, NULL);
> + }
> + }
> }
>
> static void imx_drm_crtc_reset(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
> b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 4ad67d0..6063ebe 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -319,13 +319,16 @@ static int ipu_plane_atomic_check(struct drm_plane
> *plane,
> return -EINVAL;
>
> /*
> - * since we cannot touch active IDMAC channels, we do not support
> - * resizing the enabled plane or changing its format
> + * We support resizing active plane or changing its format by
> + * forcing CRTC mode change in plane's ->atomic_check callback
> + * and disabling all appropriate active planes in CRTC's
> + * ->atomic_disable callback. The planes will be reenabled in
> + * plane's ->atomic_update callback.
> */
> if (old_fb && (state->src_w != old_state->src_w ||
> state->src_h != old_state->src_h ||
> fb->pixel_format != old_fb->pixel_format))
> - return -EINVAL;
> + crtc_state->mode_changed = true;
>
> eba = drm_plane_state_to_eba(state);
>
> @@ -336,7 +339,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
> return -EINVAL;
>
> if (old_fb && fb->pitches[0] != old_fb->pitches[0])
> - return -EINVAL;
> + crtc_state->mode_changed = true;
>
> switch (fb->pixel_format) {
> case DRM_FORMAT_YUV420:
> @@ -372,7 +375,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
> return -EINVAL;
>
> if (old_fb && old_fb->pitches[1] != fb->pitches[1])
> - return -EINVAL;
> + crtc_state->mode_changed = true;
> }
>
> return 0;
> @@ -392,8 +395,12 @@ static void ipu_plane_atomic_update(struct drm_plane
> *plane,
> enum ipu_color_space ics;
>
> if (old_state->fb) {
> - ipu_plane_atomic_set_base(ipu_plane, old_state);
> - return;
> + struct drm_crtc_state *crtc_state = state->crtc->state;
> +
> + if (!crtc_state->mode_changed) {
> + ipu_plane_atomic_set_base(ipu_plane, old_state);
> + return;
> + }
> }
>
> switch (ipu_plane->dp_flow) {
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
