[PATCH v3 1/3] drm: add generic zpos property
Daniel Vetter
daniel at ffwll.ch
Fri Jan 15 02:12:25 PST 2016
On Fri, Jan 15, 2016 at 10:09:14AM +0100, Marek Szyprowski wrote:
> Hello,
>
> On 2016-01-14 11:46, Ville Syrjälä wrote:
> >On Tue, Jan 12, 2016 at 02:39:18PM +0100, Marek Szyprowski wrote:
> >>This patch adds support for generic plane's zpos property property with
> >>well-defined semantics:
> >>- added zpos properties to drm core and plane state structures
> >>- added helpers for normalizing zpos properties of given set of planes
> >>- well defined semantics: planes are sorted by zpos values and then plane
> >> id value if zpos equals
> >>
> >>Normalized zpos values are calculated automatically when generic
> >>muttable zpos property has been initialized. Drivers can simply use
> >>plane_state->normalized_zpos in their atomic_check and/or plane_update
> >>callbacks without any additional calls to DRM core.
> >>
> >>Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
> >>---
> >> Documentation/DocBook/gpu.tmpl | 14 ++++-
> >> drivers/gpu/drm/drm_atomic.c | 4 ++
> >> drivers/gpu/drm/drm_atomic_helper.c | 116 ++++++++++++++++++++++++++++++++++++
> >> drivers/gpu/drm/drm_crtc.c | 53 ++++++++++++++++
> >> include/drm/drm_crtc.h | 14 +++++
> >> 5 files changed, 199 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> >>index 6c6e81a9eaf4..f6b7236141b6 100644
> >>--- a/Documentation/DocBook/gpu.tmpl
> >>+++ b/Documentation/DocBook/gpu.tmpl
> >>@@ -2004,7 +2004,7 @@ void intel_crt_init(struct drm_device *dev)
> >> <td valign="top" >Description/Restrictions</td>
> >> </tr>
> >> <tr>
> >>- <td rowspan="37" valign="top" >DRM</td>
> >>+ <td rowspan="38" valign="top" >DRM</td>
> >> <td valign="top" >Generic</td>
> >> <td valign="top" >“rotation”</td>
> >> <td valign="top" >BITMASK</td>
> >>@@ -2256,7 +2256,7 @@ void intel_crt_init(struct drm_device *dev)
> >> <td valign="top" >property to suggest an Y offset for a connector</td>
> >> </tr>
> >> <tr>
> >>- <td rowspan="3" valign="top" >Optional</td>
> >>+ <td rowspan="4" valign="top" >Optional</td>
> >> <td valign="top" >“scaling mode”</td>
> >> <td valign="top" >ENUM</td>
> >> <td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
> >>@@ -2280,6 +2280,16 @@ void intel_crt_init(struct drm_device *dev)
> >> <td valign="top" >TBD</td>
> >> </tr>
> >> <tr>
> >>+ <td valign="top" > "zpos" </td>
> >>+ <td valign="top" >RANGE</td>
> >>+ <td valign="top" >Min=0, Max=255</td>
> >>+ <td valign="top" >Plane</td>
> >>+ <td valign="top" >Plane's 'z' position during blending (0 for background, 255 for frontmost).
> >>+ If two planes assigned to same CRTC have equal zpos values, the plane with higher plane
> >>+ id is treated as closer to front. Can be IMMUTABLE if driver doesn't support changing
> >>+ planes' order.</td>
> >I don't think this is going to work very well. Mixing the same range
> >of 0-255 for all planes, with potentially some of them being
> >IMMUTABLE for sure won't end well, or at least won't allow userspace to
> >see if there are any constraints between the zpos of the planes.
> >
> >So I think what we should do is let the driver specify the valid range,
> >and get rid of the obj id based conflict resolution in favor of just
> >rejecting conflicts outright. In cases where you can't move the planes
> >between crtcs, the driver ought to specify the range based on the
> >number of planes present on the crtc. If planes can be moved betweens
> >crtcs the range obviously needs to be larger to accomodate all the
> >possible planes on the crtc.
> >
> >Eg. on Intel VLV/CHV we could have the following setup:
> >primary zpos 0-2
> >sprite 0 zpos 0-2
> >sprite 1 zpos 0-2
> >cursor zpos 3
> >
> >That makes it very clear the curso is always on top, and the other
> >planes can be rearranged more or less freely. These planes can't be
> >moved between crtcs, so each there's an identical set of planes for
> >each crtc.
> >
> >On old Intel hw (gen2/3) we could have something like:
> >plane A zpos 0-3
> >plane B zpos 0-3
> >plane C zpos 0-3
> >overlay zpos 0-3
> >cursor B zpos 4
> >cursor A zpos 5
> >
> >Most of these planes can be moved between crtcs, and IIRC there
> >are probably more constraints on exactly how they can be stacked, but
> >this is at least fairly close to the truth. Again the cursors are always
> >on top, and in this case the order between the two cursor planes is also
> >fixed.
>
> I wasn't aware of a hardware, which has limited configuration of zpos only
> to some planes. I thought only about 2 cases: either completely configurable
> planes arrangement, or planes fixed to some hw dependent order. I see no
> problem to let drivers to define their own limits for mutable zpos property.
>
> Now the question is weather we should allow to set the non-zero minimal
> value
> for mutable zpos? I can imagine that there might be a hardware, which has
> fixed background plane and a few configurable overlay planes. I assume that
> in such case, the calculated normalized_zpos should still start from zero.
The usual approach is to switch the property from a global one in
dev->mode_config.*_prop to a per-object one in e.g. plane->zpos_prop. We
can still register the name, but have different limits/types. That way you
could register a global mutable zpos with the range 0-3 and an immutable
zpos with values 4 or 5 for cursors. The generic decoding would still be
fairly simple.
} else if (property == plane->zpos_property) {
state->zpos = val;
} else if (property == config->zpos_property) {
state->zpos = val;
Plus some checking that no one attached both the global and obj-local
property of the same name to the same object.
Since this is a fairly simple add-on change and current drivers that
support zpos don't need it I think it makes total sense to do this as a
follow-up when we need it.
But might be good to document this somewhere (either commit message or
kerneldoc for zpos).
> >I did originally have the same obj id based sorting idea (and even
> >posted some kind of a patch for it IIRC) but that was before atomic
> >existed, so there was a real need to allow reordering the planes with
> >just a single setplane ioctl call. With atomic I don't see any real
> >benefit from it the obj id based sorting, and as I've noted there are
> >definitely downsides to it.
>
> What are the downsides of using obj id as additional sorting criteria? It
> solves all the ambiguities and simplifies checks (there is no need to check
> if there are 2 planes of the same zpos value).
I think the sorting also has an upside that it avoids having to change all
the drivers, or adding some kind of flag. Drivers which don't support zpos
just get a sorted normlized zpos. If we don't do that we'd have to put
some unique default zpos in, which is going to make things messy.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list