Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Mario Limonciello <superm1@kernel.org>
To: Aaron Erhardt <aer@tuxedocomputers.com>,
	David Airlie <airlied@gmail.com>,
	Bjorn Helgaas <bhelgaas@google.com>
Cc: "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"open list:DRM DRIVERS" <dri-devel@lists.freedesktop.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	"open list:PCI SUBSYSTEM" <linux-pci@vger.kernel.org>,
	"Daniel Dadap" <ddadap@nvidia.com>
Subject: Re: [PATCH v10 3/4] fbcon: Use screen info to find primary device
Date: Tue, 28 Oct 2025 08:15:48 -0500	[thread overview]
Message-ID: <fb519d0e-9d7f-4835-964a-c21fd24b10e8@kernel.org> (raw)
In-Reply-To: <e172ebf2-4b65-4781-b9e7-eb7bd4fa956a@tuxedocomputers.com>

On 10/28/25 5:16 AM, Aaron Erhardt wrote:
> 
> 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 <tzimmermann@suse.de>
>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> 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
> 

I wouldn't have expected a case like this and I think it means there is 
a logic error.

Can you please file a kernel bugzilla with details about your system and 
CC me?

dmesg and lspci -vvnn please

Also; please clarify which GPU shows something when booting up.

  reply	other threads:[~2025-10-28 13:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11 16:26 [PATCH v10 0/4] Adjust fbcon console device detection Mario Limonciello (AMD)
2025-08-11 16:26 ` [PATCH v10 1/4] Fix access to video_is_primary_device() when compiled without CONFIG_VIDEO Mario Limonciello (AMD)
2025-08-11 16:26 ` [PATCH v10 2/4] PCI/VGA: Replace vga_is_firmware_default() with a screen info check Mario Limonciello (AMD)
2025-09-10 14:33   ` Thomas Zimmermann
2025-10-12 18:23   ` Eric Biggers
2025-10-12 18:37     ` Mario Limonciello
2025-10-12 18:47       ` Eric Biggers
2025-10-12 19:06         ` Mario Limonciello
2025-10-13  7:04           ` Thomas Zimmermann
2025-10-13 20:55     ` Bjorn Helgaas
2025-10-28 22:50       ` Bjorn Helgaas
2025-08-11 16:26 ` [PATCH v10 3/4] fbcon: Use screen info to find primary device Mario Limonciello (AMD)
2025-09-04 20:42   ` Bjorn Helgaas
2025-10-28 10:16   ` Aaron Erhardt
2025-10-28 13:15     ` Mario Limonciello [this message]
2025-10-28 16:50       ` Thomas Zimmermann
2025-10-28 21:25         ` Mario Limonciello
2025-08-11 16:26 ` [PATCH v10 4/4] DRM: Add a new 'boot_display' attribute Mario Limonciello (AMD)
2025-09-10 14:28   ` Thomas Zimmermann
2025-09-03  4:42 ` [PATCH v10 0/4] Adjust fbcon console device detection Mario Limonciello
2025-09-04 22:36 ` Mario Limonciello

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fb519d0e-9d7f-4835-964a-c21fd24b10e8@kernel.org \
    --to=superm1@kernel.org \
    --cc=aer@tuxedocomputers.com \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=bhelgaas@google.com \
    --cc=christian.koenig@amd.com \
    --cc=ddadap@nvidia.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox