[PATCH v6 9/9] PCI: Add a new 'boot_display' attribute

Thomas Zimmermann tzimmermann at suse.de
Wed Jul 2 07:49:36 UTC 2025


Hi

Am 30.06.25 um 20:37 schrieb Mario Limonciello:
> On 6/30/2025 2:24 AM, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 27.06.25 um 17:37 schrieb Mario Limonciello:
>>> On 6/27/2025 2:07 AM, Thomas Zimmermann wrote:
>>>> Hi
>>>>
>>>> Am 27.06.25 um 06:31 schrieb Mario Limonciello:
>>>>> From: Mario Limonciello <mario.limonciello at amd.com>
>>>>>
>>>>> On systems with multiple GPUs there can be uncertainty which GPU 
>>>>> is the
>>>>> primary one used to drive the display at bootup. In order to 
>>>>> disambiguate
>>>>> this add a new sysfs attribute 'boot_display' that uses the output of
>>>>> video_is_primary_device() to populate whether a PCI device was 
>>>>> used for
>>>>> driving the display.
>>>>>
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
>>>>> ---
>>>>> v6:
>>>>>   * Only show for the device that is boot display
>>>>>   * Only create after PCI device sysfs files are initialized to 
>>>>> ensure
>>>>>     that resources are ready.
>>>>> v4:
>>>>>   * new patch
>>>>> ---
>>>>>   Documentation/ABI/testing/sysfs-bus-pci |  8 +++++
>>>>>   drivers/pci/pci-sysfs.c                 | 46 
>>>>> +++++++++++++++++++++ ++++
>>>>
>>>> The code looks good. Just one more question: could this be added 
>>>> independently from the PCI bus (at a reasonable cost)? There are 
>>>> other busses that can host the boot display. Alternatively, we'd 
>>>> add this attribute per bus as needed.
>>>
>>> It depends upon the underlying hardware implementation.  On x86 it's 
>>> always PCI and so I realized there is a requirement that PCI 
>>> resources are setup before screen_info event works.
>>>
>>> That is the v5 version of this patch would have had a potential race 
>>> condition with userspace where boot_display didn't always show '1' 
>>> if userspace read it too quickly.
>>>
>>> Other architecture's hardware implementation might have similar 
>>> problem.
>>>
>>> So in summary I think it would be better to do it per-bus.  If we 
>>> realize there is indeed code duplication we can always move this to 
>>> a common helper at that point.
>>
>> Ok, makes sense. With the kernel test robot's issues fixed:
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann at suse.de>
>
> Thanks, I've got a fix locally for it.
>>
>> I guess that interface also needs some sort of OK from user-space devs?
>>
>
> Who needs to OK it?  I do have MR's for matching userspace 
> implementations mentioned in the cover letter already.

The MRs are the right place. Maybe ask Dave Airlie for a comment. He was 
most outspoken against the original approach.

Best regards
Thomas

>
>> Best regards
>> Thomas
>>
>>>
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>>   2 files changed, 54 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/ 
>>>>> Documentation/ ABI/testing/sysfs-bus-pci
>>>>> index 69f952fffec72..8b455b1a58852 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-bus-pci
>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-pci
>>>>> @@ -612,3 +612,11 @@ Description:
>>>>>             # ls doe_features
>>>>>             0001:01        0001:02        doe_discovery
>>>>> +
>>>>> +What:        /sys/bus/pci/devices/.../boot_display
>>>>> +Date:        October 2025
>>>>> +Contact:    Linux PCI developers <linux-pci at vger.kernel.org>
>>>>> +Description:
>>>>> +        This file indicates the device was used as a boot
>>>>> +        display. If the device was used as the boot display, the 
>>>>> file
>>>>> +        will be present and contain "1".
>>>>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>>>>> index 268c69daa4d57..cc766461de1da 100644
>>>>> --- a/drivers/pci/pci-sysfs.c
>>>>> +++ b/drivers/pci/pci-sysfs.c
>>>>> @@ -30,6 +30,7 @@
>>>>>   #include <linux/msi.h>
>>>>>   #include <linux/of.h>
>>>>>   #include <linux/aperture.h>
>>>>> +#include <asm/video.h>
>>>>>   #include "pci.h"
>>>>>   #ifndef ARCH_PCI_DEV_GROUPS
>>>>> @@ -679,6 +680,13 @@ const struct attribute_group *pcibus_groups[] 
>>>>> = {
>>>>>       NULL,
>>>>>   };
>>>>> +static ssize_t boot_display_show(struct device *dev, struct 
>>>>> device_attribute *attr,
>>>>> +                 char *buf)
>>>>> +{
>>>>> +    return sysfs_emit(buf, "1\n");
>>>>> +}
>>>>> +static DEVICE_ATTR_RO(boot_display);
>>>>> +
>>>>>   static ssize_t boot_vga_show(struct device *dev, struct 
>>>>> device_attribute *attr,
>>>>>                    char *buf)
>>>>>   {
>>>>> @@ -1246,6 +1254,37 @@ static int pci_create_attr(struct pci_dev 
>>>>> *pdev, int num, int write_combine)
>>>>>       return 0;
>>>>>   }
>>>>> +/**
>>>>> + * pci_create_boot_display_file - create a file in sysfs for @dev
>>>>> + * @pdev: dev in question
>>>>> + *
>>>>> + * Creates a file `boot_display` in sysfs for the PCI device @pdev
>>>>> + * if it is the boot display device.
>>>>> + */
>>>>> +static int pci_create_boot_display_file(struct pci_dev *pdev)
>>>>> +{
>>>>> +#ifdef CONFIG_VIDEO
>>>>> +    if (video_is_primary_device(&pdev->dev))
>>>>> +        return sysfs_create_file(&pdev->dev.kobj, 
>>>>> &dev_attr_boot_display.attr);
>>>>> +#endif
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * pci_remove_boot_display_file - remove the boot display file 
>>>>> for @dev
>>>>> + * @pdev: dev in question
>>>>> + *
>>>>> + * Removes the file `boot_display` in sysfs for the PCI device @pdev
>>>>> + * if it is the boot display device.
>>>>> + */
>>>>> +static void pci_remove_boot_display_file(struct pci_dev *pdev)
>>>>> +{
>>>>> +#ifdef CONFIG_VIDEO
>>>>> +    if (video_is_primary_device(&pdev->dev))
>>>>> +        sysfs_remove_file(&pdev->dev.kobj, 
>>>>> &dev_attr_boot_display.attr);
>>>>> +#endif
>>>>> +}
>>>>> +
>>>>>   /**
>>>>>    * pci_create_resource_files - create resource files in sysfs 
>>>>> for @dev
>>>>>    * @pdev: dev in question
>>>>> @@ -1654,9 +1693,15 @@ static const struct attribute_group 
>>>>> pci_dev_resource_resize_group = {
>>>>>   int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>>>>>   {
>>>>> +    int retval;
>>>>> +
>>>>>       if (!sysfs_initialized)
>>>>>           return -EACCES;
>>>>> +    retval = pci_create_boot_display_file(pdev);
>>>>> +    if (retval)
>>>>> +        return retval;
>>>>> +
>>>>>       return pci_create_resource_files(pdev);
>>>>>   }
>>>>> @@ -1671,6 +1716,7 @@ void pci_remove_sysfs_dev_files(struct 
>>>>> pci_dev *pdev)
>>>>>       if (!sysfs_initialized)
>>>>>           return;
>>>>> +    pci_remove_boot_display_file(pdev);
>>>>>       pci_remove_resource_files(pdev);
>>>>>   }
>>>>
>>>
>>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)



More information about the dri-devel mailing list