linux-kernel - Re: [PATCH v4 1/2] drm/format-helper: Add conversion from XRGB8888 to BGR888

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <466c38c3-7f74-46db-8270-bebafacf0007@suse.de>
Date: Tue, 25 Feb 2025 08:37:32 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>,
 Aditya Garg <gargaditya08@...e.com>
Cc: "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
 "mripard@...nel.org" <mripard@...nel.org>,
 "airlied@...il.com" <airlied@...il.com>, "simona@...ll.ch"
 <simona@...ll.ch>, Kerem Karabay <kekrby@...il.com>,
 Atharva Tiwari <evepolonium@...il.com>, Aun-Ali Zaidi <admin@...eit.net>,
 Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
 "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v4 1/2] drm/format-helper: Add conversion from XRGB8888 to
 BGR888

Hi

Am 24.02.25 um 15:29 schrieb andriy.shevchenko@...ux.intel.com:
> On Mon, Feb 24, 2025 at 01:38:32PM +0000, Aditya Garg wrote:
>> From: Kerem Karabay <kekrby@...il.com>
>>
>> Add XRGB8888 emulation helper for devices that only support BGR888.
> ...
>
>> +static void drm_fb_xrgb8888_to_bgr888_line(void *dbuf, const void *sbuf, unsigned int pixels)
> Okay the xrgb8888 is the actual pixel format independently on
> the CPU endianess.
>
>> +{
>> +	u8 *dbuf8 = dbuf;
>> +	const __le32 *sbuf32 = sbuf;
> But here we assume that sbuf is __le32.
> And I think we may benefit from the __be32 there.

No, please. XRGB is the logical order. The raw physical byte order for 
DRM formats is always* little endian, hence reversed from the logical 
one. sbuf points to raw memory and is therefore __le32. DRM-format byte 
order is impossible to understand, I know. But that code is correct.

Best regards
Thomas

*) White lie: there's a DRM format flag signalling physical big 
endianess. That isn't the case here. So nothing here should ever 
indicate big endianess.


>
>> +	unsigned int x;
>> +	u32 pix;
>> +
>> +	for (x = 0; x < pixels; x++) {
>> +		pix = le32_to_cpu(sbuf32[x]);
>> +		/* write red-green-blue to output in little endianness */
>> +		*dbuf8++ = (pix & 0x00ff0000) >> 16;
>> +		*dbuf8++ = (pix & 0x0000ff00) >> 8;
>> +		*dbuf8++ = (pix & 0x000000ff) >> 0;
> 		pix = be32_to_cpu(sbuf[4 * x]) >> 8;
> 		put_unaligned_le24(pix, &dbuf[3 * x]);
>
>> +	}
> Or, after all, this __le32 magic might be not needed at all. Wouldn't the below
> be the equivalent
>
> static void drm_fb_xrgb8888_to_bgr888_line(void *dbuf, const void *sbuf, unsigned int pixels)
> {
> 	unsigned int x;
> 	u32 pix;
>
> 	for (x = 0; x < pixels; x++) {
> 		/* Read red-green-blue from input in big endianess and... */
> 		pix = get_unaligned_be24(sbuf + x * 4 + 1);
> 		/* ...write it to output in little endianness. */
> 		put_unaligned_le24(pix, dbuf + x * 3);
> 	}
> }
>
> The comments can even be dropped as the code quite clear about what's going on.
>
>> +}
> But it's up to you. I don't know which solution gives better code generation
> either.
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)