Change in coreboot[master]: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35323 )
Change subject: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage ......................................................................
soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage
PchPwrOptEnable FSP UPD is for internal testing and not really available in externally released FSP source hence assigning this UPD using devicetree config dmipwroptimize doesn't do anything.
TEST=Build and boot sarien/arcada.
Change-Id: I6da2a088fb697e57d12008fa18bd1764b3da7765 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb M src/mainboard/google/sarien/variants/arcada/devicetree.cb M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 7 files changed, 0 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/35323/1
diff --git a/src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb b/src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb index 3628264..7050b1e 100644 --- a/src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb +++ b/src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb @@ -32,7 +32,6 @@ register "psys_pmax" = "140" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "tdp_pl1_override" = "25" register "tdp_pl2_override" = "51" diff --git a/src/mainboard/google/drallion/variants/drallion/devicetree.cb b/src/mainboard/google/drallion/variants/drallion/devicetree.cb index e96f9d7..fac4d18 100644 --- a/src/mainboard/google/drallion/variants/drallion/devicetree.cb +++ b/src/mainboard/google/drallion/variants/drallion/devicetree.cb @@ -32,7 +32,6 @@ register "psys_pmax" = "140" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "tdp_pl1_override" = "25" register "tdp_pl2_override" = "51" diff --git a/src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb b/src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb index 84aacd5..f2367ff 100644 --- a/src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb +++ b/src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb @@ -34,7 +34,6 @@ register "speed_shift_enable" = "1" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "AcousticNoiseMitigation" = "1" register "SlowSlewRateForIa" = "2" diff --git a/src/mainboard/google/sarien/variants/arcada/devicetree.cb b/src/mainboard/google/sarien/variants/arcada/devicetree.cb index ebcf140..0f3023c 100644 --- a/src/mainboard/google/sarien/variants/arcada/devicetree.cb +++ b/src/mainboard/google/sarien/variants/arcada/devicetree.cb @@ -32,7 +32,6 @@ register "psys_pmax" = "140" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "tdp_pl1_override" = "25" register "tdp_pl2_override" = "51" diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb index d3aab62..cd216e5 100644 --- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb +++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb @@ -34,7 +34,6 @@ register "speed_shift_enable" = "1" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "AcousticNoiseMitigation" = "1" register "SlowSlewRateForIa" = "2" diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h index 2ebe017..bfd210d 100644 --- a/src/soc/intel/cannonlake/chip.h +++ b/src/soc/intel/cannonlake/chip.h @@ -377,9 +377,6 @@ uint8_t SlowSlewRateForSa; uint8_t SlowSlewRateForFivr;
- /* DMI Power Optimizer */ - uint8_t dmipwroptimize; - /* SATA Power Optimizer */ uint8_t satapwroptimize;
diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index 4038335..e31e62c 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -372,7 +372,6 @@ params->FastPkgCRampDisableFivr = config->FastPkgCRampDisableFivr;
/* Power Optimizer */ - params->PchPwrOptEnable = config->dmipwroptimize; params->SataPwrOptEnable = config->satapwroptimize;
/* Disable PCH ACPI timer */
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35323 )
Change subject: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage ......................................................................
Patch Set 1:
(1 comment)
So, this is not required anymore for S0
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG@7 PS1, Line 7: PchPwrOptEnable So, this is not required anymore for S0ix. This change seems to indicate that it was necessary for running S0ix on sarien/arcada: https://review.coreboot.org/c/coreboot/+/30212/
What changed? How did you identify that this UPD should not be set?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35323 )
Change subject: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG@7 PS1, Line 7: PchPwrOptEnable
So, this is not required anymore for S0ix. […]
i was actually checking if https://review.coreboot.org/c/coreboot/+/30212/ CL is actually applicable for CML FSP or not then i don't any use, it just assigning and printing no actual programming, went back to CFL and CNL FSP code and saw the same.
After discussing with PCH team, it appears that this UPD had added for some internal testing and those code we remove before making external release hence for you it would be a dummy UPD
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35323 )
Change subject: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG@7 PS1, Line 7: PchPwrOptEnable
i was actually checking if https://review.coreboot. […]
Oh okay. Can you please also run S0ix to ensure there are no surprises?
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35323 )
Change subject: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage ......................................................................
Patch Set 1: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35323 )
Change subject: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG@7 PS1, Line 7: PchPwrOptEnable
Oh okay. […]
i will get back on thus
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35323 )
Change subject: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35323/1//COMMIT_MSG@7 PS1, Line 7: PchPwrOptEnable
i will get back on thus
S0ix is working on sarien, we are running long stress test as well.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35323 )
Change subject: soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage ......................................................................
soc/intel/cnl: Remove unnecessary FSP UPD “PchPwrOptEnable” usage
PchPwrOptEnable FSP UPD is for internal testing and not really available in externally released FSP source hence assigning this UPD using devicetree config dmipwroptimize doesn't do anything.
TEST=Build and boot sarien/arcada.
Change-Id: I6da2a088fb697e57d12008fa18bd1764b3da7765 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35323 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: V Sowmya v.sowmya@intel.com --- M src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb M src/mainboard/google/sarien/variants/arcada/devicetree.cb M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/fsp_params.c 7 files changed, 0 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved V Sowmya: Looks good to me, approved
diff --git a/src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb b/src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb index 3628264..7050b1e 100644 --- a/src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb +++ b/src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb @@ -32,7 +32,6 @@ register "psys_pmax" = "140" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "tdp_pl1_override" = "25" register "tdp_pl2_override" = "51" diff --git a/src/mainboard/google/drallion/variants/drallion/devicetree.cb b/src/mainboard/google/drallion/variants/drallion/devicetree.cb index d8ef7a9..d824a55 100644 --- a/src/mainboard/google/drallion/variants/drallion/devicetree.cb +++ b/src/mainboard/google/drallion/variants/drallion/devicetree.cb @@ -32,7 +32,6 @@ register "psys_pmax" = "140" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "tdp_pl1_override" = "25" register "tdp_pl2_override" = "51" diff --git a/src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb b/src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb index 84aacd5..f2367ff 100644 --- a/src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb +++ b/src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb @@ -34,7 +34,6 @@ register "speed_shift_enable" = "1" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "AcousticNoiseMitigation" = "1" register "SlowSlewRateForIa" = "2" diff --git a/src/mainboard/google/sarien/variants/arcada/devicetree.cb b/src/mainboard/google/sarien/variants/arcada/devicetree.cb index ebcf140..0f3023c 100644 --- a/src/mainboard/google/sarien/variants/arcada/devicetree.cb +++ b/src/mainboard/google/sarien/variants/arcada/devicetree.cb @@ -32,7 +32,6 @@ register "psys_pmax" = "140" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "tdp_pl1_override" = "25" register "tdp_pl2_override" = "51" diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb index d3aab62..cd216e5 100644 --- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb +++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb @@ -34,7 +34,6 @@ register "speed_shift_enable" = "1" register "s0ix_enable" = "1" register "dptf_enable" = "1" - register "dmipwroptimize" = "1" register "satapwroptimize" = "1" register "AcousticNoiseMitigation" = "1" register "SlowSlewRateForIa" = "2" diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h index 9c7c171..451c920 100644 --- a/src/soc/intel/cannonlake/chip.h +++ b/src/soc/intel/cannonlake/chip.h @@ -387,9 +387,6 @@ uint8_t SlowSlewRateForSa; uint8_t SlowSlewRateForFivr;
- /* DMI Power Optimizer */ - uint8_t dmipwroptimize; - /* SATA Power Optimizer */ uint8_t satapwroptimize;
diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index f48a626..76d40aa 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -377,7 +377,6 @@ params->FastPkgCRampDisableFivr = config->FastPkgCRampDisableFivr;
/* Power Optimizer */ - params->PchPwrOptEnable = config->dmipwroptimize; params->SataPwrOptEnable = config->satapwroptimize;
/* Disable PCH ACPI timer */
-
Furquan Shaikh (Code Review) -
Patrick Georgi (Code Review)
-
Subrata Banik (Code Review)
-
V Sowmya (Code Review)