From: Mario Limonciello <mario.limonciello@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 some desktop
environments this can lead to increased power consumption because
secondary GPUs may be used for rendering and never go to a low power
state. 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.
Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/issues/23
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v8:
* Add bug link
* Update commit message text
* Update boot_display description text
v7:
* fix lkp failure
* Add tag
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 | 9 +++++
drivers/pci/pci-sysfs.c | 46 +++++++++++++++++++++++++
2 files changed, 55 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 69f952fffec72..a2c74d4ebeadd 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -612,3 +612,12 @@ 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@vger.kernel.org>
+Description:
+ This file indicates that displays connected to the device were
+ used to display the boot sequence. If a display connected to
+ the device was used to display the boot sequence the file will
+ be present and contain "1".
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 268c69daa4d57..6b1a0ae254d3a 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)
{
@@ -1051,6 +1059,37 @@ void pci_remove_legacy_files(struct pci_bus *b)
}
#endif /* HAVE_PCI_LEGACY */
+/**
+ * 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
+}
+
#if defined(HAVE_PCI_MMAP) || defined(ARCH_GENERIC_PCI_MMAP_RESOURCE)
/**
* pci_mmap_resource - map a PCI resource into user memory space
@@ -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);
}
--
2.43.0On Thu, Jul 17, 2025 at 12:38:12PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@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 some desktop
> environments this can lead to increased power consumption because
> secondary GPUs may be used for rendering and never go to a low power
> state. 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.
> +What: /sys/bus/pci/devices/.../boot_display
> +Date: October 2025
> +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
> +Description:
> + This file indicates that displays connected to the device were
> + used to display the boot sequence. If a display connected to
> + the device was used to display the boot sequence the file will
> + be present and contain "1".
> 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);
In addition to Mani's question about whether /sys/bus/pci/ is the
right place for this (which is a very good question), it's also been
pointed out to me that we've been trying to get rid of
pci_create_sysfs_dev_files() for years.
If it's possible to make this a static attribute that would be much,
much cleaner.
> + 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);
> }
>
> --
> 2.43.0
>
On 7/18/2025 12:25 PM, Bjorn Helgaas wrote:
> On Thu, Jul 17, 2025 at 12:38:12PM -0500, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@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 some desktop
>> environments this can lead to increased power consumption because
>> secondary GPUs may be used for rendering and never go to a low power
>> state. 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.
>
>> +What: /sys/bus/pci/devices/.../boot_display
>> +Date: October 2025
>> +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
>> +Description:
>> + This file indicates that displays connected to the device were
>> + used to display the boot sequence. If a display connected to
>> + the device was used to display the boot sequence the file will
>> + be present and contain "1".
>
>> 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);
>
> In addition to Mani's question about whether /sys/bus/pci/ is the
> right place for this (which is a very good question), it's also been
> pointed out to me that we've been trying to get rid of
> pci_create_sysfs_dev_files() for years.
>
> If it's possible to make this a static attribute that would be much,
> much cleaner.
Right - I tried to do this, but the problem is at the time the PCI
device is created the information needed to make the judgement isn't
ready. The options end up being:
* a sysfs file for every display device with 0/1
* a sysfs file that is not accurate until later in the boot
So IMO it /needs/ to come later.
>
>> + 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);
>> }
>>
>> --
>> 2.43.0
>>
On Fri, Jul 18, 2025 at 12:29:05PM -0500, Mario Limonciello wrote:
> On 7/18/2025 12:25 PM, Bjorn Helgaas wrote:
> > On Thu, Jul 17, 2025 at 12:38:12PM -0500, Mario Limonciello wrote:
> > > From: Mario Limonciello <mario.limonciello@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 some desktop
> > > environments this can lead to increased power consumption because
> > > secondary GPUs may be used for rendering and never go to a low power
> > > state. 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.
> >
> > > +What: /sys/bus/pci/devices/.../boot_display
> > > +Date: October 2025
> > > +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
> > > +Description:
> > > + This file indicates that displays connected to the device were
> > > + used to display the boot sequence. If a display connected to
> > > + the device was used to display the boot sequence the file will
> > > + be present and contain "1".
> >
> > > 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);
> >
> > In addition to Mani's question about whether /sys/bus/pci/ is the
> > right place for this (which is a very good question), it's also been
> > pointed out to me that we've been trying to get rid of
> > pci_create_sysfs_dev_files() for years.
> >
> > If it's possible to make this a static attribute that would be much,
> > much cleaner.
>
> Right - I tried to do this, but the problem is at the time the PCI device is
> created the information needed to make the judgement isn't ready. The
> options end up being:
> * a sysfs file for every display device with 0/1
> * a sysfs file that is not accurate until later in the boot
What's missing? The specifics might be helpful if someone has another
crack at getting rid of pci_create_sysfs_dev_files() in the future.
> So IMO it /needs/ to come later.
>
> >
> > > + 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);
> > > }
> > > --
> > > 2.43.0
> > >
>
On 7/18/2025 12:36 PM, Bjorn Helgaas wrote:
> On Fri, Jul 18, 2025 at 12:29:05PM -0500, Mario Limonciello wrote:
>> On 7/18/2025 12:25 PM, Bjorn Helgaas wrote:
>>> On Thu, Jul 17, 2025 at 12:38:12PM -0500, Mario Limonciello wrote:
>>>> From: Mario Limonciello <mario.limonciello@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 some desktop
>>>> environments this can lead to increased power consumption because
>>>> secondary GPUs may be used for rendering and never go to a low power
>>>> state. 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.
>>>
>>>> +What: /sys/bus/pci/devices/.../boot_display
>>>> +Date: October 2025
>>>> +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
>>>> +Description:
>>>> + This file indicates that displays connected to the device were
>>>> + used to display the boot sequence. If a display connected to
>>>> + the device was used to display the boot sequence the file will
>>>> + be present and contain "1".
>>>
>>>> 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);
>>>
>>> In addition to Mani's question about whether /sys/bus/pci/ is the
>>> right place for this (which is a very good question), it's also been
>>> pointed out to me that we've been trying to get rid of
>>> pci_create_sysfs_dev_files() for years.
>>>
>>> If it's possible to make this a static attribute that would be much,
>>> much cleaner.
>>
>> Right - I tried to do this, but the problem is at the time the PCI device is
>> created the information needed to make the judgement isn't ready. The
>> options end up being:
>> * a sysfs file for every display device with 0/1
>> * a sysfs file that is not accurate until later in the boot
>
> What's missing? The specifics might be helpful if someone has another
> crack at getting rid of pci_create_sysfs_dev_files() in the future.
The underlying SCREEN_INFO code tries to walk through all the PCI
devices in a loop, but at the time all the devices are walked the memory
regions associated with the device weren't populated.
So my earlier hack was to re-run the screen info check, and it was awful.
>
>> So IMO it /needs/ to come later.
>>
>>>
>>>> + 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);
>>>> }
>>>> --
>>>> 2.43.0
>>>>
>>
On Fri, Jul 18, 2025 at 12:44:11PM -0500, Mario Limonciello wrote:
> On 7/18/2025 12:36 PM, Bjorn Helgaas wrote:
> > On Fri, Jul 18, 2025 at 12:29:05PM -0500, Mario Limonciello wrote:
> > > On 7/18/2025 12:25 PM, Bjorn Helgaas wrote:
> > > > On Thu, Jul 17, 2025 at 12:38:12PM -0500, Mario Limonciello wrote:
> > > > > From: Mario Limonciello <mario.limonciello@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 some desktop
> > > > > environments this can lead to increased power consumption because
> > > > > secondary GPUs may be used for rendering and never go to a low power
> > > > > state. 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.
> > > >
> > > > > +What: /sys/bus/pci/devices/.../boot_display
> > > > > +Date: October 2025
> > > > > +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
> > > > > +Description:
> > > > > + This file indicates that displays connected to the device were
> > > > > + used to display the boot sequence. If a display connected to
> > > > > + the device was used to display the boot sequence the file will
> > > > > + be present and contain "1".
> > > >
> > > > > 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);
> > > >
> > > > In addition to Mani's question about whether /sys/bus/pci/ is
> > > > the right place for this (which is a very good question), it's
> > > > also been pointed out to me that we've been trying to get rid
> > > > of pci_create_sysfs_dev_files() for years.
> > > >
> > > > If it's possible to make this a static attribute that would be
> > > > much, much cleaner.
> > >
> > > Right - I tried to do this, but the problem is at the time the
> > > PCI device is created the information needed to make the
> > > judgement isn't ready. The options end up being:
> > > * a sysfs file for every display device with 0/1
> > > * a sysfs file that is not accurate until later in the boot
> >
> > What's missing? The specifics might be helpful if someone has
> > another crack at getting rid of pci_create_sysfs_dev_files() in
> > the future.
>
> The underlying SCREEN_INFO code tries to walk through all the PCI
> devices in a loop, but at the time all the devices are walked the
> memory regions associated with the device weren't populated.
Which loop are you referring to that walks through all the PCI
devices? I see this:
efifb_set_system
for_each_pci_dev(dev)
but that only looks at VGA devices and IIUC you also want to look at
non-VGA GPUs.
I don't see a loop in *this* series, where the screen_info path looks
like this:
pci_create_boot_display_file
video_is_primary_device
screen_info_pci_dev # added by "fbcon: Use screen info to find primary device"
screen_info_resources
__screen_info_pci_dev
and we're basically matching the screen_info base/address with BAR
values.
The usual problem is that BARs may not have been assigned by the time
pci_device_add() -> device_add() creates the static attributes.
So we call pci_assign_unassigned_root_bus_resources() to assign all
the BARs. Then we call pci_create_sysfs_dev_files(), where
pci_create_resource_files() creates a "resource%d" file for each BAR.
But since we're trying to find the GPU that was used by BIOS, I assume
its BARs were programmed by BIOS and we shouldn't have to wait until
after pci_assign_unassigned_root_bus_resources().
Bjorn
On 7/21/25 6:00 PM, Bjorn Helgaas wrote:
> On Fri, Jul 18, 2025 at 12:44:11PM -0500, Mario Limonciello wrote:
>> On 7/18/2025 12:36 PM, Bjorn Helgaas wrote:
>>> On Fri, Jul 18, 2025 at 12:29:05PM -0500, Mario Limonciello wrote:
>>>> On 7/18/2025 12:25 PM, Bjorn Helgaas wrote:
>>>>> On Thu, Jul 17, 2025 at 12:38:12PM -0500, Mario Limonciello wrote:
>>>>>> From: Mario Limonciello <mario.limonciello@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 some desktop
>>>>>> environments this can lead to increased power consumption because
>>>>>> secondary GPUs may be used for rendering and never go to a low power
>>>>>> state. 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.
>>>>>
>>>>>> +What: /sys/bus/pci/devices/.../boot_display
>>>>>> +Date: October 2025
>>>>>> +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
>>>>>> +Description:
>>>>>> + This file indicates that displays connected to the device were
>>>>>> + used to display the boot sequence. If a display connected to
>>>>>> + the device was used to display the boot sequence the file will
>>>>>> + be present and contain "1".
>>>>>
>>>>>> 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);
>>>>>
>>>>> In addition to Mani's question about whether /sys/bus/pci/ is
>>>>> the right place for this (which is a very good question), it's
>>>>> also been pointed out to me that we've been trying to get rid
>>>>> of pci_create_sysfs_dev_files() for years.
>>>>>
>>>>> If it's possible to make this a static attribute that would be
>>>>> much, much cleaner.
>>>>
>>>> Right - I tried to do this, but the problem is at the time the
>>>> PCI device is created the information needed to make the
>>>> judgement isn't ready. The options end up being:
>>>> * a sysfs file for every display device with 0/1
>>>> * a sysfs file that is not accurate until later in the boot
>>>
>>> What's missing? The specifics might be helpful if someone has
>>> another crack at getting rid of pci_create_sysfs_dev_files() in
>>> the future.
>>
>> The underlying SCREEN_INFO code tries to walk through all the PCI
>> devices in a loop, but at the time all the devices are walked the
>> memory regions associated with the device weren't populated.
>
> Which loop are you referring to that walks through all the PCI
> devices? I see this:
>
> efifb_set_system
> for_each_pci_dev(dev)
>
> but that only looks at VGA devices and IIUC you also want to look at
> non-VGA GPUs.
>
> I don't see a loop in *this* series, where the screen_info path looks
> like this:
>
> pci_create_boot_display_file
> video_is_primary_device
> screen_info_pci_dev # added by "fbcon: Use screen info to find primary device"
> screen_info_resources
> __screen_info_pci_dev
>
> and we're basically matching the screen_info base/address with BAR
> values.
>
> The usual problem is that BARs may not have been assigned by the time
> pci_device_add() -> device_add() creates the static attributes.
>
> So we call pci_assign_unassigned_root_bus_resources() to assign all
> the BARs. Then we call pci_create_sysfs_dev_files(), where
> pci_create_resource_files() creates a "resource%d" file for each BAR.
>
> But since we're trying to find the GPU that was used by BIOS, I assume
> its BARs were programmed by BIOS and we shouldn't have to wait until
> after pci_assign_unassigned_root_bus_resources().
>
> Bjorn
Yes it was screen_info_pci_dev() and __screen_info_pci_dev(). The
resources weren't ready on the first call into __screen_info_pci_dev().
That's why the attribute needed to be created later. But the sysfs
group update or using DRM both avoid this problem and are totally fine
alternatives.
On Mon, Jul 21, 2025 at 07:28:07PM -0500, Mario Limonciello wrote:
> On 7/21/25 6:00 PM, Bjorn Helgaas wrote:
> > On Fri, Jul 18, 2025 at 12:44:11PM -0500, Mario Limonciello wrote:
> > > On 7/18/2025 12:36 PM, Bjorn Helgaas wrote:
> > > > On Fri, Jul 18, 2025 at 12:29:05PM -0500, Mario Limonciello wrote:
> > > > > On 7/18/2025 12:25 PM, Bjorn Helgaas wrote:
> > > > > > On Thu, Jul 17, 2025 at 12:38:12PM -0500, Mario Limonciello wrote:
> > > > > > > From: Mario Limonciello <mario.limonciello@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 some desktop
> > > > > > > environments this can lead to increased power consumption because
> > > > > > > secondary GPUs may be used for rendering and never go to a low power
> > > > > > > state. 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.
> > > > > >
> > > > > > > +What: /sys/bus/pci/devices/.../boot_display
> > > > > > > +Date: October 2025
> > > > > > > +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
> > > > > > > +Description:
> > > > > > > + This file indicates that displays connected to the device were
> > > > > > > + used to display the boot sequence. If a display connected to
> > > > > > > + the device was used to display the boot sequence the file will
> > > > > > > + be present and contain "1".
> > > > > >
> > > > > > > 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);
> > > > > >
> > > > > > In addition to Mani's question about whether /sys/bus/pci/ is
> > > > > > the right place for this (which is a very good question), it's
> > > > > > also been pointed out to me that we've been trying to get rid
> > > > > > of pci_create_sysfs_dev_files() for years.
> > > > > >
> > > > > > If it's possible to make this a static attribute that would be
> > > > > > much, much cleaner.
> > > > >
> > > > > Right - I tried to do this, but the problem is at the time the
> > > > > PCI device is created the information needed to make the
> > > > > judgement isn't ready. The options end up being:
> > > > > * a sysfs file for every display device with 0/1
> > > > > * a sysfs file that is not accurate until later in the boot
> > > >
> > > > What's missing? The specifics might be helpful if someone has
> > > > another crack at getting rid of pci_create_sysfs_dev_files() in
> > > > the future.
> > >
> > > The underlying SCREEN_INFO code tries to walk through all the PCI
> > > devices in a loop, but at the time all the devices are walked the
> > > memory regions associated with the device weren't populated.
> >
> > Which loop are you referring to that walks through all the PCI
> > devices? I see this:
> >
> > efifb_set_system
> > for_each_pci_dev(dev)
> >
> > but that only looks at VGA devices and IIUC you also want to look at
> > non-VGA GPUs.
[I assume the loop is the "while (pdev =
pci_get_base_class(PCI_BASE_CLASS_DISPLAY))" in
__screen_info_pci_dev(), which indeed walks through all known PCI
devices]
> > I don't see a loop in *this* series, where the screen_info path looks
> > like this:
> >
> > pci_create_boot_display_file
> > video_is_primary_device
> > screen_info_pci_dev # added by "fbcon: Use screen info to find primary device"
> > screen_info_resources
> > __screen_info_pci_dev
> >
> > and we're basically matching the screen_info base/address with BAR
> > values.
> >
> > The usual problem is that BARs may not have been assigned by the
> > time pci_device_add() -> device_add() creates the static
> > attributes.
> >
> > So we call pci_assign_unassigned_root_bus_resources() to assign
> > all the BARs. Then we call pci_create_sysfs_dev_files(), where
> > pci_create_resource_files() creates a "resource%d" file for each
> > BAR.
> >
> > But since we're trying to find the GPU that was used by BIOS, I
> > assume its BARs were programmed by BIOS and we shouldn't have to
> > wait until after pci_assign_unassigned_root_bus_resources().
>
> Yes it was screen_info_pci_dev() and __screen_info_pci_dev(). The
> resources weren't ready on the first call into
> __screen_info_pci_dev().
>
> That's why the attribute needed to be created later.
I don't understand this. IIUC, screen_info contains addresses
programmed by BIOS. If we want to use that to match with a PCI
device, we have to compare with the BAR contents *before* Linux does
any assignments of its own.
So the only thing this should depend on is the BAR value at BIOS ->
Linux handoff, which we know at the time of device_add(), and we
should be able to do something like this:
bool pci_video_is_primary_device(struct pci_dev *pdev)
{
struct screen_info *si = &screen_info;
struct resource res[SCREEN_INFO_MAX_RESOURCES];
ssize_t i, numres;
numres = screen_info_resources(si, res, ARRAY_SIZE(res));
...
for (i = 0; i < numres; ++i) {
if (pci_find_resource(pdev, &res[i]))
return true;
}
return false;
}
static umode_t pci_dev_boot_display_is_visible(...)
{
struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
if (pci_video_is_primary_device(pdev))
return a->mode;
return 0;
}
We should be able to check each BAR of each device in this path, with
no loop through the devices at all:
pci_device_add
device_add
device_add_attrs
device_add_groups
...
create_files
grp->is_visible()
pci_dev_boot_display_is_visible
Bjorn
On 7/21/25 8:59 PM, Bjorn Helgaas wrote:
> On Mon, Jul 21, 2025 at 07:28:07PM -0500, Mario Limonciello wrote:
>> On 7/21/25 6:00 PM, Bjorn Helgaas wrote:
>>> On Fri, Jul 18, 2025 at 12:44:11PM -0500, Mario Limonciello wrote:
>>>> On 7/18/2025 12:36 PM, Bjorn Helgaas wrote:
>>>>> On Fri, Jul 18, 2025 at 12:29:05PM -0500, Mario Limonciello wrote:
>>>>>> On 7/18/2025 12:25 PM, Bjorn Helgaas wrote:
>>>>>>> On Thu, Jul 17, 2025 at 12:38:12PM -0500, Mario Limonciello wrote:
>>>>>>>> From: Mario Limonciello <mario.limonciello@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 some desktop
>>>>>>>> environments this can lead to increased power consumption because
>>>>>>>> secondary GPUs may be used for rendering and never go to a low power
>>>>>>>> state. 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.
>>>>>>>
>>>>>>>> +What: /sys/bus/pci/devices/.../boot_display
>>>>>>>> +Date: October 2025
>>>>>>>> +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
>>>>>>>> +Description:
>>>>>>>> + This file indicates that displays connected to the device were
>>>>>>>> + used to display the boot sequence. If a display connected to
>>>>>>>> + the device was used to display the boot sequence the file will
>>>>>>>> + be present and contain "1".
>>>>>>>
>>>>>>>> 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);
>>>>>>>
>>>>>>> In addition to Mani's question about whether /sys/bus/pci/ is
>>>>>>> the right place for this (which is a very good question), it's
>>>>>>> also been pointed out to me that we've been trying to get rid
>>>>>>> of pci_create_sysfs_dev_files() for years.
>>>>>>>
>>>>>>> If it's possible to make this a static attribute that would be
>>>>>>> much, much cleaner.
>>>>>>
>>>>>> Right - I tried to do this, but the problem is at the time the
>>>>>> PCI device is created the information needed to make the
>>>>>> judgement isn't ready. The options end up being:
>>>>>> * a sysfs file for every display device with 0/1
>>>>>> * a sysfs file that is not accurate until later in the boot
>>>>>
>>>>> What's missing? The specifics might be helpful if someone has
>>>>> another crack at getting rid of pci_create_sysfs_dev_files() in
>>>>> the future.
>>>>
>>>> The underlying SCREEN_INFO code tries to walk through all the PCI
>>>> devices in a loop, but at the time all the devices are walked the
>>>> memory regions associated with the device weren't populated.
>>>
>>> Which loop are you referring to that walks through all the PCI
>>> devices? I see this:
>>>
>>> efifb_set_system
>>> for_each_pci_dev(dev)
>>>
>>> but that only looks at VGA devices and IIUC you also want to look at
>>> non-VGA GPUs.
>
> [I assume the loop is the "while (pdev =
> pci_get_base_class(PCI_BASE_CLASS_DISPLAY))" in
> __screen_info_pci_dev(), which indeed walks through all known PCI
> devices]
>
>>> I don't see a loop in *this* series, where the screen_info path looks
>>> like this:
>>>
>>> pci_create_boot_display_file
>>> video_is_primary_device
>>> screen_info_pci_dev # added by "fbcon: Use screen info to find primary device"
>>> screen_info_resources
>>> __screen_info_pci_dev
>>>
>>> and we're basically matching the screen_info base/address with BAR
>>> values.
>>>
>>> The usual problem is that BARs may not have been assigned by the
>>> time pci_device_add() -> device_add() creates the static
>>> attributes.
>>>
>>> So we call pci_assign_unassigned_root_bus_resources() to assign
>>> all the BARs. Then we call pci_create_sysfs_dev_files(), where
>>> pci_create_resource_files() creates a "resource%d" file for each
>>> BAR.
>>>
>>> But since we're trying to find the GPU that was used by BIOS, I
>>> assume its BARs were programmed by BIOS and we shouldn't have to
>>> wait until after pci_assign_unassigned_root_bus_resources().
>>
>> Yes it was screen_info_pci_dev() and __screen_info_pci_dev(). The
>> resources weren't ready on the first call into
>> __screen_info_pci_dev().
>>
>> That's why the attribute needed to be created later.
>
> I don't understand this. IIUC, screen_info contains addresses
> programmed by BIOS. If we want to use that to match with a PCI
> device, we have to compare with the BAR contents *before* Linux does
> any assignments of its own.
>
> So the only thing this should depend on is the BAR value at BIOS ->
> Linux handoff, which we know at the time of device_add(), and we
> should be able to do something like this:
>
> bool pci_video_is_primary_device(struct pci_dev *pdev)
> {
> struct screen_info *si = &screen_info;
> struct resource res[SCREEN_INFO_MAX_RESOURCES];
> ssize_t i, numres;
>
> numres = screen_info_resources(si, res, ARRAY_SIZE(res));
> ...
>
> for (i = 0; i < numres; ++i) {
> if (pci_find_resource(pdev, &res[i]))
> return true;
> }
>
> return false;
> }
>
> static umode_t pci_dev_boot_display_is_visible(...)
> {
> struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
>
> if (pci_video_is_primary_device(pdev))
> return a->mode;
>
> return 0;
> }
>
> We should be able to check each BAR of each device in this path, with
> no loop through the devices at all:
>
> pci_device_add
> device_add
> device_add_attrs
> device_add_groups
> ...
> create_files
> grp->is_visible()
> pci_dev_boot_display_is_visible
>
> Bjorn
You're spot on, I did a test and this works. I'll clean it up and put
it on the list and we can decide between this way and moving to drm.
On Fri, Jul 18, 2025 at 12:44:11PM -0500, Mario Limonciello wrote: > On 7/18/2025 12:36 PM, Bjorn Helgaas wrote: > > On Fri, Jul 18, 2025 at 12:29:05PM -0500, Mario Limonciello wrote: > > > On 7/18/2025 12:25 PM, Bjorn Helgaas wrote: > > > > In addition to Mani's question about whether /sys/bus/pci/ is the > > > > right place for this (which is a very good question), it's also been > > > > pointed out to me that we've been trying to get rid of > > > > pci_create_sysfs_dev_files() for years. > > > > > > > > If it's possible to make this a static attribute that would be much, > > > > much cleaner. > > > > > > Right - I tried to do this, but the problem is at the time the PCI device is > > > created the information needed to make the judgement isn't ready. The > > > options end up being: > > > * a sysfs file for every display device with 0/1 > > > * a sysfs file that is not accurate until later in the boot > > > > What's missing? The specifics might be helpful if someone has another > > crack at getting rid of pci_create_sysfs_dev_files() in the future. > > The underlying SCREEN_INFO code tries to walk through all the PCI devices in > a loop, but at the time all the devices are walked the memory regions > associated with the device weren't populated. > > So my earlier hack was to re-run the screen info check, and it was awful. Well have you explored the sysfs_update_group() approach you mentioned earlier? https://lore.kernel.org/r/5cc01163-1feb-4a18-8060-27f4da39b2e4@kernel.org/ Thanks, Lukas
© 2016 - 2025 Red Hat, Inc.