This patch series contains two fixes for the vga16fb driver. I looked at the driver due a regression reported [0], caused by commit d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers support").
The mentioned commit didn't change any logic but just moved the platform device registration that matches the vesafb and efifb drivers to happen later. And this caused the vga16fb driver to be probed even in machines that don't have an EGA or VGA video adapter.
This is a v2 of the patch series that addresses issues pointed out by Geert Uytterhoeven.
Patch #1 is fixing the wrong check to determine if either EGA or VGA is used and patch #2 adds a check to the driver to only be loaded for EGA and VGA 16 color graphic cards.
[0]: https://bugzilla.kernel.org/show_bug.cgi?id=215001
Best regards, Javier
Changes in v2: - Make the change only for x86 (Geert Uytterhoeven) - Only check the suppported video mode for x86 (Geert Uytterhoeven).
Javier Martinez Canillas (2): video: vga16fb: Fix logic that checks for the display standard video: vga16fb: Only probe for EGA and VGA 16 color graphic cards
drivers/video/fbdev/vga16fb.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
The vga16fb framebuffer driver supports both Enhanced Graphics Adapter (EGA) and Video Graphics Array (VGA) 16 color graphic cards.
But the logic to check whether the EGA or VGA standard are used is not correct. It just checks if screen_info.orig_video_isVGA is set, but it should check if is set to VIDEO_TYPE_VGAC instead.
This means that it assumes to be VGA even if is set to VIDEO_TYPE_EGAC.
All non-x86 architectures though treat orig_video_isVGA as a boolean so only do the change for x86 and keep the old logic for the other arches.
Signed-off-by: Javier Martinez Canillas javierm@redhat.com ---
Changes in v2: - Make the change only for x86 (Geert Uytterhoeven)
drivers/video/fbdev/vga16fb.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c index e2757ff1c23d..8c464409eb99 100644 --- a/drivers/video/fbdev/vga16fb.c +++ b/drivers/video/fbdev/vga16fb.c @@ -1332,7 +1332,12 @@ static int vga16fb_probe(struct platform_device *dev) printk(KERN_INFO "vga16fb: mapped to 0x%p\n", info->screen_base); par = info->par;
+#if defined(CONFIG_X86) + par->isVGA = screen_info.orig_video_isVGA == VIDEO_TYPE_VGAC; +#else + /* non-x86 architectures treat orig_video_isVGA as a boolean flag */ par->isVGA = screen_info.orig_video_isVGA; +#endif par->palette_blanked = 0; par->vesa_blanked = 0;
The vga16fb framebuffer driver only supports Enhanced Graphics Adapter (EGA) and Video Graphics Array (VGA) 16 color graphic cards.
But it doesn't check if the adapter is one of those or if a VGA16 mode is used. This means that the driver will be probed even if a VESA BIOS Extensions (VBE) or Graphics Output Protocol (GOP) interface is used.
This issue has been present for a long time but it was only exposed by commit d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers support") since the platform device registration to match the {vesa,efi}fb drivers is done later as a consequence of that change.
All non-x86 architectures though treat orig_video_isVGA as a boolean so only do the supported video mode check for x86 and not for other arches.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215001 Fixes: d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers support") Reported-by: Kris Karas bugs-a21@moonlit-rail.com Cc: stable@vger.kernel.org # 5.15.x Signed-off-by: Javier Martinez Canillas javierm@redhat.com Tested-by: Kris Karas bugs-a21@moonlit-rail.com ---
Changes in v2: - Only check the suppported video mode for x86 (Geert Uytterhoeven).
drivers/video/fbdev/vga16fb.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c index 8c464409eb99..d21f68f3ee44 100644 --- a/drivers/video/fbdev/vga16fb.c +++ b/drivers/video/fbdev/vga16fb.c @@ -184,6 +184,25 @@ static inline void setindex(int index) vga_io_w(VGA_GFX_I, index); }
+/* Check if the video mode is supported by the driver */ +static inline int check_mode_supported(void) +{ + /* non-x86 architectures treat orig_video_isVGA as a boolean flag */ +#if defined(CONFIG_X86) + /* only EGA and VGA in 16 color graphic mode are supported */ + if (screen_info.orig_video_isVGA != VIDEO_TYPE_EGAC && + screen_info.orig_video_isVGA != VIDEO_TYPE_VGAC) + return -ENODEV; + + if (screen_info.orig_video_mode != 0x0D && /* 320x200/4 (EGA) */ + screen_info.orig_video_mode != 0x0E && /* 640x200/4 (EGA) */ + screen_info.orig_video_mode != 0x10 && /* 640x350/4 (EGA) */ + screen_info.orig_video_mode != 0x12) /* 640x480/4 (VGA) */ + return -ENODEV; +#endif + return 0; +} + static void vga16fb_pan_var(struct fb_info *info, struct fb_var_screeninfo *var) { @@ -1427,6 +1446,11 @@ static int __init vga16fb_init(void)
vga16fb_setup(option); #endif + + ret = check_mode_supported(); + if (ret) + return ret; + ret = platform_driver_register(&vga16fb_driver);
if (!ret) {
Hi,
On Mon, Jan 10, 2022 at 10:56:23AM +0100, Javier Martinez Canillas wrote:
This patch series contains two fixes for the vga16fb driver. I looked at the driver due a regression reported [0], caused by commit d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers support").
The mentioned commit didn't change any logic but just moved the platform device registration that matches the vesafb and efifb drivers to happen later. And this caused the vga16fb driver to be probed even in machines that don't have an EGA or VGA video adapter.
This is a v2 of the patch series that addresses issues pointed out by Geert Uytterhoeven.
Patch #1 is fixing the wrong check to determine if either EGA or VGA is used and patch #2 adds a check to the driver to only be loaded for EGA and VGA 16 color graphic cards.
For both patches,
Acked-by: Maxime Ripard maxime@cerno.tech
Maxime
Hi Javier, Geert, et al,
Javier Martinez Canillas wrote:
Changes in v2:
- Make the change only for x86 (Geert Uytterhoeven)
- Only check the suppported video mode for x86 (Geert Uytterhoeven).
I just updated Bug 215001 to reflect that I have tested this new, V2 patch against 4 systems, one more than last time - 2 BIOS/VGAC and 2 UEFI - and it works perfectly on all four.
Thanks, Javier, for the excellent work! I didn't test with non-X86, but the code appears to bypass the patch on non-X86, so should work fine for Geert.
Kris
Tested-By: Kris Karas bugs-a21@moonlit-rail.com
Hi Kris,
On Wed, Jan 12, 2022 at 3:19 AM Kris Karas (Bug reporting) bugs-a21@moonlit-rail.com wrote:
Javier Martinez Canillas wrote:
Changes in v2:
- Make the change only for x86 (Geert Uytterhoeven)
- Only check the suppported video mode for x86 (Geert Uytterhoeven).
I just updated Bug 215001 to reflect that I have tested this new, V2 patch against 4 systems, one more than last time - 2 BIOS/VGAC and 2 UEFI - and it works perfectly on all four.
Thanks, Javier, for the excellent work! I didn't test with non-X86, but the code appears to bypass the patch on non-X86, so should work fine for Geert.
Note that I can no longer test the PPC use case, as the hardware died a long time ago.
Tested-By: Kris Karas bugs-a21@moonlit-rail.com
Thanks!
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hello Kris,
On 1/12/22 03:19, Kris Karas (Bug reporting) wrote:
Hi Javier, Geert, et al,
Javier Martinez Canillas wrote:
Changes in v2:
- Make the change only for x86 (Geert Uytterhoeven)
- Only check the suppported video mode for x86 (Geert Uytterhoeven).
I just updated Bug 215001 to reflect that I have tested this new, V2 patch against 4 systems, one more than last time - 2 BIOS/VGAC and 2 UEFI - and it works perfectly on all four.
Thanks, Javier, for the excellent work! I didn't test with non-X86, but the code appears to bypass the patch on non-X86, so should work fine for Geert.
Kris
Tested-By: Kris Karas bugs-a21@moonlit-rail.com
Thanks a lot for testing again!
I've applied patch #1 to drm-misc-next and #2 to drm-misc-fixes.
Best regards,
dri-devel@lists.freedesktop.org
-
Geert Uytterhoeven -
Javier Martinez Canillas -
Kris Karas (Bug reporting) -
Maxime Ripard