Attention is currently required from: Jason Glenesk, Raul Rangel, EricKY Cheng, Matt DeVillier, Caveh Jalali, Paul Menzel, Tim Wawrzynczak, Eric Peers, Fred Reitberger, Karthikeyan Ramasubramanian, Boris Mittelberg, Felix Held.
Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68471 )
Change subject: common/acpi/dptc: Implement DTTS Proposal ......................................................................
Patch Set 25:
(1 comment)
File src/soc/amd/common/acpi/dptc.asl:
https://review.coreboot.org/c/coreboot/+/68471/comment/60c7e972_5b69a587 PS12, Line 44: TIN4
As your and Raul suggestions, you mean move all these change out of dptc.asl? […]
Please don't resolve comments that are still incomplete, since it makes it difficult to see there are still unanswered questions.
Yes, all of this logic should be moved into a variant-specific `.asl` file, since this tuning is not common to all Mendocino SOCs.
My idea would be to do something like: `src/soc/amd/common/acpi/dptc.asl`
Update `_SB.DPTC()` to call the variant-specific DPTC method, if it exists. That function will need to be called after the low/no battery check, so all of the Skyrim variants still support that feature. There are no convertible Whiterun devices, so in this case, we can simply do:
``` /* If _SB.DTHL is not present, then DPTC Tablet Mode is not enabled. * Throttle the SOC if the battery is not present (BTEX), the battery level is critical * (BFCR), or the battery is cutoff (BFCT). */ If (CondRefOf (_SB.DTHL) && (!_SB.PCI0.LPCB.EC0.BTEX || _SB.PCI0.LPCB.EC0.BFCR || _SB.PCI0.LPCB.EC0.BFCT)) { _SB.DTHL() Return (Zero) }
[[[ new code start ]]] /* If _SB.DVAR is not present, then variant-specific DPTC is not enabled. */ If (CondRefOf (_SB.DVAR)) { _SB.DVAR() Return (Zero) } [[[ new code end ]]]
/* If _SB.DTAB is not present, then DPTC Tablet Mode is not enabled. */ If (CondRefOf (_SB.DTAB) && (_SB.PCI0.LPCB.EC0.TBMD == One)) { _SB.DTAB() Return (Zero) } ``` - NOTE: `_SB.DVAR()` is just my example name for the variant-specific DPTC method. Feel free to pick whatever you like that makes sense.
Add a new `dptc_override.asl` to `src/mainboard/google/skyrim/variants/winterhold` (or some name that makes sense). Then, `src/mainboard/google/skyrim/dsdt.asl` can `#include` that file before `soc.asl` so the override function is visible and callable by the common `_SB.DPTC()`.
I think this limits the variant-specific `asl` changes in `src/soc/amd/common/acpi/dptc.asl` to checking if the variant-specific method exists and calling it, just like any other mode/feature (e.g., low/no battery, tablet mode).
@Raul - Thoughts on this kind of approach?