[PATCH v3 00/11] drm: Add new pixel formats for Xilinx Zynqmp

[PATCH v3 00/11] drm: Add new pixel formats for Xilinx Zynqmp

[Tomi Valkeinen]

Add new DRM pixel formats and add support for those in the Xilinx zynqmp
display driver.

All other formats except XVUY2101010 are already supported in upstream
gstreamer, but gstreamer's kmssink does not have the support yet, as it
obviously cannot support the formats without kernel having the formats.

Xilinx has support for these formats in their BSP kernel, and Xilinx has
a branch here, adding the support to gstreamer kmssink:

https://github.com/Xilinx/gst-plugins-bad.git xlnx-rebase-v1.18.5

New formats added:

DRM_FORMAT_Y8
- 8-bit Y-only
- fourcc: "GREY"
- gstreamer: GRAY8

DRM_FORMAT_Y10_P32
- 10-bit Y-only, three pixels packed into 32-bits
- fourcc: "YPA4"
- gstreamer: GRAY10_LE32

DRM_FORMAT_XV15
- Like NV12, but with 10-bit components
- fourcc: "XV15"
- gstreamer: NV12_10LE32

DRM_FORMAT_XV20
- Like NV16, but with 10-bit components
- fourcc: "XV20"
- gstreamer: NV16_10LE32

DRM_FORMAT_X403
- 10-bit planar 4:4:4, with three samples packed into 32-bits
- fourcc: "X403"
- gstreamer: Y444_10LE32

XVUY2101010
- 10-bit 4:4:4, one pixel in 32 bits
- fourcc: "XY30"

Some notes:

I know the 8-bit greyscale format has been discussed before, and the
guidance was to use DRM_FORMAT_R8. While I'm not totally against that, I
would argue that adding DRM_FORMAT_Y8 makes sense, as:

1) We can mark it as 'is_yuv' in the drm_format_info, and this can help
   the drivers handle e.g. full/limited range. Probably some hardware
   handles grayscale as a value used for all RGB components, in which case
   R8 makes sense, but at least for Xilinx hardware it is YUV, just
   Y-only, not RGB.

2) We can have the same fourcc as in v4l2. While not strictly necessary,
   it's a constant source of confusion when the fourccs differ.

3) It (possibly) makes more sense for the user to use Y8/GREY format
   instead of R8, as, in my experience, the documentation usually refers
   to gray(scale) format or Y-only format.

As we add new Y-only formats, it makes sense to have similar terms, so
we need to adjust the Y10_P32 format name accordingly.

I have made some adjustments to the formats compared to the Xilinx's
branch. E.g. The DRM_FORMAT_Y10_P32 format in Xilinx's kmssink uses
fourcc "Y10 ", and DRM_FORMAT_Y10. I didn't like those, as the format is
a packed format, three 10-bit pixels in a 32-bit container, and I think
Y10 means a 10-bit pixel in a 16-bit container.

Generally speaking, if someone has good ideas for the format define
names or fourccs, speak up, as it's not easy to invent good names =).
That said, keeping them the same as in the Xilinx trees will, of course,
be slightly easier for the users of Xilinx platforms.

I made WIP additions to modetest to support most of these formats,
partially based on Xilinx's code:

https://github.com/tomba/libdrm.git xilinx

A few thoughts about that:

modetest uses bo_create_dumb(), and as highlighted in recent discussions
in the kernel list [1], dumb buffers are only for RGB formats. They may
work for non-RGB formats, but that's platform specific. None of the
formats I add here are RGB formats. Do we want to go this way with
modetest?

