Hi,
On 03/01/18 11:16, 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.
Enforce that all planes must have unique zpos by refusing atomic update for planes with already used zpos.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
Hi,
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_plane.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index 15e5d5d325c6..6aeda3cc2f8b 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -33,6 +33,7 @@ struct omap_plane { struct drm_plane base; enum omap_plane_id id;
- int idx; const char *name;
};
@@ -99,8 +100,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 = omap_plane->idx;
priv->dispc_ops->ovl_enable(omap_plane->id, false);
} @@ -109,15 +109,17 @@ static int omap_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { struct drm_crtc_state *crtc_state;
struct drm_crtc *crtc = state->crtc;
struct drm_plane *p;
if (!state->fb) return 0;
/* crtc should only be NULL when disabling (i.e., !state->fb) */
- if (WARN_ON(!state->crtc))
- if (WARN_ON(!crtc)) return 0;
- crtc_state = drm_atomic_get_existing_crtc_state(state->state, state->crtc);
- crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); /* we should have a crtc state if the plane is attached to a crtc */ if (WARN_ON(!crtc_state)) return 0;
@@ -125,6 +127,16 @@ static int omap_plane_atomic_check(struct drm_plane *plane, if (!crtc_state->enable) return 0;
- drm_for_each_plane_mask(p, crtc->dev, crtc->state->plane_mask) {
if (plane->base.id == p->base.id)continue;if (p->state->zpos == state->zpos) {DBG("zpos must be unique (zpos: %u)", state->zpos);return -EINVAL;}- }
I think this should be done in omap_drv, or maybe in omap_crtc. You don't want to check individual planes, but you want to ensure that none of the planes have conflicting zpos'es. Strictly speaking, zpos is not exactly a plane feature (even if it's set per-plane), it's more of a composition feature.
Perhaps omap_crtc would be the best place, as the zpos must be unique on a crtc, so in the crtc's atomic check you could check that all the planes on that crtc have unique zpos.
Tomi