[PATCH/RFC 0/4] Implement standard color keying properties

[PATCH/RFC 0/4] Implement standard color keying properties

[Laurent Pinchart]

Hello,

This patch series is an attempt at implementing standard properties for color
keying support in the KMS API.

Before designing the API proposal I've analyzed the KMS drivers that support
color keying in the upstream kernel. Part of the explanation below was
initially posted in a reply to "[PATCH v2 0/2] rcar-du, vsp1: rcar-gen3: Add
support for colorkey alpha blending" and is copied here to continue the
discussion.

The armada, nouveau and rcar-du drivers expose the color key through DRM
properties. The i915 and vmwgfx drivers use custom ioctls. Here is how they
currently implement color keying.

- armada

"colorkey" range  0x00000000 0x00ffffff
"colorkey_min" range  0x00000000 0x00ffffff
"colorkey_max" range  0x00000000 0x00ffffff
"colorkey_val" range  0x00000000 0x00ffffff
"colorkey_alpha" range  0x00000000 0x00ffffff
"colorkey_mode" enum "disable", "Y component", "U component", "V component", 
"RGB", "R component", "G component", "B component"

All range properties store a RGB888 or YUV888 triplet.

The min and max properties store the comparison ranges. When a match occurs 
for one of the components, the value and alpha from the val and alpha 
properties replace the pixel. It's not clear which of the alpha "components" 
is used when a match occurs in RGB mode.

The colorkey property is a shortcut that stores identical values in min, max 
and val and 0 in alpha.

- i915

#define I915_SET_COLORKEY_NONE          (1<<0)
#define I915_SET_COLORKEY_DESTINATION   (1<<1)
#define I915_SET_COLORKEY_SOURCE        (1<<2)

struct drm_intel_sprite_colorkey {
        __u32 plane_id;
        __u32 min_value;
        __u32 channel_mask;
        __u32 max_value;
        __u32 flags;
};

- nouveau

"colorkey" range 0x00000000 0x01ffffff

The format isn't documented but it seems from the source code that bits 23:0 
store the color key value (written directly to a register, so possibly in a 
pixel format-dependent format) and bit 24 enables color keying.

- rcar-du

"colorkey" range 0x00000000 0x01ffffff

Bits 23:0 store the color key value in RGB888 (regardless of the pixel format 
of the plane) and bit 24 enables color keying. This supports Gen2 hardware 
only, where the only available more is  exact match. The pixel then becomes 
fully transparent (the hardware doesn't support custom target alpha values).

On Gen3 hardware color keying can be performed in exact RGB match, exact Y 
match or range Y match (only the max value is programmable, the min value is 
always 0). Furthermore in exact match modes the hardware can operate with a 
single match value, in which case it can then override the full ARGB or AYUV 
pixel, or double match value, in which case it can then override the alpha 
component only, but with two distinct match values each associated with a 
target alpha.

- vmwgfx

struct drm_vmw_control_stream_arg {
        __u32 stream_id;
        __u32 enabled;

        __u32 flags;
        __u32 color_key;

        __u32 handle;
        __u32 offset;
        __s32 format;
        __u32 size;
        __u32 width;
        __u32 height;
        __u32 pitch[3];

        __u32 pad64;
        struct drm_vmw_rect src;
        struct drm_vmw_rect dst;
};

The color_key field isn't documented, but the following (unused) macros hint 
that it could store an RGB888, with the color key feature enabled through the 
flags field.

#define SVGA_VIDEO_FLAG_COLORKEY        0x0001
#define SVGA_VIDEO_COLORKEY_MASK             0x00ffffff

Looking at these drivers we can already see that the hardware implementations
differ quite widely. There are however similarities, and we could express most
of the above features through a set of generic properties similar to the ones
already implemented by the armada driver. This is what the patch series
attempts to do.

- The match range can be set through minimum and maximum properties. Drivers
that support exact match only simply report an error when minimum != maximum.

- The replacement value can be set through a value property. The property
stores both the pixel value (RGB or YUV) and the alpha value Bits that are not
applicable are ignored (for instance RGB/YUV bits when the driver supports
alpha replacement only). If programmable color replacement isn't supported (as
in the R-Car Gen2 example above) the property is omitted.

- The mode can be set through a mode property. Enabling color keying through
one bit in a color property (like done by the nouveau and rcar-du drivers) is 
a hack and I don't think we should carry it forward. A mode property allows
configuring source or destination color keying.

- Part of the mode information could be deduced automatically without a need
to specify it explicitly. For instance RGB/YUV mode can be configured based on
the pixel format of the plane. Similarly, exact match vs. range match can be
configured based on whether the minimum and maximum value differ.

- The modes exposed through the mode property are left as driver-specific in
this RFC, with one "disabled" mode mandatory for all implementations. The
rationale is that a generic userspace should be able to disable color keying,
but that hardware features vary too much to standardize all modes. I'm however
starting to think that we should standardize more modes than "disabled", but I
still need to sleep over this particular issue. Ideas and comments are
welcome.

- Properties that store pixel values store them in a fixed AXYZ16161616 format
where A is the alpha value and XYZ color components that correspond to the
plane pixel format (usually RGB or YUV).

- We need to keep the existing "colorkey" properties implemented by armada, 
nouveau and rcar-du for backward compatibility reasons, but this proposed API 
doesn't require a "colorkey" property. I have reimplemented the existing
"colorkey" in the rcar-du driver as an alias for the new standard properties
to show how it can be done in a driver.


The R-Car Gen3 dual target mode feature doesn't fit in the properties proposed
here. I have no use case for that mode at the moment but I'm fairly certain
that someone will come up with one, at least if not for the R-Car for a
different display engine that provides similarly exotic features. I believe
this can for now be left for implementation through driver-specific
properties.

Alexandru Gheorghe (1):
  v4l: vsp1: Add support for colorkey alpha blending

Laurent Pinchart (3):
  drm: Add colorkey properties
  drm: rcar-du: Use standard colorkey properties
  drm: rcar-du: Add support for color keying on Gen3

 drivers/gpu/drm/drm_atomic.c            |  16 +++++
 drivers/gpu/drm/drm_blend.c             | 108 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_plane.c |  60 +++++++++++++-----
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |   2 -
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   |  17 ++++-
 drivers/media/platform/vsp1/vsp1_drm.c  |   3 +
 drivers/media/platform/vsp1/vsp1_rpf.c  |  10 ++-
 drivers/media/platform/vsp1/vsp1_rwpf.h |   5 ++
 include/drm/drm_blend.h                 |   4 ++
 include/drm/drm_plane.h                 |  28 ++++++++-
 include/media/vsp1.h                    |   5 ++
 11 files changed, 235 insertions(+), 23 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH/RFC 0/4] Implement standard color keying properties

[Laurent Pinchart]

Hello,

This patch series is an attempt at implementing standard properties for color
keying support in the KMS API.

Before designing the API proposal I've analyzed the KMS drivers that support
color keying in the upstream kernel. Part of the explanation below was
initially posted in a reply to "[PATCH v2 0/2] rcar-du, vsp1: rcar-gen3: Add
support for colorkey alpha blending" and is copied here to continue the
discussion.

The armada, nouveau and rcar-du drivers expose the color key through DRM
properties. The i915 and vmwgfx drivers use custom ioctls. Here is how they
currently implement color keying.

- armada

"colorkey" range  0x00000000 0x00ffffff
"colorkey_min" range  0x00000000 0x00ffffff
"colorkey_max" range  0x00000000 0x00ffffff
"colorkey_val" range  0x00000000 0x00ffffff
"colorkey_alpha" range  0x00000000 0x00ffffff
"colorkey_mode" enum "disable", "Y component", "U component", "V component", 
"RGB", "R component", "G component", "B component"

All range properties store a RGB888 or YUV888 triplet.

The min and max properties store the comparison ranges. When a match occurs 
for one of the components, the value and alpha from the val and alpha 
properties replace the pixel. It's not clear which of the alpha "components" 
is used when a match occurs in RGB mode.

The colorkey property is a shortcut that stores identical values in min, max 
and val and 0 in alpha.

- i915

#define I915_SET_COLORKEY_NONE          (1<<0)
#define I915_SET_COLORKEY_DESTINATION   (1<<1)
#define I915_SET_COLORKEY_SOURCE        (1<<2)

struct drm_intel_sprite_colorkey {
        __u32 plane_id;
        __u32 min_value;
        __u32 channel_mask;
        __u32 max_value;
        __u32 flags;
};

- nouveau

"colorkey" range 0x00000000 0x01ffffff

The format isn't documented but it seems from the source code that bits 23:0 
store the color key value (written directly to a register, so possibly in a 
pixel format-dependent format) and bit 24 enables color keying.

- rcar-du

"colorkey" range 0x00000000 0x01ffffff

Bits 23:0 store the color key value in RGB888 (regardless of the pixel format 
of the plane) and bit 24 enables color keying. This supports Gen2 hardware 
only, where the only available more is  exact match. The pixel then becomes 
fully transparent (the hardware doesn't support custom target alpha values).

On Gen3 hardware color keying can be performed in exact RGB match, exact Y 
match or range Y match (only the max value is programmable, the min value is 
always 0). Furthermore in exact match modes the hardware can operate with a 
single match value, in which case it can then override the full ARGB or AYUV 
pixel, or double match value, in which case it can then override the alpha 
component only, but with two distinct match values each associated with a 
target alpha.

- vmwgfx

struct drm_vmw_control_stream_arg {
        __u32 stream_id;
        __u32 enabled;

        __u32 flags;
        __u32 color_key;

        __u32 handle;
        __u32 offset;
        __s32 format;
        __u32 size;
        __u32 width;
        __u32 height;
        __u32 pitch[3];

        __u32 pad64;
        struct drm_vmw_rect src;
        struct drm_vmw_rect dst;
};

The color_key field isn't documented, but the following (unused) macros hint 
that it could store an RGB888, with the color key feature enabled through the 
flags field.

#define SVGA_VIDEO_FLAG_COLORKEY        0x0001
#define SVGA_VIDEO_COLORKEY_MASK             0x00ffffff

Looking at these drivers we can already see that the hardware implementations
differ quite widely. There are however similarities, and we could express most
of the above features through a set of generic properties similar to the ones
already implemented by the armada driver. This is what the patch series
attempts to do.

- The match range can be set through minimum and maximum properties. Drivers
that support exact match only simply report an error when minimum != maximum.

- The replacement value can be set through a value property. The property
stores both the pixel value (RGB or YUV) and the alpha value Bits that are not
applicable are ignored (for instance RGB/YUV bits when the driver supports
alpha replacement only). If programmable color replacement isn't supported (as
in the R-Car Gen2 example above) the property is omitted.

- The mode can be set through a mode property. Enabling color keying through
one bit in a color property (like done by the nouveau and rcar-du drivers) is 
a hack and I don't think we should carry it forward. A mode property allows
configuring source or destination color keying.

- Part of the mode information could be deduced automatically without a need
to specify it explicitly. For instance RGB/YUV mode can be configured based on
the pixel format of the plane. Similarly, exact match vs. range match can be
configured based on whether the minimum and maximum value differ.

- The modes exposed through the mode property are left as driver-specific in
this RFC, with one "disabled" mode mandatory for all implementations. The
rationale is that a generic userspace should be able to disable color keying,
but that hardware features vary too much to standardize all modes. I'm however
starting to think that we should standardize more modes than "disabled", but I
still need to sleep over this particular issue. Ideas and comments are
welcome.

- Properties that store pixel values store them in a fixed AXYZ16161616 format
where A is the alpha value and XYZ color components that correspond to the
plane pixel format (usually RGB or YUV).

- We need to keep the existing "colorkey" properties implemented by armada, 
nouveau and rcar-du for backward compatibility reasons, but this proposed API 
doesn't require a "colorkey" property. I have reimplemented the existing
"colorkey" in the rcar-du driver as an alias for the new standard properties
to show how it can be done in a driver.


The R-Car Gen3 dual target mode feature doesn't fit in the properties proposed
here. I have no use case for that mode at the moment but I'm fairly certain
that someone will come up with one, at least if not for the R-Car for a
different display engine that provides similarly exotic features. I believe
this can for now be left for implementation through driver-specific
properties.

Alexandru Gheorghe (1):
  v4l: vsp1: Add support for colorkey alpha blending

Laurent Pinchart (3):
  drm: Add colorkey properties
  drm: rcar-du: Use standard colorkey properties
  drm: rcar-du: Add support for color keying on Gen3

 drivers/gpu/drm/drm_atomic.c            |  16 +++++
 drivers/gpu/drm/drm_blend.c             | 108 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_plane.c |  60 +++++++++++++-----
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |   2 -
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   |  17 ++++-
 drivers/media/platform/vsp1/vsp1_drm.c  |   3 +
 drivers/media/platform/vsp1/vsp1_rpf.c  |  10 ++-
 drivers/media/platform/vsp1/vsp1_rwpf.h |   5 ++
 include/drm/drm_blend.h                 |   4 ++
 include/drm/drm_plane.h                 |  28 ++++++++-
 include/media/vsp1.h                    |   5 ++
 11 files changed, 235 insertions(+), 23 deletions(-)

-- 
Regards,

Laurent Pinchart

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

[PATCH/RFC 1/4] drm: Add colorkey properties

[Laurent Pinchart]

Color keying is the action of replacing pixels matching a given color
(or range of colors) with transparent pixels in an overlay when
performing blitting. Depending on the hardware capabilities, the
matching pixel can either become fully transparent, or gain a
programmable alpha value.

Color keying is found in a large number of devices whose capabilities
often differ, but they still have enough common features in range to
standardize color key properties. This commit adds four properties
related to color keying named colorkey.min, colorkey.max, colorkey.alpha
and colorkey.mode. Additional properties can be defined by drivers to
expose device-specific features.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/drm_atomic.c |  16 +++++++
 drivers/gpu/drm/drm_blend.c  | 108 +++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_blend.h      |   4 ++
 include/drm/drm_plane.h      |  28 ++++++++++-
 4 files changed, 155 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 37445d50816a..4f57ec25e04d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -756,6 +756,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		state->rotation = val;
 	} else if (property == plane->zpos_property) {
 		state->zpos = val;
+	} else if (property == plane->colorkey.mode_property) {
+		state->colorkey.mode = val;
+	} else if (property == plane->colorkey.min_property) {
+		state->colorkey.min = val;
+	} else if (property == plane->colorkey.max_property) {
+		state->colorkey.max = val;
+	} else if (property == plane->colorkey.value_property) {
+		state->colorkey.value = val;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -815,6 +823,14 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 		*val = state->rotation;
 	} else if (property == plane->zpos_property) {
 		*val = state->zpos;
+	} else if (property == plane->colorkey.mode_property) {
+		*val = state->colorkey.mode;
+	} else if (property == plane->colorkey.min_property) {
+		*val = state->colorkey.min;
+	} else if (property == plane->colorkey.max_property) {
+		*val = state->colorkey.max;
+	} else if (property == plane->colorkey.value_property) {
+		*val = state->colorkey.value;
 	} 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_blend.c b/drivers/gpu/drm/drm_blend.c
index 2e5e089dd912..79da7d8a22e2 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -98,6 +98,10 @@
  *   planes. Without this property the primary plane is always below the cursor
  *   plane, and ordering between all other planes is undefined.
  *
+ * - Color keying is set up with drm_plane_create_colorkey_properties(). It adds
+ *   support for replacing a range of colors with a transparent color in the
+ *   plane.
+ *
  * 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).
@@ -405,3 +409,107 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
 	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_normalize_zpos);
+
+/**
+ * drm_plane_create_colorkey_properties - create colorkey properties
+ * @plane: drm plane
+ * @modes: array of supported color keying modes
+ * @num_modes: number of modes in the modes array
+ * @replace: if true create the colorkey.replacement property
+ *
+ * This function creates the generic color keying properties and attach them to
+ * the plane to enable color keying control for blending operations.
+ *
+ * Color keying is controlled through four properties:
+ *
+ * colorkey.mode:
+ *	The mode is an enumerated property that controls how color keying
+ *	operates. Modes are driver-specific, except for a "disabled" mode that
+ *	disables color keying and is guaranteed to exist if color keying is
+ *	supported.
+ *
+ * colorkey.min, colorkey.max:
+ *	Those two properties specify the colors that are replaced by transparent
+ *	pixels. Pixel whose values are in the [min, max] range are replaced, all
+ *	other pixels are left untouched. The minimum and maximum values are
+ *	expressed as a 64-bit integer in AXYZ16161616 format, where A is the
+ *	alpha value and X, Y and Z correspond to the color components of the
+ *	plane's pixel format. In most cases XYZ will be either RGB or YUV.
+ *
+ *	When a single color key is supported instead of a range, userspace shall
+ *	set the min and max properties to the same value, and drivers return an
+ *	error from their plane atomic check when the min and max values differ.
+ *
+ *	Note that depending on the selected mode, not all components might be
+ *	used for comparison. For instance a device could support color keying in
+ *	YUV format using luma (Y) matching only, ignoring the chroma components.
+ *	This behaviour is driver-specific.
+ *
+ * colorkey.value:
+ *	This property specifies the color value that replaces pixels matching
+ *	the [min, max] range. The value is expressed in AXYZ16161616 format as
+ *	the min and max properties.
+ *
+ *	This property is optional and only present when the device supports
+ *	configurable color replacement for matching pixels in the plane. If
+ *	color keying capabilities of the device are limited to making the
+ *	matching pixels fully transparent the colorkey.value property won't be
+ *	created.
+ *
+ *	Note that depending on the device, or the selected mode, not all
+ *	components might be used for value replacement. For instance a device
+ *	could support replacing the alpha value of the matching pixels but not
+ *	its color components. This behaviour is driver-specific.
+ *
+ * The @modes parameter points to an array of all color keying modes supported
+ * by the plane. The first mode has to be named "disabled" and have value 0. All
+ * other modes are driver-specific, and at least one mode has to be provided in
+ * addition to the "disabled" mode.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_plane_create_colorkey_properties(struct drm_plane *plane,
+					 const struct drm_prop_enum_list *modes,
+					 unsigned int num_modes, bool replace)
+{
+#define CREATE_COLORKEY_PROP(plane, name, type, args...) ({		       \
+	prop = drm_property_create_##type(plane->dev, 0, "colorkey." #name,    \
+					  args);			       \
+	if (prop) {							       \
+		drm_object_attach_property(&plane->base, prop, 0);	       \
+		plane->colorkey.name##_property = prop;			       \
+	}								       \
+	prop;								       \
+})
+
+	struct drm_property *prop;
+
+	/*
+	 * A minimum of two modes are required, with the first mode must named
+	 * "disabled".
+	 */
+	if (!modes || num_modes == 0 || strcmp(modes[0].name, "disabled"))
+		return -EINVAL;
+
+	prop = CREATE_COLORKEY_PROP(plane, mode, enum, modes, num_modes);
+	if (!prop)
+		return -ENOMEM;
+
+	prop = CREATE_COLORKEY_PROP(plane, min, range, 0, U64_MAX);
+	if (!prop)
+		return -ENOMEM;
+
+	prop = CREATE_COLORKEY_PROP(plane, max, range, 0, U64_MAX);
+	if (!prop)
+		return -ENOMEM;
+
+	if (replace) {
+		prop = CREATE_COLORKEY_PROP(plane, value, range, 0, U64_MAX);
+		if (!prop)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_colorkey_properties);
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 17606026590b..1d4c965dd5e2 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -49,4 +49,8 @@ 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_colorkey_properties(struct drm_plane *plane,
+					 const struct drm_prop_enum_list *modes,
+					 unsigned int num_modes, bool replace);
 #endif
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 8185e3468a23..a5804a4ea5c3 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -52,6 +52,13 @@ struct drm_modeset_acquire_ctx;
  *	where N is the number of active planes for given crtc. Note that
  *	the driver must call drm_atomic_normalize_zpos() to update this before
  *	it can be trusted.
+ * @colorkey.mode: color key mode. 0 disabled color keying, other values are
+ *	driver-specific.
+ * @colorkey.min: color key range minimum. The value is stored in AXYZ16161616
+ *	format, where A is the alpha value and X, Y and Z correspond to the
+ *	color components of the plane's pixel format (usually RGB or YUV).
+ * @colorkey.max: color key range maximum (in AXYZ16161616 format)
+ * @colorkey.value: color key replacement value (in in AXYZ16161616 format)
  * @src: clipped source coordinates of the plane (in 16.16)
  * @dst: clipped destination coordinates of the plane
  * @state: backpointer to global drm_atomic_state
@@ -112,6 +119,14 @@ struct drm_plane_state {
 	unsigned int zpos;
 	unsigned int normalized_zpos;
 
+	/* Plane colorkey */
+	struct {
+		unsigned int mode;
+		u64 min;
+		u64 max;
+		u64 value;
+	} colorkey;
+
 	/* Clipped coordinates */
 	struct drm_rect src, dst;
 
@@ -481,9 +496,13 @@ enum drm_plane_type {
  * @funcs: helper functions
  * @properties: property tracking for this plane
  * @type: type of plane (overlay, primary, cursor)
+ * @helper_private: mid-layer private data
  * @zpos_property: zpos property for this plane
  * @rotation_property: rotation property for this plane
- * @helper_private: mid-layer private data
+ * @colorkey.mode_property: color key mode property
+ * @colorkey.min_property: color key range minimum property
+ * @colorkey.max_property: color key range maximum property
+ * @colorkey.value_property: color key replacement value property
  */
 struct drm_plane {
 	struct drm_device *dev;
@@ -558,6 +577,13 @@ struct drm_plane {
 
 	struct drm_property *zpos_property;
 	struct drm_property *rotation_property;
+
+	struct {
+		struct drm_property *mode_property;
+		struct drm_property *min_property;
+		struct drm_property *max_property;
+		struct drm_property *value_property;
+	} colorkey;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 2/4] drm: rcar-du: Use standard colorkey properties

[Laurent Pinchart]

Now that KMS has standard color keying properties, instantiate them for
all the non-primary planes. This replaces the custom colorkey field in
the driver plane state structure. The custom colorkey property is kept
to ensure backward-compatibility, but now implemented as an alias for
the standard colorkey.mode, colorkey.min and colorkey.max properties.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 60 +++++++++++++++++++++++----------
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |  2 --
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   |  1 -
 3 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 4a3d16cf3ed6..b3b43c280ead 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -432,7 +432,7 @@ static void rcar_du_plane_setup_mode(struct rcar_du_group *rgrp,
 	 * PnMR_SPIM_TP_OFF bit set in their pnmr field, disabling color keying
 	 * automatically.
 	 */
-	if ((state->colorkey & RCAR_DU_COLORKEY_MASK) == RCAR_DU_COLORKEY_NONE)
+	if (state->state.colorkey.mode == 0)
 		pnmr |= PnMR_SPIM_TP_OFF;
 
 	/* For packed YUV formats we need to select the U/V order. */
@@ -441,26 +441,30 @@ static void rcar_du_plane_setup_mode(struct rcar_du_group *rgrp,
 
 	rcar_du_plane_write(rgrp, index, PnMR, pnmr);
 
+	colorkey = ((state->state.colorkey.min >> 24) & 0x00ff0000)
+		 | ((state->state.colorkey.min >> 16) & 0x0000ff00)
+		 | ((state->state.colorkey.min >>  8) & 0x000000ff);
+
 	switch (state->format->fourcc) {
 	case DRM_FORMAT_RGB565:
-		colorkey = ((state->colorkey & 0xf80000) >> 8)
-			 | ((state->colorkey & 0x00fc00) >> 5)
-			 | ((state->colorkey & 0x0000f8) >> 3);
+		colorkey = ((colorkey & 0xf80000) >> 8)
+			 | ((colorkey & 0x00fc00) >> 5)
+			 | ((colorkey & 0x0000f8) >> 3);
 		rcar_du_plane_write(rgrp, index, PnTC2R, colorkey);
 		break;
 
 	case DRM_FORMAT_ARGB1555:
 	case DRM_FORMAT_XRGB1555:
-		colorkey = ((state->colorkey & 0xf80000) >> 9)
-			 | ((state->colorkey & 0x00f800) >> 6)
-			 | ((state->colorkey & 0x0000f8) >> 3);
+		colorkey = ((colorkey & 0xf80000) >> 9)
+			 | ((colorkey & 0x00f800) >> 6)
+			 | ((colorkey & 0x0000f8) >> 3);
 		rcar_du_plane_write(rgrp, index, PnTC2R, colorkey);
 		break;
 
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_ARGB8888:
 		rcar_du_plane_write(rgrp, index, PnTC3R,
-				    PnTC3R_CODE | (state->colorkey & 0xffffff));
+				    PnTC3R_CODE | colorkey);
 		break;
 	}
 }
@@ -575,6 +579,9 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane,
 	struct drm_rect clip;
 	int ret;
 
+	if (state->colorkey.min != state->colorkey.max)
+		return -EINVAL;
+
 	if (!state->crtc) {
 		/*
 		 * The visible field is not reset by the DRM core but only
@@ -699,7 +706,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
 	state->hwindex = -1;
 	state->source = RCAR_DU_PLANE_MEMORY;
 	state->alpha = 255;
-	state->colorkey = RCAR_DU_COLORKEY_NONE;
 	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 
 	plane->state = &state->state;
@@ -714,12 +720,17 @@ static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
 	struct rcar_du_plane_state *rstate = to_rcar_plane_state(state);
 	struct rcar_du_device *rcdu = to_rcar_plane(plane)->group->dev;
 
-	if (property == rcdu->props.alpha)
+	if (property == rcdu->props.alpha) {
 		rstate->alpha = val;
-	else if (property == rcdu->props.colorkey)
-		rstate->colorkey = val;
-	else
+	} else if (property == rcdu->props.colorkey) {
+		state->colorkey.mode = val & RCAR_DU_COLORKEY_MASK ? 1 : 0;
+		state->colorkey.min = ((val & 0x00ff0000) << 24)
+				    | ((val & 0x0000ff00) << 16)
+				    | ((val & 0x000000ff) << 8);
+		state->colorkey.max = state->colorkey.min;
+	} else {
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -732,12 +743,18 @@ static int rcar_du_plane_atomic_get_property(struct drm_plane *plane,
 		container_of(state, const struct rcar_du_plane_state, state);
 	struct rcar_du_device *rcdu = to_rcar_plane(plane)->group->dev;
 
-	if (property == rcdu->props.alpha)
+	if (property == rcdu->props.alpha) {
 		*val = rstate->alpha;
-	else if (property == rcdu->props.colorkey)
-		*val = rstate->colorkey;
-	else
+	} else if (property == rcdu->props.colorkey) {
+		u32 colorkey = ((state->colorkey.min >> 24) & 0x00ff0000)
+			     | ((state->colorkey.min >> 16) & 0x0000ff00)
+			     | ((state->colorkey.min >>  8) & 0x000000ff);
+
+		*val = colorkey | (state->colorkey.mode ?
+			RCAR_DU_COLORKEY_SOURCE : RCAR_DU_COLORKEY_NONE);
+	} else {
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -766,6 +783,11 @@ static const uint32_t formats[] = {
 	DRM_FORMAT_NV16,
 };
 
+static const struct drm_prop_enum_list colorkey_modes[] = {
+	{ 0, "disabled" },
+	{ 1, "source" },
+};
+
 int rcar_du_planes_init(struct rcar_du_group *rgrp)
 {
 	struct rcar_du_device *rcdu = rgrp->dev;
@@ -808,6 +830,10 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
 					   rcdu->props.colorkey,
 					   RCAR_DU_COLORKEY_NONE);
 		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
+		drm_plane_create_colorkey_properties(&plane->plane,
+						     colorkey_modes,
+						     ARRAY_SIZE(colorkey_modes),
+						     false);
 	}
 
 	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 890321b4665d..d8baf12cc716 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)
  * @format: information about the pixel format used by the plane
  * @hwindex: 0-based hardware plane index, -1 means unused
  * @alpha: value of the plane alpha property
- * @colorkey: value of the plane colorkey property
  */
 struct rcar_du_plane_state {
 	struct drm_plane_state state;
@@ -61,7 +60,6 @@ struct rcar_du_plane_state {
 	enum rcar_du_plane_source source;
 
 	unsigned int alpha;
-	unsigned int colorkey;
 };
 
 static inline struct rcar_du_plane_state *
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 2c260c33840b..882d1f7a328b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -68,7 +68,6 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 		.format = rcar_du_format_info(DRM_FORMAT_ARGB8888),
 		.source = RCAR_DU_PLANE_VSPD1,
 		.alpha = 255,
-		.colorkey = 0,
 	};
 
 	if (rcdu->info->gen >= 3)
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 2/4] drm: rcar-du: Use standard colorkey properties

[Laurent Pinchart]

Now that KMS has standard color keying properties, instantiate them for
all the non-primary planes. This replaces the custom colorkey field in
the driver plane state structure. The custom colorkey property is kept
to ensure backward-compatibility, but now implemented as an alias for
the standard colorkey.mode, colorkey.min and colorkey.max properties.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 60 +++++++++++++++++++++++----------
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |  2 --
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   |  1 -
 3 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 4a3d16cf3ed6..b3b43c280ead 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -432,7 +432,7 @@ static void rcar_du_plane_setup_mode(struct rcar_du_group *rgrp,
 	 * PnMR_SPIM_TP_OFF bit set in their pnmr field, disabling color keying
 	 * automatically.
 	 */
-	if ((state->colorkey & RCAR_DU_COLORKEY_MASK) == RCAR_DU_COLORKEY_NONE)
+	if (state->state.colorkey.mode == 0)
 		pnmr |= PnMR_SPIM_TP_OFF;
 
 	/* For packed YUV formats we need to select the U/V order. */
@@ -441,26 +441,30 @@ static void rcar_du_plane_setup_mode(struct rcar_du_group *rgrp,
 
 	rcar_du_plane_write(rgrp, index, PnMR, pnmr);
 
+	colorkey = ((state->state.colorkey.min >> 24) & 0x00ff0000)
+		 | ((state->state.colorkey.min >> 16) & 0x0000ff00)
+		 | ((state->state.colorkey.min >>  8) & 0x000000ff);
+
 	switch (state->format->fourcc) {
 	case DRM_FORMAT_RGB565:
-		colorkey = ((state->colorkey & 0xf80000) >> 8)
-			 | ((state->colorkey & 0x00fc00) >> 5)
-			 | ((state->colorkey & 0x0000f8) >> 3);
+		colorkey = ((colorkey & 0xf80000) >> 8)
+			 | ((colorkey & 0x00fc00) >> 5)
+			 | ((colorkey & 0x0000f8) >> 3);
 		rcar_du_plane_write(rgrp, index, PnTC2R, colorkey);
 		break;
 
 	case DRM_FORMAT_ARGB1555:
 	case DRM_FORMAT_XRGB1555:
-		colorkey = ((state->colorkey & 0xf80000) >> 9)
-			 | ((state->colorkey & 0x00f800) >> 6)
-			 | ((state->colorkey & 0x0000f8) >> 3);
+		colorkey = ((colorkey & 0xf80000) >> 9)
+			 | ((colorkey & 0x00f800) >> 6)
+			 | ((colorkey & 0x0000f8) >> 3);
 		rcar_du_plane_write(rgrp, index, PnTC2R, colorkey);
 		break;
 
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_ARGB8888:
 		rcar_du_plane_write(rgrp, index, PnTC3R,
-				    PnTC3R_CODE | (state->colorkey & 0xffffff));
+				    PnTC3R_CODE | colorkey);
 		break;
 	}
 }
@@ -575,6 +579,9 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane,
 	struct drm_rect clip;
 	int ret;
 
+	if (state->colorkey.min != state->colorkey.max)
+		return -EINVAL;
+
 	if (!state->crtc) {
 		/*
 		 * The visible field is not reset by the DRM core but only
@@ -699,7 +706,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
 	state->hwindex = -1;
 	state->source = RCAR_DU_PLANE_MEMORY;
 	state->alpha = 255;
-	state->colorkey = RCAR_DU_COLORKEY_NONE;
 	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 
 	plane->state = &state->state;
@@ -714,12 +720,17 @@ static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
 	struct rcar_du_plane_state *rstate = to_rcar_plane_state(state);
 	struct rcar_du_device *rcdu = to_rcar_plane(plane)->group->dev;
 
-	if (property == rcdu->props.alpha)
+	if (property == rcdu->props.alpha) {
 		rstate->alpha = val;
-	else if (property == rcdu->props.colorkey)
-		rstate->colorkey = val;
-	else
+	} else if (property == rcdu->props.colorkey) {
+		state->colorkey.mode = val & RCAR_DU_COLORKEY_MASK ? 1 : 0;
+		state->colorkey.min = ((val & 0x00ff0000) << 24)
+				    | ((val & 0x0000ff00) << 16)
+				    | ((val & 0x000000ff) << 8);
+		state->colorkey.max = state->colorkey.min;
+	} else {
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -732,12 +743,18 @@ static int rcar_du_plane_atomic_get_property(struct drm_plane *plane,
 		container_of(state, const struct rcar_du_plane_state, state);
 	struct rcar_du_device *rcdu = to_rcar_plane(plane)->group->dev;
 
-	if (property == rcdu->props.alpha)
+	if (property == rcdu->props.alpha) {
 		*val = rstate->alpha;
-	else if (property == rcdu->props.colorkey)
-		*val = rstate->colorkey;
-	else
+	} else if (property == rcdu->props.colorkey) {
+		u32 colorkey = ((state->colorkey.min >> 24) & 0x00ff0000)
+			     | ((state->colorkey.min >> 16) & 0x0000ff00)
+			     | ((state->colorkey.min >>  8) & 0x000000ff);
+
+		*val = colorkey | (state->colorkey.mode ?
+			RCAR_DU_COLORKEY_SOURCE : RCAR_DU_COLORKEY_NONE);
+	} else {
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -766,6 +783,11 @@ static const uint32_t formats[] = {
 	DRM_FORMAT_NV16,
 };
 
+static const struct drm_prop_enum_list colorkey_modes[] = {
+	{ 0, "disabled" },
+	{ 1, "source" },
+};
+
 int rcar_du_planes_init(struct rcar_du_group *rgrp)
 {
 	struct rcar_du_device *rcdu = rgrp->dev;
@@ -808,6 +830,10 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
 					   rcdu->props.colorkey,
 					   RCAR_DU_COLORKEY_NONE);
 		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
+		drm_plane_create_colorkey_properties(&plane->plane,
+						     colorkey_modes,
+						     ARRAY_SIZE(colorkey_modes),
+						     false);
 	}
 
 	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 890321b4665d..d8baf12cc716 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)
  * @format: information about the pixel format used by the plane
  * @hwindex: 0-based hardware plane index, -1 means unused
  * @alpha: value of the plane alpha property
- * @colorkey: value of the plane colorkey property
  */
 struct rcar_du_plane_state {
 	struct drm_plane_state state;
@@ -61,7 +60,6 @@ struct rcar_du_plane_state {
 	enum rcar_du_plane_source source;
 
 	unsigned int alpha;
-	unsigned int colorkey;
 };
 
 static inline struct rcar_du_plane_state *
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 2c260c33840b..882d1f7a328b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -68,7 +68,6 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 		.format = rcar_du_format_info(DRM_FORMAT_ARGB8888),
 		.source = RCAR_DU_PLANE_VSPD1,
 		.alpha = 255,
-		.colorkey = 0,
 	};
 
 	if (rcdu->info->gen >= 3)
-- 
Regards,

Laurent Pinchart

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

[PATCH/RFC 3/4] v4l: vsp1: Add support for colorkey alpha blending

[Laurent Pinchart]

From: Alexandru Gheorghe <Alexandru_Gheorghe@mentor.com>

The VSP2 found in R-Car Gen3 SoCs supports changing the alpha value of
source pixels that match a color key. Add support for this feature for
display pipelines through the API exposed to the DU driver.

The colorkey key value is expressed as a XYZ 888 value, where the X, Y
and Z components are either RGB or YCbCr depending on the plane format.
When the plane is configured with an RGB formats all three components
are matched by the hardware, while with an YCbCr format only the
luminance component is matched the chroma components will be ignored.

Signed-off-by: Alexandru Gheorghe <Alexandru_Gheorghe@mentor.com>
[Group all color key parameters in a structure]
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_drm.c  |  3 +++
 drivers/media/platform/vsp1/vsp1_rpf.c  | 10 ++++++++--
 drivers/media/platform/vsp1/vsp1_rwpf.h |  5 +++++
 include/media/vsp1.h                    |  5 +++++
 4 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index 7ce69f23f50a..68af99e5cfa3 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -378,6 +378,9 @@ int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index,
 	rpf->format.plane_fmt[0].bytesperline = cfg->pitch;
 	rpf->format.plane_fmt[1].bytesperline = cfg->pitch;
 	rpf->alpha = cfg->alpha;
+	rpf->colorkey.enabled = cfg->colorkey.enabled;
+	rpf->colorkey.key = cfg->colorkey.key;
+	rpf->colorkey.alpha = cfg->colorkey.alpha;
 
 	rpf->mem.addr[0] = cfg->mem[0];
 	rpf->mem.addr[1] = cfg->mem[1];
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index fe0633da5a5f..8c532f22013b 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -243,8 +243,14 @@ static void rpf_configure(struct vsp1_entity *entity,
 	}
 
 	vsp1_rpf_write(rpf, dl, VI6_RPF_MSK_CTRL, 0);
-	vsp1_rpf_write(rpf, dl, VI6_RPF_CKEY_CTRL, 0);
-
+	if (rpf->colorkey.enabled) {
+		vsp1_rpf_write(rpf, dl, VI6_RPF_CKEY_SET0,
+			       (rpf->colorkey.alpha << 24) | rpf->colorkey.key);
+		vsp1_rpf_write(rpf, dl, VI6_RPF_CKEY_CTRL,
+			       VI6_RPF_CKEY_CTRL_SAPE0);
+	} else {
+		vsp1_rpf_write(rpf, dl, VI6_RPF_CKEY_CTRL, 0);
+	}
 }
 
 static void rpf_partition(struct vsp1_entity *entity,
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h b/drivers/media/platform/vsp1/vsp1_rwpf.h
index 58215a7ab631..78119bb681f9 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.h
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
@@ -50,6 +50,11 @@ struct vsp1_rwpf {
 	unsigned int bru_input;
 
 	unsigned int alpha;
+	struct {
+		bool enabled;
+		u32 key;
+		u32 alpha;
+	} colorkey;
 
 	u32 mult_alpha;
 	u32 outfmt;
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 68a8abe4fac5..cc6a411e2312 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -49,6 +49,11 @@ struct vsp1_du_atomic_config {
 	struct v4l2_rect dst;
 	unsigned int alpha;
 	unsigned int zpos;
+	struct {
+		bool enabled;
+		u32 key;
+		u32 alpha;
+	} colorkey;
 };
 
 void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index);
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 4/4] drm: rcar-du: Add support for color keying on Gen3

[Laurent Pinchart]

Gen3 hardware supports color keying with replacement of the pixel value.
Implement it using the standard KMS colorkey properties.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 882d1f7a328b..54deb9567cd3 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -198,6 +198,13 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 		}
 	}
 
+	/* Convert the colorkey from 16 bits to 8 bits per component. */
+	cfg.colorkey.enabled = state->state.colorkey.mode;
+	cfg.colorkey.alpha = state->state.colorkey.value >> 56;
+	cfg.colorkey.key = ((state->state.colorkey.min >> 24) & 0x00ff0000)
+			 | ((state->state.colorkey.min >> 16) & 0x0000ff00)
+			 | ((state->state.colorkey.min >>  8) & 0x000000ff);
+
 	vsp1_du_atomic_update(plane->vsp->vsp, crtc->vsp_pipe,
 			      plane->index, &cfg);
 }
