linux-kernel - Re: [PATCH v2 04/10] drm/fourcc: Add DRM_FORMAT_Y10

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0b0decd1-6f19-424a-84e5-fc71dceb983c@ideasonboard.com>
Date: Wed, 15 Jan 2025 15:46:11 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Vishal Sagar <vishal.sagar@....com>,
 Anatoliy Klymenko <anatoliy.klymenko@....com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Michal Simek <michal.simek@....com>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 04/10] drm/fourcc: Add DRM_FORMAT_Y10_LE32

Hi,

On 15/01/2025 14:52, Geert Uytterhoeven wrote:
> Hi Tomi,
> 
> On Wed, Jan 15, 2025 at 1:42 PM Tomi Valkeinen
> <tomi.valkeinen@...asonboard.com> wrote:
>> On 15/01/2025 14:33, Geert Uytterhoeven wrote:
>>> On Wed, Jan 15, 2025 at 12:11 PM Tomi Valkeinen
>>> <tomi.valkeinen@...asonboard.com> wrote:
>>>> On 15/01/2025 12:33, Geert Uytterhoeven wrote:
>>>>> On Wed, Jan 15, 2025 at 10:04 AM Tomi Valkeinen
>>>>> <tomi.valkeinen@...asonboard.com> wrote:
>>>>>> Add Y10_LE32, a 10 bit greyscale format, with 3 pixels packed into
>>>>>> 32-bit container.
>>>>>>
>>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- 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_LE32    fourcc_code('Y', 'P', 'A', '4')  /* [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian */
>>>>>
>>>>> R10_LE32? Or R10_PA4?
>>>>
>>>> Can we discuss the "R" vs "Y" question under the cover letter? There's
>>>> some more context about it in there.
>>>
>>> Sorry, hadn't read the cover letter. I got attracted by "Y8" and "Y10".
>>>
>>>> I took the "LE32" from Gstreamer's format. Maybe it's a bit pointless.
>>>>
>>>> I don't know if it makes sense to add the fourcc to the DRM format name.
>>>> The fourcc is very limited. Rather, we could, say, have
>>>> DRM_FORMAT_Y10_PACKED_32 (or "R", if you insist =).
>>>>
>>>>> Does LE32 have a meaning?  My first guess just reading the subject
>>>>> was wrong ("little endian  32-bit" ;-)
>>>>
>>>> I'm not sure I follow. It's little-endian. The pixel group/unit is a
>>>> 32-bit number, where the leftmost pixel on the screen is in bits 9-0,
>>>> and the padding is in bits 31-30, and stored in memory as little-endian.
>>>
>>> Ah, the "LE" applies to the pixels inside each word.
>>
>> No, to the 32-bit container.
>>
>>> DRM formats stored in memory are always little-endian, unless the
>>> DRM_FORMAT_BIG_ENDIAN bit is set, which is what I was hinting
>>> at...
>>
>> Indeed you're right. The "LE" is pointless. So back to the bike-shedding
>> about the name =).
> 
> As the order inside the container is Y2:Y1:Y0, it _is_ little endian.
> Cfr.
> 
> #define DRM_FORMAT_YUYV  fourcc_code('Y', 'U', 'Y', 'V') /* [31:0]
> Cr0:Y1:Cb0:Y0 8:8:8:8 little endian */

Hmm, I see. I hadn't thought LE in that context, but I think it makes 
sense when there are multiple pixels in one container. So the "little 
endian" above would refer to the order of Y1 and Y0. So is Y1 the 
least-significant-pixel? =)

But, say, in

#define DRM_FORMAT_RG88		fourcc_code('R', 'G', '8', '8') /* [15:0] R:G 
8:8 little endian */

the "little endian" refers to the 16-bit value itself? Which is not 
necessary, as the default assumption is little endian.

In any case, when considering your latest point... "LE" in the name 
makes sense? But with a quick look I didn't find any formats that would 
have "big endian pixel order", so maybe we can just assume little endian 
pixel order too.

  Tomi