Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41062 )
Change subject: soc/intel/jasperlake: Apply FiVR related settings ......................................................................
Patch Set 16:
(12 comments)
https://review.coreboot.org/c/coreboot/+/41062/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41062/3//COMMIT_MSG@7 PS3, Line 7: soc/intel/jasperlake: Update FiVR UPDs for JSL.
Please remove the trailing period/dot from the git commit message summary.
Done
https://review.coreboot.org/c/coreboot/+/41062/3//COMMIT_MSG@8 PS3, Line 8:
What does this fix/improve? Describe, what FiVR is, and where you got the correct values from.
Done
https://review.coreboot.org/c/coreboot/+/41062/12//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41062/12//COMMIT_MSG@15 PS12, Line 15: are shut.
Were you able to measure the power savings?
Ack
https://review.coreboot.org/c/coreboot/+/41062/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41062/15//COMMIT_MSG@12 PS15, Line 12: BUG=None : BRANCH=None
if NONE, u can ignore specifying
Done
https://review.coreboot.org/c/coreboot/+/41062/9/src/mainboard/intel/jasperl... File src/mainboard/intel/jasperlake_rvp/ramstage.c:
https://review.coreboot.org/c/coreboot/+/41062/9/src/mainboard/intel/jasperl... PS9, Line 2: * This file is part of the coreboot project.
Please update header as per latest header […]
Ack
https://review.coreboot.org/c/coreboot/+/41062/3/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/41062/3/src/soc/intel/jasperlake/fs... PS3, Line 150: /* Fivr */
Add blank line above?
Done
https://review.coreboot.org/c/coreboot/+/41062/12/src/soc/intel/jasperlake/f... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/41062/12/src/soc/intel/jasperlake/f... PS12, Line 171: 420
shouldn't this be 420*2. […]
Done
https://review.coreboot.org/c/coreboot/+/41062/15/src/soc/intel/jasperlake/f... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/41062/15/src/soc/intel/jasperlake/f... PS15, Line 163:
remove empty line
Done
https://review.coreboot.org/c/coreboot/+/41062/15/src/soc/intel/jasperlake/f... PS15, Line 166: /* Enable External V1P05 Rail in: BIT0:S0i1/S0i2,BIT1:S0i3, BIT2:S3, BIT3:S4, BIT4:S5 */
96
Done
https://review.coreboot.org/c/coreboot/+/41062/15/src/soc/intel/jasperlake/f... PS15, Line 170:
remove empty line
Done
https://review.coreboot.org/c/coreboot/+/41062/15/src/soc/intel/jasperlake/f... PS15, Line 183:
same
Done
https://review.coreboot.org/c/coreboot/+/41062/15/src/soc/intel/jasperlake/f... PS15, Line 197: /* SATA */
give a empty line here before starting SATA
Done