Gaggery Tsai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31505
Change subject: src/soc/intel/cannonlake: Add PsysPmax setting ......................................................................
src/soc/intel/cannonlake: Add PsysPmax setting
This patch feeds PsysPmax setting to FSP through UPD and adds a psys_pmax member in chip information so that we can set PsysPmax through DT. The PsysPmax needs to be set correctly mapping to maximum system power. Otherwise, system performance would be limited due to the default PsysPmax setting in FSP is only 21W.
BUG=None BRANCH=None TEST=Set psys_pmax to an exmaple value eg 101 in DT && put debug code in FSP to print the PsysPmax value before sending to Pcode, ensure the setting is correctly programmed.
Change-Id: Ia88ea17bc661a388c5b9bc3e59abc27c9f262977 Signed-off-by: Gaggery Tsai gaggery.tsai@intel.com --- M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/31505/1
diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h index cb9ad38..1f26f1e 100644 --- a/src/soc/intel/cannonlake/chip.h +++ b/src/soc/intel/cannonlake/chip.h @@ -216,6 +216,8 @@ uint32_t tdp_psyspl3_dutycycle; /* PL4 Value in Watts */ uint32_t tdp_pl4; + /* Estimated maximum platform power in Watts */ + uint16_t psys_pmax;
/* Intel Speed Shift Technology */ uint8_t speed_shift_enable; diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index c276c86..e7c1358 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -88,6 +88,13 @@
mainboard_silicon_init_params(params);
+ /* Set PsysPmax if it is available from DT */ + if (config->psys_pmax) { + /* PsysPmax is in unit of 1/8 Watt */ + tconfig->PsysPmax = config->psys_pmax * 8; + printk(BIOS_DEBUG, "psys_pmax = %d\n", tconfig->PsysPmax); + } + /* Unlock upper 8 bytes of RTC RAM */ params->PchLockDownRtcMemoryLock = 0;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31505 )
Change subject: src/soc/intel/cannonlake: Add PsysPmax setting ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/31505/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31505/1//COMMIT_MSG@17 PS1, Line 17: exmaple example
https://review.coreboot.org/#/c/31505/1/src/soc/intel/cannonlake/fsp_params.... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/#/c/31505/1/src/soc/intel/cannonlake/fsp_params.... PS1, Line 95: psys_pmax Should this be PsysPmax? since psys_pmax is in W and PsysPmax is in units of 1/8 W.
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31505
to look at the new patch set (#2).
Change subject: src/soc/intel/cannonlake: Add PsysPmax setting ......................................................................
src/soc/intel/cannonlake: Add PsysPmax setting
This patch feeds PsysPmax setting to FSP through UPD and adds a psys_pmax member in chip information so that we can set PsysPmax through DT. The PsysPmax needs to be set correctly mapping to maximum system power. Otherwise, system performance would be limited due to the default PsysPmax setting in FSP is only 21W.
BUG=None BRANCH=None TEST=Set psys_pmax to an example value eg 101 in DT && put debug code in FSP to print the PsysPmax value before sending to Pcode, ensure the setting is correctly programmed.
Change-Id: Ia88ea17bc661a388c5b9bc3e59abc27c9f262977 Signed-off-by: Gaggery Tsai gaggery.tsai@intel.com --- M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/31505/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31505 )
Change subject: src/soc/intel/cannonlake: Add PsysPmax setting ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31505/2/src/soc/intel/cannonlake/fsp_params.... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/#/c/31505/2/src/soc/intel/cannonlake/fsp_params.... PS2, Line 93: printk(BIOS_DEBUG, "psys_pmax = %dW\n", tconfig->PsysPmax); How is this helpful? Now this would just end up printing the default value in tconfig->PsysPmax?
Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31505 )
Change subject: src/soc/intel/cannonlake: Add PsysPmax setting ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31505/2/src/soc/intel/cannonlake/fsp_params.... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/#/c/31505/2/src/soc/intel/cannonlake/fsp_params.... PS2, Line 93: printk(BIOS_DEBUG, "psys_pmax = %dW\n", tconfig->PsysPmax);
How is this helpful? Now this would just end up printing the default value in tconfig->PsysPmax?
sorry, my bad..
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31505
to look at the new patch set (#3).
Change subject: src/soc/intel/cannonlake: Add PsysPmax setting ......................................................................
src/soc/intel/cannonlake: Add PsysPmax setting
This patch feeds PsysPmax setting to FSP through UPD and adds a psys_pmax member in chip information so that we can set PsysPmax through DT. The PsysPmax needs to be set correctly mapping to maximum system power. Otherwise, system performance would be limited due to the default PsysPmax setting in FSP is only 21W.
BUG=None BRANCH=None TEST=Set psys_pmax to an example value eg 101 in DT && put debug code in FSP to print the PsysPmax value before sending to Pcode, ensure the setting is correctly programmed.
Change-Id: Ia88ea17bc661a388c5b9bc3e59abc27c9f262977 Signed-off-by: Gaggery Tsai gaggery.tsai@intel.com --- M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/31505/3
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31505 )
Change subject: src/soc/intel/cannonlake: Add PsysPmax setting ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31505 )
Change subject: src/soc/intel/cannonlake: Add PsysPmax setting ......................................................................
src/soc/intel/cannonlake: Add PsysPmax setting
This patch feeds PsysPmax setting to FSP through UPD and adds a psys_pmax member in chip information so that we can set PsysPmax through DT. The PsysPmax needs to be set correctly mapping to maximum system power. Otherwise, system performance would be limited due to the default PsysPmax setting in FSP is only 21W.
BUG=None BRANCH=None TEST=Set psys_pmax to an example value eg 101 in DT && put debug code in FSP to print the PsysPmax value before sending to Pcode, ensure the setting is correctly programmed.
Change-Id: Ia88ea17bc661a388c5b9bc3e59abc27c9f262977 Signed-off-by: Gaggery Tsai gaggery.tsai@intel.com Reviewed-on: https://review.coreboot.org/c/31505 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 2 files changed, 9 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h index cb9ad38..1f26f1e 100644 --- a/src/soc/intel/cannonlake/chip.h +++ b/src/soc/intel/cannonlake/chip.h @@ -216,6 +216,8 @@ uint32_t tdp_psyspl3_dutycycle; /* PL4 Value in Watts */ uint32_t tdp_pl4; + /* Estimated maximum platform power in Watts */ + uint16_t psys_pmax;
/* Intel Speed Shift Technology */ uint8_t speed_shift_enable; diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index c276c86..a198f1b 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -88,6 +88,13 @@
mainboard_silicon_init_params(params);
+ /* Set PsysPmax if it is available from DT */ + if (config->psys_pmax) { + printk(BIOS_DEBUG, "psys_pmax = %dW\n", config->psys_pmax); + /* PsysPmax is in unit of 1/8 Watt */ + tconfig->PsysPmax = config->psys_pmax * 8; + } + /* Unlock upper 8 bytes of RTC RAM */ params->PchLockDownRtcMemoryLock = 0;
-
Furquan Shaikh (Code Review) -
Gaggery Tsai (Code Review)
-
Patrick Georgi (Code Review)