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

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

On 2024/10/22 9:04 PM, Alex Deucher wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Tue, Oct 22, 2024 at 2:31 AM Kai-Heng Feng <kaihengf@nvidia.com> wrote:
>>
>> 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;
>> +
> 
> The problem is that the integrated graphics does not support VGA.

Right, so the check has to be used much earlier.

I wonder does the integrated GFX have _DOD/_DOS while the discrete one doesn't? 
If that's the case, vga_arb_integrated_gpu() can be used to differentiate which 
one is the boot GFX.

Kai-Heng

> 
> Alex
> 
>>           /*
>>            * 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>
>>