Change in coreboot[master]: mainboard/intel/tgl: Customize PCH VR settings for better Sx power sa...

[Shreesh Chhabbi (Code Review)]

Hello Shreesh Chhabbi,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/43992

to review the following change.

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings

Change-Id: Ic889d0952b75a636d2186d81fb679f74d50e7048
Signed-off-by: Shreesh Chhabbi 
---
M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb
M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb
2 files changed, 28 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/43992/1

diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb
index b4a121a..1aa1f26 100644
--- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb
+++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb
@@ -151,6 +151,20 @@
    	},
    }"

+	register "ext_fivr_settings" = "{
+		.configure_ext_fivr = 1,
+		.v1p05_enable_bitmap = FIVR_ENABLE_ALL_SX,
+		.vnn_enable_bitmap = FIVR_ENABLE_ALL_SX,
+		.v1p05_supported_voltage_bitmap = FIVR_VOLTAGE_NORMAL |
+						  FIVR_VOLTAGE_MIN_ACTIVE |
+						  FIVR_VOLTAGE_MIN_RETENTION,
+		.vnn_supported_voltage_bitmap = FIVR_VOLTAGE_NORMAL |
+						FIVR_VOLTAGE_MIN_ACTIVE |
+						FIVR_VOLTAGE_MIN_RETENTION,
+		.v1p05_icc_max_ma = 500,
+		.vnn_sx_voltage_mv = 1250,
+	}"
+
    device domain 0 on
    	#From EDS(575683)
    	device pci 00.0 on  end # Host Bridge		0x9A14:U/0x9A12:Y
diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb
index b08cd3c..169433b 100644
--- a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb
+++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb
@@ -147,6 +147,20 @@
    	},
    }"

+	register "ext_fivr_settings" = "{
+		.configure_ext_fivr = 1,
+		.v1p05_enable_bitmap = FIVR_ENABLE_ALL_SX,
+		.vnn_enable_bitmap = FIVR_ENABLE_ALL_SX,
+		.v1p05_supported_voltage_bitmap = FIVR_VOLTAGE_NORMAL |
+						  FIVR_VOLTAGE_MIN_ACTIVE |
+						  FIVR_VOLTAGE_MIN_RETENTION,
+		.vnn_supported_voltage_bitmap = FIVR_VOLTAGE_NORMAL |
+						FIVR_VOLTAGE_MIN_ACTIVE |
+						FIVR_VOLTAGE_MIN_RETENTION,
+		.v1p05_icc_max_ma = 500,
+		.vnn_sx_voltage_mv = 1250,
+	}"
+
    device domain 0 on
    	#From EDS(575683)
    	device pci 00.0 on  end # Host Bridge		0x9A14:U/0x9A12:Y

[Sukumar Ghorai (Code Review)]

Sukumar Ghorai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43992 )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Patch Set 1: Code-Review+1

verified

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43992 )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/43992/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43992/1//COMMIT_MSG@7 
PS1, Line 7: mainboard/intel/tgl:
mb/intel/tglrvp:

https://review.coreboot.org/c/coreboot/+/43992/1//COMMIT_MSG@7 
PS1, Line 7: settings
Do these settings come from some document? Would be good to know where one should look for them

[Shreesh Chhabbi (Code Review)]

Hello build bot (Jenkins), Sukumar Ghorai, Shreesh Chhabbi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43992

to look at the new patch set (#2).

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings

v1p05 and vnn power rails can be used as an alternative source
by-passing vccin_aux during Sx. This by-pass feature, enables us to
shutdown vccin_aux rail which is higher voltage rail compared to v1p05
and vnn. These both rails were disabled by default in FSP. Changes in
this patch are:

1. v1p05 and vnn rails are enabled and enabled supported voltage types
   in S0i1, S0i2, S0i3, S3, S4, S5 states. They were disabled by default.

2. Icc Max for v1p05 changed to 500 mA from default 100 mA.

3. vnn rail's voltage is changed to 5 V from default 4.2 V.

BUG=None
BRANCH=None
TEST="Build and boot TGL-UP3 and check VR settings with Intel ITP-XDP
debugger and verify approx 20mW SoC power savings in S0ix"

Change-Id: Ic889d0952b75a636d2186d81fb679f74d50e7048
Signed-off-by: Shreesh Chhabbi 
---
M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb
M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb
2 files changed, 28 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/43992/2

[Shreesh Chhabbi (Code Review)]

Shreesh Chhabbi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43992 )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Patch Set 2: Code-Review+1

[Sukumar Ghorai (Code Review)]

Sukumar Ghorai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43992 )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Patch Set 2:

verified

[Caveh Jalali (Code Review)]

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43992 )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Patch Set 2: Code-Review+1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43992 )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Patch Set 3: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/43992/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43992/3//COMMIT_MSG@12 
PS3, Line 12: These both
Both of these

[Shreesh Chhabbi (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Duncan Laurie, Angel Pons, Nick Vaccaro, Sukumar Ghorai, Raj Astekar, Shreesh Chhabbi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43992

to look at the new patch set (#5).

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings

v1p05 and vnn power rails can be used as an alternative source
by-passing vccin_aux during Sx. This by-pass feature, enables us to
shutdown vccin_aux rail which is higher voltage rail compared to v1p05
and vnn. Both of these rails were disabled by default in FSP. Changes in
this patch are:

1. v1p05 and vnn rails are enabled and enabled supported voltage types
   in S0i1, S0i2, S0i3, S3, S4, S5 states. They were disabled by default.

2. Icc Max for v1p05 changed to 500 mA from default 100 mA.

3. vnn rail's voltage is changed to 5 V from default 4.2 V.

BUG=None
BRANCH=None
TEST="Build and boot TGL-UP3 and check VR settings with Intel ITP-XDP
debugger and verify approx 20mW SoC power savings in S0ix"

Change-Id: Ic889d0952b75a636d2186d81fb679f74d50e7048
Signed-off-by: Shreesh Chhabbi 
---
M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb
M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb
2 files changed, 28 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/43992/5

[Shreesh Chhabbi (Code Review)]

Shreesh Chhabbi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43992 )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43992/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43992/3//COMMIT_MSG@12 
PS3, Line 12: These both
> Both of these

Done

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43992 )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Patch Set 5: Code-Review+2

[S] Change in coreboot[master]: mainboard/intel/tgl: Customize PCH VR settings for better Sx power sa...

[Martin L Roth (Code Review)]

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43992?usp=email )

Change subject: mainboard/intel/tgl: Customize PCH VR settings for better Sx power savings
......................................................................

Abandoned

This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.