Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF
The Kohaku V24 schematic adds two additional temperature sensors to the EC. Add these to the DPTF tables. Note that this requires changes in both the EC and BIOS.
BRANCH=none BUG=b:138578073 TEST=Rebuild EC and BIOS, look for new thermal sensors in kernel. 1. Build EC ``cd ~/trunk/src/platform/ec`` ``make -j BOARD=kohaku`` 2. Program EC ``./util/flash_ec --board=kohaku`` 3. Reboot device 4. Rebuild BIOS ``cd ~/trunk/src/third_party/coreboot`` ``FEATURES="noclean" FW_NAME=kohaku emerge-hatch chromeos-ec depthcharge vboot_reference libpayload coreboot-private-files intel-cmlfsp coreboot-private-files-hatch coreboot chromeos-bootimage`` 5. Use flashrom to program the BIOS 6. Reboot device 7. Log into the root console (ctrl-alt-F2 or servo) 8. Example thermal sensor information ``grep . /sys/class/thermal/t*/type`` Look for "TSR0" through "TSR3" in the output.
Change-Id: Ib8f38beae6392855927ce1249c229d7a114c72b2 Signed-off-by: Paul Fagerburg pfagerburg@chromium.org --- M src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl 1 file changed, 20 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/34765/1
diff --git a/src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl b/src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl index 06df7b1..7a90a9a 100644 --- a/src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl @@ -26,6 +26,16 @@ #define DPTF_TSR1_PASSIVE 65 #define DPTF_TSR1_CRITICAL 75
+#define DPTF_TSR2_SENSOR_ID 1 +#define DPTF_TSR2_SENSOR_NAME "Thermal Sensor 3" +#define DPTF_TSR2_PASSIVE 65 +#define DPTF_TSR2_CRITICAL 75 + +#define DPTF_TSR3_SENSOR_ID 1 +#define DPTF_TSR3_SENSOR_NAME "Thermal Sensor 4" +#define DPTF_TSR3_PASSIVE 65 +#define DPTF_TSR3_CRITICAL 75 + #define DPTF_ENABLE_CHARGER
/* Charger performance states, board-specific values from charger and EC */ @@ -40,11 +50,17 @@ /* CPU Throttle Effect on CPU */ Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 100, 50, 0, 0, 0, 0 },
- /* CPU Throttle Effect on Ambient (TSR0) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR0, 100, 94, 0, 0, 0, 0 }, + /* CPU Throttle Effect on 5V (TSR1) */ + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 100, 94, 0, 0, 0, 0 },
- /* Charger Throttle Effect on Charger (TSR1) */ - Package () { _SB.DPTF.TCHG, _SB.DPTF.TSR1, 100, 60, 0, 0, 0, 0 }, + /* Charger Throttle Effect on Charger (TSR0) */ + Package () { _SB.DPTF.TCHG, _SB.DPTF.TSR0, 100, 60, 0, 0, 0, 0 }, + + /* CPU Throttle Effect on IA (TSR2) */ + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 100, 94, 0, 0, 0, 0 }, + + /* CPU Throttle Effect on GT (TSR3) */ + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR3, 100, 94, 0, 0, 0, 0 }, })
Name (MPPC, Package ()
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34765/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34765/1//COMMIT_MSG@11 PS1, Line 11: Note that this requires changes in both the EC and BIOS. Probably good to add a Cq-Depend on the EC CL?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 53: /* CPU Throttle Effect on 5V (TSR1) */ : Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 100, 94, 0, 0, 0, 0 }, : : /* Charger Throttle Effect on Charger (TSR0) */ : Package () { _SB.DPTF.TCHG, _SB.DPTF.TSR0, 100, 60, 0, 0, 0, 0 }, Why did these change? Are the details captured somewhere?
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 30: Thermal Sensor 3 Share info on what are these "Thermal Sensor 1/2/3/4" used for on this kohaku variant?
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 31: #define DPTF_TSR2_PASSIVE 65 : #define DPTF_TSR2_CRITICAL 75 Are these initial values ? Is there any plan to tune/modify in coming weeks?
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 62: GT Share more details on this GT throttling as per above comment.
SH Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 31: #define DPTF_TSR2_PASSIVE 65 : #define DPTF_TSR2_CRITICAL 75
Are these initial values ? Is there any plan to tune/modify in coming weeks?
I think Paul referred to the new layout capture in b/138578073#comment5. We need do DPTF tuning when we get real system having 4 thermal sensor.
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 30: Thermal Sensor 3
Share info on what are these "Thermal Sensor 1/2/3/4" used for on this kohaku variant?
The Hatch reference design gave these generic names. I'll revise them for Kohaku.
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 31: #define DPTF_TSR2_PASSIVE 65 : #define DPTF_TSR2_CRITICAL 75
I think Paul referred to the new layout capture in b/138578073#comment5. […]
Yes, these values will be refined once we have hardware with the sensors.
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 53: /* CPU Throttle Effect on 5V (TSR1) */ : Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 100, 94, 0, 0, 0, 0 }, : : /* Charger Throttle Effect on Charger (TSR0) */ : Package () { _SB.DPTF.TCHG, _SB.DPTF.TSR0, 100, 60, 0, 0, 0, 0 },
Why did these change? Are the details captured somewhere?
On the Kohaku design, TSR1 is placed near the 5V supply, and TSR0 is placed near the charger, which is swapped compared to the hatch reference.
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 62: GT
Share more details on this GT throttling as per above comment.
The only information that I have is that the sensor is placed near GT. We should be able to adjust the values and provide better documentation once we have hardware.
Hello Seunghwan Kim, Sumeet R Pawnikar, Aseda Aboagye, Tim Wawrzynczak, Puthikorn Voravootivat, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34765
to look at the new patch set (#2).
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF
The Kohaku V24 schematic adds two additional temperature sensors to the EC. Add these to the DPTF tables. Note that this requires changes in both the EC and BIOS.
Cq-Depend: chromium:1740186 BRANCH=none BUG=b:138578073 TEST=Rebuild EC and BIOS, look for new thermal sensors in kernel. 1. Build EC ``cd ~/trunk/src/platform/ec`` ``make -j BOARD=kohaku`` 2. Program EC ``./util/flash_ec --board=kohaku`` 3. Reboot device 4. Rebuild BIOS ``cd ~/trunk/src/third_party/coreboot`` ``FEATURES="noclean" FW_NAME=kohaku emerge-hatch chromeos-ec depthcharge vboot_reference libpayload coreboot-private-files intel-cmlfsp coreboot-private-files-hatch coreboot chromeos-bootimage`` 5. Use flashrom to program the BIOS 6. Reboot device 7. Log into the root console (ctrl-alt-F2 or servo) 8. Example thermal sensor information ``grep . /sys/class/thermal/t*/type`` Look for "TSR0" through "TSR3" in the output.
Change-Id: Ib8f38beae6392855927ce1249c229d7a114c72b2 Signed-off-by: Paul Fagerburg pfagerburg@chromium.org --- M src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl 1 file changed, 22 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/34765/2
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34765/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34765/1//COMMIT_MSG@11 PS1, Line 11: Note that this requires changes in both the EC and BIOS.
Probably good to add a Cq-Depend on the EC CL?
Done
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/34765/1/src/mainboard/google/hatch/... PS1, Line 30: Thermal Sensor 3
The Hatch reference design gave these generic names. I'll revise them for Kohaku.
Done
Hello Seunghwan Kim, Sumeet R Pawnikar, Aseda Aboagye, Tim Wawrzynczak, Puthikorn Voravootivat, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34765
to look at the new patch set (#3).
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF
The Kohaku V24 schematic adds two additional temperature sensors to the EC. Add these to the DPTF tables.
Cq-Depend: chromium:1742914 BRANCH=none BUG=b:138578073 TEST=Rebuild EC and BIOS, look for new thermal sensors in kernel. 1. Build EC ``cd ~/trunk/src/platform/ec`` ``make -j BOARD=kohaku`` 2. Program EC ``./util/flash_ec --board=kohaku`` 3. Reboot device 4. Rebuild BIOS ``cd ~/trunk/src/third_party/coreboot`` ``FEATURES="noclean" FW_NAME=kohaku emerge-hatch chromeos-ec depthcharge vboot_reference libpayload coreboot-private-files intel-cmlfsp coreboot-private-files-hatch coreboot chromeos-bootimage`` 5. Use flashrom to program the BIOS 6. Reboot device 7. Log into the root console (ctrl-alt-F2 or servo) 8. Example thermal sensor information ``grep . /sys/class/thermal/t*/type`` Look for "TSR0" through "TSR3" in the output.
Change-Id: Ib8f38beae6392855927ce1249c229d7a114c72b2 Signed-off-by: Paul Fagerburg pfagerburg@chromium.org --- M src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl 1 file changed, 22 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/34765/3
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
Patch Set 3: Code-Review+1
I will let Sumeet +2 this.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
Patch Set 3: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34765 )
Change subject: kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF ......................................................................
kohaku: add TEMP_SENSOR_3 and TEMP_SENSOR_4 to DPTF
The Kohaku V24 schematic adds two additional temperature sensors to the EC. Add these to the DPTF tables.
Cq-Depend: chromium:1742914 BRANCH=none BUG=b:138578073 TEST=Rebuild EC and BIOS, look for new thermal sensors in kernel. 1. Build EC ``cd ~/trunk/src/platform/ec`` ``make -j BOARD=kohaku`` 2. Program EC ``./util/flash_ec --board=kohaku`` 3. Reboot device 4. Rebuild BIOS ``cd ~/trunk/src/third_party/coreboot`` ``FEATURES="noclean" FW_NAME=kohaku emerge-hatch chromeos-ec depthcharge vboot_reference libpayload coreboot-private-files intel-cmlfsp coreboot-private-files-hatch coreboot chromeos-bootimage`` 5. Use flashrom to program the BIOS 6. Reboot device 7. Log into the root console (ctrl-alt-F2 or servo) 8. Example thermal sensor information ``grep . /sys/class/thermal/t*/type`` Look for "TSR0" through "TSR3" in the output.
Change-Id: Ib8f38beae6392855927ce1249c229d7a114c72b2 Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/34765 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl 1 file changed, 22 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, but someone else must approve Sumeet R Pawnikar: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl b/src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl index 06df7b1..f6fd907 100644 --- a/src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/hatch/variants/kohaku/include/variant/acpi/dptf.asl @@ -17,15 +17,25 @@ #define DPTF_CPU_CRITICAL 105
#define DPTF_TSR0_SENSOR_ID 0 -#define DPTF_TSR0_SENSOR_NAME "Thermal Sensor 1" +#define DPTF_TSR0_SENSOR_NAME "Thermal Sensor - Charger" #define DPTF_TSR0_PASSIVE 49 #define DPTF_TSR0_CRITICAL 75
#define DPTF_TSR1_SENSOR_ID 1 -#define DPTF_TSR1_SENSOR_NAME "Thermal Sensor 2" +#define DPTF_TSR1_SENSOR_NAME "Thermal Sensor - 5V" #define DPTF_TSR1_PASSIVE 65 #define DPTF_TSR1_CRITICAL 75
+#define DPTF_TSR2_SENSOR_ID 1 +#define DPTF_TSR2_SENSOR_NAME "Thermal Sensor - IA" +#define DPTF_TSR2_PASSIVE 65 +#define DPTF_TSR2_CRITICAL 75 + +#define DPTF_TSR3_SENSOR_ID 1 +#define DPTF_TSR3_SENSOR_NAME "Thermal Sensor - GT" +#define DPTF_TSR3_PASSIVE 65 +#define DPTF_TSR3_CRITICAL 75 + #define DPTF_ENABLE_CHARGER
/* Charger performance states, board-specific values from charger and EC */ @@ -40,11 +50,17 @@ /* CPU Throttle Effect on CPU */ Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 100, 50, 0, 0, 0, 0 },
- /* CPU Throttle Effect on Ambient (TSR0) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR0, 100, 94, 0, 0, 0, 0 }, + /* CPU Throttle Effect on 5V (TSR1) */ + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 100, 60, 0, 0, 0, 0 },
- /* Charger Throttle Effect on Charger (TSR1) */ - Package () { _SB.DPTF.TCHG, _SB.DPTF.TSR1, 100, 60, 0, 0, 0, 0 }, + /* Charger Throttle Effect on Charger (TSR0) */ + Package () { _SB.DPTF.TCHG, _SB.DPTF.TSR0, 100, 94, 0, 0, 0, 0 }, + + /* CPU Throttle Effect on IA (TSR2) */ + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 100, 60, 0, 0, 0, 0 }, + + /* CPU Throttle Effect on GT (TSR3) */ + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR3, 100, 60, 0, 0, 0, 0 }, })
Name (MPPC, Package ()
-
Furquan Shaikh (Code Review) -
Martin Roth (Code Review)
-
Paul Fagerburg (Code Review)
-
SH Kim (Code Review)
-
Sumeet R Pawnikar (Code Review)