Change in ...coreboot[master]: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF
John Su has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32118
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
mb/google/sarien/variants/sarien: Update thermal configuration for DPTF
Follow thermal table for second tunning.
BUG=b:129509918 TEST=Built and tested on sarien system
Change-Id: I64844b84891dc3ab7abe9378cdca5dcf57b3e433 Signed-off-by: John Su john_su@compal.corp-partner.google.com --- M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl 2 files changed, 10 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/32118/1
diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb index 2714b60..4544b0f 100644 --- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb +++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb @@ -42,7 +42,7 @@ register "SlowSlewRateForGt" = "2" register "SlowSlewRateForSa" = "2" register "SlowSlewRateForFivr" = "2" - register "tdp_pl1_override" = "25" + register "tdp_pl1_override" = "15" register "tdp_pl2_override" = "51" register "Device4Enable" = "1" # Enable eDP device diff --git a/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl b/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl index be052f8..0cdbcd1 100644 --- a/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl @@ -13,7 +13,7 @@ * GNU General Public License for more details. */
-#define DPTF_CPU_PASSIVE 95 +#define DPTF_CPU_PASSIVE 99 #define DPTF_CPU_CRITICAL 105
/* Skin Sensor for CPU VR temperature monitor */ @@ -25,7 +25,7 @@ /* Memory Sensor for DDR temperature monitor */ #define DPTF_TSR1_SENSOR_ID 2 #define DPTF_TSR1_SENSOR_NAME "DDR" -#define DPTF_TSR1_PASSIVE 56 +#define DPTF_TSR1_PASSIVE 55 #define DPTF_TSR1_CRITICAL 100
/* M.2 Sensor for Ambient temperature monitor */ @@ -39,16 +39,16 @@
Name (DTRT, Package () { /* CPU Throttle Effect on CPU */ - Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 5000, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 250, 10, 0, 0, 0, 0 },
/* CPU Throttle Effect on Skin (TSR0) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR0, 200, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR0, 250, 10, 0, 0, 0, 0 },
/* CPU Throttle Effect on DDR (TSR1) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 2000, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 250, 10, 0, 0, 0, 0 },
/* CPU Throttle Effect on Ambient (TSR2) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 200, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 250, 10, 0, 0, 0, 0 }, })
Name (MPPC, Package () @@ -56,15 +56,15 @@ 0x2, /* Revision */ Package () { /* Power Limit 1 */ 0, /* PowerLimitIndex, 0 for Power Limit 1 */ - 3000, /* PowerLimitMinimum */ - 25000, /* PowerLimitMaximum */ + 5000, /* PowerLimitMinimum */ + 15000, /* PowerLimitMaximum */ 10000, /* TimeWindowMinimum */ 10000, /* TimeWindowMaximum */ 100 /* StepSize */ }, Package () { /* Power Limit 2 */ 1, /* PowerLimitIndex, 1 for Power Limit 2 */ - 3000, /* PowerLimitMinimum */ + 5000, /* PowerLimitMinimum */ 51000, /* PowerLimitMaximum */ 28000, /* TimeWindowMinimum */ 28000, /* TimeWindowMaximum */
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32118/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32118/1//COMMIT_MSG@9 PS1, Line 9: Follow thermal table for second tunning. Are these final tuning values based on latest system available ? OR will there be some more tuning coming in future ?
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... File src/mainboard/google/sarien/variants/sarien/devicetree.cb:
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... PS1, Line 45: 15 Any reason for this change ? We had set 25W as per this BUG=b:122343940 mentioned in CL https://review.coreboot.org/c/coreboot/+/30908
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32118/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32118/1//COMMIT_MSG@9 PS1, Line 9: Follow thermal table for second tunning.
Are these final tuning values based on latest system available ? OR will there be some more tuning c […]
Yes, these are tuning values based on latest devices and system.
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... File src/mainboard/google/sarien/variants/sarien/devicetree.cb:
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... PS1, Line 45: 15
Any reason for this change ? […]
Because Chromebook use DPTF passive policy 1, so don't need set PL2 25W to dynamic to TDP 15W, thanks.
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
Patch Set 1: Code-Review+2
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... File src/mainboard/google/sarien/variants/sarien/devicetree.cb:
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... PS1, Line 45: 15
Because Chromebook use DPTF passive policy 1, so don't need set PL2 25W to dynamic to TDP 15W, thank […]
Did you check above mentioned BUG:b:122343940 ? You only requested some time back to change it from 15W to 25W. And now you are changing it back to 15W on this bug c#1.
Also, did you get chance to check the performance benchmark impact due to this your new change from 25W to 15W ?
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... File src/mainboard/google/sarien/variants/sarien/devicetree.cb:
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... PS1, Line 45: 15
Did you check above mentioned BUG:b:122343940 ? You only requested some time back to change it from […]
For issue:122343940 PL1 set 25W is need to leverage Window system's setting at first, but Windows system use passive policy 2.0, so issue:122343940 modify PL1 to 25W is not really a issue, it just want to leverage Window system's setting, and Window check performance by benchmark, but Chromebook by Aquarium, both are different.
Due to Chromebook performance check spec is aquarium FPS:60, so PL1 change to 15W not impact performance also FPS 60,too, thanks.
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... File src/mainboard/google/sarien/variants/sarien/devicetree.cb:
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... PS1, Line 45: 15
For issue:122343940 PL1 set 25W is need to leverage Window system's setting at first, but Windows sy […]
Sumeet, Do you have concern?
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... File src/mainboard/google/sarien/variants/sarien/devicetree.cb:
https://review.coreboot.org/#/c/32118/1/src/mainboard/google/sarien/variants... PS1, Line 45: 15
Sumeet, […]
Fine with above info.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
Patch Set 1: Code-Review+1
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32118 )
Change subject: mb/google/sarien/variants/sarien: Update thermal configuration for DPTF ......................................................................
mb/google/sarien/variants/sarien: Update thermal configuration for DPTF
Follow thermal table for second tunning.
BUG=b:129509918 TEST=Built and tested on sarien system
Change-Id: I64844b84891dc3ab7abe9378cdca5dcf57b3e433 Signed-off-by: John Su john_su@compal.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32118 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Lijian Zhao lijian.zhao@intel.com Reviewed-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl 2 files changed, 10 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Lijian Zhao: Looks good to me, approved Sumeet R Pawnikar: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb index 72dea1e..caae79f 100644 --- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb +++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb @@ -42,7 +42,7 @@ register "SlowSlewRateForGt" = "2" register "SlowSlewRateForSa" = "2" register "SlowSlewRateForFivr" = "2" - register "tdp_pl1_override" = "25" + register "tdp_pl1_override" = "15" register "tdp_pl2_override" = "51" register "Device4Enable" = "1" # Enable eDP device diff --git a/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl b/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl index be052f8..0cdbcd1 100644 --- a/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/dptf.asl @@ -13,7 +13,7 @@ * GNU General Public License for more details. */
-#define DPTF_CPU_PASSIVE 95 +#define DPTF_CPU_PASSIVE 99 #define DPTF_CPU_CRITICAL 105
/* Skin Sensor for CPU VR temperature monitor */ @@ -25,7 +25,7 @@ /* Memory Sensor for DDR temperature monitor */ #define DPTF_TSR1_SENSOR_ID 2 #define DPTF_TSR1_SENSOR_NAME "DDR" -#define DPTF_TSR1_PASSIVE 56 +#define DPTF_TSR1_PASSIVE 55 #define DPTF_TSR1_CRITICAL 100
/* M.2 Sensor for Ambient temperature monitor */ @@ -39,16 +39,16 @@
Name (DTRT, Package () { /* CPU Throttle Effect on CPU */ - Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 5000, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 250, 10, 0, 0, 0, 0 },
/* CPU Throttle Effect on Skin (TSR0) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR0, 200, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR0, 250, 10, 0, 0, 0, 0 },
/* CPU Throttle Effect on DDR (TSR1) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 2000, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 250, 10, 0, 0, 0, 0 },
/* CPU Throttle Effect on Ambient (TSR2) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 200, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 250, 10, 0, 0, 0, 0 }, })
Name (MPPC, Package () @@ -56,15 +56,15 @@ 0x2, /* Revision */ Package () { /* Power Limit 1 */ 0, /* PowerLimitIndex, 0 for Power Limit 1 */ - 3000, /* PowerLimitMinimum */ - 25000, /* PowerLimitMaximum */ + 5000, /* PowerLimitMinimum */ + 15000, /* PowerLimitMaximum */ 10000, /* TimeWindowMinimum */ 10000, /* TimeWindowMaximum */ 100 /* StepSize */ }, Package () { /* Power Limit 2 */ 1, /* PowerLimitIndex, 1 for Power Limit 2 */ - 3000, /* PowerLimitMinimum */ + 5000, /* PowerLimitMinimum */ 51000, /* PowerLimitMaximum */ 28000, /* TimeWindowMinimum */ 28000, /* TimeWindowMaximum */
-
John Su (Code Review) -
Lijian Zhao (Code Review)
-
Patrick Georgi (Code Review)
-
Sumeet R Pawnikar (Code Review)