[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