Re: [PATCH] PCI/VGA: Don't assume only VGA device found is the boot VGA device

[Kai-Heng Feng <kaihengf@nvidia.com>]

Hi Luke,

On 2024/10/15 4:04 PM, Luke Jones wrote:
> On Mon, 14 Oct 2024, at 5:25 PM, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> The ASUS GA605W has a NVIDIA PCI VGA device and an AMD PCI display device.
>>
>> ```
>> 65:00.0 VGA compatible controller: NVIDIA Corporation AD106M [GeForce
>> RTX 4070 Max-Q / Mobile] (rev a1)
>> 66:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI]
>> Strix [Radeon 880M / 890M] (rev c1)
>> ```
>>
>> The fallback logic in vga_is_boot_device() flags the NVIDIA dGPU as the
>> boot VGA device, but really the eDP is connected to the AMD PCI display
>> device.
>>
>> Drop this case to avoid marking the NVIDIA dGPU as the boot VGA device.
>>
>> Suggested-by: Alex Deucher <alexander.deucher@amd.com>
>> Reported-by: Luke D. Jones <luke@ljones.dev>
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3673
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/pci/vgaarb.c | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
>> index 78748e8d2dba..05ac2b672d4b 100644
>> --- a/drivers/pci/vgaarb.c
>> +++ b/drivers/pci/vgaarb.c
>> @@ -675,13 +675,6 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
>>   		return true;
>>   	}
>>
>> -	/*
>> -	 * Vgadev has neither IO nor MEM enabled.  If we haven't found any
>> -	 * other VGA devices, it is the best candidate so far.
>> -	 */
>> -	if (!boot_vga)
>> -		return true;
>> -
>>   	return false;
>>   }
>>
>> -- 
>> 2.43.0
> 
> Hi Mario,
> 
> I can verify that this does leave the `boot_vga` attribute set as 0 for the NVIDIA device.

Does the following diff work for you?
This variant should be less risky for most systems.

diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 78748e8d2dba..3fb734cb9c1b 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -675,6 +675,9 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
                 return true;
         }

+       if (vga_arb_integrated_gpu(&pdev->dev))
+               return true;
+
         /*
          * Vgadev has neither IO nor MEM enabled.  If we haven't found any
          * other VGA devices, it is the best candidate so far.


Kai-Heng

> 
> Tested-by: Luke D. Jones <luke@ljones.dev>