Re: [PATCH] PCI: vgaarb: Include 0x0380 devices in default selection

[Mario Limonciello <mario.limonciello@amd.com>]

On 6/18/26 01:18, Aaron Ma wrote:
> Some firmware boot GPUs report class 0x0380 (PCI_CLASS_DISPLAY_OTHER)
> instead of PCI_CLASS_DISPLAY_VGA. vgaarb only registers pci_is_vga()
> devices, so those GPUs are not considered by vga_is_firmware_default().
> 
> On these systems the AMD GPU matches the EFI framebuffer but is
> excluded from vgaarb because it is class 0x0380, while the NVIDIA GPU
> does not match the EFI framebuffer but becomes vga_default_device()
> through the vgaarb's enabled-device fallback.
> 
> Register legacy VGA and 0x0380 devices in vgaarb, and expose boot_vga
> for the same set of devices, so the firmware default device can be
> selected correctly.
> 
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
>   drivers/pci/pci-sysfs.c |  3 ++-
>   drivers/pci/vgaarb.c    | 10 +++++-----
>   include/linux/pci.h     | 14 ++++++++++++++
>   3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index d37860841260c..843d83ec9550a 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1717,7 +1717,8 @@ static umode_t pci_dev_attrs_are_visible(struct kobject *kobj,
>   	struct device *dev = kobj_to_dev(kobj);
>   	struct pci_dev *pdev = to_pci_dev(dev);
>   
> -	if (a == &dev_attr_boot_vga.attr && pci_is_vga(pdev))
> +	if (a == &dev_attr_boot_vga.attr &&
> +	    pci_is_vga_or_other_display(pdev))
>   		return a->mode;
 >   >   	if (a == &dev_attr_serial_number.attr && pci_get_dsn(pdev))
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index c360eee11dd9e..0e0878189e3d8 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -796,7 +796,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
>   	}
>   
>   	if (vga_is_boot_device(vgadev)) {
> -		vgaarb_info(&pdev->dev, "setting as boot VGA device%s\n",
> +		vgaarb_info(&pdev->dev, "setting as boot display device%s\n",
>   			    vga_default_device() ?
>   			    " (overriding previous)" : "");
>   		vga_set_default_device(pdev);
> @@ -1483,8 +1483,8 @@ static int pci_notify(struct notifier_block *nb, unsigned long action,
>   
>   	vgaarb_dbg(dev, "%s\n", __func__);
>   
> -	/* Only deal with VGA class devices */
> -	if (!pci_is_vga(pdev))
> +	/* Only deal with legacy VGA and other display controller devices */
> +	if (!pci_is_vga_or_other_display(pdev)) >   		return 0;
>   
>   	/*
> @@ -1530,12 +1530,12 @@ static int __init vga_arb_device_init(void)
>   
>   	bus_register_notifier(&pci_bus_type, &pci_notifier);
>   
> -	/* Add all VGA class PCI devices by default */
> +	/* Add legacy VGA and other display controller PCI devices by default */
>   	pdev = NULL;
>   	while ((pdev =
>   		pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
>   			       PCI_ANY_ID, pdev)) != NULL) {
> -		if (pci_is_vga(pdev))
> +		if (pci_is_vga_or_other_display(pdev))
>   			vga_arbiter_add_pci_device(pdev);
>   	}
>   
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2c4454583c115..195ec1bdac863 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -792,6 +792,20 @@ static inline bool pci_is_vga(struct pci_dev *pdev)
>   	return false;
>   }
>   
> +/**
> + * pci_is_vga_or_other_display - check if the PCI device is VGA or 0x0380
> + * @pdev: PCI device
> + *
> + * Return true for legacy VGA-compatible devices and for "other display
> + * controller" devices.  Some firmware-selected boot display devices expose
> + * class 0x0380 instead of PCI_CLASS_DISPLAY_VGA.
> + */
> +static inline bool pci_is_vga_or_other_display(struct pci_dev *pdev)
> +{
> +	return pci_is_vga(pdev) ||
> +		(pdev->class >> 8) == PCI_CLASS_DISPLAY_OTHER;
> +}
> +
>   /**
>    * pci_is_display - check if the PCI device is a display controller
>    * @pdev: PCI device


This patch doesn't seem correct to me.  It's overloading the purpose of 
boot_vga to cover non VGA devices.  We should be using boot_display.

Trying to hypothesize what's going on here - I guess you're seeing 
boot_display set for more than one device due to enumeration order.

Can you see if this helps instead?

╰─❯ git diff
diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
index 152789f00fcda..9440eb3a7b15d 100644
--- a/arch/x86/video/video-common.c
+++ b/arch/x86/video/video-common.c
@@ -43,9 +43,6 @@ bool video_is_primary_device(struct device *dev)
         if (!pci_is_display(pdev))
                 return false;

-       if (pdev == vga_default_device())
-               return true;
-
  #ifdef CONFIG_SCREEN_INFO
         numres = screen_info_resources(si, res, ARRAY_SIZE(res));
         for (i = 0; i < numres; ++i) {