I also feel that the current structure of modetest is not well suited to
more complicated formats. Both the buffer allocation is a bit more
difficult (see "Add virtual_width and pixels_per_container"), and the
drawing is complicated (see, e.g., "Add support for DRM_FORMAT_XV15 &
DRM_FORMAT_XV20").

I have recently added support for these Xilinx formats to both kms++ [2] and
pykms/pixutils [3][4] (WIP), and it's not been easy... But I have to say I
think I like the template based version in kms++. That won't work in
modetest, of course, but a non-templated version might be implementable,
but probably much slower.

In any case, I slighly feel it's not worth merging the modetest patches
I have for these formats: they complicate the code quite a bit, break
the RGB-only rule, and I'm not sure if there really are (m)any users. If
we want to add support to modetest, I think a bigger rewrite of the test
pattern code might be in order.

[1] https://lore.kernel.org/all/20250109150310.219442-26-tzimmermann%40suse.de/
[2] git@github.com:tomba/kmsxx.git xilinx
[3] git@github.com:tomba/pykms.git xilinx
[4] git@github.com:tomba/pixutils.git xilinx

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Changes in v3:
- Drop "drm: xlnx: zynqmp: Fix max dma segment size". It is already
  pushed.
- Add XVUY2101010 format.
- Rename DRM_FORMAT_Y10_LE32 to DRM_FORMAT_Y10_P32.
- Link to v2: https://lore.kernel.org/r/20250115-xilinx-formats-v2-0-160327ca652a@ideasonboard.com

Changes in v2:
- I noticed V4L2 already has fourcc Y10P, referring to MIPI-style packed
  Y10 format. So I changed Y10_LE32 fourcc to YPA4. If logic has any
  relevance here, P means packed, A means 10, 4 means "in 4 bytes".
- Added tags to "Fix max dma segment size" patch
- Updated description for "Add warning for bad bpp"
- Link to v1: https://lore.kernel.org/r/20241204-xilinx-formats-v1-0-0bf2c5147db1@ideasonboard.com

---
Tomi Valkeinen (11):
      drm/fourcc: Add warning for bad bpp
      drm/fourcc: Add DRM_FORMAT_XV15/XV20
      drm/fourcc: Add DRM_FORMAT_Y8
      drm/fourcc: Add DRM_FORMAT_Y10_P32
      drm/fourcc: Add DRM_FORMAT_X403
      drm/fourcc: Add DRM_FORMAT_XVUY2101010
      drm: xlnx: zynqmp: Use drm helpers when calculating buffer sizes
      drm: xlnx: zynqmp: Add support for XV15 & XV20
      drm: xlnx: zynqmp: Add support for Y8 and Y10_LE32
      drm: xlnx: zynqmp: Add support for X403
      drm: xlnx: zynqmp: Add support for XVUY2101010

 drivers/gpu/drm/drm_fourcc.c       | 25 ++++++++++++++++++
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 54 +++++++++++++++++++++++++++++++++++---
 include/uapi/drm/drm_fourcc.h      | 21 +++++++++++++++
 3 files changed, 96 insertions(+), 4 deletions(-)
---
base-commit: f7d07bcd0651b90dda8a6962057eb5fb1807a089
change-id: 20241120-xilinx-formats-f71901621833

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

[PATCH v3 01/11] drm/fourcc: Add warning for bad bpp

[Tomi Valkeinen]

drm_format_info_bpp() cannot be used for formats which do not have an
integer bits-per-pixel in a pixel block.

E.g. DRM_FORMAT_XV15's (not yet in upstream) plane 0 has three 10-bit
pixels (Y components), and two padding bits, in a 4 byte block. That is
10.666... bits per pixel when considering the whole 4 byte block, which
is what drm_format_info_bpp() does. Thus a driver that supports such
formats cannot use drm_format_info_bpp(),

It is a driver bug if this happens, but so handle wrong calls by
printing a warning and returning 0.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 3a94ca211f9c..1e9afbf6ef99 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -457,6 +457,13 @@ unsigned int drm_format_info_bpp(const struct drm_format_info *info, int plane)
 	if (!info || plane < 0 || plane >= info->num_planes)
 		return 0;
 
+	if (info->char_per_block[plane] * 8 %
+	    (drm_format_info_block_width(info, plane) *
+	     drm_format_info_block_height(info, plane))) {
+		pr_warn("unable to return an integer bpp\n");
+		return 0;
+	}
+
 	return info->char_per_block[plane] * 8 /
 	       (drm_format_info_block_width(info, plane) *
 		drm_format_info_block_height(info, plane));

-- 
2.43.0

[PATCH v3 02/11] drm/fourcc: Add DRM_FORMAT_XV15/XV20

[Tomi Valkeinen]

Add two new pixel formats:

DRM_FORMAT_XV15 ("XV15")
DRM_FORMAT_XV20 ("XV20")

The formats are 2 plane 10 bit per component YCbCr, with the XV15 2x2
subsampled whereas XV20 is 2x1 subsampled.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c  | 8 ++++++++
 include/uapi/drm/drm_fourcc.h | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 1e9afbf6ef99..bb0a2294573b 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -346,6 +346,14 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		{ .format = DRM_FORMAT_P030,            .depth = 0,  .num_planes = 2,
 		  .char_per_block = { 4, 8, 0 }, .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 },
 		  .hsub = 2, .vsub = 2, .is_yuv = true},
+		{ .format = DRM_FORMAT_XV15,		.depth = 0,
+		  .num_planes = 2, .char_per_block = { 4, 8, 0 },
+		  .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2,
+		  .vsub = 2, .is_yuv = true },
+		{ .format = DRM_FORMAT_XV20,		.depth = 0,
+		  .num_planes = 2, .char_per_block = { 4, 8, 0 },
+		  .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2,
+		  .vsub = 1, .is_yuv = true },
 	};
 
 	unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index e41a3cec6a9e..ead756a71825 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -304,6 +304,14 @@ extern "C" {
 #define DRM_FORMAT_RGB565_A8	fourcc_code('R', '5', 'A', '8')
 #define DRM_FORMAT_BGR565_A8	fourcc_code('B', '5', 'A', '8')
 
+/*
+ * 2 plane 10 bit per component YCrCb
+ * index 0 = Y plane, [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian
+ * index 1 = Cb:Cr plane, [63:0] x:Cr2:Cb2:Cr1:x:Cb1:Cr0:Cb0 2:10:10:10:2:10:10:10 little endian
+ */
+#define DRM_FORMAT_XV15		fourcc_code('X', 'V', '1', '5') /* 2x2 subsampled Cr:Cb plane 2:10:10:10 */
+#define DRM_FORMAT_XV20		fourcc_code('X', 'V', '2', '0') /* 2x1 subsampled Cr:Cb plane 2:10:10:10 */
+
 /*
  * 2 plane YCbCr
  * index 0 = Y plane, [7:0] Y

-- 
2.43.0

[PATCH v3 03/11] drm/fourcc: Add DRM_FORMAT_Y8

[Tomi Valkeinen]

Add greyscale Y8 format.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c  | 1 +
 include/uapi/drm/drm_fourcc.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index bb0a2294573b..c8c6dd8a8d4a 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -267,6 +267,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		{ .format = DRM_FORMAT_YVU422,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_YUV444,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_YVU444,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1, .is_yuv = true },
+		{ .format = DRM_FORMAT_Y8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_NV12,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true },
 		{ .format = DRM_FORMAT_NV21,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true },
 		{ .format = DRM_FORMAT_NV16,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index ead756a71825..2778cea9c861 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -405,6 +405,9 @@ extern "C" {
 #define DRM_FORMAT_YUV444	fourcc_code('Y', 'U', '2', '4') /* non-subsampled Cb (1) and Cr (2) planes */
 #define DRM_FORMAT_YVU444	fourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */
 
+/* Greyscale formats */
+
+#define DRM_FORMAT_Y8		fourcc_code('G', 'R', 'E', 'Y')  /* 8-bit Y-only */
 
 /*
  * Format Modifiers:

-- 
2.43.0

[PATCH v3 04/11] drm/fourcc: Add DRM_FORMAT_Y10_P32

[Tomi Valkeinen]

Add Y10_P32, a 10 bit greyscale format, with 3 pixels packed into
32-bit container.

The fourcc for the format is 'YPA4', which comes from Y - Y only, P -
packed, A - 10 (as in 0xA), 4 - 4 bytes.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c  | 4 ++++
 include/uapi/drm/drm_fourcc.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index c8c6dd8a8d4a..155482d9a409 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -355,6 +355,10 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		  .num_planes = 2, .char_per_block = { 4, 8, 0 },
 		  .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2,
 		  .vsub = 1, .is_yuv = true },
+		{ .format = DRM_FORMAT_Y10_P32,		.depth = 0,
+		  .num_planes = 1, .char_per_block =  { 4, 0, 0 },
+		  .block_w = { 3, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 1,
+		  .vsub = 1, .is_yuv = true },
 	};
 
 	unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 2778cea9c861..e87349d46325 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -408,6 +408,7 @@ extern "C" {
 /* Greyscale formats */
 
 #define DRM_FORMAT_Y8		fourcc_code('G', 'R', 'E', 'Y')  /* 8-bit Y-only */
+#define DRM_FORMAT_Y10_P32	fourcc_code('Y', 'P', 'A', '4')  /* [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian */
 
 /*
  * Format Modifiers:

-- 
2.43.0

[PATCH v3 05/11] drm/fourcc: Add DRM_FORMAT_X403

[Tomi Valkeinen]

Add X403, a 3 plane non-subsampled YCbCr format.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c  | 4 ++++
 include/uapi/drm/drm_fourcc.h | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 155482d9a409..cc7e1ef4ef21 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -359,6 +359,10 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		  .num_planes = 1, .char_per_block =  { 4, 0, 0 },
 		  .block_w = { 3, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 1,
 		  .vsub = 1, .is_yuv = true },
+		{ .format = DRM_FORMAT_X403,		.depth = 0,
+		  .num_planes = 3, .char_per_block =  { 4, 4, 4 },
+		  .block_w = { 3, 3, 3 }, .block_h = { 1, 1, 1 },
+		  .hsub = 1, .vsub = 1, .is_yuv = true },
 	};
 
 	unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index e87349d46325..f936cac2aebc 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -385,6 +385,14 @@ extern "C" {
  */
 #define DRM_FORMAT_Q401		fourcc_code('Q', '4', '0', '1')
 
+/* 3 plane non-subsampled (444) YCbCr
+ * 10 bpc, 30 bits per sample image data in a single contiguous buffer.
+ * index 0: Y plane,  [31:0] x:Y2:Y1:Y0    [2:10:10:10] little endian
+ * index 1: Cb plane, [31:0] x:Cb2:Cb1:Cb0 [2:10:10:10] little endian
+ * index 2: Cr plane, [31:0] x:Cr2:Cr1:Cr0 [2:10:10:10] little endian
+ */
+#define DRM_FORMAT_X403		fourcc_code('X', '4', '0', '3')
+
 /*
  * 3 plane YCbCr
  * index 0: Y plane, [7:0] Y

-- 
2.43.0

[PATCH v3 06/11] drm/fourcc: Add DRM_FORMAT_XVUY2101010

[Tomi Valkeinen]

Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
container.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c  | 1 +
 include/uapi/drm/drm_fourcc.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index cc7e1ef4ef21..07b03b0fe9ad 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -280,6 +280,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		{ .format = DRM_FORMAT_VYUY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_XYUV8888,	.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_VUY888,          .depth = 0,  .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
+		{ .format = DRM_FORMAT_XVUY2101010,     .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_AYUV,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, .is_yuv = true },
 		{ .format = DRM_FORMAT_Y210,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
 		{ .format = DRM_FORMAT_Y212,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index f936cac2aebc..7e8aa3bd52b9 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -246,6 +246,7 @@ extern "C" {
 #define DRM_FORMAT_XVUY8888	fourcc_code('X', 'V', 'U', 'Y') /* [31:0] X:Cr:Cb:Y 8:8:8:8 little endian */
 #define DRM_FORMAT_VUY888	fourcc_code('V', 'U', '2', '4') /* [23:0] Cr:Cb:Y 8:8:8 little endian */
 #define DRM_FORMAT_VUY101010	fourcc_code('V', 'U', '3', '0') /* Y followed by U then V, 10:10:10. Non-linear modifier only */
+#define DRM_FORMAT_XVUY2101010	fourcc_code('X', 'Y', '3', '0') /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */
 
 /*
  * packed Y2xx indicate for each component, xx valid data occupy msb

-- 
2.43.0

[PATCH v3 07/11] drm: xlnx: zynqmp: Use drm helpers when calculating buffer sizes

[Tomi Valkeinen]

Use drm helpers, drm_format_info_plane_width(),
drm_format_info_plane_height() and drm_format_info_min_pitch() to
calculate sizes for the DMA.

This cleans up the code, but also makes it possible to support more
complex formats (like XV15, XV20).

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 80d1e499a18d..3d4ade571f0f 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1116,16 +1116,18 @@ int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
 		return 0;
 
 	for (i = 0; i < info->num_planes; i++) {
-		unsigned int width = state->crtc_w / (i ? info->hsub : 1);
-		unsigned int height = state->crtc_h / (i ? info->vsub : 1);
 		struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
 		struct dma_async_tx_descriptor *desc;
+		unsigned int width, height;
 		dma_addr_t dma_addr;
 
+		width = drm_format_info_plane_width(info, state->crtc_w, i);
+		height = drm_format_info_plane_height(info, state->crtc_h, i);
+
 		dma_addr = drm_fb_dma_get_gem_addr(state->fb, state, i);
 
 		dma->xt.numf = height;
-		dma->sgl.size = width * info->cpp[i];
+		dma->sgl.size = drm_format_info_min_pitch(info, i, width);
 		dma->sgl.icg = state->fb->pitches[i] - dma->sgl.size;
 		dma->xt.src_start = dma_addr;
 		dma->xt.frame_size = 1;

-- 
2.43.0

[PATCH v3 08/11] drm: xlnx: zynqmp: Add support for XV15 & XV20

[Tomi Valkeinen]

Add support for XV15 & XV20 formats.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 3d4ade571f0f..32031f52e114 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -297,6 +297,16 @@ static const struct zynqmp_disp_format avbuf_vid_fmts[] = {
 		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV16CI_420,
 		.swap		= true,
 		.sf		= scaling_factors_888,
+	}, {
+		.drm_fmt	= DRM_FORMAT_XV15,
+		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV16CI_420_10,
+		.swap		= false,
+		.sf		= scaling_factors_101010,
+	}, {
+		.drm_fmt	= DRM_FORMAT_XV20,
+		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV16CI_10,
+		.swap		= false,
+		.sf		= scaling_factors_101010,
 	},
 };
 

-- 
2.43.0

[PATCH v3 09/11] drm: xlnx: zynqmp: Add support for Y8 and Y10_LE32

[Tomi Valkeinen]

Add support for Y8 and Y10_LE32 formats. We also need to add new csc
matrices for the y-only formats.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 32031f52e114..c820df97aa28 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -307,6 +307,16 @@ static const struct zynqmp_disp_format avbuf_vid_fmts[] = {
 		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV16CI_10,
 		.swap		= false,
 		.sf		= scaling_factors_101010,
+	}, {
+		.drm_fmt	= DRM_FORMAT_Y8,
+		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MONO,
+		.swap		= false,
+		.sf		= scaling_factors_888,
+	}, {
+		.drm_fmt	= DRM_FORMAT_Y10_P32,
+		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YONLY_10,
+		.swap		= false,
+		.sf		= scaling_factors_101010,
 	},
 };
 
@@ -697,6 +707,16 @@ static const u32 csc_sdtv_to_rgb_offsets[] = {
 	0x0, 0x1800, 0x1800
 };
 
+static const u16 csc_sdtv_to_rgb_yonly_matrix[] = {
+	0x0, 0x0, 0x1000,
+	0x0, 0x0, 0x1000,
+	0x0, 0x0, 0x1000,
+};
+
+static const u32 csc_sdtv_to_rgb_yonly_offsets[] = {
+	0x1800, 0x1800, 0x0
+};
+
 /**
  * zynqmp_disp_blend_set_output_format - Set the output format of the blender
  * @disp: Display controller
@@ -846,7 +866,11 @@ static void zynqmp_disp_blend_layer_enable(struct zynqmp_disp *disp,
 				ZYNQMP_DISP_V_BLEND_LAYER_CONTROL(layer->id),
 				val);
 
-	if (layer->drm_fmt->is_yuv) {
+	if (layer->drm_fmt->format == DRM_FORMAT_Y8 ||
+	    layer->drm_fmt->format == DRM_FORMAT_Y10_P32) {
+		coeffs = csc_sdtv_to_rgb_yonly_matrix;
+		offsets = csc_sdtv_to_rgb_yonly_offsets;
+	} else if (layer->drm_fmt->is_yuv) {
 		coeffs = csc_sdtv_to_rgb_matrix;
 		offsets = csc_sdtv_to_rgb_offsets;
 	} else {

-- 
2.43.0

[PATCH v3 10/11] drm: xlnx: zynqmp: Add support for X403

[Tomi Valkeinen]

Add support for X403 format.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index c820df97aa28..7495e42677a6 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -317,6 +317,11 @@ static const struct zynqmp_disp_format avbuf_vid_fmts[] = {
 		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YONLY_10,
 		.swap		= false,
 		.sf		= scaling_factors_101010,
+	}, {
+		.drm_fmt	= DRM_FORMAT_X403,
+		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV24_10,
+		.swap		= false,
+		.sf		= scaling_factors_101010,
 	},
 };
 

-- 
2.43.0

[PATCH v3 11/11] drm: xlnx: zynqmp: Add support for XVUY2101010

[Tomi Valkeinen]

Add support for XVUY2101010 format.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 7495e42677a6..859ee050a668 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -322,6 +322,11 @@ static const struct zynqmp_disp_format avbuf_vid_fmts[] = {
 		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV24_10,
 		.swap		= false,
 		.sf		= scaling_factors_101010,
+	}, {
+		.drm_fmt	= DRM_FORMAT_XVUY2101010,
+		.buf_fmt	= ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YUV444_10,
+		.swap		= false,
+		.sf		= scaling_factors_101010,
 	},
 };
 

-- 
2.43.0

Re: [PATCH v3 01/11] drm/fourcc: Add warning for bad bpp

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Feb 12, 2025 at 04:56:05PM +0200, Tomi Valkeinen wrote:
> drm_format_info_bpp() cannot be used for formats which do not have an
> integer bits-per-pixel in a pixel block.
> 
> E.g. DRM_FORMAT_XV15's (not yet in upstream) plane 0 has three 10-bit
> pixels (Y components), and two padding bits, in a 4 byte block. That is
> 10.666... bits per pixel when considering the whole 4 byte block, which
> is what drm_format_info_bpp() does. Thus a driver that supports such
> formats cannot use drm_format_info_bpp(),
> 
> It is a driver bug if this happens, but so handle wrong calls by
> printing a warning and returning 0.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 3a94ca211f9c..1e9afbf6ef99 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -457,6 +457,13 @@ unsigned int drm_format_info_bpp(const struct drm_format_info *info, int plane)
>  	if (!info || plane < 0 || plane >= info->num_planes)
>  		return 0;
>  
> +	if (info->char_per_block[plane] * 8 %
> +	    (drm_format_info_block_width(info, plane) *
> +	     drm_format_info_block_height(info, plane))) {

Can you cache 

	    drm_format_info_block_width(info, plane) *
	    drm_format_info_block_height(info, plane)

in a local variable to avoid computing it twice ?

> +		pr_warn("unable to return an integer bpp\n");

It would be nice to print the warning in the context of the drm device,
but we don't have the required structure here.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +		return 0;
> +	}
> +
>  	return info->char_per_block[plane] * 8 /
>  	       (drm_format_info_block_width(info, plane) *
>  		drm_format_info_block_height(info, plane));

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 07/11] drm: xlnx: zynqmp: Use drm helpers when calculating buffer sizes

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Feb 12, 2025 at 04:56:11PM +0200, Tomi Valkeinen wrote:
> Use drm helpers, drm_format_info_plane_width(),
> drm_format_info_plane_height() and drm_format_info_min_pitch() to
> calculate sizes for the DMA.
> 
> This cleans up the code, but also makes it possible to support more
> complex formats (like XV15, XV20).
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/xlnx/zynqmp_disp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 80d1e499a18d..3d4ade571f0f 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -1116,16 +1116,18 @@ int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
>  		return 0;
>  
>  	for (i = 0; i < info->num_planes; i++) {
> -		unsigned int width = state->crtc_w / (i ? info->hsub : 1);
> -		unsigned int height = state->crtc_h / (i ? info->vsub : 1);
>  		struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
>  		struct dma_async_tx_descriptor *desc;
> +		unsigned int width, height;

I'd got with one variable per line.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  		dma_addr_t dma_addr;
>  
> +		width = drm_format_info_plane_width(info, state->crtc_w, i);
> +		height = drm_format_info_plane_height(info, state->crtc_h, i);
> +
>  		dma_addr = drm_fb_dma_get_gem_addr(state->fb, state, i);
>  
>  		dma->xt.numf = height;
> -		dma->sgl.size = width * info->cpp[i];
> +		dma->sgl.size = drm_format_info_min_pitch(info, i, width);
>  		dma->sgl.icg = state->fb->pitches[i] - dma->sgl.size;
>  		dma->xt.src_start = dma_addr;
>  		dma->xt.frame_size = 1;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 03/11] drm/fourcc: Add DRM_FORMAT_Y8

[Dmitry Baryshkov]

On Wed, Feb 12, 2025 at 04:56:07PM +0200, Tomi Valkeinen wrote:
> Add greyscale Y8 format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 1 +
>  include/uapi/drm/drm_fourcc.h | 3 +++
>  2 files changed, 4 insertions(+)

My 2c is that it's useful to have YUV (well, Y-only) grayscale formats.


Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

Re: [PATCH v3 02/11] drm/fourcc: Add DRM_FORMAT_XV15/XV20

[Dmitry Baryshkov]

On Wed, Feb 12, 2025 at 04:56:06PM +0200, Tomi Valkeinen wrote:
> Add two new pixel formats:
> 
> DRM_FORMAT_XV15 ("XV15")
> DRM_FORMAT_XV20 ("XV20")
> 
> The formats are 2 plane 10 bit per component YCbCr, with the XV15 2x2
> subsampled whereas XV20 is 2x1 subsampled.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 8 ++++++++
>  include/uapi/drm/drm_fourcc.h | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 1e9afbf6ef99..bb0a2294573b 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -346,6 +346,14 @@ const struct drm_format_info *__drm_format_info(u32 format)
>  		{ .format = DRM_FORMAT_P030,            .depth = 0,  .num_planes = 2,
>  		  .char_per_block = { 4, 8, 0 }, .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 },
>  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_XV15,		.depth = 0,
> +		  .num_planes = 2, .char_per_block = { 4, 8, 0 },
> +		  .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2,
> +		  .vsub = 2, .is_yuv = true },
> +		{ .format = DRM_FORMAT_XV20,		.depth = 0,
> +		  .num_planes = 2, .char_per_block = { 4, 8, 0 },
> +		  .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2,
> +		  .vsub = 1, .is_yuv = true },

It might be beneficial to use the same formatting as previous entries,
it simplifies reviewing. If the patchset is resent, it would be nice to
get that fixed..

However the patch looks correct, so


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>



>  	};
>  
>  	unsigned int i;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index e41a3cec6a9e..ead756a71825 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -304,6 +304,14 @@ extern "C" {
>  #define DRM_FORMAT_RGB565_A8	fourcc_code('R', '5', 'A', '8')
>  #define DRM_FORMAT_BGR565_A8	fourcc_code('B', '5', 'A', '8')
>  
> +/*
> + * 2 plane 10 bit per component YCrCb
> + * index 0 = Y plane, [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian
> + * index 1 = Cb:Cr plane, [63:0] x:Cr2:Cb2:Cr1:x:Cb1:Cr0:Cb0 2:10:10:10:2:10:10:10 little endian
> + */
> +#define DRM_FORMAT_XV15		fourcc_code('X', 'V', '1', '5') /* 2x2 subsampled Cr:Cb plane 2:10:10:10 */
> +#define DRM_FORMAT_XV20		fourcc_code('X', 'V', '2', '0') /* 2x1 subsampled Cr:Cb plane 2:10:10:10 */
> +
>  /*
>   * 2 plane YCbCr
>   * index 0 = Y plane, [7:0] Y
> 
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 06/11] drm/fourcc: Add DRM_FORMAT_XVUY2101010

[Dmitry Baryshkov]

On Wed, Feb 12, 2025 at 04:56:10PM +0200, Tomi Valkeinen wrote:
> Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
> container.

Is there a more common name for this format? Otherwise googling for it
reveals only your series.

> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 1 +
>  include/uapi/drm/drm_fourcc.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index cc7e1ef4ef21..07b03b0fe9ad 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -280,6 +280,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
>  		{ .format = DRM_FORMAT_VYUY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
>  		{ .format = DRM_FORMAT_XYUV8888,	.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
>  		{ .format = DRM_FORMAT_VUY888,          .depth = 0,  .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> +		{ .format = DRM_FORMAT_XVUY2101010,     .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
>  		{ .format = DRM_FORMAT_AYUV,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, .is_yuv = true },
>  		{ .format = DRM_FORMAT_Y210,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
>  		{ .format = DRM_FORMAT_Y212,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index f936cac2aebc..7e8aa3bd52b9 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -246,6 +246,7 @@ extern "C" {
>  #define DRM_FORMAT_XVUY8888	fourcc_code('X', 'V', 'U', 'Y') /* [31:0] X:Cr:Cb:Y 8:8:8:8 little endian */
>  #define DRM_FORMAT_VUY888	fourcc_code('V', 'U', '2', '4') /* [23:0] Cr:Cb:Y 8:8:8 little endian */
>  #define DRM_FORMAT_VUY101010	fourcc_code('V', 'U', '3', '0') /* Y followed by U then V, 10:10:10. Non-linear modifier only */
> +#define DRM_FORMAT_XVUY2101010	fourcc_code('X', 'Y', '3', '0') /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */
>  
>  /*
>   * packed Y2xx indicate for each component, xx valid data occupy msb
> 
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 05/11] drm/fourcc: Add DRM_FORMAT_X403

[Dmitry Baryshkov]

On Wed, Feb 12, 2025 at 04:56:09PM +0200, Tomi Valkeinen wrote:
> Add X403, a 3 plane non-subsampled YCbCr format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 4 ++++
>  include/uapi/drm/drm_fourcc.h | 8 ++++++++
>  2 files changed, 12 insertions(+)
> 

Same comment: is there any other name for the format?

-- 
With best wishes
Dmitry

Re: [PATCH v3 06/11] drm/fourcc: Add DRM_FORMAT_XVUY2101010

[Tomi Valkeinen]

Hi,

On 17/02/2025 22:15, Dmitry Baryshkov wrote:
> On Wed, Feb 12, 2025 at 04:56:10PM +0200, Tomi Valkeinen wrote:
>> Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
>> container.
> 
> Is there a more common name for this format? Otherwise googling for it
> reveals only your series.

In the cover letter I mention the gstreamer names where available (this 
particular format is not in gstreamer). AMD has these in their zynqmp 
documentation 
(https://docs.amd.com/r/en-US/ug1085-zynq-ultrascale-trm/Video-Packer-Format).

XVUY2101010 is YUV444_10BPC in AMD docs.

X403 is Y444_10LE32 in gstreamer, and YV24_10BPC in AMD docs.

I'm not sure you'll have much more luck googling with those names, 
though =).

  Tomi

Re: [PATCH v3 06/11] drm/fourcc: Add DRM_FORMAT_XVUY2101010

[Dmitry Baryshkov]

On Mon, Feb 17, 2025 at 10:27:56PM +0200, Tomi Valkeinen wrote:
> Hi,
> 
> On 17/02/2025 22:15, Dmitry Baryshkov wrote:
> > On Wed, Feb 12, 2025 at 04:56:10PM +0200, Tomi Valkeinen wrote:
> > > Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
> > > container.
> > 
> > Is there a more common name for this format? Otherwise googling for it
> > reveals only your series.
> 
> In the cover letter I mention the gstreamer names where available (this
> particular format is not in gstreamer). AMD has these in their zynqmp
> documentation (https://docs.amd.com/r/en-US/ug1085-zynq-ultrascale-trm/Video-Packer-Format).
> 
> XVUY2101010 is YUV444_10BPC in AMD docs.
> 
> X403 is Y444_10LE32 in gstreamer, and YV24_10BPC in AMD docs.
> 
> I'm not sure you'll have much more luck googling with those names, though
> =).

I'm asking, because include/uapi/drm/drm_fourcc.h has a pretty explicit
waiver: GL, Vulkan or other open standards. Otherwise normal
requirements apply and it's required to have an open-source usespace
implementation, etc.

-- 
With best wishes
Dmitry

Re: [PATCH v3 06/11] drm/fourcc: Add DRM_FORMAT_XVUY2101010

[Tomi Valkeinen]

Hi,

On 18/02/2025 05:26, Dmitry Baryshkov wrote:
> On Mon, Feb 17, 2025 at 10:27:56PM +0200, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 17/02/2025 22:15, Dmitry Baryshkov wrote:
>>> On Wed, Feb 12, 2025 at 04:56:10PM +0200, Tomi Valkeinen wrote:
>>>> Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
>>>> container.
>>>
>>> Is there a more common name for this format? Otherwise googling for it
>>> reveals only your series.
>>
>> In the cover letter I mention the gstreamer names where available (this
>> particular format is not in gstreamer). AMD has these in their zynqmp
>> documentation (https://docs.amd.com/r/en-US/ug1085-zynq-ultrascale-trm/Video-Packer-Format).
>>
>> XVUY2101010 is YUV444_10BPC in AMD docs.
>>
>> X403 is Y444_10LE32 in gstreamer, and YV24_10BPC in AMD docs.
>>
>> I'm not sure you'll have much more luck googling with those names, though
>> =).
> 
> I'm asking, because include/uapi/drm/drm_fourcc.h has a pretty explicit
> waiver: GL, Vulkan or other open standards. Otherwise normal
> requirements apply and it's required to have an open-source usespace
> implementation, etc.

I can drop DRM_FORMAT_XVUY2101010 until we get it to gstreamer. I just 
had it ready, so I thought it's better to include it than leave out.

Is the current gstreamer support enough for the other formats to fulfill 
the userspace requirement?

  Tomi

Re: [PATCH v3 06/11] drm/fourcc: Add DRM_FORMAT_XVUY2101010

[Laurent Pinchart]

On Wed, Feb 19, 2025 at 04:47:26PM +0200, Tomi Valkeinen wrote:
> Hi,
> 
> On 18/02/2025 05:26, Dmitry Baryshkov wrote:
> > On Mon, Feb 17, 2025 at 10:27:56PM +0200, Tomi Valkeinen wrote:
> >> Hi,
> >>
> >> On 17/02/2025 22:15, Dmitry Baryshkov wrote:
> >>> On Wed, Feb 12, 2025 at 04:56:10PM +0200, Tomi Valkeinen wrote:
> >>>> Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
> >>>> container.
> >>>
> >>> Is there a more common name for this format? Otherwise googling for it
> >>> reveals only your series.
> >>
> >> In the cover letter I mention the gstreamer names where available (this
> >> particular format is not in gstreamer). AMD has these in their zynqmp
> >> documentation (https://docs.amd.com/r/en-US/ug1085-zynq-ultrascale-trm/Video-Packer-Format).
> >>
> >> XVUY2101010 is YUV444_10BPC in AMD docs.
> >>
> >> X403 is Y444_10LE32 in gstreamer, and YV24_10BPC in AMD docs.
> >>
> >> I'm not sure you'll have much more luck googling with those names, though
> >> =).
> > 
> > I'm asking, because include/uapi/drm/drm_fourcc.h has a pretty explicit
> > waiver: GL, Vulkan or other open standards. Otherwise normal
> > requirements apply and it's required to have an open-source usespace
> > implementation, etc.
> 
> I can drop DRM_FORMAT_XVUY2101010 until we get it to gstreamer. I just 
> had it ready, so I thought it's better to include it than leave out.
> 
> Is the current gstreamer support enough for the other formats to fulfill 
> the userspace requirement?

We've received a green light in the past to add formats to drm_fourcc.h
that would be used by cameras only. There's no open formal standard there, but
we have libcamera as a de-facto standard. I would assume GStreamer to be
enough for a scanout format.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 06/11] drm/fourcc: Add DRM_FORMAT_XVUY2101010

[Dmitry Baryshkov]

On Wed, Feb 19, 2025 at 05:08:40PM +0200, Laurent Pinchart wrote:
> On Wed, Feb 19, 2025 at 04:47:26PM +0200, Tomi Valkeinen wrote:
> > Hi,
> > 
> > On 18/02/2025 05:26, Dmitry Baryshkov wrote:
> > > On Mon, Feb 17, 2025 at 10:27:56PM +0200, Tomi Valkeinen wrote:
> > >> Hi,
> > >>
> > >> On 17/02/2025 22:15, Dmitry Baryshkov wrote:
> > >>> On Wed, Feb 12, 2025 at 04:56:10PM +0200, Tomi Valkeinen wrote:
> > >>>> Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
> > >>>> container.
> > >>>
> > >>> Is there a more common name for this format? Otherwise googling for it
> > >>> reveals only your series.
> > >>
> > >> In the cover letter I mention the gstreamer names where available (this
> > >> particular format is not in gstreamer). AMD has these in their zynqmp
> > >> documentation (https://docs.amd.com/r/en-US/ug1085-zynq-ultrascale-trm/Video-Packer-Format).
> > >>
> > >> XVUY2101010 is YUV444_10BPC in AMD docs.
> > >>
> > >> X403 is Y444_10LE32 in gstreamer, and YV24_10BPC in AMD docs.
> > >>
> > >> I'm not sure you'll have much more luck googling with those names, though
> > >> =).
> > > 
> > > I'm asking, because include/uapi/drm/drm_fourcc.h has a pretty explicit
> > > waiver: GL, Vulkan or other open standards. Otherwise normal
> > > requirements apply and it's required to have an open-source usespace
> > > implementation, etc.
> > 
> > I can drop DRM_FORMAT_XVUY2101010 until we get it to gstreamer. I just 
> > had it ready, so I thought it's better to include it than leave out.
> > 
> > Is the current gstreamer support enough for the other formats to fulfill 
> > the userspace requirement?
> 
> We've received a green light in the past to add formats to drm_fourcc.h
> that would be used by cameras only. There's no open formal standard there, but
> we have libcamera as a de-facto standard. I would assume GStreamer to be
> enough for a scanout format.

Thanks for the clarification!

-- 
With best wishes
Dmitry

Re: [PATCH v3 06/11] drm/fourcc: Add DRM_FORMAT_XVUY2101010

[Laurent Pinchart]

On Wed, Feb 19, 2025 at 07:28:44PM +0200, Dmitry Baryshkov wrote:
> On Wed, Feb 19, 2025 at 05:08:40PM +0200, Laurent Pinchart wrote:
> > On Wed, Feb 19, 2025 at 04:47:26PM +0200, Tomi Valkeinen wrote:
> > > Hi,
> > > 
> > > On 18/02/2025 05:26, Dmitry Baryshkov wrote:
> > > > On Mon, Feb 17, 2025 at 10:27:56PM +0200, Tomi Valkeinen wrote:
> > > >> Hi,
> > > >>
> > > >> On 17/02/2025 22:15, Dmitry Baryshkov wrote:
> > > >>> On Wed, Feb 12, 2025 at 04:56:10PM +0200, Tomi Valkeinen wrote:
> > > >>>> Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
> > > >>>> container.
> > > >>>
> > > >>> Is there a more common name for this format? Otherwise googling for it
> > > >>> reveals only your series.
> > > >>
> > > >> In the cover letter I mention the gstreamer names where available (this
> > > >> particular format is not in gstreamer). AMD has these in their zynqmp
> > > >> documentation (https://docs.amd.com/r/en-US/ug1085-zynq-ultrascale-trm/Video-Packer-Format).
> > > >>
> > > >> XVUY2101010 is YUV444_10BPC in AMD docs.
> > > >>
> > > >> X403 is Y444_10LE32 in gstreamer, and YV24_10BPC in AMD docs.
> > > >>
> > > >> I'm not sure you'll have much more luck googling with those names, though
> > > >> =).
> > > > 
> > > > I'm asking, because include/uapi/drm/drm_fourcc.h has a pretty explicit
> > > > waiver: GL, Vulkan or other open standards. Otherwise normal
> > > > requirements apply and it's required to have an open-source usespace
> > > > implementation, etc.
> > > 
> > > I can drop DRM_FORMAT_XVUY2101010 until we get it to gstreamer. I just 
> > > had it ready, so I thought it's better to include it than leave out.
> > > 
> > > Is the current gstreamer support enough for the other formats to fulfill 
> > > the userspace requirement?
> > 
> > We've received a green light in the past to add formats to drm_fourcc.h
> > that would be used by cameras only. There's no open formal standard there, but
> > we have libcamera as a de-facto standard. I would assume GStreamer to be
> > enough for a scanout format.
> 
> Thanks for the clarification!

To make it extra clear, this format is not currently (as in the mainline
kernel, with Tomi's work) used for cameras, but only for display
scanout. My comment related to cameras was meant to indicate I expect
drm_fourcc.h to not strictly require all formats to match open formal
standards.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 02/11] drm/fourcc: Add DRM_FORMAT_XV15/XV20

[Tomi Valkeinen]

Hi,

On 17/02/2025 22:00, Dmitry Baryshkov wrote:
> On Wed, Feb 12, 2025 at 04:56:06PM +0200, Tomi Valkeinen wrote:
>> Add two new pixel formats:
>>
>> DRM_FORMAT_XV15 ("XV15")
>> DRM_FORMAT_XV20 ("XV20")
>>
>> The formats are 2 plane 10 bit per component YCbCr, with the XV15 2x2
>> subsampled whereas XV20 is 2x1 subsampled.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/drm_fourcc.c  | 8 ++++++++
>>   include/uapi/drm/drm_fourcc.h | 8 ++++++++
>>   2 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
>> index 1e9afbf6ef99..bb0a2294573b 100644
>> --- a/drivers/gpu/drm/drm_fourcc.c
>> +++ b/drivers/gpu/drm/drm_fourcc.c
>> @@ -346,6 +346,14 @@ const struct drm_format_info *__drm_format_info(u32 format)
>>   		{ .format = DRM_FORMAT_P030,            .depth = 0,  .num_planes = 2,
>>   		  .char_per_block = { 4, 8, 0 }, .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 },
>>   		  .hsub = 2, .vsub = 2, .is_yuv = true},
>> +		{ .format = DRM_FORMAT_XV15,		.depth = 0,
>> +		  .num_planes = 2, .char_per_block = { 4, 8, 0 },
>> +		  .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2,
>> +		  .vsub = 2, .is_yuv = true },
>> +		{ .format = DRM_FORMAT_XV20,		.depth = 0,
>> +		  .num_planes = 2, .char_per_block = { 4, 8, 0 },
>> +		  .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2,
>> +		  .vsub = 1, .is_yuv = true },
> 
> It might be beneficial to use the same formatting as previous entries,
> it simplifies reviewing. If the patchset is resent, it would be nice to
> get that fixed..

But which formatting... The previous entry (singular) is indeed a bit 
different. But many formats before DRM_FORMAT_P030 are formatted as 
these new entries (but then it again changes in earlier entries). It's a 
bit messy list...

But maybe the style in DRM_FORMAT_P030 is better than the style used in 
the new entries, so I could just go with that style.

  Tomi