@@ -383,6 +390,11 @@ static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
 	.atomic_get_property = rcar_du_vsp_plane_atomic_get_property,
 };
 
+static const struct drm_prop_enum_list colorkey_modes[] = {
+	{ 0, "disabled" },
+	{ 1, "source" },
+};
+
 int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
 		     unsigned int crtcs)
 {
@@ -441,6 +453,10 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
 					   rcdu->props.alpha, 255);
 		drm_plane_create_zpos_property(&plane->plane, 1, 1,
 					       vsp->num_planes - 1);
+		drm_plane_create_colorkey_properties(&plane->plane,
+						     colorkey_modes,
+						     ARRAY_SIZE(colorkey_modes),
+						     true);
 	}
 
 	return 0;
-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Neil Armstrong]

Hi Laurent,

On 17/12/2017 01:17, Laurent Pinchart wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent, or gain a
> programmable alpha value.
> 
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds four properties
> related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> and colorkey.mode. Additional properties can be defined by drivers to
> expose device-specific features.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |  16 +++++++
>  drivers/gpu/drm/drm_blend.c  | 108 +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_blend.h      |   4 ++
>  include/drm/drm_plane.h      |  28 ++++++++++-
>  4 files changed, 155 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 37445d50816a..4f57ec25e04d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -756,6 +756,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->rotation = val;
>  	} else if (property == plane->zpos_property) {
>  		state->zpos = val;
> +	} else if (property == plane->colorkey.mode_property) {
> +		state->colorkey.mode = val;
> +	} else if (property == plane->colorkey.min_property) {
> +		state->colorkey.min = val;
> +	} else if (property == plane->colorkey.max_property) {
> +		state->colorkey.max = val;
> +	} else if (property == plane->colorkey.value_property) {
> +		state->colorkey.value = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -815,6 +823,14 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->rotation;
>  	} else if (property == plane->zpos_property) {
>  		*val = state->zpos;
> +	} else if (property == plane->colorkey.mode_property) {
> +		*val = state->colorkey.mode;
> +	} else if (property == plane->colorkey.min_property) {
> +		*val = state->colorkey.min;
> +	} else if (property == plane->colorkey.max_property) {
> +		*val = state->colorkey.max;
> +	} else if (property == plane->colorkey.value_property) {
> +		*val = state->colorkey.value;
>  	} 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_blend.c b/drivers/gpu/drm/drm_blend.c
> index 2e5e089dd912..79da7d8a22e2 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -98,6 +98,10 @@
>   *   planes. Without this property the primary plane is always below the cursor
>   *   plane, and ordering between all other planes is undefined.
>   *
> + * - Color keying is set up with drm_plane_create_colorkey_properties(). It adds
> + *   support for replacing a range of colors with a transparent color in the
> + *   plane.
> + *
>   * 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).
> @@ -405,3 +409,107 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_normalize_zpos);
> +
> +/**
> + * drm_plane_create_colorkey_properties - create colorkey properties
> + * @plane: drm plane
> + * @modes: array of supported color keying modes
> + * @num_modes: number of modes in the modes array
> + * @replace: if true create the colorkey.replacement property
> + *
> + * This function creates the generic color keying properties and attach them to
> + * the plane to enable color keying control for blending operations.
> + *
> + * Color keying is controlled through four properties:
> + *
> + * colorkey.mode:
> + *	The mode is an enumerated property that controls how color keying
> + *	operates. Modes are driver-specific, except for a "disabled" mode that
> + *	disables color keying and is guaranteed to exist if color keying is
> + *	supported.
> + *
> + * colorkey.min, colorkey.max:
> + *	Those two properties specify the colors that are replaced by transparent
> + *	pixels. Pixel whose values are in the [min, max] range are replaced, all
> + *	other pixels are left untouched. The minimum and maximum values are
> + *	expressed as a 64-bit integer in AXYZ16161616 format, where A is the
> + *	alpha value and X, Y and Z correspond to the color components of the
> + *	plane's pixel format. In most cases XYZ will be either RGB or YUV.
> + *
> + *	When a single color key is supported instead of a range, userspace shall
> + *	set the min and max properties to the same value, and drivers return an
> + *	error from their plane atomic check when the min and max values differ.
> + *
> + *	Note that depending on the selected mode, not all components might be
> + *	used for comparison. For instance a device could support color keying in
> + *	YUV format using luma (Y) matching only, ignoring the chroma components.
> + *	This behaviour is driver-specific.
> + *
> + * colorkey.value:
> + *	This property specifies the color value that replaces pixels matching
> + *	the [min, max] range. The value is expressed in AXYZ16161616 format as
> + *	the min and max properties.
> + *
> + *	This property is optional and only present when the device supports
> + *	configurable color replacement for matching pixels in the plane. If
> + *	color keying capabilities of the device are limited to making the
> + *	matching pixels fully transparent the colorkey.value property won't be
> + *	created.
> + *
> + *	Note that depending on the device, or the selected mode, not all
> + *	components might be used for value replacement. For instance a device
> + *	could support replacing the alpha value of the matching pixels but not
> + *	its color components. This behaviour is driver-specific.
> + *
> + * The @modes parameter points to an array of all color keying modes supported
> + * by the plane. The first mode has to be named "disabled" and have value 0. All
> + * other modes are driver-specific, and at least one mode has to be provided in
> + * addition to the "disabled" mode.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_plane_create_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace)
> +{
> +#define CREATE_COLORKEY_PROP(plane, name, type, args...) ({		       \
> +	prop = drm_property_create_##type(plane->dev, 0, "colorkey." #name,    \
> +					  args);			       \
> +	if (prop) {							       \
> +		drm_object_attach_property(&plane->base, prop, 0);	       \
> +		plane->colorkey.name##_property = prop;			       \
> +	}								       \
> +	prop;								       \
> +})
> +
> +	struct drm_property *prop;
> +
> +	/*
> +	 * A minimum of two modes are required, with the first mode must named
> +	 * "disabled".
> +	 */
> +	if (!modes || num_modes == 0 || strcmp(modes[0].name, "disabled"))
> +		return -EINVAL;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, mode, enum, modes, num_modes);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, min, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, max, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	if (replace) {
> +		prop = CREATE_COLORKEY_PROP(plane, value, range, 0, U64_MAX);
> +		if (!prop)
> +			return -ENOMEM;
> +	}


