[PATCH v2 0/2] drm/blend: Add per-plane pixel blend mode property

[PATCH v2 0/2] drm/blend: Add per-plane pixel blend mode property

[Lowry Li]

Hi,

This serie aims at adding the support for pixel blend modes represent the
alpha blending equation selection in the driver. It also introduces to use
it in the malidp driver.

Let me know what you think,
Lowry

Changes for v2:
 - Moves the blending equation into the DOC comment
 - Refines the comments of drm_plane_create_blend_mode_property to not
   enumerate the #defines, but instead the string values
 - Uses fg.* instead of pixel.* and plane_alpha instead of plane.alpha
 - Introduces to use it in the malidp driver, which depends on the plane
   alpha patch

Changes from v1:
 - v1 is just the core changes to request for commments
 - Adds a pixel_blend_mode to drm_plane_state and a blend_mode_property to
   drm_plane, and related support functions
 - Defines three blend modes in drm_blend.h
 - Rebased on current drm-next

Lowry Li (2):
  drm/blend: Add per-plane pixel blend mode property
  drm/mali-dp: Implement plane alpha and pixel blend on malidp

 drivers/gpu/drm/arm/malidp_planes.c |  76 ++++++++++++++-----------
 drivers/gpu/drm/drm_atomic.c        |   4 ++
 drivers/gpu/drm/drm_atomic_helper.c |   1 +
 drivers/gpu/drm/drm_blend.c         | 110 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_blend.h             |   6 ++
 include/drm/drm_plane.h             |   6 ++
 6 files changed, 171 insertions(+), 32 deletions(-)

-- 
1.9.1

[PATCH v2 1/2] drm/blend: Add per-plane pixel blend mode property

[Lowry Li]

Pixel blend modes represent the alpha blending equation
selection, describing how the pixels from the current
plane are composited with the background.

Add a pixel_blend_mode to drm_plane_state and a
blend_mode_property to drm_plane, and related support
functions.

Defines three blend modes in drm_blend.h.

Signed-off-by: Lowry Li <lowry.li@arm.com>
---
 drivers/gpu/drm/drm_atomic.c        |   4 ++
 drivers/gpu/drm/drm_atomic_helper.c |   1 +
 drivers/gpu/drm/drm_blend.c         | 110 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_blend.h             |   6 ++
 include/drm/drm_plane.h             |   6 ++
 5 files changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7d25c42..467f8de 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -783,6 +783,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		state->src_w = val;
 	} else if (property == config->prop_src_h) {
 		state->src_h = val;
+	} else if (property == plane->blend_mode_property) {
+		state->pixel_blend_mode = val;
 	} else if (property == plane->rotation_property) {
 		if (!is_power_of_2(val & DRM_MODE_ROTATE_MASK))
 			return -EINVAL;
@@ -848,6 +850,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		*val = state->src_w;
 	} else if (property == config->prop_src_h) {
 		*val = state->src_h;
+	} else if (property == plane->blend_mode_property) {
+		*val = state->pixel_blend_mode;
 	} else if (property == plane->rotation_property) {
 		*val = state->rotation;
 	} else if (property == plane->zpos_property) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index c356545..ddbd0d2 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3484,6 +3484,7 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
 	if (plane->state) {
 		plane->state->plane = plane;
 		plane->state->rotation = DRM_MODE_ROTATE_0;
+		plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 5a81e1b..4ac45da 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -100,6 +100,41 @@
  *	planes. Without this property the primary plane is always below the cursor
  *	plane, and ordering between all other planes is undefined.
  *
+ * pixel blend mode:
+ *	Pixel blend mode is set up with drm_plane_create_blend_mode_property().
+ *	It adds a blend mode for alpha blending equation selection, describing
+ *	how the pixels from the current plane are composited with the
+ *	background.
+ *
+ *	Three alpha blending equations(note that the fg.rgb or bg.rgb notation
+ *	means each of the R, G or B channels for the foreground and background
+ *	colors, respectively):
+ *
+ *	"None": Blend formula that ignores the pixel alpha.
+ *	out.rgb = plane_alpha * fg.rgb + (1 - plane_alpha) * bg.rgb
+ *
+ *	"Pre-multiplied": Blend formula that assumes the pixel color values
+ *	have been already pre-multiplied with the alpha
+ *	channel values.
+ *	out.rgb = plane_alpha * fg.rgb + (1 - (plane_alpha * fg.alpha)) * bg.rgb
+ *
+ *	"Coverage": Blend formula that assumes the pixel color values have not
+ *	been pre-multiplied and will do so when blending them to the background
+ *	color values.
+ *	out.rgb = plane_alpha * fg.alpha * fg.rgb +
+ *		  (1 - (plane_alpha * fg.alpha)) * bg.rgb
+ *
+ *	fg.rgb: Each of the RGB component values from the plane's pixel
+ *	fg.alpha: Alpha component value from the plane's pixel
+ *	bg.rgb: Each of the RGB component values from the background
+ *	plane_alpha: Plane alpha value set by the plane alpha property (if
+ *		     applicable).
+ *
+ *	This property has no effect on formats with no pixel alpha, as fg.alpha
+ *	is assumed to be 1.0. If the plane does not expose the "alpha" property,
+ *	then plane_alpha is assumed to be 1.0, otherwise, it is the value of the
+ *	"alpha" property.
+ *
  * Note that all the property extensions described here apply either to the
  * plane or the CRTC (e.g. for the background color, which currently is not
  * exposed and assumed to be black).
@@ -409,3 +444,78 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
 	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_normalize_zpos);
+
+/**
+ * drm_plane_create_blend_mode_property - create a new blend mode property
+ * @plane: drm plane
+ * @supported_modes: bitmask of supported modes, must include
+ *		     BIT(DRM_MODE_BLEND_PREMULTI)
+ *
+ * This creates a new property describing the blend mode.
+ *
+ * The property exposed to userspace is an enumeration property (see
+ * drm_property_create_enum()) called "pixel blend mode" and has the
+ * following enumeration values:
+ *
+ * "None": Blend formula that ignores the pixel alpha.
+ *
+ * "Pre-multiplied": Blend formula that assumes the pixel color values have
+ *		     been already pre-multiplied with the alpha channel values.
+ *
+ * "Coverage": Blend formula that assumes the pixel color values have not been
+ *	       pre-multiplied and will do so when blending them to the
+ *	       background color values.
+ *
+ * RETURNS:
+ * Zero for success or -errno
+ */
+int drm_plane_create_blend_mode_property(struct drm_plane *plane,
+					 unsigned int supported_modes)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_property *prop;
+	static const struct drm_prop_enum_list props[] = {
+		{ DRM_MODE_BLEND_PIXEL_NONE, "None" },
+		{ DRM_MODE_BLEND_PREMULTI, "Pre-multiplied" },
+		{ DRM_MODE_BLEND_COVERAGE, "Coverage" },
+	};
+	unsigned int valid_mode_mask = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
+				       BIT(DRM_MODE_BLEND_PREMULTI)   |
+				       BIT(DRM_MODE_BLEND_COVERAGE);
+	int i, j = 0;
+
+	if (WARN_ON((supported_modes & ~valid_mode_mask) ||
+		    ((supported_modes & BIT(DRM_MODE_BLEND_PREMULTI)) == 0)))
+		return -EINVAL;
+
+	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
+				   "pixel blend mode",
+				   hweight32(supported_modes));
+	if (!prop)
+		return -ENOMEM;
+
+	for (i = 0; i < ARRAY_SIZE(props); i++) {
+		int ret;
+
+		if (!(BIT(props[i].type) & supported_modes))
+			continue;
+
+		ret = drm_property_add_enum(prop, j++, props[i].type,
+					    props[i].name);
+
+		if (ret) {
+			drm_property_destroy(dev, prop);
+
+			return ret;
+		}
+	}
+
+	drm_object_attach_property(&plane->base, prop, DRM_MODE_BLEND_PREMULTI);
+	plane->blend_mode_property = prop;
+
+	if (plane->state)
+		plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 1760602..2966c0d 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -27,6 +27,10 @@
 #include <linux/ctype.h>
 #include <drm/drm_mode.h>
 
+#define DRM_MODE_BLEND_PIXEL_NONE	0
+#define DRM_MODE_BLEND_PREMULTI		1
+#define DRM_MODE_BLEND_COVERAGE		2
+
 struct drm_device;
 struct drm_atomic_state;
 struct drm_plane;
@@ -49,4 +53,6 @@ int drm_plane_create_zpos_immutable_property(struct drm_plane *plane,
 					     unsigned int zpos);
 int drm_atomic_normalize_zpos(struct drm_device *dev,
 			      struct drm_atomic_state *state);
+int drm_plane_create_blend_mode_property(struct drm_plane *plane,
+					 unsigned int supported_modes);
 #endif
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index f7bf4a4..447ebe7 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -43,6 +43,8 @@
  *	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)
+ * @pixel_blend_mode: how the plane's framebuffer alpha channel is used when
+ *	blending with the background colour.
  * @rotation: rotation of the plane
  * @zpos: priority of the given plane on crtc (optional)
  *	Note that multiple active planes on the same crtc can have an identical
@@ -106,6 +108,8 @@ struct drm_plane_state {
 	uint32_t src_x, src_y;
 	uint32_t src_h, src_w;
 
+	uint16_t pixel_blend_mode;
+
 	/* Plane rotation */
 	unsigned int rotation;
 
@@ -498,6 +502,7 @@ enum drm_plane_type {
  * @type: type of plane (overlay, primary, cursor)
  * @zpos_property: zpos property for this plane
  * @rotation_property: rotation property for this plane
+ * @blend_mode_property: blend mode property for this plane
  * @helper_private: mid-layer private data
  */
 struct drm_plane {
@@ -573,6 +578,7 @@ struct drm_plane {
 
 	struct drm_property *zpos_property;
 	struct drm_property *rotation_property;
+	struct drm_property *blend_mode_property;
 
 	/**
 	 * @color_encoding_property:
-- 
1.9.1

Re: [PATCH v2 1/2] drm/blend: Add per-plane pixel blend mode property

[Brian Starkey]

Hi Lowry,

On Wed, May 30, 2018 at 07:23:53PM +0800, Lowry Li wrote:
>Pixel blend modes represent the alpha blending equation
>selection, describing how the pixels from the current
>plane are composited with the background.
>
>Add a pixel_blend_mode to drm_plane_state and a
>blend_mode_property to drm_plane, and related support
>functions.
>
>Defines three blend modes in drm_blend.h.
>
>Signed-off-by: Lowry Li <lowry.li@arm.com>

With or without the kerneldoc tweaks I've suggested below, this patch
is:

Reviewed-by: Brian Starkey <brian.starkey@arm.com>

>---
> drivers/gpu/drm/drm_atomic.c        |   4 ++
> drivers/gpu/drm/drm_atomic_helper.c |   1 +
> drivers/gpu/drm/drm_blend.c         | 110 ++++++++++++++++++++++++++++++++++++
> include/drm/drm_blend.h             |   6 ++
> include/drm/drm_plane.h             |   6 ++
> 5 files changed, 127 insertions(+)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 7d25c42..467f8de 100644
>--- a/drivers/gpu/drm/drm_atomic.c
>+++ b/drivers/gpu/drm/drm_atomic.c
>@@ -783,6 +783,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> 		state->src_w = val;
> 	} else if (property == config->prop_src_h) {
> 		state->src_h = val;
>+	} else if (property == plane->blend_mode_property) {
>+		state->pixel_blend_mode = val;
> 	} else if (property == plane->rotation_property) {
> 		if (!is_power_of_2(val & DRM_MODE_ROTATE_MASK))
> 			return -EINVAL;
>@@ -848,6 +850,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> 		*val = state->src_w;
> 	} else if (property == config->prop_src_h) {
> 		*val = state->src_h;
>+	} else if (property == plane->blend_mode_property) {
>+		*val = state->pixel_blend_mode;
> 	} else if (property == plane->rotation_property) {
> 		*val = state->rotation;
> 	} else if (property == plane->zpos_property) {
>diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>index c356545..ddbd0d2 100644
>--- a/drivers/gpu/drm/drm_atomic_helper.c
>+++ b/drivers/gpu/drm/drm_atomic_helper.c
>@@ -3484,6 +3484,7 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
> 	if (plane->state) {
> 		plane->state->plane = plane;
> 		plane->state->rotation = DRM_MODE_ROTATE_0;
>+		plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> 	}
> }
> EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
>diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
>index 5a81e1b..4ac45da 100644
>--- a/drivers/gpu/drm/drm_blend.c
>+++ b/drivers/gpu/drm/drm_blend.c
>@@ -100,6 +100,41 @@
>  *	planes. Without this property the primary plane is always below the cursor
>  *	plane, and ordering between all other planes is undefined.
>  *
>+ * pixel blend mode:
>+ *	Pixel blend mode is set up with drm_plane_create_blend_mode_property().
>+ *	It adds a blend mode for alpha blending equation selection, describing
>+ *	how the pixels from the current plane are composited with the
>+ *	background.
>+ *
>+ *	Three alpha blending equations(note that the fg.rgb or bg.rgb notation
>+ *	means each of the R, G or B channels for the foreground and background
>+ *	colors, respectively):
>+ *
>+ *	"None": Blend formula that ignores the pixel alpha.
>+ *	out.rgb = plane_alpha * fg.rgb + (1 - plane_alpha) * bg.rgb
>+ *
>+ *	"Pre-multiplied": Blend formula that assumes the pixel color values
>+ *	have been already pre-multiplied with the alpha
>+ *	channel values.
>+ *	out.rgb = plane_alpha * fg.rgb + (1 - (plane_alpha * fg.alpha)) * bg.rgb
>+ *
>+ *	"Coverage": Blend formula that assumes the pixel color values have not
>+ *	been pre-multiplied and will do so when blending them to the background
>+ *	color values.
>+ *	out.rgb = plane_alpha * fg.alpha * fg.rgb +
>+ *		  (1 - (plane_alpha * fg.alpha)) * bg.rgb
>+ *
>+ *	fg.rgb: Each of the RGB component values from the plane's pixel
>+ *	fg.alpha: Alpha component value from the plane's pixel
>+ *	bg.rgb: Each of the RGB component values from the background
>+ *	plane_alpha: Plane alpha value set by the plane alpha property (if
>+ *		     applicable).
>+ *
>+ *	This property has no effect on formats with no pixel alpha, as fg.alpha
>+ *	is assumed to be 1.0. If the plane does not expose the "alpha" property,
>+ *	then plane_alpha is assumed to be 1.0, otherwise, it is the value of the
>+ *	"alpha" property.
>+ *

I did a little fiddling to make this render nicely in the
documentation output. If you like it, you can apply the hunk below:

-- >8 --

diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 714b0602127d..7b5c3307d4c1 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -113,34 +113,45 @@
   *	how the pixels from the current plane are composited with the
   *	background.
   *
- *	Three alpha blending equations(note that the fg.rgb or bg.rgb notation
- *	means each of the R, G or B channels for the foreground and background
- *	colors, respectively):
- *
- *	"None": Blend formula that ignores the pixel alpha.
- *	out.rgb = plane_alpha * fg.rgb + (1 - plane_alpha) * bg.rgb
- *
- *	"Pre-multiplied": Blend formula that assumes the pixel color values
- *	have been already pre-multiplied with the alpha
- *	channel values.
- *	out.rgb = plane_alpha * fg.rgb + (1 - (plane_alpha * fg.alpha)) * bg.rgb
- *
- *	"Coverage": Blend formula that assumes the pixel color values have not
- *	been pre-multiplied and will do so when blending them to the background
- *	color values.
- *	out.rgb = plane_alpha * fg.alpha * fg.rgb +
- *		  (1 - (plane_alpha * fg.alpha)) * bg.rgb
- *
- *	fg.rgb: Each of the RGB component values from the plane's pixel
- *	fg.alpha: Alpha component value from the plane's pixel
- *	bg.rgb: Each of the RGB component values from the background
- *	plane_alpha: Plane alpha value set by the plane alpha property (if
- *		     applicable).
- *
- *	This property has no effect on formats with no pixel alpha, as fg.alpha
- *	is assumed to be 1.0. If the plane does not expose the "alpha" property,
- *	then plane_alpha is assumed to be 1.0, otherwise, it is the value of the
- *	"alpha" property.
+ *	Three alpha blending equations are defined:
+ *
+ *	"None":
+ *		Blend formula that ignores the pixel alpha::
+ *
+ *			out.rgb = plane_alpha * fg.rgb +
+ *				(1 - plane_alpha) * bg.rgb
+ *
+ *	"Pre-multiplied":
+ *		Blend formula that assumes the pixel color values
+ *		have been already pre-multiplied with the alpha
+ *		channel values::
+ *
+ *			out.rgb = plane_alpha * fg.rgb +
+ *				(1 - (plane_alpha * fg.alpha)) * bg.rgb
+ *
+ *	"Coverage":
+ *		Blend formula that assumes the pixel color values have not
+ *		been pre-multiplied and will do so when blending them to the
+ *		background color values::
+ *
+ *			out.rgb = plane_alpha * fg.alpha * fg.rgb +
+ *				(1 - (plane_alpha * fg.alpha)) * bg.rgb
+ *
+ *	Using the following symbols:
+ *
+ *	``fg.rgb``:
+ *		Each of the RGB component values from the plane's pixel
+ *	``fg.alpha``:
+ *		Alpha component value from the plane's pixel. If the plane's
+ *		pixel format has no alpha component, then this is assumed to be
+ *		1.0. In these cases, this property has no effect, as all three
+ *		equations become equivalent.
+ *	``bg.rgb``:
+ *		Each of the RGB component values from the background
+ *	``plane_alpha``:
+ *		Plane alpha value set by the plane "alpha" property. If the
+ *		plane does not expose the "alpha" property, then this is
+ *		assumed to be 1.0
   *
   * Note that all the property extensions described here apply either to the
   * plane or the CRTC (e.g. for the background color, which currently is not

-- >8 --

>  * Note that all the property extensions described here apply either to the
>  * plane or the CRTC (e.g. for the background color, which currently is not
>  * exposed and assumed to be black).
>@@ -409,3 +444,78 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
> 	return 0;
> }
> EXPORT_SYMBOL(drm_atomic_normalize_zpos);
>+
>+/**
>+ * drm_plane_create_blend_mode_property - create a new blend mode property
>+ * @plane: drm plane
>+ * @supported_modes: bitmask of supported modes, must include
>+ *		     BIT(DRM_MODE_BLEND_PREMULTI)
>+ *
>+ * This creates a new property describing the blend mode.
>+ *
>+ * The property exposed to userspace is an enumeration property (see
>+ * drm_property_create_enum()) called "pixel blend mode" and has the
>+ * following enumeration values:
>+ *
>+ * "None": Blend formula that ignores the pixel alpha.
>+ *
>+ * "Pre-multiplied": Blend formula that assumes the pixel color values have
>+ *		     been already pre-multiplied with the alpha channel values.
>+ *
>+ * "Coverage": Blend formula that assumes the pixel color values have not been
>+ *	       pre-multiplied and will do so when blending them to the
>+ *	       background color values.

Similar thing here - if you put the descriptions (after the ":") on a
new line, and indent them one more level, then it renders much more
nicely:

 * "None":
 *	Blend formula that ignores the pixel alpha.

Thanks,
-Brian

>+ *
>+ * RETURNS:
>+ * Zero for success or -errno
>+ */
>+int drm_plane_create_blend_mode_property(struct drm_plane *plane,
>+					 unsigned int supported_modes)
>+{
>+	struct drm_device *dev = plane->dev;
>+	struct drm_property *prop;
>+	static const struct drm_prop_enum_list props[] = {
>+		{ DRM_MODE_BLEND_PIXEL_NONE, "None" },
>+		{ DRM_MODE_BLEND_PREMULTI, "Pre-multiplied" },
>+		{ DRM_MODE_BLEND_COVERAGE, "Coverage" },
>+	};
>+	unsigned int valid_mode_mask = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
>+				       BIT(DRM_MODE_BLEND_PREMULTI)   |
>+				       BIT(DRM_MODE_BLEND_COVERAGE);
>+	int i, j = 0;
>+
>+	if (WARN_ON((supported_modes & ~valid_mode_mask) ||
>+		    ((supported_modes & BIT(DRM_MODE_BLEND_PREMULTI)) == 0)))
>+		return -EINVAL;
>+
>+	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
>+				   "pixel blend mode",
>+				   hweight32(supported_modes));
>+	if (!prop)
>+		return -ENOMEM;
>+
>+	for (i = 0; i < ARRAY_SIZE(props); i++) {
>+		int ret;
>+
>+		if (!(BIT(props[i].type) & supported_modes))
>+			continue;
>+
>+		ret = drm_property_add_enum(prop, j++, props[i].type,
>+					    props[i].name);
>+
>+		if (ret) {
>+			drm_property_destroy(dev, prop);
>+
>+			return ret;
>+		}
>+	}
>+
>+	drm_object_attach_property(&plane->base, prop, DRM_MODE_BLEND_PREMULTI);
>+	plane->blend_mode_property = prop;
>+
>+	if (plane->state)
>+		plane->state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>+
>+	return 0;
>+}
>+EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
>diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
>index 1760602..2966c0d 100644
>--- a/include/drm/drm_blend.h
>+++ b/include/drm/drm_blend.h
>@@ -27,6 +27,10 @@
> #include <linux/ctype.h>
> #include <drm/drm_mode.h>
>
>+#define DRM_MODE_BLEND_PIXEL_NONE	0
>+#define DRM_MODE_BLEND_PREMULTI		1
>+#define DRM_MODE_BLEND_COVERAGE		2
>+
> struct drm_device;
> struct drm_atomic_state;
> struct drm_plane;
>@@ -49,4 +53,6 @@ int drm_plane_create_zpos_immutable_property(struct drm_plane *plane,
> 					     unsigned int zpos);
> int drm_atomic_normalize_zpos(struct drm_device *dev,
> 			      struct drm_atomic_state *state);
>+int drm_plane_create_blend_mode_property(struct drm_plane *plane,
>+					 unsigned int supported_modes);
> #endif
>diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>index f7bf4a4..447ebe7 100644
>--- a/include/drm/drm_plane.h
>+++ b/include/drm/drm_plane.h
>@@ -43,6 +43,8 @@
>  *	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)
>+ * @pixel_blend_mode: how the plane's framebuffer alpha channel is used when
>+ *	blending with the background colour.
>  * @rotation: rotation of the plane
>  * @zpos: priority of the given plane on crtc (optional)
>  *	Note that multiple active planes on the same crtc can have an identical
>@@ -106,6 +108,8 @@ struct drm_plane_state {
> 	uint32_t src_x, src_y;
> 	uint32_t src_h, src_w;
>
>+	uint16_t pixel_blend_mode;
>+
> 	/* Plane rotation */
> 	unsigned int rotation;
>
>@@ -498,6 +502,7 @@ enum drm_plane_type {
>  * @type: type of plane (overlay, primary, cursor)
>  * @zpos_property: zpos property for this plane
>  * @rotation_property: rotation property for this plane
>+ * @blend_mode_property: blend mode property for this plane
>  * @helper_private: mid-layer private data
>  */
> struct drm_plane {
>@@ -573,6 +578,7 @@ struct drm_plane {
>
> 	struct drm_property *zpos_property;
> 	struct drm_property *rotation_property;
>+	struct drm_property *blend_mode_property;
>
> 	/**
> 	 * @color_encoding_property:
>-- 
>1.9.1
>

Re: [PATCH v2 1/2] drm/blend: Add per-plane pixel blend mode property

[Maarten Lankhorst]

Hey,

Op 30-05-18 om 13:23 schreef Lowry Li:
> Pixel blend modes represent the alpha blending equation
> selection, describing how the pixels from the current
> plane are composited with the background.
>
> Add a pixel_blend_mode to drm_plane_state and a
> blend_mode_property to drm_plane, and related support
> functions.
>
> Defines three blend modes in drm_blend.h.
>
> Signed-off-by: Lowry Li <lowry.li@arm.com>
> ---
>  drivers/gpu/drm/drm_atomic.c        |   4 ++
>  drivers/gpu/drm/drm_atomic_helper.c |   1 +
>  drivers/gpu/drm/drm_blend.c         | 110 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_blend.h             |   6 ++
>  include/drm/drm_plane.h             |   6 ++
>  5 files changed, 127 insertions(+)
Can you rebase this on top of a kernel with alpha property support? Getting some nasty conflicts otherwise..

~Maarten

Re: [PATCH v2 1/2] drm/blend: Add per-plane pixel blend mode property

[Emil Velikov]

Hi Lowry,

Small drive-by suggestion. Haven't checked if others have pointed it
out previously :-\

On 30 May 2018 at 12:23, Lowry Li <lowry.li@arm.com> wrote:

> +/**
> + * drm_plane_create_blend_mode_property - create a new blend mode property
> + * @plane: drm plane
> + * @supported_modes: bitmask of supported modes, must include
> + *                  BIT(DRM_MODE_BLEND_PREMULTI)
> + *
There are two possible blend modes (ignoring 'none'), yet premult must
be supported.
What if the hardware can do only "coverage"?

One-liner explaining why things are as-is would be a great move.

HTH
Emil

[PATCH v2 2/2] drm/mali-dp: Implement plane alpha and pixel blend on malidp

[Lowry Li]

Check the pixel blending mode and plane alpha value when
do the plane_check. Mali DP supports blending the current plane
with the background either based on the pixel alpha blending
mode or by using the layer's alpha value, but not both at the
same time. If both case, plane_check will return failed.

Set the HW when doing plane_update accordingly. If plane alpha
is the 0xffff, set the PREM bit accordingly. If not we'd set
ALPHA bit as zero and layer alpha value.

Signed-off-by: Lowry Li <lowry.li@arm.com>
---
 drivers/gpu/drm/arm/malidp_planes.c | 76 +++++++++++++++++++++----------------
 1 file changed, 44 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 7a44897..daa3f4f 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -35,6 +35,7 @@
 #define   LAYER_COMP_MASK		(0x3 << 12)
 #define   LAYER_COMP_PIXEL		(0x3 << 12)
 #define   LAYER_COMP_PLANE		(0x2 << 12)
+#define   LAYER_PMUL_ENABLE		(0x1 << 14)
 #define   LAYER_ALPHA_OFFSET		(16)
 #define   LAYER_ALPHA_MASK		(0xff)
 #define   LAYER_ALPHA(x)		(((x) & LAYER_ALPHA_MASK) << LAYER_ALPHA_OFFSET)
@@ -182,6 +183,7 @@ static int malidp_de_plane_check(struct drm_plane *plane,
 	struct malidp_plane_state *ms = to_malidp_plane_state(state);
 	bool rotated = state->rotation & MALIDP_ROTATED_MASK;
 	struct drm_framebuffer *fb;
+	u16 pixel_alpha = state->pixel_blend_mode;
 	int i, ret;
 
 	if (!state->crtc || !state->fb)
@@ -244,6 +246,11 @@ static int malidp_de_plane_check(struct drm_plane *plane,
 		ms->rotmem_size = val;
 	}
 
+	/* HW can't support plane + pixel blending */
+	if ((state->alpha != DRM_BLEND_ALPHA_OPAQUE) &&
+	    (pixel_alpha != DRM_MODE_BLEND_PIXEL_NONE))
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -325,31 +332,33 @@ static void malidp_de_plane_update(struct drm_plane *plane,
 {
 	struct malidp_plane *mp;
 	struct malidp_plane_state *ms = to_malidp_plane_state(plane->state);
+	struct drm_plane_state *state = plane->state;
+	u16 pixel_alpha = state->pixel_blend_mode;
+	u8 plane_alpha = state->alpha >> 8;
 	u32 src_w, src_h, dest_w, dest_h, val;
 	int i;
-	bool format_has_alpha = plane->state->fb->format->has_alpha;
 
 	mp = to_malidp_plane(plane);
 
 	/* convert src values from Q16 fixed point to integer */
-	src_w = plane->state->src_w >> 16;
-	src_h = plane->state->src_h >> 16;
-	dest_w = plane->state->crtc_w;
-	dest_h = plane->state->crtc_h;
+	src_w = state->src_w >> 16;
+	src_h = state->src_h >> 16;
+	dest_w = state->crtc_w;
+	dest_h = state->crtc_h;
 
 	malidp_hw_write(mp->hwdev, ms->format, mp->layer->base);
 
 	for (i = 0; i < ms->n_planes; i++) {
 		/* calculate the offset for the layer's plane registers */
 		u16 ptr = mp->layer->ptr + (i << 4);
-		dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(plane->state->fb,
-							     plane->state, i);
+		dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(state->fb,
+							     state, i);
 
 		malidp_hw_write(mp->hwdev, lower_32_bits(fb_addr), ptr);
 		malidp_hw_write(mp->hwdev, upper_32_bits(fb_addr), ptr + 4);
 	}
 	malidp_de_set_plane_pitches(mp, ms->n_planes,
-				    plane->state->fb->pitches);
+				    state->fb->pitches);
 
 	if ((plane->state->color_encoding != old_state->color_encoding) ||
 	    (plane->state->color_range != old_state->color_range))
@@ -362,8 +371,8 @@ static void malidp_de_plane_update(struct drm_plane *plane,
 	malidp_hw_write(mp->hwdev, LAYER_H_VAL(dest_w) | LAYER_V_VAL(dest_h),
 			mp->layer->base + MALIDP_LAYER_COMP_SIZE);
 
-	malidp_hw_write(mp->hwdev, LAYER_H_VAL(plane->state->crtc_x) |
-			LAYER_V_VAL(plane->state->crtc_y),
+	malidp_hw_write(mp->hwdev, LAYER_H_VAL(state->crtc_x) |
+			LAYER_V_VAL(state->crtc_y),
 			mp->layer->base + MALIDP_LAYER_OFFSET);
 
 	if (mp->layer->id == DE_SMART)
@@ -376,38 +385,35 @@ static void malidp_de_plane_update(struct drm_plane *plane,
 	val &= ~LAYER_ROT_MASK;
 
 	/* setup the rotation and axis flip bits */
-	if (plane->state->rotation & DRM_MODE_ROTATE_MASK)
+	if (state->rotation & DRM_MODE_ROTATE_MASK)
 		val |= ilog2(plane->state->rotation & DRM_MODE_ROTATE_MASK) <<
 		       LAYER_ROT_OFFSET;
-	if (plane->state->rotation & DRM_MODE_REFLECT_X)
+	if (state->rotation & DRM_MODE_REFLECT_X)
 		val |= LAYER_H_FLIP;
-	if (plane->state->rotation & DRM_MODE_REFLECT_Y)
+	if (state->rotation & DRM_MODE_REFLECT_Y)
 		val |= LAYER_V_FLIP;
 
-	val &= ~LAYER_COMP_MASK;
-	if (format_has_alpha) {
-
-		/*
-		 * always enable pixel alpha blending until we have a way
-		 * to change blend modes
-		 */
-		val |= LAYER_COMP_PIXEL;
-	} else {
-
-		/*
-		 * do not enable pixel alpha blending as the color channel
-		 * does not have any alpha information
-		 */
-		val |= LAYER_COMP_PLANE;
-
-		/* Set layer alpha coefficient to 0xff ie fully opaque */
+	val &= ~(LAYER_COMP_MASK | LAYER_PMUL_ENABLE);
+
+	if (state->alpha != DRM_BLEND_ALPHA_OPAQUE) {
+		val |= LAYER_COMP_PLANE | LAYER_ALPHA(plane_alpha);
+	} else if (state->fb->format->has_alpha) {
+		/* We only care about blend mode if the format has alpha */
+		switch (pixel_alpha) {
+		case DRM_MODE_BLEND_PREMULTI:
+			val |= LAYER_COMP_PIXEL | LAYER_PMUL_ENABLE;
+			break;
+		case DRM_MODE_BLEND_COVERAGE:
+			val |= LAYER_COMP_PIXEL;
+			break;
+		}
 		val |= LAYER_ALPHA(0xff);
 	}
 
 	val &= ~LAYER_FLOWCFG(LAYER_FLOWCFG_MASK);
-	if (plane->state->crtc) {
+	if (state->crtc) {
 		struct malidp_crtc_state *m =
-			to_malidp_crtc_state(plane->state->crtc->state);
+			to_malidp_crtc_state(state->crtc->state);
 
 		if (m->scaler_config.scale_enable &&
 		    m->scaler_config.plane_src_id == mp->layer->id)
@@ -446,6 +452,9 @@ int malidp_de_planes_init(struct drm_device *drm)
 	unsigned long crtcs = 1 << drm->mode_config.num_crtc;
 	unsigned long flags = DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_180 |
 			      DRM_MODE_ROTATE_270 | DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y;
+	unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
+				  BIT(DRM_MODE_BLEND_PREMULTI)   |
+				  BIT(DRM_MODE_BLEND_COVERAGE);
 	u32 *formats;
 	int ret, i, j, n;
 
@@ -498,6 +507,9 @@ int malidp_de_planes_init(struct drm_device *drm)
 		malidp_hw_write(malidp->dev, MALIDP_ALPHA_LUT,
 				plane->layer->base + MALIDP_LAYER_COMPOSE);
 
+		drm_plane_create_alpha_property(&plane->base);
+		drm_plane_create_blend_mode_property(&plane->base, blend_caps);
+
 		/* Attach the YUV->RGB property only to video layers */
 		if (id & (DE_VIDEO1 | DE_VIDEO2)) {
 			/* default encoding for YUV->RGB is BT601 NARROW */
-- 
1.9.1

Re: [PATCH v2 2/2] drm/mali-dp: Implement plane alpha and pixel blend on malidp

[Brian Starkey]

Hi Lowry,

On Wed, May 30, 2018 at 07:23:54PM +0800, Lowry Li wrote:
>Check the pixel blending mode and plane alpha value when
>do the plane_check. Mali DP supports blending the current plane
>with the background either based on the pixel alpha blending
>mode or by using the layer's alpha value, but not both at the
>same time. If both case, plane_check will return failed.
>
>Set the HW when doing plane_update accordingly. If plane alpha
>is the 0xffff, set the PREM bit accordingly. If not we'd set
>ALPHA bit as zero and layer alpha value.

The code looks correct - but the description of the PREM bit seems
wrong here. Did you mean "set the pixel blending bits accordingly"?

With that fixed:

Reviewed-by: Brian Starkey <brian.starkey@arm.com>

>
>Signed-off-by: Lowry Li <lowry.li@arm.com>
>---
> drivers/gpu/drm/arm/malidp_planes.c | 76 +++++++++++++++++++++----------------
> 1 file changed, 44 insertions(+), 32 deletions(-)
>
>diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
>index 7a44897..daa3f4f 100644
>--- a/drivers/gpu/drm/arm/malidp_planes.c
>+++ b/drivers/gpu/drm/arm/malidp_planes.c
>@@ -35,6 +35,7 @@
> #define   LAYER_COMP_MASK		(0x3 << 12)
> #define   LAYER_COMP_PIXEL		(0x3 << 12)
> #define   LAYER_COMP_PLANE		(0x2 << 12)
>+#define   LAYER_PMUL_ENABLE		(0x1 << 14)
> #define   LAYER_ALPHA_OFFSET		(16)
> #define   LAYER_ALPHA_MASK		(0xff)
> #define   LAYER_ALPHA(x)		(((x) & LAYER_ALPHA_MASK) << LAYER_ALPHA_OFFSET)
>@@ -182,6 +183,7 @@ static int malidp_de_plane_check(struct drm_plane *plane,
> 	struct malidp_plane_state *ms = to_malidp_plane_state(state);
> 	bool rotated = state->rotation & MALIDP_ROTATED_MASK;
> 	struct drm_framebuffer *fb;
>+	u16 pixel_alpha = state->pixel_blend_mode;
> 	int i, ret;
>
> 	if (!state->crtc || !state->fb)
>@@ -244,6 +246,11 @@ static int malidp_de_plane_check(struct drm_plane *plane,
> 		ms->rotmem_size = val;
> 	}
>
>+	/* HW can't support plane + pixel blending */
>+	if ((state->alpha != DRM_BLEND_ALPHA_OPAQUE) &&
>+	    (pixel_alpha != DRM_MODE_BLEND_PIXEL_NONE))
>+		return -EINVAL;
>+
> 	return 0;
> }
>
>@@ -325,31 +332,33 @@ static void malidp_de_plane_update(struct drm_plane *plane,
> {
> 	struct malidp_plane *mp;
> 	struct malidp_plane_state *ms = to_malidp_plane_state(plane->state);
>+	struct drm_plane_state *state = plane->state;
>+	u16 pixel_alpha = state->pixel_blend_mode;
>+	u8 plane_alpha = state->alpha >> 8;
> 	u32 src_w, src_h, dest_w, dest_h, val;
> 	int i;
>-	bool format_has_alpha = plane->state->fb->format->has_alpha;
>
> 	mp = to_malidp_plane(plane);
>
> 	/* convert src values from Q16 fixed point to integer */
>-	src_w = plane->state->src_w >> 16;
>-	src_h = plane->state->src_h >> 16;
>-	dest_w = plane->state->crtc_w;
>-	dest_h = plane->state->crtc_h;
>+	src_w = state->src_w >> 16;
>+	src_h = state->src_h >> 16;
>+	dest_w = state->crtc_w;
>+	dest_h = state->crtc_h;
>
> 	malidp_hw_write(mp->hwdev, ms->format, mp->layer->base);
>
> 	for (i = 0; i < ms->n_planes; i++) {
> 		/* calculate the offset for the layer's plane registers */
> 		u16 ptr = mp->layer->ptr + (i << 4);
>-		dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(plane->state->fb,
>-							     plane->state, i);
>+		dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(state->fb,
>+							     state, i);
>
> 		malidp_hw_write(mp->hwdev, lower_32_bits(fb_addr), ptr);
> 		malidp_hw_write(mp->hwdev, upper_32_bits(fb_addr), ptr + 4);
> 	}
> 	malidp_de_set_plane_pitches(mp, ms->n_planes,
>-				    plane->state->fb->pitches);
>+				    state->fb->pitches);
>
> 	if ((plane->state->color_encoding != old_state->color_encoding) ||
> 	    (plane->state->color_range != old_state->color_range))
>@@ -362,8 +371,8 @@ static void malidp_de_plane_update(struct drm_plane *plane,
> 	malidp_hw_write(mp->hwdev, LAYER_H_VAL(dest_w) | LAYER_V_VAL(dest_h),
> 			mp->layer->base + MALIDP_LAYER_COMP_SIZE);
>
>-	malidp_hw_write(mp->hwdev, LAYER_H_VAL(plane->state->crtc_x) |
>-			LAYER_V_VAL(plane->state->crtc_y),
>+	malidp_hw_write(mp->hwdev, LAYER_H_VAL(state->crtc_x) |
>+			LAYER_V_VAL(state->crtc_y),
> 			mp->layer->base + MALIDP_LAYER_OFFSET);
>
> 	if (mp->layer->id == DE_SMART)
>@@ -376,38 +385,35 @@ static void malidp_de_plane_update(struct drm_plane *plane,
> 	val &= ~LAYER_ROT_MASK;
>
> 	/* setup the rotation and axis flip bits */
>-	if (plane->state->rotation & DRM_MODE_ROTATE_MASK)
>+	if (state->rotation & DRM_MODE_ROTATE_MASK)
> 		val |= ilog2(plane->state->rotation & DRM_MODE_ROTATE_MASK) <<
> 		       LAYER_ROT_OFFSET;
>-	if (plane->state->rotation & DRM_MODE_REFLECT_X)
>+	if (state->rotation & DRM_MODE_REFLECT_X)
> 		val |= LAYER_H_FLIP;
>-	if (plane->state->rotation & DRM_MODE_REFLECT_Y)
>+	if (state->rotation & DRM_MODE_REFLECT_Y)
> 		val |= LAYER_V_FLIP;
>
>-	val &= ~LAYER_COMP_MASK;
>-	if (format_has_alpha) {
>-
>-		/*
>-		 * always enable pixel alpha blending until we have a way
>-		 * to change blend modes
>-		 */
>-		val |= LAYER_COMP_PIXEL;
>-	} else {
>-
>-		/*
>-		 * do not enable pixel alpha blending as the color channel
>-		 * does not have any alpha information
>-		 */
>-		val |= LAYER_COMP_PLANE;
>-
>-		/* Set layer alpha coefficient to 0xff ie fully opaque */
>+	val &= ~(LAYER_COMP_MASK | LAYER_PMUL_ENABLE);
>+
>+	if (state->alpha != DRM_BLEND_ALPHA_OPAQUE) {
>+		val |= LAYER_COMP_PLANE | LAYER_ALPHA(plane_alpha);
>+	} else if (state->fb->format->has_alpha) {
>+		/* We only care about blend mode if the format has alpha */
>+		switch (pixel_alpha) {
>+		case DRM_MODE_BLEND_PREMULTI:
>+			val |= LAYER_COMP_PIXEL | LAYER_PMUL_ENABLE;
>+			break;
>+		case DRM_MODE_BLEND_COVERAGE:
>+			val |= LAYER_COMP_PIXEL;
>+			break;
>+		}
> 		val |= LAYER_ALPHA(0xff);
> 	}
>
> 	val &= ~LAYER_FLOWCFG(LAYER_FLOWCFG_MASK);
>-	if (plane->state->crtc) {
>+	if (state->crtc) {
> 		struct malidp_crtc_state *m =
>-			to_malidp_crtc_state(plane->state->crtc->state);
>+			to_malidp_crtc_state(state->crtc->state);
>
> 		if (m->scaler_config.scale_enable &&
> 		    m->scaler_config.plane_src_id == mp->layer->id)
>@@ -446,6 +452,9 @@ int malidp_de_planes_init(struct drm_device *drm)
> 	unsigned long crtcs = 1 << drm->mode_config.num_crtc;
> 	unsigned long flags = DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_180 |
> 			      DRM_MODE_ROTATE_270 | DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y;
>+	unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
>+				  BIT(DRM_MODE_BLEND_PREMULTI)   |
>+				  BIT(DRM_MODE_BLEND_COVERAGE);
> 	u32 *formats;
> 	int ret, i, j, n;
>
>@@ -498,6 +507,9 @@ int malidp_de_planes_init(struct drm_device *drm)
> 		malidp_hw_write(malidp->dev, MALIDP_ALPHA_LUT,
> 				plane->layer->base + MALIDP_LAYER_COMPOSE);
>
>+		drm_plane_create_alpha_property(&plane->base);
>+		drm_plane_create_blend_mode_property(&plane->base, blend_caps);
>+
> 		/* Attach the YUV->RGB property only to video layers */
> 		if (id & (DE_VIDEO1 | DE_VIDEO2)) {
> 			/* default encoding for YUV->RGB is BT601 NARROW */
>-- 
>1.9.1
>

Re: [PATCH v2 2/2] drm/mali-dp: Implement plane alpha and pixel blend on malidp

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 11675 bytes --]

Hi Lowry,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.17-rc7]
[cannot apply to next-20180531]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Lowry-Li/drm-blend-Add-per-plane-pixel-blend-mode-property/20180601-180033
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/arm/malidp_planes.c: In function 'malidp_de_plane_check':
>> drivers/gpu/drm/arm/malidp_planes.c:250:12: error: 'struct drm_plane_state' has no member named 'alpha'
     if ((state->alpha != DRM_BLEND_ALPHA_OPAQUE) &&
               ^~
>> drivers/gpu/drm/arm/malidp_planes.c:250:23: error: 'DRM_BLEND_ALPHA_OPAQUE' undeclared (first use in this function)
     if ((state->alpha != DRM_BLEND_ALPHA_OPAQUE) &&
                          ^~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/arm/malidp_planes.c:250:23: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/arm/malidp_planes.c: In function 'malidp_de_plane_update':
   drivers/gpu/drm/arm/malidp_planes.c:337:24: error: 'struct drm_plane_state' has no member named 'alpha'
     u8 plane_alpha = state->alpha >> 8;
                           ^~
   drivers/gpu/drm/arm/malidp_planes.c:398:11: error: 'struct drm_plane_state' has no member named 'alpha'
     if (state->alpha != DRM_BLEND_ALPHA_OPAQUE) {
              ^~
   drivers/gpu/drm/arm/malidp_planes.c:398:22: error: 'DRM_BLEND_ALPHA_OPAQUE' undeclared (first use in this function)
     if (state->alpha != DRM_BLEND_ALPHA_OPAQUE) {
                         ^~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/arm/malidp_planes.c: In function 'malidp_de_planes_init':
>> drivers/gpu/drm/arm/malidp_planes.c:510:3: error: implicit declaration of function 'drm_plane_create_alpha_property'; did you mean 'drm_plane_create_zpos_property'? [-Werror=implicit-function-declaration]
      drm_plane_create_alpha_property(&plane->base);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      drm_plane_create_zpos_property
   cc1: some warnings being treated as errors

vim +250 drivers/gpu/drm/arm/malidp_planes.c

   178	
   179	static int malidp_de_plane_check(struct drm_plane *plane,
   180					 struct drm_plane_state *state)
   181	{
   182		struct malidp_plane *mp = to_malidp_plane(plane);
   183		struct malidp_plane_state *ms = to_malidp_plane_state(state);
   184		bool rotated = state->rotation & MALIDP_ROTATED_MASK;
   185		struct drm_framebuffer *fb;
   186		u16 pixel_alpha = state->pixel_blend_mode;
   187		int i, ret;
   188	
   189		if (!state->crtc || !state->fb)
   190			return 0;
   191	
   192		fb = state->fb;
   193	
   194		ms->format = malidp_hw_get_format_id(&mp->hwdev->hw->map,
   195						     mp->layer->id,
   196						     fb->format->format);
   197		if (ms->format == MALIDP_INVALID_FORMAT_ID)
   198			return -EINVAL;
   199	
   200		ms->n_planes = fb->format->num_planes;
   201		for (i = 0; i < ms->n_planes; i++) {
   202			u8 alignment = malidp_hw_get_pitch_align(mp->hwdev, rotated);
   203			if (fb->pitches[i] & (alignment - 1)) {
   204				DRM_DEBUG_KMS("Invalid pitch %u for plane %d\n",
   205					      fb->pitches[i], i);
   206				return -EINVAL;
   207			}
   208		}
   209	
   210		if ((state->crtc_w > mp->hwdev->max_line_size) ||
   211		    (state->crtc_h > mp->hwdev->max_line_size) ||
   212		    (state->crtc_w < mp->hwdev->min_line_size) ||
   213		    (state->crtc_h < mp->hwdev->min_line_size))
   214			return -EINVAL;
   215	
   216		/*
   217		 * DP550/650 video layers can accept 3 plane formats only if
   218		 * fb->pitches[1] == fb->pitches[2] since they don't have a
   219		 * third plane stride register.
   220		 */
   221		if (ms->n_planes == 3 &&
   222		    !(mp->hwdev->hw->features & MALIDP_DEVICE_LV_HAS_3_STRIDES) &&
   223		    (state->fb->pitches[1] != state->fb->pitches[2]))
   224			return -EINVAL;
   225	
   226		ret = malidp_se_check_scaling(mp, state);
   227		if (ret)
   228			return ret;
   229	
   230		/* packed RGB888 / BGR888 can't be rotated or flipped */
   231		if (state->rotation != DRM_MODE_ROTATE_0 &&
   232		    (fb->format->format == DRM_FORMAT_RGB888 ||
   233		     fb->format->format == DRM_FORMAT_BGR888))
   234			return -EINVAL;
   235	
   236		ms->rotmem_size = 0;
   237		if (state->rotation & MALIDP_ROTATED_MASK) {
   238			int val;
   239	
   240			val = mp->hwdev->hw->rotmem_required(mp->hwdev, state->crtc_h,
   241							     state->crtc_w,
   242							     fb->format->format);
   243			if (val < 0)
   244				return val;
   245	
   246			ms->rotmem_size = val;
   247		}
   248	
   249		/* HW can't support plane + pixel blending */
 > 250		if ((state->alpha != DRM_BLEND_ALPHA_OPAQUE) &&
   251		    (pixel_alpha != DRM_MODE_BLEND_PIXEL_NONE))
   252			return -EINVAL;
   253	
   254		return 0;
   255	}
   256	
   257	static void malidp_de_set_plane_pitches(struct malidp_plane *mp,
   258						int num_planes, unsigned int pitches[3])
   259	{
   260		int i;
   261		int num_strides = num_planes;
   262	
   263		if (!mp->layer->stride_offset)
   264			return;
   265	
   266		if (num_planes == 3)
   267			num_strides = (mp->hwdev->hw->features &
   268				       MALIDP_DEVICE_LV_HAS_3_STRIDES) ? 3 : 2;
   269	
   270		for (i = 0; i < num_strides; ++i)
   271			malidp_hw_write(mp->hwdev, pitches[i],
   272					mp->layer->base +
   273					mp->layer->stride_offset + i * 4);
   274	}
   275	
   276	static const s16
   277	malidp_yuv2rgb_coeffs[][DRM_COLOR_RANGE_MAX][MALIDP_COLORADJ_NUM_COEFFS] = {
   278		[DRM_COLOR_YCBCR_BT601][DRM_COLOR_YCBCR_LIMITED_RANGE] = {
   279			1192,    0, 1634,
   280			1192, -401, -832,
   281			1192, 2066,    0,
   282			  64,  512,  512
   283		},
   284		[DRM_COLOR_YCBCR_BT601][DRM_COLOR_YCBCR_FULL_RANGE] = {
   285			1024,    0, 1436,
   286			1024, -352, -731,
   287			1024, 1815,    0,
   288			   0,  512,  512
   289		},
   290		[DRM_COLOR_YCBCR_BT709][DRM_COLOR_YCBCR_LIMITED_RANGE] = {
   291			1192,    0, 1836,
   292			1192, -218, -546,
   293			1192, 2163,    0,
   294			  64,  512,  512
   295		},
   296		[DRM_COLOR_YCBCR_BT709][DRM_COLOR_YCBCR_FULL_RANGE] = {
   297			1024,    0, 1613,
   298			1024, -192, -479,
   299			1024, 1900,    0,
   300			   0,  512,  512
   301		},
   302		[DRM_COLOR_YCBCR_BT2020][DRM_COLOR_YCBCR_LIMITED_RANGE] = {
   303			1024,    0, 1476,
   304			1024, -165, -572,
   305			1024, 1884,    0,
   306			   0,  512,  512
   307		},
   308		[DRM_COLOR_YCBCR_BT2020][DRM_COLOR_YCBCR_FULL_RANGE] = {
   309			1024,    0, 1510,
   310			1024, -168, -585,
   311			1024, 1927,    0,
   312			   0,  512,  512
   313		}
   314	};
   315	
   316	static void malidp_de_set_color_encoding(struct malidp_plane *plane,
   317						 enum drm_color_encoding enc,
   318						 enum drm_color_range range)
   319	{
   320		unsigned int i;
   321	
   322		for (i = 0; i < MALIDP_COLORADJ_NUM_COEFFS; i++) {
   323			/* coefficients are signed, two's complement values */
   324			malidp_hw_write(plane->hwdev, malidp_yuv2rgb_coeffs[enc][range][i],
   325					plane->layer->base + plane->layer->yuv2rgb_offset +
   326					i * 4);
   327		}
   328	}
   329	
   330	static void malidp_de_plane_update(struct drm_plane *plane,
   331					   struct drm_plane_state *old_state)
   332	{
   333		struct malidp_plane *mp;
   334		struct malidp_plane_state *ms = to_malidp_plane_state(plane->state);
   335		struct drm_plane_state *state = plane->state;
   336		u16 pixel_alpha = state->pixel_blend_mode;
   337		u8 plane_alpha = state->alpha >> 8;
   338		u32 src_w, src_h, dest_w, dest_h, val;
   339		int i;
   340	
   341		mp = to_malidp_plane(plane);
   342	
   343		/* convert src values from Q16 fixed point to integer */
   344		src_w = state->src_w >> 16;
   345		src_h = state->src_h >> 16;
   346		dest_w = state->crtc_w;
   347		dest_h = state->crtc_h;
   348	
   349		malidp_hw_write(mp->hwdev, ms->format, mp->layer->base);
   350	
   351		for (i = 0; i < ms->n_planes; i++) {
   352			/* calculate the offset for the layer's plane registers */
   353			u16 ptr = mp->layer->ptr + (i << 4);
   354			dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(state->fb,
   355								     state, i);
   356	
   357			malidp_hw_write(mp->hwdev, lower_32_bits(fb_addr), ptr);
   358			malidp_hw_write(mp->hwdev, upper_32_bits(fb_addr), ptr + 4);
   359		}
   360		malidp_de_set_plane_pitches(mp, ms->n_planes,
   361					    state->fb->pitches);
   362	
   363		if ((plane->state->color_encoding != old_state->color_encoding) ||
   364		    (plane->state->color_range != old_state->color_range))
   365			malidp_de_set_color_encoding(mp, plane->state->color_encoding,
   366						     plane->state->color_range);
   367	
   368		malidp_hw_write(mp->hwdev, LAYER_H_VAL(src_w) | LAYER_V_VAL(src_h),
   369				mp->layer->base + MALIDP_LAYER_SIZE);
   370	
   371		malidp_hw_write(mp->hwdev, LAYER_H_VAL(dest_w) | LAYER_V_VAL(dest_h),
   372				mp->layer->base + MALIDP_LAYER_COMP_SIZE);
   373	
   374		malidp_hw_write(mp->hwdev, LAYER_H_VAL(state->crtc_x) |
   375				LAYER_V_VAL(state->crtc_y),
   376				mp->layer->base + MALIDP_LAYER_OFFSET);
   377	
   378		if (mp->layer->id == DE_SMART)
   379			malidp_hw_write(mp->hwdev,
   380					LAYER_H_VAL(src_w) | LAYER_V_VAL(src_h),
   381					mp->layer->base + MALIDP550_LS_R1_IN_SIZE);
   382	
   383		/* first clear the rotation bits */
   384		val = malidp_hw_read(mp->hwdev, mp->layer->base + MALIDP_LAYER_CONTROL);
   385		val &= ~LAYER_ROT_MASK;
   386	
   387		/* setup the rotation and axis flip bits */
   388		if (state->rotation & DRM_MODE_ROTATE_MASK)
   389			val |= ilog2(plane->state->rotation & DRM_MODE_ROTATE_MASK) <<
   390			       LAYER_ROT_OFFSET;
   391		if (state->rotation & DRM_MODE_REFLECT_X)
   392			val |= LAYER_H_FLIP;
   393		if (state->rotation & DRM_MODE_REFLECT_Y)
   394			val |= LAYER_V_FLIP;
   395	
   396		val &= ~(LAYER_COMP_MASK | LAYER_PMUL_ENABLE);
   397	
 > 398		if (state->alpha != DRM_BLEND_ALPHA_OPAQUE) {
   399			val |= LAYER_COMP_PLANE | LAYER_ALPHA(plane_alpha);
   400		} else if (state->fb->format->has_alpha) {
   401			/* We only care about blend mode if the format has alpha */
   402			switch (pixel_alpha) {
   403			case DRM_MODE_BLEND_PREMULTI:
   404				val |= LAYER_COMP_PIXEL | LAYER_PMUL_ENABLE;
   405				break;
   406			case DRM_MODE_BLEND_COVERAGE:
   407				val |= LAYER_COMP_PIXEL;
   408				break;
   409			}
   410			val |= LAYER_ALPHA(0xff);
   411		}
   412	
   413		val &= ~LAYER_FLOWCFG(LAYER_FLOWCFG_MASK);
   414		if (state->crtc) {
   415			struct malidp_crtc_state *m =
   416				to_malidp_crtc_state(state->crtc->state);
   417	
   418			if (m->scaler_config.scale_enable &&
   419			    m->scaler_config.plane_src_id == mp->layer->id)
   420				val |= LAYER_FLOWCFG(LAYER_FLOWCFG_SCALE_SE);
   421		}
   422	
   423		/* set the 'enable layer' bit */
   424		val |= LAYER_ENABLE;
   425	
   426		malidp_hw_write(mp->hwdev, val,
   427				mp->layer->base + MALIDP_LAYER_CONTROL);
   428	}
   429	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59076 bytes --]

Re: [PATCH v2 0/2] drm/blend: Add per-plane pixel blend mode property

[Sean Paul]

On Wed, May 30, 2018 at 07:23:52PM +0800, Lowry Li wrote:
> Hi,
> 
> This serie aims at adding the support for pixel blend modes represent the
> alpha blending equation selection in the driver. It also introduces to use
> it in the malidp driver.
> 
> Let me know what you think,

Hi Lowry,
Thank you for doing this work. I know this is something that is missing for
proper Android support, so it's most welcome.

Do you have userspace patches using this property?

Sean


> Lowry
> 
> Changes for v2:
>  - Moves the blending equation into the DOC comment
>  - Refines the comments of drm_plane_create_blend_mode_property to not
>    enumerate the #defines, but instead the string values
>  - Uses fg.* instead of pixel.* and plane_alpha instead of plane.alpha
>  - Introduces to use it in the malidp driver, which depends on the plane
>    alpha patch
> 
> Changes from v1:
>  - v1 is just the core changes to request for commments
>  - Adds a pixel_blend_mode to drm_plane_state and a blend_mode_property to
>    drm_plane, and related support functions
>  - Defines three blend modes in drm_blend.h
>  - Rebased on current drm-next
> 
> Lowry Li (2):
>   drm/blend: Add per-plane pixel blend mode property
>   drm/mali-dp: Implement plane alpha and pixel blend on malidp
> 
>  drivers/gpu/drm/arm/malidp_planes.c |  76 ++++++++++++++-----------
>  drivers/gpu/drm/drm_atomic.c        |   4 ++
>  drivers/gpu/drm/drm_atomic_helper.c |   1 +
>  drivers/gpu/drm/drm_blend.c         | 110 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_blend.h             |   6 ++
>  include/drm/drm_plane.h             |   6 ++
>  6 files changed, 171 insertions(+), 32 deletions(-)
> 
> -- 
> 1.9.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS