[PATCH v1 0/5] drm/vkms: Add YUV support to writeback connector

[PATCH v1 0/5] drm/vkms: Add YUV support to writeback connector

[Robert Mader]

This series implements support for various YUV/YCbCr formats in the vkms
writeback connector. The goal is to facilitate driver development of hardware
writeback connectors that (only) support these formats. In order to do so the
series adds two new optional connector properties, WRITEBACK_COLOR_ENCODING and
WRITEBACK_COLOR_RANGE, modeled after the existing plane properties
COLOR_ENCODING and COLOR_RANGE. These are needed by user space clients to know -
and optionally control - the matrix coefficients and quantization range for YUV
to RGB conversions.

While I'm aware of plans to deprecate the COLOR_ENCODING and COLOR_RANGE plane
properties in the context of ongoing color work, it is unclear if and when the
successor will get implemented in vkms. Thus, and in light of the optional and
non-invasive nature of the new properties, I hope there is no strong resistance
against the later.

The implementation tries to reuse and mirror the existing YUV support for planes
as much as possible in order to minimize maintenance burden.

On the user space side a Weston compositor and client implementation can be
found here:
https://gitlab.freedesktop.org/rmader/weston/-/commits/vkms-writeback-yuv-v1

It can be tested with the following steps:
1. Run `weston --debug --drm-device=card0` from a TTY to run Weston with vkms
   and the screenshot protocol enabled.
2. Run e.g. `WAYLAND_DISPLAY=wayland-1 weston-screenshooter -s 1 -b 2 -c 2 -r 1 -f P010`
   in order to capture a screenshot with DRM_FORMAT_P010, ITU-R BT.2020 YCbCr
   color encoding and YCbCr full range.
3. The resulting .yuv files for now lack metadata, but can be opened by various
   clients such as ffmpeg or YUView (see link below) when adding the information
   about the used format, size and encoding/range.
   See https://flathub.org/apps/de.rwth_aachen.ient.YUView

Given that Mesas gbm implementation does not support allocation YUV buffers yet,
the client implementation (weston-screenshooter) uses udmabuf instead. This
approach should work with any writeback connector that has an IOMMU and supports
buffer import from virtual system memory.

The series is build on top of "drm/vkms: Add support for multiple plane formats"
v7, which adds plane/input support for formats like DRM_FORMAT_P010 that are
also covered, see
https://lore.kernel.org/dri-devel/20250703-b4-new-color-formats-v7-0-15fd8fd2e15c@bootlin.com/

Thus I'm taking the freedom to CC everyone involved there, hope that's ok :)

Additional notes:
 - Weston requires a successful gbm_bo_import() of the buffer. Thus when
   testing with llvmpipe, a recent Mesa version (>=25.2) is required. For the
   formats DRM_FORMAT_NV61, DRM_FORMAT_NV24 and DRM_FORMAT_NV42 support is not
   upstream yet, but the following branch can be used for testing:
   https://gitlab.freedesktop.org/rmader/mesa/-/commits/yuv-formats-nv61-nv24-nv42

It is co-developed with a Weston implementation

Robert Mader (5):
  drm: Add WRITEBACK_COLOR_ENCODING and WRITEBACK_COLOR_RANGE properties
  drm/vkms: Add RGB to YUV conversion matrices
  drm/vkms: Add WRITEBACK_COLOR_ENCODING and WRITEBACK_COLOR_RANGE
    properties
  drm/vkms: Add support for writing semiplanar YUV/YCbCr formats
  drm/vkms: Add YUV formats to writeback connector

 drivers/gpu/drm/drm_atomic_uapi.c             |   8 +
 drivers/gpu/drm/drm_color_mgmt.c              |  90 +++++
 drivers/gpu/drm/vkms/tests/vkms_format_test.c |  51 ++-
 drivers/gpu/drm/vkms/vkms_drv.h               |  51 ++-
 drivers/gpu/drm/vkms/vkms_formats.c           | 325 +++++++++++++++++-
 drivers/gpu/drm/vkms/vkms_formats.h           |   7 +
 drivers/gpu/drm/vkms/vkms_writeback.c         |  68 +++-
 include/drm/drm_color_mgmt.h                  |   7 +
 include/drm/drm_connector.h                   |  17 +
 include/drm/drm_mode_config.h                 |  14 +
 10 files changed, 591 insertions(+), 47 deletions(-)

-- 
2.50.1

[PATCH v1 1/5] drm: Add WRITEBACK_COLOR_ENCODING and WRITEBACK_COLOR_RANGE properties

[Robert Mader]

Essentially copying the COLOR_ENCODING and COLOR_RANGE plane properties,
reusing the drm_color_encoding and drm_color_range enums. This
allows us to advertise and configure the matrix coefficients and ranges
used to convert between RGB and YCbCr formats and thus helps writeback
connector implementations that want to advertise YCbCr formats.

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  8 +++
 drivers/gpu/drm/drm_color_mgmt.c  | 90 +++++++++++++++++++++++++++++++
 include/drm/drm_color_mgmt.h      |  7 +++
 include/drm/drm_connector.h       | 17 ++++++
 include/drm/drm_mode_config.h     | 14 +++++
 5 files changed, 136 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index ecc73d52bfae..12379673dea1 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -774,6 +774,10 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 
 		return set_out_fence_for_connector(state->state, connector,
 						   fence_ptr);
+	} else if (property == config->writeback_color_encoding) {
+		state->writeback_color_encoding = val;
+	} else if (property == config->writeback_color_range) {
+		state->writeback_color_range = val;
 	} else if (property == connector->max_bpc_property) {
 		state->max_requested_bpc = val;
 	} else if (property == connector->privacy_screen_sw_state_property) {
@@ -859,6 +863,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = 0;
 	} else if (property == config->writeback_out_fence_ptr_property) {
 		*val = 0;
+	} else if (property == config->writeback_color_encoding) {
+		*val = state->writeback_color_encoding;
+	} else if (property == config->writeback_color_range) {
+		*val = state->writeback_color_range;
 	} else if (property == connector->max_bpc_property) {
 		*val = state->max_requested_bpc;
 	} else if (property == connector->privacy_screen_sw_state_property) {
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 37a3270bc3c2..04444c215bf4 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -29,6 +29,7 @@
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_print.h>
+#include <drm/drm_writeback.h>
 #include <kunit/visibility.h>
 
 #include "drm_crtc_internal.h"
@@ -592,6 +593,95 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
 }
 EXPORT_SYMBOL(drm_plane_create_color_properties);
 
+/**
+ * drm_writeback_create_color_properties - color encoding related writeback connector properties
+ * @wb_connector: writeback connector object
+ * @supported_encodings: bitfield indicating supported color encodings
+ * @supported_ranges: bitfileld indicating supported color ranges
+ * @default_encoding: default color encoding
+ * @default_range: default color range
+ *
+ * Create and attach writeback connector specific WRITEBACK_COLOR_ENCODING and
+ * WRITEBACK_COLOR_RANGE properties to @wb_connector. The supported encodings
+ * and ranges should be provided in supported_encodings and supported_ranges
+ * bitmasks. Each bit set in the bitmask indicates that its number as enum
+ * value is supported.
+ */
+int drm_writeback_create_color_properties(struct drm_writeback_connector *wb_connector,
+					  u32 supported_encodings,
+					  u32 supported_ranges,
+					  enum drm_color_encoding default_encoding,
+					  enum drm_color_range default_range)
+{
+	struct drm_connector *connector = &wb_connector->base;
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+	struct drm_prop_enum_list enum_list[MAX_T(int, DRM_COLOR_ENCODING_MAX,
+						       DRM_COLOR_RANGE_MAX)];
+	int i, len;
+
+	if (WARN_ON(supported_encodings == 0 ||
+		    (supported_encodings & -BIT(DRM_COLOR_ENCODING_MAX)) != 0 ||
+		    (supported_encodings & BIT(default_encoding)) == 0))
+		return -EINVAL;
+
+	if (WARN_ON(supported_ranges == 0 ||
+		    (supported_ranges & -BIT(DRM_COLOR_RANGE_MAX)) != 0 ||
+		    (supported_ranges & BIT(default_range)) == 0))
+		return -EINVAL;
+
+	if (!dev->mode_config.writeback_color_encoding) {
+		len = 0;
+		for (i = 0; i < DRM_COLOR_ENCODING_MAX; i++) {
+			if ((supported_encodings & BIT(i)) == 0)
+				continue;
+
+			enum_list[len].type = i;
+			enum_list[len].name = color_encoding_name[i];
+			len++;
+		}
+
+		prop = drm_property_create_enum(dev, 0,
+						"WRITEBACK_COLOR_ENCODING",
+						enum_list, len);
+		if (!prop)
+			return -ENOMEM;
+		dev->mode_config.writeback_color_encoding = prop;
+	}
+
+	drm_object_attach_property(&connector->base,
+				   dev->mode_config.writeback_color_encoding,
+				   default_encoding);
+	if (connector->state)
+		connector->state->writeback_color_encoding = default_encoding;
+
+	if (!dev->mode_config.writeback_color_range) {
+		len = 0;
+		for (i = 0; i < DRM_COLOR_RANGE_MAX; i++) {
+			if ((supported_ranges & BIT(i)) == 0)
+				continue;
+
+			enum_list[len].type = i;
+			enum_list[len].name = color_range_name[i];
+			len++;
+		}
+
+		prop = drm_property_create_enum(dev, 0, "WRITEBACK_COLOR_RANGE",
+						enum_list, len);
+		if (!prop)
+			return -ENOMEM;
+		dev->mode_config.writeback_color_range = prop;
+	}
+	drm_object_attach_property(&connector->base,
+				   dev->mode_config.writeback_color_range,
+				   default_range);
+	if (connector->state)
+		connector->state->writeback_color_range = default_range;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_writeback_create_color_properties);
+
 /**
  * drm_color_lut_check - check validity of lookup table
  * @lut: property blob containing LUT to check
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 6cb577f6dba6..60146e3063fd 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -29,6 +29,7 @@
 
 struct drm_crtc;
 struct drm_plane;
+struct drm_writeback_connector;
 
 /**
  * drm_color_lut_extract - clamp and round LUT entries
@@ -91,6 +92,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
 				      enum drm_color_encoding default_encoding,
 				      enum drm_color_range default_range);
 
+int drm_writeback_create_color_properties(struct drm_writeback_connector *wb_connector,
+					  u32 supported_encodings,
+					  u32 supported_ranges,
+					  enum drm_color_encoding default_encoding,
+					  enum drm_color_range default_range);
+
 /**
  * enum drm_color_lut_tests - hw-specific LUT tests to perform
  *
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8f34f4b8183d..178bfbcb13e3 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -28,6 +28,7 @@
 #include <linux/ctype.h>
 #include <linux/hdmi.h>
 #include <linux/notifier.h>
+#include <drm/drm_color_mgmt.h>
 #include <drm/drm_mode_object.h>
 #include <drm/drm_util.h>
 #include <drm/drm_property.h>
@@ -1120,6 +1121,22 @@ struct drm_connector_state {
 	 */
 	struct drm_writeback_job *writeback_job;
 
+	/**
+	 * @writeback_color_encoding:
+	 *
+	 * Color encoding for non RGB formats used by writeback connectors.
+	 * See also: drm_writeback_create_color_properties()
+	 */
+	enum drm_color_encoding writeback_color_encoding;
+
+	/**
+	 * @writeback_color_range:
+	 *
+	 * Color encoding for non RGB formats used by writeback connectors.
+	 * See also: drm_writeback_create_color_properties()
+	 */
+	enum drm_color_range writeback_color_range;
+
 	/**
 	 * @max_requested_bpc: Connector property to limit the maximum bit
 	 * depth of the pixels.
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 2e848b816218..1d2a7cb6d410 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -863,6 +863,20 @@ struct drm_mode_config {
 	 */
 	struct drm_property *writeback_out_fence_ptr_property;
 
