Change in coreboot[master]: Helios: Add fan speed control for TSR3 sensor with DPTF settings
Sumeet R Pawnikar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Helios: Add fan speed control for TSR3 sensor with DPTF settings
Add granular fan speed control for TSR3 sensor which is used for CPU cooling. Update passive temperature threshold from 44C to 62C for the same TSR3 sensor.
BRANCH=firmware-hatch-12672.B BUG=b:155347298 TEST=Built and test on Helios system
Change-Id: Ia545ce2c072d48a9a351fe3ad7ea66fe55f80e64 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl 1 file changed, 9 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/41425/1
diff --git a/src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl b/src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl index 886e519..8ac1c0b 100644 --- a/src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl @@ -26,8 +26,15 @@
#define DPTF_TSR3_SENSOR_ID 3 #define DPTF_TSR3_SENSOR_NAME "CPU" -#define DPTF_TSR3_PASSIVE 44 +#define DPTF_TSR3_PASSIVE 62 #define DPTF_TSR3_CRITICAL 90 +#define DPTF_TSR3_ACTIVE_AC0 60 +#define DPTF_TSR3_ACTIVE_AC1 57 +#define DPTF_TSR3_ACTIVE_AC2 55 +#define DPTF_TSR3_ACTIVE_AC3 52 +#define DPTF_TSR3_ACTIVE_AC4 50 +#define DPTF_TSR3_ACTIVE_AC5 48 +#define DPTF_TSR3_ACTIVE_AC6 45
#define DPTF_ENABLE_CHARGER #define DPTF_ENABLE_FAN_CONTROL @@ -83,7 +90,7 @@ 0, 0, 0 }, Package () { - _SB.DPTF.TFN1, _SB.DPTF.TSR3, 100, 0, 0, 0, 0, 0, 0, 0, + _SB.DPTF.TFN1, _SB.DPTF.TSR3, 100, 90, 80, 60, 40, 30, 20, 10, 0, 0, 0 } })
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41425
to look at the new patch set (#2).
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
[TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings
Add granular fan speed control for TSR3 sensor which is used for CPU cooling. Update passive temperature threshold from 44C to 62C for the same TSR3 sensor.
BRANCH=firmware-hatch-12672.B BUG=b:155347298 TEST=Built and test on Helios system
Change-Id: Ia545ce2c072d48a9a351fe3ad7ea66fe55f80e64 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl 1 file changed, 9 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/41425/2
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Mengqi Guo, Hank Lin, Puthikorn Voravootivat, Tim Wawrzynczak, Kane Chen, Ken Lu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41425
to look at the new patch set (#3).
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
[TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings
Add granular fan speed control for TSR3 sensor which is used for CPU cooling. Update passive temperature threshold from 44C to 62C for the same TSR3 sensor.
BRANCH=firmware-hatch-12672.B BUG=b:155347298,154074920 TEST=Built and test on Helios system
Change-Id: Ia545ce2c072d48a9a351fe3ad7ea66fe55f80e64 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl 1 file changed, 9 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/41425/3
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Mengqi Guo, Hank Lin, Puthikorn Voravootivat, Tim Wawrzynczak, Kane Chen, Ken Lu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41425
to look at the new patch set (#4).
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
[TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings
Add granular fan speed control for TSR3 sensor which is used for CPU cooling. Also, update passive temperature threshold from 44C to 50C for the same TSR3 sensor to prevent performance impact.
BRANCH=firmware-hatch-12672.B BUG=b:155347298,154074920 TEST=Built and test on Helios system
Change-Id: Ia545ce2c072d48a9a351fe3ad7ea66fe55f80e64 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl 1 file changed, 9 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/41425/4
Mengqi Guo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... PS4, Line 93: 100 Does this mean at 50 degrees Celsius, both FAN and CPU throttle are at 100%?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... PS4, Line 93: 100
Does this mean at 50 degrees Celsius, both FAN and CPU throttle are at 100%?
My understanding is that 'Weight' is a number indicating how much of an effect (relatively speaking) a Source has on a Target. At 90C from TSR3, the fan will be running at 6700 RPM (line 57).
Puthikorn Voravootivat has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... PS4, Line 93: 100
My understanding is that 'Weight' is a number indicating how much of an effect (relatively speaking) […]
The 100 is weight. Since every thing is 100, there is no effect here. The example of this is you can use weight to throttle charger before cpu.
The 90/80/60... are fan speed.
So at DPTF_TSR3_ACTIVE_AC0 (50C) fan run at speed 90 (=6700 rpm)
But the fan speed is probably trigger sooner from TSR1, 6700rpm @ 43C (DPTF_TSR1_ACTIVE_AC0)
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... PS4, Line 93: 100
The 100 is weight. Since every thing is 100, there is no effect here. […]
Here these entreies are 90, 80, 60,...,10 are the mapped control points for DFPS. When TSR3 temp (temperature) is below 44C (as per last active threshold *_AC6) defined value above at line 37, fan will be off. => When TSR3 temp hits 44C (as per active threshold *_AC6 at line 37), fan will start rotating for mapped AC6 entry in this DART table which is 10 value here. Based on this 10 value, it maps to control point in DFPS table to 10 at line 65 and fan runs at 800 rpm. => Now if TSR3 temp still increases and reaches to 45C (as per next active threshold *_AC5 at line 36), fan speed will increase. Now, this *_AC5 temp entry mapped to 20 in this DART teble. And this maps to control point in DFPS table to 20 at line 64 and fun speed increases to 1608 rpm. => This way if temp keeps increasing to next *_ACx threshold, the fan speed will increases and runs at higher speed following this *ACx value in this DART table and maps to control value and speed rpm value. => Also, whenever temp goes below particular *_ACx threshold, it follows the similar mechanism and mapping and fan speed slows down to lower value. Hope, I am able to explain this flow in most simpler way.
Mengqi Guo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... PS4, Line 93: 100
Here these entreies are 90, 80, 60,...,10 are the mapped control points for DFPS. […]
Thanks everyone for the detailed explanation.
My understanding is fan will kick in at 44C, and fan activity increases as temperature increases. Fan activity hits max when TSR3 hits 50C.
CPU starts to throttle when TSR3 hits 50C.
This way fan activity starts before CPU throttle.
TSR1 also controls fan activity.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/41425/4/src/mainboard/google/hatch/... PS4, Line 93: 100
Thanks everyone for the detailed explanation. […]
Things that can activate the fan: CPU, TSR0, TSR1, TSR2, TSR3 (see DART). CPU throttling is in DTRT below (TSR3 & TSR0 can both start the throttling; you will need to see the throttling tables (_TSS I believe, but I think that's actually empty so I'm not sure where the throttling values come from...), but is initiated by the _PASSIVE temperatures for Targets (second item) in each DTRT package.
Ken Lu has uploaded a new patch set (#5) to the change originally created by Sumeet R Pawnikar. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
[TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings
Add granular fan speed control for TSR3 sensor which is used for CPU cooling. Also, update passive temperature threshold from 44C to 50C for the same TSR3 sensor to prevent performance impact.
BRANCH=firmware-hatch-12672.B BUG=b:155347298,154074920 TEST=Built and test on Helios system
Change-Id: Ia545ce2c072d48a9a351fe3ad7ea66fe55f80e64 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl 1 file changed, 9 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/41425/5
Ken Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 5:
Modify DPTF_TSR3_PASSIVE change from 50 to be 47 .
Ken Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 5: Code-Review+1
Opal Voravootivat has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41425 )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41425/5/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/41425/5/src/mainboard/google/hatch/... PS5, Line 100: 60 This should be higher to reduce performance variant under load.
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41425?usp=email )
Change subject: [TEST] Helios: Add fan speed control for TSR3 sensor with DPTF settings ......................................................................
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.
-
Ken Lu (Code Review) -
Martin L Roth (Code Review)
-
Mengqi Guo (Code Review)
-
Opal Voravootivat (Code Review)
-
Puthikorn Voravootivat (Code Review)
-
Sumeet R Pawnikar (Code Review)
-
Tim Wawrzynczak (Code Review)