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

From: "Michal Suchánek" <msuchanek@suse.de>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Zimmermann <tzimmermann@suse.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,
	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, 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: Tue, 11 Oct 2022 23:38:41 +0200	[thread overview]
Message-ID: <20221011213841.GA28810@kitsune.suse.cz> (raw)
In-Reply-To: <9162f41f-28c3-493c-ab54-b1c4a2fdf494@app.fastmail.com>

On Tue, Oct 11, 2022 at 10:06:59PM +0200, Arnd Bergmann wrote:
> 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.

The original code was added with
commit 7f29b87a7779 ("powerpc: offb: add support for foreign endianness")

The hardware is either big-endian or runtime-switchable-endian. It makes
sense to assume big-endian when runnig big-endian and the DT does not
specify endian which is likely on a historical system.

It also makes sense to assume that on system with
runtime-switchable-endian the DT specifies the framebuffer endian.

If systems that only do little-endian exist or emerge later then it also
makes sense to assume that the framebuffer matches the host if not
specified.

I don't really see a problem here.

BTW is this used on arm and on what platform?

I do not see any bindings in dts.

Thanks

Michal