1. Patchew
  2. linux
  3. [PATCH v6 0/9] Adjust fbcon console device detection
  4. View patch

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

Mario Limonciello posted 9 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
Posted by Mario Limonciello 3 months, 1 week ago 
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 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@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 +++++++++++++++++++++++++
 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@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);
 }
 
-- 
2.43.0
Re: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
Posted by kernel test robot 3 months, 1 week ago 
Hi Mario,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus tiwai-sound/for-next tiwai-sound/for-linus tip/x86/core linus/master v6.16-rc3 next-20250627]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PCI-Add-helper-for-checking-if-a-PCI-device-is-a-display-controller/20250627-123349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20250627043108.3141206-10-superm1%40kernel.org
patch subject: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
config: powerpc-bluestone_defconfig (https://download.01.org/0day-ci/archive/20250628/202506282240.aqA0j5M3-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project e04c938cc08a90ae60440ce22d072ebc69d67ee8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250628/202506282240.aqA0j5M3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506282240.aqA0j5M3-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pci/pci-sysfs.c:688:8: warning: unused variable 'dev_attr_boot_display' [-Wunused-variable]
     688 | static DEVICE_ATTR_RO(boot_display);
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/device.h:199:26: note: expanded from macro 'DEVICE_ATTR_RO'
     199 |         struct device_attribute dev_attr_##_name = __ATTR_RO(_name)
         |                                 ^~~~~~~~~~~~~~~~
   <scratch space>:54:1: note: expanded from here
      54 | dev_attr_boot_display
         | ^~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +/dev_attr_boot_display +688 drivers/pci/pci-sysfs.c

   682	
   683	static ssize_t boot_display_show(struct device *dev, struct device_attribute *attr,
   684					 char *buf)
   685	{
   686		return sysfs_emit(buf, "1\n");
   687	}
 > 688	static DEVICE_ATTR_RO(boot_display);
   689	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
Posted by kernel test robot 3 months, 1 week ago 
Hi Mario,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus tiwai-sound/for-next tiwai-sound/for-linus tip/x86/core linus/master v6.16-rc3 next-20250627]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PCI-Add-helper-for-checking-if-a-PCI-device-is-a-display-controller/20250627-123349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20250627043108.3141206-10-superm1%40kernel.org
patch subject: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
config: arc-randconfig-001-20250628 (https://download.01.org/0day-ci/archive/20250628/202506281332.JQb144bv-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250628/202506281332.JQb144bv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506281332.JQb144bv-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   drivers/pci/pci-sysfs.c: In function 'pci_create_sysfs_dev_files':
>> drivers/pci/pci-sysfs.c:1701:11: error: implicit declaration of function 'pci_create_boot_display_file'; did you mean 'pci_create_sysfs_dev_files'? [-Werror=implicit-function-declaration]
     retval = pci_create_boot_display_file(pdev);
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
              pci_create_sysfs_dev_files
   drivers/pci/pci-sysfs.c: In function 'pci_remove_sysfs_dev_files':
>> drivers/pci/pci-sysfs.c:1719:2: error: implicit declaration of function 'pci_remove_boot_display_file'; did you mean 'pci_remove_sysfs_dev_files'? [-Werror=implicit-function-declaration]
     pci_remove_boot_display_file(pdev);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
     pci_remove_sysfs_dev_files
   In file included from include/linux/pci.h:37,
                    from drivers/pci/pci-sysfs.c:19:
   At top level:
>> include/linux/device.h:199:26: warning: 'dev_attr_boot_display' defined but not used [-Wunused-variable]
     struct device_attribute dev_attr_##_name = __ATTR_RO(_name)
                             ^~~~~~~~~
   drivers/pci/pci-sysfs.c:688:8: note: in expansion of macro 'DEVICE_ATTR_RO'
    static DEVICE_ATTR_RO(boot_display);
           ^~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +1701 drivers/pci/pci-sysfs.c

  1693	
  1694	int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
  1695	{
  1696		int retval;
  1697	
  1698		if (!sysfs_initialized)
  1699			return -EACCES;
  1700	
> 1701		retval = pci_create_boot_display_file(pdev);
  1702		if (retval)
  1703			return retval;
  1704	
  1705		return pci_create_resource_files(pdev);
  1706	}
  1707	
  1708	/**
  1709	 * pci_remove_sysfs_dev_files - cleanup PCI specific sysfs files
  1710	 * @pdev: device whose entries we should free
  1711	 *
  1712	 * Cleanup when @pdev is removed from sysfs.
  1713	 */
  1714	void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
  1715	{
  1716		if (!sysfs_initialized)
  1717			return;
  1718	
> 1719		pci_remove_boot_display_file(pdev);
  1720		pci_remove_resource_files(pdev);
  1721	}
  1722	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
Posted by Thomas Zimmermann 3 months, 1 week ago 
Hi

Am 27.06.25 um 06:31 schrieb Mario Limonciello:
> 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 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@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.

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@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)
Re: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
Posted by Mario Limonciello 3 months, 1 week ago 
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@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@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.

> 
> 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@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);
>>   }
>
Re: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
Posted by Thomas Zimmermann 3 months, 1 week ago 
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@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@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@suse.de>

I guess that interface also needs some sort of OK from user-space devs?

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@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)
Re: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
Posted by Mario Limonciello 3 months, 1 week ago 
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@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@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@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.

> 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@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);
>>>>   }
>>>
>>
>
Re: [PATCH v6 9/9] PCI: Add a new 'boot_display' attribute
Posted by Thomas Zimmermann 3 months, 1 week ago 
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@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@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@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@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)

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.