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

[PATCH v4 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 v4:
- Reformat the drm_format_info entries a bit
- Calculate block size only once in drm_format_info_bpp()
- Declare local variables in separate lines
- Add review tags
- Fix commit message referring to Y10_LE32 (should be Y10_P32)
- Link to v3: https://lore.kernel.org/r/20250212-xilinx-formats-v3-0-90d0fe106995@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_P32
      drm: xlnx: zynqmp: Add support for X403
      drm: xlnx: zynqmp: Add support for XVUY2101010

 drivers/gpu/drm/drm_fourcc.c       | 28 ++++++++++++++++---
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 55 +++++++++++++++++++++++++++++++++++---
 include/uapi/drm/drm_fourcc.h      | 21 +++++++++++++++
 3 files changed, 97 insertions(+), 7 deletions(-)
---
base-commit: 2f9d51740cc30e0d2c8a23a55b1e20cf2513c250
change-id: 20241120-xilinx-formats-f71901621833

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

[PATCH v4 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.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

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

-- 
2.43.0

[PATCH v4 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.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c  | 6 ++++++
 include/uapi/drm/drm_fourcc.h | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 2f5781f5dcda..e101d1b99aeb 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -346,6 +346,12 @@ 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 81202a50dc9e..1247b814bd66 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

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

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Mar 26, 2025 at 03:22:45PM +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.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 6 ++++++
>  include/uapi/drm/drm_fourcc.h | 8 ++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 2f5781f5dcda..e101d1b99aeb 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -346,6 +346,12 @@ 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 appears we can never have too much (or enough) documentation, as
reading the format info documentation leaves me with unanswered
questions.

Looking at drm_format_info_min_pitch():

uint64_t drm_format_info_min_pitch(const struct drm_format_info *info,
				   int plane, unsigned int buffer_width)
{
	if (!info || plane < 0 || plane >= info->num_planes)
		return 0;

	return DIV_ROUND_UP_ULL((u64)buffer_width * info->char_per_block[plane],
			    drm_format_info_block_width(info, plane) *
			    drm_format_info_block_height(info, plane));
}

For the first plane, the function will return `buffer_width * 4 / 3`
(rouding up), which I think is right. For the second plane, it will
return `buffer_width * 8 / 3`, which I believe is wrong as the format is
subsampled by a factor 2 horizontally. It seems that either
char_per_block and block_w need to take horizontal subsampling into
account (and therefore be 8 and 6 for the second plane), or
drm_format_info_min_pitch() should consider .hsub. Or there's something
else I'm missing :-)

>  	};
>  
>  	unsigned int i;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 81202a50dc9e..1247b814bd66 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

I believe this is right, but I have a hard time validating it, as I
think the corresponding figures in UG1085 are incorrect (they show a
8bpp format as far as I can tell). Do I assume correctly that you've
tested the formats ?

> + */
> +#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

-- 
Regards,

Laurent Pinchart

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

[Tomi Valkeinen]

Hi,

On 28/03/2025 00:34, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Wed, Mar 26, 2025 at 03:22:45PM +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.
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/drm_fourcc.c  | 6 ++++++
>>   include/uapi/drm/drm_fourcc.h | 8 ++++++++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
>> index 2f5781f5dcda..e101d1b99aeb 100644
>> --- a/drivers/gpu/drm/drm_fourcc.c
>> +++ b/drivers/gpu/drm/drm_fourcc.c
>> @@ -346,6 +346,12 @@ 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 appears we can never have too much (or enough) documentation, as
> reading the format info documentation leaves me with unanswered
> questions.
> 
> Looking at drm_format_info_min_pitch():
> 
> uint64_t drm_format_info_min_pitch(const struct drm_format_info *info,
> 				   int plane, unsigned int buffer_width)
> {
> 	if (!info || plane < 0 || plane >= info->num_planes)
> 		return 0;
> 
> 	return DIV_ROUND_UP_ULL((u64)buffer_width * info->char_per_block[plane],
> 			    drm_format_info_block_width(info, plane) *
> 			    drm_format_info_block_height(info, plane));
> }
> 
> For the first plane, the function will return `buffer_width * 4 / 3`
> (rouding up), which I think is right. For the second plane, it will
> return `buffer_width * 8 / 3`, which I believe is wrong as the format is
> subsampled by a factor 2 horizontally. It seems that either
> char_per_block and block_w need to take horizontal subsampling into
> account (and therefore be 8 and 6 for the second plane), or
> drm_format_info_min_pitch() should consider .hsub. Or there's something
> else I'm missing :-)

The buffer_width is already divided by the hsub, in 
drm_format_info_plane_width().

>>   	};
>>   
>>   	unsigned int i;
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index 81202a50dc9e..1247b814bd66 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
> 
> I believe this is right, but I have a hard time validating it, as I
> think the corresponding figures in UG1085 are incorrect (they show a
> 8bpp format as far as I can tell). Do I assume correctly that you've
> tested the formats ?

Yes. kms++'s master branch has support for all the formats in this series.

  Tomi

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

[Laurent Pinchart]

On Fri, Mar 28, 2025 at 12:05:40PM +0200, Tomi Valkeinen wrote:
> On 28/03/2025 00:34, Laurent Pinchart wrote:
> > On Wed, Mar 26, 2025 at 03:22:45PM +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.
> >>
> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> ---
> >>   drivers/gpu/drm/drm_fourcc.c  | 6 ++++++
> >>   include/uapi/drm/drm_fourcc.h | 8 ++++++++
> >>   2 files changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> >> index 2f5781f5dcda..e101d1b99aeb 100644
> >> --- a/drivers/gpu/drm/drm_fourcc.c
> >> +++ b/drivers/gpu/drm/drm_fourcc.c
> >> @@ -346,6 +346,12 @@ 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 appears we can never have too much (or enough) documentation, as
> > reading the format info documentation leaves me with unanswered
> > questions.
> > 
> > Looking at drm_format_info_min_pitch():
> > 
> > uint64_t drm_format_info_min_pitch(const struct drm_format_info *info,
> > 				   int plane, unsigned int buffer_width)
> > {
> > 	if (!info || plane < 0 || plane >= info->num_planes)
> > 		return 0;
> > 
> > 	return DIV_ROUND_UP_ULL((u64)buffer_width * info->char_per_block[plane],
> > 			    drm_format_info_block_width(info, plane) *
> > 			    drm_format_info_block_height(info, plane));
> > }
> > 
> > For the first plane, the function will return `buffer_width * 4 / 3`
> > (rouding up), which I think is right. For the second plane, it will
> > return `buffer_width * 8 / 3`, which I believe is wrong as the format is
> > subsampled by a factor 2 horizontally. It seems that either
> > char_per_block and block_w need to take horizontal subsampling into
> > account (and therefore be 8 and 6 for the second plane), or
> > drm_format_info_min_pitch() should consider .hsub. Or there's something
> > else I'm missing :-)
> 
> The buffer_width is already divided by the hsub, in 
> drm_format_info_plane_width().

The function documentation doesn't clearly indicate if buffer_width is
the width of the buffer (as in the width of the first plane), or the
width of the requested plane. The variable name implies (for me) that
it's the width of the buffer, not the plane.

I've checked users of the function, and it seems to be a mess. The
following users pass the plane width to the function:

drivers/gpu/drm/drm_framebuffer.c
drivers/gpu/drm/drm_gem_framebuffer_helper.c
drivers/gpu/drm/tests/drm_format_test.c

However, the following users seem to pass the full buffer width:

drivers/gpu/drm/rockchip/rockchip_drm_vop.c
drivers/gpu/drm/tests/drm_format_helper_test.c

> >>   	};
> >>   
> >>   	unsigned int i;
> >> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >> index 81202a50dc9e..1247b814bd66 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
> > 
> > I believe this is right, but I have a hard time validating it, as I
> > think the corresponding figures in UG1085 are incorrect (they show a
> > 8bpp format as far as I can tell). Do I assume correctly that you've
> > tested the formats ?
> 
> Yes. kms++'s master branch has support for all the formats in this series.
> 
>   Tomi

-- 
Regards,

Laurent Pinchart

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

[Tomi Valkeinen]

Add greyscale Y8 format.

Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
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 e101d1b99aeb..355aaf7b5e9e 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 1247b814bd66..751b8087b35f 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

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

[Geert Uytterhoeven]

Hi Tomi,

On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
> Add greyscale Y8 format.
>
> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Thanks for your patch!

> --- 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 */

This format differs from e.g. DRM_FORMAT_R8, which encodes
the number of bits in the FOURCC format. What do you envision
for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Tomi Valkeinen]

Hi,

On 26/03/2025 15:52, Geert Uytterhoeven wrote:
> Hi Tomi,
> 
> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
>> Add greyscale Y8 format.
>>
>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 
> Thanks for your patch!
> 
>> --- 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 */
> 
> This format differs from e.g. DRM_FORMAT_R8, which encodes
> the number of bits in the FOURCC format. What do you envision
> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?

I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's 
not required, but different fourccs for the same formats do confuse.

So, generally speaking, I'd pick an existing fourcc from v4l2 side if 
possible, and if not, invent a new one.

  Tomi

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

[Pekka Paalanen]

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

On Wed, 26 Mar 2025 15:55:18 +0200
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> Hi,
> 
> On 26/03/2025 15:52, Geert Uytterhoeven wrote:
> > Hi Tomi,
> > 
> > On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> > <tomi.valkeinen@ideasonboard.com> wrote:  
> >> Add greyscale Y8 format.
> >>
> >> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>  
> > 
> > Thanks for your patch!
> >   
> >> --- 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 */  
> > 
> > This format differs from e.g. DRM_FORMAT_R8, which encodes
> > the number of bits in the FOURCC format. What do you envision
> > for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?  
> 
> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's 
> not required, but different fourccs for the same formats do confuse.
> 
> So, generally speaking, I'd pick an existing fourcc from v4l2 side if 
> possible, and if not, invent a new one.

Hi Tomi,

what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?

Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
defined by MatrixCoefficients (H.273 terminology)?

That would be my intuitive assumption following how YCbCr is handled.
Is it obvious enough, or should there be a comment to that effect?


Thanks,
pq

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

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

[Tomi Valkeinen]

Hi,

On 27/03/2025 11:20, Pekka Paalanen wrote:
> On Wed, 26 Mar 2025 15:55:18 +0200
> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:
> 
>> Hi,
>>
>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:
>>> Hi Tomi,
>>>
>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
>>> <tomi.valkeinen@ideasonboard.com> wrote:
>>>> Add greyscale Y8 format.
>>>>
>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>>
>>> Thanks for your patch!
>>>    
>>>> --- 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 */
>>>
>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
>>> the number of bits in the FOURCC format. What do you envision
>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?
>>
>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
>> not required, but different fourccs for the same formats do confuse.
>>
>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
>> possible, and if not, invent a new one.
> 
> Hi Tomi,
> 
> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> 
> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> defined by MatrixCoefficients (H.273 terminology)?
> 
> That would be my intuitive assumption following how YCbCr is handled.
> Is it obvious enough, or should there be a comment to that effect?

You raise an interesting point. Is it defined how a display driver, that 
supports R8 as a format, shows R8 on screen? I came into this in the 
context of grayscale formats, so I thought R8 would be handled as (R, R, 
R) in RGB. But you say (R, 0, 0), which... also makes sense.

I think that's a new argument in favor of Y8: Y8 means Y-only, so the 
meaning is more explicit.

How I see that the display controller would deal with Y8 (depending on 
the HW):

- Take the Y value as a greyscale value, if the HW supports greyscale 
format directly.
- Use the Y as YCbCr (Y, Cb-neutral, Cr-neutral), and use that if the HW 
supports YCbCr directly.
- Use the Y as YCbCr as above, and convert to RGB in the usual way.

And as it's an YUV format, the limited/full range applies, which I 
believe is not usually applied to RGB formats.

Does this make sense?

  Tomi

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

[Pekka Paalanen]

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

On Thu, 27 Mar 2025 16:21:16 +0200
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> Hi,
> 
> On 27/03/2025 11:20, Pekka Paalanen wrote:
> > On Wed, 26 Mar 2025 15:55:18 +0200
> > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:
> >   
> >> Hi,
> >>
> >> On 26/03/2025 15:52, Geert Uytterhoeven wrote:  
> >>> Hi Tomi,
> >>>
> >>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> >>> <tomi.valkeinen@ideasonboard.com> wrote:  
> >>>> Add greyscale Y8 format.
> >>>>
> >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>  
> >>>
> >>> Thanks for your patch!
> >>>      
> >>>> --- 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 */  
> >>>
> >>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> >>> the number of bits in the FOURCC format. What do you envision
> >>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?  
> >>
> >> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> >> not required, but different fourccs for the same formats do confuse.
> >>
> >> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> >> possible, and if not, invent a new one.  
> > 
> > Hi Tomi,
> > 
> > what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > 
> > Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > defined by MatrixCoefficients (H.273 terminology)?
> > 
> > That would be my intuitive assumption following how YCbCr is handled.
> > Is it obvious enough, or should there be a comment to that effect?  
> 
> You raise an interesting point. Is it defined how a display driver, that 
> supports R8 as a format, shows R8 on screen? I came into this in the 
> context of grayscale formats, so I thought R8 would be handled as (R, R, 
> R) in RGB. But you say (R, 0, 0), which... also makes sense.

That is a good question too. I based my assumption on OpenGL behavior
of R8.

Single channel displays do exist I believe, but being single-channel,
expansion on the other channels is likely meaningless. Hm, but for the
KMS color pipeline, it would be meaningful, like with a CTM.
Interesting.

I don't know. Maybe Geert does?

> I think that's a new argument in favor of Y8: Y8 means Y-only, so the 
> meaning is more explicit.
> 
> How I see that the display controller would deal with Y8 (depending on 
> the HW):
> 
> - Take the Y value as a greyscale value, if the HW supports greyscale 
> format directly.
> - Use the Y as YCbCr (Y, Cb-neutral, Cr-neutral), and use that if the HW 
> supports YCbCr directly.
> - Use the Y as YCbCr as above, and convert to RGB in the usual way.
> 
> And as it's an YUV format, the limited/full range applies, which I 
> believe is not usually applied to RGB formats.
> 
> Does this make sense?

It makes perfect sense to me.

The KMS color pipeline is defined in terms of full-range RGB, so if any
of those KMS properties is set that might make a difference, the driver
is forced to go through RGB, regardless of the hardware signal format.


Thanks,
pq

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

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

[Geert Uytterhoeven]

Hi Pekka,

