acpi: add Darwin OSI quirk for Apple Mac hardware

[guest-seuros]

Summary

Mac firmware hides the Intel integrated GPU (iGPU) on dual GPU x86
systems, i.e., with AMD/NVIDIA dGPUs, when the Darwin OSI is not
installed via ACPI.

Prior to this change, FreeBSD always used the dGPU. This is fine in
practice, but consumed more power than when the iGPU is used,
resulting in reduced battery life.

Linux handles this in drivers/acpi/osi.c by detecting Apple
hardware via DMI, disabling all Windows OSI strings, and
by explicitly installing the Darwin OSI ACPI handler. This change
applies equivalent logic to the acpi(4) driver on FreeBSD.

This feature can be enabled/disabled using the
hw.acpi.apple_darwin_osi tunable. Setting this tunable to 0
restores the previous behavior by explicitly disabling the added
support.

Test Plan

NB: this quirk may behave differently between differently on various Mac models.

@ngie 's Notes:

1. Verified that the Intel bridge devices were enumerated when the quirk was disabled, but not when it was enabled.
   1. This behaves differently depending on the models and whether or not the Intel/DRM modules have been loaded in the kernel. On my MacBookPro 16,1, for instance, all it did was enumerate additional Northbridge/Thunderbolt devices--which presumably would entail the iGPU, if the appropriate driver support was present in the kernel.
2. Confirmed that dmesg showed/hid the relevant debug messages when booted with boot -v and when not booted with boot -v.

• hw.acpi.apple_darwin_osi=0: pciconf -lv (MacBookPro 16,1): https://reviews.freebsd.org/P692
• hw.acpi.apple_darwin_osi=1: pciconf -lv (MacBookPro 16,1): https://reviews.freebsd.org/P694

@guest-seuros 's Notes:

% pciconf -lv | grep -A4 "class=0x0300"
vgapci1@pci0:0:2:0 │ Intel Crystal Well (Iris Pro 5200) 
vgapci0@pci0:1:0:0 │ AMD Venus XT (R9 M370X)

Note (I did not try this with NVIDIA based Apple; those are older)

[jrtc27]

sys/dev/acpica/acpi.c

4846

Why's only this one guarded by bootverbose? If anything isn't the error more important to show?

[guest-seuros]

sys/dev/acpica/acpi.c

4846

The success message document an intentional ACPI policy override.

The failure path is diagnostic-only, so it is appropriately gated by bootverbose to avoid user-facing noise.

I tested this on many Apple systems ranging from 2011 to 2019.

While it behaves correctly on those platforms, older or less common models (pre 2011) may expose ACPICA edge cases, which is why the failure path is keept for debuging. (i will get few of those machines soon to test).

If this code proves to be stable across every range of Apple hardware and ACPICA versions, the failure branch itself could potentially and the logging be removed in the future.

[obiwac]

looks good!

which function are you adding this to, acpi_reset_interfaces? phab says context not available and the line numbers don't seem to match up.

sys/dev/acpica/acpi.c

4839

nit but: wouldn't you want to want to warn if removing the window vendor strings failed too, if you think adding the Darwin vendor string failing is likely enough to warrant a warning message?

[adrian]

Any other comments? I'd like to land this into -HEAD this week and see how it plays out!

[ngie]

Would this please make this into a SYSCTL+TUNABLE? It would be nice if that's at all possible, because then you could do graphics switching post-boot. I don't know if it's possible, but it would be nice. macOS supports graphics switching dynamically at runtime/based on power states.
Also, are there any caveats associated with picking one of the GPUs over the other, e.g., one supports dual displays at a higher resolution, whereas the other only supports single displays at a lower resolution?

[guest-seuros]

@ngie i prefered to keep this diff scoped to ACPI discovery only, but the switching is important too.

This change is limited to ACPI OSI behavior and device enumeration.
Runtime switching is planned as follow-up work once this has landed and been validated on more real hardware.

In practice, the immediate goal is power management rather than seamless switching.
On the affected Apple systems, both GPUs are currently powered by default. Disabling one of them is already doable and I have tested this successfully, which results in the expected power savings.

Runtime switching is technically possible, but is not fully solved yet. The current state is:
Both GPUs powered => AMD only: works
AMD only => re-enable Intel and disable AMD: works
2 GPUs powered => Intel only -> re-enable AMD: currently crashes

This makes me think that AMD reinitialisation still needs more work, and I'm investigating the correct bring-up sequence.

[guest-seuros]

sys/dev/acpica/acpi.c

4839

Adding the Darwin OSI string is a requirement for this change.
Removing the Windows vendor strings is a best-effort cleanup.

I can remove this cleanup code if needed.

[obiwac]

sys/dev/acpica/acpi.c

4839

no, that's fair. For some reason i thought https://www.kernel.org/doc/Documentation/acpi/osi.txt (last section) implied clearing the windows strings as a necessary requirement

[ngie]

In D54762#1252920, @adrian wrote:

Any other comments? I'd like to land this into -HEAD this week and see how it plays out!

Taking the patch for a spin on my MBP16,1 right now if I can get the silly image to build (was running into unrelated build problems earlier with llvm...).

[ngie]

sys/dev/acpica/acpi.c

4843–4845

I'm going to hide the proposed device_printf(..) behind bootverbose -- it doesn't really benefit general purpose users.

[ngie]

• Clean up comments regarding this feature and clarify items about how the OSI gets installed/removed.
• Add a bootverbose message to note when the quirk does not apply, i.e., when the logic is run on non-Apple platforms.
• Hide the OSI install message behind bootverbose.

[ngie]

Fix accidental build break

[ngie]

Remove redundant _interface I added in the tunable name

[ngie]

I passed it back to @guest-seuros so he can look at the diff, make any desired changes, and get credit for the diff once it lands.

[ngie]

sys/dev/acpica/acpi.c

4919–4942

I might have garbled up the comment a bit. The flow probably could stand to be fixed, but I think the content is close to the desired end-state.