Hello SH Kim,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/40115
to review the following change.
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
mb/google/nightfury: Update DPTF parameters
Applying initial DPTF parameters for nightfury from intermal thermal team. Will update after further thermal/performance tuning.
BUG=b:149226871 BRANCH=firmware-hatch-12672.B TEST=built and verified FAN worked by DPTF active policy
Signed-off-by: Seunghwan Kim sh_.kim@samsung.corp-partner.google.com Change-Id: I712bdd8edc999ef7ee33f4adf21893be12e86bec --- M src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl 1 file changed, 53 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40115/1
diff --git a/src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl b/src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl index edfad4b..e6fe028 100644 --- a/src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl @@ -13,38 +13,31 @@ * GNU General Public License for more details. */
-#define DPTF_CPU_PASSIVE 50 +#define DPTF_CPU_PASSIVE 90 #define DPTF_CPU_CRITICAL 105
#define DPTF_TSR0_SENSOR_ID 0 #define DPTF_TSR0_SENSOR_NAME "Thermal Sensor - Charger" -#define DPTF_TSR0_PASSIVE 45 +#define DPTF_TSR0_PASSIVE 75 #define DPTF_TSR0_CRITICAL 90 -#define DPTF_TSR0_TABLET_PASSIVE 32 -#define DPTF_TSR0_TABLET_CRITICAL 90
#define DPTF_TSR1_SENSOR_ID 1 #define DPTF_TSR1_SENSOR_NAME "Thermal Sensor - 5V" -#define DPTF_TSR1_PASSIVE 45 +#define DPTF_TSR1_PASSIVE 70 #define DPTF_TSR1_CRITICAL 90 -#define DPTF_TSR1_TABLET_PASSIVE 32 -#define DPTF_TSR1_TABLET_CRITICAL 90 +#define DPTF_TSR1_ACTIVE_AC0 48 +#define DPTF_TSR1_ACTIVE_AC1 46 +#define DPTF_TSR1_ACTIVE_AC2 44 +#define DPTF_TSR1_ACTIVE_AC3 41 +#define DPTF_TSR1_ACTIVE_AC4 39
#define DPTF_TSR2_SENSOR_ID 2 -#define DPTF_TSR2_SENSOR_NAME "Thermal Sensor - IA" -#define DPTF_TSR2_PASSIVE 45 +#define DPTF_TSR2_SENSOR_NAME "Thermal Sensor - GT" +#define DPTF_TSR2_PASSIVE 75 #define DPTF_TSR2_CRITICAL 90 -#define DPTF_TSR2_TABLET_PASSIVE 32 -#define DPTF_TSR2_TABLET_CRITICAL 90 - -#define DPTF_TSR3_SENSOR_ID 3 -#define DPTF_TSR3_SENSOR_NAME "Thermal Sensor - GT" -#define DPTF_TSR3_PASSIVE 45 -#define DPTF_TSR3_CRITICAL 90 -#define DPTF_TSR3_TABLET_PASSIVE 32 -#define DPTF_TSR3_TABLET_CRITICAL 90
#define DPTF_ENABLE_CHARGER +#define DPTF_ENABLE_FAN_CONTROL
/* Charger performance states, board-specific values from charger and EC */ Name (CHPS, Package () { @@ -54,6 +47,47 @@ Package () { 0, 0, 0, 0, 8, 0x200, "mA", 0 }, /* 0.5A */ })
+/* DFPS: Fan Performance States */ +Name (DFPS, Package () { + 0, // Revision + /* Control, Trip Point, Speed, NoiseLevel, Power */ + Package () {90, 0xFFFFFFFF, 5900, 220, 2200}, + Package () {80, 0xFFFFFFFF, 5400, 180, 1800}, + Package () {70, 0xFFFFFFFF, 4900, 145, 1450}, + Package () {60, 0xFFFFFFFF, 4500, 115, 1150}, + Package () {50, 0xFFFFFFFF, 4000, 90, 900}, + Package () {40, 0xFFFFFFFF, 3000, 55, 550}, + Package () {30, 0xFFFFFFFF, 2200, 30, 300}, + Package () {20, 0xFFFFFFFF, 1600, 15, 150}, + Package () {10, 0xFFFFFFFF, 800, 10, 100}, + Package () {0, 0xFFFFFFFF, 0, 0, 50} +}) + +Name (DART, Package () { + /* Fan effect on CPU */ + 0, // Revision + Package () { + /* + * Source, Target, Weight, AC0, AC1, AC2, AC3, AC4, AC5, AC6, + * AC7, AC8, AC9 + */ + _SB.DPTF.TFN1, _SB.PCI0.TCPU, 100, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0 + }, + Package () { + _SB.DPTF.TFN1, _SB.DPTF.TSR0, 100, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0 + }, + Package () { + _SB.DPTF.TFN1, _SB.DPTF.TSR1, 100, 70, 63, 54, 48, 44, 0, 0, + 0, 0, 0 + }, + Package () { + _SB.DPTF.TFN1, _SB.DPTF.TSR2, 100, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0 + } +}) + Name (DTRT, Package () { /* CPU Throttle Effect on CPU */ Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 100, 10, 0, 0, 0, 0 }, @@ -64,11 +98,8 @@ /* 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) */ + /* CPU Throttle Effect on GT (TSR2) */ Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 100, 10, 0, 0, 0, 0 }, - - /* CPU Throttle Effect on GT (TSR3) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR3, 100, 10, 0, 0, 0, 0 }, })
Name (MPPC, Package ()
Hello Patrick Georgi, Furquan Shaikh, Tim Wawrzynczak, SH Kim,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40115
to look at the new patch set (#2).
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
mb/google/nightfury: Update DPTF parameters
Applying initial DPTF parameters for nightfury from internal thermal team. Will update after further thermal/performance tuning.
BUG=b:149226871 BRANCH=firmware-hatch-12672.B TEST=built and verified FAN worked by DPTF active policy
Signed-off-by: Seunghwan Kim sh_.kim@samsung.corp-partner.google.com Change-Id: I712bdd8edc999ef7ee33f4adf21893be12e86bec --- M src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl 1 file changed, 53 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40115/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40115 )
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40115/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40115/2//COMMIT_MSG@9 PS2, Line 9: Applying Imperative mood: Apply …
https://review.coreboot.org/c/coreboot/+/40115/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/40115/2/src/mainboard/google/hatch/... PS2, Line 75: 0, 0, 0 Fits on line above in 96 characters.
Hello Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Tim Wawrzynczak, SH Kim,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40115
to look at the new patch set (#3).
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
mb/google/nightfury: Update DPTF parameters
Apply initial DPTF parameters for nightfury from internal thermal team. Will update after further thermal/performance tuning.
BUG=b:149226871 BRANCH=firmware-hatch-12672.B TEST=built and verified FAN worked by DPTF active policy
Signed-off-by: Seunghwan Kim sh_.kim@samsung.corp-partner.google.com Change-Id: I712bdd8edc999ef7ee33f4adf21893be12e86bec --- M src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl 1 file changed, 53 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40115/3
Hello Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Tim Wawrzynczak, SH Kim,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40115
to look at the new patch set (#4).
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
mb/google/nightfury: Update DPTF parameters
Apply initial DPTF parameters for nightfury from internal thermal team. Will update after further thermal/performance tuning.
BUG=b:149226871 BRANCH=firmware-hatch-12672.B TEST=built and verified FAN worked by DPTF active policy
Signed-off-by: Seunghwan Kim sh_.kim@samsung.corp-partner.google.com Change-Id: I712bdd8edc999ef7ee33f4adf21893be12e86bec --- M src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl 1 file changed, 48 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40115/4
shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40115 )
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40115/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40115/2//COMMIT_MSG@9 PS2, Line 9: Applying
Imperative mood: Apply …
Done
https://review.coreboot.org/c/coreboot/+/40115/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/40115/2/src/mainboard/google/hatch/... PS2, Line 75: 0, 0, 0
Fits on line above in 96 characters.
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40115 )
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
Patch Set 4: Code-Review+2
shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40115 )
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
Patch Set 4:
Patch Set 4: Code-Review+2
We need this change for upcoming build, can we merge this into FW branch quickly?
Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40115 )
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
mb/google/nightfury: Update DPTF parameters
Apply initial DPTF parameters for nightfury from internal thermal team. Will update after further thermal/performance tuning.
BUG=b:149226871 BRANCH=firmware-hatch-12672.B TEST=built and verified FAN worked by DPTF active policy
Signed-off-by: Seunghwan Kim sh_.kim@samsung.corp-partner.google.com Change-Id: I712bdd8edc999ef7ee33f4adf21893be12e86bec Reviewed-on: https://review.coreboot.org/c/coreboot/+/40115 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl 1 file changed, 48 insertions(+), 22 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl b/src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl index 894e2f0..cdb4258 100644 --- a/src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/hatch/variants/nightfury/include/variant/acpi/dptf.asl @@ -1,38 +1,31 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* This file is part of the coreboot project. */
-#define DPTF_CPU_PASSIVE 50 +#define DPTF_CPU_PASSIVE 90 #define DPTF_CPU_CRITICAL 105
#define DPTF_TSR0_SENSOR_ID 0 #define DPTF_TSR0_SENSOR_NAME "Thermal Sensor - Charger" -#define DPTF_TSR0_PASSIVE 45 +#define DPTF_TSR0_PASSIVE 75 #define DPTF_TSR0_CRITICAL 90 -#define DPTF_TSR0_TABLET_PASSIVE 32 -#define DPTF_TSR0_TABLET_CRITICAL 90
#define DPTF_TSR1_SENSOR_ID 1 #define DPTF_TSR1_SENSOR_NAME "Thermal Sensor - 5V" -#define DPTF_TSR1_PASSIVE 45 +#define DPTF_TSR1_PASSIVE 70 #define DPTF_TSR1_CRITICAL 90 -#define DPTF_TSR1_TABLET_PASSIVE 32 -#define DPTF_TSR1_TABLET_CRITICAL 90 +#define DPTF_TSR1_ACTIVE_AC0 48 +#define DPTF_TSR1_ACTIVE_AC1 46 +#define DPTF_TSR1_ACTIVE_AC2 44 +#define DPTF_TSR1_ACTIVE_AC3 41 +#define DPTF_TSR1_ACTIVE_AC4 39
#define DPTF_TSR2_SENSOR_ID 2 -#define DPTF_TSR2_SENSOR_NAME "Thermal Sensor - IA" -#define DPTF_TSR2_PASSIVE 45 +#define DPTF_TSR2_SENSOR_NAME "Thermal Sensor - GT" +#define DPTF_TSR2_PASSIVE 75 #define DPTF_TSR2_CRITICAL 90 -#define DPTF_TSR2_TABLET_PASSIVE 32 -#define DPTF_TSR2_TABLET_CRITICAL 90 - -#define DPTF_TSR3_SENSOR_ID 3 -#define DPTF_TSR3_SENSOR_NAME "Thermal Sensor - GT" -#define DPTF_TSR3_PASSIVE 45 -#define DPTF_TSR3_CRITICAL 90 -#define DPTF_TSR3_TABLET_PASSIVE 32 -#define DPTF_TSR3_TABLET_CRITICAL 90
#define DPTF_ENABLE_CHARGER +#define DPTF_ENABLE_FAN_CONTROL
/* Charger performance states, board-specific values from charger and EC */ Name (CHPS, Package () { @@ -42,6 +35,42 @@ Package () { 0, 0, 0, 0, 8, 0x200, "mA", 0 }, /* 0.5A */ })
+/* DFPS: Fan Performance States */ +Name (DFPS, Package () { + 0, // Revision + /* Control, Trip Point, Speed, NoiseLevel, Power */ + Package () {90, 0xFFFFFFFF, 5900, 220, 2200}, + Package () {80, 0xFFFFFFFF, 5400, 180, 1800}, + Package () {70, 0xFFFFFFFF, 4900, 145, 1450}, + Package () {60, 0xFFFFFFFF, 4500, 115, 1150}, + Package () {50, 0xFFFFFFFF, 4000, 90, 900}, + Package () {40, 0xFFFFFFFF, 3000, 55, 550}, + Package () {30, 0xFFFFFFFF, 2200, 30, 300}, + Package () {20, 0xFFFFFFFF, 1600, 15, 150}, + Package () {10, 0xFFFFFFFF, 800, 10, 100}, + Package () {0, 0xFFFFFFFF, 0, 0, 50} +}) + +Name (DART, Package () { + 0, // Revision + + /* + * Source, Target, Weight, AC0, AC1, AC2, AC3, AC4, AC5, AC6, AC7, AC8, AC9 + */ + Package () { + _SB.DPTF.TFN1, _SB.PCI0.TCPU, 100, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + }, + Package () { + _SB.DPTF.TFN1, _SB.DPTF.TSR0, 100, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + }, + Package () { + _SB.DPTF.TFN1, _SB.DPTF.TSR1, 100, 70, 63, 54, 48, 44, 0, 0, 0, 0, 0 + }, + Package () { + _SB.DPTF.TFN1, _SB.DPTF.TSR2, 100, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + } +}) + Name (DTRT, Package () { /* CPU Throttle Effect on CPU */ Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 100, 10, 0, 0, 0, 0 }, @@ -52,11 +81,8 @@ /* 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) */ + /* CPU Throttle Effect on GT (TSR2) */ Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 100, 10, 0, 0, 0, 0 }, - - /* CPU Throttle Effect on GT (TSR3) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR3, 100, 10, 0, 0, 0, 0 }, })
Name (MPPC, Package ()
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40115 )
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
Patch Set 5:
Patch Set 4:
Patch Set 4: Code-Review+2
We need this change for upcoming build, can we merge this into FW branch quickly?
Done.
https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40115 )
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
Patch Set 5:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2176 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2175 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2174
Please note: This test is under development and might not be accurate at all!
shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40115 )
Change subject: mb/google/nightfury: Update DPTF parameters ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 4:
Patch Set 4: Code-Review+2
We need this change for upcoming build, can we merge this into FW branch quickly?
Done.
https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Thanks Shelley!
-
9elements QA (Code Review) -
Paul Menzel (Code Review)
-
Shelley Chen (Code Review)
-
shkim (Code Review)
-
Tim Wawrzynczak (Code Review)