[PATCH v2] drm: Don't force all planes to be added to the state due to zpos
Daniel Vetter
daniel at ffwll.ch
Mon Oct 10 15:31:34 UTC 2016
On Mon, Oct 10, 2016 at 05:50:56PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> We don't want all planes to be added to the state whenever a
> plane with fixed zpos gets enabled/disabled. This is true
> especially for eg. cursor planes on i915, as we want cursor
> updates to go through w/o throttling. Same holds for drivers
> that don't support zpos at all (i915 actually falls into this
> category right now since we've not yet added zpos support).
>
> Allow drivers more freedom by letting them deal with zpos
> themselves instead of doing it in drm_atomic_helper_check_planes()
> unconditionally. Let's just inline the required calls into all
> the driver that currently depend on this.
>
> v2: Inline the stuff into the drivers instead of adding another
> helper, document things better (Daniel)
>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Marek Szyprowski <m.szyprowski at samsung.com>
> Cc: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> Cc: Vincent Abriou <vincent.abriou at st.com>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Inki Dae <inki.dae at samsung.com>
> Cc: Joonyoung Shim <jy0922.shim at samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim at samsung.com>
> Cc: Kyungmin Park <kyungmin.park at samsung.com>
> Cc: Lyude <cpaul at redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: stable at vger.kernel.org
> Fixes: 44d1240d006c ("drm: add generic zpos property")
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 4 ----
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 20 ++++++++++++++++++++
> drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 +
> drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 12 ++++++++++--
> drivers/gpu/drm/sti/sti_drv.c | 22 +++++++++++++++++++++-
> include/drm/drm_plane.h | 8 +++++++-
> 7 files changed, 60 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index c3f83476f996..21f992605541 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -594,10 +594,6 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> struct drm_plane_state *plane_state;
> int i, ret = 0;
>
> - ret = drm_atomic_normalize_zpos(dev, state);
> - if (ret)
> - return ret;
> -
> for_each_plane_in_state(state, plane, plane_state, i) {
> const struct drm_plane_helper_funcs *funcs;
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index def78c8c1780..f86e7c846678 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -262,6 +262,26 @@ int exynos_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state,
> return 0;
> }
>
> +int exynos_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_normalize_zpos(dev, state);
> + if (ret)
> + return ret;
> +
> + ret = drm_atomic_helper_check_planes(dev, state);
> + if (ret)
> + return ret;
> +
> + return ret;
> +}
> +
> static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
> {
> struct drm_exynos_file_private *file_priv;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index d215149e737b..80c4d5b81689 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -301,6 +301,7 @@ static inline int exynos_dpi_bind(struct drm_device *dev,
>
> int exynos_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state,
> bool nonblock);
> +int exynos_atomic_check(struct drm_device *dev, struct drm_atomic_state *state);
>
>
> extern struct platform_driver fimd_driver;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index 40ce841eb952..23cce0a3f5fc 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -190,7 +190,7 @@ dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer *fb, int index)
> static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
> .fb_create = exynos_user_fb_create,
> .output_poll_changed = exynos_drm_output_poll_changed,
> - .atomic_check = drm_atomic_helper_check,
> + .atomic_check = exynos_atomic_check,
> .atomic_commit = exynos_atomic_commit,
> };
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index bd9c3bb9252c..392c7e6de042 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -231,8 +231,16 @@ static int rcar_du_atomic_check(struct drm_device *dev,
> struct rcar_du_device *rcdu = dev->dev_private;
> int ret;
>
> - ret = drm_atomic_helper_check(dev, state);
> - if (ret < 0)
> + ret = drm_atomic_helper_check_modeset(dev, state);
> + if (ret)
> + return ret;
> +
> + ret = drm_atomic_normalize_zpos(dev, state);
> + if (ret)
> + return ret;
> +
> + ret = drm_atomic_helper_check_planes(dev, state);
> + if (ret)
> return ret;
>
> if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index 2784919a7366..9df308565f6c 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -195,6 +195,26 @@ static void sti_atomic_work(struct work_struct *work)
> sti_atomic_complete(private, private->commit.state);
> }
>
> +static int sti_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_normalize_zpos(dev, state);
> + if (ret)
> + return ret;
> +
> + ret = drm_atomic_helper_check_planes(dev, state);
> + if (ret)
> + return ret;
> +
> + return ret;
> +}
> +
> static int sti_atomic_commit(struct drm_device *drm,
> struct drm_atomic_state *state, bool nonblock)
> {
> @@ -248,7 +268,7 @@ static void sti_output_poll_changed(struct drm_device *ddev)
> static const struct drm_mode_config_funcs sti_mode_config_funcs = {
> .fb_create = drm_fb_cma_create,
> .output_poll_changed = sti_output_poll_changed,
> - .atomic_check = drm_atomic_helper_check,
> + .atomic_check = sti_atomic_check,
> .atomic_commit = sti_atomic_commit,
> };
>
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 43cf193e54d6..8b4dc62470ff 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -47,8 +47,14 @@ struct drm_crtc;
> * @src_h: height of visible portion of plane (in 16.16)
> * @rotation: rotation of the plane
> * @zpos: priority of the given plane on crtc (optional)
> + * Note that multiple active planes on the same crtc can have an identical
> + * zpos value. The rule to solving the conflict is to compare the plane
> + * object IDs; the plane with a higher ID must be stacked on top of a
> + * plane with a lower ID.
> * @normalized_zpos: normalized value of zpos: unique, range from 0 to N-1
> - * where N is the number of active planes for given crtc
> + * where N is the number of active planes for given crtc. Note that
> + * the driver must call drm_atomic_normalize_zpos() to update this before
> + * it can be trusted.
Hm, for longer kerneldoc for struct members I prefer the in-line layout.
More readable overall, and better grouped. Either way:
Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
I'll pick it up once driver maintainers had a bit of time to ack it.
-Daniel
> * @src: clipped source coordinates of the plane (in 16.16)
> * @dst: clipped destination coordinates of the plane
> * @visible: visibility of the plane
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list