Re: [PATCH v2] PCI/VGA: Enable firmware default device detection on LoongArch

[Thomas Zimmermann]

Hi

Am 29.10.26 um 04:34 schrieb wuqianhai@xxxxxxxxxxx:
> From: wuqianhai <wuqianhai@xxxxxxxxxxx>
>
> On LoongArch, PCI resource assignment completes during subsys_initcall
> (acpi_init), before vga_arb_device_init() runs.  By that time the
> firmware framebuffer BAR may already have been relocated, leaving
> screen_info's lfb_base stale.  Enable vga_is_firmware_default() for
> LoongArch and refresh screen_info in vga_arb_device_init() before the
> boot device is matched.
>
> Unlike x86, sysfb_primary_display is only defined when CONFIG_SYSFB is
> enabled on LoongArch, so have VGA_ARB select SYSFB there.
>
> Signed-off-by: wuqianhai <wuqianhai@xxxxxxxxxxx>
> ---
> Changes in v2:
> - Add VGA_ARB's select of SYSFB for LoongArch to avoid an undefined
>    reference when CONFIG_SCREEN_INFO is not set.
> - Restrict the screen_info_apply_fixups() call to CONFIG_LOONGARCH so
>    x86 and other architectures are unaffected.
> - Enable vga_is_firmware_default() for LoongArch.
> ---
>   drivers/pci/Kconfig  |  1 +
>   drivers/pci/vgaarb.c | 12 +++++++++++-
>   2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 0c7408509ba2..55cc60f5d312 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -256,6 +256,7 @@ config VGA_ARB
>   	default y
>   	depends on !S390
>   	select SCREEN_INFO if X86
> +	select SYSFB if LOONGARCH
>   	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 3e58b2c05c85..1b7e915629f9 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -566,7 +566,7 @@ EXPORT_SYMBOL(vga_put);
>   
>   static bool vga_is_firmware_default(struct pci_dev *pdev)
>   {
> -#if defined CONFIG_X86
> +#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
>   	return pdev == screen_info_pci_dev(&sysfb_primary_display.screen);
>   #else
>   	return false;
> @@ -1527,6 +1527,16 @@ static int __init vga_arb_device_init(void)
>   	int rc;
>   	struct pci_dev *pdev;
>   
> +#if defined(CONFIG_LOONGARCH)
> +	/*
> +	 * On LoongArch, PCI resource assignment completes during
> +	 * subsys_initcall (acpi_init), before the VGA arbiter runs.  Refresh
> +	 * screen_info so vga_is_firmware_default() can match the relocated
> +	 * BAR.
> +	 */
> +	screen_info_apply_fixups();
> +#endif

No per-arch defines in generic code.  All the setup should work the same 
on all architectures.  All global screen-info state should be maintained 
by drivers/firmware/sysfb.c. Otherwise the framebuffer setup gets even 
more convoluted than it already is.

My question is why does vga_is_firmware_default() run before 
sysfb_init()? [1] Does acpi_init call into it? [1] 
https://elixir.bootlin.com/linux/v7.3/source/drivers/firmware/sysfb.c#L225


Apart from that, we can decouple screen_info_apply_fixups() from 
sysfb_init().  After relocation happens, the PCI code could call the 
helper directly and let it update the global screen_info.

Best regards
Thomas


> +
>   	rc = misc_register(&vga_arb_device);
>   	if (rc < 0)
>   		pr_err("error %d registering device\n", rc);

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)