Hi Javier, On Fri, Jan 7, 2022 at 9:00 PM Javier Martinez Canillas <javi...@redhat.com> wrote: > 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. > > 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-...@moonlit-rail.com> > Cc: <sta...@vger.kernel.org> # 5.15.x > Signed-off-by: Javier Martinez Canillas <javi...@redhat.com> > Tested-by: Kris Karas <bugs-...@moonlit-rail.com>
Thanks for your patch! > --- a/drivers/video/fbdev/vga16fb.c > +++ b/drivers/video/fbdev/vga16fb.c > @@ -1422,6 +1422,18 @@ static int __init vga16fb_init(void) > > vga16fb_setup(option); > #endif > + > + /* 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; Probably these checks should be wrapped inside a check for CONFIG_X86? All non-x86 architectures (except for 2 MIPS platforms) treat orig_video_isVGA as a boolean flag, and just assign 1 to it. > + > + 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; > + Likewise. A long time ago, I used vga16fb on a PPC box to use a standard PC graphics card (initialized using an emulator for the card's BIOS ROM), as a second display. The above changes would break such a use case. > ret = platform_driver_register(&vga16fb_driver); > > if (!ret) { Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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