Attention is currently required from: Jason Glenesk, Jason Nien, Matt DeVillier, Martin Roth, Tim Van Patten, Fred Reitberger, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67189 )
Change subject: soc/amd: Throttle SOC during low/no battery ......................................................................
Patch Set 7:
(2 comments)
File src/mainboard/google/skyrim/variants/skyrim/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/67189/comment/a770ef58_6d9b3cf2 PS5, Line 234: # Normal
The Normal values should have existed when `acipgen_dptci()` was added for Mendocino. […]
No one is setting `dptc_enable`, so we don't have to worry about writing incorrect data. We should move the skyrim specific CL into it's own CL. This makes reverting easier.
I would add all the non-low-battery config in a new CL. That way you can explain where you got the values from, and why they are correct for skyrim. I would then have a different CL that adds the low-battery overrides and also sets `dptc_tablet_mode_enable`. Having the different CLs will make it easier to bisect and blame any regressions.
File src/soc/amd/mendocino/chip.h:
https://review.coreboot.org/c/coreboot/+/67189/comment/b66a3c8c_d94a00f4 PS7, Line 68: vrm_current_limit_mA Do we need to pass these into FSP as well?
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/thir...
If so, let's have a new CL that adds the 3 new parameters.
-
Raul Rangel (Code Review)