I will merge your changes for the next version, thanks zpos property is not a new property sti, exynos and r-car already used it. I know some not upstreamed code use it to manage zordering. For example I have wrote an Android hwcomposer using this property: https://git.linaro.org/people/benjamin.gaignard/hwcomposer.git/blob/HEAD:/hwcomposer.cpp#l591 I hope those patches could help to standardize zordering in userland. 2016-07-20 12:11 GMT+02:00 Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>: > On Thu, Jul 07, 2016 at 03:01:21PM +0200, Benjamin Gaignard wrote: >> From: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >> >> version 5: >> - remove zpos range check and comeback to 0 to N-1 >> normalization algorithm >> >> version 4: >> - make sure that normalized zpos value is stay >> in the defined property range and warn user if not >> >> This patch adds support for generic plane's zpos property property with >> well-defined semantics: >> - added zpos properties to plane 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@xxxxxxxxxxx> >> >> Compare to Marek's original patch zpos property is now specific to each >> plane and no more to the core. >> Normalize function take care of the range of per plane defined range >> before set normalized_zpos. >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx> >> >> Cc: Inki Dae <inki.dae@xxxxxxxxxxx> >> Cc: Daniel Vetter <daniel@xxxxxxxx> >> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> >> Cc: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx> >> Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx> >> Cc: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx> >> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> >> Cc: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx> >> Cc: Gustavo Padovan <gustavo@xxxxxxxxxxx> >> Cc: vincent.abriou@xxxxxx >> Cc: fabien.dessenne@xxxxxx >> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/Makefile | 2 +- >> drivers/gpu/drm/drm_atomic.c | 4 + >> drivers/gpu/drm/drm_atomic_helper.c | 6 + >> drivers/gpu/drm/drm_blend.c | 227 ++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/drm_crtc_internal.h | 4 + >> include/drm/drm_crtc.h | 30 +++++ >> 6 files changed, 272 insertions(+), 1 deletion(-) >> create mode 100644 drivers/gpu/drm/drm_blend.c >> >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >> index e3dba6f..7fbcf3f 100644 >> --- a/drivers/gpu/drm/Makefile >> +++ b/drivers/gpu/drm/Makefile >> @@ -2,7 +2,7 @@ >> # Makefile for the drm device driver. This driver provides support for the >> # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. >> >> -drm-y := drm_auth.o drm_bufs.o drm_cache.o \ >> +drm-y := drm_auth.o drm_bufs.o drm_blend.o drm_cache.o \ >> drm_context.o drm_dma.o \ >> drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \ >> drm_lock.o drm_memory.o drm_drv.o drm_vm.o \ >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >> index 3cee084..e503b8f 100644 >> --- a/drivers/gpu/drm/drm_atomic.c >> +++ b/drivers/gpu/drm/drm_atomic.c >> @@ -712,6 +712,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, >> state->src_h = val; >> } else if (property == config->rotation_property) { >> state->rotation = val; >> + } else if (property == plane->zpos_property) { >> + state->zpos = val; >> } else if (plane->funcs->atomic_set_property) { >> return plane->funcs->atomic_set_property(plane, state, >> property, val); >> @@ -768,6 +770,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, >> *val = state->src_h; >> } else if (property == config->rotation_property) { >> *val = state->rotation; >> + } else if (property == plane->zpos_property) { >> + *val = state->zpos; >> } else if (plane->funcs->atomic_get_property) { >> return plane->funcs->atomic_get_property(plane, state, property, val); >> } else { >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c >> index de7fddc..0694ae1 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c >> @@ -32,6 +32,8 @@ >> #include <drm/drm_atomic_helper.h> >> #include <linux/fence.h> >> >> +#include "drm_crtc_internal.h" >> + >> /** >> * DOC: overview >> * >> @@ -592,6 +594,10 @@ drm_atomic_helper_check_planes(struct drm_device *dev, >> struct drm_plane_state *plane_state; >> int i, ret = 0; >> >> + ret = drm_atomic_helper_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/drm_blend.c b/drivers/gpu/drm/drm_blend.c >> new file mode 100644 >> index 0000000..5ae6e0e >> --- /dev/null >> +++ b/drivers/gpu/drm/drm_blend.c >> @@ -0,0 +1,227 @@ >> +/* >> + * Copyright (C) 2016 Samsung Electronics Co.Ltd >> + * Authors: >> + * Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >> + * >> + * DRM core plane blending related functions >> + * >> + * Permission to use, copy, modify, distribute, and sell this software and its >> + * documentation for any purpose is hereby granted without fee, provided that >> + * the above copyright notice appear in all copies and that both that copyright >> + * notice and this permission notice appear in supporting documentation, and >> + * that the name of the copyright holders not be used in advertising or >> + * publicity pertaining to distribution of the software without specific, >> + * written prior permission. The copyright holders make no representations >> + * about the suitability of this software for any purpose. It is provided "as >> + * is" without express or implied warranty. >> + * >> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, >> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO >> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR >> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, >> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER >> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE >> + * OF THIS SOFTWARE. >> + */ >> +#include <drm/drmP.h> >> +#include <drm/drm_atomic.h> >> +#include <drm/drm_crtc.h> >> +#include <linux/export.h> >> +#include <linux/slab.h> >> +#include <linux/sort.h> >> + >> +#include "drm_internal.h" >> + >> +/** >> + * drm_plane_create_zpos_property - create mutable zpos property >> + * @plane: drm plane >> + * @min: minimal possible value of zpos property >> + * @max: maximal possible value of zpos property >> + * >> + * This function initializes generic mutable zpos property and enables support >> + * for it in drm core. Drivers can then attach this property to planes to enable >> + * support for configurable planes arrangement during blending operation. >> + * Once mutable zpos property has been enabled, the DRM core will automatically >> + * calculate drm_plane_state->normalized_zpos values. Usually min should be set >> + * to 0 and max to maximal number of planes for given crtc - 1. >> + * >> + * If zpos of some planes cannot be changed (like fixed background or >> + * cursor/topmost planes), driver should adjust min/max values and assign those >> + * planes immutable zpos property with lower or higher values (for more >> + * information, see drm_mode_create_zpos_immutable_property() function). In such >> + * case driver should also assign proper initial zpos values for all planes in >> + * its plane_reset() callback, so the planes will be always sorted properly. >> + * >> + * Returns: >> + * Zero on success, negative errno on failure. >> + */ >> +int drm_plane_create_zpos_property(struct drm_plane *plane, >> + unsigned int min, unsigned int max) > > I think we also want to pass the initial value here, and set the plane > state accordingly. > >> +{ >> + struct drm_property *prop; >> + >> + prop = drm_property_create_range(plane->dev, 0, "zpos", min, max); >> + if (!prop) >> + return -ENOMEM; >> + >> + drm_object_attach_property(&plane->base, prop, min); >> + >> + plane->zpos_property = prop; >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_plane_create_zpos_property); >> + >> +/** >> + * drm_plane_create_zpos_immutable_property - create immuttable zpos property >> + * @plane: drm plane >> + * @min: minimal possible value of zpos property >> + * @max: maximal possible value of zpos property >> + * >> + * This function initializes generic immutable zpos property and enables >> + * support for it in drm core. Using this property driver lets userspace >> + * to get the arrangement of the planes for blending operation and notifies >> + * it that the hardware (or driver) doesn't support changing of the planes' >> + * order. >> + * >> + * Returns: >> + * Zero on success, negative errno on failure. >> + */ >> +int drm_plane_create_zpos_immutable_property(struct drm_plane *plane, >> + unsigned int min, unsigned int max) > > What's the point of passing min/max here? I would think we'd just want > the one value. And the earlier comment about the plane state holds here > as well. > >> +{ >> + struct drm_property *prop; >> + >> + prop = drm_property_create_range(plane->dev, DRM_MODE_PROP_IMMUTABLE, >> + "zpos", min, max); >> + if (!prop) >> + return -ENOMEM; >> + >> + drm_object_attach_property(&plane->base, prop, min); >> + >> + plane->zpos_property = prop; >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_plane_create_zpos_immutable_property); >> + >> +static int drm_atomic_state_zpos_cmp(const void *a, const void *b) >> +{ >> + const struct drm_plane_state *sa = *(struct drm_plane_state **)a; >> + const struct drm_plane_state *sb = *(struct drm_plane_state **)b; >> + >> + if (sa->zpos != sb->zpos) >> + return sa->zpos - sb->zpos; >> + else >> + return sa->plane->base.id - sb->plane->base.id; >> +} >> + >> +/** >> + * drm_atomic_helper_crtc_normalize_zpos - calculate normalized zpos values >> + * @crtc: crtc with planes, which have to be considered for normalization >> + * @crtc_state: new atomic state to apply >> + * >> + * This function checks new states of all planes assigned to given crtc and >> + * calculates normalized zpos value for them. Planes are compared first by their >> + * zpos values, then by plane id (if zpos equals). Plane with lowest zpos value >> + * is at the bottom. The plane_state->normalized_zpos is then filled with unique >> + * values from 0 to number of active planes in crtc minus one. >> + * >> + * RETURNS >> + * Zero for success or -errno >> + */ >> +int drm_atomic_helper_crtc_normalize_zpos(struct drm_crtc *crtc, >> + struct drm_crtc_state *crtc_state) >> +{ >> + struct drm_atomic_state *state = crtc_state->state; >> + struct drm_device *dev = crtc->dev; >> + int total_planes = dev->mode_config.num_total_plane; >> + struct drm_plane_state **states; >> + struct drm_plane *plane; >> + int i, n = 0; >> + int ret = 0; >> + >> + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n", >> + crtc->base.id, crtc->name); >> + >> + states = kmalloc_array(total_planes, sizeof(*states), GFP_TEMPORARY); >> + if (!states) >> + return -ENOMEM; >> + >> + /* >> + * Normalization process might create new states for planes which >> + * normalized_zpos has to be recalculated. >> + */ >> + drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) { >> + struct drm_plane_state *plane_state = >> + drm_atomic_get_plane_state(state, plane); >> + if (IS_ERR(plane_state)) { >> + ret = PTR_ERR(plane_state); >> + goto done; >> + } >> + states[n++] = plane_state; >> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] processing zpos value %d\n", >> + plane->base.id, plane->name, >> + plane_state->zpos); >> + } >> + >> + sort(states, n, sizeof(*states), drm_atomic_state_zpos_cmp, NULL); >> + >> + for (i = 0; i < n; i++) { >> + plane = states[i]->plane; >> + >> + states[i]->normalized_zpos = i; >> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n", >> + plane->base.id, plane->name, i); >> + } >> + crtc_state->zpos_changed = true; > > This needs to be reset back in duplicate_state. > > I did those changes locally, and then I went ahead and implemented zpos > support for some Intel platforms, mainly to make sure this thing is good > enough for us. It doesn't result in the prettiest thing, but it does > appear to work at least: > git://github.com/vsyrjala/linux.git vlv_chv_zpos > > I still need to write some actual igt test for this stuff I think. And > then we have the "do we have actual userspace for this?" problem. Was > there an atomic weston branch somewhere that can use zpos? > >> + >> +done: >> + kfree(states); >> + return ret; >> +} >> +EXPORT_SYMBOL(drm_atomic_helper_crtc_normalize_zpos); >> + >> +/** >> + * drm_atomic_helper_normalize_zpos - calculate normalized zpos values for all >> + * crtcs >> + * @dev: DRM device >> + * @state: atomic state of DRM device >> + * >> + * This function calculates normalized zpos value for all modified planes in >> + * the provided atomic state of DRM device. For more information, see >> + * drm_atomic_helper_crtc_normalize_zpos() function. >> + * >> + * RETURNS >> + * Zero for success or -errno >> + */ >> +int drm_atomic_helper_normalize_zpos(struct drm_device *dev, >> + struct drm_atomic_state *state) >> +{ >> + struct drm_crtc *crtc; >> + struct drm_crtc_state *crtc_state; >> + struct drm_plane *plane; >> + struct drm_plane_state *plane_state; >> + int i, ret = 0; >> + >> + for_each_plane_in_state(state, plane, plane_state, i) { >> + crtc = plane_state->crtc; >> + if (!crtc) >> + continue; >> + if (plane->state->zpos != plane_state->zpos) { >> + crtc_state = >> + drm_atomic_get_existing_crtc_state(state, crtc); >> + crtc_state->zpos_changed = true; >> + } >> + } >> + >> + for_each_crtc_in_state(state, crtc, crtc_state, i) { >> + if (crtc_state->plane_mask != crtc->state->plane_mask || >> + crtc_state->zpos_changed) { >> + ret = drm_atomic_helper_crtc_normalize_zpos(crtc, >> + crtc_state); >> + if (ret) >> + return ret; >> + } >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_atomic_helper_normalize_zpos); >> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h >> index 47a500b..0c34e6d 100644 >> --- a/drivers/gpu/drm/drm_crtc_internal.h >> +++ b/drivers/gpu/drm/drm_crtc_internal.h >> @@ -128,3 +128,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, >> >> int drm_modeset_register_all(struct drm_device *dev); >> void drm_modeset_unregister_all(struct drm_device *dev); >> + >> +/* drm_blend.c */ >> +int drm_atomic_helper_normalize_zpos(struct drm_device *dev, >> + struct drm_atomic_state *state); >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >> index f5469d3..3936ddf 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -308,6 +308,7 @@ struct drm_plane_helper_funcs; >> * @mode_changed: crtc_state->mode or crtc_state->enable has been changed >> * @active_changed: crtc_state->active has been toggled. >> * @connectors_changed: connectors to this crtc have been updated >> + * @zpos_changed: zpos values of planes on this crtc have been updated >> * @color_mgmt_changed: color management properties have changed (degamma or >> * gamma LUT or CSC matrix) >> * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes >> @@ -344,6 +345,7 @@ struct drm_crtc_state { >> bool mode_changed : 1; >> bool active_changed : 1; >> bool connectors_changed : 1; >> + bool zpos_changed : 1; >> bool color_mgmt_changed : 1; >> >> /* attached planes bitmask: >> @@ -1396,6 +1398,9 @@ struct drm_connector { >> * @src_w: width of visible portion of plane (in 16.16) >> * @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) >> + * @normalized_zpos: normalized value of zpos: unique, range from 0 to N-1 >> + * for given crtc >> * @state: backpointer to global drm_atomic_state >> */ >> struct drm_plane_state { >> @@ -1416,6 +1421,10 @@ struct drm_plane_state { >> /* Plane rotation */ >> unsigned int rotation; >> >> + /* Plane zpos */ >> + unsigned int zpos; >> + unsigned int normalized_zpos; >> + >> struct drm_atomic_state *state; >> }; >> >> @@ -1675,6 +1684,7 @@ enum drm_plane_type { >> * @properties: property tracking for this plane >> * @type: type of plane (overlay, primary, cursor) >> * @state: current atomic state for this plane >> + * @zpos_property: zpos property for this plane >> * @helper_private: mid-layer private data >> */ >> struct drm_plane { >> @@ -1716,6 +1726,8 @@ struct drm_plane { >> const struct drm_plane_helper_funcs *helper_private; >> >> struct drm_plane_state *state; >> + >> + struct drm_property *zpos_property; >> }; >> >> /** >> @@ -2773,6 +2785,24 @@ extern void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, >> uint degamma_lut_size, >> bool has_ctm, >> uint gamma_lut_size); >> + >> +int drm_plane_atomic_set_zpos_property(struct drm_plane *plane, >> + struct drm_plane_state *state, >> + struct drm_property *property, >> + uint64_t val); >> + >> +int drm_plane_atomic_get_zpos_property(struct drm_plane *plane, >> + const struct drm_plane_state *state, >> + struct drm_property *property, >> + uint64_t *val); >> + >> +int drm_plane_create_zpos_property(struct drm_plane *plane, >> + unsigned int min, unsigned int max); >> + >> +int drm_plane_create_zpos_immutable_property(struct drm_plane *plane, >> + unsigned int min, >> + unsigned int max); >> + >> /* Helpers */ >> struct drm_mode_object *drm_mode_object_find(struct drm_device *dev, >> uint32_t id, uint32_t type); >> -- >> 1.9.1 > > -- > Ville Syrjälä > Intel OTC -- Benjamin Gaignard Graphic Study Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |