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>
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v10:
 * Rebase on 6.17-rc1
 * Squash 'fbcon: Stop using screen_info_pci_dev()'
---
 arch/x86/video/video-common.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
index 81fc97a2a837a..e0aeee99bc99e 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,11 @@ EXPORT_SYMBOL(pgprot_framebuffer);
 
 bool video_is_primary_device(struct device *dev)
 {
+#ifdef CONFIG_SCREEN_INFO
+	struct screen_info *si = &screen_info;
+	struct resource res[SCREEN_INFO_MAX_RESOURCES];
+	ssize_t i, numres;
+#endif
 	struct pci_dev *pdev;
 
 	if (!dev_is_pci(dev))
@@ -34,7 +40,24 @@ 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
+	numres = screen_info_resources(si, res, ARRAY_SIZE(res));
+	for (i = 0; i < numres; ++i) {
+		if (!(res[i].flags & IORESOURCE_MEM))
+			continue;
+
+		if (pci_find_resource(pdev, &res[i]))
+			return true;
+	}
+#endif
+
+	return false;
 }
 EXPORT_SYMBOL(video_is_primary_device);
 
-- 
2.43.0

On Mon, Aug 11, 2025 at 11:26:05AM -0500, Mario Limonciello (AMD) wrote:
> 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.

I have a question regarding this change. To me, the function name 
video_is_primary_device() implies that there is only one primary GPU.
I would also expect that the 'boot_display' attribute added later in 
the patch series based on this function is only set for one GPU, but 
that is not necessarily the case. Since I'm working on a user-space
program that reads the 'boot_display' attribute, I need to know what
behavior is intended in order to do a correct implementation.

> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v10:
>  * Rebase on 6.17-rc1
>  * Squash 'fbcon: Stop using screen_info_pci_dev()'
> ---
>  arch/x86/video/video-common.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
> index 81fc97a2a837a..e0aeee99bc99e 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,11 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>  
>  bool video_is_primary_device(struct device *dev)
>  {
> +#ifdef CONFIG_SCREEN_INFO
> +	struct screen_info *si = &screen_info;
> +	struct resource res[SCREEN_INFO_MAX_RESOURCES];
> +	ssize_t i, numres;
> +#endif
>  	struct pci_dev *pdev;
>  
>  	if (!dev_is_pci(dev))
> @@ -34,7 +40,24 @@ 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;

This can mark a VGA device as primary GPU.

> +
> +#ifdef CONFIG_SCREEN_INFO
> +	numres = screen_info_resources(si, res, ARRAY_SIZE(res));
> +	for (i = 0; i < numres; ++i) {
> +		if (!(res[i].flags & IORESOURCE_MEM))
> +			continue;
> +
> +		if (pci_find_resource(pdev, &res[i]))
> +			return true;
> +	}
> +#endif

And then the new code can also choose a primary GPU.

> +
> +	return false;
>  }
>  EXPORT_SYMBOL(video_is_primary_device);
>  

In particular, I have hardware that has this exact configuration where
two GPUs are marked as primary and have a 'boot_display' attribute: the
first one through vga_default_device(), the second one through the new
detection method.

Is this intended?

Kind regards
Aaron

On 10/28/25 5:16 AM, Aaron Erhardt wrote:
> 
> On Mon, Aug 11, 2025 at 11:26:05AM -0500, Mario Limonciello (AMD) wrote:
>> 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.
> 
> I have a question regarding this change. To me, the function name
> video_is_primary_device() implies that there is only one primary GPU.
> I would also expect that the 'boot_display' attribute added later in
> the patch series based on this function is only set for one GPU, but
> that is not necessarily the case. Since I'm working on a user-space
> program that reads the 'boot_display' attribute, I need to know what
> behavior is intended in order to do a correct implementation.
> 
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> v10:
>>   * Rebase on 6.17-rc1
>>   * Squash 'fbcon: Stop using screen_info_pci_dev()'
>> ---
>>   arch/x86/video/video-common.c | 25 ++++++++++++++++++++++++-
>>   1 file changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
>> index 81fc97a2a837a..e0aeee99bc99e 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,11 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>>   
>>   bool video_is_primary_device(struct device *dev)
>>   {
>> +#ifdef CONFIG_SCREEN_INFO
>> +	struct screen_info *si = &screen_info;
>> +	struct resource res[SCREEN_INFO_MAX_RESOURCES];
>> +	ssize_t i, numres;
>> +#endif
>>   	struct pci_dev *pdev;
>>   
>>   	if (!dev_is_pci(dev))
>> @@ -34,7 +40,24 @@ 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;
> 
> This can mark a VGA device as primary GPU.
> 
>> +
>> +#ifdef CONFIG_SCREEN_INFO
>> +	numres = screen_info_resources(si, res, ARRAY_SIZE(res));
>> +	for (i = 0; i < numres; ++i) {
>> +		if (!(res[i].flags & IORESOURCE_MEM))
>> +			continue;
>> +
>> +		if (pci_find_resource(pdev, &res[i]))
>> +			return true;
>> +	}
>> +#endif
> 
> And then the new code can also choose a primary GPU.
> 
>> +
>> +	return false;
>>   }
>>   EXPORT_SYMBOL(video_is_primary_device);
>>   
> 
> In particular, I have hardware that has this exact configuration where
> two GPUs are marked as primary and have a 'boot_display' attribute: the
> first one through vga_default_device(), the second one through the new
> detection method.
> 
> Is this intended?
> 
> Kind regards
> Aaron
> 

I wouldn't have expected a case like this and I think it means there is 
a logic error.

Can you please file a kernel bugzilla with details about your system and 
CC me?

dmesg and lspci -vvnn please

Also; please clarify which GPU shows something when booting up.

Hi

Am 28.10.25 um 14:15 schrieb Mario Limonciello:
> On 10/28/25 5:16 AM, Aaron Erhardt wrote:
>>
>> On Mon, Aug 11, 2025 at 11:26:05AM -0500, Mario Limonciello (AMD) wrote:
>>> 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.
>>
>> I have a question regarding this change. To me, the function name
>> video_is_primary_device() implies that there is only one primary GPU.
>> I would also expect that the 'boot_display' attribute added later in
>> the patch series based on this function is only set for one GPU, but
>> that is not necessarily the case. Since I'm working on a user-space
>> program that reads the 'boot_display' attribute, I need to know what
>> behavior is intended in order to do a correct implementation.
>>
>>>
>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>>> ---
>>> v10:
>>>   * Rebase on 6.17-rc1
>>>   * Squash 'fbcon: Stop using screen_info_pci_dev()'
>>> ---
>>>   arch/x86/video/video-common.c | 25 ++++++++++++++++++++++++-
>>>   1 file changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/video/video-common.c 
>>> b/arch/x86/video/video-common.c
>>> index 81fc97a2a837a..e0aeee99bc99e 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,11 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>>>     bool video_is_primary_device(struct device *dev)
>>>   {
>>> +#ifdef CONFIG_SCREEN_INFO
>>> +    struct screen_info *si = &screen_info;
>>> +    struct resource res[SCREEN_INFO_MAX_RESOURCES];
>>> +    ssize_t i, numres;
>>> +#endif
>>>       struct pci_dev *pdev;
>>>         if (!dev_is_pci(dev))
>>> @@ -34,7 +40,24 @@ 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;
>>
>> This can mark a VGA device as primary GPU.

Is the value returned from vga_default_device() eq to NULL?

>>
>>> +
>>> +#ifdef CONFIG_SCREEN_INFO
>>> +    numres = screen_info_resources(si, res, ARRAY_SIZE(res));
>>> +    for (i = 0; i < numres; ++i) {
>>> +        if (!(res[i].flags & IORESOURCE_MEM))
>>> +            continue;
>>> +
>>> +        if (pci_find_resource(pdev, &res[i]))
>>> +            return true;
>>> +    }
>>> +#endif
>>
>> And then the new code can also choose a primary GPU.

Maybe we should drop this block or move it to [1]? At [1] it would only 
run if the more sophisticated vgaarb has been disabled.

The vgaarb now selects CONFIG_SCREEN_INFO and already tests for 
overlapping resources. There's nothing here that vgaarb shouldn't 
already do. Yet, I don't understand how only one of the can be true at a 
time.

[1] 
https://elixir.bootlin.com/linux/v6.18-rc3/source/include/linux/vgaarb.h#L55

>>
>>> +
>>> +    return false;
>>>   }
>>>   EXPORT_SYMBOL(video_is_primary_device);
>>
>> In particular, I have hardware that has this exact configuration where
>> two GPUs are marked as primary and have a 'boot_display' attribute: the
>> first one through vga_default_device(), the second one through the new
>> detection method.
>>
>> Is this intended?
>>
>> Kind regards
>> Aaron
>>
>
> I wouldn't have expected a case like this and I think it means there 
> is a logic error.
>
> Can you please file a kernel bugzilla with details about your system 
> and CC me?
>
> dmesg and lspci -vvnn please
>
> Also; please clarify which GPU shows something when booting up.

Agreed.

Best regards
Thomas


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

On 10/28/25 11:50 AM, Thomas Zimmermann wrote:
> Hi
> 
> Am 28.10.25 um 14:15 schrieb Mario Limonciello:
>> On 10/28/25 5:16 AM, Aaron Erhardt wrote:
>>>
>>> On Mon, Aug 11, 2025 at 11:26:05AM -0500, Mario Limonciello (AMD) wrote:
>>>> 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.
>>>
>>> I have a question regarding this change. To me, the function name
>>> video_is_primary_device() implies that there is only one primary GPU.
>>> I would also expect that the 'boot_display' attribute added later in
>>> the patch series based on this function is only set for one GPU, but
>>> that is not necessarily the case. Since I'm working on a user-space
>>> program that reads the 'boot_display' attribute, I need to know what
>>> behavior is intended in order to do a correct implementation.
>>>
>>>>
>>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>>>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>>>> ---
>>>> v10:
>>>>   * Rebase on 6.17-rc1
>>>>   * Squash 'fbcon: Stop using screen_info_pci_dev()'
>>>> ---
>>>>   arch/x86/video/video-common.c | 25 ++++++++++++++++++++++++-
>>>>   1 file changed, 24 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video- 
>>>> common.c
>>>> index 81fc97a2a837a..e0aeee99bc99e 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,11 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>>>>     bool video_is_primary_device(struct device *dev)
>>>>   {
>>>> +#ifdef CONFIG_SCREEN_INFO
>>>> +    struct screen_info *si = &screen_info;
>>>> +    struct resource res[SCREEN_INFO_MAX_RESOURCES];
>>>> +    ssize_t i, numres;
>>>> +#endif
>>>>       struct pci_dev *pdev;
>>>>         if (!dev_is_pci(dev))
>>>> @@ -34,7 +40,24 @@ 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;
>>>
>>> This can mark a VGA device as primary GPU.
> 
> Is the value returned from vga_default_device() eq to NULL?
> 
>>>
>>>> +
>>>> +#ifdef CONFIG_SCREEN_INFO
>>>> +    numres = screen_info_resources(si, res, ARRAY_SIZE(res));
>>>> +    for (i = 0; i < numres; ++i) {
>>>> +        if (!(res[i].flags & IORESOURCE_MEM))
>>>> +            continue;
>>>> +
>>>> +        if (pci_find_resource(pdev, &res[i]))
>>>> +            return true;
>>>> +    }
>>>> +#endif
>>>
>>> And then the new code can also choose a primary GPU.
> 
> Maybe we should drop this block or move it to [1]? At [1] it would only 
> run if the more sophisticated vgaarb has been disabled.
> 
> The vgaarb now selects CONFIG_SCREEN_INFO and already tests for 
> overlapping resources. There's nothing here that vgaarb shouldn't 
> already do. Yet, I don't understand how only one of the can be true at a 
> time.
> 
> [1] https://elixir.bootlin.com/linux/v6.18-rc3/source/include/linux/ 
> vgaarb.h#L55

There is a hunk at the end of vga_is_boot_device() which I think is 
causing this issue.  We have one VGA device which is NOT the boot 
display but the code figures if it found nothing this must be right.

	/*
	 * Vgadev has neither IO nor MEM enabled.  If we haven't found any
	 * other VGA devices, it is the best candidate so far.
	 */
	if (!boot_vga)
		return true;

I feel the right solution is to drop this now.

> 
>>>
>>>> +
>>>> +    return false;
>>>>   }
>>>>   EXPORT_SYMBOL(video_is_primary_device);
>>>
>>> In particular, I have hardware that has this exact configuration where
>>> two GPUs are marked as primary and have a 'boot_display' attribute: the
>>> first one through vga_default_device(), the second one through the new
>>> detection method.
>>>
>>> Is this intended?
>>>
>>> Kind regards
>>> Aaron
>>>
>>
>> I wouldn't have expected a case like this and I think it means there 
>> is a logic error.
>>
>> Can you please file a kernel bugzilla with details about your system 
>> and CC me?
>>
>> dmesg and lspci -vvnn please
>>
>> Also; please clarify which GPU shows something when booting up.
> 
> Agreed.
> 
> Best regards
> Thomas
> 
> 

On Mon, Aug 11, 2025 at 11:26:05AM -0500, Mario Limonciello (AMD) wrote:
> 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>
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>

I don't think you need my ack for this, but it does look fine to me.

I wish __screen_info_pci_dev() didn't have to use pci_get_base_class()
to iterate through all the devices, but you didn't change that and
maybe somebody will dream up a more efficient way someday.

Let me know if you need anything more from me.  Thanks for persevering
with this!

> ---
> v10:
>  * Rebase on 6.17-rc1
>  * Squash 'fbcon: Stop using screen_info_pci_dev()'
> ---
>  arch/x86/video/video-common.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
> index 81fc97a2a837a..e0aeee99bc99e 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,11 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>  
>  bool video_is_primary_device(struct device *dev)
>  {
> +#ifdef CONFIG_SCREEN_INFO
> +	struct screen_info *si = &screen_info;
> +	struct resource res[SCREEN_INFO_MAX_RESOURCES];
> +	ssize_t i, numres;
> +#endif
>  	struct pci_dev *pdev;
>  
>  	if (!dev_is_pci(dev))
> @@ -34,7 +40,24 @@ 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
> +	numres = screen_info_resources(si, res, ARRAY_SIZE(res));
> +	for (i = 0; i < numres; ++i) {
> +		if (!(res[i].flags & IORESOURCE_MEM))
> +			continue;
> +
> +		if (pci_find_resource(pdev, &res[i]))
> +			return true;
> +	}
> +#endif
> +
> +	return false;
>  }
>  EXPORT_SYMBOL(video_is_primary_device);
>  
> -- 
> 2.43.0
>