Message ID | 20240617110725.23330-1-tzimmermann@suse.de |
---|---|
State | New |
Headers | show |
Series | [v2] fbdev: vesafb: Detect VGA compatibility from screen info's VESA attributes | expand |
Am 17.06.24 um 13:06 schrieb Thomas Zimmermann: > 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. > > Besides fixing the bug in vesafb, this commit introduces a helper that > reads the correct bit from screen_info. > > v2: > - clarify comment on non-VGA modes (Helge) > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 setup code") > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> > Cc: <stable@vger.kernel.org> # v2.6.23+ > --- > drivers/video/fbdev/vesafb.c | 2 +- > include/linux/screen_info.h | 10 ++++++++++ > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c > index 8ab64ae4cad3e..5a161750a3aee 100644 > --- a/drivers/video/fbdev/vesafb.c > +++ b/drivers/video/fbdev/vesafb.c > @@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device *dev) > if (si->orig_video_isVGA != VIDEO_TYPE_VLFB) > return -ENODEV; > > - vga_compat = (si->capabilities & 2) ? 0 : 1; > + vga_compat = !__screen_info_vbe_mode_nonvga(si); > vesafb_fix.smem_start = si->lfb_base; > vesafb_defined.bits_per_pixel = si->lfb_depth; > if (15 == vesafb_defined.bits_per_pixel) > diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h > index 75303c126285a..d21f8e4e9f4a4 100644 > --- a/include/linux/screen_info.h > +++ b/include/linux/screen_info.h > @@ -49,6 +49,16 @@ static inline u64 __screen_info_lfb_size(const struct screen_info *si, unsigned > return lfb_size; > } > > +static inline bool __screen_info_vbe_mode_nonvga(const struct screen_info *si) > +{ > + /* > + * VESA modes typically run on VGA hardware. Set bit 5 signal that this 'signals' > + * is not the case. Drivers can then not make use of VGA resources. See > + * Sec 4.4 of the VBE 2.0 spec. > + */ > + return si->vesa_attributes & BIT(5); > +} > + > static inline unsigned int __screen_info_video_type(unsigned int type) > { > switch (type) {
Hi Am 17.06.24 um 14:42 schrieb Helge Deller: > On 6/17/24 13:30, Thomas Zimmermann wrote: >> >> >> Am 17.06.24 um 13:06 schrieb Thomas Zimmermann: >>> 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. >>> >>> Besides fixing the bug in vesafb, this commit introduces a helper that >>> reads the correct bit from screen_info. >>> >>> v2: >>> - clarify comment on non-VGA modes (Helge) >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> Fixes: 5e8ddcbe8692 ("Video mode probing support for the new x86 >>> setup code") >>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >>> Cc: <stable@vger.kernel.org> # v2.6.23+ >>> --- >>> drivers/video/fbdev/vesafb.c | 2 +- >>> include/linux/screen_info.h | 10 ++++++++++ >>> 2 files changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/video/fbdev/vesafb.c >>> b/drivers/video/fbdev/vesafb.c >>> index 8ab64ae4cad3e..5a161750a3aee 100644 >>> --- a/drivers/video/fbdev/vesafb.c >>> +++ b/drivers/video/fbdev/vesafb.c >>> @@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device >>> *dev) >>> if (si->orig_video_isVGA != VIDEO_TYPE_VLFB) >>> return -ENODEV; >>> - vga_compat = (si->capabilities & 2) ? 0 : 1; >>> + vga_compat = !__screen_info_vbe_mode_nonvga(si); >>> vesafb_fix.smem_start = si->lfb_base; >>> vesafb_defined.bits_per_pixel = si->lfb_depth; >>> if (15 == vesafb_defined.bits_per_pixel) >>> diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h >>> index 75303c126285a..d21f8e4e9f4a4 100644 >>> --- a/include/linux/screen_info.h >>> +++ b/include/linux/screen_info.h >>> @@ -49,6 +49,16 @@ static inline u64 __screen_info_lfb_size(const >>> struct screen_info *si, unsigned >>> return lfb_size; >>> } >>> +static inline bool __screen_info_vbe_mode_nonvga(const struct >>> screen_info *si) >>> +{ >>> + /* >>> + * VESA modes typically run on VGA hardware. Set bit 5 signal >>> that this >> >> 'signals' > > I've fixed this up in your patch and applied it to the fbdev git tree. > No need to send new patch... Great, thank you so much. Best regards Thomas > > Thanks! > Helge >
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c index 8ab64ae4cad3e..5a161750a3aee 100644 --- a/drivers/video/fbdev/vesafb.c +++ b/drivers/video/fbdev/vesafb.c @@ -271,7 +271,7 @@ static int vesafb_probe(struct platform_device *dev) if (si->orig_video_isVGA != VIDEO_TYPE_VLFB) return -ENODEV; - vga_compat = (si->capabilities & 2) ? 0 : 1; + vga_compat = !__screen_info_vbe_mode_nonvga(si); vesafb_fix.smem_start = si->lfb_base; vesafb_defined.bits_per_pixel = si->lfb_depth; if (15 == vesafb_defined.bits_per_pixel) diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h index 75303c126285a..d21f8e4e9f4a4 100644 --- a/include/linux/screen_info.h +++ b/include/linux/screen_info.h @@ -49,6 +49,16 @@ static inline u64 __screen_info_lfb_size(const struct screen_info *si, unsigned return lfb_size; } +static inline bool __screen_info_vbe_mode_nonvga(const struct screen_info *si) +{ + /* + * VESA modes typically run on VGA hardware. Set bit 5 signal that this + * is not the case. Drivers can then not make use of VGA resources. See + * Sec 4.4 of the VBE 2.0 spec. + */ + return si->vesa_attributes & BIT(5); +} + static inline unsigned int __screen_info_video_type(unsigned int type) { switch (type) {