Hi Javier
Am 13.06.24 um 11:35 schrieb Javier Martinez Canillas:
Thomas Zimmermann tzimmermann@suse.de writes:
Hello Thomas,
Test the vesa_attributes field in struct screen_info for compatibility with VGA hardware. Vesafb currently tests bit 1 in screen_info's capabilities field, It sets the framebuffer address size and is unrelated to VGA.
Section 4.4 of the Vesa VBE 2.0 specifications defines that bit 5 in the mode's attributes field signals VGA compatibility. The mode is compatible with VGA hardware if the bit is clear. In that case, the driver can access VGA state of the VBE's underlying hardware. The vesafb driver uses this feature to program the color LUT in palette modes. Without, colors might be incorrect.
The problem got introduced in commit 89ec4c238e7a ("[PATCH] vesafb: Fix incorrect logo colors in x86_64"). It incorrectly stores the mode attributes in the screen_info's capabilities field and updates vesafb accordingly. Later, commit 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code") fixed the screen_info, but did not update vesafb. Color output still tends to work, because bit 1 in capabilities is usually 0.
How did you find this ?
I was reading through vesafb and found that [1] and [2] look surprisingly similar, which makes no sense. So I started looking where bit 1 came from. The flag signals a 64-bit framebuffer address for EFI (see VIDEO_CAPABILITY_64BIT_BASE https://elixir.bootlin.com/linux/latest/C/ident/VIDEO_CAPABILITY_64BIT_BASE). But old VESA framebuffers are usually located within the first 32-bit range. So the bit is mostly 0 and vesafb works as expected.
[1] https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/vesafb.c#... [2] https://elixir.bootlin.com/linux/latest/source/include/linux/screen_info.h#L...
Besides fixing the bug in vesafb, this commit introduces a helper that reads the correct bit from screen_info.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code") Cc: stable@vger.kernel.org # v2.6.23+
The patch looks correct to me after your explanation in the commit message and looking at the mentioned commits.
Reviewed-by: Javier Martinez Canillas javierm@redhat.com
Thanks a lot.
Best regards Thomas