Re: [PATCH 07/11] drm/fbdevdrm: Add DRM <-> fbdev pixel-format conversion

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Mar 27, 2019 at 09:28:49AM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 26.03.19 um 17:29 schrieb Ville Syrjälä:
> >> +
> >> +static bool is_c8(const struct fb_info* fb_info)
> >> +{
> >> +	return fb_info->var.bits_per_pixel = 8;
> >> +}
> >> +
> >> +static bool is_rgb565_be(const struct fb_info* fb_info)
> >> +{
> >> +	return (fb_info->var.bits_per_pixel = 16) &&
> >> +	       (fb_info->var.red.offset = 0) &&
> >> +	       (fb_info->var.red.length = 5) &&
> >> +	       (fb_info->var.green.offset = 5) &&
> >> +	       (fb_info->var.green.length = 6) &&
> >> +	       (fb_info->var.blue.offset = 11) &&
> >> +	       (fb_info->var.blue.length = 5);
> >> +}
> > 
> > You can't distinguish LE vs. BE like this.
> > 
> > Maybe FBINFO_FOREIGN_ENDIAN is trustworthy?
> > 
> 
> The test function means 'is the framebuffer in RGB565 format if the host
> uses big-endian access'. Further below in the file, there are macros
> that reverse the order of fields for little-endian hosts.

And that does not work. Wrong endianness swaps bytes, not components.

> 
> However, mapping all this to DRM formats is confusing.
> 
> According to the documentation, DRM_FORMAT_ is always in little-endian
> format. But does that mean that the device uses little endian or that
> the host uses little endian? If the device and the host disagree on
> endianess, which takes precedence?

It just means "the pixel value listed is stored in memory 
least significant byte first".

> 
> In the end, I tried different combinations of tests and DRM formats, and
> checked the resulting output on the screen.

This was just pointed out to me recently:
https://github.com/afrantzis/pixel-format-guide

It seems to interpret drm formats correctly.

Though I wouldn't mind if someone improved the drm_fourcc.h docs since
everyone except me seems to get confused by the current wording.

-- 
Ville Syrjälä
Intel