Attention is currently required from: David Wu, Tim Wawrzynczak, Ren Kuo, Nick Vaccaro, Alan Huang, EricR Lai. Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62466 )
Change subject: mb/google/brya/var/kano: Update TDP PL1, PL2, and PL4 for U28 SKUs ......................................................................
Patch Set 3:
(5 comments)
Patchset:
PS3:
@Sumeet, in variant_update_power_limits not change the soc_config value, only update the dpft settin […]
I feel it's fine.
File src/mainboard/google/brya/variants/kano/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/62466/comment/ac3a009e_0c141ab5 PS3, Line 134: 20000 Lowering PL1 max power value will impact the overall system performance.
https://review.coreboot.org/c/coreboot/+/62466/comment/03d6aeb9_06c96221 PS3, Line 141: 43000 Normally, we keep max and min power value the same for PL2. So, I would suggest if you are changing PL2 max from 40000mW to 43000mW, change PL2 min as well to 43000mW.
File src/mainboard/google/brya/variants/kano/ramstage.c:
https://review.coreboot.org/c/coreboot/+/62466/comment/1f4defef_578d6ae7 PS3, Line 10: 43000 Normally, we keep max and min value the same for PL2. So, I would suggest if you are changing PL2 max from 40000mW to 43000mW, change PL2 min as well to 43000mW.
https://review.coreboot.org/c/coreboot/+/62466/comment/95d8b4a9_89871f7f PS3, Line 10: 20000 Do you mean these new values are suggested by your thermal team for this platform ? Is that the reason these values are changed here ? Can you please add me in this bug, I can't access it. I would like to understand the reason behind this change. Thanks.
If you are lowering the PL1 max value from 28000mW to 20000mW, it will impact the overall system performance. Did you test with any performance benchmarks with this new value ? Did you check any impact ?