Change in coreboot[master]: mb/google/drallion/variants/drallion: Update thermal configuration fo...
John Su has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
mb/google/drallion/variants/drallion: Update thermal configuration for DPTF
Follow thermal table(b/144464314#1) for first tunning.
BUG=b:144464314 TEST=Built and tested on drallion
Change-Id: I4546622cdc6efb2bf2eb973cfc5c6f22c40cc6ef Signed-off-by: John Su john_su@compal.corp-partner.google.com --- M src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl 1 file changed, 17 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/36860/1
diff --git a/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl b/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl index 73e1dec..3ff23b7 100644 --- a/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl @@ -13,42 +13,42 @@ * GNU General Public License for more details. */
-#define DPTF_CPU_PASSIVE 98 -#define DPTF_CPU_CRITICAL 108 +#define DPTF_CPU_PASSIVE 99 +#define DPTF_CPU_CRITICAL 127
/* Skin Sensor for CPU VR temperature monitor */ #define DPTF_TSR0_SENSOR_ID 1 #define DPTF_TSR0_SENSOR_NAME "Skin" -#define DPTF_TSR0_PASSIVE 55 -#define DPTF_TSR0_CRITICAL 100 +#define DPTF_TSR0_PASSIVE 90 +#define DPTF_TSR0_CRITICAL 127
/* Memory Sensor for DDR temperature monitor */ #define DPTF_TSR1_SENSOR_ID 2 #define DPTF_TSR1_SENSOR_NAME "DDR" -#define DPTF_TSR1_PASSIVE 53 -#define DPTF_TSR1_CRITICAL 100 +#define DPTF_TSR1_PASSIVE 45 +#define DPTF_TSR1_CRITICAL 127
/* M.2 Sensor for Ambient temperature monitor */ #define DPTF_TSR2_SENSOR_ID 3 #define DPTF_TSR2_SENSOR_NAME "Ambient" -#define DPTF_TSR2_PASSIVE 38 -#define DPTF_TSR2_CRITICAL 93 +#define DPTF_TSR2_PASSIVE 50 +#define DPTF_TSR2_CRITICAL 127
#undef DPTF_ENABLE_FAN_CONTROL #undef DPTF_ENABLE_CHARGER
Name (DTRT, Package () { /* CPU Throttle Effect on CPU */ - Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 500, 100, 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, 400, 40, 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, 300, 50, 2, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 250, 10, 2, 0, 0, 0 },
/* CPU Throttle Effect on Ambient (TSR2) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 1000, 100, 1, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 250, 10, 1, 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 */ - 21000, /* PowerLimitMaximum */ - 28000, /* TimeWindowMinimum */ - 28000, /* TimeWindowMaximum */ + 4000, /* PowerLimitMinimum */ + 15000, /* PowerLimitMaximum */ + 10000, /* TimeWindowMinimum */ + 10000, /* TimeWindowMaximum */ 100 /* StepSize */ }, Package () { /* Power Limit 2 */ 1, /* PowerLimitIndex, 1 for Power Limit 2 */ - 15000, /* PowerLimitMinimum */ + 4000, /* PowerLimitMinimum */ 51000, /* PowerLimitMaximum */ 28000, /* TimeWindowMinimum */ 28000, /* TimeWindowMaximum */
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
This change is ready for review.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 17: 127 please share the details on how these new high values are helping compared to previous old values? Did you observe any issues with old values ? Thanks.
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 60: 15 What's the reason for changing this from 21W to 15W for PL1 Maximum value ? What's the TDP of this designed system ?
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 62: 10 Does these new lower (10000) values of TimeWindowMinimum/Maximum helping you to address any existing issues with old (28000) values ?
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 67: 4 please provide detailed reasons for changing this to such a lower value ? Did you check any performance impact with this change ?
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 17: 127
please share the details on how these new high values are helping compared to previous old values? D […]
From b/144464314#8 We don't use critical policy to protect system overheat, we use thermal sensor PSV point and Tcc function to protect system overheat.
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 60: 15
What's the reason for changing this from 21W to 15W for PL1 Maximum value ? What's the TDP of this d […]
Our project TDP is 15W, the reason is DPTF just use passive policy 1, so we set PL1 Max=TDP, and we run performance test by Aquarium 1000 fishes also can meet target 44+ FPS
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 62: 10
Does these new lower (10000) values of TimeWindowMinimum/Maximum helping you to address any existing […]
There is no issue about this change, BTW is there any setting you recommendation about this value?
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 67: 4
please provide detailed reasons for changing this to such a lower value ? Did you check any performa […]
Which one do you confused? PL1 min or PL2 min? there will have a change that we will revise PL2 min from 4000 to 15000.
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 1:
Could you get us some feedback?
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2:
This change is ready for review.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36860/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/36860/2/src/mainboard/google/dralli... PS2, Line 70: 0 Kindly, confirm these new values for PowerLimit2 for TimeWindow Minimum & Maximum. Earlier these values were 28000 but here it's increased to 280000. Also, check and confirm on the similar values for above PowerLimit1.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36860/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36860/2//COMMIT_MSG@9 PS2, Line 9: tunning tuning
Hello Sumeet R Pawnikar, Mathew King, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36860
to look at the new patch set (#3).
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
mb/google/drallion/variants/drallion: Update thermal configuration for DPTF
Follow thermal table for first tuning.
BUG=b:144464314 TEST=Built and tested on drallion
Change-Id: I4546622cdc6efb2bf2eb973cfc5c6f22c40cc6ef Signed-off-by: John Su john_su@compal.corp-partner.google.com --- M src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl 1 file changed, 18 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/36860/3
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36860/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36860/2//COMMIT_MSG@9 PS2, Line 9: tunning
tuning
Done
https://review.coreboot.org/c/coreboot/+/36860/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/36860/2/src/mainboard/google/dralli... PS2, Line 70: 0
Kindly, confirm these new values for PowerLimit2 for TimeWindow Minimum & Maximum. […]
https://partnerissuetracker.corp.google.com/issues/144464314#comment13
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/36860/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/36860/2/src/mainboard/google/dralli... PS2, Line 70: 0
As per the comments in this issue, this change looks fine.
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 3:
(1 comment)
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/36860/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/36860/2/src/mainboard/google/dralli... PS2, Line 70: 0
As per the comments in this issue, this change looks fine.
Ack
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 3: Code-Review+2
(4 comments)
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 17: 127
From b/144464314#8 […]
Ack
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 60: 15
Our project TDP is 15W, the reason is DPTF just use passive policy 1, so we set PL1 Max=TDP, […]
Ack
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 62: 10
There is no issue about this change, BTW is there any setting you recommendation about this value?
Ack
https://review.coreboot.org/c/coreboot/+/36860/1/src/mainboard/google/dralli... PS1, Line 67: 4
Which one do you confused? PL1 min or PL2 min? there will have a change that we will revise PL2 min […]
Ack
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36860 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
mb/google/drallion/variants/drallion: Update thermal configuration for DPTF
Follow thermal table for first tuning.
BUG=b:144464314 TEST=Built and tested on drallion
Change-Id: I4546622cdc6efb2bf2eb973cfc5c6f22c40cc6ef Signed-off-by: John Su john_su@compal.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36860 Reviewed-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Reviewed-by: EricR Lai ericr_lai@compal.corp-partner.google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl 1 file changed, 18 insertions(+), 18 deletions(-)
Approvals: build bot (Jenkins): Verified Sumeet R Pawnikar: Looks good to me, approved EricR Lai: Looks good to me, approved
diff --git a/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl b/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl index 73e1dec..4ecdf1a 100644 --- a/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl @@ -13,42 +13,42 @@ * GNU General Public License for more details. */
-#define DPTF_CPU_PASSIVE 98 -#define DPTF_CPU_CRITICAL 108 +#define DPTF_CPU_PASSIVE 99 +#define DPTF_CPU_CRITICAL 127
/* Skin Sensor for CPU VR temperature monitor */ #define DPTF_TSR0_SENSOR_ID 1 #define DPTF_TSR0_SENSOR_NAME "Skin" -#define DPTF_TSR0_PASSIVE 55 -#define DPTF_TSR0_CRITICAL 100 +#define DPTF_TSR0_PASSIVE 64 +#define DPTF_TSR0_CRITICAL 127
/* Memory Sensor for DDR temperature monitor */ #define DPTF_TSR1_SENSOR_ID 2 #define DPTF_TSR1_SENSOR_NAME "DDR" -#define DPTF_TSR1_PASSIVE 53 -#define DPTF_TSR1_CRITICAL 100 +#define DPTF_TSR1_PASSIVE 54 +#define DPTF_TSR1_CRITICAL 127
/* M.2 Sensor for Ambient temperature monitor */ #define DPTF_TSR2_SENSOR_ID 3 #define DPTF_TSR2_SENSOR_NAME "Ambient" -#define DPTF_TSR2_PASSIVE 38 -#define DPTF_TSR2_CRITICAL 93 +#define DPTF_TSR2_PASSIVE 40 +#define DPTF_TSR2_CRITICAL 127
#undef DPTF_ENABLE_FAN_CONTROL #undef DPTF_ENABLE_CHARGER
Name (DTRT, Package () { /* CPU Throttle Effect on CPU */ - Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 500, 100, 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, 400, 40, 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, 300, 50, 2, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 250, 10, 2, 0, 0, 0 },
/* CPU Throttle Effect on Ambient (TSR2) */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 1000, 100, 1, 0, 0, 0 }, + Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 250, 10, 1, 0, 0, 0 }, })
Name (MPPC, Package () @@ -56,18 +56,18 @@ 0x2, /* Revision */ Package () { /* Power Limit 1 */ 0, /* PowerLimitIndex, 0 for Power Limit 1 */ - 3000, /* PowerLimitMinimum */ - 21000, /* PowerLimitMaximum */ - 28000, /* TimeWindowMinimum */ - 28000, /* TimeWindowMaximum */ + 4000, /* PowerLimitMinimum */ + 15000, /* PowerLimitMaximum */ + 100000, /* TimeWindowMinimum */ + 100000, /* TimeWindowMaximum */ 100 /* StepSize */ }, Package () { /* Power Limit 2 */ 1, /* PowerLimitIndex, 1 for Power Limit 2 */ 15000, /* PowerLimitMinimum */ 51000, /* PowerLimitMaximum */ - 28000, /* TimeWindowMinimum */ - 28000, /* TimeWindowMaximum */ + 280000, /* TimeWindowMinimum */ + 280000, /* TimeWindowMaximum */ 100 /* StepSize */ } })
-
EricR Lai (Code Review) -
Furquan Shaikh (Code Review)
-
John Su (Code Review)
-
Paul Menzel (Code Review)
-
Sumeet R Pawnikar (Code Review)