On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen
<pekka.paalanen@haloniitty.fi> wrote:
> On Thu, 27 Mar 2025 16:21:16 +0200
> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:
> > On 27/03/2025 11:20, Pekka Paalanen wrote:
> > > On Wed, 26 Mar 2025 15:55:18 +0200
> > > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:
> > >> On 26/03/2025 15:52, Geert Uytterhoeven wrote:
> > >>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> > >>> <tomi.valkeinen@ideasonboard.com> wrote:
> > >>>> Add greyscale Y8 format.
> > >>>>
> > >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > >>>
> > >>> Thanks for your patch!
> > >>>
> > >>>> --- 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 */
> > >>>
> > >>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > >>> the number of bits in the FOURCC format. What do you envision
> > >>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?
> > >>
> > >> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > >> not required, but different fourccs for the same formats do confuse.
> > >>
> > >> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > >> possible, and if not, invent a new one.
> > >
> > > what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > >
> > > Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > defined by MatrixCoefficients (H.273 terminology)?
> > >
> > > That would be my intuitive assumption following how YCbCr is handled.
> > > Is it obvious enough, or should there be a comment to that effect?
> >
> > You raise an interesting point. Is it defined how a display driver, that
> > supports R8 as a format, shows R8 on screen? I came into this in the
> > context of grayscale formats, so I thought R8 would be handled as (R, R,
> > R) in RGB. But you say (R, 0, 0), which... also makes sense.
>
> That is a good question too. I based my assumption on OpenGL behavior
> of R8.
>
> Single channel displays do exist I believe, but being single-channel,
> expansion on the other channels is likely meaningless. Hm, but for the
> KMS color pipeline, it would be meaningful, like with a CTM.
> Interesting.
>
> I don't know. Maybe Geert does?

I did some digging, and was a bit surprised that it was you who told
me to use R8 instead of Y8?
https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Pekka Paalanen]

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

On Thu, 27 Mar 2025 17:35:39 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Pekka,
> 
> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen
> <pekka.paalanen@haloniitty.fi> wrote:
> > On Thu, 27 Mar 2025 16:21:16 +0200
> > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:  
> > > On 27/03/2025 11:20, Pekka Paalanen wrote:  
> > > > On Wed, 26 Mar 2025 15:55:18 +0200
> > > > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:  
> > > >> On 26/03/2025 15:52, Geert Uytterhoeven wrote:  
> > > >>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> > > >>> <tomi.valkeinen@ideasonboard.com> wrote:  
> > > >>>> Add greyscale Y8 format.
> > > >>>>
> > > >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>  
> > > >>>
> > > >>> Thanks for your patch!
> > > >>>  
> > > >>>> --- 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 */  
> > > >>>
> > > >>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > >>> the number of bits in the FOURCC format. What do you envision
> > > >>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?  
> > > >>
> > > >> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > >> not required, but different fourccs for the same formats do confuse.
> > > >>
> > > >> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > >> possible, and if not, invent a new one.  
> > > >
> > > > what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > >
> > > > Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > defined by MatrixCoefficients (H.273 terminology)?
> > > >
> > > > That would be my intuitive assumption following how YCbCr is handled.
> > > > Is it obvious enough, or should there be a comment to that effect?  
> > >
> > > You raise an interesting point. Is it defined how a display driver, that
> > > supports R8 as a format, shows R8 on screen? I came into this in the
> > > context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > R) in RGB. But you say (R, 0, 0), which... also makes sense.  
> >
> > That is a good question too. I based my assumption on OpenGL behavior
> > of R8.
> >
> > Single channel displays do exist I believe, but being single-channel,
> > expansion on the other channels is likely meaningless. Hm, but for the
> > KMS color pipeline, it would be meaningful, like with a CTM.
> > Interesting.
> >
> > I don't know. Maybe Geert does?  
> 
> I did some digging, and was a bit surprised that it was you who told
> me to use R8 instead of Y8?
> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell

Hi Geert,

indeed I did. I never thought of the question of expansion to R,G,B
before. Maybe that expansion is what spells R8 and Y8 apart?

I do think that expansion needs to be specified, so that the KMS color
pipeline computations are defined. There is a big difference between
multiplying these with an arbitrary 3x3 matrix (e.g. CTM):

- (R, 0, 0)
- (R, R, R)
- (c1 * Y, c2 * Y, c3 * Y)

I forgot to consider that in the discussion of single-channel displays,
because the displays obviously do not consider any other channel than
the one.

Using DRM_FORMAT_Y8 FB with a single-channel display might even be
surprising, because the proposed Y8 definition would result in c1 * Y,
and not Y. The default c1 comes from the BT.601 matrix IIRC?

Therefore I think the difference between R8 and Y8 has been found. Now
we just need to determine whether R8 means (R, 0, 0) or (R, R, R) to
nail down the color operations as well. There are questions like what
is the outcome at the video signal level when we have one KMS plane
with an R8 FB and another KMS plane with an RGBA8888 FB on the same
CRTC? What about Y8 or NV12 in the mix? What if the video signal is
single-channel, RGB, or YCbCr?


Thanks,
pq

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

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

[Laurent Pinchart]

On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:
> On Thu, 27 Mar 2025 17:35:39 +0100
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> > Hi Pekka,
> > 
> > On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen
> > <pekka.paalanen@haloniitty.fi> wrote:
> > > On Thu, 27 Mar 2025 16:21:16 +0200
> > > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:  
> > > > On 27/03/2025 11:20, Pekka Paalanen wrote:  
> > > > > On Wed, 26 Mar 2025 15:55:18 +0200
> > > > > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:  
> > > > >> On 26/03/2025 15:52, Geert Uytterhoeven wrote:  
> > > > >>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> > > > >>> <tomi.valkeinen@ideasonboard.com> wrote:  
> > > > >>>> Add greyscale Y8 format.
> > > > >>>>
> > > > >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>  
> > > > >>>
> > > > >>> Thanks for your patch!
> > > > >>>  
> > > > >>>> --- 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 */  
> > > > >>>
> > > > >>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > >>> the number of bits in the FOURCC format. What do you envision
> > > > >>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?  
> > > > >>
> > > > >> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > >> not required, but different fourccs for the same formats do confuse.
> > > > >>
> > > > >> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > >> possible, and if not, invent a new one.  
> > > > >
> > > > > what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > >
> > > > > Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > > 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > > defined by MatrixCoefficients (H.273 terminology)?
> > > > >
> > > > > That would be my intuitive assumption following how YCbCr is handled.
> > > > > Is it obvious enough, or should there be a comment to that effect?  
> > > >
> > > > You raise an interesting point. Is it defined how a display driver, that
> > > > supports R8 as a format, shows R8 on screen? I came into this in the
> > > > context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > R) in RGB. But you say (R, 0, 0), which... also makes sense.  
> > >
> > > That is a good question too. I based my assumption on OpenGL behavior
> > > of R8.
> > >
> > > Single channel displays do exist I believe, but being single-channel,
> > > expansion on the other channels is likely meaningless. Hm, but for the
> > > KMS color pipeline, it would be meaningful, like with a CTM.
> > > Interesting.
> > >
> > > I don't know. Maybe Geert does?  
> > 
> > I did some digging, and was a bit surprised that it was you who told
> > me to use R8 instead of Y8?
> > https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell
> 
> Hi Geert,
> 
> indeed I did. I never thought of the question of expansion to R,G,B
> before. Maybe that expansion is what spells R8 and Y8 apart?
> 
> I do think that expansion needs to be specified, so that the KMS color
> pipeline computations are defined. There is a big difference between
> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> 
> - (R, 0, 0)
> - (R, R, R)
> - (c1 * Y, c2 * Y, c3 * Y)

I'd be very surprised by an YUV to RGB conversion matrix where the first
column would contain different values. What we need to take into account
though is quantization (full vs. limited range).

> I forgot to consider that in the discussion of single-channel displays,
> because the displays obviously do not consider any other channel than
> the one.
> 
> Using DRM_FORMAT_Y8 FB with a single-channel display might even be
> surprising, because the proposed Y8 definition would result in c1 * Y,
> and not Y. The default c1 comes from the BT.601 matrix IIRC?
> 
> Therefore I think the difference between R8 and Y8 has been found. Now
> we just need to determine whether R8 means (R, 0, 0) or (R, R, R) to
> nail down the color operations as well. There are questions like what
> is the outcome at the video signal level when we have one KMS plane
> with an R8 FB and another KMS plane with an RGBA8888 FB on the same
> CRTC? What about Y8 or NV12 in the mix? What if the video signal is
> single-channel, RGB, or YCbCr?

-- 
Regards,

Laurent Pinchart

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

[Pekka Paalanen]

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

On Mon, 31 Mar 2025 11:21:35 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:
> > On Thu, 27 Mar 2025 17:35:39 +0100
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >   
> > > Hi Pekka,
> > > 
> > > On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen
> > > <pekka.paalanen@haloniitty.fi> wrote:  
> > > > On Thu, 27 Mar 2025 16:21:16 +0200
> > > > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:    
> > > > > On 27/03/2025 11:20, Pekka Paalanen wrote:    
> > > > > > On Wed, 26 Mar 2025 15:55:18 +0200
> > > > > > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:    
> > > > > >> On 26/03/2025 15:52, Geert Uytterhoeven wrote:    
> > > > > >>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> > > > > >>> <tomi.valkeinen@ideasonboard.com> wrote:    
> > > > > >>>> Add greyscale Y8 format.
> > > > > >>>>
> > > > > >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>    
> > > > > >>>
> > > > > >>> Thanks for your patch!
> > > > > >>>    
> > > > > >>>> --- 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 */    
> > > > > >>>
> > > > > >>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > > >>> the number of bits in the FOURCC format. What do you envision
> > > > > >>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?    
> > > > > >>
> > > > > >> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > > >> not required, but different fourccs for the same formats do confuse.
> > > > > >>
> > > > > >> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > > >> possible, and if not, invent a new one.    
> > > > > >
> > > > > > what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > > >
> > > > > > Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > > > 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > > > defined by MatrixCoefficients (H.273 terminology)?
> > > > > >
> > > > > > That would be my intuitive assumption following how YCbCr is handled.
> > > > > > Is it obvious enough, or should there be a comment to that effect?    
> > > > >
> > > > > You raise an interesting point. Is it defined how a display driver, that
> > > > > supports R8 as a format, shows R8 on screen? I came into this in the
> > > > > context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > > R) in RGB. But you say (R, 0, 0), which... also makes sense.    
> > > >
> > > > That is a good question too. I based my assumption on OpenGL behavior
> > > > of R8.
> > > >
> > > > Single channel displays do exist I believe, but being single-channel,
> > > > expansion on the other channels is likely meaningless. Hm, but for the
> > > > KMS color pipeline, it would be meaningful, like with a CTM.
> > > > Interesting.
> > > >
> > > > I don't know. Maybe Geert does?    
> > > 
> > > I did some digging, and was a bit surprised that it was you who told
> > > me to use R8 instead of Y8?
> > > https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell  
> > 
> > Hi Geert,
> > 
> > indeed I did. I never thought of the question of expansion to R,G,B
> > before. Maybe that expansion is what spells R8 and Y8 apart?
> > 
> > I do think that expansion needs to be specified, so that the KMS color
> > pipeline computations are defined. There is a big difference between
> > multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > 
> > - (R, 0, 0)
> > - (R, R, R)
> > - (c1 * Y, c2 * Y, c3 * Y)  
> 
> I'd be very surprised by an YUV to RGB conversion matrix where the first
> column would contain different values. What we need to take into account
> though is quantization (full vs. limited range).

A good point, are the Y coefficients always 1.0 after quantization
range has been accounted for?

That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
(R, 0, 0), so we have both variants. Or do we need Y-formats at all?

Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
in the KMS color processing?


Thanks,
pq

> > I forgot to consider that in the discussion of single-channel displays,
> > because the displays obviously do not consider any other channel than
> > the one.
> > 
> > Using DRM_FORMAT_Y8 FB with a single-channel display might even be
> > surprising, because the proposed Y8 definition would result in c1 * Y,
> > and not Y. The default c1 comes from the BT.601 matrix IIRC?
> > 
> > Therefore I think the difference between R8 and Y8 has been found. Now
> > we just need to determine whether R8 means (R, 0, 0) or (R, R, R) to
> > nail down the color operations as well. There are questions like what
> > is the outcome at the video signal level when we have one KMS plane
> > with an R8 FB and another KMS plane with an RGBA8888 FB on the same
> > CRTC? What about Y8 or NV12 in the mix? What if the video signal is
> > single-channel, RGB, or YCbCr?  
> 


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

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

[Laurent Pinchart]

On Mon, Mar 31, 2025 at 01:53:37PM +0300, Pekka Paalanen wrote:
> On Mon, 31 Mar 2025 11:21:35 +0300 Laurent Pinchart wrote:
> > On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:
> > > On Thu, 27 Mar 2025 17:35:39 +0100 Geert Uytterhoeven wrote:
> > > > On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen wrote:
> > > > > On Thu, 27 Mar 2025 16:21:16 +0200 Tomi Valkeinen wrote:
> > > > > > On 27/03/2025 11:20, Pekka Paalanen wrote:    
> > > > > > > On Wed, 26 Mar 2025 15:55:18 +0200 Tomi Valkeinen wrote:
> > > > > > >> On 26/03/2025 15:52, Geert Uytterhoeven wrote:
> > > > > > >>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen wrote:
> > > > > > >>>> Add greyscale Y8 format.
> > > > > > >>>>
> > > > > > >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>    
> > > > > > >>>
> > > > > > >>> Thanks for your patch!
> > > > > > >>>    
> > > > > > >>>> --- 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 */    
> > > > > > >>>
> > > > > > >>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > > > >>> the number of bits in the FOURCC format. What do you envision
> > > > > > >>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?    
> > > > > > >>
> > > > > > >> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > > > >> not required, but different fourccs for the same formats do confuse.
> > > > > > >>
> > > > > > >> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > > > >> possible, and if not, invent a new one.    
> > > > > > >
> > > > > > > what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > > > >
> > > > > > > Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > > > > 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > > > > defined by MatrixCoefficients (H.273 terminology)?
> > > > > > >
> > > > > > > That would be my intuitive assumption following how YCbCr is handled.
> > > > > > > Is it obvious enough, or should there be a comment to that effect?    
> > > > > >
> > > > > > You raise an interesting point. Is it defined how a display driver, that
> > > > > > supports R8 as a format, shows R8 on screen? I came into this in the
> > > > > > context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > > > R) in RGB. But you say (R, 0, 0), which... also makes sense.    
> > > > >
> > > > > That is a good question too. I based my assumption on OpenGL behavior
> > > > > of R8.
> > > > >
> > > > > Single channel displays do exist I believe, but being single-channel,
> > > > > expansion on the other channels is likely meaningless. Hm, but for the
> > > > > KMS color pipeline, it would be meaningful, like with a CTM.
> > > > > Interesting.
> > > > >
> > > > > I don't know. Maybe Geert does?    
> > > > 
> > > > I did some digging, and was a bit surprised that it was you who told
> > > > me to use R8 instead of Y8?
> > > > https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell  
> > > 
> > > Hi Geert,
> > > 
> > > indeed I did. I never thought of the question of expansion to R,G,B
> > > before. Maybe that expansion is what spells R8 and Y8 apart?
> > > 
> > > I do think that expansion needs to be specified, so that the KMS color
> > > pipeline computations are defined. There is a big difference between
> > > multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > > 
> > > - (R, 0, 0)
> > > - (R, R, R)
> > > - (c1 * Y, c2 * Y, c3 * Y)  
> > 
> > I'd be very surprised by an YUV to RGB conversion matrix where the first
> > column would contain different values. What we need to take into account
> > though is quantization (full vs. limited range).
> 
> A good point, are the Y coefficients always 1.0 after quantization
> range has been accounted for?

