[PATCH 1/9] firmware/sysfb: Fix EFI/VESA format selection

[Javier Martinez Canillas]

On 12/13/22 21:12, Thomas Zimmermann wrote:
> Select color format for EFI/VESA firmware scanout buffer from the
> number of bits per pixel and the position of the individual color
> components. Fixes the selected format for the buffer in several odd
> cases. For example, XRGB1555 has been reported as ARGB1555 because
> of the different use of depth and transparency in VESA and Linux.
> 
> Bits-per-pixel is always the pixel's raw number of bits; including
> alpha and filler bits. It is preferred over color depth, which has a
> different meaning among various components and standards.
> 
> Also do not compare reserved bits and transparency bits to each other.
> These values have different meanings, as reserved bits include filler
> bits while transparency does not.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
>  drivers/firmware/sysfb_simplefb.c | 43 ++++++++++++++++++++++++++-----
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 

[...]

> +	/*
> +	 * The meaning of depth and bpp for direct-color formats is
> +	 * inconsistent:
> +	 *
> +	 *  - DRM format info specifies depth as the number of color
> +	 *    bits; including alpha, but not including filler bits.
> +	 *  - Linux' EFI platform code computes lfb_depth from the
> +	 *    individual color channels, including the reserved bits.
> +	 *  - VBE 1.1 defines lfb_depth for XRGB1555 as 16, but later
> +	 *    versions use 15.
> +	 *  - On the kernel command line, 'bpp' of 32 is usually
> +	 *    XRGB8888 including the filler bits, but 15 is XRGB1555
> +	 *    not including the filler bit.
> +	 *
> +	 * It's not easily possible to fix this in struct screen_info,
> +	 * as this could break UAPI. The best solution is to compute
> +	 * bits_per_pixel here and ignore lfb_depth. In the loop below,
> +	 * ignore simplefb formats with alpha bits, as EFI and VESA
> +	 * don't specify alpha channels.
> +	 */

Thanks a lot for adding this comment. It's very insightful.

Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat