Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42436 )
Change subject: mb/google/volteer: Override power limits with SKU-specific limits ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42436/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/42436/1/src/mainboard/google/voltee... PS1, Line 28: override_power_limits
i think we need checks here to never increase the limits beyond […]
You're right, the PLn limits are SoC-specific (the PsysPLn ones are mainboard/system-specific). Intel came up with these numbers from our reference board's thermal & VR solution. I don't believe we are allowing partners that much latitude with those aspects of system design, so I think these numbers should apply to all of them. LMK if you disagree.
https://review.coreboot.org/c/coreboot/+/42436/1/src/mainboard/google/voltee... PS1, Line 46: power_limits_config
you could make this into an array and put all the values in devicetree itself
That's an interesting thought. With nice symbolic constants for the indexes, that may be more readable even. #define POWER_LIMITS_4_CORE 0 #define POWER_LIMITS_2_CORE 1 then use those in devicetree. let me give that a try.