#undef CREATE_COLORKEY_PROP ?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_colorkey_properties);
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 17606026590b..1d4c965dd5e2 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -49,4 +49,8 @@ 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_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace);
>  #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 8185e3468a23..a5804a4ea5c3 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -52,6 +52,13 @@ struct drm_modeset_acquire_ctx;
>   *	where N is the number of active planes for given crtc. Note that
>   *	the driver must call drm_atomic_normalize_zpos() to update this before
>   *	it can be trusted.
> + * @colorkey.mode: color key mode. 0 disabled color keying, other values are
> + *	driver-specific.
> + * @colorkey.min: color key range minimum. The value is stored in AXYZ16161616
> + *	format, where A is the alpha value and X, Y and Z correspond to the
> + *	color components of the plane's pixel format (usually RGB or YUV).
> + * @colorkey.max: color key range maximum (in AXYZ16161616 format)
> + * @colorkey.value: color key replacement value (in in AXYZ16161616 format)
>   * @src: clipped source coordinates of the plane (in 16.16)
>   * @dst: clipped destination coordinates of the plane
>   * @state: backpointer to global drm_atomic_state
> @@ -112,6 +119,14 @@ struct drm_plane_state {
>  	unsigned int zpos;
>  	unsigned int normalized_zpos;
>  
> +	/* Plane colorkey */
> +	struct {
> +		unsigned int mode;
> +		u64 min;
> +		u64 max;
> +		u64 value;
> +	} colorkey;
> +
>  	/* Clipped coordinates */
>  	struct drm_rect src, dst;
>  
> @@ -481,9 +496,13 @@ enum drm_plane_type {
>   * @funcs: helper functions
>   * @properties: property tracking for this plane
>   * @type: type of plane (overlay, primary, cursor)
> + * @helper_private: mid-layer private data
>   * @zpos_property: zpos property for this plane
>   * @rotation_property: rotation property for this plane
> - * @helper_private: mid-layer private data
> + * @colorkey.mode_property: color key mode property
> + * @colorkey.min_property: color key range minimum property
> + * @colorkey.max_property: color key range maximum property
> + * @colorkey.value_property: color key replacement value property
>   */
>  struct drm_plane {
>  	struct drm_device *dev;
> @@ -558,6 +577,13 @@ struct drm_plane {
>  
>  	struct drm_property *zpos_property;
>  	struct drm_property *rotation_property;
> +
> +	struct {
> +		struct drm_property *mode_property;
> +		struct drm_property *min_property;
> +		struct drm_property *max_property;
> +		struct drm_property *value_property;
> +	} colorkey;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> 

Apart from that,

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Neil Armstrong]

Hi Laurent,

On 17/12/2017 01:17, Laurent Pinchart wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent, or gain a
> programmable alpha value.
> 
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds four properties
> related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> and colorkey.mode. Additional properties can be defined by drivers to
> expose device-specific features.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |  16 +++++++
>  drivers/gpu/drm/drm_blend.c  | 108 +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_blend.h      |   4 ++
>  include/drm/drm_plane.h      |  28 ++++++++++-
>  4 files changed, 155 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 37445d50816a..4f57ec25e04d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -756,6 +756,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->rotation = val;
>  	} else if (property == plane->zpos_property) {
>  		state->zpos = val;
> +	} else if (property == plane->colorkey.mode_property) {
> +		state->colorkey.mode = val;
> +	} else if (property == plane->colorkey.min_property) {
> +		state->colorkey.min = val;
> +	} else if (property == plane->colorkey.max_property) {
> +		state->colorkey.max = val;
> +	} else if (property == plane->colorkey.value_property) {
> +		state->colorkey.value = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -815,6 +823,14 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->rotation;
>  	} else if (property == plane->zpos_property) {
>  		*val = state->zpos;
> +	} else if (property == plane->colorkey.mode_property) {
> +		*val = state->colorkey.mode;
> +	} else if (property == plane->colorkey.min_property) {
> +		*val = state->colorkey.min;
> +	} else if (property == plane->colorkey.max_property) {
> +		*val = state->colorkey.max;
> +	} else if (property == plane->colorkey.value_property) {
> +		*val = state->colorkey.value;
>  	} 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_blend.c b/drivers/gpu/drm/drm_blend.c
> index 2e5e089dd912..79da7d8a22e2 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -98,6 +98,10 @@
>   *   planes. Without this property the primary plane is always below the cursor
>   *   plane, and ordering between all other planes is undefined.
>   *
> + * - Color keying is set up with drm_plane_create_colorkey_properties(). It adds
> + *   support for replacing a range of colors with a transparent color in the
> + *   plane.
> + *
>   * 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).
> @@ -405,3 +409,107 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_normalize_zpos);
> +
> +/**
> + * drm_plane_create_colorkey_properties - create colorkey properties
> + * @plane: drm plane
> + * @modes: array of supported color keying modes
> + * @num_modes: number of modes in the modes array
> + * @replace: if true create the colorkey.replacement property
> + *
> + * This function creates the generic color keying properties and attach them to
> + * the plane to enable color keying control for blending operations.
> + *
> + * Color keying is controlled through four properties:
> + *
> + * colorkey.mode:
> + *	The mode is an enumerated property that controls how color keying
> + *	operates. Modes are driver-specific, except for a "disabled" mode that
> + *	disables color keying and is guaranteed to exist if color keying is
> + *	supported.
> + *
> + * colorkey.min, colorkey.max:
> + *	Those two properties specify the colors that are replaced by transparent
> + *	pixels. Pixel whose values are in the [min, max] range are replaced, all
> + *	other pixels are left untouched. The minimum and maximum values are
> + *	expressed as a 64-bit integer in AXYZ16161616 format, where A is the
> + *	alpha value and X, Y and Z correspond to the color components of the
> + *	plane's pixel format. In most cases XYZ will be either RGB or YUV.
> + *
> + *	When a single color key is supported instead of a range, userspace shall
> + *	set the min and max properties to the same value, and drivers return an
> + *	error from their plane atomic check when the min and max values differ.
> + *
> + *	Note that depending on the selected mode, not all components might be
> + *	used for comparison. For instance a device could support color keying in
> + *	YUV format using luma (Y) matching only, ignoring the chroma components.
> + *	This behaviour is driver-specific.
> + *
> + * colorkey.value:
> + *	This property specifies the color value that replaces pixels matching
> + *	the [min, max] range. The value is expressed in AXYZ16161616 format as
> + *	the min and max properties.
> + *
> + *	This property is optional and only present when the device supports
> + *	configurable color replacement for matching pixels in the plane. If
> + *	color keying capabilities of the device are limited to making the
> + *	matching pixels fully transparent the colorkey.value property won't be
> + *	created.
> + *
> + *	Note that depending on the device, or the selected mode, not all
> + *	components might be used for value replacement. For instance a device
> + *	could support replacing the alpha value of the matching pixels but not
> + *	its color components. This behaviour is driver-specific.
> + *
> + * The @modes parameter points to an array of all color keying modes supported
> + * by the plane. The first mode has to be named "disabled" and have value 0. All
> + * other modes are driver-specific, and at least one mode has to be provided in
> + * addition to the "disabled" mode.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_plane_create_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace)
> +{
> +#define CREATE_COLORKEY_PROP(plane, name, type, args...) ({		       \
> +	prop = drm_property_create_##type(plane->dev, 0, "colorkey." #name,    \
> +					  args);			       \
> +	if (prop) {							       \
> +		drm_object_attach_property(&plane->base, prop, 0);	       \
> +		plane->colorkey.name##_property = prop;			       \
> +	}								       \
> +	prop;								       \
> +})
> +
> +	struct drm_property *prop;
> +
> +	/*
> +	 * A minimum of two modes are required, with the first mode must named
> +	 * "disabled".
> +	 */
> +	if (!modes || num_modes == 0 || strcmp(modes[0].name, "disabled"))
> +		return -EINVAL;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, mode, enum, modes, num_modes);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, min, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, max, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	if (replace) {
> +		prop = CREATE_COLORKEY_PROP(plane, value, range, 0, U64_MAX);
> +		if (!prop)
> +			return -ENOMEM;
> +	}


#undef CREATE_COLORKEY_PROP ?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_colorkey_properties);
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 17606026590b..1d4c965dd5e2 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -49,4 +49,8 @@ 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_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace);
>  #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 8185e3468a23..a5804a4ea5c3 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -52,6 +52,13 @@ struct drm_modeset_acquire_ctx;
>   *	where N is the number of active planes for given crtc. Note that
>   *	the driver must call drm_atomic_normalize_zpos() to update this before
>   *	it can be trusted.
> + * @colorkey.mode: color key mode. 0 disabled color keying, other values are
> + *	driver-specific.
> + * @colorkey.min: color key range minimum. The value is stored in AXYZ16161616
> + *	format, where A is the alpha value and X, Y and Z correspond to the
> + *	color components of the plane's pixel format (usually RGB or YUV).
> + * @colorkey.max: color key range maximum (in AXYZ16161616 format)
> + * @colorkey.value: color key replacement value (in in AXYZ16161616 format)
>   * @src: clipped source coordinates of the plane (in 16.16)
>   * @dst: clipped destination coordinates of the plane
>   * @state: backpointer to global drm_atomic_state
> @@ -112,6 +119,14 @@ struct drm_plane_state {
>  	unsigned int zpos;
>  	unsigned int normalized_zpos;
>  
> +	/* Plane colorkey */
> +	struct {
> +		unsigned int mode;
> +		u64 min;
> +		u64 max;
> +		u64 value;
> +	} colorkey;
> +
>  	/* Clipped coordinates */
>  	struct drm_rect src, dst;
>  
> @@ -481,9 +496,13 @@ enum drm_plane_type {
>   * @funcs: helper functions
>   * @properties: property tracking for this plane
>   * @type: type of plane (overlay, primary, cursor)
> + * @helper_private: mid-layer private data
>   * @zpos_property: zpos property for this plane
>   * @rotation_property: rotation property for this plane
> - * @helper_private: mid-layer private data
> + * @colorkey.mode_property: color key mode property
> + * @colorkey.min_property: color key range minimum property
> + * @colorkey.max_property: color key range maximum property
> + * @colorkey.value_property: color key replacement value property
>   */
>  struct drm_plane {
>  	struct drm_device *dev;
> @@ -558,6 +577,13 @@ struct drm_plane {
>  
>  	struct drm_property *zpos_property;
>  	struct drm_property *rotation_property;
> +
> +	struct {
> +		struct drm_property *mode_property;
> +		struct drm_property *min_property;
> +		struct drm_property *max_property;
> +		struct drm_property *value_property;
> +	} colorkey;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> 

Apart from that,

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Laurent Pinchart]

Hi Neil,

On Tuesday, 19 December 2017 11:00:28 EET Neil Armstrong wrote:
> On 17/12/2017 01:17, Laurent Pinchart wrote:
> > Color keying is the action of replacing pixels matching a given color
> > (or range of colors) with transparent pixels in an overlay when
> > performing blitting. Depending on the hardware capabilities, the
> > matching pixel can either become fully transparent, or gain a
> > programmable alpha value.
> > 
> > Color keying is found in a large number of devices whose capabilities
> > often differ, but they still have enough common features in range to
> > standardize color key properties. This commit adds four properties
> > related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> > and colorkey.mode. Additional properties can be defined by drivers to
> > expose device-specific features.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/drm_atomic.c |  16 +++++++
> >  drivers/gpu/drm/drm_blend.c  | 108 ++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_blend.h      |   4 ++
> >  include/drm/drm_plane.h      |  28 ++++++++++-
> >  4 files changed, 155 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 37445d50816a..4f57ec25e04d 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -756,6 +756,14 @@ static int drm_atomic_plane_set_property(struct

[snip]

> > +int drm_plane_create_colorkey_properties(struct drm_plane *plane,
> > +					 const struct drm_prop_enum_list *modes,
> > +					 unsigned int num_modes, bool replace)
> > +{
> > +#define CREATE_COLORKEY_PROP(plane, name, type, args...) ({		       \
> > +	prop = drm_property_create_##type(plane->dev, 0, "colorkey." #name,    
\
> > +					  args);			       \
> > +	if (prop) {							       \
> > +		drm_object_attach_property(&plane->base, prop, 0);	       \
> > +		plane->colorkey.name##_property = prop;			       \
> > +	}								       \
> > +	prop;								       \
> > +})
> > +
> > +	struct drm_property *prop;
> > +
> > +	/*
> > +	 * A minimum of two modes are required, with the first mode must named
> > +	 * "disabled".
> > +	 */
> > +	if (!modes || num_modes == 0 || strcmp(modes[0].name, "disabled"))
> > +		return -EINVAL;
> > +
> > +	prop = CREATE_COLORKEY_PROP(plane, mode, enum, modes, num_modes);
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	prop = CREATE_COLORKEY_PROP(plane, min, range, 0, U64_MAX);
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	prop = CREATE_COLORKEY_PROP(plane, max, range, 0, U64_MAX);
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	if (replace) {
> > +		prop = CREATE_COLORKEY_PROP(plane, value, range, 0, U64_MAX);
> > +		if (!prop)
> > +			return -ENOMEM;
> > +	}
> 
> #undef CREATE_COLORKEY_PROP ?

That's a good idea, I'll fix it in the next version.

> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_plane_create_colorkey_properties);

[snip]

> Apart from that,
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Maxime Ripard]

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

On Sun, Dec 17, 2017 at 02:17:21AM +0200, Laurent Pinchart wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent, or gain a
> programmable alpha value.
> 
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds four properties
> related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> and colorkey.mode. Additional properties can be defined by drivers to
> expose device-specific features.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Sorry for the delay,
Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH/RFC 4/4] drm: rcar-du: Add support for color keying on Gen3

[Maxime Ripard]

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

Hi,

On Sun, Dec 17, 2017 at 02:17:24AM +0200, Laurent Pinchart wrote:
> +static const struct drm_prop_enum_list colorkey_modes[] = {
> +	{ 0, "disabled" },
> +	{ 1, "source" },
> +};
> +
>  int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
>  		     unsigned int crtcs)
>  {
> @@ -441,6 +453,10 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
>  					   rcdu->props.alpha, 255);
>  		drm_plane_create_zpos_property(&plane->plane, 1, 1,
>  					       vsp->num_planes - 1);
> +		drm_plane_create_colorkey_properties(&plane->plane,
> +						     colorkey_modes,
> +						     ARRAY_SIZE(colorkey_modes),
> +						     true);

You seem to define the same list in your enumeration between your
patch 2 and this one. Can this be something made generic too?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH/RFC 0/4] Implement standard color keying properties

[Dmitry Osipenko]

Hello Laurent,

On 17.12.2017 03:17, Laurent Pinchart wrote:
> Hello,
> 
> This patch series is an attempt at implementing standard properties for color
> keying support in the KMS API.

I was looking at implementing colorkey support for NVIDIA Tegra (older Tegra's
in particular) and Daniel Vetter suggested that colorkey should be implemented
as a generic plane property, instead of a custom one that I had in the patch
[0]. I've applied your RFC patch and replaced custom property with the generic,
it works well.

Very likely that all HW should be capable of making pixel completely transparent
when it matches a specified color, that could be one of common modes. Though
there could be limitations, like Tegra's aren't able to do blending-over of
planes if colorkey'ing is enabled. The 'colorkey.mode' property allows driver to
expose both common properties and a custom ones. In case of Tegra we could
implement a common property such that atomic commit will be rejected if planes
blending mode aren't compatible with the enabled colorkey'ing, and have a custom
property for a custom HW-aware application that won't be rejected (for example
Opentegra's Xv extension). The common modes could be derived later, once generic
property will get more usage by a variety of drivers. For now I don't see any
issues with your approach and hope to see this series applied in upstream to get
use of them, please continue your effort.

[0] https://patchwork.kernel.org/patch/10342849/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 0/4] Implement standard color keying properties

[Dmitry Osipenko]

Hello Laurent,

On 17.12.2017 03:17, Laurent Pinchart wrote:
> Hello,
> 
> This patch series is an attempt at implementing standard properties for color
> keying support in the KMS API.

I was looking at implementing colorkey support for NVIDIA Tegra (older Tegra's
in particular) and Daniel Vetter suggested that colorkey should be implemented
as a generic plane property, instead of a custom one that I had in the patch
[0]. I've applied your RFC patch and replaced custom property with the generic,
it works well.

Very likely that all HW should be capable of making pixel completely transparent
when it matches a specified color, that could be one of common modes. Though
there could be limitations, like Tegra's aren't able to do blending-over of
planes if colorkey'ing is enabled. The 'colorkey.mode' property allows driver to
expose both common properties and a custom ones. In case of Tegra we could
implement a common property such that atomic commit will be rejected if planes
blending mode aren't compatible with the enabled colorkey'ing, and have a custom
property for a custom HW-aware application that won't be rejected (for example
Opentegra's Xv extension). The common modes could be derived later, once generic
property will get more usage by a variety of drivers. For now I don't see any
issues with your approach and hope to see this series applied in upstream to get
use of them, please continue your effort.

[0] https://patchwork.kernel.org/patch/10342849/

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Dmitry Osipenko]

On 17.12.2017 03:17, Laurent Pinchart wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent, or gain a
> programmable alpha value.
> 
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds four properties
> related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> and colorkey.mode. Additional properties can be defined by drivers to
> expose device-specific features.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>

Note that this patch needs to be rebased now.

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Dmitry Osipenko]

On 17.12.2017 03:17, Laurent Pinchart wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent, or gain a
> programmable alpha value.
> 
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds four properties
> related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> and colorkey.mode. Additional properties can be defined by drivers to
> expose device-specific features.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>

Note that this patch needs to be rebased now.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Ville Syrjälä]

