[PATCH v2] PCI/VGA: Select SCREEN_INFO

[PATCH v2] PCI/VGA: Select SCREEN_INFO

[Mario Limonciello (AMD)]

commit 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with
a screen info check") introduced an implicit dependency upon SCREEN_INFO
by removing the open coded implementation.

If a user didn't have CONFIG_SCREEN_INFO set vga_is_firmware_default()
would now return false.  Add a select for SCREEN_INFO to ensure that the
VGA arbiter works as intended. Also drop the now dead code.

Reported-by: Eric Biggers <ebiggers@kernel.org>
Closes: https://lore.kernel.org/linux-pci/20251012182302.GA3412@sol/
Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with a screen info check")
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v2:
 * drop dead code (Ilpo)
---
 drivers/pci/Kconfig  | 1 +
 drivers/pci/vgaarb.c | 8 +-------
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 7065a8e5f9b14..c35fed47addd5 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -306,6 +306,7 @@ config VGA_ARB
 	bool "VGA Arbitration" if EXPERT
 	default y
 	depends on (PCI && !S390)
+	select SCREEN_INFO
 	help
 	  Some "legacy" VGA devices implemented on PCI typically have the same
 	  hard-decoded addresses as they did on ISA. When multiple PCI devices
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index b58f94ee48916..8c8c420ff5b55 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -556,13 +556,7 @@ EXPORT_SYMBOL(vga_put);
 
 static bool vga_is_firmware_default(struct pci_dev *pdev)
 {
-#ifdef CONFIG_SCREEN_INFO
-	struct screen_info *si = &screen_info;
-
-	return pdev == screen_info_pci_dev(si);
-#else
-	return false;
-#endif
+	return pdev == screen_info_pci_dev(&screen_info);
 }
 
 static bool vga_arb_integrated_gpu(struct device *dev)
-- 
2.43.0

Re: [PATCH v2] PCI/VGA: Select SCREEN_INFO

[Thomas Zimmermann]

Hi

Am 13.10.25 um 17:44 schrieb Mario Limonciello (AMD):
> commit 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with
> a screen info check") introduced an implicit dependency upon SCREEN_INFO
> by removing the open coded implementation.
>
> If a user didn't have CONFIG_SCREEN_INFO set vga_is_firmware_default()
> would now return false.  Add a select for SCREEN_INFO to ensure that the
> VGA arbiter works as intended. Also drop the now dead code.
>
> Reported-by: Eric Biggers <ebiggers@kernel.org>
> Closes: https://lore.kernel.org/linux-pci/20251012182302.GA3412@sol/
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with a screen info check")
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

I think we should take this patch in any case, even if it would not fix 
the reporter's problem.

Best regards
Thomas

> ---
> v2:
>   * drop dead code (Ilpo)
> ---
>   drivers/pci/Kconfig  | 1 +
>   drivers/pci/vgaarb.c | 8 +-------
>   2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 7065a8e5f9b14..c35fed47addd5 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -306,6 +306,7 @@ config VGA_ARB
>   	bool "VGA Arbitration" if EXPERT
>   	default y
>   	depends on (PCI && !S390)
> +	select SCREEN_INFO
>   	help
>   	  Some "legacy" VGA devices implemented on PCI typically have the same
>   	  hard-decoded addresses as they did on ISA. When multiple PCI devices
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index b58f94ee48916..8c8c420ff5b55 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -556,13 +556,7 @@ EXPORT_SYMBOL(vga_put);
>   
>   static bool vga_is_firmware_default(struct pci_dev *pdev)
>   {
> -#ifdef CONFIG_SCREEN_INFO
> -	struct screen_info *si = &screen_info;
> -
> -	return pdev == screen_info_pci_dev(si);
> -#else
> -	return false;
> -#endif
> +	return pdev == screen_info_pci_dev(&screen_info);
>   }
>   
>   static bool vga_arb_integrated_gpu(struct device *dev)

-- 
--
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 v2] PCI/VGA: Select SCREEN_INFO

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

On Mon, 13 Oct 2025, Mario Limonciello (AMD) wrote:

> commit 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with
> a screen info check") introduced an implicit dependency upon SCREEN_INFO
> by removing the open coded implementation.
> 
> If a user didn't have CONFIG_SCREEN_INFO set vga_is_firmware_default()
> would now return false.  Add a select for SCREEN_INFO to ensure that the
> VGA arbiter works as intended. Also drop the now dead code.
> 
> Reported-by: Eric Biggers <ebiggers@kernel.org>
> Closes: https://lore.kernel.org/linux-pci/20251012182302.GA3412@sol/
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with a screen info check")
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v2:
>  * drop dead code (Ilpo)
> ---
>  drivers/pci/Kconfig  | 1 +
>  drivers/pci/vgaarb.c | 8 +-------
>  2 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 7065a8e5f9b14..c35fed47addd5 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -306,6 +306,7 @@ config VGA_ARB
>  	bool "VGA Arbitration" if EXPERT
>  	default y
>  	depends on (PCI && !S390)
> +	select SCREEN_INFO
>  	help
>  	  Some "legacy" VGA devices implemented on PCI typically have the same
>  	  hard-decoded addresses as they did on ISA. When multiple PCI devices
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index b58f94ee48916..8c8c420ff5b55 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -556,13 +556,7 @@ EXPORT_SYMBOL(vga_put);
>  
>  static bool vga_is_firmware_default(struct pci_dev *pdev)
>  {
> -#ifdef CONFIG_SCREEN_INFO
> -	struct screen_info *si = &screen_info;
> -
> -	return pdev == screen_info_pci_dev(si);
> -#else
> -	return false;
> -#endif
> +	return pdev == screen_info_pci_dev(&screen_info);
>  }
>  
>  static bool vga_arb_integrated_gpu(struct device *dev)

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH v2] PCI/VGA: Select SCREEN_INFO

[Bjorn Helgaas]

On Mon, Oct 13, 2025 at 10:44:23AM -0500, Mario Limonciello (AMD) wrote:
> commit 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with
> a screen info check") introduced an implicit dependency upon SCREEN_INFO
> by removing the open coded implementation.
> 
> If a user didn't have CONFIG_SCREEN_INFO set vga_is_firmware_default()
> would now return false.  Add a select for SCREEN_INFO to ensure that the
> VGA arbiter works as intended. Also drop the now dead code.
> 
> Reported-by: Eric Biggers <ebiggers@kernel.org>
> Closes: https://lore.kernel.org/linux-pci/20251012182302.GA3412@sol/
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with a screen info check")
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>

Applied to for-linus for v6.18, thanks!

> ---
> v2:
>  * drop dead code (Ilpo)
> ---
>  drivers/pci/Kconfig  | 1 +
>  drivers/pci/vgaarb.c | 8 +-------
>  2 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 7065a8e5f9b14..c35fed47addd5 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -306,6 +306,7 @@ config VGA_ARB
>  	bool "VGA Arbitration" if EXPERT
>  	default y
>  	depends on (PCI && !S390)
> +	select SCREEN_INFO
>  	help
>  	  Some "legacy" VGA devices implemented on PCI typically have the same
>  	  hard-decoded addresses as they did on ISA. When multiple PCI devices
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index b58f94ee48916..8c8c420ff5b55 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -556,13 +556,7 @@ EXPORT_SYMBOL(vga_put);
>  
>  static bool vga_is_firmware_default(struct pci_dev *pdev)
>  {
> -#ifdef CONFIG_SCREEN_INFO
> -	struct screen_info *si = &screen_info;
> -
> -	return pdev == screen_info_pci_dev(si);
> -#else
> -	return false;
> -#endif
> +	return pdev == screen_info_pci_dev(&screen_info);
>  }
>  
>  static bool vga_arb_integrated_gpu(struct device *dev)
> -- 
> 2.43.0
> 

Re: [PATCH v2] PCI/VGA: Select SCREEN_INFO

[Bjorn Helgaas]

On Mon, Oct 13, 2025 at 12:55:25PM -0500, Bjorn Helgaas wrote:
> On Mon, Oct 13, 2025 at 10:44:23AM -0500, Mario Limonciello (AMD) wrote:
> > commit 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with
> > a screen info check") introduced an implicit dependency upon SCREEN_INFO
> > by removing the open coded implementation.
> > 
> > If a user didn't have CONFIG_SCREEN_INFO set vga_is_firmware_default()
> > would now return false.  Add a select for SCREEN_INFO to ensure that the
> > VGA arbiter works as intended. Also drop the now dead code.
> > 
> > Reported-by: Eric Biggers <ebiggers@kernel.org>
> > Closes: https://lore.kernel.org/linux-pci/20251012182302.GA3412@sol/
> > Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Fixes: 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with a screen info check")
> > Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> 
> Applied to for-linus for v6.18, thanks!

Oops, dropped because of this regression:

  https://lore.kernel.org/r/176038554347.1442.9483731885505420131@15dd6324cc71

> > ---
> > v2:
> >  * drop dead code (Ilpo)
> > ---
> >  drivers/pci/Kconfig  | 1 +
> >  drivers/pci/vgaarb.c | 8 +-------
> >  2 files changed, 2 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> > index 7065a8e5f9b14..c35fed47addd5 100644
> > --- a/drivers/pci/Kconfig
> > +++ b/drivers/pci/Kconfig
> > @@ -306,6 +306,7 @@ config VGA_ARB
> >  	bool "VGA Arbitration" if EXPERT
> >  	default y
> >  	depends on (PCI && !S390)
> > +	select SCREEN_INFO
> >  	help
> >  	  Some "legacy" VGA devices implemented on PCI typically have the same
> >  	  hard-decoded addresses as they did on ISA. When multiple PCI devices
> > diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> > index b58f94ee48916..8c8c420ff5b55 100644
> > --- a/drivers/pci/vgaarb.c
> > +++ b/drivers/pci/vgaarb.c
> > @@ -556,13 +556,7 @@ EXPORT_SYMBOL(vga_put);
> >  
> >  static bool vga_is_firmware_default(struct pci_dev *pdev)
> >  {
> > -#ifdef CONFIG_SCREEN_INFO
> > -	struct screen_info *si = &screen_info;
> > -
> > -	return pdev == screen_info_pci_dev(si);
> > -#else
> > -	return false;
> > -#endif
> > +	return pdev == screen_info_pci_dev(&screen_info);
> >  }
> >  
> >  static bool vga_arb_integrated_gpu(struct device *dev)
> > -- 
> > 2.43.0
> > 

Re: [PATCH v2] PCI/VGA: Select SCREEN_INFO

[Mario Limonciello]

On 10/13/25 3:47 PM, Bjorn Helgaas wrote:
> On Mon, Oct 13, 2025 at 12:55:25PM -0500, Bjorn Helgaas wrote:
>> On Mon, Oct 13, 2025 at 10:44:23AM -0500, Mario Limonciello (AMD) wrote:
>>> commit 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with
>>> a screen info check") introduced an implicit dependency upon SCREEN_INFO
>>> by removing the open coded implementation.
>>>
>>> If a user didn't have CONFIG_SCREEN_INFO set vga_is_firmware_default()
>>> would now return false.  Add a select for SCREEN_INFO to ensure that the
>>> VGA arbiter works as intended. Also drop the now dead code.
>>>
>>> Reported-by: Eric Biggers <ebiggers@kernel.org>
>>> Closes: https://lore.kernel.org/linux-pci/20251012182302.GA3412@sol/
>>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Fixes: 337bf13aa9dda ("PCI/VGA: Replace vga_is_firmware_default() with a screen info check")
>>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>>
>> Applied to for-linus for v6.18, thanks!
> 
> Oops, dropped because of this regression:
> 
>    https://lore.kernel.org/r/176038554347.1442.9483731885505420131@15dd6324cc71

Ah thanks for that.  I hadn't had non-x86 to test.

I'll try to cross compile to repro and come up with a solution.

> 
>>> ---
>>> v2:
>>>   * drop dead code (Ilpo)
>>> ---
>>>   drivers/pci/Kconfig  | 1 +
>>>   drivers/pci/vgaarb.c | 8 +-------
>>>   2 files changed, 2 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
>>> index 7065a8e5f9b14..c35fed47addd5 100644
>>> --- a/drivers/pci/Kconfig
>>> +++ b/drivers/pci/Kconfig
>>> @@ -306,6 +306,7 @@ config VGA_ARB
>>>   	bool "VGA Arbitration" if EXPERT
>>>   	default y
>>>   	depends on (PCI && !S390)
>>> +	select SCREEN_INFO
>>>   	help
>>>   	  Some "legacy" VGA devices implemented on PCI typically have the same
>>>   	  hard-decoded addresses as they did on ISA. When multiple PCI devices
>>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
>>> index b58f94ee48916..8c8c420ff5b55 100644
>>> --- a/drivers/pci/vgaarb.c
>>> +++ b/drivers/pci/vgaarb.c
>>> @@ -556,13 +556,7 @@ EXPORT_SYMBOL(vga_put);
>>>   
>>>   static bool vga_is_firmware_default(struct pci_dev *pdev)
>>>   {
>>> -#ifdef CONFIG_SCREEN_INFO
>>> -	struct screen_info *si = &screen_info;
>>> -
>>> -	return pdev == screen_info_pci_dev(si);
>>> -#else
>>> -	return false;
>>> -#endif
>>> +	return pdev == screen_info_pci_dev(&screen_info);
>>>   }
>>>   
>>>   static bool vga_arb_integrated_gpu(struct device *dev)
>>> -- 
>>> 2.43.0
>>>