[PATCH v5 0/4] Generic zpos property

[PATCH v5 0/4] Generic zpos property

[Benjamin Gaignard]

version 5:
rebased on drm-next where Documentation/DocBook/gpu.tmpl doesn't
exist anymore.
rework sti patch because some plane functions have changed since v4

version 4:
make sure that normalized zpos value is stay in the defined property 
range and warn user if not.
Fix NULL pointer bug in rcar-du while setting zpos value.
No changes in the other drivers.

version 3:
use kmalloc_array instead of kmalloc.
Correct normalize_zpos computation (comeback to Mareck original code)

version 2:
add a zpos property into drm_plane structure to simplify code.
This allow to get/set zpos value in core and not in drivers code.
Fix various remarks.

version 1:
refactor Marek's patches to have per plane zpos property instead of only
one in core.

Benjamin Gaignard (2):
  drm: sti: use generic zpos for plane
  drm: rcar: use generic code for managing zpos plane property

Marek Szyprowski (2):
  drm: add generic zpos property
  drm/exynos: use generic code for managing zpos plane property

 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 +
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |   2 -
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  67 ++-------
 drivers/gpu/drm/exynos/exynos_mixer.c     |   6 +-
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c    |   2 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h     |   1 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c     |   5 -
 drivers/gpu/drm/rcar-du/rcar_du_plane.c   |   9 +-
 drivers/gpu/drm/rcar-du/rcar_du_plane.h   |   2 -
 drivers/gpu/drm/sti/sti_cursor.c          |   4 +-
 drivers/gpu/drm/sti/sti_gdp.c             |   4 +-
 drivers/gpu/drm/sti/sti_hqvdp.c           |   4 +-
 drivers/gpu/drm/sti/sti_mixer.c           |   9 +-
 drivers/gpu/drm/sti/sti_plane.c           |  76 ++++------
 drivers/gpu/drm/sti/sti_plane.h           |   7 +-
 include/drm/drm_crtc.h                    |  30 ++++
 20 files changed, 324 insertions(+), 147 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_blend.c

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: vincent.abriou@st.com
Cc: fabien.dessenne@st.com
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 1/4] drm: add generic zpos property

[Benjamin Gaignard]

From: Marek Szyprowski <m.szyprowski@samsung.com>

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@samsung.com>

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@linaro.org>

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: vincent.abriou@st.com
Cc: fabien.dessenne@st.com
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 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@samsung.com>
+ *
+ * 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)
+{
+	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)
+{
+	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;
+
+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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/4] drm: sti: use generic zpos for plane

[Benjamin Gaignard]

remove private zpos property and use instead the generic new.
zpos range is now fixed per plane type and normalized before
being using in mixer.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: vincent.abriou@st.com
Cc: fabien.dessenne@st.com
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/sti/sti_cursor.c |  4 +--
 drivers/gpu/drm/sti/sti_gdp.c    |  4 +--
 drivers/gpu/drm/sti/sti_hqvdp.c  |  4 +--
 drivers/gpu/drm/sti/sti_mixer.c  |  9 ++---
 drivers/gpu/drm/sti/sti_plane.c  | 76 ++++++++++++++--------------------------
 drivers/gpu/drm/sti/sti_plane.h  |  7 +---
 6 files changed, 36 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index a263bbb..3b53f7f 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -349,8 +349,8 @@ struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = sti_cursor_destroy,
