[PATCH 0/2] drm: add generic zpos property

[PATCH 0/2] drm: add generic zpos property

[Benjamin Gaignard]

The original patches have been done by Marek:
https://lists.freedesktop.org/archives/dri-devel/2016-January/099572.html

I have just adapt them to make zpos depend on plane and no more on drm core.

Since zpos range can be define per plane normalize function takes care of
the min and max before compute normalized zpos value.

Benjamin Gaignard (2):
  drm: add generic zpos property
  drm: sti: use generic zpos for plane

 Documentation/DocBook/gpu.tmpl      |  10 ++
 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         | 242 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_crtc_internal.h |   3 +
 drivers/gpu/drm/sti/sti_mixer.c     |   2 +-
 drivers/gpu/drm/sti/sti_plane.c     |  82 +++++-------
 drivers/gpu/drm/sti/sti_plane.h     |   2 -
 include/drm/drm_crtc.h              |  17 +++
 10 files changed, 314 insertions(+), 56 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_blend.c

-- 
1.9.1

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

[PATCH 1/2] drm: add generic zpos property

[Benjamin Gaignard]

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
mutable 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: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 Documentation/DocBook/gpu.tmpl      |  10 ++
 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         | 242 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_crtc_internal.h |   3 +
 include/drm/drm_crtc.h              |  17 +++
 7 files changed, 283 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_blend.c

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 1692c4d..b7a5161 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2147,6 +2147,16 @@ void intel_crt_init(struct drm_device *dev)
 		the underlying hardware).</td>
 	</tr>
 	<tr>
+	<td valign="top" > "zpos" </td>
+	<td valign="top" >RANGE</td>
+	<td valign="top" >Min= driver dependent, Max= driver dependent</td>
+	<td valign="top" >Plane</td>
+	<td valign="top" >Plane's 'z' position during blending operation (0 for background, highest for frontmost).
+		If two planes assigned to same CRTC have equal zpos values, the plane with higher plane
+		id is treated as closer to front. Can be IMMUTABLE if driver doesn't support changing
+		planes' order. Exact value range is driver dependent.</td>
+	</tr>
+	<tr>
 	<td rowspan="20" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6eb94fc..18dc563 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 8ee1db8..bc42582 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -701,6 +701,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);
@@ -757,6 +759,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 4befe25..ed61f9d 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
  *
@@ -587,6 +589,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..e79367e
--- /dev/null
+++ b/drivers/gpu/drm/drm_blend.c
@@ -0,0 +1,242 @@
+/*
+ * 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 <linux/slab.h>
+#include <linux/sort.h>
+#include <linux/export.h>
+#include <drm/drmP.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_atomic.h>
+
+#include "drm_internal.h"
+
+#define MAX(a, b) (((a) > (b))?(a):(b))
+
+/**
+ * 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;
+	int zpos_a = (sa->zpos << 16) + sa->plane->base.id;
+	int zpos_b = (sb->zpos << 16) + sb->plane->base.id;
+
+	return zpos_a - zpos_b;
+}
+
+/**
+ * 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, zpos, n = 0;
+	int ret = 0;
+
+	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
+			 crtc->base.id, crtc->name);
+
+	states = kmalloc(total_planes * sizeof(*states), GFP_KERNEL);
+	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 fail;
+		}
+		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, zpos = 0; i < n; i++, zpos++) {
+		plane = states[i]->plane;
+
+		if (!plane->zpos_property) {
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		/*
+		 * make sure that zpos is at least equal to the minimun
+		 * of the property range.
+		 */
+		zpos = MAX(zpos, plane->zpos_property->values[0]);
+
+		if (zpos > plane->zpos_property->values[1]) {
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		states[i]->normalized_zpos = zpos;
+		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
+				 plane->base.id, plane->name, zpos);
+	}
+	crtc_state->zpos_changed = true;
+
+fail:
+	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 247dc8b..c87cd95 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -42,3 +42,6 @@ int drm_atomic_get_property(struct drm_mode_object *obj,
 int drm_mode_atomic_ioctl(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv);
 
+/* 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 e0170bf..4fe19f1 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -305,6 +305,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
@@ -340,6 +341,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:
@@ -1263,6 +1265,9 @@ struct drm_connector {
  *	plane (in 16.16)
  * @src_w: width of visible portion of plane (in 16.16)
  * @src_h: height of visible portion of plane (in 16.16)
+ * @zpos: priority of the given plane on crtc (optional)
+ * @normalized_zpos: normalized value of zpos: unique, range from 0 to N
+ *	 for given crtc
  * @state: backpointer to global drm_atomic_state
  */
 struct drm_plane_state {
@@ -1283,6 +1288,10 @@ struct drm_plane_state {
 	/* Plane rotation */
 	unsigned int rotation;
 
+	/* Plane zpos */
+	unsigned int zpos;
+	unsigned int normalized_zpos;
+
 	struct drm_atomic_state *state;
 };
 
@@ -1514,6 +1523,7 @@ enum drm_plane_type {
  * 	drm_mode_set_config_internal() to implement correct refcounting.
  * @funcs: helper functions
  * @properties: property tracking for this plane
+ * @zpos_property: attached zpos property for this plane
  * @type: type of plane (overlay, primary, cursor)
  * @state: current atomic state for this plane
  */
@@ -1540,6 +1550,7 @@ struct drm_plane {
 	const struct drm_plane_funcs *funcs;
 
 	struct drm_object_properties properties;
+	struct drm_property *zpos_property;
 
 	enum drm_plane_type type;
 
@@ -2551,6 +2562,12 @@ extern struct drm_property *drm_mode_create_rotation_property(struct drm_device
 extern unsigned int drm_rotation_simplify(unsigned int rotation,
 					  unsigned int supported_rotations);
 
+extern int drm_plane_create_zpos_property(struct drm_plane *plane,
+					  unsigned int min, unsigned int max);
+extern int drm_plane_create_zpos_immutable_property(struct drm_plane *plane,
+						    unsigned int min,
+						    unsigned int max);
+
 /* Helpers */
 
 static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
-- 
1.9.1

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

[PATCH 2/2] 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>
---
 drivers/gpu/drm/sti/sti_mixer.c |  2 +-
 drivers/gpu/drm/sti/sti_plane.c | 82 +++++++++++++++--------------------------
 drivers/gpu/drm/sti/sti_plane.h |  2 -
 3 files changed, 31 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_mixer.c b/drivers/gpu/drm/sti/sti_mixer.c
index e7425c3..040abfe 100644
--- a/drivers/gpu/drm/sti/sti_mixer.c
+++ b/drivers/gpu/drm/sti/sti_mixer.c
@@ -245,7 +245,7 @@ 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 + 1;
 	unsigned int i;
 	u32 mask, val;
 
diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index f10c98d..ee35ba3 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) {
@@ -114,69 +105,56 @@ static void sti_plane_destroy(struct drm_plane *drm_plane)
 	drm_plane_cleanup(drm_plane);
 }
 
-static 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 8;
 	}
+	return 0;
+}
 
-	return -EINVAL;
+static 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:
+		drm_plane_create_zpos_immutable_property(drm_plane, 0, 0);
+		break;
+	case DRM_PLANE_TYPE_OVERLAY:
+		drm_plane_create_zpos_property(drm_plane, 1, 7);
+		break;
+	case DRM_PLANE_TYPE_CURSOR:
+		drm_plane_create_zpos_immutable_property(drm_plane, 8, 8);
+		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));
 }
 
 struct drm_plane_funcs sti_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = sti_plane_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,
 };
diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
index c50a3b9..2747773 100644
--- a/drivers/gpu/drm/sti/sti_plane.h
+++ b/drivers/gpu/drm/sti/sti_plane.h
@@ -68,14 +68,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;
 };
 
-- 
1.9.1

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

Re: [PATCH 0/2] drm: add generic zpos property

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 480 bytes --]

On Wed, Mar 30, 2016 at 01:34:30PM +0200, Benjamin Gaignard wrote:
> The original patches have been done by Marek:
> https://lists.freedesktop.org/archives/dri-devel/2016-January/099572.html

It's good practice to mention this in the commit message of the patch
that you've reworked because the git repository won't contain the text
from the cover letter. Typically you'd add something like this:

	Based on work by Marek Szyprowski <m.szyprowski@samsung.com>

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 0/2] drm: add generic zpos property

[Marek Szyprowski]

Hello,

On 2016-03-30 13:34, Benjamin Gaignard wrote:
> The original patches have been done by Marek:
> https://lists.freedesktop.org/archives/dri-devel/2016-January/099572.html
>
> I have just adapt them to make zpos depend on plane and no more on drm core.
>
> Since zpos range can be define per plane normalize function takes care of
> the min and max before compute normalized zpos value.

Thanks for handling this. I was terribly busy with internal stuff and had no
time to check all my emails. Usually if one continues someone else patches,
it is a good practice to somehow credit the original author in the final
patch.

If changes are small, I usually keep the original authorship and only add
note before my signoff (see commit 855ed04a3758b205e84b269f92d26ab36ed8e2f7
for the example). When the whole code has been rewritten, modified or 
heavily
extended, I changed authorship and added respective note to the commit 
message
(see commit e8d9d1f5485b52ec3c4d7af839e6914438f6c285 for such example). Feel
free to use any of those styles.

> Benjamin Gaignard (2):
>    drm: add generic zpos property
>    drm: sti: use generic zpos for plane
>
>   Documentation/DocBook/gpu.tmpl      |  10 ++
>   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         | 242 ++++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/drm_crtc_internal.h |   3 +
>   drivers/gpu/drm/sti/sti_mixer.c     |   2 +-
>   drivers/gpu/drm/sti/sti_plane.c     |  82 +++++-------
>   drivers/gpu/drm/sti/sti_plane.h     |   2 -
>   include/drm/drm_crtc.h              |  17 +++
>   10 files changed, 314 insertions(+), 56 deletions(-)
>   create mode 100644 drivers/gpu/drm/drm_blend.c
>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

Re: [PATCH 0/2] drm: add generic zpos property

[Emil Velikov]

Hi Benjamin,

On 30 March 2016 at 12:34, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> The original patches have been done by Marek:
> https://lists.freedesktop.org/archives/dri-devel/2016-January/099572.html
>
> I have just adapt them to make zpos depend on plane and no more on drm core.
>
> Since zpos range can be define per plane normalize function takes care of
> the min and max before compute normalized zpos value.
>
> Benjamin Gaignard (2):
>   drm: add generic zpos property
>   drm: sti: use generic zpos for plane
>
From a quick look it seems that you've updated the patches as opposed
to rewriting them, correct ? If so shouldn't the authorship be
preserved and a changelog added within the patch (before or after the
--- line) ?

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

Re: [PATCH 0/2] drm: add generic zpos property

[Emil Velikov]

On 1 April 2016 at 14:35, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> Hi Benjamin,
>
> On 30 March 2016 at 12:34, Benjamin Gaignard
> <benjamin.gaignard@linaro.org> wrote:
>> The original patches have been done by Marek:
>> https://lists.freedesktop.org/archives/dri-devel/2016-January/099572.html
>>
>> I have just adapt them to make zpos depend on plane and no more on drm core.
>>
>> Since zpos range can be define per plane normalize function takes care of
>> the min and max before compute normalized zpos value.
>>
>> Benjamin Gaignard (2):
>>   drm: add generic zpos property
>>   drm: sti: use generic zpos for plane
>>
> From a quick look it seems that you've updated the patches as opposed
> to rewriting them, correct ? If so shouldn't the authorship be
> preserved and a changelog added within the patch (before or after the
> --- line) ?
>
The remainder of the thread just showed up in my inbox. Apologies for the noise.

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