Re: [PATCH] drm/panfrost: Give name to anonymous coredump object union

[Alyssa Rosenzweig <alyssa@collabora.com>]

On Tue, Sep 20, 2022 at 02:26:52PM +0100, Steven Price wrote:
> On 19/09/2022 07:44, Adri??n Larumbe wrote:
> > Hi Steven,
> > 
> > On 13.09.2022 09:45, Steven Price wrote:
> >> On 12/09/2022 17:44, Adri??n Larumbe wrote:
> >>> Building Mesa's Perfetto requires including the panfrost drm uAPI header in
> >>> C++ code, but the C++ compiler requires anonymous unions to have only
> >>> public non-static data members.
> >>>
> >>> Commit 730c2bf4ad39 ("drm/panfrost: Add support for devcoredump")
> >>> introduces one such union, breaking the Mesa build.
> >>>
> >>> Give it a name, and also rename pan_reg_hdr structure because it will
> >>> always be prefixed by the union name.
> >>>
> >>> Bug: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7195
> >>>
> >>> Signed-off-by: Adri??n Larumbe <adrian.larumbe@collabora.com>
> > 
> >> Ouch! It's frustrating how C++ isn't quite a superset of C. However I
> >> think we can solve this with a simpler patch, I'd appreciate testing
> >> that this does indeed fix the build issues with Mesa with all supported
> >> compilers (I'm not so familiar with C++):
> > 
> > I just tested your changes on Mesa and they do fix the build.
> 
> Thanks Adri??n!
> 
> Alyssa: Could you give your R-b if you're happy with this change? It
> would be good to get this fixed before it hits -rc1.

R-b, however the issue isn't totally gone: in a separate but related
issue, apparently the __le types aren't portable and the devcoredump
support has now broken the panfrost (mesa) build for FreeBSD, which has
a UAPI-compatible reimplementation of panfrost.ko ...

Do we maybe want to change all the __le to u at the same time? If we
have to break UAPI, better do it before the UAPI is actually merged.
Panfrost is probably broken in far worse ways on big endian anyway. Or
maybe we want to keep doing little-endian but in u32 containers and have
conversions in the kernel for big-endian CPUs. Or maybe we want to just
"we don't care about big endian, because you'll have worse problems", at
a GPU level Mali hasn't supported big endian since Midgard so I doubt
the recent DDKs would work on BE either.

Anyway, ideally we'd solve both at once, and soon, so we don't have to
revert the devcoredump stuff from mesa.

Thanks,

Alyssa