As far as I understand, they should be. RGB is full range, so the Y
range should map to [0.0, 1.0] in RGB space. I'm also not aware of any
colorspace where a grey colour would have different R, G and B values.

There's a related but separate question: if Y is a luma (in Y'CbCr
terms), it will not be linear, compared to the Y luminance (YCbCr). We
have a DEGAMMA_LUT to linearize data, but that's in the CRTC output
path, not in the plane path, and I don't see any API element to specify
the transfer function of data input to a CRTC.

> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> (R, 0, 0), so we have both variants. Or do we need Y-formats at all?
> 
> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> in the KMS color processing?
> 
> > > I forgot to consider that in the discussion of single-channel displays,
> > > because the displays obviously do not consider any other channel than
> > > the one.
> > > 
> > > Using DRM_FORMAT_Y8 FB with a single-channel display might even be
> > > surprising, because the proposed Y8 definition would result in c1 * Y,
> > > and not Y. The default c1 comes from the BT.601 matrix IIRC?
> > > 
> > > Therefore I think the difference between R8 and Y8 has been found. Now
> > > we just need to determine whether R8 means (R, 0, 0) or (R, R, R) to
> > > nail down the color operations as well. There are questions like what
> > > is the outcome at the video signal level when we have one KMS plane
> > > with an R8 FB and another KMS plane with an RGBA8888 FB on the same
> > > CRTC? What about Y8 or NV12 in the mix? What if the video signal is
> > > single-channel, RGB, or YCbCr?  

-- 
Regards,

Laurent Pinchart

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

[Pekka Paalanen]

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

On Mon, 31 Mar 2025 17:54:56 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> On Mon, Mar 31, 2025 at 01:53:37PM +0300, Pekka Paalanen wrote:
> > On Mon, 31 Mar 2025 11:21:35 +0300 Laurent Pinchart wrote:  
> > > On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:  
> > > > On Thu, 27 Mar 2025 17:35:39 +0100 Geert Uytterhoeven wrote:  
> > > > > On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen wrote:  
> > > > > > On Thu, 27 Mar 2025 16:21:16 +0200 Tomi Valkeinen wrote:  
> > > > > > > On 27/03/2025 11:20, Pekka Paalanen wrote:      
> > > > > > > > On Wed, 26 Mar 2025 15:55:18 +0200 Tomi Valkeinen wrote:  
> > > > > > > >> On 26/03/2025 15:52, Geert Uytterhoeven wrote:  
> > > > > > > >>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen wrote:  
> > > > > > > >>>> Add greyscale Y8 format.
> > > > > > > >>>>
> > > > > > > >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > > >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>      
> > > > > > > >>>
> > > > > > > >>> Thanks for your patch!
> > > > > > > >>>      
> > > > > > > >>>> --- 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 */      
> > > > > > > >>>
> > > > > > > >>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > > > > >>> the number of bits in the FOURCC format. What do you envision
> > > > > > > >>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?      
> > > > > > > >>
> > > > > > > >> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > > > > >> not required, but different fourccs for the same formats do confuse.
> > > > > > > >>
> > > > > > > >> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > > > > >> possible, and if not, invent a new one.      
> > > > > > > >
> > > > > > > > what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > > > > >
> > > > > > > > Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > > > > > 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > > > > > defined by MatrixCoefficients (H.273 terminology)?
> > > > > > > >
> > > > > > > > That would be my intuitive assumption following how YCbCr is handled.
> > > > > > > > Is it obvious enough, or should there be a comment to that effect?      
> > > > > > >
> > > > > > > You raise an interesting point. Is it defined how a display driver, that
> > > > > > > supports R8 as a format, shows R8 on screen? I came into this in the
> > > > > > > context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > > > > R) in RGB. But you say (R, 0, 0), which... also makes sense.      
> > > > > >
> > > > > > That is a good question too. I based my assumption on OpenGL behavior
> > > > > > of R8.
> > > > > >
> > > > > > Single channel displays do exist I believe, but being single-channel,
> > > > > > expansion on the other channels is likely meaningless. Hm, but for the
> > > > > > KMS color pipeline, it would be meaningful, like with a CTM.
> > > > > > Interesting.
> > > > > >
> > > > > > I don't know. Maybe Geert does?      
> > > > > 
> > > > > I did some digging, and was a bit surprised that it was you who told
> > > > > me to use R8 instead of Y8?
> > > > > https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell    
> > > > 
> > > > Hi Geert,
> > > > 
> > > > indeed I did. I never thought of the question of expansion to R,G,B
> > > > before. Maybe that expansion is what spells R8 and Y8 apart?
> > > > 
> > > > I do think that expansion needs to be specified, so that the KMS color
> > > > pipeline computations are defined. There is a big difference between
> > > > multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > > > 
> > > > - (R, 0, 0)
> > > > - (R, R, R)
> > > > - (c1 * Y, c2 * Y, c3 * Y)    
> > > 
> > > I'd be very surprised by an YUV to RGB conversion matrix where the first
> > > column would contain different values. What we need to take into account
> > > though is quantization (full vs. limited range).  
> > 
> > A good point, are the Y coefficients always 1.0 after quantization
> > range has been accounted for?  
> 
> As far as I understand, they should be. RGB is full range, so the Y
> range should map to [0.0, 1.0] in RGB space. I'm also not aware of any
> colorspace where a grey colour would have different R, G and B values.
> 
> There's a related but separate question: if Y is a luma (in Y'CbCr
> terms), it will not be linear, compared to the Y luminance (YCbCr). We

I did not think anyone would use optical YCbCr, since that tends to
not occur in color transformation chains. Hence, to me Y in YCbCr is
Y', the electrical quantity, luma, unless otherwise explicitly stated.
Likewise, sounds like you take Y'CbCr to mean Y'C'bC'r.

(I like to use the term optical instead of linear, because of "linear
with respect to what?".)

> have a DEGAMMA_LUT to linearize data, but that's in the CRTC output
> path, not in the plane path, and I don't see any API element to specify
> the transfer function of data input to a CRTC.

That's what
https://lore.kernel.org/dri-devel/20241220043410.416867-5-alex.hung@amd.com/
addresses.


Thanks,
pq

> > That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> > (R, 0, 0), so we have both variants. Or do we need Y-formats at all?
> > 
> > Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> > in the KMS color processing?
> >   
> > > > I forgot to consider that in the discussion of single-channel displays,
> > > > because the displays obviously do not consider any other channel than
> > > > the one.
> > > > 
> > > > Using DRM_FORMAT_Y8 FB with a single-channel display might even be
> > > > surprising, because the proposed Y8 definition would result in c1 * Y,
> > > > and not Y. The default c1 comes from the BT.601 matrix IIRC?
> > > > 
> > > > Therefore I think the difference between R8 and Y8 has been found. Now
> > > > we just need to determine whether R8 means (R, 0, 0) or (R, R, R) to
> > > > nail down the color operations as well. There are questions like what
> > > > is the outcome at the video signal level when we have one KMS plane
> > > > with an R8 FB and another KMS plane with an RGBA8888 FB on the same
> > > > CRTC? What about Y8 or NV12 in the mix? What if the video signal is
> > > > single-channel, RGB, or YCbCr?    
> 


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

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

[Pekka Paalanen]

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

On Mon, 31 Mar 2025 13:53:37 +0300
Pekka Paalanen <pekka.paalanen@haloniitty.fi> wrote:

> On Mon, 31 Mar 2025 11:21:35 +0300
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> 
> > On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:  
> > > On Thu, 27 Mar 2025 17:35:39 +0100
> > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > >     
> > > > Hi Pekka,
> > > > 
> > > > On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen
> > > > <pekka.paalanen@haloniitty.fi> wrote:    
> > > > > On Thu, 27 Mar 2025 16:21:16 +0200
> > > > > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:      
> > > > > > On 27/03/2025 11:20, Pekka Paalanen wrote:      
> > > > > > > On Wed, 26 Mar 2025 15:55:18 +0200
> > > > > > > Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:      
> > > > > > >> On 26/03/2025 15:52, Geert Uytterhoeven wrote:      
> > > > > > >>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> > > > > > >>> <tomi.valkeinen@ideasonboard.com> wrote:      
> > > > > > >>>> Add greyscale Y8 format.
> > > > > > >>>>
> > > > > > >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>      
> > > > > > >>>
> > > > > > >>> Thanks for your patch!
> > > > > > >>>      
> > > > > > >>>> --- 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 */      
> > > > > > >>>
> > > > > > >>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > > > >>> the number of bits in the FOURCC format. What do you envision
> > > > > > >>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?      
> > > > > > >>
> > > > > > >> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > > > >> not required, but different fourccs for the same formats do confuse.
> > > > > > >>
> > > > > > >> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > > > >> possible, and if not, invent a new one.      
> > > > > > >
> > > > > > > what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > > > >
> > > > > > > Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > > > > 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > > > > defined by MatrixCoefficients (H.273 terminology)?
> > > > > > >
> > > > > > > That would be my intuitive assumption following how YCbCr is handled.
> > > > > > > Is it obvious enough, or should there be a comment to that effect?      
> > > > > >
> > > > > > You raise an interesting point. Is it defined how a display driver, that
> > > > > > supports R8 as a format, shows R8 on screen? I came into this in the
> > > > > > context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > > > R) in RGB. But you say (R, 0, 0), which... also makes sense.      
> > > > >
> > > > > That is a good question too. I based my assumption on OpenGL behavior
> > > > > of R8.
> > > > >
> > > > > Single channel displays do exist I believe, but being single-channel,
> > > > > expansion on the other channels is likely meaningless. Hm, but for the
> > > > > KMS color pipeline, it would be meaningful, like with a CTM.
> > > > > Interesting.
> > > > >
> > > > > I don't know. Maybe Geert does?      
> > > > 
> > > > I did some digging, and was a bit surprised that it was you who told
> > > > me to use R8 instead of Y8?
> > > > https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell    
> > > 
> > > Hi Geert,
> > > 
> > > indeed I did. I never thought of the question of expansion to R,G,B
> > > before. Maybe that expansion is what spells R8 and Y8 apart?
> > > 
> > > I do think that expansion needs to be specified, so that the KMS color
> > > pipeline computations are defined. There is a big difference between
> > > multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > > 
> > > - (R, 0, 0)
> > > - (R, R, R)
> > > - (c1 * Y, c2 * Y, c3 * Y)    
> > 
> > I'd be very surprised by an YUV to RGB conversion matrix where the first
> > column would contain different values. What we need to take into account
> > though is quantization (full vs. limited range).  

Quantization range is indeed good to note. R8 would be always full
range, but Y8 would follow COLOR_RANGE property.

> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> (R, 0, 0), so we have both variants.
> 
> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> in the KMS color processing?

I think this 0.0 - 1.0 nominal range definition for the abstract KMS
color processing is necessary.

It also means that limited range Y8 data, when containing values 0-15
or 240-255, would produce negative and greater than 1.0 values,
respectively. They might get immediately clamped to 0.0 - 1.0 with the
first color operation they face, though, but the concept seems
important and carrying over to the new color pipelines UAPI which might
choose not to clamp.


Thanks,
pq

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

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

[Tomi Valkeinen]

Hi,

On 01/04/2025 16:27, Pekka Paalanen wrote:
> On Mon, 31 Mar 2025 13:53:37 +0300
> Pekka Paalanen <pekka.paalanen@haloniitty.fi> wrote:
> 
>> On Mon, 31 Mar 2025 11:21:35 +0300
>> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>>
>>> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:
>>>> On Thu, 27 Mar 2025 17:35:39 +0100
>>>> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>      
>>>>> Hi Pekka,
>>>>>
>>>>> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen
>>>>> <pekka.paalanen@haloniitty.fi> wrote:
>>>>>> On Thu, 27 Mar 2025 16:21:16 +0200
>>>>>> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:
>>>>>>> On 27/03/2025 11:20, Pekka Paalanen wrote:
>>>>>>>> On Wed, 26 Mar 2025 15:55:18 +0200
>>>>>>>> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:
>>>>>>>>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:
>>>>>>>>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
>>>>>>>>>> <tomi.valkeinen@ideasonboard.com> wrote:
>>>>>>>>>>> Add greyscale Y8 format.
>>>>>>>>>>>
>>>>>>>>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>>>>>>>>>
>>>>>>>>>> Thanks for your patch!
>>>>>>>>>>       
>>>>>>>>>>> --- 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 */
>>>>>>>>>>
>>>>>>>>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
>>>>>>>>>> the number of bits in the FOURCC format. What do you envision
>>>>>>>>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?
>>>>>>>>>
>>>>>>>>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
>>>>>>>>> not required, but different fourccs for the same formats do confuse.
>>>>>>>>>
>>>>>>>>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
>>>>>>>>> possible, and if not, invent a new one.
>>>>>>>>
>>>>>>>> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
>>>>>>>>
>>>>>>>> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
>>>>>>>> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
>>>>>>>> defined by MatrixCoefficients (H.273 terminology)?
>>>>>>>>
>>>>>>>> That would be my intuitive assumption following how YCbCr is handled.
>>>>>>>> Is it obvious enough, or should there be a comment to that effect?
>>>>>>>
>>>>>>> You raise an interesting point. Is it defined how a display driver, that
>>>>>>> supports R8 as a format, shows R8 on screen? I came into this in the
>>>>>>> context of grayscale formats, so I thought R8 would be handled as (R, R,
>>>>>>> R) in RGB. But you say (R, 0, 0), which... also makes sense.
>>>>>>
>>>>>> That is a good question too. I based my assumption on OpenGL behavior
>>>>>> of R8.
>>>>>>
>>>>>> Single channel displays do exist I believe, but being single-channel,
>>>>>> expansion on the other channels is likely meaningless. Hm, but for the
>>>>>> KMS color pipeline, it would be meaningful, like with a CTM.
>>>>>> Interesting.
>>>>>>
>>>>>> I don't know. Maybe Geert does?
>>>>>
>>>>> I did some digging, and was a bit surprised that it was you who told
>>>>> me to use R8 instead of Y8?
>>>>> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell
>>>>
>>>> Hi Geert,
>>>>
>>>> indeed I did. I never thought of the question of expansion to R,G,B
>>>> before. Maybe that expansion is what spells R8 and Y8 apart?
>>>>
>>>> I do think that expansion needs to be specified, so that the KMS color
>>>> pipeline computations are defined. There is a big difference between
>>>> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
>>>>
>>>> - (R, 0, 0)
>>>> - (R, R, R)
>>>> - (c1 * Y, c2 * Y, c3 * Y)
>>>
>>> I'd be very surprised by an YUV to RGB conversion matrix where the first
>>> column would contain different values. What we need to take into account
>>> though is quantization (full vs. limited range).
> 
> Quantization range is indeed good to note. R8 would be always full
> range, but Y8 would follow COLOR_RANGE property.
> 
>> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
>> (R, 0, 0), so we have both variants.
>>
>> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
>> in the KMS color processing?
> 
> I think this 0.0 - 1.0 nominal range definition for the abstract KMS
> color processing is necessary.
> 
> It also means that limited range Y8 data, when containing values 0-15
> or 240-255, would produce negative and greater than 1.0 values,
> respectively. They might get immediately clamped to 0.0 - 1.0 with the
> first color operation they face, though, but the concept seems
> important and carrying over to the new color pipelines UAPI which might
> choose not to clamp.

Is the behavior of values outside the limited range something that needs 
to be defined? We can't know how each piece of HW behaves with 
"undefined" input, so should we not just define the behavior as platform 
specific?

In any case: I can't say I fully understood all the discussions wrt. 
color spaces. But my immediate interest is, of course, this series =). 
So is there something that you think should be improved here?

My understanding is that the Y-only pixel formats behave in a well 
defined way (or, as well defined as the YUV formats), and there's 
nothing more to add here. Is that right?

  Tomi

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

[Pekka Paalanen]

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

On Wed, 16 Apr 2025 11:59:43 +0300
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> Hi,
> 
> On 01/04/2025 16:27, Pekka Paalanen wrote:
> > On Mon, 31 Mar 2025 13:53:37 +0300
> > Pekka Paalanen <pekka.paalanen@haloniitty.fi> wrote:
> >   
> >> On Mon, 31 Mar 2025 11:21:35 +0300
> >> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> >>  
> >>> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:  
> >>>> On Thu, 27 Mar 2025 17:35:39 +0100
> >>>> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >>>>        
> >>>>> Hi Pekka,
> >>>>>
> >>>>> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen
> >>>>> <pekka.paalanen@haloniitty.fi> wrote:  
> >>>>>> On Thu, 27 Mar 2025 16:21:16 +0200
> >>>>>> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:  
> >>>>>>> On 27/03/2025 11:20, Pekka Paalanen wrote:  
> >>>>>>>> On Wed, 26 Mar 2025 15:55:18 +0200
> >>>>>>>> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:  
> >>>>>>>>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:  
> >>>>>>>>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen
> >>>>>>>>>> <tomi.valkeinen@ideasonboard.com> wrote:  
> >>>>>>>>>>> Add greyscale Y8 format.
> >>>>>>>>>>>
> >>>>>>>>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>>>>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>  
> >>>>>>>>>>
> >>>>>>>>>> Thanks for your patch!
> >>>>>>>>>>         
> >>>>>>>>>>> --- 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 */  
> >>>>>>>>>>
> >>>>>>>>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> >>>>>>>>>> the number of bits in the FOURCC format. What do you envision
> >>>>>>>>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?  
> >>>>>>>>>
> >>>>>>>>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> >>>>>>>>> not required, but different fourccs for the same formats do confuse.
> >>>>>>>>>
> >>>>>>>>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> >>>>>>>>> possible, and if not, invent a new one.  
> >>>>>>>>
> >>>>>>>> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> >>>>>>>>
> >>>>>>>> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> >>>>>>>> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> >>>>>>>> defined by MatrixCoefficients (H.273 terminology)?
> >>>>>>>>
> >>>>>>>> That would be my intuitive assumption following how YCbCr is handled.
> >>>>>>>> Is it obvious enough, or should there be a comment to that effect?  
> >>>>>>>
> >>>>>>> You raise an interesting point. Is it defined how a display driver, that
> >>>>>>> supports R8 as a format, shows R8 on screen? I came into this in the
> >>>>>>> context of grayscale formats, so I thought R8 would be handled as (R, R,
> >>>>>>> R) in RGB. But you say (R, 0, 0), which... also makes sense.  
> >>>>>>
> >>>>>> That is a good question too. I based my assumption on OpenGL behavior
> >>>>>> of R8.
> >>>>>>
> >>>>>> Single channel displays do exist I believe, but being single-channel,
> >>>>>> expansion on the other channels is likely meaningless. Hm, but for the
> >>>>>> KMS color pipeline, it would be meaningful, like with a CTM.
> >>>>>> Interesting.
> >>>>>>
> >>>>>> I don't know. Maybe Geert does?  
> >>>>>
> >>>>> I did some digging, and was a bit surprised that it was you who told
> >>>>> me to use R8 instead of Y8?
> >>>>> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell  
> >>>>
> >>>> Hi Geert,
> >>>>
> >>>> indeed I did. I never thought of the question of expansion to R,G,B
> >>>> before. Maybe that expansion is what spells R8 and Y8 apart?
> >>>>
> >>>> I do think that expansion needs to be specified, so that the KMS color
> >>>> pipeline computations are defined. There is a big difference between
> >>>> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> >>>>
> >>>> - (R, 0, 0)
> >>>> - (R, R, R)
> >>>> - (c1 * Y, c2 * Y, c3 * Y)  
> >>>
> >>> I'd be very surprised by an YUV to RGB conversion matrix where the first
> >>> column would contain different values. What we need to take into account
> >>> though is quantization (full vs. limited range).  
> > 
> > Quantization range is indeed good to note. R8 would be always full
> > range, but Y8 would follow COLOR_RANGE property.
> >   
> >> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> >> (R, 0, 0), so we have both variants.
> >>
> >> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> >> in the KMS color processing?  
> > 
> > I think this 0.0 - 1.0 nominal range definition for the abstract KMS
> > color processing is necessary.
> > 
> > It also means that limited range Y8 data, when containing values 0-15
> > or 240-255, would produce negative and greater than 1.0 values,
> > respectively. They might get immediately clamped to 0.0 - 1.0 with the
> > first color operation they face, though, but the concept seems
> > important and carrying over to the new color pipelines UAPI which might
> > choose not to clamp.  
> 
> Is the behavior of values outside the limited range something that needs 
> to be defined? We can't know how each piece of HW behaves with 
> "undefined" input, so should we not just define the behavior as platform 
> specific?

Hi Tomi,

it's not undefined nor illegal input in general. The so-called
sub-black and super-white ranges exist for a reason, and they are
intended to be used in video processing to avoid clipping in
intermediate processing steps when a filter overshoots a bit. There are
also practices that depend on them, like PLUGE calibration with
traditional signals on a display: https://www.itu.int/rec/R-REC-BT.814

I think it would be really good to have defined behaviour if at all
possible.

> In any case: I can't say I fully understood all the discussions wrt. 
> color spaces. But my immediate interest is, of course, this series =). 
> So is there something that you think should be improved here?

Right, the range discussion is a tangent and applies to all YUV
formats, so it's not a new question.

> My understanding is that the Y-only pixel formats behave in a well 
> defined way (or, as well defined as the YUV formats), and there's 
> nothing more to add here. Is that right?

There are two things:

- Y8 follows COLOR_RANGE property, just like all other YUV formats.
- Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).

I'd like these explicitly written down, so that they become obvious to
everyone. I suspect either one might be easy to forget when writing
code and taking shortcuts without thinking.


Laurent,

