Attention is currently required from: Subrata Banik, Paul Menzel, Tim Wawrzynczak, Karthik Ramasubramanian. Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63675 )
Change subject: soc/intel/jasperlake: Enable fewer wakeups to reduce SoC power consumption ......................................................................
Patch Set 4:
(9 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63675/comment/09bc2c49_aea90811 PS3, Line 7: soc/intel/jasperlake: Add a workaround for cnvi
Maybe: […]
keep the prefix because this is for JSL only.
https://review.coreboot.org/c/coreboot/+/63675/comment/f86c97bc_746b7556 PS3, Line 9: add a workaround to mitigate : the higher SoC power consumption in S0iX
… work around higher SoC power consumption in S0iX, when CNVI has background activity.
Done
https://review.coreboot.org/c/coreboot/+/63675/comment/a55ebaa0_76bc4573 PS3, Line 15: Those settings are correct.
Please add the exact power consumption values.
The exact power consumption value is not a fixed value. It depends on what kind of background activity.
File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/63675/comment/5de463ae_dd04cc8d PS3, Line 430: AC9560(JfP2)
Please add a space before the (.
Done
https://review.coreboot.org/c/coreboot/+/63675/comment/1c2d57c7_0f426076 PS3, Line 430: chipsets :
Please remove the space before the colon.
Done
https://review.coreboot.org/c/coreboot/+/63675/comment/31b23c6a_17254b78 PS3, Line 431: AX201(HrP2)
Ditto.
Done
https://review.coreboot.org/c/coreboot/+/63675/comment/da3a697e_d9ed7533 PS3, Line 433: 1: Enabled(fewer wakes, lower power); 0: Disabled(more wakes, higher power)
Please use true/false, and add a space before the (.
Done
https://review.coreboot.org/c/coreboot/+/63675/comment/7ed429b6_4fdba0c5 PS3, Line 435: bool cnvi_s0ix_wa;
Add “power usage” or something similar to the name?
Done
File src/soc/intel/jasperlake/finalize.c:
https://review.coreboot.org/c/coreboot/+/63675/comment/021eee96_ad0a38bb PS3, Line 65: (1 << 0)
possible to create a macro?
Use the BIT macro here.