Re: [PATCH v4 5/5] drm/ofdrm: Support big-endian scanout buffers

From: Thomas Zimmermann <tzimmermann@suse.de>
To: Arnd Bergmann <arnd@arndb.de>,
	Javier Martinez Canillas <javierm@redhat.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Helge Deller <deller@gmx.de>,
	maxime@cerno.tech, sam@ravnborg.org,
	Michal Suchanek <msuchanek@suse.de>,
	Michael Ellerman <mpe@ellerman.id.au>,
	benh@kernel.crashing.org, Paul Mackerras <paulus@samba.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	mark.cave-ayland@ilande.co.uk
Cc: linux-fbdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4 5/5] drm/ofdrm: Support big-endian scanout buffers
Date: Wed, 12 Oct 2022 08:46:07 +0200	[thread overview]
Message-ID: <fda959d7-1bae-716f-f01b-66d9db9096e0@suse.de> (raw)
In-Reply-To: <9162f41f-28c3-493c-ab54-b1c4a2fdf494@app.fastmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1729 bytes --]

Hi

Am 11.10.22 um 22:06 schrieb Arnd Bergmann:
> On Tue, Oct 11, 2022, at 1:30 PM, Thomas Zimmermann wrote:
>> Am 11.10.22 um 09:46 schrieb Javier Martinez Canillas:
>>>> +static bool display_get_big_endian_of(struct drm_device *dev, struct device_node *of_node)
>>>> +{
>>>> +	bool big_endian;
>>>> +
>>>> +#ifdef __BIG_ENDIAN
>>>> +	big_endian = true;
>>>> +	if (of_get_property(of_node, "little-endian", NULL))
>>>> +		big_endian = false;
>>>> +#else
>>>> +	big_endian = false;
>>>> +	if (of_get_property(of_node, "big-endian", NULL))
>>>> +		big_endian = true;
>>>> +#endif
>>>> +
>>>> +	return big_endian;
>>>> +}
>>>> +
>>>
>>> Ah, I see. The heuristic then is whether the build is BE or LE or if the Device
>>> Tree has an explicit node defining the endianess. The patch looks good to me:
>>
>> Yes. I took this test from offb.
> 
> Has the driver been tested with little-endian kernels though? While
> ppc32 kernels are always BE, you can build kernels as either big-endian
> or little-endian for most (modern) powerpc64 and arm/arm64 hardware,
> and I don't see why that should change the defaults of the driver
> when describing the same framebuffer hardware.

Yes, I tested this on qemu's ppc64le and ppc64.

Best regards
Thomas

> 
> I could understand having a default to e.g. big-endian on all powerpc and
> a default for little-endian on all arm, but having it tied to the
> way the kernel is built seems wrong, and doesn't make sense in a
> DT binding either.
> 
>        Arnd

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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