On Sun, Dec 17, 2017 at 02:17:21AM +0200, Laurent Pinchart wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent, or gain a
> programmable alpha value.
> 
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds four properties
> related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> and colorkey.mode. Additional properties can be defined by drivers to
> expose device-specific features.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |  16 +++++++
>  drivers/gpu/drm/drm_blend.c  | 108 +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_blend.h      |   4 ++
>  include/drm/drm_plane.h      |  28 ++++++++++-
>  4 files changed, 155 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 37445d50816a..4f57ec25e04d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -756,6 +756,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->rotation = val;
>  	} else if (property == plane->zpos_property) {
>  		state->zpos = val;
> +	} else if (property == plane->colorkey.mode_property) {
> +		state->colorkey.mode = val;
> +	} else if (property == plane->colorkey.min_property) {
> +		state->colorkey.min = val;
> +	} else if (property == plane->colorkey.max_property) {
> +		state->colorkey.max = val;
> +	} else if (property == plane->colorkey.value_property) {
> +		state->colorkey.value = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -815,6 +823,14 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->rotation;
>  	} else if (property == plane->zpos_property) {
>  		*val = state->zpos;
> +	} else if (property == plane->colorkey.mode_property) {
> +		*val = state->colorkey.mode;
> +	} else if (property == plane->colorkey.min_property) {
> +		*val = state->colorkey.min;
> +	} else if (property == plane->colorkey.max_property) {
> +		*val = state->colorkey.max;
> +	} else if (property == plane->colorkey.value_property) {
> +		*val = state->colorkey.value;
>  	} 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_blend.c b/drivers/gpu/drm/drm_blend.c
> index 2e5e089dd912..79da7d8a22e2 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -98,6 +98,10 @@
>   *   planes. Without this property the primary plane is always below the cursor
>   *   plane, and ordering between all other planes is undefined.
>   *
> + * - Color keying is set up with drm_plane_create_colorkey_properties(). It adds
> + *   support for replacing a range of colors with a transparent color in the
> + *   plane.
> + *
>   * 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).
> @@ -405,3 +409,107 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_normalize_zpos);
> +
> +/**
> + * drm_plane_create_colorkey_properties - create colorkey properties
> + * @plane: drm plane
> + * @modes: array of supported color keying modes
> + * @num_modes: number of modes in the modes array
> + * @replace: if true create the colorkey.replacement property
> + *
> + * This function creates the generic color keying properties and attach them to
> + * the plane to enable color keying control for blending operations.
> + *
> + * Color keying is controlled through four properties:
> + *
> + * colorkey.mode:
> + *	The mode is an enumerated property that controls how color keying
> + *	operates. Modes are driver-specific, except for a "disabled" mode that
> + *	disables color keying and is guaranteed to exist if color keying is
> + *	supported.

I don't see why we need driver specific modes. It should be possible to
come up with some standard modes. I suppose a simple src vs. dst doesn't
suffice, but if we combine that with a bit more information it should be
good enough? Eg. i915 would expose something like src-min-max and
dst-value-mask.

> + *
> + * colorkey.min, colorkey.max:
> + *	Those two properties specify the colors that are replaced by transparent
> + *	pixels. Pixel whose values are in the [min, max] range are replaced, all
> + *	other pixels are left untouched. The minimum and maximum values are
> + *	expressed as a 64-bit integer in AXYZ16161616 format, where A is the
> + *	alpha value and X, Y and Z correspond to the color components of the
> + *	plane's pixel format. In most cases XYZ will be either RGB or YUV.
> + *
> + *	When a single color key is supported instead of a range, userspace shall
> + *	set the min and max properties to the same value, and drivers return an
> + *	error from their plane atomic check when the min and max values differ.
> + *
> + *	Note that depending on the selected mode, not all components might be
> + *	used for comparison. For instance a device could support color keying in
> + *	YUV format using luma (Y) matching only, ignoring the chroma components.
> + *	This behaviour is driver-specific.

This doesn't quite seem to support the common value+mask approach.

Your luma matching thing could maybe be expressed via the mask by
just setting both the key value and mask to zero for any component
that shouldn't participate in the match. Although I suppose for i915
we'd need to have a src-min-max-mask type of mode for that, and we
couldn't actually support arbitrary masks in that case (just 0 and ~0).
So maybe there should be separate mode for that to indicate that
particular restriction.

> + *
> + * colorkey.value:

"value" makes me think of the key value. replacement-value or some
such would be a bit more descriptive.

> + *	This property specifies the color value that replaces pixels matching
> + *	the [min, max] range. The value is expressed in AXYZ16161616 format as
> + *	the min and max properties.
> + *
> + *	This property is optional and only present when the device supports
> + *	configurable color replacement for matching pixels in the plane. If
> + *	color keying capabilities of the device are limited to making the
> + *	matching pixels fully transparent the colorkey.value property won't be
> + *	created.
> + *
> + *	Note that depending on the device, or the selected mode, not all
> + *	components might be used for value replacement. For instance a device
> + *	could support replacing the alpha value of the matching pixels but not
> + *	its color components. This behaviour is driver-specific.
> + *
> + * The @modes parameter points to an array of all color keying modes supported
> + * by the plane. The first mode has to be named "disabled" and have value 0. All
> + * other modes are driver-specific, and at least one mode has to be provided in
> + * addition to the "disabled" mode.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_plane_create_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace)
> +{
> +#define CREATE_COLORKEY_PROP(plane, name, type, args...) ({		       \
> +	prop = drm_property_create_##type(plane->dev, 0, "colorkey." #name,    \
> +					  args);			       \
> +	if (prop) {							       \
> +		drm_object_attach_property(&plane->base, prop, 0);	       \
> +		plane->colorkey.name##_property = prop;			       \
> +	}								       \
> +	prop;								       \
> +})
> +
> +	struct drm_property *prop;
> +
> +	/*
> +	 * A minimum of two modes are required, with the first mode must named
> +	 * "disabled".
> +	 */
> +	if (!modes || num_modes == 0 || strcmp(modes[0].name, "disabled"))
> +		return -EINVAL;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, mode, enum, modes, num_modes);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, min, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, max, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	if (replace) {
> +		prop = CREATE_COLORKEY_PROP(plane, value, range, 0, U64_MAX);
> +		if (!prop)
> +			return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_colorkey_properties);
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 17606026590b..1d4c965dd5e2 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -49,4 +49,8 @@ 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_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace);
>  #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 8185e3468a23..a5804a4ea5c3 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -52,6 +52,13 @@ struct drm_modeset_acquire_ctx;
>   *	where N is the number of active planes for given crtc. Note that
>   *	the driver must call drm_atomic_normalize_zpos() to update this before
>   *	it can be trusted.
> + * @colorkey.mode: color key mode. 0 disabled color keying, other values are
> + *	driver-specific.
> + * @colorkey.min: color key range minimum. The value is stored in AXYZ16161616
> + *	format, where A is the alpha value and X, Y and Z correspond to the
> + *	color components of the plane's pixel format (usually RGB or YUV).
> + * @colorkey.max: color key range maximum (in AXYZ16161616 format)
> + * @colorkey.value: color key replacement value (in in AXYZ16161616 format)
>   * @src: clipped source coordinates of the plane (in 16.16)
>   * @dst: clipped destination coordinates of the plane
>   * @state: backpointer to global drm_atomic_state
> @@ -112,6 +119,14 @@ struct drm_plane_state {
>  	unsigned int zpos;
>  	unsigned int normalized_zpos;
>  
> +	/* Plane colorkey */
> +	struct {
> +		unsigned int mode;
> +		u64 min;
> +		u64 max;
> +		u64 value;
> +	} colorkey;
> +
>  	/* Clipped coordinates */
>  	struct drm_rect src, dst;
>  
> @@ -481,9 +496,13 @@ enum drm_plane_type {
>   * @funcs: helper functions
>   * @properties: property tracking for this plane
>   * @type: type of plane (overlay, primary, cursor)
> + * @helper_private: mid-layer private data
>   * @zpos_property: zpos property for this plane
>   * @rotation_property: rotation property for this plane
> - * @helper_private: mid-layer private data
> + * @colorkey.mode_property: color key mode property
> + * @colorkey.min_property: color key range minimum property
> + * @colorkey.max_property: color key range maximum property
> + * @colorkey.value_property: color key replacement value property
>   */
>  struct drm_plane {
>  	struct drm_device *dev;
> @@ -558,6 +577,13 @@ struct drm_plane {
>  
>  	struct drm_property *zpos_property;
>  	struct drm_property *rotation_property;
> +
> +	struct {
> +		struct drm_property *mode_property;
> +		struct drm_property *min_property;
> +		struct drm_property *max_property;
> +		struct drm_property *value_property;
> +	} colorkey;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Ville Syrjälä]

On Sun, Dec 17, 2017 at 02:17:21AM +0200, Laurent Pinchart wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent, or gain a
> programmable alpha value.
> 
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds four properties
> related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> and colorkey.mode. Additional properties can be defined by drivers to
> expose device-specific features.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |  16 +++++++
>  drivers/gpu/drm/drm_blend.c  | 108 +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_blend.h      |   4 ++
>  include/drm/drm_plane.h      |  28 ++++++++++-
>  4 files changed, 155 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 37445d50816a..4f57ec25e04d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -756,6 +756,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->rotation = val;
>  	} else if (property == plane->zpos_property) {
>  		state->zpos = val;
> +	} else if (property == plane->colorkey.mode_property) {
> +		state->colorkey.mode = val;
> +	} else if (property == plane->colorkey.min_property) {
> +		state->colorkey.min = val;
> +	} else if (property == plane->colorkey.max_property) {
> +		state->colorkey.max = val;
> +	} else if (property == plane->colorkey.value_property) {
> +		state->colorkey.value = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -815,6 +823,14 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->rotation;
>  	} else if (property == plane->zpos_property) {
>  		*val = state->zpos;
> +	} else if (property == plane->colorkey.mode_property) {
> +		*val = state->colorkey.mode;
> +	} else if (property == plane->colorkey.min_property) {
> +		*val = state->colorkey.min;
> +	} else if (property == plane->colorkey.max_property) {
> +		*val = state->colorkey.max;
> +	} else if (property == plane->colorkey.value_property) {
> +		*val = state->colorkey.value;
>  	} 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_blend.c b/drivers/gpu/drm/drm_blend.c
> index 2e5e089dd912..79da7d8a22e2 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -98,6 +98,10 @@
>   *   planes. Without this property the primary plane is always below the cursor
>   *   plane, and ordering between all other planes is undefined.
>   *
> + * - Color keying is set up with drm_plane_create_colorkey_properties(). It adds
> + *   support for replacing a range of colors with a transparent color in the
> + *   plane.
> + *
>   * 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).
> @@ -405,3 +409,107 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_normalize_zpos);
> +
> +/**
> + * drm_plane_create_colorkey_properties - create colorkey properties
> + * @plane: drm plane
> + * @modes: array of supported color keying modes
> + * @num_modes: number of modes in the modes array
> + * @replace: if true create the colorkey.replacement property
> + *
> + * This function creates the generic color keying properties and attach them to
> + * the plane to enable color keying control for blending operations.
> + *
> + * Color keying is controlled through four properties:
> + *
> + * colorkey.mode:
> + *	The mode is an enumerated property that controls how color keying
> + *	operates. Modes are driver-specific, except for a "disabled" mode that
> + *	disables color keying and is guaranteed to exist if color keying is
> + *	supported.

I don't see why we need driver specific modes. It should be possible to
come up with some standard modes. I suppose a simple src vs. dst doesn't
suffice, but if we combine that with a bit more information it should be
good enough? Eg. i915 would expose something like src-min-max and
dst-value-mask.

> + *
> + * colorkey.min, colorkey.max:
> + *	Those two properties specify the colors that are replaced by transparent
> + *	pixels. Pixel whose values are in the [min, max] range are replaced, all
> + *	other pixels are left untouched. The minimum and maximum values are
> + *	expressed as a 64-bit integer in AXYZ16161616 format, where A is the
> + *	alpha value and X, Y and Z correspond to the color components of the
> + *	plane's pixel format. In most cases XYZ will be either RGB or YUV.
> + *
> + *	When a single color key is supported instead of a range, userspace shall
> + *	set the min and max properties to the same value, and drivers return an
> + *	error from their plane atomic check when the min and max values differ.
> + *
> + *	Note that depending on the selected mode, not all components might be
> + *	used for comparison. For instance a device could support color keying in
> + *	YUV format using luma (Y) matching only, ignoring the chroma components.
> + *	This behaviour is driver-specific.

This doesn't quite seem to support the common value+mask approach.

Your luma matching thing could maybe be expressed via the mask by
just setting both the key value and mask to zero for any component
that shouldn't participate in the match. Although I suppose for i915
we'd need to have a src-min-max-mask type of mode for that, and we
couldn't actually support arbitrary masks in that case (just 0 and ~0).
So maybe there should be separate mode for that to indicate that
particular restriction.

> + *
> + * colorkey.value:

"value" makes me think of the key value. replacement-value or some
such would be a bit more descriptive.

> + *	This property specifies the color value that replaces pixels matching
> + *	the [min, max] range. The value is expressed in AXYZ16161616 format as
> + *	the min and max properties.
> + *
> + *	This property is optional and only present when the device supports
> + *	configurable color replacement for matching pixels in the plane. If
> + *	color keying capabilities of the device are limited to making the
> + *	matching pixels fully transparent the colorkey.value property won't be
> + *	created.
> + *
> + *	Note that depending on the device, or the selected mode, not all
> + *	components might be used for value replacement. For instance a device
> + *	could support replacing the alpha value of the matching pixels but not
> + *	its color components. This behaviour is driver-specific.
> + *
> + * The @modes parameter points to an array of all color keying modes supported
> + * by the plane. The first mode has to be named "disabled" and have value 0. All
> + * other modes are driver-specific, and at least one mode has to be provided in
> + * addition to the "disabled" mode.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_plane_create_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace)
> +{
> +#define CREATE_COLORKEY_PROP(plane, name, type, args...) ({		       \
> +	prop = drm_property_create_##type(plane->dev, 0, "colorkey." #name,    \
> +					  args);			       \
> +	if (prop) {							       \
> +		drm_object_attach_property(&plane->base, prop, 0);	       \
> +		plane->colorkey.name##_property = prop;			       \
> +	}								       \
> +	prop;								       \
> +})
> +
> +	struct drm_property *prop;
> +
> +	/*
> +	 * A minimum of two modes are required, with the first mode must named
> +	 * "disabled".
> +	 */
> +	if (!modes || num_modes == 0 || strcmp(modes[0].name, "disabled"))
> +		return -EINVAL;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, mode, enum, modes, num_modes);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, min, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, max, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	if (replace) {
> +		prop = CREATE_COLORKEY_PROP(plane, value, range, 0, U64_MAX);
> +		if (!prop)
> +			return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_colorkey_properties);
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 17606026590b..1d4c965dd5e2 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -49,4 +49,8 @@ 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_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace);
>  #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 8185e3468a23..a5804a4ea5c3 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -52,6 +52,13 @@ struct drm_modeset_acquire_ctx;
>   *	where N is the number of active planes for given crtc. Note that
>   *	the driver must call drm_atomic_normalize_zpos() to update this before
>   *	it can be trusted.
> + * @colorkey.mode: color key mode. 0 disabled color keying, other values are
> + *	driver-specific.
> + * @colorkey.min: color key range minimum. The value is stored in AXYZ16161616
> + *	format, where A is the alpha value and X, Y and Z correspond to the
> + *	color components of the plane's pixel format (usually RGB or YUV).
> + * @colorkey.max: color key range maximum (in AXYZ16161616 format)
> + * @colorkey.value: color key replacement value (in in AXYZ16161616 format)
>   * @src: clipped source coordinates of the plane (in 16.16)
>   * @dst: clipped destination coordinates of the plane
>   * @state: backpointer to global drm_atomic_state
> @@ -112,6 +119,14 @@ struct drm_plane_state {
>  	unsigned int zpos;
>  	unsigned int normalized_zpos;
>  
> +	/* Plane colorkey */
> +	struct {
> +		unsigned int mode;
> +		u64 min;
> +		u64 max;
> +		u64 value;
> +	} colorkey;
> +
>  	/* Clipped coordinates */
>  	struct drm_rect src, dst;
>  
> @@ -481,9 +496,13 @@ enum drm_plane_type {
>   * @funcs: helper functions
>   * @properties: property tracking for this plane
>   * @type: type of plane (overlay, primary, cursor)
> + * @helper_private: mid-layer private data
>   * @zpos_property: zpos property for this plane
>   * @rotation_property: rotation property for this plane
> - * @helper_private: mid-layer private data
> + * @colorkey.mode_property: color key mode property
> + * @colorkey.min_property: color key range minimum property
> + * @colorkey.max_property: color key range maximum property
> + * @colorkey.value_property: color key replacement value property
>   */
>  struct drm_plane {
>  	struct drm_device *dev;
> @@ -558,6 +577,13 @@ struct drm_plane {
>  
>  	struct drm_property *zpos_property;
>  	struct drm_property *rotation_property;
> +
> +	struct {
> +		struct drm_property *mode_property;
> +		struct drm_property *min_property;
> +		struct drm_property *max_property;
> +		struct drm_property *value_property;
> +	} colorkey;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrj�l�
Intel

Re: [PATCH/RFC 1/4] drm: Add colorkey properties

[Ville Syrjälä]

On Sun, Dec 17, 2017 at 02:17:21AM +0200, Laurent Pinchart wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent, or gain a
> programmable alpha value.
> 
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds four properties
> related to color keying named colorkey.min, colorkey.max, colorkey.alpha
> and colorkey.mode. Additional properties can be defined by drivers to
> expose device-specific features.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |  16 +++++++
>  drivers/gpu/drm/drm_blend.c  | 108 +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_blend.h      |   4 ++
>  include/drm/drm_plane.h      |  28 ++++++++++-
>  4 files changed, 155 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 37445d50816a..4f57ec25e04d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -756,6 +756,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->rotation = val;
>  	} else if (property == plane->zpos_property) {
>  		state->zpos = val;
> +	} else if (property == plane->colorkey.mode_property) {
> +		state->colorkey.mode = val;
> +	} else if (property == plane->colorkey.min_property) {
> +		state->colorkey.min = val;
> +	} else if (property == plane->colorkey.max_property) {
> +		state->colorkey.max = val;
> +	} else if (property == plane->colorkey.value_property) {
> +		state->colorkey.value = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -815,6 +823,14 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->rotation;
>  	} else if (property == plane->zpos_property) {
>  		*val = state->zpos;
> +	} else if (property == plane->colorkey.mode_property) {
> +		*val = state->colorkey.mode;
> +	} else if (property == plane->colorkey.min_property) {
> +		*val = state->colorkey.min;
> +	} else if (property == plane->colorkey.max_property) {
> +		*val = state->colorkey.max;
> +	} else if (property == plane->colorkey.value_property) {
> +		*val = state->colorkey.value;
>  	} 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_blend.c b/drivers/gpu/drm/drm_blend.c
> index 2e5e089dd912..79da7d8a22e2 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -98,6 +98,10 @@
>   *   planes. Without this property the primary plane is always below the cursor
>   *   plane, and ordering between all other planes is undefined.
>   *
> + * - Color keying is set up with drm_plane_create_colorkey_properties(). It adds
> + *   support for replacing a range of colors with a transparent color in the
> + *   plane.
> + *
>   * 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).
> @@ -405,3 +409,107 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_normalize_zpos);
> +
> +/**
> + * drm_plane_create_colorkey_properties - create colorkey properties
> + * @plane: drm plane
> + * @modes: array of supported color keying modes
> + * @num_modes: number of modes in the modes array
> + * @replace: if true create the colorkey.replacement property
> + *
> + * This function creates the generic color keying properties and attach them to
> + * the plane to enable color keying control for blending operations.
> + *
> + * Color keying is controlled through four properties:
> + *
> + * colorkey.mode:
> + *	The mode is an enumerated property that controls how color keying
> + *	operates. Modes are driver-specific, except for a "disabled" mode that
> + *	disables color keying and is guaranteed to exist if color keying is
> + *	supported.

I don't see why we need driver specific modes. It should be possible to
come up with some standard modes. I suppose a simple src vs. dst doesn't
suffice, but if we combine that with a bit more information it should be
good enough? Eg. i915 would expose something like src-min-max and
dst-value-mask.

> + *
> + * colorkey.min, colorkey.max:
> + *	Those two properties specify the colors that are replaced by transparent
> + *	pixels. Pixel whose values are in the [min, max] range are replaced, all
> + *	other pixels are left untouched. The minimum and maximum values are
> + *	expressed as a 64-bit integer in AXYZ16161616 format, where A is the
> + *	alpha value and X, Y and Z correspond to the color components of the
> + *	plane's pixel format. In most cases XYZ will be either RGB or YUV.
> + *
> + *	When a single color key is supported instead of a range, userspace shall
> + *	set the min and max properties to the same value, and drivers return an
> + *	error from their plane atomic check when the min and max values differ.
> + *
> + *	Note that depending on the selected mode, not all components might be
> + *	used for comparison. For instance a device could support color keying in
> + *	YUV format using luma (Y) matching only, ignoring the chroma components.
> + *	This behaviour is driver-specific.

This doesn't quite seem to support the common value+mask approach.

Your luma matching thing could maybe be expressed via the mask by
just setting both the key value and mask to zero for any component
that shouldn't participate in the match. Although I suppose for i915
we'd need to have a src-min-max-mask type of mode for that, and we
couldn't actually support arbitrary masks in that case (just 0 and ~0).
So maybe there should be separate mode for that to indicate that
particular restriction.

> + *
> + * colorkey.value:

"value" makes me think of the key value. replacement-value or some
such would be a bit more descriptive.

> + *	This property specifies the color value that replaces pixels matching
> + *	the [min, max] range. The value is expressed in AXYZ16161616 format as
> + *	the min and max properties.
> + *
> + *	This property is optional and only present when the device supports
> + *	configurable color replacement for matching pixels in the plane. If
> + *	color keying capabilities of the device are limited to making the
> + *	matching pixels fully transparent the colorkey.value property won't be
> + *	created.
> + *
> + *	Note that depending on the device, or the selected mode, not all
> + *	components might be used for value replacement. For instance a device
> + *	could support replacing the alpha value of the matching pixels but not
> + *	its color components. This behaviour is driver-specific.
> + *
> + * The @modes parameter points to an array of all color keying modes supported
> + * by the plane. The first mode has to be named "disabled" and have value 0. All
> + * other modes are driver-specific, and at least one mode has to be provided in
> + * addition to the "disabled" mode.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_plane_create_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace)
> +{
> +#define CREATE_COLORKEY_PROP(plane, name, type, args...) ({		       \
> +	prop = drm_property_create_##type(plane->dev, 0, "colorkey." #name,    \
> +					  args);			       \
> +	if (prop) {							       \
> +		drm_object_attach_property(&plane->base, prop, 0);	       \
> +		plane->colorkey.name##_property = prop;			       \
> +	}								       \
> +	prop;								       \
> +})
> +
> +	struct drm_property *prop;
> +
> +	/*
> +	 * A minimum of two modes are required, with the first mode must named
> +	 * "disabled".
> +	 */
> +	if (!modes || num_modes == 0 || strcmp(modes[0].name, "disabled"))
> +		return -EINVAL;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, mode, enum, modes, num_modes);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, min, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	prop = CREATE_COLORKEY_PROP(plane, max, range, 0, U64_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	if (replace) {
> +		prop = CREATE_COLORKEY_PROP(plane, value, range, 0, U64_MAX);
> +		if (!prop)
> +			return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_colorkey_properties);
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 17606026590b..1d4c965dd5e2 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -49,4 +49,8 @@ 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_colorkey_properties(struct drm_plane *plane,
> +					 const struct drm_prop_enum_list *modes,
> +					 unsigned int num_modes, bool replace);
>  #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 8185e3468a23..a5804a4ea5c3 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -52,6 +52,13 @@ struct drm_modeset_acquire_ctx;
>   *	where N is the number of active planes for given crtc. Note that
>   *	the driver must call drm_atomic_normalize_zpos() to update this before
>   *	it can be trusted.
> + * @colorkey.mode: color key mode. 0 disabled color keying, other values are
> + *	driver-specific.
> + * @colorkey.min: color key range minimum. The value is stored in AXYZ16161616
> + *	format, where A is the alpha value and X, Y and Z correspond to the
> + *	color components of the plane's pixel format (usually RGB or YUV).
> + * @colorkey.max: color key range maximum (in AXYZ16161616 format)
> + * @colorkey.value: color key replacement value (in in AXYZ16161616 format)
>   * @src: clipped source coordinates of the plane (in 16.16)
>   * @dst: clipped destination coordinates of the plane
>   * @state: backpointer to global drm_atomic_state
> @@ -112,6 +119,14 @@ struct drm_plane_state {
>  	unsigned int zpos;
>  	unsigned int normalized_zpos;
>  
> +	/* Plane colorkey */
> +	struct {
> +		unsigned int mode;
> +		u64 min;
> +		u64 max;
> +		u64 value;
> +	} colorkey;
> +
>  	/* Clipped coordinates */
>  	struct drm_rect src, dst;
>  
> @@ -481,9 +496,13 @@ enum drm_plane_type {
>   * @funcs: helper functions
>   * @properties: property tracking for this plane
>   * @type: type of plane (overlay, primary, cursor)
> + * @helper_private: mid-layer private data
>   * @zpos_property: zpos property for this plane
>   * @rotation_property: rotation property for this plane
> - * @helper_private: mid-layer private data
> + * @colorkey.mode_property: color key mode property
> + * @colorkey.min_property: color key range minimum property
> + * @colorkey.max_property: color key range maximum property
> + * @colorkey.value_property: color key replacement value property
>   */
>  struct drm_plane {
>  	struct drm_device *dev;
> @@ -558,6 +577,13 @@ struct drm_plane {
>  
>  	struct drm_property *zpos_property;
>  	struct drm_property *rotation_property;
> +
> +	struct {
> +		struct drm_property *mode_property;
> +		struct drm_property *min_property;
> +		struct drm_property *max_property;
> +		struct drm_property *value_property;
> +	} colorkey;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel