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

Mario Limonciello posted 9 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH v9 8/9] fbcon: Use screen info to find primary device
Posted by Mario Limonciello 2 months, 3 weeks ago
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
Re: [PATCH v9 8/9] fbcon: Use screen info to find primary device
Posted by Bjorn Helgaas 2 months, 2 weeks ago
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
Posted by Mario Limonciello 2 months, 2 weeks ago
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
Posted by Bjorn Helgaas 2 months, 2 weeks ago
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
Posted by Mario Limonciello 2 months, 2 weeks ago
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
>>>>
>>