Attention is currently required from: Jason Glenesk, Jason Nien, EricKY Cheng, Matt DeVillier, Chris Wang, Martin Roth, Fred Reitberger, Felix Held.
Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68649 )
Change subject: soc/amd/mendocino: Enhance DPTC_INPUT to support 13 DPTC thermal parameters ......................................................................
Patch Set 31:
(1 comment)
File src/soc/amd/mendocino/root_complex.c:
https://review.coreboot.org/c/coreboot/+/68649/comment/56309812_d9f5a44b PS16, Line 393: #else
IMO we need to refactor the parameter code and allow us to specify the parameters we want to change in the device tree.
I like the idea of simplifying in this area and reducing duplication of values, but it needs to be done carefully. For example, low/no battery will throttle the current limits (among others), while the thermal tables will modify other limits. In that case, this type of scenario could occur:
1. Normal Mode, fully unthrottled 1. Thermal limit reached, throttle based on tables B-E 1. Battery drains, throttle SOC 1. Charger is plugged in, battery charges, unthrottle the SOC
Step (4) is where the complexity comes in: which parameters need to be restored (and to what values) when there is overlap of modified values between the tables (e.g., `sustained_power_limit_mW`)? Setting every value for each table means we don't need to know which table was previously applied, since none of the old values will persist.
In this case, using your example tables, we need to apply both `stt_default_config` (to restore everything, including the VRM limits) and then the appropriate thermal table to apply the desired `sustained_power_limit_mW` (among others). Currently, we only support setting one (full) table at a time, so that logic would need to be updated as well.
It's just code, so anything is possible. I'd prefer to leave this as a follow-on improvement though, so we can make progress here and then refactor once we have all of the requirements.