diff mbox series

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

Message ID 20250326-xilinx-formats-v4-3-322a300c6d72@ideasonboard.com (mailing list archive)
State New
Headers show
Series drm: Add new pixel formats for Xilinx Zynqmp | expand

Commit Message

Tomi Valkeinen March 26, 2025, 1:22 p.m. UTC
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(+)

Comments

Geert Uytterhoeven March 26, 2025, 1:52 p.m. UTC | #1
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
Tomi Valkeinen March 26, 2025, 1:55 p.m. UTC | #2
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
Pekka Paalanen March 27, 2025, 9:20 a.m. UTC | #3
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
Tomi Valkeinen March 27, 2025, 2:21 p.m. UTC | #4
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
Pekka Paalanen March 27, 2025, 3:58 p.m. UTC | #5
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
Geert Uytterhoeven March 27, 2025, 4:35 p.m. UTC | #6
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
diff mbox series

Patch

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: