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

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

On 6/21/26 23:12, Aaron Ma wrote:
> On Mon, Jun 22, 2026 at 12:00 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>>
>>
>> On 6/21/26 20:50, Aaron Ma wrote:
>>> On Mon, Jun 22, 2026 at 1:15 AM Mario Limonciello
>>> <mario.limonciello@amd.com> wrote:
>>>>
>>>>
>>>>
>>>> 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) {
>>>
>>> Hi Mario,
>>>
>>> I tested your change. boot_display becomes correct, but the issue is not fixed.
>>>
>>> The kernel state becomes:
>>>
>>>     AMD:    boot_display=1, boot_vga absent
>>>     NVIDIA: boot_display absent, boot_vga=1
>>>
>>> Current Xorg checks boot_display || boot_vga, so both GPUs still match.
>>> The selected primary GPU still depends on Xorg's DRM device enumeration order.
>>>
>>> So boot_vga is still wrong, and that still matters. The kernel should not
>>> expose boot_vga=1 for NVIDIA when the firmware framebuffer is on AMD.
>>>
>>> My patch makes boot_vga and boot_display agree on the AMD boot display.
>>>
>>> Thanks,
>>> Aaron
>>
>> Got it.  I still don't think it's correct to overload boot_vga.  It
>> would be better for us to not show boot_vga at all - because it's not
>> correct.
>>
>> Maybe we should drop the fallback code in VGA arbiter that is setting
>> NVIDIA as boot_vga at all.
>>
>> Could you look at doing that?
>>
> 
> Hi Mario,
> 
> I looked at dropping the fallback. I don't think we should remove it globally;
> old systems may still need vga_default_device() when no firmware framebuffer
> owner can be identified.
> 

What exactly happens on such old systems?  Any chance you have anything 
in your cert lab that's really old you could try?

I have to wonder if the boot_display code will work on them at least and 
we can just rely upon that.

I don't want to keep this fallback path around as tech debt if we don't 
need it and it's actively causing problems.

> For v2 I will keep the fallback, but avoid the wrong result on this system. The
> 0x0380 firmware display can become the default/boot_vga device, while legacy
> VGA decoding and ownership stay limited to pci_is_vga() devices.
> 
> So this keeps compatibility, but avoids reporting NVIDIA as boot_vga when the
> firmware framebuffer is on AMD.
> 

I'll look over the patch.