On Thu, 31 Oct 2024, at 1:58 AM, Kai-Heng Feng wrote: > On 2024/10/25 8:55 PM, Alex Deucher wrote: >> External email: Use caution opening links or attachments >> >> >> On Fri, Oct 25, 2024 at 3:51 AM Kai-Heng Feng <kaihengf@xxxxxxxxxx> wrote: >>> >>> >>> >>> On 2024/10/23 11:27 PM, Alex Deucher wrote: >>>> External email: Use caution opening links or attachments >>>> >>>> >>>> On Tue, Oct 22, 2024 at 9:27 PM Kai-Heng Feng <kaihengf@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxx> >>>>>>>>> >>>>>>>>> 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@xxxxxxx> >>>>>>>>> Reported-by: Luke D. Jones <luke@xxxxxxxxxx> >>>>>>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3673 >>>>>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> >>>>>>>>> --- >>>>>>>>> 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. >>>> >>>> I think the problem is that the boot GPU is being conflated with vga >>>> arb. In this case the iGPU has no VGA so has no reason to be involved >>>> in vga arb. Trying to mess with any vga related resources on it could >>>> be problematic. Do higher levels of the stack look at vga arb to >>>> determine the "primary" GPU? >>> >>> Hmm, I wonder if all those heuristic are needed for EFI based system? >>> >>> Can we assume that what being used by UEFI GOP is the primary GFX device? >> >> Yes, I believe so. The SBIOS should use the GOP device as determined >> by the user preference. I.e.., in the bios configuration you can >> generally select iGPU or PEG for the primary display. > > UEFI spec, 10.3.3.1 ACPI _ADR Device Path > > "The _ADR device path is used to contain video output device attributes to > support the Graphics Output Protocol. The device path can contain multiple _ADR > entries if multiple video output devices are displaying the same output." > > Luke, can you please see what are the _ADR values of the iGPU and dGPU? > Maybe we > can find which one was used by GOP this way. I'm not sure what I'm looking at here, but initial search shows: Device (VGA) { Name (_ADR, Zero) // _ADR: Address Name (DOSA, Zero) Method (_DOS, 1, NotSerialized) // _DOS: Disable Output Switching { DOSA = Arg0 } Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices { M460 ("PLA-ASL-\\_SB.PCI0.GPPA.VGA._DOD\n", Zero, Zero, Zero, Zero, Zero, Zero) Return (Package (0x07) { 0x00010110, 0x00010210, 0x00010220, 0x00010230, 0x00010240, 0x00031000, 0x00032000 }) } Device (LCD) { Name (_ADR, 0x0110) // _ADR: Address Name (BCLB, Package (0x34) { https://gitlab.com/asus-linux/reverse-engineering/-/blob/master/uncategorized/GA605WI/dsdt.dsl?ref_type=heads#L4666 And: Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices { Return (Package (0x01) { 0x8000A450 }) } Device (LCD0) { Method (_ADR, 0, Serialized) // _ADR: Address { Return (0x8000A450) } https://gitlab.com/asus-linux/reverse-engineering/-/blob/master/uncategorized/GA605WI/ssdt1.dsl?ref_type=heads#L411 The links are direct to the lines I thought were relevant in the dumped DSDT. Luke. > > Kai-Heng > >> >> Alex >> >>> >>> Kai-Heng >>> >>>> >>>> Alex >>>> >>>>> >>>>> 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@xxxxxxxxxx> >>>>>>> >>>>> >>>