Hello Geert and Kara, On 1/9/22 21:20, Kris Karas (Bug reporting) wrote: > Groetje, Geert, > > Geert Uytterhoeven wrote: >> >>> - par->isVGA = screen_info.orig_video_isVGA; >>> + par->isVGA = screen_info.orig_video_isVGA == VIDEO_TYPE_VGAC; >> All non-x86 architectures (except for 2 MIPS platforms) treat >> orig_video_isVGA as a boolean flag, and just assign 1 to it. >> Hence this change would break them. > > I see a bit of a conflict with using orig_video_isVGA as a boolean. All > the modern architecture-agnostic driver code, such as sysfb, > sysfb_simplefb, and efifb, all use and expect orig_video_isVGA to be an > integer. On the other hand, the VGA driver for XEN first sets > orig_video_isVGA = 1 (boolean), and then VIDEO_TYPE_VLFB or > VIDEO_TYPE_EFI (integer). Overloading the definition for > orig_video_isVGA to be both boolean and integer - within the same file - > seems like a recipe for bugs to me. > Agreed with Kara on this. I believe the non-x86 arches should be fixed to set it to the correct value. > That said, I think that wrapping the par->isVGA code, above, within a > check for CONFIG_X86 seems safe and expedient. But I would be much > happier if the non-x86 architectures would set it to a proper integer > value (even if fake) that coincidentally satisfies boolean "true", say > VIDEO_TYPE_VGAC; that way, there would be no confusion as to data type > in all the more recent architecture-agnostic framebuffer code. > Yes, I'll post a v2 to do that but hopefully the CONFIG_X86 guard could be dropped in the future once all the architectures are fixed. > Kris > Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