-	.set_property = sti_plane_set_property,
-	.reset = drm_atomic_helper_plane_reset,
+	.set_property = drm_atomic_helper_plane_set_property,
+	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 	.late_register = sti_cursor_late_register,
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index bf63086..b8d942c 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -886,8 +886,8 @@ struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = sti_gdp_destroy,
-	.set_property = sti_plane_set_property,
-	.reset = drm_atomic_helper_plane_reset,
+	.set_property = drm_atomic_helper_plane_set_property,
+	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 	.late_register = sti_gdp_late_register,
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index 33d2f42..5a435c1 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1254,8 +1254,8 @@ struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = sti_hqvdp_destroy,
-	.set_property = sti_plane_set_property,
-	.reset = drm_atomic_helper_plane_reset,
+	.set_property = drm_atomic_helper_plane_set_property,
+	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 	.late_register = sti_hqvdp_late_register,
diff --git a/drivers/gpu/drm/sti/sti_mixer.c b/drivers/gpu/drm/sti/sti_mixer.c
index 1885c7a..7d9aea8 100644
--- a/drivers/gpu/drm/sti/sti_mixer.c
+++ b/drivers/gpu/drm/sti/sti_mixer.c
@@ -239,13 +239,10 @@ static void sti_mixer_set_background_area(struct sti_mixer *mixer,
 
 int sti_mixer_set_plane_depth(struct sti_mixer *mixer, struct sti_plane *plane)
 {
-	int plane_id, depth = plane->zorder;
+	int plane_id, depth = plane->drm_plane.state->normalized_zpos;
 	unsigned int i;
 	u32 mask, val;
 
-	if ((depth < 1) || (depth > GAM_MIXER_NB_DEPTH_LEVEL))
-		return 1;
-
 	switch (plane->desc) {
 	case STI_GDP_0:
 		plane_id = GAM_DEPTH_GDP0_ID;
@@ -278,8 +275,8 @@ int sti_mixer_set_plane_depth(struct sti_mixer *mixer, struct sti_plane *plane)
 			break;
 	}
 
-	mask |= GAM_DEPTH_MASK_ID << (3 * (depth - 1));
-	plane_id = plane_id << (3 * (depth - 1));
+	mask |= GAM_DEPTH_MASK_ID << (3 * depth);
+	plane_id = plane_id << (3 * depth);
 
 	DRM_DEBUG_DRIVER("%s %s depth=%d\n", sti_mixer_to_str(mixer),
 			 sti_plane_to_str(plane), depth);
diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index 0cf3335..6472333 100644
--- a/drivers/gpu/drm/sti/sti_plane.c
+++ b/drivers/gpu/drm/sti/sti_plane.c
@@ -14,15 +14,6 @@
 #include "sti_drv.h"
 #include "sti_plane.h"
 
-/* (Background) < GDP0 < GDP1 < HQVDP0 < GDP2 < GDP3 < (ForeGround) */
-enum sti_plane_desc sti_plane_default_zorder[] = {
-	STI_GDP_0,
-	STI_GDP_1,
-	STI_HQVDP_0,
-	STI_GDP_2,
-	STI_GDP_3,
-};
-
 const char *sti_plane_to_str(struct sti_plane *plane)
 {
 	switch (plane->desc) {
@@ -96,59 +87,44 @@ void sti_plane_update_fps(struct sti_plane *plane,
 			 plane->fps_info.fips_str);
 }
 
-int sti_plane_set_property(struct drm_plane *drm_plane,
-			   struct drm_property *property,
-			   uint64_t val)
+static int sti_plane_get_default_zpos(enum drm_plane_type type)
 {
-	struct drm_device *dev = drm_plane->dev;
-	struct sti_private *private = dev->dev_private;
-	struct sti_plane *plane = to_sti_plane(drm_plane);
-
-	DRM_DEBUG_DRIVER("\n");
-
-	if (property == private->plane_zorder_property) {
-		plane->zorder = val;
+	switch (type) {
+	case DRM_PLANE_TYPE_PRIMARY:
 		return 0;
+	case DRM_PLANE_TYPE_OVERLAY:
+		return 1;
+	case DRM_PLANE_TYPE_CURSOR:
+		return 7;
 	}
+	return 0;
+}
 
-	return -EINVAL;
+void sti_plane_reset(struct drm_plane *plane)
+{
+	drm_atomic_helper_plane_reset(plane);
+	plane->state->zpos = sti_plane_get_default_zpos(plane->type);
 }
 
-static void sti_plane_attach_zorder_property(struct drm_plane *drm_plane)
+static void sti_plane_attach_zorder_property(struct drm_plane *drm_plane,
+					     enum drm_plane_type type)
 {
-	struct drm_device *dev = drm_plane->dev;
-	struct sti_private *private = dev->dev_private;
-	struct sti_plane *plane = to_sti_plane(drm_plane);
-	struct drm_property *prop;
-
-	prop = private->plane_zorder_property;
-	if (!prop) {
-		prop = drm_property_create_range(dev, 0, "zpos", 1,
-						 GAM_MIXER_NB_DEPTH_LEVEL);
-		if (!prop)
-			return;
-
-		private->plane_zorder_property = prop;
+	switch (type) {
+	case DRM_PLANE_TYPE_PRIMARY:
+	case DRM_PLANE_TYPE_OVERLAY:
+		drm_plane_create_zpos_property(drm_plane, 0, 6);
+		break;
+	case DRM_PLANE_TYPE_CURSOR:
+		drm_plane_create_zpos_immutable_property(drm_plane, 7, 7);
+		break;
 	}
-
-	drm_object_attach_property(&drm_plane->base, prop, plane->zorder);
 }
 
 void sti_plane_init_property(struct sti_plane *plane,
 			     enum drm_plane_type type)
 {
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(sti_plane_default_zorder); i++)
-		if (sti_plane_default_zorder[i] == plane->desc)
-			break;
-
-	plane->zorder = i + 1;
-
-	if (type == DRM_PLANE_TYPE_OVERLAY)
-		sti_plane_attach_zorder_property(&plane->drm_plane);
+	sti_plane_attach_zorder_property(&plane->drm_plane, type);
 
-	DRM_DEBUG_DRIVER("drm plane:%d mapped to %s with zorder:%d\n",
-			 plane->drm_plane.base.id,
-			 sti_plane_to_str(plane), plane->zorder);
+	DRM_DEBUG_DRIVER("drm plane:%d mapped to %s \n",
+			 plane->drm_plane.base.id, sti_plane_to_str(plane));
 }
diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
index e0ea1dd..ce3e8d6 100644
--- a/drivers/gpu/drm/sti/sti_plane.h
+++ b/drivers/gpu/drm/sti/sti_plane.h
@@ -66,14 +66,12 @@ struct sti_fps_info {
  * @plane:              drm plane it is bound to (if any)
  * @desc:               plane type & id
  * @status:             to know the status of the plane
- * @zorder:             plane z-order
  * @fps_info:           frame per second info
  */
 struct sti_plane {
 	struct drm_plane drm_plane;
 	enum sti_plane_desc desc;
 	enum sti_plane_status status;
-	int zorder;
 	struct sti_fps_info fps_info;
 };
 
@@ -82,10 +80,7 @@ void sti_plane_update_fps(struct sti_plane *plane,
 			  bool new_frame,
 			  bool new_field);
 
-int sti_plane_set_property(struct drm_plane *drm_plane,
-			   struct drm_property *property,
-			   uint64_t val);
-
 void sti_plane_init_property(struct sti_plane *plane,
 			     enum drm_plane_type type);
+void sti_plane_reset(struct drm_plane *plane);
 #endif
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 3/4] drm/exynos: use generic code for managing zpos plane property

[Benjamin Gaignard]

From: Marek Szyprowski <m.szyprowski@samsung.com>

This patch replaces zpos property handling custom code in Exynos DRM
driver with calls to generic DRM code.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: vincent.abriou@st.com
Cc: fabien.dessenne@st.com
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  2 -
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 67 +++++--------------------------
 drivers/gpu/drm/exynos/exynos_mixer.c     |  6 ++-
 3 files changed, 13 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index cc33ec9..b34a7b9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -64,7 +64,6 @@ struct exynos_drm_plane_state {
 	struct exynos_drm_rect src;
 	unsigned int h_ratio;
 	unsigned int v_ratio;
-	unsigned int zpos;
 };
 
 static inline struct exynos_drm_plane_state *
@@ -221,7 +220,6 @@ struct exynos_drm_private {
 	 * this array is used to be aware of which crtc did it request vblank.
 	 */
 	struct drm_crtc *crtc[MAX_CRTC];
-	struct drm_property *plane_zpos_property;
 
 	struct device *dma_dev;
 	unsigned long da_start;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 77f12c0..915e804 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -139,9 +139,9 @@ static void exynos_drm_plane_reset(struct drm_plane *plane)
 
 	exynos_state = kzalloc(sizeof(*exynos_state), GFP_KERNEL);
 	if (exynos_state) {
-		exynos_state->zpos = exynos_plane->config->zpos;
 		plane->state = &exynos_state->base;
 		plane->state->plane = plane;
+		plane->state->zpos = exynos_plane->config->zpos;
 	}
 }
 
@@ -157,7 +157,6 @@ exynos_drm_plane_duplicate_state(struct drm_plane *plane)
 		return NULL;
 
 	__drm_atomic_helper_plane_duplicate_state(plane, &copy->base);
-	copy->zpos = exynos_state->zpos;
 	return &copy->base;
 }
 
@@ -170,43 +169,6 @@ static void exynos_drm_plane_destroy_state(struct drm_plane *plane,
 	kfree(old_exynos_state);
 }
 
-static int exynos_drm_plane_atomic_set_property(struct drm_plane *plane,
-						struct drm_plane_state *state,
-						struct drm_property *property,
-						uint64_t val)
-{
-	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
-	struct exynos_drm_plane_state *exynos_state =
-					to_exynos_plane_state(state);
-	struct exynos_drm_private *dev_priv = plane->dev->dev_private;
-	const struct exynos_drm_plane_config *config = exynos_plane->config;
-
-	if (property == dev_priv->plane_zpos_property &&
-	    (config->capabilities & EXYNOS_DRM_PLANE_CAP_ZPOS))
-		exynos_state->zpos = val;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
-static int exynos_drm_plane_atomic_get_property(struct drm_plane *plane,
-					  const struct drm_plane_state *state,
-					  struct drm_property *property,
-					  uint64_t *val)
-{
-	const struct exynos_drm_plane_state *exynos_state =
-		container_of(state, const struct exynos_drm_plane_state, base);
-	struct exynos_drm_private *dev_priv = plane->dev->dev_private;
-
-	if (property == dev_priv->plane_zpos_property)
-		*val = exynos_state->zpos;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
 static struct drm_plane_funcs exynos_plane_funcs = {
 	.update_plane	= drm_atomic_helper_update_plane,
 	.disable_plane	= drm_atomic_helper_disable_plane,
@@ -215,8 +177,6 @@ static struct drm_plane_funcs exynos_plane_funcs = {
 	.reset		= exynos_drm_plane_reset,
 	.atomic_duplicate_state = exynos_drm_plane_duplicate_state,
 	.atomic_destroy_state = exynos_drm_plane_destroy_state,
-	.atomic_set_property = exynos_drm_plane_atomic_set_property,
-	.atomic_get_property = exynos_drm_plane_atomic_get_property,
 };
 
 static int
@@ -304,23 +264,13 @@ static const struct drm_plane_helper_funcs plane_helper_funcs = {
 };
 
 static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
-					      unsigned int zpos)
+					      bool immutable)
 {
-	struct drm_device *dev = plane->dev;
-	struct exynos_drm_private *dev_priv = dev->dev_private;
-	struct drm_property *prop;
-
-	prop = dev_priv->plane_zpos_property;
-	if (!prop) {
-		prop = drm_property_create_range(dev, 0, "zpos",
-						 0, MAX_PLANE - 1);
-		if (!prop)
-			return;
-
-		dev_priv->plane_zpos_property = prop;
-	}
-
-	drm_object_attach_property(&plane->base, prop, zpos);
+	if (immutable)
+		drm_plane_create_zpos_immutable_property(plane,
+							 0, MAX_PLANE - 1);
+	else
+		drm_plane_create_zpos_property(plane, 0, MAX_PLANE - 1);
 }
 
 int exynos_plane_init(struct drm_device *dev,
@@ -346,7 +296,8 @@ int exynos_plane_init(struct drm_device *dev,
 	exynos_plane->index = index;
 	exynos_plane->config = config;
 
-	exynos_plane_attach_zpos_property(&exynos_plane->base, config->zpos);
+	exynos_plane_attach_zpos_property(&exynos_plane->base,
+			   !(config->capabilities & EXYNOS_DRM_PLANE_CAP_ZPOS));
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 74a4269..e1d47f9 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -477,6 +477,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
 	struct drm_display_mode *mode = &state->base.crtc->state->adjusted_mode;
 	struct mixer_resources *res = &ctx->mixer_res;
 	struct drm_framebuffer *fb = state->base.fb;
+	unsigned int priority = state->base.normalized_zpos + 1;
 	unsigned long flags;
 	dma_addr_t luma_addr[2], chroma_addr[2];
 	bool tiled_mode = false;
@@ -561,7 +562,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
 
 	mixer_cfg_scan(ctx, mode->vdisplay);
 	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
-	mixer_cfg_layer(ctx, plane->index, state->zpos + 1, true);
+	mixer_cfg_layer(ctx, plane->index, priority, true);
 	mixer_cfg_vp_blend(ctx);
 	mixer_run(ctx);
 
@@ -586,6 +587,7 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 	struct drm_display_mode *mode = &state->base.crtc->state->adjusted_mode;
 	struct mixer_resources *res = &ctx->mixer_res;
 	struct drm_framebuffer *fb = state->base.fb;
+	unsigned int priority = state->base.normalized_zpos + 1;
 	unsigned long flags;
 	unsigned int win = plane->index;
 	unsigned int x_ratio = 0, y_ratio = 0;
@@ -677,7 +679,7 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 
 	mixer_cfg_scan(ctx, mode->vdisplay);
 	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
-	mixer_cfg_layer(ctx, win, state->zpos + 1, true);
+	mixer_cfg_layer(ctx, win, priority, true);
 	mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->pixel_format));
 
 	/* layer update mandatory for mixer 16.0.33.0 */
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 4/4] drm: rcar: use generic code for managing zpos plane property

[Benjamin Gaignard]

version 4:
fix null pointer issue while setting zpos in plane reset function

This patch replaces zpos property handling custom code in rcar DRM
driver with calls to generic DRM code.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 2 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   | 1 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 5 -----
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 9 ++-------
 drivers/gpu/drm/rcar-du/rcar_du_plane.h | 2 --
 5 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 0d8bdda..28d2cb6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -196,7 +196,7 @@ void rcar_du_crtc_route_output(struct drm_crtc *crtc,
 
 static unsigned int plane_zpos(struct rcar_du_plane *plane)
 {
-	return to_rcar_plane_state(plane->plane.state)->zpos;
+	return plane->plane.state->normalized_zpos;
 }
 
 static const struct rcar_du_format_info *
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index ed35467..c843c31 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -92,7 +92,6 @@ struct rcar_du_device {
 	struct {
 		struct drm_property *alpha;
 		struct drm_property *colorkey;
-		struct drm_property *zpos;
 	} props;
 
 	unsigned int dpad0_source;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 6bb032d..f03eb55 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -527,11 +527,6 @@ static int rcar_du_properties_init(struct rcar_du_device *rcdu)
 	if (rcdu->props.colorkey == NULL)
 		return -ENOMEM;
 
-	rcdu->props.zpos =
-		drm_property_create_range(rcdu->ddev, 0, "zpos", 1, 7);
-	if (rcdu->props.zpos == NULL)
-		return -ENOMEM;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index bfe31ca..dc9bb96 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -652,7 +652,7 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
 	state->source = RCAR_DU_PLANE_MEMORY;
 	state->alpha = 255;
 	state->colorkey = RCAR_DU_COLORKEY_NONE;
-	state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
+	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 
 	plane->state = &state->state;
 	plane->state->plane = plane;
@@ -670,8 +670,6 @@ static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
 		rstate->alpha = val;
 	else if (property == rcdu->props.colorkey)
 		rstate->colorkey = val;
-	else if (property == rcdu->props.zpos)
-		rstate->zpos = val;
 	else
 		return -EINVAL;
 
@@ -690,8 +688,6 @@ static int rcar_du_plane_atomic_get_property(struct drm_plane *plane,
 		*val = rstate->alpha;
 	else if (property == rcdu->props.colorkey)
 		*val = rstate->colorkey;
-	else if (property == rcdu->props.zpos)
-		*val = rstate->zpos;
 	else
 		return -EINVAL;
 
@@ -763,8 +759,7 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
 		drm_object_attach_property(&plane->plane.base,
 					   rcdu->props.colorkey,
 					   RCAR_DU_COLORKEY_NONE);
-		drm_object_attach_property(&plane->plane.base,
-					   rcdu->props.zpos, 1);
+		drm_plane_create_zpos_property(&plane->plane, 1, 7);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
index b18b7b2..8b91dd3 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
@@ -51,7 +51,6 @@ static inline struct rcar_du_plane *to_rcar_plane(struct drm_plane *plane)
  * @hwindex: 0-based hardware plane index, -1 means unused
  * @alpha: value of the plane alpha property
  * @colorkey: value of the plane colorkey property
- * @zpos: value of the plane zpos property
  */
 struct rcar_du_plane_state {
 	struct drm_plane_state state;
@@ -62,7 +61,6 @@ struct rcar_du_plane_state {
 
 	unsigned int alpha;
 	unsigned int colorkey;
-	unsigned int zpos;
 };
 
 static inline struct rcar_du_plane_state *
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Generic zpos property

[Tobias Jakobi]

Hello Benjamin,


Benjamin Gaignard wrote:
> version 5:
> rebased on drm-next where Documentation/DocBook/gpu.tmpl doesn't
> exist anymore.
I think the documentation has just moved to Documentation/gpu, so the
zpos property should be documented there then.


With best wishes,
Tobias


> rework sti patch because some plane functions have changed since v4
> 
> version 4:
> make sure that normalized zpos value is stay in the defined property 
> range and warn user if not.
> Fix NULL pointer bug in rcar-du while setting zpos value.
> No changes in the other drivers.
> 
> version 3:
> use kmalloc_array instead of kmalloc.
> Correct normalize_zpos computation (comeback to Mareck original code)
> 
> version 2:
> add a zpos property into drm_plane structure to simplify code.
> This allow to get/set zpos value in core and not in drivers code.
> Fix various remarks.
> 
> version 1:
> refactor Marek's patches to have per plane zpos property instead of only
> one in core.
> 
> Benjamin Gaignard (2):
>   drm: sti: use generic zpos for plane
>   drm: rcar: use generic code for managing zpos plane property
> 
> Marek Szyprowski (2):
>   drm: add generic zpos property
>   drm/exynos: use generic code for managing zpos plane property
> 
>  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 +
>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |   2 -
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  67 ++-------
>  drivers/gpu/drm/exynos/exynos_mixer.c     |   6 +-
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    |   2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |   1 -
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |   5 -
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |   9 +-
>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |   2 -
>  drivers/gpu/drm/sti/sti_cursor.c          |   4 +-
>  drivers/gpu/drm/sti/sti_gdp.c             |   4 +-
>  drivers/gpu/drm/sti/sti_hqvdp.c           |   4 +-
>  drivers/gpu/drm/sti/sti_mixer.c           |   9 +-
>  drivers/gpu/drm/sti/sti_plane.c           |  76 ++++------
>  drivers/gpu/drm/sti/sti_plane.h           |   7 +-
>  include/drm/drm_crtc.h                    |  30 ++++
>  20 files changed, 324 insertions(+), 147 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_blend.c
> 
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: vincent.abriou@st.com
> Cc: fabien.dessenne@st.com
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 

Re: [PATCH v5 0/4] Generic zpos property

[Benjamin Gaignard]

zpos new fields are correctly documented in drm-kms.html after running
make htmldocs.

Benjamin

2016-07-07 16:01 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
> Hello Benjamin,
>
>
> Benjamin Gaignard wrote:
>> version 5:
>> rebased on drm-next where Documentation/DocBook/gpu.tmpl doesn't
>> exist anymore.
> I think the documentation has just moved to Documentation/gpu, so the
> zpos property should be documented there then.
>
>
> With best wishes,
> Tobias
>
>
>> rework sti patch because some plane functions have changed since v4
>>
>> version 4:
>> make sure that normalized zpos value is stay in the defined property
>> range and warn user if not.
>> Fix NULL pointer bug in rcar-du while setting zpos value.
>> No changes in the other drivers.
>>
>> version 3:
>> use kmalloc_array instead of kmalloc.
>> Correct normalize_zpos computation (comeback to Mareck original code)
>>
>> version 2:
>> add a zpos property into drm_plane structure to simplify code.
>> This allow to get/set zpos value in core and not in drivers code.
>> Fix various remarks.
>>
>> version 1:
>> refactor Marek's patches to have per plane zpos property instead of only
>> one in core.
>>
>> Benjamin Gaignard (2):
>>   drm: sti: use generic zpos for plane
>>   drm: rcar: use generic code for managing zpos plane property
>>
>> Marek Szyprowski (2):
>>   drm: add generic zpos property
>>   drm/exynos: use generic code for managing zpos plane property
>>
>>  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 +
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |   2 -
>>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  67 ++-------
>>  drivers/gpu/drm/exynos/exynos_mixer.c     |   6 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    |   2 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |   1 -
>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |   5 -
>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |   9 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |   2 -
>>  drivers/gpu/drm/sti/sti_cursor.c          |   4 +-
>>  drivers/gpu/drm/sti/sti_gdp.c             |   4 +-
>>  drivers/gpu/drm/sti/sti_hqvdp.c           |   4 +-
>>  drivers/gpu/drm/sti/sti_mixer.c           |   9 +-
>>  drivers/gpu/drm/sti/sti_plane.c           |  76 ++++------
>>  drivers/gpu/drm/sti/sti_plane.h           |   7 +-
>>  include/drm/drm_crtc.h                    |  30 ++++
>>  20 files changed, 324 insertions(+), 147 deletions(-)
>>  create mode 100644 drivers/gpu/drm/drm_blend.c
>>
>> Cc: Inki Dae <inki.dae@samsung.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>> Cc: Gustavo Padovan <gustavo@padovan.org>
>> Cc: vincent.abriou@st.com
>> Cc: fabien.dessenne@st.com
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 4/4] drm: rcar: use generic code for managing zpos plane property

[Laurent Pinchart]

Hi Benjamin,

Thank you for the patch.

The git://linuxtv.org/media_tree.git vsp1 branch will be merged in v4.8 and 
will conflict with this series. If you target v4.8 you should rebase the 
patches on top of that branch.

For your convenience I've pushed a branch in which I've resolved the conflicts 
for v4 of your patches series to

git://linuxtv.org/pinchartl/media.git drm/du/zorder

On Thursday 07 Jul 2016 15:01:24 Benjamin Gaignard wrote:
> version 4:
> fix null pointer issue while setting zpos in plane reset function
> 
> This patch replaces zpos property handling custom code in rcar DRM
> driver with calls to generic DRM code.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   | 1 -
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 5 -----
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 9 ++-------
>  drivers/gpu/drm/rcar-du/rcar_du_plane.h | 2 --
>  5 files changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 0d8bdda..28d2cb6 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -196,7 +196,7 @@ void rcar_du_crtc_route_output(struct drm_crtc *crtc,
> 
>  static unsigned int plane_zpos(struct rcar_du_plane *plane)
>  {
> -	return to_rcar_plane_state(plane->plane.state)->zpos;
> +	return plane->plane.state->normalized_zpos;
>  }
> 
>  static const struct rcar_du_format_info *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index ed35467..c843c31 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -92,7 +92,6 @@ struct rcar_du_device {
>  	struct {
>  		struct drm_property *alpha;
>  		struct drm_property *colorkey;
> -		struct drm_property *zpos;
>  	} props;
> 
>  	unsigned int dpad0_source;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 6bb032d..f03eb55 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -527,11 +527,6 @@ static int rcar_du_properties_init(struct
> rcar_du_device *rcdu) if (rcdu->props.colorkey == NULL)
>  		return -ENOMEM;
> 
> -	rcdu->props.zpos =
> -		drm_property_create_range(rcdu->ddev, 0, "zpos", 1, 7);
> -	if (rcdu->props.zpos == NULL)
> -		return -ENOMEM;
> -
>  	return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index bfe31ca..dc9bb96 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -652,7 +652,7 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
> state->source = RCAR_DU_PLANE_MEMORY;
>  	state->alpha = 255;
>  	state->colorkey = RCAR_DU_COLORKEY_NONE;
> -	state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
> +	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
> 
>  	plane->state = &state->state;
>  	plane->state->plane = plane;
> @@ -670,8 +670,6 @@ static int rcar_du_plane_atomic_set_property(struct
> drm_plane *plane, rstate->alpha = val;
>  	else if (property == rcdu->props.colorkey)
>  		rstate->colorkey = val;
> -	else if (property == rcdu->props.zpos)
> -		rstate->zpos = val;
>  	else
>  		return -EINVAL;
> 
> @@ -690,8 +688,6 @@ static int rcar_du_plane_atomic_get_property(struct
> drm_plane *plane, *val = rstate->alpha;
>  	else if (property == rcdu->props.colorkey)
>  		*val = rstate->colorkey;
> -	else if (property == rcdu->props.zpos)
> -		*val = rstate->zpos;
>  	else
>  		return -EINVAL;
> 
> @@ -763,8 +759,7 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
>  		drm_object_attach_property(&plane->plane.base,
>  					   rcdu->props.colorkey,
>  					   RCAR_DU_COLORKEY_NONE);
> -		drm_object_attach_property(&plane->plane.base,
> -					   rcdu->props.zpos, 1);
> +		drm_plane_create_zpos_property(&plane->plane, 1, 7);
>  	}
> 
>  	return 0;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
> b/drivers/gpu/drm/rcar-du/rcar_du_plane.h index b18b7b2..8b91dd3 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
> @@ -51,7 +51,6 @@ static inline struct rcar_du_plane *to_rcar_plane(struct
> drm_plane *plane) * @hwindex: 0-based hardware plane index, -1 means unused
>   * @alpha: value of the plane alpha property
>   * @colorkey: value of the plane colorkey property
> - * @zpos: value of the plane zpos property
>   */
>  struct rcar_du_plane_state {
>  	struct drm_plane_state state;
> @@ -62,7 +61,6 @@ struct rcar_du_plane_state {
> 
>  	unsigned int alpha;
>  	unsigned int colorkey;
> -	unsigned int zpos;
>  };
> 
>  static inline struct rcar_du_plane_state *

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Generic zpos property

[Tobias Jakobi]

Benjamin Gaignard wrote:
> zpos new fields are correctly documented in drm-kms.html after running
> make htmldocs.
I'm not sure I understand. Where does htmldocs get the information from
then?

- Tobias


> Benjamin
> 
> 2016-07-07 16:01 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
>> Hello Benjamin,
>>
>>
>> Benjamin Gaignard wrote:
>>> version 5:
>>> rebased on drm-next where Documentation/DocBook/gpu.tmpl doesn't
>>> exist anymore.
>> I think the documentation has just moved to Documentation/gpu, so the
>> zpos property should be documented there then.
>>
>>
>> With best wishes,
>> Tobias
>>
>>
>>> rework sti patch because some plane functions have changed since v4
>>>
>>> version 4:
>>> make sure that normalized zpos value is stay in the defined property
>>> range and warn user if not.
>>> Fix NULL pointer bug in rcar-du while setting zpos value.
>>> No changes in the other drivers.
>>>
>>> version 3:
>>> use kmalloc_array instead of kmalloc.
>>> Correct normalize_zpos computation (comeback to Mareck original code)
>>>
>>> version 2:
>>> add a zpos property into drm_plane structure to simplify code.
>>> This allow to get/set zpos value in core and not in drivers code.
>>> Fix various remarks.
>>>
>>> version 1:
>>> refactor Marek's patches to have per plane zpos property instead of only
>>> one in core.
>>>
>>> Benjamin Gaignard (2):
>>>   drm: sti: use generic zpos for plane
>>>   drm: rcar: use generic code for managing zpos plane property
>>>
>>> Marek Szyprowski (2):
>>>   drm: add generic zpos property
>>>   drm/exynos: use generic code for managing zpos plane property
>>>
>>>  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 +
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |   2 -
>>>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  67 ++-------
>>>  drivers/gpu/drm/exynos/exynos_mixer.c     |   6 +-
>>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    |   2 +-
>>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |   1 -
>>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |   5 -
>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |   9 +-
>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |   2 -
>>>  drivers/gpu/drm/sti/sti_cursor.c          |   4 +-
>>>  drivers/gpu/drm/sti/sti_gdp.c             |   4 +-
>>>  drivers/gpu/drm/sti/sti_hqvdp.c           |   4 +-
>>>  drivers/gpu/drm/sti/sti_mixer.c           |   9 +-
>>>  drivers/gpu/drm/sti/sti_plane.c           |  76 ++++------
>>>  drivers/gpu/drm/sti/sti_plane.h           |   7 +-
>>>  include/drm/drm_crtc.h                    |  30 ++++
>>>  20 files changed, 324 insertions(+), 147 deletions(-)
>>>  create mode 100644 drivers/gpu/drm/drm_blend.c
>>>
>>> Cc: Inki Dae <inki.dae@samsung.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>>> Cc: Gustavo Padovan <gustavo@padovan.org>
>>> Cc: vincent.abriou@st.com
>>> Cc: fabien.dessenne@st.com
>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>
> 
> 
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Generic zpos property

[Benjamin Gaignard]

htmldocs get information directly from the comments in .h and .c files

2016-07-07 18:09 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
> Benjamin Gaignard wrote:
>> zpos new fields are correctly documented in drm-kms.html after running
>> make htmldocs.
> I'm not sure I understand. Where does htmldocs get the information from
> then?
>
> - Tobias
>
>
>> Benjamin
>>
>> 2016-07-07 16:01 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
>>> Hello Benjamin,
>>>
>>>
>>> Benjamin Gaignard wrote:
>>>> version 5:
>>>> rebased on drm-next where Documentation/DocBook/gpu.tmpl doesn't
>>>> exist anymore.
>>> I think the documentation has just moved to Documentation/gpu, so the
>>> zpos property should be documented there then.
>>>
>>>
>>> With best wishes,
>>> Tobias
>>>
>>>
>>>> rework sti patch because some plane functions have changed since v4
>>>>
>>>> version 4:
>>>> make sure that normalized zpos value is stay in the defined property
>>>> range and warn user if not.
>>>> Fix NULL pointer bug in rcar-du while setting zpos value.
>>>> No changes in the other drivers.
>>>>
>>>> version 3:
>>>> use kmalloc_array instead of kmalloc.
>>>> Correct normalize_zpos computation (comeback to Mareck original code)
>>>>
>>>> version 2:
>>>> add a zpos property into drm_plane structure to simplify code.
>>>> This allow to get/set zpos value in core and not in drivers code.
>>>> Fix various remarks.
>>>>
>>>> version 1:
>>>> refactor Marek's patches to have per plane zpos property instead of only
>>>> one in core.
>>>>
>>>> Benjamin Gaignard (2):
>>>>   drm: sti: use generic zpos for plane
>>>>   drm: rcar: use generic code for managing zpos plane property
>>>>
>>>> Marek Szyprowski (2):
>>>>   drm: add generic zpos property
>>>>   drm/exynos: use generic code for managing zpos plane property
>>>>
>>>>  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 +
>>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |   2 -
>>>>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  67 ++-------
>>>>  drivers/gpu/drm/exynos/exynos_mixer.c     |   6 +-
>>>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    |   2 +-
>>>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |   1 -
>>>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |   5 -
>>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |   9 +-
>>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |   2 -
>>>>  drivers/gpu/drm/sti/sti_cursor.c          |   4 +-
>>>>  drivers/gpu/drm/sti/sti_gdp.c             |   4 +-
>>>>  drivers/gpu/drm/sti/sti_hqvdp.c           |   4 +-
>>>>  drivers/gpu/drm/sti/sti_mixer.c           |   9 +-
>>>>  drivers/gpu/drm/sti/sti_plane.c           |  76 ++++------
>>>>  drivers/gpu/drm/sti/sti_plane.h           |   7 +-
>>>>  include/drm/drm_crtc.h                    |  30 ++++
>>>>  20 files changed, 324 insertions(+), 147 deletions(-)
>>>>  create mode 100644 drivers/gpu/drm/drm_blend.c
>>>>
>>>> Cc: Inki Dae <inki.dae@samsung.com>
>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>>> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>>>> Cc: Gustavo Padovan <gustavo@padovan.org>
>>>> Cc: vincent.abriou@st.com
>>>> Cc: fabien.dessenne@st.com
>>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>
>>>
>>
>>
>>
>



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 4/4] drm: rcar: use generic code for managing zpos plane property

[Benjamin Gaignard]

Hi Laurent,

May I suggest to merge only zpos core files (i.e the first patch of
this series) and sti patches ?
Like that we can progress and we can resend your patch for rcar when
media_tree vsp1 branch
will be merged into drm-next?

Dave, Daniel: is it possible to do like this ?

Regards,
Benjamin


2016-07-07 17:53 GMT+02:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> Hi Benjamin,
>
> Thank you for the patch.
>
> The git://linuxtv.org/media_tree.git vsp1 branch will be merged in v4.8 and
> will conflict with this series. If you target v4.8 you should rebase the
> patches on top of that branch.
>
> For your convenience I've pushed a branch in which I've resolved the conflicts
> for v4 of your patches series to
>
> git://linuxtv.org/pinchartl/media.git drm/du/zorder
>
> On Thursday 07 Jul 2016 15:01:24 Benjamin Gaignard wrote:
>> version 4:
>> fix null pointer issue while setting zpos in plane reset function
>>
>> This patch replaces zpos property handling custom code in rcar DRM
>> driver with calls to generic DRM code.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 2 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   | 1 -
>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 5 -----
>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 9 ++-------
>>  drivers/gpu/drm/rcar-du/rcar_du_plane.h | 2 --
>>  5 files changed, 3 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 0d8bdda..28d2cb6 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> @@ -196,7 +196,7 @@ void rcar_du_crtc_route_output(struct drm_crtc *crtc,
>>
>>  static unsigned int plane_zpos(struct rcar_du_plane *plane)
>>  {
>> -     return to_rcar_plane_state(plane->plane.state)->zpos;
>> +     return plane->plane.state->normalized_zpos;
>>  }
>>
>>  static const struct rcar_du_format_info *
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index ed35467..c843c31 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> @@ -92,7 +92,6 @@ struct rcar_du_device {
>>       struct {
>>               struct drm_property *alpha;
>>               struct drm_property *colorkey;
>> -             struct drm_property *zpos;
>>       } props;
>>
>>       unsigned int dpad0_source;
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 6bb032d..f03eb55 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> @@ -527,11 +527,6 @@ static int rcar_du_properties_init(struct
>> rcar_du_device *rcdu) if (rcdu->props.colorkey == NULL)
>>               return -ENOMEM;
>>
>> -     rcdu->props.zpos =
>> -             drm_property_create_range(rcdu->ddev, 0, "zpos", 1, 7);
>> -     if (rcdu->props.zpos == NULL)
>> -             return -ENOMEM;
>> -
>>       return 0;
>>  }
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index bfe31ca..dc9bb96 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>> @@ -652,7 +652,7 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
>> state->source = RCAR_DU_PLANE_MEMORY;
>>       state->alpha = 255;
>>       state->colorkey = RCAR_DU_COLORKEY_NONE;
>> -     state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>> +     state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>>
>>       plane->state = &state->state;
>>       plane->state->plane = plane;
>> @@ -670,8 +670,6 @@ static int rcar_du_plane_atomic_set_property(struct
>> drm_plane *plane, rstate->alpha = val;
>>       else if (property == rcdu->props.colorkey)
>>               rstate->colorkey = val;
>> -     else if (property == rcdu->props.zpos)
>> -             rstate->zpos = val;
>>       else
>>               return -EINVAL;
>>
>> @@ -690,8 +688,6 @@ static int rcar_du_plane_atomic_get_property(struct
>> drm_plane *plane, *val = rstate->alpha;
>>       else if (property == rcdu->props.colorkey)
>>               *val = rstate->colorkey;
>> -     else if (property == rcdu->props.zpos)
>> -             *val = rstate->zpos;
>>       else
>>               return -EINVAL;
>>
>> @@ -763,8 +759,7 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
>>               drm_object_attach_property(&plane->plane.base,
>>                                          rcdu->props.colorkey,
>>                                          RCAR_DU_COLORKEY_NONE);
>> -             drm_object_attach_property(&plane->plane.base,
>> -                                        rcdu->props.zpos, 1);
>> +             drm_plane_create_zpos_property(&plane->plane, 1, 7);
>>       }
>>
>>       return 0;
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>> b/drivers/gpu/drm/rcar-du/rcar_du_plane.h index b18b7b2..8b91dd3 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>> @@ -51,7 +51,6 @@ static inline struct rcar_du_plane *to_rcar_plane(struct
>> drm_plane *plane) * @hwindex: 0-based hardware plane index, -1 means unused
>>   * @alpha: value of the plane alpha property
>>   * @colorkey: value of the plane colorkey property
>> - * @zpos: value of the plane zpos property
>>   */
>>  struct rcar_du_plane_state {
>>       struct drm_plane_state state;
>> @@ -62,7 +61,6 @@ struct rcar_du_plane_state {
>>
>>       unsigned int alpha;
>>       unsigned int colorkey;
>> -     unsigned int zpos;
>>  };
>>
>>  static inline struct rcar_du_plane_state *
>
> --
> Regards,
>
> Laurent Pinchart
>



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 1/4] drm: add generic zpos property

[Benjamin Gaignard]

Hi Laurent and Ville,

your feedback on this patch would be appreciated in order to get it
merged for kernel 4.8

Regards,
Benjamin

2016-07-07 15:01 GMT+02:00 Benjamin Gaignard <benjamin.gaignard@linaro.org>:
> From: Marek Szyprowski <m.szyprowski@samsung.com>
>
> 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@samsung.com>
>
> 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@linaro.org>
>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: vincent.abriou@st.com
> Cc: fabien.dessenne@st.com
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  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@samsung.com>
> + *
> + * 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)
> +{
> +       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)
> +{
> +       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;
> +
> +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
>



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 0/4] Generic zpos property

[Daniel Vetter]

On Fri, Jul 08, 2016 at 09:06:18AM +0200, Benjamin Gaignard wrote:
> htmldocs get information directly from the comments in .h and .c files

You're both correct: There's another table which is supposed to list _all_
properties. It's a bit a mess, and should be split up/cleaned up, but
that's a longer-term thing. Please add zpos there too.

See Documentation/gpu/kms-properties.csv

Cheers, Daniel
> 
> 2016-07-07 18:09 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
> > Benjamin Gaignard wrote:
> >> zpos new fields are correctly documented in drm-kms.html after running
> >> make htmldocs.
> > I'm not sure I understand. Where does htmldocs get the information from
> > then?
> >
> > - Tobias
> >
> >
> >> Benjamin
> >>
> >> 2016-07-07 16:01 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
> >>> Hello Benjamin,
> >>>
> >>>
> >>> Benjamin Gaignard wrote:
> >>>> version 5:
> >>>> rebased on drm-next where Documentation/DocBook/gpu.tmpl doesn't
> >>>> exist anymore.
> >>> I think the documentation has just moved to Documentation/gpu, so the
> >>> zpos property should be documented there then.
> >>>
> >>>
> >>> With best wishes,
> >>> Tobias
> >>>
> >>>
> >>>> rework sti patch because some plane functions have changed since v4
> >>>>
> >>>> version 4:
> >>>> make sure that normalized zpos value is stay in the defined property
> >>>> range and warn user if not.
> >>>> Fix NULL pointer bug in rcar-du while setting zpos value.
> >>>> No changes in the other drivers.
> >>>>
> >>>> version 3:
> >>>> use kmalloc_array instead of kmalloc.
> >>>> Correct normalize_zpos computation (comeback to Mareck original code)
> >>>>
> >>>> version 2:
> >>>> add a zpos property into drm_plane structure to simplify code.
> >>>> This allow to get/set zpos value in core and not in drivers code.
> >>>> Fix various remarks.
> >>>>
> >>>> version 1:
> >>>> refactor Marek's patches to have per plane zpos property instead of only
> >>>> one in core.
> >>>>
> >>>> Benjamin Gaignard (2):
> >>>>   drm: sti: use generic zpos for plane
> >>>>   drm: rcar: use generic code for managing zpos plane property
> >>>>
> >>>> Marek Szyprowski (2):
> >>>>   drm: add generic zpos property
> >>>>   drm/exynos: use generic code for managing zpos plane property
> >>>>
> >>>>  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 +
> >>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |   2 -
> >>>>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  67 ++-------
> >>>>  drivers/gpu/drm/exynos/exynos_mixer.c     |   6 +-
> >>>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    |   2 +-
> >>>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |   1 -
> >>>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |   5 -
> >>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |   9 +-
> >>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |   2 -
> >>>>  drivers/gpu/drm/sti/sti_cursor.c          |   4 +-
> >>>>  drivers/gpu/drm/sti/sti_gdp.c             |   4 +-
> >>>>  drivers/gpu/drm/sti/sti_hqvdp.c           |   4 +-
> >>>>  drivers/gpu/drm/sti/sti_mixer.c           |   9 +-
> >>>>  drivers/gpu/drm/sti/sti_plane.c           |  76 ++++------
> >>>>  drivers/gpu/drm/sti/sti_plane.h           |   7 +-
> >>>>  include/drm/drm_crtc.h                    |  30 ++++
> >>>>  20 files changed, 324 insertions(+), 147 deletions(-)
> >>>>  create mode 100644 drivers/gpu/drm/drm_blend.c
> >>>>
> >>>> Cc: Inki Dae <inki.dae@samsung.com>
> >>>> Cc: Daniel Vetter <daniel@ffwll.ch>
> >>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> >>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> >>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> >>>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> >>>> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> >>>> Cc: Gustavo Padovan <gustavo@padovan.org>
> >>>> Cc: vincent.abriou@st.com
> >>>> Cc: fabien.dessenne@st.com
> >>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>
> >>>
> >>
> >>
> >>
> >
> 
> 
> 
> -- 
> Benjamin Gaignard
> 
> Graphic Working Group
> 
> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro: Facebook | Twitter | Blog

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Generic zpos property

[Benjamin Gaignard]

What is the required tool to edit this .csv file ?

2016-07-12 16:01 GMT+02:00 Daniel Vetter <daniel@ffwll.ch>:
> On Fri, Jul 08, 2016 at 09:06:18AM +0200, Benjamin Gaignard wrote:
>> htmldocs get information directly from the comments in .h and .c files
>
> You're both correct: There's another table which is supposed to list _all_
> properties. It's a bit a mess, and should be split up/cleaned up, but
> that's a longer-term thing. Please add zpos there too.
>
> See Documentation/gpu/kms-properties.csv
>
> Cheers, Daniel
>>
>> 2016-07-07 18:09 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
>> > Benjamin Gaignard wrote:
>> >> zpos new fields are correctly documented in drm-kms.html after running
>> >> make htmldocs.
>> > I'm not sure I understand. Where does htmldocs get the information from
>> > then?
>> >
>> > - Tobias
>> >
>> >
>> >> Benjamin
>> >>
>> >> 2016-07-07 16:01 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
>> >>> Hello Benjamin,
>> >>>
>> >>>
>> >>> Benjamin Gaignard wrote:
>> >>>> version 5:
>> >>>> rebased on drm-next where Documentation/DocBook/gpu.tmpl doesn't
>> >>>> exist anymore.
>> >>> I think the documentation has just moved to Documentation/gpu, so the
>> >>> zpos property should be documented there then.
>> >>>
>> >>>
>> >>> With best wishes,
>> >>> Tobias
>> >>>
>> >>>
>> >>>> rework sti patch because some plane functions have changed since v4
>> >>>>
>> >>>> version 4:
>> >>>> make sure that normalized zpos value is stay in the defined property
>> >>>> range and warn user if not.
>> >>>> Fix NULL pointer bug in rcar-du while setting zpos value.
>> >>>> No changes in the other drivers.
>> >>>>
>> >>>> version 3:
>> >>>> use kmalloc_array instead of kmalloc.
>> >>>> Correct normalize_zpos computation (comeback to Mareck original code)
>> >>>>
>> >>>> version 2:
>> >>>> add a zpos property into drm_plane structure to simplify code.
>> >>>> This allow to get/set zpos value in core and not in drivers code.
>> >>>> Fix various remarks.
>> >>>>
>> >>>> version 1:
>> >>>> refactor Marek's patches to have per plane zpos property instead of only
>> >>>> one in core.
>> >>>>
>> >>>> Benjamin Gaignard (2):
>> >>>>   drm: sti: use generic zpos for plane
>> >>>>   drm: rcar: use generic code for managing zpos plane property
>> >>>>
>> >>>> Marek Szyprowski (2):
>> >>>>   drm: add generic zpos property
>> >>>>   drm/exynos: use generic code for managing zpos plane property
>> >>>>
>> >>>>  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 +
>> >>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |   2 -
>> >>>>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  67 ++-------
>> >>>>  drivers/gpu/drm/exynos/exynos_mixer.c     |   6 +-
>> >>>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    |   2 +-
>> >>>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |   1 -
>> >>>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |   5 -
>> >>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |   9 +-
>> >>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |   2 -
>> >>>>  drivers/gpu/drm/sti/sti_cursor.c          |   4 +-
>> >>>>  drivers/gpu/drm/sti/sti_gdp.c             |   4 +-
>> >>>>  drivers/gpu/drm/sti/sti_hqvdp.c           |   4 +-
>> >>>>  drivers/gpu/drm/sti/sti_mixer.c           |   9 +-
>> >>>>  drivers/gpu/drm/sti/sti_plane.c           |  76 ++++------
>> >>>>  drivers/gpu/drm/sti/sti_plane.h           |   7 +-
>> >>>>  include/drm/drm_crtc.h                    |  30 ++++
>> >>>>  20 files changed, 324 insertions(+), 147 deletions(-)
>> >>>>  create mode 100644 drivers/gpu/drm/drm_blend.c
>> >>>>
>> >>>> Cc: Inki Dae <inki.dae@samsung.com>
>> >>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> >>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> >>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>> >>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>> >>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>> >>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> >>>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> >>>> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>> >>>> Cc: Gustavo Padovan <gustavo@padovan.org>
>> >>>> Cc: vincent.abriou@st.com
>> >>>> Cc: fabien.dessenne@st.com
>> >>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> >>>>
>> >>>
>> >>
>> >>
>> >>
>> >
>>
>>
>>
>> --
>> Benjamin Gaignard
>>
>> Graphic Working Group
>>
>> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro: Facebook | Twitter | Blog
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 0/4] Generic zpos property

[Daniel Vetter]

On Tue, Jul 12, 2016 at 04:47:15PM +0200, Benjamin Gaignard wrote:
> What is the required tool to edit this .csv file ?

You're favourite text editor, it's a csv (= comman separated values). Just
pls check the end result in the generated docs with

$ make htmldocs

The makefiles should complain if you don't have all the sphinx bits needed
installed.
-Daniel

> 
> 2016-07-12 16:01 GMT+02:00 Daniel Vetter <daniel@ffwll.ch>:
> > On Fri, Jul 08, 2016 at 09:06:18AM +0200, Benjamin Gaignard wrote:
> >> htmldocs get information directly from the comments in .h and .c files
> >
> > You're both correct: There's another table which is supposed to list _all_
> > properties. It's a bit a mess, and should be split up/cleaned up, but
> > that's a longer-term thing. Please add zpos there too.
> >
> > See Documentation/gpu/kms-properties.csv
> >
> > Cheers, Daniel
> >>
> >> 2016-07-07 18:09 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
> >> > Benjamin Gaignard wrote:
> >> >> zpos new fields are correctly documented in drm-kms.html after running
> >> >> make htmldocs.
> >> > I'm not sure I understand. Where does htmldocs get the information from
> >> > then?
> >> >
> >> > - Tobias
> >> >
> >> >
> >> >> Benjamin
> >> >>
> >> >> 2016-07-07 16:01 GMT+02:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
> >> >>> Hello Benjamin,
> >> >>>
> >> >>>
> >> >>> Benjamin Gaignard wrote:
> >> >>>> version 5:
> >> >>>> rebased on drm-next where Documentation/DocBook/gpu.tmpl doesn't
> >> >>>> exist anymore.
> >> >>> I think the documentation has just moved to Documentation/gpu, so the
> >> >>> zpos property should be documented there then.
> >> >>>
> >> >>>
> >> >>> With best wishes,
> >> >>> Tobias
> >> >>>
> >> >>>
> >> >>>> rework sti patch because some plane functions have changed since v4
> >> >>>>
> >> >>>> version 4:
> >> >>>> make sure that normalized zpos value is stay in the defined property
> >> >>>> range and warn user if not.
> >> >>>> Fix NULL pointer bug in rcar-du while setting zpos value.
> >> >>>> No changes in the other drivers.
> >> >>>>
> >> >>>> version 3:
> >> >>>> use kmalloc_array instead of kmalloc.
> >> >>>> Correct normalize_zpos computation (comeback to Mareck original code)
> >> >>>>
> >> >>>> version 2:
> >> >>>> add a zpos property into drm_plane structure to simplify code.
> >> >>>> This allow to get/set zpos value in core and not in drivers code.
> >> >>>> Fix various remarks.
> >> >>>>
> >> >>>> version 1:
> >> >>>> refactor Marek's patches to have per plane zpos property instead of only
> >> >>>> one in core.
> >> >>>>
> >> >>>> Benjamin Gaignard (2):
> >> >>>>   drm: sti: use generic zpos for plane
> >> >>>>   drm: rcar: use generic code for managing zpos plane property
> >> >>>>
> >> >>>> Marek Szyprowski (2):
> >> >>>>   drm: add generic zpos property
> >> >>>>   drm/exynos: use generic code for managing zpos plane property
> >> >>>>
> >> >>>>  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 +
> >> >>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |   2 -
> >> >>>>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  67 ++-------
> >> >>>>  drivers/gpu/drm/exynos/exynos_mixer.c     |   6 +-
> >> >>>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    |   2 +-
> >> >>>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |   1 -
> >> >>>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |   5 -
> >> >>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |   9 +-
> >> >>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |   2 -
> >> >>>>  drivers/gpu/drm/sti/sti_cursor.c          |   4 +-
> >> >>>>  drivers/gpu/drm/sti/sti_gdp.c             |   4 +-
> >> >>>>  drivers/gpu/drm/sti/sti_hqvdp.c           |   4 +-
> >> >>>>  drivers/gpu/drm/sti/sti_mixer.c           |   9 +-
> >> >>>>  drivers/gpu/drm/sti/sti_plane.c           |  76 ++++------
> >> >>>>  drivers/gpu/drm/sti/sti_plane.h           |   7 +-
> >> >>>>  include/drm/drm_crtc.h                    |  30 ++++
> >> >>>>  20 files changed, 324 insertions(+), 147 deletions(-)
> >> >>>>  create mode 100644 drivers/gpu/drm/drm_blend.c
> >> >>>>
> >> >>>> Cc: Inki Dae <inki.dae@samsung.com>
> >> >>>> Cc: Daniel Vetter <daniel@ffwll.ch>
> >> >>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> >>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> >> >>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> >> >>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >> >>>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> >> >>>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> >> >>>> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> >> >>>> Cc: Gustavo Padovan <gustavo@padovan.org>
> >> >>>> Cc: vincent.abriou@st.com
> >> >>>> Cc: fabien.dessenne@st.com
> >> >>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> >>>>
> >> >>>
> >> >>
> >> >>
> >> >>
> >> >
> >>
> >>
> >>
> >> --
> >> Benjamin Gaignard
> >>
> >> Graphic Working Group
> >>
> >> Linaro.org │ Open source software for ARM SoCs
> >>
> >> Follow Linaro: Facebook | Twitter | Blog
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> 
> 
> -- 
> Benjamin Gaignard
> 
> Graphic Working Group
> 
> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro: Facebook | Twitter | Blog

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/4] drm: add generic zpos property

[Ville Syrjälä]

On Thu, Jul 07, 2016 at 03:01:21PM +0200, Benjamin Gaignard wrote:
> From: Marek Szyprowski <m.szyprowski@samsung.com>
> 
> 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@samsung.com>
> 
> 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@linaro.org>
> 
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: vincent.abriou@st.com
> Cc: fabien.dessenne@st.com
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  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@samsung.com>
> + *
> + * 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

Re: [PATCH v5 1/4] drm: add generic zpos property

[Benjamin Gaignard]

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@linux.intel.com>:
> On Thu, Jul 07, 2016 at 03:01:21PM +0200, Benjamin Gaignard wrote:
>> From: Marek Szyprowski <m.szyprowski@samsung.com>
>>
>> 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@samsung.com>
>>
>> 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@linaro.org>
>>
>> Cc: Inki Dae <inki.dae@samsung.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>> Cc: Gustavo Padovan <gustavo@padovan.org>
>> Cc: vincent.abriou@st.com
>> Cc: fabien.dessenne@st.com
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>  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@samsung.com>
>> + *
>> + * 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