On Mon, Aug 11, 2025 at 11:26:05AM -0500, Mario Limonciello (AMD) wrote:
> 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.

I have a question regarding this change. To me, the function name 
video_is_primary_device() implies that there is only one primary GPU.
I would also expect that the 'boot_display' attribute added later in 
the patch series based on this function is only set for one GPU, but 
that is not necessarily the case. Since I'm working on a user-space
program that reads the 'boot_display' attribute, I need to know what
behavior is intended in order to do a correct implementation.

> 
> Reviewed-by: Thomas Zimmermann <[email protected]>
> Suggested-by: Thomas Zimmermann <[email protected]>
> Suggested-by: Bjorn Helgaas <[email protected]>
> Signed-off-by: Mario Limonciello (AMD) <[email protected]>
> ---
> v10:
>  * Rebase on 6.17-rc1
>  * Squash 'fbcon: Stop using screen_info_pci_dev()'
> ---
>  arch/x86/video/video-common.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
> index 81fc97a2a837a..e0aeee99bc99e 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,11 @@ EXPORT_SYMBOL(pgprot_framebuffer);
>  
>  bool video_is_primary_device(struct device *dev)
>  {
> +#ifdef CONFIG_SCREEN_INFO
> +     struct screen_info *si = &screen_info;
> +     struct resource res[SCREEN_INFO_MAX_RESOURCES];
> +     ssize_t i, numres;
> +#endif
>       struct pci_dev *pdev;
>  
>       if (!dev_is_pci(dev))
> @@ -34,7 +40,24 @@ 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;

This can mark a VGA device as primary GPU.

> +
> +#ifdef CONFIG_SCREEN_INFO
> +     numres = screen_info_resources(si, res, ARRAY_SIZE(res));
> +     for (i = 0; i < numres; ++i) {
> +             if (!(res[i].flags & IORESOURCE_MEM))
> +                     continue;
> +
> +             if (pci_find_resource(pdev, &res[i]))
> +                     return true;
> +     }
> +#endif

And then the new code can also choose a primary GPU.

> +
> +     return false;
>  }
>  EXPORT_SYMBOL(video_is_primary_device);
>  

In particular, I have hardware that has this exact configuration where
two GPUs are marked as primary and have a 'boot_display' attribute: the
first one through vga_default_device(), the second one through the new
detection method.

Is this intended?

Kind regards
Aaron

Reply via email to