Re: [PATCH] drm: fourcc byteorder: brings header file comments in line with reality.
From: Ilia Mirkin
Date: Sat Apr 22 2017 - 01:08:15 EST
On Fri, Apr 21, 2017 at 12:59 PM, Ville SyrjÃlÃ
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Fri, Apr 21, 2017 at 10:49:49AM -0400, Ilia Mirkin wrote:
>> On Fri, Apr 21, 2017 at 3:58 AM, Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:
>> > While working on graphics support for virtual machines on ppc64 (which
>> > exists in both little and big endian variants) I've figured the comments
>> > for various drm fourcc formats in the header file don't match reality.
>> >
>> > Comments says the RGB formats are little endian, but in practice they
>> > are native endian. Look at the drm_mode_legacy_fb_format() helper. It
>> > maps -- for example -- bpp/depth 32/24 to DRM_FORMAT_XRGB8888, no matter
>> > whenever the machine is little endian or big endian. The users of this
>> > function (fbdev emulation, DRM_IOCTL_MODE_ADDFB) expect the framebuffer
>> > is native endian, not little endian. Most userspace also operates on
>> > native endian only.
>> >
>> > So, go update the comments for all 16+24+32 bpp RGB formats.
>> >
>> > Leaving the yuv formats as-is. I have no idea if and how those are used
>> > on bigendian machines.
>>
>> I think this is premature. The current situation is that I can't get
>> modetest to work *at all* on my NV34 / BE setup (I mean, it runs, just
>> the colors displayed are wrong). I believe that currently it packs
>> things in "cpu native endian". I've tried futzing with that without
>> much success, although I didn't spend too much time on it. I have a
>> NV34 plugged into my LE setup as well although I haven't tested to
>> double-check that it all works there. However I'm quite sure it used
>> to, as I used modetest to help develop the YUV overlay support for
>> those GPUs.
>
> I just took a quick stab at fixing modetest to respect the current
> wording in drm_fourcc.h:
>
> git://github.com/vsyrjala/libdrm.git modetest_endian
Looks like there was some careless testing on my part :( So ... it
looks like the current modetest without those changes does, in fact,
work on NV34/BE. With the changes, it breaks (and the handling of the
b* modes is a little broken in those patches -- they're not selectable
from the cmdline.) Which means that, as Michel & co predicted, it
appears to be taking BE input not LE input. This is very surprising to
me, but it is what it is. As I mentioned before, the details of how
the "BE" mode works on the GPUs is largely unknown to us beyond a few
basics. Note that only XR24 works, AR24 ends up with all black
displayed. This also happens on LE.
Furthermore, all of YUYV, UYVY, and NV12 plane overlays don't appear
to work properly. The first half of the overlay is OK (but I think
compressed), while the second half is gibberish. Testing this on my
board plugged into a LE CPU, I also get the same type of issue, so I'm
guessing that it's just bitrot of the feature. (Or modetest gained a
bug.)
Cheers,
-ilia