Re: [PATCH] PCI/VGA: Look at all PCI display devices in VGA arbiter

[Mario Limonciello <superm1@kernel.org>]

On 6/17/25 10:53 AM, Daniel Dadap wrote:
> On Mon, Jun 16, 2025 at 09:14:04PM +0200, Lukas Wunner wrote:
>> On Mon, Jun 16, 2025 at 10:05:48AM -0500, Daniel Dadap wrote:
>>> On Sat, Jun 14, 2025 at 09:04:52AM +0200, Lukas Wunner wrote:
>>>> On Fri, Jun 13, 2025 at 04:47:20PM -0500, Daniel Dadap wrote:
>>>>> Ideally we'd be able to actually query which GPU is connected to
>>>>> the panel at the time we're making this determination, but I don't
>>>>> think there's a uniform way to do this at the moment. Selecting the
>>>>> integrated GPU seems like a reasonable heuristic, since I'm not
>>>>> aware of any systems where the internal panel defaults to dGPU
>>>>> connection, since that would defeat the purpose of having a hybrid
>>>>> GPU system in the first place
>>>>
>>>> Intel-based dual-GPU MacBook Pros boot with the panel switched to the
>>>> dGPU by default.  This is done for Windows compatibility because Apple
>>>> never bothered to implement dynamic GPU switching on Windows.
>>>>
>>>> The boot firmware can be told to switch the panel to the iGPU via an
>>>> EFI variable, but that's not something the kernel can or should depend on.
>>>>
>>>> MacBook Pros introduced since 2013/2014 hide the iGPU if the panel is
>>>> switched to the dGPU on boot, but the kernel is now unhiding it since
>>>> 71e49eccdca6.
>>>
>>> This is good to know. Is vga_switcheroo initialized by the time the code
>>> in this patch runs? If so, maybe we should query switcheroo to determine
>>> the GPU which is connected to the panel and favor that one, then fall
>>> back to the "iGPU is probably right" heuristic otherwise.
>>
>> Right now vga_switcheroo doesn't seem to provide a function to query the
>> current mux state.
>>
>> The driver for the mux on MacBook Pros, apple_gmux.c, can be modular,
>> so may be loaded fairly late.
> 
> Yeah, that's what I was afraid of.
> 
>>
>> Personally I'm booting my MacBook Pro via EFI, so the GPU in use is
>> whatever efifb is talking to.  However it is possible to boot these
>> machines in a legacy CSM mode and I don't know what the situation
>> looks like in that case.
>>
> 
> Skimming through the code, this seems like the sort of thing that the
> existing check in vga_is_firmware_default() is testing. I'm not familiar
> enough with the relevant code to intuitively know whether it is supposed
> to work for UEFI or legacy VGA or both (I think both?).
> 
> Mario, on the affected system, what does vga_is_firmware_default() return
> on each of the GPUs? (I'd expect it to return true for the iGPU and false
> for the dGPU, but I think the (boot_vga && boot_vga->is_firmware_default)
> test would cause vga_is_boot_device() to return false if the vga_default
> is passed into it a second time. That made sense for the way that the
> function was originally called, to test if the passed-in vgadev is any
> "better" than vga_default, and from a quick skim it doesn't seem like it
> would get called multiple times in the new code either, but I worry that
> if someone else decides they need to call vga_is_boot_device() a second
> time in the future it might return a false result for vga_default.)
> 

Right "now" on an unpatched kernel it won't run 'at all' because 
vga_arb_device_init() only matches VGA class devices.

Both GPUs are not VGA compatible, which is what lead to the patch in 
this thread starting to match display class devices too.