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

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

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?