Matthew Ziegelbaum has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: ambassador: update fan table and usb configs ......................................................................
ambassador: update fan table and usb configs
BUG=b:173134210 TEST=flash to DUT
Change-Id: I9f727f0f7e2eb7fe70385ebc843558d51e1860c5 Signed-off-by: Matt Ziegelbaum ziegs@google.com --- M src/mainboard/google/hatch/variants/ambassador/overridetree.cb 1 file changed, 97 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/47556/1
diff --git a/src/mainboard/google/hatch/variants/ambassador/overridetree.cb b/src/mainboard/google/hatch/variants/ambassador/overridetree.cb index adb00e4..71a1657 100644 --- a/src/mainboard/google/hatch/variants/ambassador/overridetree.cb +++ b/src/mainboard/google/hatch/variants/ambassador/overridetree.cb @@ -1,6 +1,10 @@ chip soc/intel/cannonlake - # Enable heci communication - register "HeciEnabled" = "1" + register "tcc_offset" = "5" # TCC of 95C + + register "power_limits_config" = "{ + .tdp_pl1_override = 15, + .tdp_pl2_override = 51, + }"
# Auto-switch between X4 NVMe and X2 NVMe. register "TetonGlacierMode" = "1" @@ -62,9 +66,6 @@ .pre_emp_bias = USB2_BIAS_28P15MV, .pre_emp_bit = USB2_HALF_BIT_PRE_EMP, }" # Type-A port 0 - register "usb2_ports[6]" = "USB2_PORT_EMPTY" - register "usb2_ports[7]" = "USB2_PORT_EMPTY" - register "usb2_ports[8]" = "USB2_PORT_EMPTY" register "usb2_ports[9]" = "{ .enable = 1, .ocpin = OC_SKIP, @@ -74,12 +75,76 @@ .pre_emp_bit = USB2_HALF_BIT_PRE_EMP, }" # BT
- register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC2)" # Type-A Port 2 - register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC3)" # Type-A Port 3 - register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC1)" # Type-A Port 1 + register "usb3_ports[0]" = "{ + .enable = 1, + .ocpin = OC2, + .tx_de_emp = 0x00, + .tx_downscale_amp = 0x00, + .gen2_tx_rate0_uniq_tran_enable = 0, + .gen2_tx_rate0_uniq_tran = 0x00, + .gen2_tx_rate1_uniq_tran_enable = 0, + .gen2_tx_rate1_uniq_tran = 0x00, + .gen2_tx_rate2_uniq_tran_enable = 1, + .gen2_tx_rate2_uniq_tran = 0x4c, + .gen2_tx_rate3_uniq_tran_enable = 0, + .gen2_tx_rate3_uniq_tran = 0x00, + .gen2_rx_tuning_enable = 0x0f, + .gen2_rx_tuning_params = 0x45, + .gen2_rx_filter_sel = 0x44, + }" # Type-A Port 2 + register "usb3_ports[1]" = "USB3_PORT_GEN2_DEFAULT(OC3)" # Type-A Port 3 + register "usb3_ports[2]" = "{ + .enable = 1, + .ocpin = OC1, + .tx_de_emp = 0x00, + .tx_downscale_amp = 0x00, + .gen2_tx_rate0_uniq_tran_enable = 0, + .gen2_tx_rate0_uniq_tran = 0x00, + .gen2_tx_rate1_uniq_tran_enable = 0, + .gen2_tx_rate1_uniq_tran = 0x00, + .gen2_tx_rate2_uniq_tran_enable = 1, + .gen2_tx_rate2_uniq_tran = 0x4c, + .gen2_tx_rate3_uniq_tran_enable = 0, + .gen2_tx_rate3_uniq_tran = 0x00, + .gen2_rx_tuning_enable = 0x0f, + .gen2_rx_tuning_params = 0x3d, + .gen2_rx_filter_sel = 0x44, + }" # Type-A Port 1 register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC_SKIP)" # Type-C - register "usb3_ports[4]" = "USB3_PORT_DEFAULT(OC0)" # Type-A Port 0 - register "usb3_ports[5]" = "USB3_PORT_DEFAULT(OC_SKIP)" # Type-A Port 4 + register "usb3_ports[4]" = "{ + .enable = 1, + .ocpin = OC0, + .tx_de_emp = 0x00, + .tx_downscale_amp = 0x00, + .gen2_tx_rate0_uniq_tran_enable = 0, + .gen2_tx_rate0_uniq_tran = 0x00, + .gen2_tx_rate1_uniq_tran_enable = 0, + .gen2_tx_rate1_uniq_tran = 0x00, + .gen2_tx_rate2_uniq_tran_enable = 1, + .gen2_tx_rate2_uniq_tran = 0x4c, + .gen2_tx_rate3_uniq_tran_enable = 0, + .gen2_tx_rate3_uniq_tran = 0x00, + .gen2_rx_tuning_enable = 0x0f, + .gen2_rx_tuning_params = 0x45, + .gen2_rx_filter_sel = 0x44, + }" # Type-A Port 0 + register "usb3_ports[5]" = "{ + .enable = 1, + .ocpin = OC_SKIP, + .tx_de_emp = 0x00, + .tx_downscale_amp = 0x00, + .gen2_tx_rate0_uniq_tran_enable = 0, + .gen2_tx_rate0_uniq_tran = 0x00, + .gen2_tx_rate1_uniq_tran_enable = 0, + .gen2_tx_rate1_uniq_tran = 0x00, + .gen2_tx_rate2_uniq_tran_enable = 1, + .gen2_tx_rate2_uniq_tran = 0x4c, + .gen2_tx_rate3_uniq_tran_enable = 0, + .gen2_tx_rate3_uniq_tran = 0x00, + .gen2_rx_tuning_enable = 0x0f, + .gen2_rx_tuning_params = 0x45, + .gen2_rx_filter_sel = 0x44, + }" # Type-A Port 4
# Bitmap for Wake Enable on USB attach/detach register "usb2_wake_enable_bitmap" = "USB_PORT_WAKE_ENABLE(1) | \ @@ -208,29 +273,30 @@ chip drivers/intel/dptf ## Active Policy register "policies.active[0]" = "{.target=DPTF_CPU, - .thresholds={TEMP_PCT(90, 85), - TEMP_PCT(85, 75), - TEMP_PCT(80, 65), - TEMP_PCT(75, 55), - TEMP_PCT(70, 45),}}" - register "policies.active[1]" = "{.target=DPTF_TEMP_SENSOR_0, - .thresholds={TEMP_PCT(50, 85), - TEMP_PCT(47, 75), - TEMP_PCT(45, 65), - TEMP_PCT(42, 55), - TEMP_PCT(39, 45),}}" + .thresholds={TEMP_PCT(94, 0),}}" + register "policies.active[1]" = "{.target=DPTF_TEMP_SENSOR_1, + .thresholds={TEMP_PCT(70, 100), + TEMP_PCT(66, 90), + TEMP_PCT(62, 80), + TEMP_PCT(58, 70), + TEMP_PCT(53, 60), + TEMP_PCT(48, 50), + TEMP_PCT(43, 40), + TEMP_PCT(38, 30),}}"
## Passive Policy - register "policies.passive[0]" = "DPTF_PASSIVE(CPU, CPU, 93, 5000)" - register "policies.passive[1]" = "DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 65, 6000)" + register "policies.passive[0]" = "DPTF_PASSIVE(CPU, CPU, 95, 5000)" + register "policies.passive[1]" = "DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 75, 5000)" + register "policies.passive[2]" = "DPTF_PASSIVE(CPU, TEMP_SENSOR_1, 75, 5000)"
## Critical Policy register "policies.critical[0]" = "DPTF_CRITICAL(CPU, 100, SHUTDOWN)" - register "policies.critical[1]" = "DPTF_CRITICAL(TEMP_SENSOR_0, 75, SHUTDOWN)" + register "policies.critical[1]" = "DPTF_CRITICAL(TEMP_SENSOR_0, 85, SHUTDOWN)" + register "policies.critical[2]" = "DPTF_CRITICAL(TEMP_SENSOR_1, 85, SHUTDOWN)"
## Power Limits Control # PL1 is fixed at 15W, avg over 28-32s interval - # 25-64W PL2 in 1000mW increments, avg over 28-32s interval + # 15-51W PL2 in 1000mW increments, avg over 28-32s interval register "controls.power_limits.pl1" = "{ .min_power = 15000, .max_power = 15000, @@ -239,7 +305,7 @@ .granularity = 200,}" register "controls.power_limits.pl2" = "{ .min_power = 25000, - .max_power = 64000, + .max_power = 51000, .time_window_min = 28 * MSECS_PER_SEC, .time_window_max = 32 * MSECS_PER_SEC, .granularity = 1000,}" @@ -370,6 +436,7 @@ device i2c 4a on end end end # I2C #3, Realtek RTD2142. + device pci 16.0 on end # Management Engine Interface 1 device pci 19.0 on chip drivers/i2c/generic register "hid" = ""10EC5682"" @@ -396,8 +463,11 @@ register "device_index" = "0" device pci 00.0 on end end + register "PcieRpSlotImplemented[6]" = "1" end # RTL8111H Ethernet NIC - device pci 1d.2 on end # PCI Express Port 11 (X2 NVMe) + device pci 1d.2 on # PCI Express Port 11 (X2 NVMe) + register "PcieRpSlotImplemented[10]" = "1" + end device pci 1e.3 off end # GSPI #1 end
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: ambassador: update fan table and usb configs ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47556/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/ambassador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47556/1/src/mainboard/google/hatch/... PS1, Line 3: egister "HeciEnabled" = "1" intentionally dropped? Probably because you are enabling 16.0? It would be good to add that to commit message.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: ambassador: update fan table and usb configs ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47556/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47556/1//COMMIT_MSG@7 PS1, Line 7: ambassador mb/google/hatch/var/ambassador: …
https://review.coreboot.org/c/coreboot/+/47556/1//COMMIT_MSG@8 PS1, Line 8: Please describe the problem, and where you got the values from.
Hello build bot (Jenkins), Furquan Shaikh, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47556
to look at the new patch set (#2).
Change subject: mb/google/hatch/var/ambassador: update fan table and usb configs ......................................................................
mb/google/hatch/var/ambassador: update fan table and usb configs
Fan table: provided by the ODM (see attachment in bug) based on measurements with EVT unit.
USB config: based on USB 3.1 gen 2 tuning done for Puff (see http://b/150515720).
BUG=b:173134210 TEST=flash to DUT
Change-Id: I9f727f0f7e2eb7fe70385ebc843558d51e1860c5 Signed-off-by: Matt Ziegelbaum ziegs@google.com --- M src/mainboard/google/hatch/variants/ambassador/overridetree.cb 1 file changed, 97 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/47556/2
Matthew Ziegelbaum has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: mb/google/hatch/var/ambassador: update fan table and usb configs ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47556/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47556/1//COMMIT_MSG@7 PS1, Line 7: ambassador
mb/google/hatch/var/ambassador: …
Done
https://review.coreboot.org/c/coreboot/+/47556/1//COMMIT_MSG@8 PS1, Line 8:
Please describe the problem, and where you got the values from.
Done
https://review.coreboot.org/c/coreboot/+/47556/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/ambassador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47556/1/src/mainboard/google/hatch/... PS1, Line 3: egister "HeciEnabled" = "1"
intentionally dropped? Probably because you are enabling 16. […]
Yes--I had cherrypicked https://review.coreboot.org/plugins/gitiles/coreboot/+/50a1072180f05c20ec13d... before it was submitted, and then forgot when I uploaded the CL. Rebased now.
Matthew Ziegelbaum has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: mb/google/hatch/var/ambassador: update fan table and usb configs ......................................................................
Patch Set 2:
Friendly ping 😊
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: mb/google/hatch/var/ambassador: update fan table and usb configs ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47556/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47556/2//COMMIT_MSG@9 PS2, Line 9: Fan table: provided by the ODM (see attachment in bug) based on : measurements with EVT unit. : : USB config: based on USB 3.1 gen 2 tuning done for Puff : (see http://b/150515720). Two items warrant two separate commits.
https://review.coreboot.org/c/coreboot/+/47556/2//COMMIT_MSG@13 PS2, Line 13: http:// Remove, as it’s not a valid URL?
Hello build bot (Jenkins), Furquan Shaikh, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47556
to look at the new patch set (#3).
Change subject: mb/google/hatch/var/ambassador: update fan table and tdp config ......................................................................
mb/google/hatch/var/ambassador: update fan table and tdp config
Fan table: provided by the ODM (see attachment in bug) based on measurements with EVT unit.
BUG=b:173134210 TEST=flash to DUT
Change-Id: I9f727f0f7e2eb7fe70385ebc843558d51e1860c5 Signed-off-by: Matt Ziegelbaum ziegs@google.com --- M src/mainboard/google/hatch/variants/ambassador/overridetree.cb 1 file changed, 24 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/47556/3
Matthew Ziegelbaum has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: mb/google/hatch/var/ambassador: update fan table and tdp config ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47556/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47556/2//COMMIT_MSG@9 PS2, Line 9: Fan table: provided by the ODM (see attachment in bug) based on : measurements with EVT unit. : : USB config: based on USB 3.1 gen 2 tuning done for Puff : (see http://b/150515720).
Two items warrant two separate commits.
Done--after talking with the ODM I'm going to hold off on a USB change, but I split out the FSP option change to its own CL in https://review.coreboot.org/c/coreboot/+/47685
https://review.coreboot.org/c/coreboot/+/47556/2//COMMIT_MSG@13 PS2, Line 13: http://
Remove, as it’s not a valid URL?
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: mb/google/hatch/var/ambassador: update fan table and tdp config ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47556 )
Change subject: mb/google/hatch/var/ambassador: update fan table and tdp config ......................................................................
mb/google/hatch/var/ambassador: update fan table and tdp config
Fan table: provided by the ODM (see attachment in bug) based on measurements with EVT unit.
BUG=b:173134210 TEST=flash to DUT
Change-Id: I9f727f0f7e2eb7fe70385ebc843558d51e1860c5 Signed-off-by: Matt Ziegelbaum ziegs@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47556 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/variants/ambassador/overridetree.cb 1 file changed, 24 insertions(+), 16 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/ambassador/overridetree.cb b/src/mainboard/google/hatch/variants/ambassador/overridetree.cb index 835a8aa..7b46153 100644 --- a/src/mainboard/google/hatch/variants/ambassador/overridetree.cb +++ b/src/mainboard/google/hatch/variants/ambassador/overridetree.cb @@ -1,4 +1,11 @@ chip soc/intel/cannonlake + register "tcc_offset" = "5" # TCC of 95C + + register "power_limits_config" = "{ + .tdp_pl1_override = 15, + .tdp_pl2_override = 51, + }" + # Auto-switch between X4 NVMe and X2 NVMe. register "TetonGlacierMode" = "1"
@@ -205,29 +212,30 @@ chip drivers/intel/dptf ## Active Policy register "policies.active[0]" = "{.target=DPTF_CPU, - .thresholds={TEMP_PCT(90, 85), - TEMP_PCT(85, 75), - TEMP_PCT(80, 65), - TEMP_PCT(75, 55), - TEMP_PCT(70, 45),}}" - register "policies.active[1]" = "{.target=DPTF_TEMP_SENSOR_0, - .thresholds={TEMP_PCT(50, 85), - TEMP_PCT(47, 75), - TEMP_PCT(45, 65), - TEMP_PCT(42, 55), - TEMP_PCT(39, 45),}}" + .thresholds={TEMP_PCT(94, 0),}}" + register "policies.active[1]" = "{.target=DPTF_TEMP_SENSOR_1, + .thresholds={TEMP_PCT(70, 100), + TEMP_PCT(66, 90), + TEMP_PCT(62, 80), + TEMP_PCT(58, 70), + TEMP_PCT(53, 60), + TEMP_PCT(48, 50), + TEMP_PCT(43, 40), + TEMP_PCT(38, 30),}}"
## Passive Policy - register "policies.passive[0]" = "DPTF_PASSIVE(CPU, CPU, 93, 5000)" - register "policies.passive[1]" = "DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 65, 6000)" + register "policies.passive[0]" = "DPTF_PASSIVE(CPU, CPU, 95, 5000)" + register "policies.passive[1]" = "DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 75, 5000)" + register "policies.passive[2]" = "DPTF_PASSIVE(CPU, TEMP_SENSOR_1, 75, 5000)"
## Critical Policy register "policies.critical[0]" = "DPTF_CRITICAL(CPU, 100, SHUTDOWN)" - register "policies.critical[1]" = "DPTF_CRITICAL(TEMP_SENSOR_0, 75, SHUTDOWN)" + register "policies.critical[1]" = "DPTF_CRITICAL(TEMP_SENSOR_0, 85, SHUTDOWN)" + register "policies.critical[2]" = "DPTF_CRITICAL(TEMP_SENSOR_1, 85, SHUTDOWN)"
## Power Limits Control # PL1 is fixed at 15W, avg over 28-32s interval - # 25-64W PL2 in 1000mW increments, avg over 28-32s interval + # 15-51W PL2 in 1000mW increments, avg over 28-32s interval register "controls.power_limits.pl1" = "{ .min_power = 15000, .max_power = 15000, @@ -236,7 +244,7 @@ .granularity = 200,}" register "controls.power_limits.pl2" = "{ .min_power = 25000, - .max_power = 64000, + .max_power = 51000, .time_window_min = 28 * MSECS_PER_SEC, .time_window_max = 32 * MSECS_PER_SEC, .granularity = 1000,}"
-
Furquan Shaikh (Code Review) -
Matthew Ziegelbaum (Code Review)
-
Paul Menzel (Code Review)