On Tue, Jun 17, 2025 at 11:06:00AM -0500, Mario Limonciello wrote: > 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. > > Sure, I was curious what vga_is_firmware_default() returns for each of the GPUs when run, regardless of whether or not it's currently being run in the existing code - I'm wondering if vga_is_firmware_default() can be a better tie breaker (or at least a first line tie breaker) than the one you have now which tests for iGPU. I kind of went off on a tangent about vga_is_boot_device() being possibly unreliable for a second time caller when I was checking the callers of vga_is_firmware_default().
