Re: [PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Christian KÃnig]

Adding Harry to this mail thread as well, cause is one of the people 
really affected by this.

Christian.

Am 21.04.2017 um 15:21 schrieb Christian KÃnig:
> Am 21.04.2017 um 15:12 schrieb Gerd Hoffmann:
> >    Hi,
> >
> > > > "native" to me feels more like "native to the GPU" since these things
> > > > really are tied to the GPU not the CPU. That's also why I went with 
> > > > the
> > > > explicit endianness originally so that the driver could properly 
> > > > declare
> > > > what the GPU supports.
> > > And to be honest I would really prefer to stick with that approach for
> > > exactly that reason.
> > >
> > > The proposed change would require that drivers have different code path
> > > for different CPU byte order. Those code path tend to be not tested 
> > > very
> > > well and are additional complexity we probably don't want inside the 
> > > driver.
> > We can add fixed-endian #defines without too much effort, at least for
> > the 8 bits per color formats.  In qemu we have the same problem, only
> > with pixman.  Those formats are native endian too, but often we have to
> > handle a fixed format, so we did this:
> >
> > /*
> >   * pixman image formats are defined to be native endian,
> >   * that means host byte order on qemu.  So we go define
> >   * fixed formats here for cases where it is needed, like
> >   * feeding libjpeg / libpng and writing screenshots.
> >   */
> >
> > #ifdef HOST_WORDS_BIGENDIAN
> > # define PIXMAN_BE_r8g8b8     PIXMAN_r8g8b8
> > # define PIXMAN_BE_x8r8g8b8   PIXMAN_x8r8g8b8
> > # define PIXMAN_BE_a8r8g8b8   PIXMAN_a8r8g8b8
> > # define PIXMAN_BE_b8g8r8x8   PIXMAN_b8g8r8x8
> > # define PIXMAN_BE_b8g8r8a8   PIXMAN_b8g8r8a8
> > # define PIXMAN_BE_r8g8b8x8   PIXMAN_r8g8b8x8
> > # define PIXMAN_BE_r8g8b8a8   PIXMAN_r8g8b8a8
> > # define PIXMAN_BE_x8b8g8r8   PIXMAN_x8b8g8r8
> > # define PIXMAN_BE_a8b8g8r8   PIXMAN_a8b8g8r8
> > # define PIXMAN_LE_x8r8g8b8   PIXMAN_b8g8r8x8
> > #else
> > # define PIXMAN_BE_r8g8b8     PIXMAN_b8g8r8
> > # define PIXMAN_BE_x8r8g8b8   PIXMAN_b8g8r8x8
> > # define PIXMAN_BE_a8r8g8b8   PIXMAN_b8g8r8a8
> > # define PIXMAN_BE_b8g8r8x8   PIXMAN_x8r8g8b8
> > # define PIXMAN_BE_b8g8r8a8   PIXMAN_a8r8g8b8
> > # define PIXMAN_BE_r8g8b8x8   PIXMAN_x8b8g8r8
> > # define PIXMAN_BE_r8g8b8a8   PIXMAN_a8b8g8r8
> > # define PIXMAN_BE_x8b8g8r8   PIXMAN_r8g8b8x8
> > # define PIXMAN_BE_a8b8g8r8   PIXMAN_r8g8b8a8
> > # define PIXMAN_LE_x8r8g8b8   PIXMAN_x8r8g8b8
> > #endif
>
> Exactly what Mesa did as well.
>
> > > My personal opinion is that formats in drm_fourcc.h should be
> > > independent of the CPU byte order and the function
> > > drm_mode_legacy_fb_format() and drivers depending on that incorrect
> > > assumption be fixed instead.
> > The problem is this isn't a kernel-internal thing any more. With the
> > addition of the ADDFB2 ioctl the fourcc codes became part of the
> > kernel/userspace abi ...
>
> I know and that's exactly the reason I'm going to object those changes.
>
> The kernel/userspace abi is fixed and changing it like this could 
> potentially break drivers I'm the co-maintainer of. So that whole 
> approach is a clear NAK from my side.
>
> If you find a driver or userspace which doesn't use the formats as 
> defined in the comments of drm_fourcc.h the fix the driver instead of 
> trying to adjust the common header to broken behavior. Cause the later 
> will clearly cause problems with drivers who correctly implemented the 
> interface.
>
> Regards,
> Christian.
>
> > cheers,
> >    Gerd
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx