Re: [PATCH 8/8] fbdev: Simplify fb_is_primary_device for x86

2023-04-05 Thread Javier Martinez Canillas
Daniel Vetter  writes:

> vga_default_device really is supposed to cover all corners, at least
> for x86. Additionally checking for rom shadowing should be redundant,
> because the bios/fw only does that for the boot vga device.
>
> If this turns out to be wrong, then most likely that's a special case
> which should be added to the vgaarb code, not replicated all over.
>
> Patch motived by changes to the aperture helpers, which also have this
> open code in a bunch of places, and which are all removed in a
> clean-up series. This function here is just for selecting the default
> fbdev device for fbcon, but I figured for consistency it might be good
> to throw this patch in on top.
>
> Note that the shadow rom check predates vgaarb, which was only wired
> up in 88674088d10c ("x86: Use vga_default_device() when determining
> whether an fb is primary"). That patch doesn't explain why we still
> fall back to the shadow rom check.
>
> Signed-off-by: Daniel Vetter 
> Cc: Daniel Vetter 
> Cc: Helge Deller 
> Cc: Daniel Vetter 
> Cc: Javier Martinez Canillas 
> Cc: Thomas Zimmermann 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: x...@kernel.org
> Cc: "H. Peter Anvin" 
> ---
>  arch/x86/video/fbdev.c | 13 +

[...]

> + if (pci_dev == vga_default_device())
>   return 1;
>   return 0;

or just return pci_dev == vga_default_device() ;

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat



[PATCH 8/8] fbdev: Simplify fb_is_primary_device for x86

2023-04-04 Thread Daniel Vetter
vga_default_device really is supposed to cover all corners, at least
for x86. Additionally checking for rom shadowing should be redundant,
because the bios/fw only does that for the boot vga device.

If this turns out to be wrong, then most likely that's a special case
which should be added to the vgaarb code, not replicated all over.

Patch motived by changes to the aperture helpers, which also have this
open code in a bunch of places, and which are all removed in a
clean-up series. This function here is just for selecting the default
fbdev device for fbcon, but I figured for consistency it might be good
to throw this patch in on top.

Note that the shadow rom check predates vgaarb, which was only wired
up in 88674088d10c ("x86: Use vga_default_device() when determining
whether an fb is primary"). That patch doesn't explain why we still
fall back to the shadow rom check.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Helge Deller 
Cc: Daniel Vetter 
Cc: Javier Martinez Canillas 
Cc: Thomas Zimmermann 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
 arch/x86/video/fbdev.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
index 9fd24846d094..5ec4eafbb981 100644
--- a/arch/x86/video/fbdev.c
+++ b/arch/x86/video/fbdev.c
@@ -14,26 +14,15 @@
 int fb_is_primary_device(struct fb_info *info)
 {
struct device *device = info->device;
-   struct pci_dev *default_device = vga_default_device();
struct pci_dev *pci_dev;
-   struct resource *res;
 
if (!device || !dev_is_pci(device))
return 0;
 
pci_dev = to_pci_dev(device);
 
-   if (default_device) {
-   if (pci_dev == default_device)
-   return 1;
-   return 0;
-   }
-
-   res = pci_dev->resource + PCI_ROM_RESOURCE;
-
-   if (res->flags & IORESOURCE_ROM_SHADOW)
+   if (pci_dev == vga_default_device())
return 1;
-
return 0;
 }
 EXPORT_SYMBOL(fb_is_primary_device);
-- 
2.40.0