DRM_FORMAT_* byte order (was: Re: [PATCH] drm: virtio: fix virtio_gpu_cursor_formats)

[Ville Syrjälä]

On Thu, Apr 06, 2017 at 08:27:47PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 06, 2017 at 10:29:43AM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > >  static const uint32_t virtio_gpu_cursor_formats[] = {
> > > > +#ifdef __BIG_ENDIAN
> > > > +	DRM_FORMAT_BGRA8888,
> > > > +#else
> > > >  	DRM_FORMAT_ARGB8888,
> > > > +#endif
> > > 
> > > DRM formats are supposed to be little endian, so this isn't really
> > > correct.
> > 
> > Well, maybe they where *intended* to be little endian at some point in
> > the past.  The actual code appears to interpret them as native endian
> > though.
> > 
> > Lets take a simple example, the bochs driver (qemu sdvga).  It supports
> > 32 bpp with depth 24 (DRM_FORMAT_XRGB8888) as the one and only
> > framebuffer format (see bochs_user_framebuffer_create).  We still had to
> > add a special register to the virtual hardware so the guest can signal
> > to the host whenever the framebuffer is big endian or little endian (see
> > bochs_hw_init), so both ppc64 and ppc64le guests work properly with the
> > qemu stdvga.
> > 
> > So, bigendian guests assume that DRM_FORMAT_XRGB8888 is big endian not
> > little endian.  And given that the fourcc codes are used in the
> > userspace/kernel API too (see DRM_IOCTL_MODE_ADDFB2) I think we can't
> > change that any more ...
> 
> Sigh. That makes mixed endian systems pretty much hopeless :(

Hmm. Maybe it's still possible to salvage something by redefining the
BIG_ENDIAN format bit to mean the "the other endianness". Ugly but it
might still result in something usable.

-- 
Ville Syrjälä
Intel OTC