Re: [PATCH v3] PCI/VGA: Select SCREEN_INFO on X86

["Mario Limonciello (AMD) (kernel.org)" <superm1@kernel.org>]

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
>