* [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86
@ 2025-10-13 22:08 Mario Limonciello (AMD)
2025-10-14 6:50 ` Eric Biggers
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Mario Limonciello (AMD) @ 2025-10-13 22:08 UTC (permalink / raw)
To: mario.limonciello, bhelgaas, tzimmermann, superm1
Cc: Eric Biggers, Ilpo Järvinen, linux-pci
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. SCREEN_INFO is only used on X86 so add add a
conditional select for SCREEN_INFO to ensure that the VGA arbiter works
as intended.
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")
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/pci/Kconfig | 1 +
drivers/pci/vgaarb.c | 6 ++----
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 7065a8e5f9b14..f94f5d384362e 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 if X86
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..436fa7f4c3873 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -556,10 +556,8 @@ 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);
+#if defined CONFIG_X86
+ return pdev == screen_info_pci_dev(&screen_info);
#else
return false;
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86
2025-10-13 22:08 [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86 Mario Limonciello (AMD)
@ 2025-10-14 6:50 ` Eric Biggers
2025-10-14 8:24 ` Thomas Zimmermann
2025-10-14 22:27 ` Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Eric Biggers @ 2025-10-14 6:50 UTC (permalink / raw)
To: Mario Limonciello (AMD)
Cc: mario.limonciello, bhelgaas, tzimmermann, Ilpo Järvinen,
linux-pci
On Mon, Oct 13, 2025 at 05:08:26PM -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. SCREEN_INFO is only used on X86 so add add a
> conditional select for SCREEN_INFO to ensure that the VGA arbiter works
> as intended.
>
> 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")
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Eric Biggers <ebiggers@kernel.org>
- Eric
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86
2025-10-13 22:08 [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86 Mario Limonciello (AMD)
2025-10-14 6:50 ` Eric Biggers
@ 2025-10-14 8:24 ` Thomas Zimmermann
2025-10-14 22:02 ` Mario Limonciello (AMD) (kernel.org)
2025-10-14 22:27 ` Bjorn Helgaas
2 siblings, 1 reply; 5+ messages in thread
From: Thomas Zimmermann @ 2025-10-14 8:24 UTC (permalink / raw)
To: Mario Limonciello (AMD), mario.limonciello, bhelgaas
Cc: Eric Biggers, Ilpo Järvinen, linux-pci
Am 14.10.25 um 00:08 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. SCREEN_INFO is only used on X86 so add add a
> conditional select for SCREEN_INFO to ensure that the VGA arbiter works
> as intended.
>
> 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")
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> drivers/pci/Kconfig | 1 +
> drivers/pci/vgaarb.c | 6 ++----
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 7065a8e5f9b14..f94f5d384362e 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 if X86
On x86 screen_info comes from [1]. On other systems it's at [2].
You can try selecting CONFIG_SYSFB instead, but that will likely run
into trouble with CONFIG_EFI=n.
[1]
https://elixir.bootlin.com/linux/v6.17.1/source/arch/x86/kernel/setup.c#L214
[2]
https://elixir.bootlin.com/linux/v6.17.1/source/drivers/firmware/efi/efi-init.c#L63
I guess, the current patch should work for the use case. I'll keep in
mind to make the screen_info state easier to select.
Best regards
Thomas
> 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..436fa7f4c3873 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -556,10 +556,8 @@ 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);
> +#if defined CONFIG_X86
> + return pdev == screen_info_pci_dev(&screen_info);
> #else
> return false;
> #endif
--
--
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)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86
2025-10-14 8:24 ` Thomas Zimmermann
@ 2025-10-14 22:02 ` Mario Limonciello (AMD) (kernel.org)
0 siblings, 0 replies; 5+ messages in thread
From: Mario Limonciello (AMD) (kernel.org) @ 2025-10-14 22:02 UTC (permalink / raw)
To: Thomas Zimmermann, mario.limonciello, bhelgaas
Cc: Eric Biggers, Ilpo Järvinen, linux-pci
On 10/14/2025 3:24 AM, Thomas Zimmermann wrote:
>
>
> Am 14.10.25 um 00:08 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. SCREEN_INFO is only used on X86 so add add a
>> conditional select for SCREEN_INFO to ensure that the VGA arbiter works
>> as intended.
>>
>> 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")
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> drivers/pci/Kconfig | 1 +
>> drivers/pci/vgaarb.c | 6 ++----
>> 2 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
>> index 7065a8e5f9b14..f94f5d384362e 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 if X86
>
> On x86 screen_info comes from [1]. On other systems it's at [2].
>
> You can try selecting CONFIG_SYSFB instead, but that will likely run
> into trouble with CONFIG_EFI=n.
>
> [1] https://elixir.bootlin.com/linux/v6.17.1/source/arch/x86/kernel/
> setup.c#L214
> [2] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/firmware/
> efi/efi-init.c#L63
>
> I guess, the current patch should work for the use case. I'll keep in
> mind to make the screen_info state easier to select.
>
Yeah I noticed these when I was putting this together and when I
compared the pre-337bf13aa9dda code I decided KISS is the better
approach, especially to fix the 6.18 regression.
> Best regards
> Thomas
>
>> 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..436fa7f4c3873 100644
>> --- a/drivers/pci/vgaarb.c
>> +++ b/drivers/pci/vgaarb.c
>> @@ -556,10 +556,8 @@ 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);
>> +#if defined CONFIG_X86
>> + return pdev == screen_info_pci_dev(&screen_info);
>> #else
>> return false;
>> #endif
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86
2025-10-13 22:08 [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86 Mario Limonciello (AMD)
2025-10-14 6:50 ` Eric Biggers
2025-10-14 8:24 ` Thomas Zimmermann
@ 2025-10-14 22:27 ` Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2025-10-14 22:27 UTC (permalink / raw)
To: Mario Limonciello (AMD)
Cc: mario.limonciello, bhelgaas, tzimmermann, Eric Biggers,
Ilpo Järvinen, linux-pci
On Mon, Oct 13, 2025 at 05:08:26PM -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. SCREEN_INFO is only used on X86 so add add a
> conditional select for SCREEN_INFO to ensure that the VGA arbiter works
> as intended.
>
> 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")
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Applied to pci/for-linus for v6.18, thanks!
> ---
> drivers/pci/Kconfig | 1 +
> drivers/pci/vgaarb.c | 6 ++----
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 7065a8e5f9b14..f94f5d384362e 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 if X86
> 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..436fa7f4c3873 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -556,10 +556,8 @@ 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);
> +#if defined CONFIG_X86
> + return pdev == screen_info_pci_dev(&screen_info);
> #else
> return false;
> #endif
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-14 22:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-13 22:08 [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86 Mario Limonciello (AMD)
2025-10-14 6:50 ` Eric Biggers
2025-10-14 8:24 ` Thomas Zimmermann
2025-10-14 22:02 ` Mario Limonciello (AMD) (kernel.org)
2025-10-14 22:27 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox