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

[Mario Limonciello <superm1@kernel.org>]

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
>>>>
>>