Hi Peter,
Thank you for the patch.
On Tuesday, 9 January 2018 13:45:56 EET Peter Ujfalusi wrote:
> Use the plane index as default zpos for all planes. Even if the
> application is not setting zpos/zorder explicitly we will have unique zpos
> for each plane.
>
> Planes with identical zpos value will result undefined behavior:
> disappearing planes, screen flickering and it is not supported by the
> hardware.
>
> Enforce that all planes must have unique zpos on the given crtc by
> returning error when duplicate zpos value is requested.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> Hi,
>
> Changes since v4:
> - further simplify the zpos checking by using a mask and a single loop
> - Commit message has been extended
>
> Changes since v3:
> - Use drm_plane_index() instead of storing the same index wothin omap_plane
> struct
> - Optimize the zpos validation loop so we avoid extra checks.
>
> Changes since v2:
> - The check for duplicate zpos is moved to omap_crtc
>
> Changes since v1:
> - Dropped the zpos normalization related patches
> - New patch based on the discussion, see commit message.
>
> Regards,
> Peter
>
> drivers/gpu/drm/omapdrm/omap_crtc.c | 23 ++++++++++++++++++++++-
> drivers/gpu/drm/omapdrm/omap_plane.c | 15 ++++-----------
> 2 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 1b8154e58d18..941a6440fc8e
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -486,6 +486,27 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc
> *crtc) }
> }
>
> +static int omap_crtc_validate_zpos(struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> +{
> + struct drm_plane *plane;
> + const struct drm_plane_state *pstate;
> + unsigned int zpos_mask = 0;
> +
> + /* Check the crts's planes against duplicated zpos value */
> + drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> + if (zpos_mask & BIT(pstate->zpos)) {
> + DBG("crtc%u: zpos must be unique (zpos: %u)",
> + crtc->index, pstate->zpos);
> + return -EINVAL;
> + }
> +
> + zpos_mask |= BIT(pstate->zpos);
> + }
> +
> + return 0;
> +}
> +
> static int omap_crtc_atomic_check(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> @@ -509,7 +530,7 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc,
> omap_crtc_state->rotation = pri_state->rotation;
> }
>
> - return 0;
> + return omap_crtc_validate_zpos(crtc, state);
> }
>
> static void omap_crtc_atomic_begin(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index 7d789d1551a1..39f25210bef1
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -97,8 +97,7 @@ static void omap_plane_atomic_disable(struct drm_plane
> *plane, struct omap_plane *omap_plane = to_omap_plane(plane);
>
> plane->state->rotation = DRM_MODE_ROTATE_0;
> - plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
> - ? 0 : omap_plane->id;
> + plane->state->zpos = drm_plane_index(plane);
>
> priv->dispc_ops->ovl_enable(omap_plane->id, false);
> }
> @@ -184,18 +183,12 @@ void omap_plane_install_properties(struct drm_plane
> *plane,
>
> static void omap_plane_reset(struct drm_plane *plane)
> {
> - struct omap_plane *omap_plane = to_omap_plane(plane);
> -
> drm_atomic_helper_plane_reset(plane);
> if (!plane->state)
> return;
>
> - /*
> - * Set the zpos default depending on whether we are a primary or overlay
> - * plane.
> - */
> - plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
> - ? 0 : omap_plane->id;
> + /* Set the zpos to the plane index. */
> + plane->state->zpos = drm_plane_index(plane);
> }
>
> static int omap_plane_atomic_set_property(struct drm_plane *plane,
> @@ -295,7 +288,7 @@ struct drm_plane *omap_plane_init(struct drm_device
> *dev, drm_plane_helper_add(plane, &omap_plane_helper_funcs);
>
> omap_plane_install_properties(plane, &plane->base);
> - drm_plane_create_zpos_property(plane, 0, 0, num_planes - 1);
> + drm_plane_create_zpos_property(plane, idx, 0, num_planes - 1);
>
> return plane;
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
