[PATCH v9 0/9] Adjust fbcon console device detection

[PATCH v9 0/9] Adjust fbcon console device detection

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

Systems with more than one GPU userspace doesn't know which one to be
used to treat as primary.  The concept of primary is important to be
able to decide which GPU is used for display and  which is used for
rendering.  If it's guessed wrong then both GPUs will be kept awake
burning a lot of power.

Historically it would use the "boot_vga" attribute but this isn't
present on modern GPUs.

This series started out as changes to VGA arbiter to try to handle a case
of a system with 2 GPUs that are not VGA devices and avoid changes to
userspace.  This was discussed but decided not to overload the VGA arbiter
for non VGA devices.

Instead move the x86 specific detection of framebuffer resources into x86
specific code that the fbcon can use to properly identify the primary
device. This code is still called from the VGA arbiter, and the logic does
not change there. To avoid regression default to VGA arbiter and only fall
back to looking up with x86 specific detection method.

In order for userspace to also be able to discover which device was the
primary video display device create a new sysfs file 'boot_display'.

A matching userspace implementation for this file is available here:
Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/merge_requests/39
Link: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2038

Dave Airlie has been pinged for a comment on this approach.
Dave had suggested in the past [1]:

"
 But yes if that doesn't work, then maybe we need to make the boot_vga
 flag mean boot_display_gpu, and fix it in the kernel
"

This was one of the approached tried in earlier revisions and it was
rejected in favor of creating a new sysfs file (which is what this
version does).

It is suggested that this series merge entirely through the PCI tree.

Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/merge_requests/37#note_2938602 [1]

v9:
 * Add more to cover letter
 * Add bug link to last patch
 * Update commit message for last patch
 * Update boot_display documentation description

Mario Limonciello (9):
  PCI: Add helper for checking if a PCI device is a display controller
  vfio/pci: Use pci_is_display()
  vga_switcheroo: Use pci_is_display()
  iommu/vt-d: Use pci_is_display()
  ALSA: hda: Use pci_is_display()
  Fix access to video_is_primary_device() when compiled without
    CONFIG_VIDEO
  PCI/VGA: Replace vga_is_firmware_default() with a screen info check
  fbcon: Use screen info to find primary device
  PCI: Add a new 'boot_display' attribute

 Documentation/ABI/testing/sysfs-bus-pci |  9 +++++
 arch/parisc/include/asm/video.h         |  2 +-
 arch/sparc/include/asm/video.h          |  2 ++
 arch/x86/include/asm/video.h            |  2 ++
 arch/x86/video/video-common.c           | 17 ++++++++-
 drivers/gpu/vga/vga_switcheroo.c        |  2 +-
 drivers/iommu/intel/iommu.c             |  2 +-
 drivers/pci/pci-sysfs.c                 | 46 +++++++++++++++++++++++++
 drivers/pci/vgaarb.c                    | 31 +++--------------
 drivers/vfio/pci/vfio_pci_igd.c         |  3 +-
 include/linux/pci.h                     | 15 ++++++++
 sound/hda/hdac_i915.c                   |  2 +-
 sound/pci/hda/hda_intel.c               |  4 +--
 13 files changed, 102 insertions(+), 35 deletions(-)

-- 
2.43.0

[PATCH v9 1/9] PCI: Add helper for checking if a PCI device is a display controller

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

Several places in the kernel do class shifting to match whether a
PCI device is display class.  Introduce a helper for those places to
use.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Daniel Dadap <ddadap@nvidia.com>
Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 include/linux/pci.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 05e68f35f3923..e77754e43c629 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -744,6 +744,21 @@ static inline bool pci_is_vga(struct pci_dev *pdev)
 	return false;
 }
 
+/**
+ * pci_is_display - Check if a PCI device is a display controller
+ * @pdev: Pointer to the PCI device structure
+ *
+ * This function determines whether the given PCI device corresponds
+ * to a display controller. Display controllers are typically used
+ * for graphical output and are identified based on their class code.
+ *
+ * Return: true if the PCI device is a display controller, false otherwise.
+ */
+static inline bool pci_is_display(struct pci_dev *pdev)
+{
+	return (pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY;
+}
+
 #define for_each_pci_bridge(dev, bus)				\
 	list_for_each_entry(dev, &bus->devices, bus_list)	\
 		if (!pci_is_bridge(dev)) {} else
-- 
2.43.0

[PATCH v9 2/9] vfio/pci: Use pci_is_display()

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

The inline pci_is_display() helper does the same thing.  Use it.

Acked-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Daniel Dadap <ddadap@nvidia.com>
Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/vfio/pci/vfio_pci_igd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c
index ef490a4545f48..988b6919c2c31 100644
--- a/drivers/vfio/pci/vfio_pci_igd.c
+++ b/drivers/vfio/pci/vfio_pci_igd.c
@@ -437,8 +437,7 @@ static int vfio_pci_igd_cfg_init(struct vfio_pci_core_device *vdev)
 
 bool vfio_pci_is_intel_display(struct pci_dev *pdev)
 {
-	return (pdev->vendor == PCI_VENDOR_ID_INTEL) &&
-	       ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY);
+	return (pdev->vendor == PCI_VENDOR_ID_INTEL) && pci_is_display(pdev);
 }
 
 int vfio_pci_igd_init(struct vfio_pci_core_device *vdev)
-- 
2.43.0

[PATCH v9 3/9] vga_switcheroo: Use pci_is_display()

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

The inline pci_is_display() helper does the same thing.  Use it.

Reviewed-by: Daniel Dadap <ddadap@nvidia.com>
Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/vga/vga_switcheroo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 18f2c92beff8e..68e45a26e85f7 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -437,7 +437,7 @@ find_active_client(struct list_head *head)
  */
 bool vga_switcheroo_client_probe_defer(struct pci_dev *pdev)
 {
-	if ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
+	if (pci_is_display(pdev)) {
 		/*
 		 * apple-gmux is needed on pre-retina MacBook Pro
 		 * to probe the panel if pdev is the inactive GPU.
-- 
2.43.0

[PATCH v9 4/9] iommu/vt-d: Use pci_is_display()

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

The inline pci_is_display() helper does the same thing.  Use it.

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Daniel Dadap <ddadap@nvidia.com>
Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/iommu/intel/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 148b944143b81..cad9ed1016cfc 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -34,7 +34,7 @@
 #define ROOT_SIZE		VTD_PAGE_SIZE
 #define CONTEXT_SIZE		VTD_PAGE_SIZE
 
-#define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
+#define IS_GFX_DEVICE(pdev) pci_is_display(pdev)
 #define IS_USB_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_SERIAL_USB)
 #define IS_ISA_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA)
 #define IS_AZALIA(pdev) ((pdev)->vendor == 0x8086 && (pdev)->device == 0x3a3e)
-- 
2.43.0

[PATCH v9 5/9] ALSA: hda: Use pci_is_display()

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

The inline pci_is_display() helper does the same thing.  Use it.

Reviewed-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Daniel Dadap <ddadap@nvidia.com>
Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 sound/hda/hdac_i915.c     | 2 +-
 sound/pci/hda/hda_intel.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index e9425213320ea..44438c799f957 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -155,7 +155,7 @@ static int i915_gfx_present(struct pci_dev *hdac_pci)
 
 	for_each_pci_dev(display_dev) {
 		if (display_dev->vendor != PCI_VENDOR_ID_INTEL ||
-		    (display_dev->class >> 16) != PCI_BASE_CLASS_DISPLAY)
+		    !pci_is_display(display_dev))
 			continue;
 
 		if (pci_match_id(denylist, display_dev))
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 439cf1bda6e66..75badb5c69b8e 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1465,7 +1465,7 @@ static struct pci_dev *get_bound_vga(struct pci_dev *pci)
 				 * the dGPU is the one who is involved in
 				 * vgaswitcheroo.
 				 */
-				if (((p->class >> 16) == PCI_BASE_CLASS_DISPLAY) &&
+				if (pci_is_display(p) &&
 				    (atpx_present() || apple_gmux_detect(NULL, NULL)))
 					return p;
 				pci_dev_put(p);
@@ -1477,7 +1477,7 @@ static struct pci_dev *get_bound_vga(struct pci_dev *pci)
 			p = pci_get_domain_bus_and_slot(pci_domain_nr(pci->bus),
 							pci->bus->number, 0);
 			if (p) {
-				if ((p->class >> 16) == PCI_BASE_CLASS_DISPLAY)
+				if (pci_is_display(p))
 					return p;
 				pci_dev_put(p);
 			}
-- 
2.43.0

[PATCH v9 6/9] Fix access to video_is_primary_device() when compiled without CONFIG_VIDEO

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

When compiled without CONFIG_VIDEO the architecture specific
implementations of video_is_primary_device() include prototypes and
assume that video-common.c will be linked. Guard against this so that the
fallback inline implementation that returns false will be used when
compiled without CONFIG_VIDEO.

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506221312.49Fy1aNA-lkp@intel.com/
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v5:
 * add tag
v4:
 * new patch
---
 arch/parisc/include/asm/video.h | 2 +-
 arch/sparc/include/asm/video.h  | 2 ++
 arch/x86/include/asm/video.h    | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/parisc/include/asm/video.h b/arch/parisc/include/asm/video.h
index c5dff3223194a..a9d50ebd6e769 100644
--- a/arch/parisc/include/asm/video.h
+++ b/arch/parisc/include/asm/video.h
@@ -6,7 +6,7 @@
 
 struct device;
 
-#if defined(CONFIG_STI_CORE)
+#if defined(CONFIG_STI_CORE) && defined(CONFIG_VIDEO)
 bool video_is_primary_device(struct device *dev);
 #define video_is_primary_device video_is_primary_device
 #endif
diff --git a/arch/sparc/include/asm/video.h b/arch/sparc/include/asm/video.h
index a6f48f52db584..773717b6d4914 100644
--- a/arch/sparc/include/asm/video.h
+++ b/arch/sparc/include/asm/video.h
@@ -19,8 +19,10 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
 #define pgprot_framebuffer pgprot_framebuffer
 #endif
 
+#ifdef CONFIG_VIDEO
 bool video_is_primary_device(struct device *dev);
 #define video_is_primary_device video_is_primary_device
+#endif
 
 static inline void fb_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
 {
diff --git a/arch/x86/include/asm/video.h b/arch/x86/include/asm/video.h
index 0950c9535fae9..08ec328203ef8 100644
--- a/arch/x86/include/asm/video.h
+++ b/arch/x86/include/asm/video.h
@@ -13,8 +13,10 @@ pgprot_t pgprot_framebuffer(pgprot_t prot,
 			    unsigned long offset);
 #define pgprot_framebuffer pgprot_framebuffer
 
+#ifdef CONFIG_VIDEO
 bool video_is_primary_device(struct device *dev);
 #define video_is_primary_device video_is_primary_device
+#endif
 
 #include <asm-generic/video.h>
 
-- 
2.43.0

[PATCH v9 7/9] PCI/VGA: Replace vga_is_firmware_default() with a screen info check

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

vga_is_firmware_default() checks firmware resources to find the owner
framebuffer resources to find the firmware PCI device.  This is an
open coded implementation of screen_info_pci_dev().  Switch to using
screen_info_pci_dev() instead.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v6:
 * fix lkp robot error
v5:
 * split from next patch
---
 drivers/pci/vgaarb.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 78748e8d2dbae..b58f94ee48916 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -556,34 +556,13 @@ EXPORT_SYMBOL(vga_put);
 
 static bool vga_is_firmware_default(struct pci_dev *pdev)
 {
-#if defined(CONFIG_X86)
-	u64 base = screen_info.lfb_base;
-	u64 size = screen_info.lfb_size;
-	struct resource *r;
-	u64 limit;
+#ifdef CONFIG_SCREEN_INFO
+	struct screen_info *si = &screen_info;
 
-	/* Select the device owning the boot framebuffer if there is one */
-
-	if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
-		base |= (u64)screen_info.ext_lfb_base << 32;
-
-	limit = base + size;
-
-	/* Does firmware framebuffer belong to us? */
-	pci_dev_for_each_resource(pdev, r) {
-		if (resource_type(r) != IORESOURCE_MEM)
-			continue;
-
-		if (!r->start || !r->end)
-			continue;
-
-		if (base < r->start || limit >= r->end)
-			continue;
-
-		return true;
-	}
-#endif
+	return pdev == screen_info_pci_dev(si);
+#else
 	return false;
+#endif
 }
 
 static bool vga_arb_integrated_gpu(struct device *dev)
-- 
2.43.0

[PATCH v9 8/9] fbcon: Use screen info to find primary device

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

On systems with non VGA GPUs fbcon can't find the primary GPU because
video_is_primary_device() only checks the VGA arbiter.

Add a screen info check to video_is_primary_device() so that callers
can get accurate data on such systems.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v8:
 * add guards for the non CONFIG_SCREEN_INFO case
v5:
 * Only change video-common.c
v4:
 * use helper
---
 arch/x86/video/video-common.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
index 81fc97a2a837a..4bbfffec4b640 100644
--- a/arch/x86/video/video-common.c
+++ b/arch/x86/video/video-common.c
@@ -9,6 +9,7 @@
 
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/screen_info.h>
 #include <linux/vgaarb.h>
 
 #include <asm/video.h>
@@ -27,6 +28,9 @@ EXPORT_SYMBOL(pgprot_framebuffer);
 
 bool video_is_primary_device(struct device *dev)
 {
+#ifdef CONFIG_SCREEN_INFO
+	struct screen_info *si = &screen_info;
+#endif
 	struct pci_dev *pdev;
 
 	if (!dev_is_pci(dev))
@@ -34,7 +38,18 @@ bool video_is_primary_device(struct device *dev)
 
 	pdev = to_pci_dev(dev);
 
-	return (pdev == vga_default_device());
+	if (!pci_is_display(pdev))
+		return false;
+
+	if (pdev == vga_default_device())
+		return true;
+
+#ifdef CONFIG_SCREEN_INFO
+	if (pdev == screen_info_pci_dev(si))
+		return true;
+#endif
+
+	return false;
 }
 EXPORT_SYMBOL(video_is_primary_device);
 
-- 
2.43.0

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

[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 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.0

Re: [PATCH v9 0/9] Adjust fbcon console device detection

[Bjorn Helgaas]

On Thu, Jul 17, 2025 at 12:38:03PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> Systems with more than one GPU userspace doesn't know which one to be
> used to treat as primary.  The concept of primary is important to be
> able to decide which GPU is used for display and  which is used for
> rendering.  If it's guessed wrong then both GPUs will be kept awake
> burning a lot of power.
> 
> Historically it would use the "boot_vga" attribute but this isn't
> present on modern GPUs.
> 
> This series started out as changes to VGA arbiter to try to handle a case
> of a system with 2 GPUs that are not VGA devices and avoid changes to
> userspace.  This was discussed but decided not to overload the VGA arbiter
> for non VGA devices.
> 
> Instead move the x86 specific detection of framebuffer resources into x86
> specific code that the fbcon can use to properly identify the primary
> device. This code is still called from the VGA arbiter, and the logic does
> not change there. To avoid regression default to VGA arbiter and only fall
> back to looking up with x86 specific detection method.
> 
> In order for userspace to also be able to discover which device was the
> primary video display device create a new sysfs file 'boot_display'.
> 
> A matching userspace implementation for this file is available here:
> Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/merge_requests/39
> Link: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2038
> 
> Dave Airlie has been pinged for a comment on this approach.
> Dave had suggested in the past [1]:
> 
> "
>  But yes if that doesn't work, then maybe we need to make the boot_vga
>  flag mean boot_display_gpu, and fix it in the kernel
> "
> 
> This was one of the approached tried in earlier revisions and it was
> rejected in favor of creating a new sysfs file (which is what this
> version does).
> 
> It is suggested that this series merge entirely through the PCI tree.
> 
> Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/merge_requests/37#note_2938602 [1]
> 
> v9:
>  * Add more to cover letter
>  * Add bug link to last patch
>  * Update commit message for last patch
>  * Update boot_display documentation description
> 
> Mario Limonciello (9):
>   PCI: Add helper for checking if a PCI device is a display controller
>   vfio/pci: Use pci_is_display()
>   vga_switcheroo: Use pci_is_display()
>   iommu/vt-d: Use pci_is_display()
>   ALSA: hda: Use pci_is_display()
>   Fix access to video_is_primary_device() when compiled without
>     CONFIG_VIDEO
>   PCI/VGA: Replace vga_is_firmware_default() with a screen info check
>   fbcon: Use screen info to find primary device
>   PCI: Add a new 'boot_display' attribute
> 
>  Documentation/ABI/testing/sysfs-bus-pci |  9 +++++
>  arch/parisc/include/asm/video.h         |  2 +-
>  arch/sparc/include/asm/video.h          |  2 ++
>  arch/x86/include/asm/video.h            |  2 ++
>  arch/x86/video/video-common.c           | 17 ++++++++-
>  drivers/gpu/vga/vga_switcheroo.c        |  2 +-
>  drivers/iommu/intel/iommu.c             |  2 +-
>  drivers/pci/pci-sysfs.c                 | 46 +++++++++++++++++++++++++
>  drivers/pci/vgaarb.c                    | 31 +++--------------
>  drivers/vfio/pci/vfio_pci_igd.c         |  3 +-
>  include/linux/pci.h                     | 15 ++++++++
>  sound/hda/hdac_i915.c                   |  2 +-
>  sound/pci/hda/hda_intel.c               |  4 +--
>  13 files changed, 102 insertions(+), 35 deletions(-)

Applied to pci/boot-display for v6.17, thanks!

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

[Bjorn Helgaas]

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.

> +	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 v9 9/9] PCI: Add a new 'boot_display' attribute

[Mario Limonciello]

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

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

[Bjorn Helgaas]

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

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

[Mario Limonciello]

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

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

[Lukas Wunner]

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

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

[Bjorn Helgaas]

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

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

[Mario Limonciello]

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.

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

[Bjorn Helgaas]

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

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

[Mario Limonciello]

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.

Re: [PATCH v9 8/9] fbcon: Use screen info to find primary device

[Bjorn Helgaas]

On Thu, Jul 17, 2025 at 12:38:11PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> On systems with non VGA GPUs fbcon can't find the primary GPU because
> video_is_primary_device() only checks the VGA arbiter.
> 
> Add a screen info check to video_is_primary_device() so that callers
> can get accurate data on such systems.

This relies on screen_info, which I think is an x86 BIOS-ism.  Isn't
there a UEFI console path?  How does that compare with this?  Is that
relevant or is it something completely different?

>  bool video_is_primary_device(struct device *dev)
>  {
> +#ifdef CONFIG_SCREEN_INFO
> +	struct screen_info *si = &screen_info;
> +#endif
>  	struct pci_dev *pdev;
>  
>  	if (!dev_is_pci(dev))
> @@ -34,7 +38,18 @@ bool video_is_primary_device(struct device *dev)
>  
>  	pdev = to_pci_dev(dev);
>  
> -	return (pdev == vga_default_device());
> +	if (!pci_is_display(pdev))
> +		return false;
> +
> +	if (pdev == vga_default_device())
> +		return true;
> +
> +#ifdef CONFIG_SCREEN_INFO
> +	if (pdev == screen_info_pci_dev(si))
> +		return true;
> +#endif
> +
> +	return false;
>  }
>  EXPORT_SYMBOL(video_is_primary_device);
>  
> -- 
> 2.43.0
> 

Re: [PATCH v9 8/9] fbcon: Use screen info to find primary device

[Mario Limonciello]

On 7/22/25 9:38 AM, Bjorn Helgaas wrote:
> On Thu, Jul 17, 2025 at 12:38:11PM -0500, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> On systems with non VGA GPUs fbcon can't find the primary GPU because
>> video_is_primary_device() only checks the VGA arbiter.
>>
>> Add a screen info check to video_is_primary_device() so that callers
>> can get accurate data on such systems.
> 
> This relies on screen_info, which I think is an x86 BIOS-ism.  Isn't
> there a UEFI console path?  How does that compare with this?  Is that
> relevant or is it something completely different?

When I created and tested this I actually did this on a UEFI system 
(which provides a UEFI GOP driver).
  >
>>   bool video_is_primary_device(struct device *dev)
>>   {
>> +#ifdef CONFIG_SCREEN_INFO
>> +	struct screen_info *si = &screen_info;
>> +#endif
>>   	struct pci_dev *pdev;
>>   
>>   	if (!dev_is_pci(dev))
>> @@ -34,7 +38,18 @@ bool video_is_primary_device(struct device *dev)
>>   
>>   	pdev = to_pci_dev(dev);
>>   
>> -	return (pdev == vga_default_device());
>> +	if (!pci_is_display(pdev))
>> +		return false;
>> +
>> +	if (pdev == vga_default_device())
>> +		return true;
>> +
>> +#ifdef CONFIG_SCREEN_INFO
>> +	if (pdev == screen_info_pci_dev(si))
>> +		return true;
>> +#endif
>> +
>> +	return false;
>>   }
>>   EXPORT_SYMBOL(video_is_primary_device);
>>   
>> -- 
>> 2.43.0
>>

Re: [PATCH v9 8/9] fbcon: Use screen info to find primary device

[Bjorn Helgaas]

On Tue, Jul 22, 2025 at 09:45:28AM -0500, Mario Limonciello wrote:
> On 7/22/25 9:38 AM, Bjorn Helgaas wrote:
> > On Thu, Jul 17, 2025 at 12:38:11PM -0500, Mario Limonciello wrote:
> > > From: Mario Limonciello <mario.limonciello@amd.com>
> > > 
> > > On systems with non VGA GPUs fbcon can't find the primary GPU because
> > > video_is_primary_device() only checks the VGA arbiter.
> > > 
> > > Add a screen info check to video_is_primary_device() so that callers
> > > can get accurate data on such systems.
> > 
> > This relies on screen_info, which I think is an x86 BIOS-ism.  Isn't
> > there a UEFI console path?  How does that compare with this?  Is that
> > relevant or is it something completely different?
> 
> When I created and tested this I actually did this on a UEFI system (which
> provides a UEFI GOP driver).

I guess screen_info is actually *not* an x86 BIOS-ism, and on UEFI
systems, we do actually rely on UEFI, e.g., in efi_setup_gop(),
alloc_screen_info(), init_screen_info()?

But this patch is x86-specific, so I'm guessing the same problem could
occur on arm64, Loongson, or other UEFI platforms, and this series
doesn't address those?

> > >   bool video_is_primary_device(struct device *dev)
> > >   {
> > > +#ifdef CONFIG_SCREEN_INFO
> > > +	struct screen_info *si = &screen_info;
> > > +#endif
> > >   	struct pci_dev *pdev;
> > >   	if (!dev_is_pci(dev))
> > > @@ -34,7 +38,18 @@ bool video_is_primary_device(struct device *dev)
> > >   	pdev = to_pci_dev(dev);
> > > -	return (pdev == vga_default_device());
> > > +	if (!pci_is_display(pdev))
> > > +		return false;
> > > +
> > > +	if (pdev == vga_default_device())
> > > +		return true;
> > > +
> > > +#ifdef CONFIG_SCREEN_INFO
> > > +	if (pdev == screen_info_pci_dev(si))
> > > +		return true;
> > > +#endif
> > > +
> > > +	return false;
> > >   }
> > >   EXPORT_SYMBOL(video_is_primary_device);
> > > -- 
> > > 2.43.0
> > > 
> 

Re: [PATCH v9 8/9] fbcon: Use screen info to find primary device

[Mario Limonciello]

On 7/22/25 10:33 AM, Bjorn Helgaas wrote:
> On Tue, Jul 22, 2025 at 09:45:28AM -0500, Mario Limonciello wrote:
>> On 7/22/25 9:38 AM, Bjorn Helgaas wrote:
>>> On Thu, Jul 17, 2025 at 12:38:11PM -0500, Mario Limonciello wrote:
>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>
>>>> On systems with non VGA GPUs fbcon can't find the primary GPU because
>>>> video_is_primary_device() only checks the VGA arbiter.
>>>>
>>>> Add a screen info check to video_is_primary_device() so that callers
>>>> can get accurate data on such systems.
>>>
>>> This relies on screen_info, which I think is an x86 BIOS-ism.  Isn't
>>> there a UEFI console path?  How does that compare with this?  Is that
>>> relevant or is it something completely different?
>>
>> When I created and tested this I actually did this on a UEFI system (which
>> provides a UEFI GOP driver).
> 
> I guess screen_info is actually *not* an x86 BIOS-ism, and on UEFI
> systems, we do actually rely on UEFI, e.g., in efi_setup_gop(),
> alloc_screen_info(), init_screen_info()?

Right.  This all works because of the framebuffer allocated pre-boot and 
reused by the kernel.

> 
> But this patch is x86-specific, so I'm guessing the same problem could
> occur on arm64, Loongson, or other UEFI platforms, and this series
> doesn't address those?

I've never seen a multi GPU solution on another architecture, but that 
of course doesn't preclude one being created some day.

The series lays the groundwork that if it happens on another 
architecture we can easily add an architecture specific solution for 
those.  If the solution is the same we could switch to a common helper.

> 
>>>>    bool video_is_primary_device(struct device *dev)
>>>>    {
>>>> +#ifdef CONFIG_SCREEN_INFO
>>>> +	struct screen_info *si = &screen_info;
>>>> +#endif
>>>>    	struct pci_dev *pdev;
>>>>    	if (!dev_is_pci(dev))
>>>> @@ -34,7 +38,18 @@ bool video_is_primary_device(struct device *dev)
>>>>    	pdev = to_pci_dev(dev);
>>>> -	return (pdev == vga_default_device());
>>>> +	if (!pci_is_display(pdev))
>>>> +		return false;
>>>> +
>>>> +	if (pdev == vga_default_device())
>>>> +		return true;
>>>> +
>>>> +#ifdef CONFIG_SCREEN_INFO
>>>> +	if (pdev == screen_info_pci_dev(si))
>>>> +		return true;
>>>> +#endif
>>>> +
>>>> +	return false;
>>>>    }
>>>>    EXPORT_SYMBOL(video_is_primary_device);
>>>> -- 
>>>> 2.43.0
>>>>
>>

Re: [PATCH v9 0/9] Adjust fbcon console device detection

[Bjorn Helgaas]

On Thu, Jul 17, 2025 at 03:56:58PM -0500, Bjorn Helgaas wrote:
> On Thu, Jul 17, 2025 at 12:38:03PM -0500, Mario Limonciello wrote:
> > From: Mario Limonciello <mario.limonciello@amd.com>
> > 
> > Systems with more than one GPU userspace doesn't know which one to be
> > used to treat as primary.  The concept of primary is important to be
> > able to decide which GPU is used for display and  which is used for
> > rendering.  If it's guessed wrong then both GPUs will be kept awake
> > burning a lot of power.
> > 
> > Historically it would use the "boot_vga" attribute but this isn't
> > present on modern GPUs.
> > 
> > This series started out as changes to VGA arbiter to try to handle a case
> > of a system with 2 GPUs that are not VGA devices and avoid changes to
> > userspace.  This was discussed but decided not to overload the VGA arbiter
> > for non VGA devices.
> > 
> > Instead move the x86 specific detection of framebuffer resources into x86
> > specific code that the fbcon can use to properly identify the primary
> > device. This code is still called from the VGA arbiter, and the logic does
> > not change there. To avoid regression default to VGA arbiter and only fall
> > back to looking up with x86 specific detection method.
> > 
> > In order for userspace to also be able to discover which device was the
> > primary video display device create a new sysfs file 'boot_display'.
> > 
> > A matching userspace implementation for this file is available here:
> > Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/merge_requests/39
> > Link: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2038
> > 
> > Dave Airlie has been pinged for a comment on this approach.
> > Dave had suggested in the past [1]:
> > 
> > "
> >  But yes if that doesn't work, then maybe we need to make the boot_vga
> >  flag mean boot_display_gpu, and fix it in the kernel
> > "
> > 
> > This was one of the approached tried in earlier revisions and it was
> > rejected in favor of creating a new sysfs file (which is what this
> > version does).
> > 
> > It is suggested that this series merge entirely through the PCI tree.
> > 
> > Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/merge_requests/37#note_2938602 [1]
> > 
> > v9:
> >  * Add more to cover letter
> >  * Add bug link to last patch
> >  * Update commit message for last patch
> >  * Update boot_display documentation description
> > 
> > Mario Limonciello (9):
> >   PCI: Add helper for checking if a PCI device is a display controller
> >   vfio/pci: Use pci_is_display()
> >   vga_switcheroo: Use pci_is_display()
> >   iommu/vt-d: Use pci_is_display()
> >   ALSA: hda: Use pci_is_display()
> >   Fix access to video_is_primary_device() when compiled without
> >     CONFIG_VIDEO
> >   PCI/VGA: Replace vga_is_firmware_default() with a screen info check
> >   fbcon: Use screen info to find primary device
> >   PCI: Add a new 'boot_display' attribute
> > 
> >  Documentation/ABI/testing/sysfs-bus-pci |  9 +++++
> >  arch/parisc/include/asm/video.h         |  2 +-
> >  arch/sparc/include/asm/video.h          |  2 ++
> >  arch/x86/include/asm/video.h            |  2 ++
> >  arch/x86/video/video-common.c           | 17 ++++++++-
> >  drivers/gpu/vga/vga_switcheroo.c        |  2 +-
> >  drivers/iommu/intel/iommu.c             |  2 +-
> >  drivers/pci/pci-sysfs.c                 | 46 +++++++++++++++++++++++++
> >  drivers/pci/vgaarb.c                    | 31 +++--------------
> >  drivers/vfio/pci/vfio_pci_igd.c         |  3 +-
> >  include/linux/pci.h                     | 15 ++++++++
> >  sound/hda/hdac_i915.c                   |  2 +-
> >  sound/pci/hda/hda_intel.c               |  4 +--
> >  13 files changed, 102 insertions(+), 35 deletions(-)
> 
> Applied to pci/boot-display for v6.17, thanks!

I kept the pci_is_display() changes but deferred the following for now:

  Fix access to video_is_primary_device() when compiled without CONFIG_VIDEO
  PCI/VGA: Replace vga_is_firmware_default() with a screen info check
  fbcon: Use screen info to find primary device
  PCI: Add a new 'boot_display' attribute

I think the boot_display attribute isn't quite baked yet and I don't
want to add something when it looks like we're immediately going to
change the implementation and maybe the sysfs location.

Bjorn

Re: [PATCH v9 0/9] Adjust fbcon console device detection

[Mario Limonciello]

On 7/24/2025 1:36 PM, Bjorn Helgaas wrote:
> On Thu, Jul 17, 2025 at 03:56:58PM -0500, Bjorn Helgaas wrote:
>> On Thu, Jul 17, 2025 at 12:38:03PM -0500, Mario Limonciello wrote:
>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> Systems with more than one GPU userspace doesn't know which one to be
>>> used to treat as primary.  The concept of primary is important to be
>>> able to decide which GPU is used for display and  which is used for
>>> rendering.  If it's guessed wrong then both GPUs will be kept awake
>>> burning a lot of power.
>>>
>>> Historically it would use the "boot_vga" attribute but this isn't
>>> present on modern GPUs.
>>>
>>> This series started out as changes to VGA arbiter to try to handle a case
>>> of a system with 2 GPUs that are not VGA devices and avoid changes to
>>> userspace.  This was discussed but decided not to overload the VGA arbiter
>>> for non VGA devices.
>>>
>>> Instead move the x86 specific detection of framebuffer resources into x86
>>> specific code that the fbcon can use to properly identify the primary
>>> device. This code is still called from the VGA arbiter, and the logic does
>>> not change there. To avoid regression default to VGA arbiter and only fall
>>> back to looking up with x86 specific detection method.
>>>
>>> In order for userspace to also be able to discover which device was the
>>> primary video display device create a new sysfs file 'boot_display'.
>>>
>>> A matching userspace implementation for this file is available here:
>>> Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/merge_requests/39
>>> Link: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2038
>>>
>>> Dave Airlie has been pinged for a comment on this approach.
>>> Dave had suggested in the past [1]:
>>>
>>> "
>>>   But yes if that doesn't work, then maybe we need to make the boot_vga
>>>   flag mean boot_display_gpu, and fix it in the kernel
>>> "
>>>
>>> This was one of the approached tried in earlier revisions and it was
>>> rejected in favor of creating a new sysfs file (which is what this
>>> version does).
>>>
>>> It is suggested that this series merge entirely through the PCI tree.
>>>
>>> Link: https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/merge_requests/37#note_2938602 [1]
>>>
>>> v9:
>>>   * Add more to cover letter
>>>   * Add bug link to last patch
>>>   * Update commit message for last patch
>>>   * Update boot_display documentation description
>>>
>>> Mario Limonciello (9):
>>>    PCI: Add helper for checking if a PCI device is a display controller
>>>    vfio/pci: Use pci_is_display()
>>>    vga_switcheroo: Use pci_is_display()
>>>    iommu/vt-d: Use pci_is_display()
>>>    ALSA: hda: Use pci_is_display()
>>>    Fix access to video_is_primary_device() when compiled without
>>>      CONFIG_VIDEO
>>>    PCI/VGA: Replace vga_is_firmware_default() with a screen info check
>>>    fbcon: Use screen info to find primary device
>>>    PCI: Add a new 'boot_display' attribute
>>>
>>>   Documentation/ABI/testing/sysfs-bus-pci |  9 +++++
>>>   arch/parisc/include/asm/video.h         |  2 +-
>>>   arch/sparc/include/asm/video.h          |  2 ++
>>>   arch/x86/include/asm/video.h            |  2 ++
>>>   arch/x86/video/video-common.c           | 17 ++++++++-
>>>   drivers/gpu/vga/vga_switcheroo.c        |  2 +-
>>>   drivers/iommu/intel/iommu.c             |  2 +-
>>>   drivers/pci/pci-sysfs.c                 | 46 +++++++++++++++++++++++++
>>>   drivers/pci/vgaarb.c                    | 31 +++--------------
>>>   drivers/vfio/pci/vfio_pci_igd.c         |  3 +-
>>>   include/linux/pci.h                     | 15 ++++++++
>>>   sound/hda/hdac_i915.c                   |  2 +-
>>>   sound/pci/hda/hda_intel.c               |  4 +--
>>>   13 files changed, 102 insertions(+), 35 deletions(-)
>>
>> Applied to pci/boot-display for v6.17, thanks!
> 
> I kept the pci_is_display() changes but deferred the following for now:
> 
>    Fix access to video_is_primary_device() when compiled without CONFIG_VIDEO
>    PCI/VGA: Replace vga_is_firmware_default() with a screen info check
>    fbcon: Use screen info to find primary device
>    PCI: Add a new 'boot_display' attribute
> 
> I think the boot_display attribute isn't quite baked yet and I don't
> want to add something when it looks like we're immediately going to
> change the implementation and maybe the sysfs location.
> 
> Bjorn

Thanks for the update.

The patch moving to DRM does have an Acked-by.

At this point do you think there is still a shot at a squashed/rebased 
version of those for 6.17, or should I rebase and submit the outcome for 
discussion targeting 6.18 after the merge window?