Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 230: register "policies.enabled.passive" = "1" : register "policies.enabled.critical" = "1" : register "policies.enabled.active" = "1"
could these be auto-detected by non-zero values in the different policy structures?
Good idea.
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 261: .source=DPTF_CPU, .target=DPTF_CPU, .temp=95, .period=5000}"
you could define a macro for generating these: […]
Oooh, now that's thinking with your noggin, that's a great idea!
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 294: Fan Performance Control (Percent, Speed, Noise, Power)
Not sure if you want to go there but it seems like this table could be calculated with some input va […]
For this table, RPM is nearly perfectly linear with %, although noise and power are not quite as linear...
My guess is that, because we have fine-grained control on, it probably linearly interpolates between each setpoint.
Also I would guess that power could be used if you use one of the more advanced policies, but I'm guessing noise level & power are ignored in our implementation.
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 311: 2
this seems to be hardcoded in the ASL, maybe it should just default to '2' and allow overriding?
This is supposed to be the hysteresis inherent in the system, so either analog hysteresis on a thermocouple or digital hysteresis in the firmware. Also the entire DPTF implementation was "customized" for ChromeOS. What if I shove generating these in the vendorcode CL instead and leave them out here?