[PATCH v3 0/7] Adjust fbcon console device detection

[PATCH v3 0/7] Adjust fbcon console device detection

[Mario Limonciello]

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

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 [1].  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 to fbcon, fall back to VGA arbiter.

In order for userspace to also be able to discover which device was the
primary framebuffer create a link to that device from fbcon.

v2->v3:
 * Pick up tags
 * Drop old patch 6
 * Add 2 new patches for fbcon

Link: https://lore.kernel.org/linux-pci/20250617175910.1640546-1-superm1@kernel.org/ [1]

Mario Limonciello (7):
  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()
  PCI/VGA: Move check for firmware default out of VGA arbiter
  fbcon: Make a symlink to the device selected as primary

 arch/x86/video/video-common.c    | 28 +++++++++++++++++++++++++
 drivers/gpu/vga/vga_switcheroo.c |  2 +-
 drivers/iommu/intel/iommu.c      |  2 +-
 drivers/pci/vgaarb.c             | 36 ++------------------------------
 drivers/vfio/pci/vfio_pci_igd.c  |  3 +--
 drivers/video/fbdev/core/fbcon.c | 10 ++++++++-
 include/linux/pci.h              | 15 +++++++++++++
 sound/hda/hdac_i915.c            |  2 +-
 sound/pci/hda/hda_intel.c        |  4 ++--
 9 files changed, 60 insertions(+), 42 deletions(-)

-- 
2.43.0

[PATCH v3 1/7] 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.

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 v3 2/7] 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 v3 3/7] 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 v3 4/7] 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: 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 7aa3932251b2f..17267cd476ce7 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 v3 5/7] 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: 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 e5210ed48ddf1..a165c44b43940 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 v3 6/7] PCI/VGA: Move check for firmware default out of VGA arbiter

[Mario Limonciello]

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

The x86 specific check for whether a framebuffer belongs to a device
works for display devices as well as VGA devices.  Callers to
video_is_primary_device() can benefit from checking non-VGA display
devices.

Move the x86 specific check into x86 specific code, and adjust VGA
arbiter to call that code as well. This allows fbcon to find the
right PCI device on systems that don't have VGA devices.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/x86/video/video-common.c | 28 +++++++++++++++++++++++++++
 drivers/pci/vgaarb.c          | 36 ++---------------------------------
 2 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
index 81fc97a2a837a..718116e35e450 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,13 +28,40 @@ EXPORT_SYMBOL(pgprot_framebuffer);
 
 bool video_is_primary_device(struct device *dev)
 {
+	u64 base = screen_info.lfb_base;
+	u64 size = screen_info.lfb_size;
 	struct pci_dev *pdev;
+	struct resource *r;
+	u64 limit;
 
 	if (!dev_is_pci(dev))
 		return false;
 
 	pdev = to_pci_dev(dev);
 
+	if (!pci_is_display(pdev))
+		return false;
+
+	/* 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;
+	}
+
 	return (pdev == vga_default_device());
 }
 EXPORT_SYMBOL(video_is_primary_device);
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 78748e8d2dbae..15ab58c70b016 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -26,12 +26,12 @@
 #include <linux/poll.h>
 #include <linux/miscdevice.h>
 #include <linux/slab.h>
-#include <linux/screen_info.h>
 #include <linux/vt.h>
 #include <linux/console.h>
 #include <linux/acpi.h>
 #include <linux/uaccess.h>
 #include <linux/vgaarb.h>
+#include <asm/video.h>
 
 static void vga_arbiter_notify_clients(void);
 
@@ -554,38 +554,6 @@ void vga_put(struct pci_dev *pdev, unsigned int rsrc)
 }
 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;
-
-	/* 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 false;
-}
-
 static bool vga_arb_integrated_gpu(struct device *dev)
 {
 #if defined(CONFIG_ACPI)
@@ -623,7 +591,7 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
 	if (boot_vga && boot_vga->is_firmware_default)
 		return false;
 
-	if (vga_is_firmware_default(pdev)) {
+	if (video_is_primary_device(&pdev->dev)) {
 		vgadev->is_firmware_default = true;
 		return true;
 	}
-- 
2.43.0

[PATCH v3 7/7] fbcon: Make a symlink to the device selected as primary

[Mario Limonciello]

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

Knowing which device is the primary device can be useful for userspace
to make decisions on which device to start a display server.

Create a link to that device called 'primary_device'.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/video/fbdev/core/fbcon.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 2df48037688d1..46f21570723e5 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2934,7 +2934,7 @@ static void fbcon_select_primary(struct fb_info *info)
 {
 	if (!map_override && primary_device == -1 &&
 	    video_is_primary_device(info->device)) {
-		int i;
+		int i, r;
 
 		printk(KERN_INFO "fbcon: %s (fb%i) is primary device\n",
 		       info->fix.id, info->node);
@@ -2949,6 +2949,10 @@ static void fbcon_select_primary(struct fb_info *info)
 			       first_fb_vc + 1, last_fb_vc + 1);
 			info_idx = primary_device;
 		}
+		r = sysfs_create_link(&fbcon_device->kobj, &info->device->kobj,
+				      "primary_device");
+		if (r)
+			pr_err("fbcon: Failed to link to primary device: %d\n", r);
 	}
 
 }
@@ -3376,6 +3380,10 @@ void __init fb_console_init(void)
 
 void __exit fb_console_exit(void)
 {
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY
+	if (primary_device != -1)
+		sysfs_remove_link(&fbcon_device->kobj, "primary_device");
+#endif
 #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
 	console_lock();
 	if (deferred_takeover)
-- 
2.43.0

Re: [PATCH v3 5/7] ALSA: hda: Use pci_is_display()

[Takashi Iwai]

On Fri, 20 Jun 2025 04:49:41 +0200,
Mario Limonciello wrote:
> 
> 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>

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

> ---
>  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 e5210ed48ddf1..a165c44b43940 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
> 

Re: [PATCH v3 6/7] PCI/VGA: Move check for firmware default out of VGA arbiter

[Thomas Zimmermann]

Hi

Am 20.06.25 um 04:49 schrieb Mario Limonciello:
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> The x86 specific check for whether a framebuffer belongs to a device
> works for display devices as well as VGA devices.  Callers to
> video_is_primary_device() can benefit from checking non-VGA display
> devices.
>
> Move the x86 specific check into x86 specific code, and adjust VGA
> arbiter to call that code as well. This allows fbcon to find the
> right PCI device on systems that don't have VGA devices.
>
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   arch/x86/video/video-common.c | 28 +++++++++++++++++++++++++++
>   drivers/pci/vgaarb.c          | 36 ++---------------------------------
>   2 files changed, 30 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
> index 81fc97a2a837a..718116e35e450 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,13 +28,40 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>   
>   bool video_is_primary_device(struct device *dev)

I'm not sure I understand this patch. video_is_primary_device() already 
exists for 3 architectures, including x86. [1] Adding it here should 
produce an error. (?)

[1] https://elixir.bootlin.com/linux/v6.15.2/A/ident/video_is_primary_device

The code on x86 is

bool 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/bool>video_is_primary_device 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/video_is_primary_device>(structdevice 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/device>*dev) { 
structpci_dev 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/pci_dev>*pdev; 
if(!dev_is_pci 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/dev_is_pci>(dev)) 
returnfalse <https://elixir.bootlin.com/linux/v6.15.2/C/ident/false>; 
pdev=to_pci_dev 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/to_pci_dev>(dev); 
return(pdev==vga_default_device 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/vga_default_device>()); }

I was thinking about extending it to test for additional properties, 
like this

bool 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/bool>video_is_primary_device 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/video_is_primary_device>(structdevice 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/device>*dev) { 
structpci_dev 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/pci_dev>*pdev; 
if(!dev_is_pci 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/dev_is_pci>(dev)) 
returnfalse <https://elixir.bootlin.com/linux/v6.15.2/C/ident/false>; 
pdev=to_pci_dev 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/to_pci_dev>(dev); 
if(pdev==vga_default_device 
<https://elixir.bootlin.com/linux/v6.15.2/C/ident/vga_default_device>()) 
return true for_each_pci_dev() { // test if display and could be 
primary. } return false; // nothing found }


This would then be called from per-device sysfs code that export a 
property similar to boot_vga (such as boot_display).


The issue is currently just an x86 problem, but I can imagine something 
similar happening on ARM. There we'd have to go through the DT tree to 
figure out the primary device. That's a problem for a later patch set, 
but we should keep this in mind.

>   {
> +	u64 base = screen_info.lfb_base;
> +	u64 size = screen_info.lfb_size;
>   	struct pci_dev *pdev;
> +	struct resource *r;
> +	u64 limit;
>   
>   	if (!dev_is_pci(dev))
>   		return false;
>   
>   	pdev = to_pci_dev(dev);
>   
> +	if (!pci_is_display(pdev))
> +		return false;
> +
> +	/* 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;
> +	}
> +

You can drop all this code and call screen_info_pci_dev() instead. I 
simply never got to update vgaarb to use it.

[2] 
https://elixir.bootlin.com/linux/v6.15.2/source/drivers/video/screen_info_pci.c#L109

>   	return (pdev == vga_default_device());
>   }
>   EXPORT_SYMBOL(video_is_primary_device);
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index 78748e8d2dbae..15ab58c70b016 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -26,12 +26,12 @@
>   #include <linux/poll.h>
>   #include <linux/miscdevice.h>
>   #include <linux/slab.h>
> -#include <linux/screen_info.h>
>   #include <linux/vt.h>
>   #include <linux/console.h>
>   #include <linux/acpi.h>
>   #include <linux/uaccess.h>
>   #include <linux/vgaarb.h>
> +#include <asm/video.h>
>   
>   static void vga_arbiter_notify_clients(void);
>   
> @@ -554,38 +554,6 @@ void vga_put(struct pci_dev *pdev, unsigned int rsrc)
>   }
>   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;
> -
> -	/* 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 false;
> -}
> -
>   static bool vga_arb_integrated_gpu(struct device *dev)
>   {
>   #if defined(CONFIG_ACPI)
> @@ -623,7 +591,7 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
>   	if (boot_vga && boot_vga->is_firmware_default)
>   		return false;
>   
> -	if (vga_is_firmware_default(pdev)) {
> +	if (video_is_primary_device(&pdev->dev)) {

Maybe not change this because you don't want to end up with non-VGA 
devices here.

Best regards
Thomas

>   		vgadev->is_firmware_default = true;
>   		return true;
>   	}

-- 
--
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 v3 7/7] fbcon: Make a symlink to the device selected as primary

[Thomas Zimmermann]

Hi

Am 20.06.25 um 04:49 schrieb Mario Limonciello:
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> Knowing which device is the primary device can be useful for userspace
> to make decisions on which device to start a display server.
>
> Create a link to that device called 'primary_device'.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/video/fbdev/core/fbcon.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 2df48037688d1..46f21570723e5 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c

You cannot rely on this, as fbcon might be disabled entirely.

Best regards
Thomas

> @@ -2934,7 +2934,7 @@ static void fbcon_select_primary(struct fb_info *info)
>   {
>   	if (!map_override && primary_device == -1 &&
>   	    video_is_primary_device(info->device)) {
> -		int i;
> +		int i, r;
>   
>   		printk(KERN_INFO "fbcon: %s (fb%i) is primary device\n",
>   		       info->fix.id, info->node);
> @@ -2949,6 +2949,10 @@ static void fbcon_select_primary(struct fb_info *info)
>   			       first_fb_vc + 1, last_fb_vc + 1);
>   			info_idx = primary_device;
>   		}
> +		r = sysfs_create_link(&fbcon_device->kobj, &info->device->kobj,
> +				      "primary_device");
> +		if (r)
> +			pr_err("fbcon: Failed to link to primary device: %d\n", r);
>   	}
>   
>   }
> @@ -3376,6 +3380,10 @@ void __init fb_console_init(void)
>   
>   void __exit fb_console_exit(void)
>   {
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY
> +	if (primary_device != -1)
> +		sysfs_remove_link(&fbcon_device->kobj, "primary_device");
> +#endif
>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>   	console_lock();
>   	if (deferred_takeover)

-- 
--
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 v3 7/7] fbcon: Make a symlink to the device selected as primary

[Mario Limonciello]

On 6/20/25 3:47 AM, Thomas Zimmermann wrote:
> Hi
> 
> Am 20.06.25 um 04:49 schrieb Mario Limonciello:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> Knowing which device is the primary device can be useful for userspace
>> to make decisions on which device to start a display server.
>>
>> Create a link to that device called 'primary_device'.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/video/fbdev/core/fbcon.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/ 
>> core/fbcon.c
>> index 2df48037688d1..46f21570723e5 100644
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
> 
> You cannot rely on this, as fbcon might be disabled entirely.

So the other idea I had was to have a new file boot_console.

How would you feel about this instead (or even in addition to the symlink)?

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 268c69daa4d5..8535950b4c0f 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_console_show(struct device *dev, struct 
device_attribute *attr,
+                                char *buf)
+{
+       return sysfs_emit(buf, "%u\n", video_is_primary_device(dev));
+}
+static DEVICE_ATTR_RO(boot_console);
+
  static ssize_t boot_vga_show(struct device *dev, struct 
device_attribute *attr,
                              char *buf)
  {
@@ -1698,6 +1706,7 @@ late_initcall(pci_sysfs_init);

  static struct attribute *pci_dev_dev_attrs[] = {
         &dev_attr_boot_vga.attr,
+       &dev_attr_boot_console.attr,
         NULL,
  };

@@ -1710,6 +1719,9 @@ static umode_t pci_dev_attrs_are_visible(struct 
kobject *kobj,
         if (a == &dev_attr_boot_vga.attr && pci_is_vga(pdev))
                 return a->mode;

+       if (a == &dev_attr_boot_console.attr && pci_is_display(pdev))
+               return a->mode;
+
         return 0;
  }


> 
> Best regards
> Thomas
> 
>> @@ -2934,7 +2934,7 @@ static void fbcon_select_primary(struct fb_info 
>> *info)
>>   {
>>       if (!map_override && primary_device == -1 &&
>>           video_is_primary_device(info->device)) {
>> -        int i;
>> +        int i, r;
>>           printk(KERN_INFO "fbcon: %s (fb%i) is primary device\n",
>>                  info->fix.id, info->node);
>> @@ -2949,6 +2949,10 @@ static void fbcon_select_primary(struct fb_info 
>> *info)
>>                      first_fb_vc + 1, last_fb_vc + 1);
>>               info_idx = primary_device;
>>           }
>> +        r = sysfs_create_link(&fbcon_device->kobj, &info->device->kobj,
>> +                      "primary_device");
>> +        if (r)
>> +            pr_err("fbcon: Failed to link to primary device: %d\n", r);
>>       }
>>   }
>> @@ -3376,6 +3380,10 @@ void __init fb_console_init(void)
>>   void __exit fb_console_exit(void)
>>   {
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY
>> +    if (primary_device != -1)
>> +        sysfs_remove_link(&fbcon_device->kobj, "primary_device");
>> +#endif
>>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>>       console_lock();
>>       if (deferred_takeover)
> 

Re: [PATCH v3 6/7] PCI/VGA: Move check for firmware default out of VGA arbiter

[Mario Limonciello]

On 6/20/2025 3:45 AM, Thomas Zimmermann wrote:
> Hi
> 
> Am 20.06.25 um 04:49 schrieb Mario Limonciello:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> The x86 specific check for whether a framebuffer belongs to a device
>> works for display devices as well as VGA devices.  Callers to
>> video_is_primary_device() can benefit from checking non-VGA display
>> devices.
>>
>> Move the x86 specific check into x86 specific code, and adjust VGA
>> arbiter to call that code as well. This allows fbcon to find the
>> right PCI device on systems that don't have VGA devices.
>>
>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   arch/x86/video/video-common.c | 28 +++++++++++++++++++++++++++
>>   drivers/pci/vgaarb.c          | 36 ++---------------------------------
>>   2 files changed, 30 insertions(+), 34 deletions(-)
>>
>> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video- 
>> common.c
>> index 81fc97a2a837a..718116e35e450 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,13 +28,40 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>>   bool video_is_primary_device(struct device *dev)
> 
> I'm not sure I understand this patch. video_is_primary_device() already 
> exists for 3 architectures, including x86. [1] Adding it here should 
> produce an error. (?)

I wasn't adding a new implementation of it, I was augmenting the x86 
implementation.

But I guess based on your below point it just needs to call 
screen_info_pci_dev().

> 
> [1] https://elixir.bootlin.com/linux/v6.15.2/A/ident/ 
> video_is_primary_device
> 
> The code on x86 is
> 
> bool <https://elixir.bootlin.com/linux/v6.15.2/C/ident/ 
> bool>video_is_primary_device <https://elixir.bootlin.com/linux/v6.15.2/ 
> C/ident/video_is_primary_device>(structdevice <https:// 
> elixir.bootlin.com/linux/v6.15.2/C/ident/device>*dev) { structpci_dev 
> <https://elixir.bootlin.com/linux/v6.15.2/C/ident/pci_dev>*pdev; if(! 
> dev_is_pci <https://elixir.bootlin.com/linux/v6.15.2/C/ident/ 
> dev_is_pci>(dev)) returnfalse <https://elixir.bootlin.com/linux/v6.15.2/ 
> C/ident/false>; pdev=to_pci_dev <https://elixir.bootlin.com/linux/ 
> v6.15.2/C/ident/to_pci_dev>(dev); return(pdev==vga_default_device 
> <https://elixir.bootlin.com/linux/v6.15.2/C/ident/vga_default_device>()); }
> 
> I was thinking about extending it to test for additional properties, 
> like this
> 
> bool <https://elixir.bootlin.com/linux/v6.15.2/C/ident/ 
> bool>video_is_primary_device <https://elixir.bootlin.com/linux/v6.15.2/ 
> C/ident/video_is_primary_device>(structdevice <https:// 
> elixir.bootlin.com/linux/v6.15.2/C/ident/device>*dev) { structpci_dev 
> <https://elixir.bootlin.com/linux/v6.15.2/C/ident/pci_dev>*pdev; if(! 
> dev_is_pci <https://elixir.bootlin.com/linux/v6.15.2/C/ident/ 
> dev_is_pci>(dev)) returnfalse <https://elixir.bootlin.com/linux/v6.15.2/ 
> C/ident/false>; pdev=to_pci_dev <https://elixir.bootlin.com/linux/ 
> v6.15.2/C/ident/to_pci_dev>(dev); if(pdev==vga_default_device <https:// 
> elixir.bootlin.com/linux/v6.15.2/C/ident/vga_default_device>()) return 
> true for_each_pci_dev() { // test if display and could be primary. } 
> return false; // nothing found }
> 

The above looks like some bad copy / paste.  Could you clarify?

> 
> This would then be called from per-device sysfs code that export a 
> property similar to boot_vga (such as boot_display).

Here's the other idea I had in mind.

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 268c69daa4d57..8535950b4c0f3 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_console_show(struct device *dev, struct 
device_attribute *attr,
+                                char *buf)
+{
+       return sysfs_emit(buf, "%u\n", video_is_primary_device(dev));
+}
+static DEVICE_ATTR_RO(boot_console);
+
  static ssize_t boot_vga_show(struct device *dev, struct 
device_attribute *attr,
                              char *buf)
  {
@@ -1698,6 +1706,7 @@ late_initcall(pci_sysfs_init);

  static struct attribute *pci_dev_dev_attrs[] = {
         &dev_attr_boot_vga.attr,
+       &dev_attr_boot_console.attr,
         NULL,
  };

@@ -1710,6 +1719,9 @@ static umode_t pci_dev_attrs_are_visible(struct 
kobject *kobj,
         if (a == &dev_attr_boot_vga.attr && pci_is_vga(pdev))
                 return a->mode;

+       if (a == &dev_attr_boot_console.attr && pci_is_display(pdev))
+               return a->mode;
+
         return 0;
  }


> 
> 
> The issue is currently just an x86 problem, but I can imagine something 
> similar happening on ARM. There we'd have to go through the DT tree to 
> figure out the primary device. That's a problem for a later patch set, 
> but we should keep this in mind.

I think that the sysfs file idea above would work for any arch.

> 
>>   {
>> +    u64 base = screen_info.lfb_base;
>> +    u64 size = screen_info.lfb_size;
>>       struct pci_dev *pdev;
>> +    struct resource *r;
>> +    u64 limit;
>>       if (!dev_is_pci(dev))
>>           return false;
>>       pdev = to_pci_dev(dev);
>> +    if (!pci_is_display(pdev))
>> +        return false;
>> +
>> +    /* 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;
>> +    }
>> +
> 
> You can drop all this code and call screen_info_pci_dev() instead. I 
> simply never got to update vgaarb to use it.

👍

> 
> [2] https://elixir.bootlin.com/linux/v6.15.2/source/drivers/video/ 
> screen_info_pci.c#L109
> 
>>       return (pdev == vga_default_device());
>>   }
>>   EXPORT_SYMBOL(video_is_primary_device);
>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
>> index 78748e8d2dbae..15ab58c70b016 100644
>> --- a/drivers/pci/vgaarb.c
>> +++ b/drivers/pci/vgaarb.c
>> @@ -26,12 +26,12 @@
>>   #include <linux/poll.h>
>>   #include <linux/miscdevice.h>
>>   #include <linux/slab.h>
>> -#include <linux/screen_info.h>
>>   #include <linux/vt.h>
>>   #include <linux/console.h>
>>   #include <linux/acpi.h>
>>   #include <linux/uaccess.h>
>>   #include <linux/vgaarb.h>
>> +#include <asm/video.h>
>>   static void vga_arbiter_notify_clients(void);
>> @@ -554,38 +554,6 @@ void vga_put(struct pci_dev *pdev, unsigned int 
>> rsrc)
>>   }
>>   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;
>> -
>> -    /* 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 false;
>> -}
>> -
>>   static bool vga_arb_integrated_gpu(struct device *dev)
>>   {
>>   #if defined(CONFIG_ACPI)
>> @@ -623,7 +591,7 @@ static bool vga_is_boot_device(struct vga_device 
>> *vgadev)
>>       if (boot_vga && boot_vga->is_firmware_default)
>>           return false;
>> -    if (vga_is_firmware_default(pdev)) {
>> +    if (video_is_primary_device(&pdev->dev)) {
> 
> Maybe not change this because you don't want to end up with non-VGA 
> devices here.

👍

> 
> Best regards
> Thomas
> 
>>           vgadev->is_firmware_default = true;
>>           return true;
>>       }
> 

Re: [PATCH v3 6/7] PCI/VGA: Move check for firmware default out of VGA arbiter

[kernel test robot]

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 awilliam-vfio/next awilliam-vfio/for-linus tip/x86/core linus/master v6.16-rc2 next-20250620]
[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/20250620-105220
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20250620024943.3415685-7-superm1%40kernel.org
patch subject: [PATCH v3 6/7] PCI/VGA: Move check for firmware default out of VGA arbiter
config: sparc-defconfig (https://download.01.org/0day-ci/archive/20250622/202506221312.49Fy1aNA-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250622/202506221312.49Fy1aNA-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/202506221312.49Fy1aNA-lkp@intel.com/

All errors (new ones prefixed by >>):

   sparc-linux-ld: drivers/pci/vgaarb.o: in function `vga_arbiter_add_pci_device':
>> vgaarb.c:(.text+0x14ec): undefined reference to `video_is_primary_device'
>> sparc-linux-ld: vgaarb.c:(.text+0x174c): undefined reference to `video_is_primary_device'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 7/7] fbcon: Make a symlink to the device selected as primary

[Thomas Zimmermann]

Hi

Am 20.06.25 um 17:56 schrieb Mario Limonciello:
> On 6/20/25 3:47 AM, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 20.06.25 um 04:49 schrieb Mario Limonciello:
>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> Knowing which device is the primary device can be useful for userspace
>>> to make decisions on which device to start a display server.
>>>
>>> Create a link to that device called 'primary_device'.
>>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>>   drivers/video/fbdev/core/fbcon.c | 10 +++++++++-
>>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/ 
>>> core/fbcon.c
>>> index 2df48037688d1..46f21570723e5 100644
>>> --- a/drivers/video/fbdev/core/fbcon.c
>>> +++ b/drivers/video/fbdev/core/fbcon.c
>>
>> You cannot rely on this, as fbcon might be disabled entirely.
>
> So the other idea I had was to have a new file boot_console.

'console' already has a meaning, so I'd prefer boot_display. Apart from 
naming, this is a good idea.

>
> How would you feel about this instead (or even in addition to the 
> symlink)?

We likely won't need the symlink then.

Best regards
Thomas

>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 268c69daa4d5..8535950b4c0f 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_console_show(struct device *dev, struct 
> device_attribute *attr,
> +                                char *buf)
> +{
> +       return sysfs_emit(buf, "%u\n", video_is_primary_device(dev));
> +}
> +static DEVICE_ATTR_RO(boot_console);
> +
>  static ssize_t boot_vga_show(struct device *dev, struct 
> device_attribute *attr,
>                              char *buf)
>  {
> @@ -1698,6 +1706,7 @@ late_initcall(pci_sysfs_init);
>
>  static struct attribute *pci_dev_dev_attrs[] = {
>         &dev_attr_boot_vga.attr,
> +       &dev_attr_boot_console.attr,
>         NULL,
>  };
>
> @@ -1710,6 +1719,9 @@ static umode_t pci_dev_attrs_are_visible(struct 
> kobject *kobj,
>         if (a == &dev_attr_boot_vga.attr && pci_is_vga(pdev))
>                 return a->mode;
>
> +       if (a == &dev_attr_boot_console.attr && pci_is_display(pdev))
> +               return a->mode;
> +
>         return 0;
>  }
>
>
>>
>> Best regards
>> Thomas
>>
>>> @@ -2934,7 +2934,7 @@ static void fbcon_select_primary(struct 
>>> fb_info *info)
>>>   {
>>>       if (!map_override && primary_device == -1 &&
>>>           video_is_primary_device(info->device)) {
>>> -        int i;
>>> +        int i, r;
>>>           printk(KERN_INFO "fbcon: %s (fb%i) is primary device\n",
>>>                  info->fix.id, info->node);
>>> @@ -2949,6 +2949,10 @@ static void fbcon_select_primary(struct 
>>> fb_info *info)
>>>                      first_fb_vc + 1, last_fb_vc + 1);
>>>               info_idx = primary_device;
>>>           }
>>> +        r = sysfs_create_link(&fbcon_device->kobj, 
>>> &info->device->kobj,
>>> +                      "primary_device");
>>> +        if (r)
>>> +            pr_err("fbcon: Failed to link to primary device: %d\n", 
>>> r);
>>>       }
>>>   }
>>> @@ -3376,6 +3380,10 @@ void __init fb_console_init(void)
>>>   void __exit fb_console_exit(void)
>>>   {
>>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY
>>> +    if (primary_device != -1)
>>> +        sysfs_remove_link(&fbcon_device->kobj, "primary_device");
>>> +#endif
>>>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>>>       console_lock();
>>>       if (deferred_takeover)
>>
>

-- 
--
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 v3 6/7] PCI/VGA: Move check for firmware default out of VGA arbiter

[Thomas Zimmermann]

Hi

Am 21.06.25 um 00:17 schrieb Mario Limonciello:
> On 6/20/2025 3:45 AM, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 20.06.25 um 04:49 schrieb Mario Limonciello:
>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> The x86 specific check for whether a framebuffer belongs to a device
>>> works for display devices as well as VGA devices.  Callers to
>>> video_is_primary_device() can benefit from checking non-VGA display
>>> devices.
>>>
>>> Move the x86 specific check into x86 specific code, and adjust VGA
>>> arbiter to call that code as well. This allows fbcon to find the
>>> right PCI device on systems that don't have VGA devices.
>>>
>>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>>   arch/x86/video/video-common.c | 28 +++++++++++++++++++++++++++
>>>   drivers/pci/vgaarb.c          | 36 
>>> ++---------------------------------
>>>   2 files changed, 30 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video- 
>>> common.c
>>> index 81fc97a2a837a..718116e35e450 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,13 +28,40 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>>>   bool video_is_primary_device(struct device *dev)
>>
>> I'm not sure I understand this patch. video_is_primary_device() 
>> already exists for 3 architectures, including x86. [1] Adding it here 
>> should produce an error. (?)
>
> I wasn't adding a new implementation of it, I was augmenting the x86 
> implementation.

Indeed. Apologies, I must have somehow misread the patch. So this is 
essentially doing what I proposed.

>
> But I guess based on your below point it just needs to call 
> screen_info_pci_dev().

Yeah, the helper already does everything necessary.


>>
>> [1] https://elixir.bootlin.com/linux/v6.15.2/A/ident/ 
>> video_is_primary_device
>>
>> The code on x86 is
>>
>> bool <https://elixir.bootlin.com/linux/v6.15.2/C/ident/ 
>> bool>video_is_primary_device 
>> <https://elixir.bootlin.com/linux/v6.15.2/ 
>> C/ident/video_is_primary_device>(structdevice <https:// 
>> elixir.bootlin.com/linux/v6.15.2/C/ident/device>*dev) { structpci_dev 
>> <https://elixir.bootlin.com/linux/v6.15.2/C/ident/pci_dev>*pdev; if(! 
>> dev_is_pci <https://elixir.bootlin.com/linux/v6.15.2/C/ident/ 
>> dev_is_pci>(dev)) returnfalse 
>> <https://elixir.bootlin.com/linux/v6.15.2/ C/ident/false>; 
>> pdev=to_pci_dev <https://elixir.bootlin.com/linux/ 
>> v6.15.2/C/ident/to_pci_dev>(dev); return(pdev==vga_default_device 
>> <https://elixir.bootlin.com/linux/v6.15.2/C/ident/vga_default_device>()); 
>> }
>>
>> I was thinking about extending it to test for additional properties, 
>> like this
>>
>> bool <https://elixir.bootlin.com/linux/v6.15.2/C/ident/ 
>> bool>video_is_primary_device 
>> <https://elixir.bootlin.com/linux/v6.15.2/ 
>> C/ident/video_is_primary_device>(structdevice <https:// 
>> elixir.bootlin.com/linux/v6.15.2/C/ident/device>*dev) { structpci_dev 
>> <https://elixir.bootlin.com/linux/v6.15.2/C/ident/pci_dev>*pdev; if(! 
>> dev_is_pci <https://elixir.bootlin.com/linux/v6.15.2/C/ident/ 
>> dev_is_pci>(dev)) returnfalse 
>> <https://elixir.bootlin.com/linux/v6.15.2/ C/ident/false>; 
>> pdev=to_pci_dev <https://elixir.bootlin.com/linux/ 
>> v6.15.2/C/ident/to_pci_dev>(dev); if(pdev==vga_default_device 
>> <https:// 
>> elixir.bootlin.com/linux/v6.15.2/C/ident/vga_default_device>()) 
>> return true for_each_pci_dev() { // test if display and could be 
>> primary. } return false; // nothing found }
>>
>
> The above looks like some bad copy / paste.  Could you clarify?

Oh, well. I really messed up my reply. :D

What I meant is what you already implemented, but with the existing helper:

bool video_is_primary_device(dev)
{
     if (dev == vga_default_device())
       return true

     if (dev == screen_info_pci_device())
       return true

     return false
}


One thing to keep in minds is that video_is_primary_device() currently 
returns false by default. IDK if that's a problem for user space, but 
user space should at least pick a reasonable fallback in that case.

Best regards
Thomas


>
>>
>> This would then be called from per-device sysfs code that export a 
>> property similar to boot_vga (such as boot_display).
>
> Here's the other idea I had in mind.
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 268c69daa4d57..8535950b4c0f3 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_console_show(struct device *dev, struct 
> device_attribute *attr,
> +                                char *buf)
> +{
> +       return sysfs_emit(buf, "%u\n", video_is_primary_device(dev));
> +}
> +static DEVICE_ATTR_RO(boot_console);
> +
>  static ssize_t boot_vga_show(struct device *dev, struct 
> device_attribute *attr,
>                              char *buf)
>  {
> @@ -1698,6 +1706,7 @@ late_initcall(pci_sysfs_init);
>
>  static struct attribute *pci_dev_dev_attrs[] = {
>         &dev_attr_boot_vga.attr,
> +       &dev_attr_boot_console.attr,
>         NULL,
>  };
>
> @@ -1710,6 +1719,9 @@ static umode_t pci_dev_attrs_are_visible(struct 
> kobject *kobj,
>         if (a == &dev_attr_boot_vga.attr && pci_is_vga(pdev))
>                 return a->mode;
>
> +       if (a == &dev_attr_boot_console.attr && pci_is_display(pdev))
> +               return a->mode;
> +
>         return 0;
>  }
>
>
>>
>>
>> The issue is currently just an x86 problem, but I can imagine 
>> something similar happening on ARM. There we'd have to go through the 
>> DT tree to figure out the primary device. That's a problem for a 
>> later patch set, but we should keep this in mind.
>
> I think that the sysfs file idea above would work for any arch.
>
>>
>>>   {
>>> +    u64 base = screen_info.lfb_base;
>>> +    u64 size = screen_info.lfb_size;
>>>       struct pci_dev *pdev;
>>> +    struct resource *r;
>>> +    u64 limit;
>>>       if (!dev_is_pci(dev))
>>>           return false;
>>>       pdev = to_pci_dev(dev);
>>> +    if (!pci_is_display(pdev))
>>> +        return false;
>>> +
>>> +    /* 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;
>>> +    }
>>> +
>>
>> You can drop all this code and call screen_info_pci_dev() instead. I 
>> simply never got to update vgaarb to use it.
>
> 👍
>
>>
>> [2] https://elixir.bootlin.com/linux/v6.15.2/source/drivers/video/ 
>> screen_info_pci.c#L109
>>
>>>       return (pdev == vga_default_device());
>>>   }
>>>   EXPORT_SYMBOL(video_is_primary_device);
>>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
>>> index 78748e8d2dbae..15ab58c70b016 100644
>>> --- a/drivers/pci/vgaarb.c
>>> +++ b/drivers/pci/vgaarb.c
>>> @@ -26,12 +26,12 @@
>>>   #include <linux/poll.h>
>>>   #include <linux/miscdevice.h>
>>>   #include <linux/slab.h>
>>> -#include <linux/screen_info.h>
>>>   #include <linux/vt.h>
>>>   #include <linux/console.h>
>>>   #include <linux/acpi.h>
>>>   #include <linux/uaccess.h>
>>>   #include <linux/vgaarb.h>
>>> +#include <asm/video.h>
>>>   static void vga_arbiter_notify_clients(void);
>>> @@ -554,38 +554,6 @@ void vga_put(struct pci_dev *pdev, unsigned int 
>>> rsrc)
>>>   }
>>>   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;
>>> -
>>> -    /* 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 false;
>>> -}
>>> -
>>>   static bool vga_arb_integrated_gpu(struct device *dev)
>>>   {
>>>   #if defined(CONFIG_ACPI)
>>> @@ -623,7 +591,7 @@ static bool vga_is_boot_device(struct vga_device 
>>> *vgadev)
>>>       if (boot_vga && boot_vga->is_firmware_default)
>>>           return false;
>>> -    if (vga_is_firmware_default(pdev)) {
>>> +    if (video_is_primary_device(&pdev->dev)) {
>>
>> Maybe not change this because you don't want to end up with non-VGA 
>> devices here.
>
> 👍
>
>>
>> Best regards
>> Thomas
>>
>>>           vgadev->is_firmware_default = true;
>>>           return true;
>>>       }
>>
>

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