+	/**
+	 * @writeback_color_encoding: Optional property for writeback
+	 * connectors, color encoding for non RGB formats.
+	 * See also: drm_writeback_create_color_properties()
+	 */
+	struct drm_property *writeback_color_encoding;
+
+	/**
+	 * @writeback_color_range: Optional property for writeback connectors,
+	 * color range for non RGB formats.
+	 * See also: drm_writeback_create_color_properties()
+	 */
+	struct drm_property *writeback_color_range;
+
 	/**
 	 * @hdr_output_metadata_property: Connector property containing hdr
 	 * metatada. This will be provided by userspace compositors based
-- 
2.50.1

[PATCH v1 2/5] drm/vkms: Add RGB to YUV conversion matrices

[Robert Mader]

In preparation for YUV/YCbCr pixel format support in the writeback
connector. The implementation is meant to mirror the existing
get_conversion_matrix_to_argb_u16() as close as possible.

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 drivers/gpu/drm/vkms/vkms_formats.c | 153 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_formats.h |   4 +
 2 files changed, 157 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index dfb8e13cba87..560b56fbf4fb 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -787,6 +787,20 @@ static const struct conversion_matrix yuv_bt601_full = {
 	.y_offset = 0,
 };
 
+/*
+ * numpy.around(numpy.linalg.inv(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
+ *                                                   is_legal = False,
+ *                                                   bits = 8)) * 2**32).astype(int)
+ */
+static const struct conversion_matrix rgb_to_yuv_bt601_full = {
+	.matrix = {
+		{ 1284195222, 2521145803,  489626272 },
+		{ -724715136, -1422768512, 2147483648 },
+		{ 2147483648, -1798249503, -349234145 },
+	},
+	.y_offset = 0,
+};
+
 /*
  * numpy.around(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
  *                                  is_legal = True,
@@ -801,6 +815,20 @@ static const struct conversion_matrix yuv_bt601_limited = {
 	.y_offset = 16,
 };
 
+/*
+ * numpy.around(numpy.linalg.inv(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
+ *                                                   is_legal = True,
+ *                                                   bits = 8)) * 2**32).astype(int)
+ */
+static const struct conversion_matrix rgb_to_yuv_bt601_limited = {
+	.matrix = {
+		{ 1102897073, 2165219336,  420502563 },
+		{ -636612512, -1249804497, 1886417008 },
+		{ 1886417008, -1579638779, -306778230 },
+	},
+	.y_offset = 16,
+};
+
 /*
  * numpy.around(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
  *                                  is_legal = False,
@@ -815,6 +843,20 @@ static const struct conversion_matrix yuv_bt709_full = {
 	.y_offset = 0,
 };
 
+/*
+ * numpy.around(numpy.linalg.inv(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
+ *                                                   is_legal = False,
+ *                                                   bits = 8)) * 2**32).astype(int)
+ */
+static const struct conversion_matrix rgb_to_yuv_bt709_full = {
+	.matrix = {
+		{ 913110047,  3071760610,  310096639 },
+		{ -492083449, -1655400199, 2147483648 },
+		{ 2147483648, -1950571889, -196911759 },
+	},
+	.y_offset = 0,
+};
+
 /*
  * numpy.around(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
  *                                  is_legal = True,
@@ -829,6 +871,20 @@ static const struct conversion_matrix yuv_bt709_limited = {
 	.y_offset = 16,
 };
 
+/*
+ * numpy.around(numpy.linalg.inv(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
+ *                                                   is_legal = True,
+ *                                                   bits = 8)) * 2**32).astype(int)
+ */
+static const struct conversion_matrix rgb_to_yuv_bt709_limited = {
+	.matrix = {
+		{ 784200393,  2638100289,  266318290 },
+		{ -432261539, -1454155469, 1886417008 },
+		{ 1886417008, -1713443541, -172973467 },
+	},
+	.y_offset = 16,
+};
+
 /*
  * numpy.around(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
  *                                  is_legal = False,
@@ -843,6 +899,20 @@ static const struct conversion_matrix yuv_bt2020_full = {
 	.y_offset = 0,
 };
 
+/*
+ * numpy.around(numpy.linalg.inv(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
+ *                                                   is_legal = False,
+ *                                                   bits = 8)) * 2**32).astype(int)
+ */
+static const struct conversion_matrix rgb_to_yuv_bt2020_full = {
+	.matrix = {
+		{ 1128287909, 2911987827,  254691561 },
+		{ -599706553, -1547777095, 2147483648 },
+		{ 2147483648, -1974764564, -172719084 },
+	},
+	.y_offset = 0,
+};
+
 /*
  * numpy.around(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
  *                                  is_legal = True,
@@ -857,6 +927,20 @@ static const struct conversion_matrix yuv_bt2020_limited = {
 	.y_offset = 16,
 };
 
+/*
+ * numpy.around(numpy.linalg.inv(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
+ *                                                   is_legal = True,
+ *                                                   bits = 8)) * 2**32).astype(int)
+ */
+static const struct conversion_matrix rgb_to_yuv_bt2020_limited = {
+	.matrix = {
+		{ 969000204,  2500883663,  218735105 },
+		{ -526801050, -1359615958, 1886417008 },
+		{ 1886417008, -1734695147, -151721862 },
+	},
+	.y_offset = 16,
+};
+
 /**
  * swap_uv_columns() - Swap u and v column of a given matrix
  *
@@ -869,6 +953,18 @@ static void swap_uv_columns(struct conversion_matrix *matrix)
 	swap(matrix->matrix[2][2], matrix->matrix[2][1]);
 }
 
+/**
+ * swap_uv_columns_transposed() - Swap u and v column of a given matrix
+ *
+ * @matrix: Matrix in which column are swapped
+ */
+static void swap_uv_columns_transposed(struct conversion_matrix *matrix)
+{
+	swap(matrix->matrix[1][0], matrix->matrix[2][0]);
+	swap(matrix->matrix[1][1], matrix->matrix[2][1]);
+	swap(matrix->matrix[1][2], matrix->matrix[2][2]);
+}
+
 /**
  * get_conversion_matrix_to_argb_u16() - Retrieve the correct yuv to rgb conversion matrix for a
  * given encoding and range.
@@ -935,6 +1031,63 @@ void get_conversion_matrix_to_argb_u16(u32 format,
 }
 EXPORT_SYMBOL(get_conversion_matrix_to_argb_u16);
 
+void get_conversion_matrix_from_argb_u16(u32 format,
+					 enum drm_color_encoding encoding,
+					 enum drm_color_range range,
+					 struct conversion_matrix *matrix)
+{
+	const struct conversion_matrix *matrix_to_copy;
+	bool limited_range;
+
+	switch (range) {
+	case DRM_COLOR_YCBCR_LIMITED_RANGE:
+		limited_range = true;
+		break;
+	case DRM_COLOR_YCBCR_FULL_RANGE:
+		limited_range = false;
+		break;
+	case DRM_COLOR_RANGE_MAX:
+		limited_range = false;
+		WARN_ONCE(true, "The requested range is not supported.");
+		break;
+	}
+
+	switch (encoding) {
+	case DRM_COLOR_YCBCR_BT601:
+		matrix_to_copy = limited_range ? &rgb_to_yuv_bt601_limited :
+						 &rgb_to_yuv_bt601_full;
+		break;
+	case DRM_COLOR_YCBCR_BT709:
+		matrix_to_copy = limited_range ? &rgb_to_yuv_bt709_limited :
+						 &rgb_to_yuv_bt709_full;
+		break;
+	case DRM_COLOR_YCBCR_BT2020:
+		matrix_to_copy = limited_range ? &rgb_to_yuv_bt2020_limited :
+						 &rgb_to_yuv_bt2020_full;
+		break;
+	case DRM_COLOR_ENCODING_MAX:
+		matrix_to_copy = &no_operation;
+		WARN_ONCE(true, "The requested encoding is not supported.");
+		break;
+	}
+
+	memcpy(matrix, matrix_to_copy, sizeof(*matrix_to_copy));
+
+	switch (format) {
+	case DRM_FORMAT_YVU420:
+	case DRM_FORMAT_YVU422:
+	case DRM_FORMAT_YVU444:
+	case DRM_FORMAT_NV21:
+	case DRM_FORMAT_NV61:
+	case DRM_FORMAT_NV42:
+		swap_uv_columns_transposed(matrix);
+		break;
+	default:
+		break;
+	}
+}
+EXPORT_SYMBOL(get_conversion_matrix_from_argb_u16);
+
 /**
  * get_pixel_write_function() - Retrieve the correct write_pixel function for a specific format.
  * The returned pointer is NULL for unsupported pixel formats. The caller must ensure that the
diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h
index eeb208cdd6b1..9367672b6b43 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.h
+++ b/drivers/gpu/drm/vkms/vkms_formats.h
@@ -13,6 +13,10 @@ void get_conversion_matrix_to_argb_u16(u32 format, enum drm_color_encoding encod
 				       enum drm_color_range range,
 				       struct conversion_matrix *matrix);
 
+void get_conversion_matrix_from_argb_u16(u32 format, enum drm_color_encoding encoding,
+					 enum drm_color_range range,
+					 struct conversion_matrix *matrix);
+
 #if IS_ENABLED(CONFIG_KUNIT)
 struct pixel_argb_u16 argb_u16_from_yuv161616(const struct conversion_matrix *matrix,
 					      u16 y, u16 channel_1, u16 channel_2);
-- 
2.50.1

[PATCH v1 3/5] drm/vkms: Add WRITEBACK_COLOR_ENCODING and WRITEBACK_COLOR_RANGE properties

[Robert Mader]

Advertise the supported values and add the conversion matrix of the
currently selected values to writeback jobs.

This does not have any effect yet as no YUV/YCbCr formats are advertised.

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 drivers/gpu/drm/vkms/vkms_drv.h       | 33 ++++++++---------
 drivers/gpu/drm/vkms/vkms_writeback.c | 51 +++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 8013c31efe3b..7fa58e17c286 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -81,10 +81,27 @@ struct line_buffer {
  */
 typedef void (*pixel_write_t)(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel);
 
+/**
+ * struct conversion_matrix - Matrix to use for a specific encoding and range
+ *
+ * @matrix: Conversion matrix from yuv to rgb. The matrix is stored in a row-major manner and is
+ * used to compute rgb values from yuv values:
+ *     [[r],[g],[b]] = @matrix * [[y],[u],[v]]
+ *   OR for yvu formats:
+ *     [[r],[g],[b]] = @matrix * [[y],[v],[u]]
+ *  The values of the matrix are signed fixed-point values with 32 bits fractional part.
+ * @y_offset: Offset to apply on the y value.
+ */
+struct conversion_matrix {
+	s64 matrix[3][3];
+	int y_offset;
+};
+
 struct vkms_writeback_job {
 	struct iosys_map data[DRM_FORMAT_MAX_PLANES];
 	struct vkms_frame_info wb_frame_info;
 	pixel_write_t pixel_write;
+	struct conversion_matrix conversion_matrix;
 };
 
 /**
@@ -119,22 +136,6 @@ typedef void (*pixel_read_line_t)(const struct vkms_plane_state *plane, int x_st
 				  int y_start, enum pixel_read_direction direction, int count,
 				  struct pixel_argb_u16 out_pixel[]);
 
-/**
- * struct conversion_matrix - Matrix to use for a specific encoding and range
- *
- * @matrix: Conversion matrix from yuv to rgb. The matrix is stored in a row-major manner and is
- * used to compute rgb values from yuv values:
- *     [[r],[g],[b]] = @matrix * [[y],[u],[v]]
- *   OR for yvu formats:
- *     [[r],[g],[b]] = @matrix * [[y],[v],[u]]
- *  The values of the matrix are signed fixed-point values with 32 bits fractional part.
- * @y_offset: Offset to apply on the y value.
- */
-struct conversion_matrix {
-	s64 matrix[3][3];
-	int y_offset;
-};
-
 /**
  * struct vkms_plane_state - Driver specific plane state
  * @base: base plane state
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
index 45d69a3b85f6..59e7a980533c 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -14,6 +14,9 @@
 #include "vkms_drv.h"
 #include "vkms_formats.h"
 
+#define DEFAULT_COLOR_ENCODING DRM_COLOR_YCBCR_BT601
+#define DEFAULT_COLOR_RANGE DRM_COLOR_YCBCR_LIMITED_RANGE
+
 static const u32 vkms_wb_formats[] = {
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_XRGB8888,
@@ -23,9 +26,17 @@ static const u32 vkms_wb_formats[] = {
 	DRM_FORMAT_RGB565
 };
 
+static void vkms_wb_connector_reset(struct drm_connector *connector)
+{
+	drm_atomic_helper_connector_reset(connector);
+
+	connector->state->writeback_color_encoding = DEFAULT_COLOR_ENCODING;
+	connector->state->writeback_color_range = DEFAULT_COLOR_RANGE;
+}
+
 static const struct drm_connector_funcs vkms_wb_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.reset = drm_atomic_helper_connector_reset,
+	.reset = vkms_wb_connector_reset,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
@@ -60,6 +71,21 @@ static int vkms_wb_atomic_check(struct drm_connector *connector,
 	if (ret < 0)
 		return ret;
 
+	if (conn_state->writeback_color_encoding != DRM_COLOR_YCBCR_BT601 &&
+	    conn_state->writeback_color_encoding != DRM_COLOR_YCBCR_BT709 &&
+	    conn_state->writeback_color_encoding != DRM_COLOR_YCBCR_BT2020) {
+		DRM_DEBUG_KMS("Invalid color encoding %u\n",
+			      conn_state->writeback_color_encoding);
+		return -EINVAL;
+	}
+
+	if (conn_state->writeback_color_range != DRM_COLOR_YCBCR_LIMITED_RANGE &&
+	    conn_state->writeback_color_range != DRM_COLOR_YCBCR_FULL_RANGE) {
+		DRM_DEBUG_KMS("Invalid color range %u\n",
+			      conn_state->writeback_color_range);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -151,6 +177,10 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
 	spin_unlock_irq(&output->composer_lock);
 	drm_writeback_queue_job(wb_conn, connector_state);
 	active_wb->pixel_write = get_pixel_write_function(wb_format);
+	get_conversion_matrix_from_argb_u16(wb_format,
+					    conn_state->writeback_color_encoding,
+					    conn_state->writeback_color_range,
+					    &active_wb->conversion_matrix);
 	drm_rect_init(&wb_frame_info->src, 0, 0, crtc_width, crtc_height);
 	drm_rect_init(&wb_frame_info->dst, 0, 0, crtc_width, crtc_height);
 }
@@ -179,9 +209,26 @@ int vkms_enable_writeback_connector(struct vkms_device *vkmsdev,
 
 	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
 
-	return drmm_writeback_connector_init(&vkmsdev->drm, wb,
+	ret = drmm_writeback_connector_init(&vkmsdev->drm, wb,
 					     &vkms_wb_connector_funcs,
 					     &vkms_output->wb_encoder,
 					     vkms_wb_formats,
 					     ARRAY_SIZE(vkms_wb_formats));
+	if (ret)
+		return ret;
+
+	ret = drm_writeback_create_color_properties(wb,
+						    BIT(DRM_COLOR_YCBCR_BT601) |
+						    BIT(DRM_COLOR_YCBCR_BT709) |
+						    BIT(DRM_COLOR_YCBCR_BT2020),
+						    BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
+						    BIT(DRM_COLOR_YCBCR_FULL_RANGE),
+						    DEFAULT_COLOR_ENCODING,
+						    DEFAULT_COLOR_RANGE);
+	if (ret) {
+		DRM_ERROR("adding color properties failed: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
 }
-- 
2.50.1

[PATCH v1 4/5] drm/vkms: Add support for writing semiplanar YUV/YCbCr formats

[Robert Mader]

Analogous to the existing pixel_read_line_t and vkms_plane_state
implementation.

Note that the chroma siting is implecitely set ITU-T Rec.H.273
Chroma420SampleLocType 2, which should match the existing plane code
and the corresponding Mesa shader paths.

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 drivers/gpu/drm/vkms/tests/vkms_format_test.c |  51 ++++--
 drivers/gpu/drm/vkms/vkms_drv.h               |  36 +++-
 drivers/gpu/drm/vkms/vkms_formats.c           | 172 ++++++++++++++++--
 drivers/gpu/drm/vkms/vkms_formats.h           |   3 +
 4 files changed, 225 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/vkms/tests/vkms_format_test.c b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
index a7788fbc45dc..68b070160dca 100644
--- a/drivers/gpu/drm/vkms/tests/vkms_format_test.c
+++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
@@ -13,19 +13,6 @@
 
 MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
 
-/**
- * struct pixel_yuv_u16 - Internal representation of a pixel color.
- * @y: Luma value, stored in 16 bits, without padding, using
- *     machine endianness
- * @u: Blue difference chroma value, stored in 16 bits, without padding, using
- *     machine endianness
- * @v: Red difference chroma value, stored in 16 bits, without padding, using
- *     machine endianness
- */
-struct pixel_yuv_u16 {
-	u16 y, u, v;
-};
-
 /*
  * struct yuv_u16_to_argb_u16_case - Reference values to test the color
  * conversions in VKMS between YUV to ARGB
@@ -252,6 +239,43 @@ static void vkms_format_test_yuv_u16_to_argb_u16(struct kunit *test)
 	}
 }
 
+/*
+ * vkms_format_test_yuv_u16_to_argb_u16 - Testing the conversion between ARGB
+ * colors to YUV colors in VKMS
+ *
+ * This test will use the functions get_conversion_matrix_from_argb_u16 and
+ * yuv161616_from_argb_u16 to convert ARGB colors (stored in
+ * yuv_u16_to_argb_u16_cases) into YUV colors.
+ *
+ * The conversion between YUV and RGB is not totally reversible, so there may be
+ * some difference between the expected value and the result.
+ */
+static void vkms_format_test_argb_u16_to_yuv_u16(struct kunit *test)
+{
+	const struct yuv_u16_to_argb_u16_case *param = test->param_value;
+	struct pixel_yuv_u16 yuv;
+
+	for (size_t i = 0; i < param->n_colors; i++) {
+		const struct format_pair *color = &param->colors[i];
+		struct conversion_matrix matrix;
+
+		get_conversion_matrix_from_argb_u16
+			(DRM_FORMAT_NV12, param->encoding, param->range, &matrix);
+
+		yuv = yuv161616_from_argb_u16(&matrix, &color->argb);
+
+		KUNIT_EXPECT_LE_MSG(test, abs_diff(color->yuv.y, yuv.y), 0x1ff,
+				    "On the Y channel of the color %s expected 0x%04x, got 0x%04x",
+				    color->name, color->yuv.y, yuv.y);
+		KUNIT_EXPECT_LE_MSG(test, abs_diff(color->yuv.u, yuv.u), 0x1ff,
+				    "On the U channel of the color %s expected 0x%04x, got 0x%04x",
+				    color->name, color->yuv.u, yuv.u);
+		KUNIT_EXPECT_LE_MSG(test, abs_diff(color->yuv.v, yuv.v), 0x1ff,
+				    "On the V channel of the color %s expected 0x%04x, got 0x%04x",
+				    color->name, color->yuv.v, yuv.v);
+	}
+}
+
 static void vkms_format_test_yuv_u16_to_argb_u16_case_desc(struct yuv_u16_to_argb_u16_case *t,
 							   char *desc)
 {
@@ -265,6 +289,7 @@ KUNIT_ARRAY_PARAM(yuv_u16_to_argb_u16, yuv_u16_to_argb_u16_cases,
 
 static struct kunit_case vkms_format_test_cases[] = {
 	KUNIT_CASE_PARAM(vkms_format_test_yuv_u16_to_argb_u16, yuv_u16_to_argb_u16_gen_params),
+	KUNIT_CASE_PARAM(vkms_format_test_argb_u16_to_yuv_u16, yuv_u16_to_argb_u16_gen_params),
 	{}
 };
 
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 7fa58e17c286..29dddc973ef6 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -66,21 +66,24 @@ struct pixel_argb_u16 {
 	u16 a, r, g, b;
 };
 
+/**
+ * struct pixel_yuv_u16 - Internal representation of a pixel color.
+ * @y: Luma value, stored in 16 bits, without padding, using
+ *     machine endianness
+ * @u: Blue difference chroma value, stored in 16 bits, without padding, using
+ *     machine endianness
+ * @v: Red difference chroma value, stored in 16 bits, without padding, using
+ *     machine endianness
+ */
+struct pixel_yuv_u16 {
+	u16 y, u, v;
+};
+
 struct line_buffer {
 	size_t n_pixels;
 	struct pixel_argb_u16 *pixels;
 };
 
-/**
- * typedef pixel_write_t - These functions are used to read a pixel from a
- * &struct pixel_argb_u16, convert it in a specific format and write it in the @out_pixel
- * buffer.
- *
- * @out_pixel: destination address to write the pixel
- * @in_pixel: pixel to write
- */
-typedef void (*pixel_write_t)(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel);
-
 /**
  * struct conversion_matrix - Matrix to use for a specific encoding and range
  *
@@ -97,6 +100,19 @@ struct conversion_matrix {
 	int y_offset;
 };
 
+/**
+ * typedef pixel_write_t - These functions are used to read a pixel from a
+ * &struct pixel_argb_u16, convert it in a specific format and write it in the @out_pixel
+ * buffer.
+ *
+ * @out_pixel: destination address to write the pixel
+ * @in_pixel: pixel to write
+ */
+typedef void (*pixel_write_t)(const struct conversion_matrix *matrix,
+			      u8 *out_pixel, u8 *out_pixel_plane_2,
+			      u8 *out_pixel_plane_3,
+			      const struct pixel_argb_u16 *in_pixel);
+
 struct vkms_writeback_job {
 	struct iosys_map data[DRM_FORMAT_MAX_PLANES];
 	struct vkms_frame_info wb_frame_info;
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index 560b56fbf4fb..048268304c27 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -585,7 +585,9 @@ static void planar_yuv_read_line(const struct vkms_plane_state *plane, int x_sta
  * They are used in vkms_writeback_row() to convert and store a pixel from the src_buffer to
  * the writeback buffer.
  */
-static void argb_u16_to_ARGB8888(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel)
+static void argb_u16_to_ARGB8888(const struct conversion_matrix *matrix,
+				 u8 *out_pixel, u8 *out_pixel_plane_2, u8 *out_pixel_plane_3,
+				 const struct pixel_argb_u16 *in_pixel)
 {
 	/*
 	 * This sequence below is important because the format's byte order is
@@ -603,7 +605,10 @@ static void argb_u16_to_ARGB8888(u8 *out_pixel, const struct pixel_argb_u16 *in_
 	out_pixel[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257);
 }
 
-static void argb_u16_to_XRGB8888(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel)
+static void argb_u16_to_XRGB8888(const struct conversion_matrix *matrix,
+				 u8 *out_pixel, u8 *out_pixel_plane_2,
+				 u8 *out_pixel_plane_3,
+				 const struct pixel_argb_u16 *in_pixel)
 {
 	out_pixel[3] = 0xff;
 	out_pixel[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257);
@@ -611,7 +616,10 @@ static void argb_u16_to_XRGB8888(u8 *out_pixel, const struct pixel_argb_u16 *in_
 	out_pixel[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257);
 }
 
-static void argb_u16_to_ABGR8888(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel)
+static void argb_u16_to_ABGR8888(const struct conversion_matrix *matrix,
+				 u8 *out_pixel, u8 *out_pixel_plane_2,
+				 u8 *out_pixel_plane_3,
+				 const struct pixel_argb_u16 *in_pixel)
 {
 	out_pixel[3] = DIV_ROUND_CLOSEST(in_pixel->a, 257);
 	out_pixel[2] = DIV_ROUND_CLOSEST(in_pixel->b, 257);
@@ -619,7 +627,10 @@ static void argb_u16_to_ABGR8888(u8 *out_pixel, const struct pixel_argb_u16 *in_
 	out_pixel[0] = DIV_ROUND_CLOSEST(in_pixel->r, 257);
 }
 
-static void argb_u16_to_ARGB16161616(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel)
+static void argb_u16_to_ARGB16161616(const struct conversion_matrix *matrix,
+				     u8 *out_pixel, u8 *out_pixel_plane_2,
+				     u8 *out_pixel_plane_3,
+				     const struct pixel_argb_u16 *in_pixel)
 {
 	__le16 *pixel = (__le16 *)out_pixel;
 
@@ -629,7 +640,10 @@ static void argb_u16_to_ARGB16161616(u8 *out_pixel, const struct pixel_argb_u16
 	pixel[0] = cpu_to_le16(in_pixel->b);
 }
 
-static void argb_u16_to_XRGB16161616(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel)
+static void argb_u16_to_XRGB16161616(const struct conversion_matrix *matrix,
+				     u8 *out_pixel, u8 *out_pixel_plane_2,
+				     u8 *out_pixel_plane_3,
+				     const struct pixel_argb_u16 *in_pixel)
 {
 	__le16 *pixel = (__le16 *)out_pixel;
 
@@ -639,7 +653,10 @@ static void argb_u16_to_XRGB16161616(u8 *out_pixel, const struct pixel_argb_u16
 	pixel[0] = cpu_to_le16(in_pixel->b);
 }
 
-static void argb_u16_to_RGB565(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel)
+static void argb_u16_to_RGB565(const struct conversion_matrix *matrix,
+			       u8 *out_pixel, u8 *out_pixel_plane_2,
+			       u8 *out_pixel_plane_3,
+			       const struct pixel_argb_u16 *in_pixel)
 {
 	__le16 *pixel = (__le16 *)out_pixel;
 
@@ -657,6 +674,96 @@ static void argb_u16_to_RGB565(u8 *out_pixel, const struct pixel_argb_u16 *in_pi
 	*pixel = cpu_to_le16(r << 11 | g << 5 | b);
 }
 
+VISIBLE_IF_KUNIT
+struct pixel_yuv_u16 yuv161616_from_argb_u16(const struct conversion_matrix *matrix,
+					     const struct pixel_argb_u16 *in_pixel)
+{
+	struct pixel_yuv_u16 out_pixel;
+	s64 fp_r, fp_g, fp_b;
+	s64 fp_y, fp_channel_1, fp_channel_2;
+
+	fp_r = drm_int2fixp((int)in_pixel->r * 257);
+	fp_g = drm_int2fixp((int)in_pixel->g * 257);
+	fp_b = drm_int2fixp((int)in_pixel->b * 257);
+
+	fp_y = drm_fixp_mul(matrix->matrix[0][0], fp_r) +
+	       drm_fixp_mul(matrix->matrix[0][1], fp_g) +
+	       drm_fixp_mul(matrix->matrix[0][2], fp_b);
+	fp_channel_1 = drm_fixp_mul(matrix->matrix[1][0], fp_r) +
+		       drm_fixp_mul(matrix->matrix[1][1], fp_g) +
+		       drm_fixp_mul(matrix->matrix[1][2], fp_b);
+	fp_channel_2 = drm_fixp_mul(matrix->matrix[2][0], fp_r) +
+		       drm_fixp_mul(matrix->matrix[2][1], fp_g) +
+		       drm_fixp_mul(matrix->matrix[2][2], fp_b);
+
+	fp_y = drm_fixp2int_round(fp_y);
+	fp_channel_1 = drm_fixp2int_round(fp_channel_1);
+	fp_channel_2 = drm_fixp2int_round(fp_channel_2);
+
+	fp_y = DIV_ROUND_CLOSEST(fp_y, 257);
+	fp_channel_1 = DIV_ROUND_CLOSEST(fp_channel_1, 257);
+	fp_channel_2 = DIV_ROUND_CLOSEST(fp_channel_2, 257);
+
+	fp_y += matrix->y_offset * 257;
+	fp_channel_1 += 128 * 257;
+	fp_channel_2 += 128 * 257;
+
+	out_pixel.y = clamp(fp_y, 0, 0xffff);
+	out_pixel.u = clamp(fp_channel_1, 0, 0xffff);
+	out_pixel.v = clamp(fp_channel_2, 0, 0xffff);
+
+	return out_pixel;
+}
+EXPORT_SYMBOL_IF_KUNIT(yuv161616_from_argb_u16);
+
+static void argb_u16_to_YUV888_semiplanar_2plane(const struct conversion_matrix *matrix,
+						 u8 *out_pixel,
+						 u8 *out_pixel_plane_2,
+						 u8 *out_pixel_plane_3,
+						 const struct pixel_argb_u16 *in_pixel)
+{
+	struct pixel_yuv_u16 yuv;
+
+	yuv = yuv161616_from_argb_u16(matrix, in_pixel);
+
+	out_pixel[0] = DIV_ROUND_CLOSEST(yuv.y, 257);
+	out_pixel_plane_2[0] = DIV_ROUND_CLOSEST(yuv.u, 257);
+	out_pixel_plane_2[1] = DIV_ROUND_CLOSEST(yuv.v, 257);
+}
+
+static void argb_u16_to_YUV161616_semiplanar_2plane(const struct conversion_matrix *matrix,
+						    u8 *out_pixel,
+						    u8 *out_pixel_plane_2,
+						    u8 *out_pixel_plane_3,
+						    const struct pixel_argb_u16 *in_pixel)
+{
+	struct pixel_yuv_u16 yuv;
+
+	yuv = yuv161616_from_argb_u16(matrix, in_pixel);
+
+	out_pixel[0] = yuv.y & 0xff;
+	out_pixel[1] = yuv.y >> 8;
+	out_pixel_plane_2[0] = yuv.u & 0xff;
+	out_pixel_plane_2[1] = yuv.u >> 8;
+	out_pixel_plane_2[2] = yuv.v & 0xff;
+	out_pixel_plane_2[3] = yuv.v >> 8;
+}
+
+static void argb_u16_to_YUV888_semiplanar_3plane(const struct conversion_matrix *matrix,
+						 u8 *out_pixel,
+						 u8 *out_pixel_plane_2,
+						 u8 *out_pixel_plane_3,
+						 const struct pixel_argb_u16 *in_pixel)
+{
+	struct pixel_yuv_u16 yuv;
+
+	yuv = yuv161616_from_argb_u16(matrix, in_pixel);
+
+	out_pixel[0] = DIV_ROUND_CLOSEST(yuv.y, 257);
+	out_pixel_plane_2[0] = DIV_ROUND_CLOSEST(yuv.u, 257);
+	out_pixel_plane_3[0] = DIV_ROUND_CLOSEST(yuv.v, 257);
+}
+
 /**
  * vkms_writeback_row() - Generic loop for all supported writeback format. It is executed just
  * after the blending to write a line in the writeback buffer.
@@ -669,16 +776,35 @@ void vkms_writeback_row(struct vkms_writeback_job *wb,
 			const struct line_buffer *src_buffer, int y)
 {
 	struct vkms_frame_info *frame_info = &wb->wb_frame_info;
-	int x_dst = frame_info->dst.x1;
-	u8 *dst_pixels;
-	int rem_x, rem_y;
-
-	packed_pixels_addr(frame_info, x_dst, y, 0, &dst_pixels, &rem_x, &rem_y);
+	const struct drm_format_info *format = frame_info->fb->format;
 	struct pixel_argb_u16 *in_pixels = src_buffer->pixels;
-	int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst), src_buffer->n_pixels);
+	int x_limit;
+
+	x_limit= min_t(size_t, drm_rect_width(&frame_info->dst),
+		       src_buffer->n_pixels);
+
+	for (size_t x = 0; x < x_limit; x++) {
+		u8 *plane_1;
+		u8 *plane_2 = NULL;
+		u8 *plane_3 = NULL;
+
+		packed_pixels_addr_1x1(frame_info, x, y, 0, &plane_1);
+		if (format->num_planes > 1) {
+			packed_pixels_addr_1x1(frame_info,
+					       x / frame_info->fb->format->hsub,
+					       y / frame_info->fb->format->vsub, 1,
+					       &plane_2);
+		}
+		if (format->num_planes > 2) {
+			packed_pixels_addr_1x1(frame_info,
+					       x / frame_info->fb->format->hsub,
+					       y / frame_info->fb->format->vsub, 2,
+					       &plane_3);
+		}
 
-	for (size_t x = 0; x < x_limit; x++, dst_pixels += frame_info->fb->format->cpp[0])
-		wb->pixel_write(dst_pixels, &in_pixels[x]);
+		wb->pixel_write(&wb->conversion_matrix, plane_1, plane_2,
+				plane_3, &in_pixels[x]);
+	}
 }
 
 /**
@@ -1110,6 +1236,24 @@ pixel_write_t get_pixel_write_function(u32 format)
 		return &argb_u16_to_XRGB16161616;
 	case DRM_FORMAT_RGB565:
 		return &argb_u16_to_RGB565;
+	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_NV16:
+	case DRM_FORMAT_NV24:
+	case DRM_FORMAT_NV21:
+	case DRM_FORMAT_NV61:
+	case DRM_FORMAT_NV42:
+		return &argb_u16_to_YUV888_semiplanar_2plane;
+	case DRM_FORMAT_P010:
+	case DRM_FORMAT_P012:
+	case DRM_FORMAT_P016:
+		return &argb_u16_to_YUV161616_semiplanar_2plane;
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YUV422:
+	case DRM_FORMAT_YUV444:
+	case DRM_FORMAT_YVU420:
+	case DRM_FORMAT_YVU422:
+	case DRM_FORMAT_YVU444:
+		return &argb_u16_to_YUV888_semiplanar_3plane;
 	default:
 		/*
 		 * This is a bug in vkms_writeback_atomic_check. All the supported
diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h
index 9367672b6b43..bf317c3c058a 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.h
+++ b/drivers/gpu/drm/vkms/vkms_formats.h
@@ -20,6 +20,9 @@ void get_conversion_matrix_from_argb_u16(u32 format, enum drm_color_encoding enc
 #if IS_ENABLED(CONFIG_KUNIT)
 struct pixel_argb_u16 argb_u16_from_yuv161616(const struct conversion_matrix *matrix,
 					      u16 y, u16 channel_1, u16 channel_2);
+
+struct pixel_yuv_u16 yuv161616_from_argb_u16(const struct conversion_matrix *matrix,
+					     const struct pixel_argb_u16 *in_pixel);
 #endif
 
 #endif /* _VKMS_FORMATS_H_ */
-- 
2.50.1

[PATCH v1 5/5] drm/vkms: Add YUV formats to writeback connector

[Robert Mader]

Now that all plumbing is done expose the new formats.

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 drivers/gpu/drm/vkms/vkms_writeback.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
index 59e7a980533c..f3fbfeff01be 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -23,7 +23,22 @@ static const u32 vkms_wb_formats[] = {
 	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_XRGB16161616,
 	DRM_FORMAT_ARGB16161616,
-	DRM_FORMAT_RGB565
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_NV12,
+	DRM_FORMAT_NV16,
+	DRM_FORMAT_NV24,
+	DRM_FORMAT_NV21,
+	DRM_FORMAT_NV61,
+	DRM_FORMAT_NV42,
+	DRM_FORMAT_YUV420,
+	DRM_FORMAT_YUV422,
+	DRM_FORMAT_YUV444,
+	DRM_FORMAT_YVU420,
+	DRM_FORMAT_YVU422,
+	DRM_FORMAT_YVU444,
+	DRM_FORMAT_P010,
+	DRM_FORMAT_P012,
+	DRM_FORMAT_P016,
 };
 
 static void vkms_wb_connector_reset(struct drm_connector *connector)
-- 
2.50.1

Re: [PATCH v1 2/5] drm/vkms: Add RGB to YUV conversion matrices

[Daniel Stone]

Hi Rob,

On Wed, 13 Aug 2025 at 18:06, Robert Mader <robert.mader@collabora.com> wrote:
> +       switch (format) {
> +       case DRM_FORMAT_YVU420:
> +       case DRM_FORMAT_YVU422:
> +       case DRM_FORMAT_YVU444:
> +       case DRM_FORMAT_NV21:
> +       case DRM_FORMAT_NV61:
> +       case DRM_FORMAT_NV42:

I wonder if this could use a drm_format_info field for chroma order,
instead of open-coding a list here?

Cheers,
Daniel

Re: [PATCH v1 3/5] drm/vkms: Add WRITEBACK_COLOR_ENCODING and WRITEBACK_COLOR_RANGE properties

[Daniel Stone]

Hi Rob,

On Wed, 13 Aug 2025 at 18:06, Robert Mader <robert.mader@collabora.com> wrote:
> @@ -60,6 +71,21 @@ static int vkms_wb_atomic_check(struct drm_connector *connector,
>         if (ret < 0)
>                 return ret;
>
> +       if (conn_state->writeback_color_encoding != DRM_COLOR_YCBCR_BT601 &&
> +           conn_state->writeback_color_encoding != DRM_COLOR_YCBCR_BT709 &&
> +           conn_state->writeback_color_encoding != DRM_COLOR_YCBCR_BT2020) {
> +               DRM_DEBUG_KMS("Invalid color encoding %u\n",
> +                             conn_state->writeback_color_encoding);
> +               return -EINVAL;
> +       }
> +
> +       if (conn_state->writeback_color_range != DRM_COLOR_YCBCR_LIMITED_RANGE &&
> +           conn_state->writeback_color_range != DRM_COLOR_YCBCR_FULL_RANGE) {
> +               DRM_DEBUG_KMS("Invalid color range %u\n",
> +                             conn_state->writeback_color_range);
> +               return -EINVAL;
> +       }

I didn't think you needed this check, as the core property code should
already disallow setting an enum not in the supported list?

As this only takes effect on YUV, I suspect you might be better off
adding a PASSTHROUGH or NOOP or similar value, which would be required
to be used for RGB framebuffers, with it being required to specify the
range and primaries for YUV formats.

That being said, I don't think these are specific to YUV, as RGB can
also have the same 16-235 squash applied to it. So maybe it is better
off being generic?

Cheers,
Daniel

Re: [PATCH v1 2/5] drm/vkms: Add RGB to YUV conversion matrices

[Robert Mader]

Hey, thanks for the feedback.

On 14.08.25 11:57, Daniel Stone wrote:
> Hi Rob,
>
> On Wed, 13 Aug 2025 at 18:06, Robert Mader <robert.mader@collabora.com> wrote:
>> +       switch (format) {
>> +       case DRM_FORMAT_YVU420:
>> +       case DRM_FORMAT_YVU422:
>> +       case DRM_FORMAT_YVU444:
>> +       case DRM_FORMAT_NV21:
>> +       case DRM_FORMAT_NV61:
>> +       case DRM_FORMAT_NV42:
> I wonder if this could use a drm_format_info field for chroma order,
> instead of open-coding a list here?

Generally agree, however note that this is directly copied from 
`get_conversion_matrix_to_argb_u16()`. So this is not a new pattern in 
this file - and there are several other drivers with comparable ones, 
see e.g. `has_uv_swapped()` in rockchip_drm_vop.c. So while a new field 
drm_format_info would make for a nice cleanup, I'd argue that would be 
better suited in a dedicated series.

What IMO would make sense here is introduce a `has_uv_swapped()` helper, 
so we at least don't repeat ourselves. Will do that in v2 - does that 
work for you?

>
> Cheers,
> Daniel

-- 
Robert Mader
Consultant Software Developer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718

Re: [PATCH v1 2/5] drm/vkms: Add RGB to YUV conversion matrices

[Daniel Stone]

Hey there,

On Mon, 18 Aug 2025 at 16:03, Robert Mader <robert.mader@collabora.com> wrote:
> On 14.08.25 11:57, Daniel Stone wrote:
> > On Wed, 13 Aug 2025 at 18:06, Robert Mader <robert.mader@collabora.com> wrote:
> >> +       switch (format) {
> >> +       case DRM_FORMAT_YVU420:
> >> +       case DRM_FORMAT_YVU422:
> >> +       case DRM_FORMAT_YVU444:
> >> +       case DRM_FORMAT_NV21:
> >> +       case DRM_FORMAT_NV61:
> >> +       case DRM_FORMAT_NV42:
> > I wonder if this could use a drm_format_info field for chroma order,
> > instead of open-coding a list here?
>
> Generally agree, however note that this is directly copied from
> `get_conversion_matrix_to_argb_u16()`. So this is not a new pattern in
> this file - and there are several other drivers with comparable ones,
> see e.g. `has_uv_swapped()` in rockchip_drm_vop.c. So while a new field
> drm_format_info would make for a nice cleanup, I'd argue that would be
> better suited in a dedicated series.
>
> What IMO would make sense here is introduce a `has_uv_swapped()` helper,
> so we at least don't repeat ourselves. Will do that in v2 - does that
> work for you?

Yeah, that sounds good to me!

Cheers,
Daniel