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

["Luke Jones" <luke@ljones.dev>]

On Wed, 23 Oct 2024, at 3:27 AM, Kai-Heng Feng wrote:
> 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.

Discrete might be missing the _DOS? I'm not sure how to interpret the gpu related DSL. You can see the dump here https://gitlab.com/asus-linux/reverse-engineering/-/tree/master/uncategorized/GA605WI?ref_type=heads

ssdt1.dsl contains _DOD and looks to be the discrete unit, does not contain _DOS. the dsdt contains both under "Device (VGA)"

Regards,
Luke.

> 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>
>>>