I did find a case where (Y', neutral, neutral) does *not* seem to expand
to RGB=(Y, Y, Y): ICtCp. The conversion from ICtCp to L'M'S' does
produce (Y', Y', Y'), but the LMS-to-RGB matrix scrambles it.

I didn't dig through BT.2020 constant-luminance Y'C'bcC'rc, but I
wouldn't be surprised if it scrambled too.

Of course, both of the above are not just one matrix. They require two
matrices and the transfer characteristic each to compute. KMS color
operations cannot implement those today, but with the colorop pipelines
they will if the hardware does it.

That's why I think it's important to document the assumption of Cb and
Cr when not part of the pixel format, and not write down a specific
expansion to RGB like (Y, Y, Y).


Thanks,
pq

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

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

[Laurent Pinchart]

Hi Pekka,

On Thu, Apr 17, 2025 at 11:13:15AM +0300, Pekka Paalanen wrote:
> On Wed, 16 Apr 2025 11:59:43 +0300 Tomi Valkeinen wrote:
> > On 01/04/2025 16:27, Pekka Paalanen wrote:
> > > On Mon, 31 Mar 2025 13:53:37 +0300 Pekka Paalanen wrote:
> > >> On Mon, 31 Mar 2025 11:21:35 +0300 Laurent Pinchart wrote:
> > >>> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:
> > >>>> On Thu, 27 Mar 2025 17:35:39 +0100 Geert Uytterhoeven wrote:
> > >>>>> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen wrote:
> > >>>>>> On Thu, 27 Mar 2025 16:21:16 +0200 Tomi Valkeinen wrote:
> > >>>>>>> On 27/03/2025 11:20, Pekka Paalanen wrote:
> > >>>>>>>> On Wed, 26 Mar 2025 15:55:18 +0200 Tomi Valkeinen wrote:
> > >>>>>>>>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:
> > >>>>>>>>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen wrote:
> > >>>>>>>>>>> Add greyscale Y8 format.
> > >>>>>>>>>>>
> > >>>>>>>>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >>>>>>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>  
> > >>>>>>>>>>
> > >>>>>>>>>> Thanks for your patch!
> > >>>>>>>>>>         
> > >>>>>>>>>>> --- 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 */  
> > >>>>>>>>>>
> > >>>>>>>>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > >>>>>>>>>> the number of bits in the FOURCC format. What do you envision
> > >>>>>>>>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?  
> > >>>>>>>>>
> > >>>>>>>>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > >>>>>>>>> not required, but different fourccs for the same formats do confuse.
> > >>>>>>>>>
> > >>>>>>>>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > >>>>>>>>> possible, and if not, invent a new one.  
> > >>>>>>>>
> > >>>>>>>> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > >>>>>>>>
> > >>>>>>>> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > >>>>>>>> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > >>>>>>>> defined by MatrixCoefficients (H.273 terminology)?
> > >>>>>>>>
> > >>>>>>>> That would be my intuitive assumption following how YCbCr is handled.
> > >>>>>>>> Is it obvious enough, or should there be a comment to that effect?  
> > >>>>>>>
> > >>>>>>> You raise an interesting point. Is it defined how a display driver, that
> > >>>>>>> supports R8 as a format, shows R8 on screen? I came into this in the
> > >>>>>>> context of grayscale formats, so I thought R8 would be handled as (R, R,
> > >>>>>>> R) in RGB. But you say (R, 0, 0), which... also makes sense.  
> > >>>>>>
> > >>>>>> That is a good question too. I based my assumption on OpenGL behavior
> > >>>>>> of R8.
> > >>>>>>
> > >>>>>> Single channel displays do exist I believe, but being single-channel,
> > >>>>>> expansion on the other channels is likely meaningless. Hm, but for the
> > >>>>>> KMS color pipeline, it would be meaningful, like with a CTM.
> > >>>>>> Interesting.
> > >>>>>>
> > >>>>>> I don't know. Maybe Geert does?  
> > >>>>>
> > >>>>> I did some digging, and was a bit surprised that it was you who told
> > >>>>> me to use R8 instead of Y8?
> > >>>>> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell  
> > >>>>
> > >>>> Hi Geert,
> > >>>>
> > >>>> indeed I did. I never thought of the question of expansion to R,G,B
> > >>>> before. Maybe that expansion is what spells R8 and Y8 apart?
> > >>>>
> > >>>> I do think that expansion needs to be specified, so that the KMS color
> > >>>> pipeline computations are defined. There is a big difference between
> > >>>> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > >>>>
> > >>>> - (R, 0, 0)
> > >>>> - (R, R, R)
> > >>>> - (c1 * Y, c2 * Y, c3 * Y)  
> > >>>
> > >>> I'd be very surprised by an YUV to RGB conversion matrix where the first
> > >>> column would contain different values. What we need to take into account
> > >>> though is quantization (full vs. limited range).  
> > > 
> > > Quantization range is indeed good to note. R8 would be always full
> > > range, but Y8 would follow COLOR_RANGE property.
> > >   
> > >> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> > >> (R, 0, 0), so we have both variants.
> > >>
> > >> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> > >> in the KMS color processing?  
> > > 
> > > I think this 0.0 - 1.0 nominal range definition for the abstract KMS
> > > color processing is necessary.
> > > 
> > > It also means that limited range Y8 data, when containing values 0-15
> > > or 240-255, would produce negative and greater than 1.0 values,
> > > respectively. They might get immediately clamped to 0.0 - 1.0 with the
> > > first color operation they face, though, but the concept seems
> > > important and carrying over to the new color pipelines UAPI which might
> > > choose not to clamp.  
> > 
> > Is the behavior of values outside the limited range something that needs 
> > to be defined? We can't know how each piece of HW behaves with 
> > "undefined" input, so should we not just define the behavior as platform 
> > specific?
> 
> Hi Tomi,
> 
> it's not undefined nor illegal input in general. The so-called
> sub-black and super-white ranges exist for a reason, and they are
> intended to be used in video processing to avoid clipping in
> intermediate processing steps when a filter overshoots a bit. There are
> also practices that depend on them, like PLUGE calibration with
> traditional signals on a display: https://www.itu.int/rec/R-REC-BT.814
> 
> I think it would be really good to have defined behaviour if at all
> possible.
> 
> > In any case: I can't say I fully understood all the discussions wrt. 
> > color spaces. But my immediate interest is, of course, this series =). 
> > So is there something that you think should be improved here?
> 
> Right, the range discussion is a tangent and applies to all YUV
> formats, so it's not a new question.
> 
> > My understanding is that the Y-only pixel formats behave in a well 
> > defined way (or, as well defined as the YUV formats), and there's 
> > nothing more to add here. Is that right?
> 
> There are two things:
> 
> - Y8 follows COLOR_RANGE property, just like all other YUV formats.
> - Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).
> 
> I'd like these explicitly written down, so that they become obvious to
> everyone. I suspect either one might be easy to forget when writing
> code and taking shortcuts without thinking.
> 
> 
> Laurent,
> 
> I did find a case where (Y', neutral, neutral) does *not* seem to expand
> to RGB=(Y, Y, Y): ICtCp. The conversion from ICtCp to L'M'S' does
> produce (Y', Y', Y'), but the LMS-to-RGB matrix scrambles it.
> 
> I didn't dig through BT.2020 constant-luminance Y'C'bcC'rc, but I
> wouldn't be surprised if it scrambled too.
> 
> Of course, both of the above are not just one matrix. They require two
> matrices and the transfer characteristic each to compute. KMS color
> operations cannot implement those today, but with the colorop pipelines
> they will if the hardware does it.
> 
> That's why I think it's important to document the assumption of Cb and
> Cr when not part of the pixel format, and not write down a specific
> expansion to RGB like (Y, Y, Y).

Every time I discuss color spaces, the scopes of "RGB" and "YUV" seem to
expand more and more. This makes me wonder how we define those two
concepts. Taking the conversion from RGB to ICtCp as an example, would
you consider LMS and L'M'S' as "RGB" formats, and ICtCp as a "YUV"
format ?

-- 
Regards,

Laurent Pinchart

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

[Pekka Paalanen]

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

On Mon, 21 Apr 2025 17:50:39 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> Hi Pekka,
> 
> On Thu, Apr 17, 2025 at 11:13:15AM +0300, Pekka Paalanen wrote:
> > On Wed, 16 Apr 2025 11:59:43 +0300 Tomi Valkeinen wrote:  
> > > On 01/04/2025 16:27, Pekka Paalanen wrote:  
> > > > On Mon, 31 Mar 2025 13:53:37 +0300 Pekka Paalanen wrote:  
> > > >> On Mon, 31 Mar 2025 11:21:35 +0300 Laurent Pinchart wrote:  
> > > >>> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:  
> > > >>>> On Thu, 27 Mar 2025 17:35:39 +0100 Geert Uytterhoeven wrote:  
> > > >>>>> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen wrote:  
> > > >>>>>> On Thu, 27 Mar 2025 16:21:16 +0200 Tomi Valkeinen wrote:  
> > > >>>>>>> On 27/03/2025 11:20, Pekka Paalanen wrote:  
> > > >>>>>>>> On Wed, 26 Mar 2025 15:55:18 +0200 Tomi Valkeinen wrote:  
> > > >>>>>>>>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:  
> > > >>>>>>>>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen wrote:  
> > > >>>>>>>>>>> Add greyscale Y8 format.
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > >>>>>>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>    
> > > >>>>>>>>>>
> > > >>>>>>>>>> Thanks for your patch!
> > > >>>>>>>>>>           
> > > >>>>>>>>>>> --- 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 */    
> > > >>>>>>>>>>
> > > >>>>>>>>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > >>>>>>>>>> the number of bits in the FOURCC format. What do you envision
> > > >>>>>>>>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?    
> > > >>>>>>>>>
> > > >>>>>>>>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > >>>>>>>>> not required, but different fourccs for the same formats do confuse.
> > > >>>>>>>>>
> > > >>>>>>>>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > >>>>>>>>> possible, and if not, invent a new one.    
> > > >>>>>>>>
> > > >>>>>>>> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > >>>>>>>>
> > > >>>>>>>> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > >>>>>>>> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > >>>>>>>> defined by MatrixCoefficients (H.273 terminology)?
> > > >>>>>>>>
> > > >>>>>>>> That would be my intuitive assumption following how YCbCr is handled.
> > > >>>>>>>> Is it obvious enough, or should there be a comment to that effect?    
> > > >>>>>>>
> > > >>>>>>> You raise an interesting point. Is it defined how a display driver, that
> > > >>>>>>> supports R8 as a format, shows R8 on screen? I came into this in the
> > > >>>>>>> context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > >>>>>>> R) in RGB. But you say (R, 0, 0), which... also makes sense.    
> > > >>>>>>
> > > >>>>>> That is a good question too. I based my assumption on OpenGL behavior
> > > >>>>>> of R8.
> > > >>>>>>
> > > >>>>>> Single channel displays do exist I believe, but being single-channel,
> > > >>>>>> expansion on the other channels is likely meaningless. Hm, but for the
> > > >>>>>> KMS color pipeline, it would be meaningful, like with a CTM.
> > > >>>>>> Interesting.
> > > >>>>>>
> > > >>>>>> I don't know. Maybe Geert does?    
> > > >>>>>
> > > >>>>> I did some digging, and was a bit surprised that it was you who told
> > > >>>>> me to use R8 instead of Y8?
> > > >>>>> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell    
> > > >>>>
> > > >>>> Hi Geert,
> > > >>>>
> > > >>>> indeed I did. I never thought of the question of expansion to R,G,B
> > > >>>> before. Maybe that expansion is what spells R8 and Y8 apart?
> > > >>>>
> > > >>>> I do think that expansion needs to be specified, so that the KMS color
> > > >>>> pipeline computations are defined. There is a big difference between
> > > >>>> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > > >>>>
> > > >>>> - (R, 0, 0)
> > > >>>> - (R, R, R)
> > > >>>> - (c1 * Y, c2 * Y, c3 * Y)    
> > > >>>
> > > >>> I'd be very surprised by an YUV to RGB conversion matrix where the first
> > > >>> column would contain different values. What we need to take into account
> > > >>> though is quantization (full vs. limited range).    
> > > > 
> > > > Quantization range is indeed good to note. R8 would be always full
> > > > range, but Y8 would follow COLOR_RANGE property.
> > > >     
> > > >> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> > > >> (R, 0, 0), so we have both variants.
> > > >>
> > > >> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> > > >> in the KMS color processing?    
> > > > 
> > > > I think this 0.0 - 1.0 nominal range definition for the abstract KMS
> > > > color processing is necessary.
> > > > 
> > > > It also means that limited range Y8 data, when containing values 0-15
> > > > or 240-255, would produce negative and greater than 1.0 values,
> > > > respectively. They might get immediately clamped to 0.0 - 1.0 with the
> > > > first color operation they face, though, but the concept seems
> > > > important and carrying over to the new color pipelines UAPI which might
> > > > choose not to clamp.    
> > > 
> > > Is the behavior of values outside the limited range something that needs 
> > > to be defined? We can't know how each piece of HW behaves with 
> > > "undefined" input, so should we not just define the behavior as platform 
> > > specific?  
> > 
> > Hi Tomi,
> > 
> > it's not undefined nor illegal input in general. The so-called
> > sub-black and super-white ranges exist for a reason, and they are
> > intended to be used in video processing to avoid clipping in
> > intermediate processing steps when a filter overshoots a bit. There are
> > also practices that depend on them, like PLUGE calibration with
> > traditional signals on a display: https://www.itu.int/rec/R-REC-BT.814
> > 
> > I think it would be really good to have defined behaviour if at all
> > possible.
> >   
> > > In any case: I can't say I fully understood all the discussions wrt. 
> > > color spaces. But my immediate interest is, of course, this series =). 
> > > So is there something that you think should be improved here?  
> > 
> > Right, the range discussion is a tangent and applies to all YUV
> > formats, so it's not a new question.
> >   
> > > My understanding is that the Y-only pixel formats behave in a well 
> > > defined way (or, as well defined as the YUV formats), and there's 
> > > nothing more to add here. Is that right?  
> > 
> > There are two things:
> > 
> > - Y8 follows COLOR_RANGE property, just like all other YUV formats.
> > - Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).
> > 
> > I'd like these explicitly written down, so that they become obvious to
> > everyone. I suspect either one might be easy to forget when writing
> > code and taking shortcuts without thinking.
> > 
> > 
> > Laurent,
> > 
> > I did find a case where (Y', neutral, neutral) does *not* seem to expand
> > to RGB=(Y, Y, Y): ICtCp. The conversion from ICtCp to L'M'S' does
> > produce (Y', Y', Y'), but the LMS-to-RGB matrix scrambles it.
> > 
> > I didn't dig through BT.2020 constant-luminance Y'C'bcC'rc, but I
> > wouldn't be surprised if it scrambled too.
> > 
> > Of course, both of the above are not just one matrix. They require two
> > matrices and the transfer characteristic each to compute. KMS color
> > operations cannot implement those today, but with the colorop pipelines
> > they will if the hardware does it.
> > 
> > That's why I think it's important to document the assumption of Cb and
> > Cr when not part of the pixel format, and not write down a specific
> > expansion to RGB like (Y, Y, Y).  
> 
> Every time I discuss color spaces, the scopes of "RGB" and "YUV" seem to
> expand more and more. This makes me wonder how we define those two
> concepts. Taking the conversion from RGB to ICtCp as an example, would
> you consider LMS and L'M'S' as "RGB" formats, and ICtCp as a "YUV"
> format ?

Hi Laurent,

sorry for the confusion. In this specific context, my use of RGB and
YUV refers to the channels in DRM pixel formats. It might have been
better if all channels in all pixel formats were "anonymous" and merely
numbered because all formats can be used for any color model, but this
is what we have.

There is some disambiguation in
https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
The doc is some years old, so nowadays I might phrase things
differently, but maybe it's easier to read for those new to things as I
wrote it when I was just learning things.

I would classify ICtCp in the YUV pixel format category, because the
CtCp plane can be sub-sampled (right?). I would classify LMS and L'M'S'
in the RGB pixel format category because they are not sub-sampled AFAIK
although they also do not actually appear as buffer contents, so the
relation to pixel formats is... theoretical.

IOW, we have a completely artificial split of DRM pixel formats to RGB
and YUV where the only essential difference is that YUV formats can have
sub-sampled variants and RGB formats do not.


Thanks,
pq

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

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

[Geert Uytterhoeven]

Hi Pekka,

On Tue, 22 Apr 2025 at 11:11, Pekka Paalanen
<pekka.paalanen@haloniitty.fi> wrote:
> On Mon, 21 Apr 2025 17:50:39 +0300
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > On Thu, Apr 17, 2025 at 11:13:15AM +0300, Pekka Paalanen wrote:
> > > On Wed, 16 Apr 2025 11:59:43 +0300 Tomi Valkeinen wrote:
> > > > On 01/04/2025 16:27, Pekka Paalanen wrote:
> > > > > On Mon, 31 Mar 2025 13:53:37 +0300 Pekka Paalanen wrote:
> > > > >> On Mon, 31 Mar 2025 11:21:35 +0300 Laurent Pinchart wrote:
> > > > >>> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:
> > > > >>>> On Thu, 27 Mar 2025 17:35:39 +0100 Geert Uytterhoeven wrote:
> > > > >>>>> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen wrote:
> > > > >>>>>> On Thu, 27 Mar 2025 16:21:16 +0200 Tomi Valkeinen wrote:
> > > > >>>>>>> On 27/03/2025 11:20, Pekka Paalanen wrote:
> > > > >>>>>>>> On Wed, 26 Mar 2025 15:55:18 +0200 Tomi Valkeinen wrote:
> > > > >>>>>>>>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:
> > > > >>>>>>>>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen wrote:
> > > > >>>>>>>>>>> Add greyscale Y8 format.
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > >>>>>>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> Thanks for your patch!
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>> --- 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 */
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > >>>>>>>>>> the number of bits in the FOURCC format. What do you envision
> > > > >>>>>>>>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?
> > > > >>>>>>>>>
> > > > >>>>>>>>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > >>>>>>>>> not required, but different fourccs for the same formats do confuse.
> > > > >>>>>>>>>
> > > > >>>>>>>>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > >>>>>>>>> possible, and if not, invent a new one.
> > > > >>>>>>>>
> > > > >>>>>>>> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > >>>>>>>>
> > > > >>>>>>>> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > >>>>>>>> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > >>>>>>>> defined by MatrixCoefficients (H.273 terminology)?
> > > > >>>>>>>>
> > > > >>>>>>>> That would be my intuitive assumption following how YCbCr is handled.
> > > > >>>>>>>> Is it obvious enough, or should there be a comment to that effect?
> > > > >>>>>>>
> > > > >>>>>>> You raise an interesting point. Is it defined how a display driver, that
> > > > >>>>>>> supports R8 as a format, shows R8 on screen? I came into this in the
> > > > >>>>>>> context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > >>>>>>> R) in RGB. But you say (R, 0, 0), which... also makes sense.
> > > > >>>>>>
> > > > >>>>>> That is a good question too. I based my assumption on OpenGL behavior
> > > > >>>>>> of R8.
> > > > >>>>>>
> > > > >>>>>> Single channel displays do exist I believe, but being single-channel,
> > > > >>>>>> expansion on the other channels is likely meaningless. Hm, but for the
> > > > >>>>>> KMS color pipeline, it would be meaningful, like with a CTM.
> > > > >>>>>> Interesting.
> > > > >>>>>>
> > > > >>>>>> I don't know. Maybe Geert does?
> > > > >>>>>
> > > > >>>>> I did some digging, and was a bit surprised that it was you who told
> > > > >>>>> me to use R8 instead of Y8?
> > > > >>>>> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell
> > > > >>>>
> > > > >>>> Hi Geert,
> > > > >>>>
> > > > >>>> indeed I did. I never thought of the question of expansion to R,G,B
> > > > >>>> before. Maybe that expansion is what spells R8 and Y8 apart?
> > > > >>>>
> > > > >>>> I do think that expansion needs to be specified, so that the KMS color
> > > > >>>> pipeline computations are defined. There is a big difference between
> > > > >>>> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > > > >>>>
> > > > >>>> - (R, 0, 0)
> > > > >>>> - (R, R, R)
> > > > >>>> - (c1 * Y, c2 * Y, c3 * Y)
> > > > >>>
> > > > >>> I'd be very surprised by an YUV to RGB conversion matrix where the first
> > > > >>> column would contain different values. What we need to take into account
> > > > >>> though is quantization (full vs. limited range).
> > > > >
> > > > > Quantization range is indeed good to note. R8 would be always full
> > > > > range, but Y8 would follow COLOR_RANGE property.
> > > > >
> > > > >> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> > > > >> (R, 0, 0), so we have both variants.
> > > > >>
> > > > >> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> > > > >> in the KMS color processing?
> > > > >
> > > > > I think this 0.0 - 1.0 nominal range definition for the abstract KMS
> > > > > color processing is necessary.
> > > > >
> > > > > It also means that limited range Y8 data, when containing values 0-15
> > > > > or 240-255, would produce negative and greater than 1.0 values,
> > > > > respectively. They might get immediately clamped to 0.0 - 1.0 with the
> > > > > first color operation they face, though, but the concept seems
> > > > > important and carrying over to the new color pipelines UAPI which might
> > > > > choose not to clamp.
> > > >
> > > > Is the behavior of values outside the limited range something that needs
> > > > to be defined? We can't know how each piece of HW behaves with
> > > > "undefined" input, so should we not just define the behavior as platform
> > > > specific?
> > >
> > > Hi Tomi,
> > >
> > > it's not undefined nor illegal input in general. The so-called
> > > sub-black and super-white ranges exist for a reason, and they are
> > > intended to be used in video processing to avoid clipping in
> > > intermediate processing steps when a filter overshoots a bit. There are
> > > also practices that depend on them, like PLUGE calibration with
> > > traditional signals on a display: https://www.itu.int/rec/R-REC-BT.814
> > >
> > > I think it would be really good to have defined behaviour if at all
> > > possible.
> > >
> > > > In any case: I can't say I fully understood all the discussions wrt.
> > > > color spaces. But my immediate interest is, of course, this series =).
> > > > So is there something that you think should be improved here?
> > >
> > > Right, the range discussion is a tangent and applies to all YUV
> > > formats, so it's not a new question.
> > >
> > > > My understanding is that the Y-only pixel formats behave in a well
> > > > defined way (or, as well defined as the YUV formats), and there's
> > > > nothing more to add here. Is that right?
> > >
> > > There are two things:
> > >
> > > - Y8 follows COLOR_RANGE property, just like all other YUV formats.
> > > - Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).
> > >
> > > I'd like these explicitly written down, so that they become obvious to
> > > everyone. I suspect either one might be easy to forget when writing
> > > code and taking shortcuts without thinking.
> > >
> > >
> > > Laurent,
> > >
> > > I did find a case where (Y', neutral, neutral) does *not* seem to expand
> > > to RGB=(Y, Y, Y): ICtCp. The conversion from ICtCp to L'M'S' does
> > > produce (Y', Y', Y'), but the LMS-to-RGB matrix scrambles it.
> > >
> > > I didn't dig through BT.2020 constant-luminance Y'C'bcC'rc, but I
> > > wouldn't be surprised if it scrambled too.
> > >
> > > Of course, both of the above are not just one matrix. They require two
> > > matrices and the transfer characteristic each to compute. KMS color
> > > operations cannot implement those today, but with the colorop pipelines
> > > they will if the hardware does it.
> > >
> > > That's why I think it's important to document the assumption of Cb and
> > > Cr when not part of the pixel format, and not write down a specific
> > > expansion to RGB like (Y, Y, Y).
> >
> > Every time I discuss color spaces, the scopes of "RGB" and "YUV" seem to
> > expand more and more. This makes me wonder how we define those two
> > concepts. Taking the conversion from RGB to ICtCp as an example, would
> > you consider LMS and L'M'S' as "RGB" formats, and ICtCp as a "YUV"
> > format ?
>
> sorry for the confusion. In this specific context, my use of RGB and
> YUV refers to the channels in DRM pixel formats. It might have been
> better if all channels in all pixel formats were "anonymous" and merely
> numbered because all formats can be used for any color model, but this
> is what we have.
>
> There is some disambiguation in
> https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
> The doc is some years old, so nowadays I might phrase things
> differently, but maybe it's easier to read for those new to things as I
> wrote it when I was just learning things.
>
> I would classify ICtCp in the YUV pixel format category, because the
> CtCp plane can be sub-sampled (right?). I would classify LMS and L'M'S'
> in the RGB pixel format category because they are not sub-sampled AFAIK
> although they also do not actually appear as buffer contents, so the
> relation to pixel formats is... theoretical.
>
> IOW, we have a completely artificial split of DRM pixel formats to RGB
> and YUV where the only essential difference is that YUV formats can have
> sub-sampled variants and RGB formats do not.

RGB can be subsampled, too...
https://en.wikipedia.org/wiki/Bayer_filter

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Pekka Paalanen]

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

On Tue, 22 Apr 2025 11:41:29 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Pekka,
> 
> On Tue, 22 Apr 2025 at 11:11, Pekka Paalanen
> <pekka.paalanen@haloniitty.fi> wrote:
> > On Mon, 21 Apr 2025 17:50:39 +0300
> > Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:  
> > > On Thu, Apr 17, 2025 at 11:13:15AM +0300, Pekka Paalanen wrote:  
> > > > On Wed, 16 Apr 2025 11:59:43 +0300 Tomi Valkeinen wrote:  
> > > > > On 01/04/2025 16:27, Pekka Paalanen wrote:  
> > > > > > On Mon, 31 Mar 2025 13:53:37 +0300 Pekka Paalanen wrote:  
> > > > > >> On Mon, 31 Mar 2025 11:21:35 +0300 Laurent Pinchart wrote:  
> > > > > >>> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:  
> > > > > >>>> On Thu, 27 Mar 2025 17:35:39 +0100 Geert Uytterhoeven wrote:  
> > > > > >>>>> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen wrote:  
> > > > > >>>>>> On Thu, 27 Mar 2025 16:21:16 +0200 Tomi Valkeinen wrote:  
> > > > > >>>>>>> On 27/03/2025 11:20, Pekka Paalanen wrote:  
> > > > > >>>>>>>> On Wed, 26 Mar 2025 15:55:18 +0200 Tomi Valkeinen wrote:  
> > > > > >>>>>>>>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:  
> > > > > >>>>>>>>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen wrote:  
> > > > > >>>>>>>>>>> Add greyscale Y8 format.
> > > > > >>>>>>>>>>>
> > > > > >>>>>>>>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > >>>>>>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>  
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>> Thanks for your patch!
> > > > > >>>>>>>>>>  
> > > > > >>>>>>>>>>> --- 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 */  
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > > >>>>>>>>>> the number of bits in the FOURCC format. What do you envision
> > > > > >>>>>>>>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?  
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > > >>>>>>>>> not required, but different fourccs for the same formats do confuse.
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > > >>>>>>>>> possible, and if not, invent a new one.  
> > > > > >>>>>>>>
> > > > > >>>>>>>> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > > >>>>>>>>
> > > > > >>>>>>>> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > > >>>>>>>> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > > >>>>>>>> defined by MatrixCoefficients (H.273 terminology)?
> > > > > >>>>>>>>
> > > > > >>>>>>>> That would be my intuitive assumption following how YCbCr is handled.
> > > > > >>>>>>>> Is it obvious enough, or should there be a comment to that effect?  
> > > > > >>>>>>>
> > > > > >>>>>>> You raise an interesting point. Is it defined how a display driver, that
> > > > > >>>>>>> supports R8 as a format, shows R8 on screen? I came into this in the
> > > > > >>>>>>> context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > > >>>>>>> R) in RGB. But you say (R, 0, 0), which... also makes sense.  
> > > > > >>>>>>
> > > > > >>>>>> That is a good question too. I based my assumption on OpenGL behavior
> > > > > >>>>>> of R8.
> > > > > >>>>>>
> > > > > >>>>>> Single channel displays do exist I believe, but being single-channel,
> > > > > >>>>>> expansion on the other channels is likely meaningless. Hm, but for the
> > > > > >>>>>> KMS color pipeline, it would be meaningful, like with a CTM.
> > > > > >>>>>> Interesting.
> > > > > >>>>>>
> > > > > >>>>>> I don't know. Maybe Geert does?  
> > > > > >>>>>
> > > > > >>>>> I did some digging, and was a bit surprised that it was you who told
> > > > > >>>>> me to use R8 instead of Y8?
> > > > > >>>>> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell  
> > > > > >>>>
> > > > > >>>> Hi Geert,
> > > > > >>>>
> > > > > >>>> indeed I did. I never thought of the question of expansion to R,G,B
> > > > > >>>> before. Maybe that expansion is what spells R8 and Y8 apart?
> > > > > >>>>
> > > > > >>>> I do think that expansion needs to be specified, so that the KMS color
> > > > > >>>> pipeline computations are defined. There is a big difference between
> > > > > >>>> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > > > > >>>>
> > > > > >>>> - (R, 0, 0)
> > > > > >>>> - (R, R, R)
> > > > > >>>> - (c1 * Y, c2 * Y, c3 * Y)  
> > > > > >>>
> > > > > >>> I'd be very surprised by an YUV to RGB conversion matrix where the first
> > > > > >>> column would contain different values. What we need to take into account
> > > > > >>> though is quantization (full vs. limited range).  
> > > > > >
> > > > > > Quantization range is indeed good to note. R8 would be always full
> > > > > > range, but Y8 would follow COLOR_RANGE property.
> > > > > >  
> > > > > >> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> > > > > >> (R, 0, 0), so we have both variants.
> > > > > >>
> > > > > >> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> > > > > >> in the KMS color processing?  
> > > > > >
> > > > > > I think this 0.0 - 1.0 nominal range definition for the abstract KMS
> > > > > > color processing is necessary.
> > > > > >
> > > > > > It also means that limited range Y8 data, when containing values 0-15
> > > > > > or 240-255, would produce negative and greater than 1.0 values,
> > > > > > respectively. They might get immediately clamped to 0.0 - 1.0 with the
> > > > > > first color operation they face, though, but the concept seems
> > > > > > important and carrying over to the new color pipelines UAPI which might
> > > > > > choose not to clamp.  
> > > > >
> > > > > Is the behavior of values outside the limited range something that needs
> > > > > to be defined? We can't know how each piece of HW behaves with
> > > > > "undefined" input, so should we not just define the behavior as platform
> > > > > specific?  
> > > >
> > > > Hi Tomi,
> > > >
> > > > it's not undefined nor illegal input in general. The so-called
> > > > sub-black and super-white ranges exist for a reason, and they are
> > > > intended to be used in video processing to avoid clipping in
> > > > intermediate processing steps when a filter overshoots a bit. There are
> > > > also practices that depend on them, like PLUGE calibration with
> > > > traditional signals on a display: https://www.itu.int/rec/R-REC-BT.814
> > > >
> > > > I think it would be really good to have defined behaviour if at all
> > > > possible.
> > > >  
> > > > > In any case: I can't say I fully understood all the discussions wrt.
> > > > > color spaces. But my immediate interest is, of course, this series =).
> > > > > So is there something that you think should be improved here?  
> > > >
> > > > Right, the range discussion is a tangent and applies to all YUV
> > > > formats, so it's not a new question.
> > > >  
> > > > > My understanding is that the Y-only pixel formats behave in a well
> > > > > defined way (or, as well defined as the YUV formats), and there's
> > > > > nothing more to add here. Is that right?  
> > > >
> > > > There are two things:
> > > >
> > > > - Y8 follows COLOR_RANGE property, just like all other YUV formats.
> > > > - Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).
> > > >
> > > > I'd like these explicitly written down, so that they become obvious to
> > > > everyone. I suspect either one might be easy to forget when writing
> > > > code and taking shortcuts without thinking.
> > > >
> > > >
> > > > Laurent,
> > > >
> > > > I did find a case where (Y', neutral, neutral) does *not* seem to expand
> > > > to RGB=(Y, Y, Y): ICtCp. The conversion from ICtCp to L'M'S' does
> > > > produce (Y', Y', Y'), but the LMS-to-RGB matrix scrambles it.
> > > >
> > > > I didn't dig through BT.2020 constant-luminance Y'C'bcC'rc, but I
> > > > wouldn't be surprised if it scrambled too.
> > > >
> > > > Of course, both of the above are not just one matrix. They require two
> > > > matrices and the transfer characteristic each to compute. KMS color
> > > > operations cannot implement those today, but with the colorop pipelines
> > > > they will if the hardware does it.
> > > >
> > > > That's why I think it's important to document the assumption of Cb and
> > > > Cr when not part of the pixel format, and not write down a specific
> > > > expansion to RGB like (Y, Y, Y).  
> > >
> > > Every time I discuss color spaces, the scopes of "RGB" and "YUV" seem to
> > > expand more and more. This makes me wonder how we define those two
> > > concepts. Taking the conversion from RGB to ICtCp as an example, would
> > > you consider LMS and L'M'S' as "RGB" formats, and ICtCp as a "YUV"
> > > format ?  
> >
> > sorry for the confusion. In this specific context, my use of RGB and
> > YUV refers to the channels in DRM pixel formats. It might have been
> > better if all channels in all pixel formats were "anonymous" and merely
> > numbered because all formats can be used for any color model, but this
> > is what we have.
> >
> > There is some disambiguation in
> > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
> > The doc is some years old, so nowadays I might phrase things
> > differently, but maybe it's easier to read for those new to things as I
> > wrote it when I was just learning things.
> >
> > I would classify ICtCp in the YUV pixel format category, because the
> > CtCp plane can be sub-sampled (right?). I would classify LMS and L'M'S'
> > in the RGB pixel format category because they are not sub-sampled AFAIK
> > although they also do not actually appear as buffer contents, so the
> > relation to pixel formats is... theoretical.
> >
> > IOW, we have a completely artificial split of DRM pixel formats to RGB
> > and YUV where the only essential difference is that YUV formats can have
> > sub-sampled variants and RGB formats do not.  
> 
> RGB can be subsampled, too...
> https://en.wikipedia.org/wiki/Bayer_filter

That's true. What difference are we left with, then?

We have DRM pixel formats which imply some color model, but do not
define the variant of each color model (e.g. the Y'CbCr-to-RGB matrix).

I guess the implied color model then implies which API, e.g. KMS plane
property, is responsible for defining the variant.


Thanks,
pq

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

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

[Geert Uytterhoeven]

Hi Pekka,

On Tue, 22 Apr 2025 at 12:01, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> On Tue, 22 Apr 2025 11:41:29 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, 22 Apr 2025 at 11:11, Pekka Paalanen
> > <pekka.paalanen@haloniitty.fi> wrote:
> > > On Mon, 21 Apr 2025 17:50:39 +0300
> > > Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > > > On Thu, Apr 17, 2025 at 11:13:15AM +0300, Pekka Paalanen wrote:
> > > > > On Wed, 16 Apr 2025 11:59:43 +0300 Tomi Valkeinen wrote:
> > > > > > On 01/04/2025 16:27, Pekka Paalanen wrote:
> > > > > > > On Mon, 31 Mar 2025 13:53:37 +0300 Pekka Paalanen wrote:
> > > > > > >> On Mon, 31 Mar 2025 11:21:35 +0300 Laurent Pinchart wrote:
> > > > > > >>> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:
> > > > > > >>>> On Thu, 27 Mar 2025 17:35:39 +0100 Geert Uytterhoeven wrote:
> > > > > > >>>>> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen wrote:
> > > > > > >>>>>> On Thu, 27 Mar 2025 16:21:16 +0200 Tomi Valkeinen wrote:
> > > > > > >>>>>>> On 27/03/2025 11:20, Pekka Paalanen wrote:
> > > > > > >>>>>>>> On Wed, 26 Mar 2025 15:55:18 +0200 Tomi Valkeinen wrote:
> > > > > > >>>>>>>>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:
> > > > > > >>>>>>>>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen wrote:
> > > > > > >>>>>>>>>>> Add greyscale Y8 format.
> > > > > > >>>>>>>>>>>
> > > > > > >>>>>>>>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > >>>>>>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > > > > > >>>>>>>>>>
> > > > > > >>>>>>>>>> Thanks for your patch!
> > > > > > >>>>>>>>>>
> > > > > > >>>>>>>>>>> --- 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 */
> > > > > > >>>>>>>>>>
> > > > > > >>>>>>>>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > > > >>>>>>>>>> the number of bits in the FOURCC format. What do you envision
> > > > > > >>>>>>>>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > > > >>>>>>>>> not required, but different fourccs for the same formats do confuse.
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > > > >>>>>>>>> possible, and if not, invent a new one.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > > > >>>>>>>> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > > > >>>>>>>> defined by MatrixCoefficients (H.273 terminology)?
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> That would be my intuitive assumption following how YCbCr is handled.
> > > > > > >>>>>>>> Is it obvious enough, or should there be a comment to that effect?
> > > > > > >>>>>>>
> > > > > > >>>>>>> You raise an interesting point. Is it defined how a display driver, that
> > > > > > >>>>>>> supports R8 as a format, shows R8 on screen? I came into this in the
> > > > > > >>>>>>> context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > > > >>>>>>> R) in RGB. But you say (R, 0, 0), which... also makes sense.
> > > > > > >>>>>>
> > > > > > >>>>>> That is a good question too. I based my assumption on OpenGL behavior
> > > > > > >>>>>> of R8.
> > > > > > >>>>>>
> > > > > > >>>>>> Single channel displays do exist I believe, but being single-channel,
> > > > > > >>>>>> expansion on the other channels is likely meaningless. Hm, but for the
> > > > > > >>>>>> KMS color pipeline, it would be meaningful, like with a CTM.
> > > > > > >>>>>> Interesting.
> > > > > > >>>>>>
> > > > > > >>>>>> I don't know. Maybe Geert does?
> > > > > > >>>>>
> > > > > > >>>>> I did some digging, and was a bit surprised that it was you who told
> > > > > > >>>>> me to use R8 instead of Y8?
> > > > > > >>>>> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell
> > > > > > >>>>
> > > > > > >>>> Hi Geert,
> > > > > > >>>>
> > > > > > >>>> indeed I did. I never thought of the question of expansion to R,G,B
> > > > > > >>>> before. Maybe that expansion is what spells R8 and Y8 apart?
> > > > > > >>>>
> > > > > > >>>> I do think that expansion needs to be specified, so that the KMS color
> > > > > > >>>> pipeline computations are defined. There is a big difference between
> > > > > > >>>> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > > > > > >>>>
> > > > > > >>>> - (R, 0, 0)
> > > > > > >>>> - (R, R, R)
> > > > > > >>>> - (c1 * Y, c2 * Y, c3 * Y)
> > > > > > >>>
> > > > > > >>> I'd be very surprised by an YUV to RGB conversion matrix where the first
> > > > > > >>> column would contain different values. What we need to take into account
> > > > > > >>> though is quantization (full vs. limited range).
> > > > > > >
> > > > > > > Quantization range is indeed good to note. R8 would be always full
> > > > > > > range, but Y8 would follow COLOR_RANGE property.
> > > > > > >
> > > > > > >> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> > > > > > >> (R, 0, 0), so we have both variants.
> > > > > > >>
> > > > > > >> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> > > > > > >> in the KMS color processing?
> > > > > > >
> > > > > > > I think this 0.0 - 1.0 nominal range definition for the abstract KMS
> > > > > > > color processing is necessary.
> > > > > > >
> > > > > > > It also means that limited range Y8 data, when containing values 0-15
> > > > > > > or 240-255, would produce negative and greater than 1.0 values,
> > > > > > > respectively. They might get immediately clamped to 0.0 - 1.0 with the
> > > > > > > first color operation they face, though, but the concept seems
> > > > > > > important and carrying over to the new color pipelines UAPI which might
> > > > > > > choose not to clamp.
> > > > > >
> > > > > > Is the behavior of values outside the limited range something that needs
> > > > > > to be defined? We can't know how each piece of HW behaves with
> > > > > > "undefined" input, so should we not just define the behavior as platform
> > > > > > specific?
> > > > >
> > > > > Hi Tomi,
> > > > >
> > > > > it's not undefined nor illegal input in general. The so-called
> > > > > sub-black and super-white ranges exist for a reason, and they are
> > > > > intended to be used in video processing to avoid clipping in
> > > > > intermediate processing steps when a filter overshoots a bit. There are
> > > > > also practices that depend on them, like PLUGE calibration with
> > > > > traditional signals on a display: https://www.itu.int/rec/R-REC-BT.814
> > > > >
> > > > > I think it would be really good to have defined behaviour if at all
> > > > > possible.
> > > > >
> > > > > > In any case: I can't say I fully understood all the discussions wrt.
> > > > > > color spaces. But my immediate interest is, of course, this series =).
> > > > > > So is there something that you think should be improved here?
> > > > >
> > > > > Right, the range discussion is a tangent and applies to all YUV
> > > > > formats, so it's not a new question.
> > > > >
> > > > > > My understanding is that the Y-only pixel formats behave in a well
> > > > > > defined way (or, as well defined as the YUV formats), and there's
> > > > > > nothing more to add here. Is that right?
> > > > >
> > > > > There are two things:
> > > > >
> > > > > - Y8 follows COLOR_RANGE property, just like all other YUV formats.
> > > > > - Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).
> > > > >
> > > > > I'd like these explicitly written down, so that they become obvious to
> > > > > everyone. I suspect either one might be easy to forget when writing
> > > > > code and taking shortcuts without thinking.
> > > > >
> > > > >
> > > > > Laurent,
> > > > >
> > > > > I did find a case where (Y', neutral, neutral) does *not* seem to expand
> > > > > to RGB=(Y, Y, Y): ICtCp. The conversion from ICtCp to L'M'S' does
> > > > > produce (Y', Y', Y'), but the LMS-to-RGB matrix scrambles it.
> > > > >
> > > > > I didn't dig through BT.2020 constant-luminance Y'C'bcC'rc, but I
> > > > > wouldn't be surprised if it scrambled too.
> > > > >
> > > > > Of course, both of the above are not just one matrix. They require two
> > > > > matrices and the transfer characteristic each to compute. KMS color
> > > > > operations cannot implement those today, but with the colorop pipelines
> > > > > they will if the hardware does it.
> > > > >
> > > > > That's why I think it's important to document the assumption of Cb and
> > > > > Cr when not part of the pixel format, and not write down a specific
> > > > > expansion to RGB like (Y, Y, Y).
> > > >
> > > > Every time I discuss color spaces, the scopes of "RGB" and "YUV" seem to
> > > > expand more and more. This makes me wonder how we define those two
> > > > concepts. Taking the conversion from RGB to ICtCp as an example, would
> > > > you consider LMS and L'M'S' as "RGB" formats, and ICtCp as a "YUV"
> > > > format ?
> > >
> > > sorry for the confusion. In this specific context, my use of RGB and
> > > YUV refers to the channels in DRM pixel formats. It might have been
> > > better if all channels in all pixel formats were "anonymous" and merely
> > > numbered because all formats can be used for any color model, but this
> > > is what we have.
> > >
> > > There is some disambiguation in
> > > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
> > > The doc is some years old, so nowadays I might phrase things
> > > differently, but maybe it's easier to read for those new to things as I
> > > wrote it when I was just learning things.
> > >
> > > I would classify ICtCp in the YUV pixel format category, because the
> > > CtCp plane can be sub-sampled (right?). I would classify LMS and L'M'S'
> > > in the RGB pixel format category because they are not sub-sampled AFAIK
> > > although they also do not actually appear as buffer contents, so the
> > > relation to pixel formats is... theoretical.
> > >
> > > IOW, we have a completely artificial split of DRM pixel formats to RGB
> > > and YUV where the only essential difference is that YUV formats can have
> > > sub-sampled variants and RGB formats do not.
> >
> > RGB can be subsampled, too...
> > https://en.wikipedia.org/wiki/Bayer_filter
>
> That's true. What difference are we left with, then?

RGB contains three monochromatic color channels (which may differ from
Red, Green, and Blue, cfr. truncated RGn and Rn formats).
YUV contains one luminance and two chrominance channels.

> We have DRM pixel formats which imply some color model, but do not
> define the variant of each color model (e.g. the Y'CbCr-to-RGB matrix).
>
> I guess the implied color model then implies which API, e.g. KMS plane
> property, is responsible for defining the variant.

Probably (IANADX --- I Am Not A Drm eXpert ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Pekka Paalanen]

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

On Tue, 22 Apr 2025 12:12:21 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Pekka,
> 
> On Tue, 22 Apr 2025 at 12:01, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > On Tue, 22 Apr 2025 11:41:29 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > On Tue, 22 Apr 2025 at 11:11, Pekka Paalanen
> > > <pekka.paalanen@haloniitty.fi> wrote:  
> > > > On Mon, 21 Apr 2025 17:50:39 +0300
> > > > Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:  
> > > > > On Thu, Apr 17, 2025 at 11:13:15AM +0300, Pekka Paalanen wrote:  
> > > > > > On Wed, 16 Apr 2025 11:59:43 +0300 Tomi Valkeinen wrote:  
> > > > > > > On 01/04/2025 16:27, Pekka Paalanen wrote:  
> > > > > > > > On Mon, 31 Mar 2025 13:53:37 +0300 Pekka Paalanen wrote:  
> > > > > > > >> On Mon, 31 Mar 2025 11:21:35 +0300 Laurent Pinchart wrote:  
> > > > > > > >>> On Mon, Mar 31, 2025 at 10:54:46AM +0300, Pekka Paalanen wrote:  
> > > > > > > >>>> On Thu, 27 Mar 2025 17:35:39 +0100 Geert Uytterhoeven wrote:  
> > > > > > > >>>>> On Thu, 27 Mar 2025 at 16:59, Pekka Paalanen wrote:  
> > > > > > > >>>>>> On Thu, 27 Mar 2025 16:21:16 +0200 Tomi Valkeinen wrote:  
> > > > > > > >>>>>>> On 27/03/2025 11:20, Pekka Paalanen wrote:  
> > > > > > > >>>>>>>> On Wed, 26 Mar 2025 15:55:18 +0200 Tomi Valkeinen wrote:  
> > > > > > > >>>>>>>>> On 26/03/2025 15:52, Geert Uytterhoeven wrote:  
> > > > > > > >>>>>>>>>> On Wed, 26 Mar 2025 at 14:23, Tomi Valkeinen wrote:  
> > > > > > > >>>>>>>>>>> Add greyscale Y8 format.
> > > > > > > >>>>>>>>>>>
> > > > > > > >>>>>>>>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > > >>>>>>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>  
> > > > > > > >>>>>>>>>>
> > > > > > > >>>>>>>>>> Thanks for your patch!
> > > > > > > >>>>>>>>>>  
> > > > > > > >>>>>>>>>>> --- 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 */  
> > > > > > > >>>>>>>>>>
> > > > > > > >>>>>>>>>> This format differs from e.g. DRM_FORMAT_R8, which encodes
> > > > > > > >>>>>>>>>> the number of bits in the FOURCC format. What do you envision
> > > > > > > >>>>>>>>>> for e.g. DRM_FORMAT_Y16? fourcc_code('G', 'R', '1', '6')?  
> > > > > > > >>>>>>>>>
> > > > > > > >>>>>>>>> I wanted to use the same fourcc as on V4L2 side. Strictly speaking it's
> > > > > > > >>>>>>>>> not required, but different fourccs for the same formats do confuse.
> > > > > > > >>>>>>>>>
> > > > > > > >>>>>>>>> So, generally speaking, I'd pick an existing fourcc from v4l2 side if
> > > > > > > >>>>>>>>> possible, and if not, invent a new one.  
> > > > > > > >>>>>>>>
> > > > > > > >>>>>>>> what's the actual difference between DRM_FORMAT_R8 and DRM_FORMAT_Y8?
> > > > > > > >>>>>>>>
> > > > > > > >>>>>>>> Is the difference that when R8 gets expanded to RGB, it becomes (R, 0,
> > > > > > > >>>>>>>> 0), but Y8 gets expanded to (c1 * Y, c2 * Y, c3 * Y) where c1..c3 are
> > > > > > > >>>>>>>> defined by MatrixCoefficients (H.273 terminology)?
> > > > > > > >>>>>>>>
> > > > > > > >>>>>>>> That would be my intuitive assumption following how YCbCr is handled.
> > > > > > > >>>>>>>> Is it obvious enough, or should there be a comment to that effect?  
> > > > > > > >>>>>>>
> > > > > > > >>>>>>> You raise an interesting point. Is it defined how a display driver, that
> > > > > > > >>>>>>> supports R8 as a format, shows R8 on screen? I came into this in the
> > > > > > > >>>>>>> context of grayscale formats, so I thought R8 would be handled as (R, R,
> > > > > > > >>>>>>> R) in RGB. But you say (R, 0, 0), which... also makes sense.  
> > > > > > > >>>>>>
> > > > > > > >>>>>> That is a good question too. I based my assumption on OpenGL behavior
> > > > > > > >>>>>> of R8.
> > > > > > > >>>>>>
> > > > > > > >>>>>> Single channel displays do exist I believe, but being single-channel,
> > > > > > > >>>>>> expansion on the other channels is likely meaningless. Hm, but for the
> > > > > > > >>>>>> KMS color pipeline, it would be meaningful, like with a CTM.
> > > > > > > >>>>>> Interesting.
> > > > > > > >>>>>>
> > > > > > > >>>>>> I don't know. Maybe Geert does?  
> > > > > > > >>>>>
> > > > > > > >>>>> I did some digging, and was a bit surprised that it was you who told
> > > > > > > >>>>> me to use R8 instead of Y8?
> > > > > > > >>>>> https://lore.kernel.org/all/20220202111954.6ee9a10c@eldfell  
> > > > > > > >>>>
> > > > > > > >>>> Hi Geert,
> > > > > > > >>>>
> > > > > > > >>>> indeed I did. I never thought of the question of expansion to R,G,B
> > > > > > > >>>> before. Maybe that expansion is what spells R8 and Y8 apart?
> > > > > > > >>>>
> > > > > > > >>>> I do think that expansion needs to be specified, so that the KMS color
> > > > > > > >>>> pipeline computations are defined. There is a big difference between
> > > > > > > >>>> multiplying these with an arbitrary 3x3 matrix (e.g. CTM):
> > > > > > > >>>>
> > > > > > > >>>> - (R, 0, 0)
> > > > > > > >>>> - (R, R, R)
> > > > > > > >>>> - (c1 * Y, c2 * Y, c3 * Y)  
> > > > > > > >>>
> > > > > > > >>> I'd be very surprised by an YUV to RGB conversion matrix where the first
> > > > > > > >>> column would contain different values. What we need to take into account
> > > > > > > >>> though is quantization (full vs. limited range).  
> > > > > > > >
> > > > > > > > Quantization range is indeed good to note. R8 would be always full
> > > > > > > > range, but Y8 would follow COLOR_RANGE property.
> > > > > > > >  
> > > > > > > >> That makes Y8 produce (Y, Y, Y), and we have our answer: R8 should be
> > > > > > > >> (R, 0, 0), so we have both variants.
> > > > > > > >>
> > > > > > > >> Can we specify Y, R, G and B be nominal values in the range 0.0 - 1.0
> > > > > > > >> in the KMS color processing?  
> > > > > > > >
> > > > > > > > I think this 0.0 - 1.0 nominal range definition for the abstract KMS
> > > > > > > > color processing is necessary.
> > > > > > > >
> > > > > > > > It also means that limited range Y8 data, when containing values 0-15
> > > > > > > > or 240-255, would produce negative and greater than 1.0 values,
> > > > > > > > respectively. They might get immediately clamped to 0.0 - 1.0 with the
> > > > > > > > first color operation they face, though, but the concept seems
> > > > > > > > important and carrying over to the new color pipelines UAPI which might
> > > > > > > > choose not to clamp.  
> > > > > > >
> > > > > > > Is the behavior of values outside the limited range something that needs
> > > > > > > to be defined? We can't know how each piece of HW behaves with
> > > > > > > "undefined" input, so should we not just define the behavior as platform
> > > > > > > specific?  
> > > > > >
> > > > > > Hi Tomi,
> > > > > >
> > > > > > it's not undefined nor illegal input in general. The so-called
> > > > > > sub-black and super-white ranges exist for a reason, and they are
> > > > > > intended to be used in video processing to avoid clipping in
> > > > > > intermediate processing steps when a filter overshoots a bit. There are
> > > > > > also practices that depend on them, like PLUGE calibration with
> > > > > > traditional signals on a display: https://www.itu.int/rec/R-REC-BT.814
> > > > > >
> > > > > > I think it would be really good to have defined behaviour if at all
> > > > > > possible.
> > > > > >  
> > > > > > > In any case: I can't say I fully understood all the discussions wrt.
> > > > > > > color spaces. But my immediate interest is, of course, this series =).
> > > > > > > So is there something that you think should be improved here?  
> > > > > >
> > > > > > Right, the range discussion is a tangent and applies to all YUV
> > > > > > formats, so it's not a new question.
> > > > > >  
> > > > > > > My understanding is that the Y-only pixel formats behave in a well
> > > > > > > defined way (or, as well defined as the YUV formats), and there's
> > > > > > > nothing more to add here. Is that right?  
> > > > > >
> > > > > > There are two things:
> > > > > >
> > > > > > - Y8 follows COLOR_RANGE property, just like all other YUV formats.
> > > > > > - Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).
> > > > > >
> > > > > > I'd like these explicitly written down, so that they become obvious to
> > > > > > everyone. I suspect either one might be easy to forget when writing
> > > > > > code and taking shortcuts without thinking.
> > > > > >
> > > > > >
> > > > > > Laurent,
> > > > > >
> > > > > > I did find a case where (Y', neutral, neutral) does *not* seem to expand
> > > > > > to RGB=(Y, Y, Y): ICtCp. The conversion from ICtCp to L'M'S' does
> > > > > > produce (Y', Y', Y'), but the LMS-to-RGB matrix scrambles it.
> > > > > >
> > > > > > I didn't dig through BT.2020 constant-luminance Y'C'bcC'rc, but I
> > > > > > wouldn't be surprised if it scrambled too.
> > > > > >
> > > > > > Of course, both of the above are not just one matrix. They require two
> > > > > > matrices and the transfer characteristic each to compute. KMS color
> > > > > > operations cannot implement those today, but with the colorop pipelines
> > > > > > they will if the hardware does it.
> > > > > >
> > > > > > That's why I think it's important to document the assumption of Cb and
> > > > > > Cr when not part of the pixel format, and not write down a specific
> > > > > > expansion to RGB like (Y, Y, Y).  
> > > > >
> > > > > Every time I discuss color spaces, the scopes of "RGB" and "YUV" seem to
> > > > > expand more and more. This makes me wonder how we define those two
> > > > > concepts. Taking the conversion from RGB to ICtCp as an example, would
> > > > > you consider LMS and L'M'S' as "RGB" formats, and ICtCp as a "YUV"
> > > > > format ?  
> > > >
> > > > sorry for the confusion. In this specific context, my use of RGB and
> > > > YUV refers to the channels in DRM pixel formats. It might have been
> > > > better if all channels in all pixel formats were "anonymous" and merely
> > > > numbered because all formats can be used for any color model, but this
> > > > is what we have.
> > > >
> > > > There is some disambiguation in
> > > > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
> > > > The doc is some years old, so nowadays I might phrase things
> > > > differently, but maybe it's easier to read for those new to things as I
> > > > wrote it when I was just learning things.
> > > >
> > > > I would classify ICtCp in the YUV pixel format category, because the
> > > > CtCp plane can be sub-sampled (right?). I would classify LMS and L'M'S'
> > > > in the RGB pixel format category because they are not sub-sampled AFAIK
> > > > although they also do not actually appear as buffer contents, so the
> > > > relation to pixel formats is... theoretical.
> > > >
> > > > IOW, we have a completely artificial split of DRM pixel formats to RGB
> > > > and YUV where the only essential difference is that YUV formats can have
> > > > sub-sampled variants and RGB formats do not.  
> > >
> > > RGB can be subsampled, too...
> > > https://en.wikipedia.org/wiki/Bayer_filter  
> >
> > That's true. What difference are we left with, then?  
> 
> RGB contains three monochromatic color channels (which may differ from
> Red, Green, and Blue, cfr. truncated RGn and Rn formats).
> YUV contains one luminance and two chrominance channels.

I think I get what you mean. RGB are directly related to the power of
each of the primary emitters in a display. YUV OTOH carries the overall
luma and the difference from neutral (chroma) signals. This difference
is the difference between color models.

Monochromatic is not the right word here, unless you explicitly refer
to the primaries of BT.2020 which are laser-like. Most display
primaries are far from monochromatic. The closer to monochromatic
display primaries get, the more pronounced the differences between
color vision in people become, even if the people were all classified
as having normal color vision.


Thanks,
pq

> > We have DRM pixel formats which imply some color model, but do not
> > define the variant of each color model (e.g. the Y'CbCr-to-RGB matrix).
> >
> > I guess the implied color model then implies which API, e.g. KMS plane
> > property, is responsible for defining the variant.  
> 
> Probably (IANADX --- I Am Not A Drm eXpert ;-)
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 


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

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

[Tomi Valkeinen]

Hi Pekka,

On 17/04/2025 11:13, Pekka Paalanen wrote:
>> My understanding is that the Y-only pixel formats behave in a well
>> defined way (or, as well defined as the YUV formats), and there's
>> nothing more to add here. Is that right?
> 
> There are two things:
> 
> - Y8 follows COLOR_RANGE property, just like all other YUV formats.
> - Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).
> 
> I'd like these explicitly written down, so that they become obvious to
> everyone. I suspect either one might be easy to forget when writing
> code and taking shortcuts without thinking.

I didn't find a suitable place in the docs for this, but would this, in 
the drm_fourcc.h, be enough:

/*
  * Y-only (greyscale) formats
  *
  * The Y-only formats are handled similarly to the YCbCr formats in the 
display
  * pipeline, with the Cb and Cr implicitly neutral (0.0 in nominal 
values). This
  * also means that COLOR_RANGE property applies to the Y-only 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 */

  Tomi

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

[Pekka Paalanen]

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

On Fri, 25 Apr 2025 11:38:28 +0300
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> Hi Pekka,
> 
> On 17/04/2025 11:13, Pekka Paalanen wrote:
> >> My understanding is that the Y-only pixel formats behave in a well
> >> defined way (or, as well defined as the YUV formats), and there's
> >> nothing more to add here. Is that right?  
> > 
> > There are two things:
> > 
> > - Y8 follows COLOR_RANGE property, just like all other YUV formats.
> > - Y8 implies that Cb and Cr are both neutral (0.0 in nominal values).
> > 
> > I'd like these explicitly written down, so that they become obvious to
> > everyone. I suspect either one might be easy to forget when writing
> > code and taking shortcuts without thinking.  
> 
> I didn't find a suitable place in the docs for this, but would this, in 
> the drm_fourcc.h, be enough:
> 
> /*
>   * Y-only (greyscale) formats
>   *
>   * The Y-only formats are handled similarly to the YCbCr formats in the 
> display
>   * pipeline, with the Cb and Cr implicitly neutral (0.0 in nominal 
> values). This
>   * also means that COLOR_RANGE property applies to the Y-only 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 */
> 

Hi Tomi,

I would be very happy with that.


Thanks,
pq

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

[PATCH v4 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  | 3 +++
 include/uapi/drm/drm_fourcc.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 355aaf7b5e9e..e5f04f7ec164 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -353,6 +353,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		{ .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 },
+		{ .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 751b8087b35f..f6316adbeb97 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

Re: [PATCH v4 04/11] drm/fourcc: Add DRM_FORMAT_Y10_P32

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Mar 26, 2025 at 03:22:47PM +0200, Tomi Valkeinen wrote:
> 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>

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

> ---
>  drivers/gpu/drm/drm_fourcc.c  | 3 +++
>  include/uapi/drm/drm_fourcc.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 355aaf7b5e9e..e5f04f7ec164 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -353,6 +353,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
>  		{ .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 },
> +		{ .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 751b8087b35f..f6316adbeb97 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:

-- 
Regards,

Laurent Pinchart

[PATCH v4 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  | 3 +++
 include/uapi/drm/drm_fourcc.h | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index e5f04f7ec164..60684f99f4a7 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -356,6 +356,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		{ .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 },
+		{ .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 f6316adbeb97..10d77f6f6e95 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

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

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Mar 26, 2025 at 03:22:48PM +0200, Tomi Valkeinen wrote:
> Add X403, a 3 plane non-subsampled YCbCr format.

I'd add 10-bpp somewhere in there.

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 3 +++
>  include/uapi/drm/drm_fourcc.h | 8 ++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index e5f04f7ec164..60684f99f4a7 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -356,6 +356,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
>  		{ .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 },
> +		{ .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 f6316adbeb97..10d77f6f6e95 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

/*
 * 3 plane non-subsampled (444) YCbCr

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

> + * 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

-- 
Regards,

Laurent Pinchart

[PATCH v4 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 60684f99f4a7..81e5fcdcc234 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 10d77f6f6e95..552438128f51 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

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

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Mar 26, 2025 at 03:22:49PM +0200, Tomi Valkeinen wrote:
> Add XVUY2101010, a 10 bits per component YCbCr format in a 32 bit
> container.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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 60684f99f4a7..81e5fcdcc234 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 10d77f6f6e95..552438128f51 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

-- 
Regards,

Laurent Pinchart

[PATCH v4 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).

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 80d1e499a18d..b9883ea2d03e 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1116,16 +1116,19 @@ 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;
 		dma_addr_t dma_addr;
+		unsigned int width;
+		unsigned int height;
+
+		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 v4 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 b9883ea2d03e..1dc77f2e4262 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

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

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Mar 26, 2025 at 03:22:51PM +0200, Tomi Valkeinen wrote:
> Add support for XV15 & XV20 formats.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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 b9883ea2d03e..1dc77f2e4262 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,
>  	},
>  };
>  

-- 
Regards,

Laurent Pinchart

[PATCH v4 09/11] drm: xlnx: zynqmp: Add support for Y8 and Y10_P32

[Tomi Valkeinen]

Add support for Y8 and Y10_P32 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 1dc77f2e4262..ae8b4073edf6 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

Re: [PATCH v4 09/11] drm: xlnx: zynqmp: Add support for Y8 and Y10_P32

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Mar 26, 2025 at 03:22:52PM +0200, Tomi Valkeinen wrote:
> Add support for Y8 and Y10_P32 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 1dc77f2e4262..ae8b4073edf6 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,

Assuming the DRM format definitions get approved, this looks good to me.

>  	},
>  };
>  
> @@ -697,6 +707,16 @@ static const u32 csc_sdtv_to_rgb_offsets[] = {
>  	0x0, 0x1800, 0x1800
>  };
>  
> +static const u16 csc_sdtv_to_rgb_yonly_matrix[] = {

TODO: Add support for colorspaces to the driver.

> +	0x0, 0x0, 0x1000,
> +	0x0, 0x0, 0x1000,
> +	0x0, 0x0, 0x1000,

This surprises me a bit, I was expecting 0x1000 to be in the first
column. What am I missing ?

> +};
> +
> +static const u32 csc_sdtv_to_rgb_yonly_offsets[] = {
> +	0x1800, 0x1800, 0x0

Why do you need offsets ? Those values correspond to -128 in a 8-bit
range, and that's what would need to be applied to the chroma values.
There's no chroma here. I think you could use csc_zero_offsets.

> +};
> +
>  /**
>   * 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 {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 09/11] drm: xlnx: zynqmp: Add support for Y8 and Y10_P32

[Tomi Valkeinen]

Hi,

On 28/03/2025 00:52, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Wed, Mar 26, 2025 at 03:22:52PM +0200, Tomi Valkeinen wrote:
>> Add support for Y8 and Y10_P32 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 1dc77f2e4262..ae8b4073edf6 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,
> 
> Assuming the DRM format definitions get approved, this looks good to me.
> 
>>   	},
>>   };
>>   
>> @@ -697,6 +707,16 @@ static const u32 csc_sdtv_to_rgb_offsets[] = {
>>   	0x0, 0x1800, 0x1800
>>   };
>>   
>> +static const u16 csc_sdtv_to_rgb_yonly_matrix[] = {
> 
> TODO: Add support for colorspaces to the driver.
> 
>> +	0x0, 0x0, 0x1000,
>> +	0x0, 0x0, 0x1000,
>> +	0x0, 0x0, 0x1000,
> 
> This surprises me a bit, I was expecting 0x1000 to be in the first
> column. What am I missing ?

All this is undocumented (afaics). But my understanding is that as this 
is a single channel format, the Y data is in the "lowest" channel, which 
is handled by the rightmost column in the matrix.

>> +};
>> +
>> +static const u32 csc_sdtv_to_rgb_yonly_offsets[] = {
>> +	0x1800, 0x1800, 0x0
> 
> Why do you need offsets ? Those values correspond to -128 in a 8-bit
> range, and that's what would need to be applied to the chroma values.
> There's no chroma here. I think you could use csc_zero_offsets.

Indeed, the values are not needed. The only value in the offsets that 
matters is the last one (matching the rightmost column in the matrix), 
which needs to be 0. But I think it makes sense to have the array here, 
otherwise one needs to dig into the code to see what are the offsets for 
y-only.

  Tomi

[PATCH v4 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 ae8b4073edf6..ce685dfbf31f 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

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

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Mar 26, 2025 at 03:22:53PM +0200, Tomi Valkeinen wrote:
> Add support for X403 format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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 ae8b4073edf6..ce685dfbf31f 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,
>  	},
>  };
>  

-- 
Regards,

Laurent Pinchart

[PATCH v4 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 ce685dfbf31f..79f58e06f38f 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 v4 11/11] drm: xlnx: zynqmp: Add support for XVUY2101010

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Mar 26, 2025 at 03:22:54PM +0200, Tomi Valkeinen wrote:
> 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 ce685dfbf31f..79f58e06f38f 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,

I'll have to trust your word on this, the documentation is just too
wrong in too many places to trust it. Assuming you've tested this
format,

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

>  	},
>  };
>  

-- 
Regards,

Laurent Pinchart