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/+/38658 )
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 fine tuning. 1. Update PSV values for sensers. 2. Change PL1 min value from 4w to 5w. 3. Change PL1 max value from 15w to 12w. 3. Change PL2 min value from 15w to 12w.
BUG=b:148627484 TEST=Built and tested on drallion
Change-Id: I957d41e3c14f6dbcec8c3555382895698beabe40 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, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/38658/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 4ecdf1a..be8fa39 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 @@ -19,19 +19,19 @@ /* Skin Sensor for CPU VR temperature monitor */ #define DPTF_TSR0_SENSOR_ID 1 #define DPTF_TSR0_SENSOR_NAME "Skin" -#define DPTF_TSR0_PASSIVE 64 +#define DPTF_TSR0_PASSIVE 60 #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 54 +#define DPTF_TSR1_PASSIVE 52 #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 40 +#define DPTF_TSR2_PASSIVE 90 #define DPTF_TSR2_CRITICAL 127
#undef DPTF_ENABLE_FAN_CONTROL @@ -56,15 +56,15 @@ 0x2, /* Revision */ Package () { /* Power Limit 1 */ 0, /* PowerLimitIndex, 0 for Power Limit 1 */ - 4000, /* PowerLimitMinimum */ - 15000, /* PowerLimitMaximum */ + 5000, /* PowerLimitMinimum */ + 12000, /* PowerLimitMaximum */ 100000, /* TimeWindowMinimum */ 100000, /* TimeWindowMaximum */ 100 /* StepSize */ }, Package () { /* Power Limit 2 */ 1, /* PowerLimitIndex, 1 for Power Limit 2 */ - 15000, /* PowerLimitMinimum */ + 12000, /* PowerLimitMinimum */ 51000, /* PowerLimitMaximum */ 280000, /* TimeWindowMinimum */ 280000, /* TimeWindowMaximum */
John Su has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/38658 )
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 fine tuning. 1. Update PSV values for sensers. 2. Change PL1 min value from 4w to 5w. 3. Change PL1 max value from 15w to 12w. 4. Change PL2 min value from 15w to 12w.
BUG=b:148627484 TEST=Built and tested on drallion
Change-Id: I957d41e3c14f6dbcec8c3555382895698beabe40 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, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/38658/2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2: Code-Review+2
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2:
(2 comments)
I do not have bug access for this. Please, add me in this bug for more detailed analysis on this. Thanks.
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... PS2, Line 34: 90 How did you arrive to such a high temperature thershold value for Ambient sensor for passive thermal throttling action ? Is it safe to put this high thershold ?
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... PS2, Line 60: 12 Did you check the performance impact due to reduction of this PL1 and PL2 values ? Kindly share any performance analysis results for these new values on bug as comment.
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... PS2, Line 34: 90
How did you arrive to such a high temperature thershold value for Ambient sensor for passive thermal […]
Yes, for our test item request the thermal sensor trigger point is all in control.
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... PS2, Line 60: 12
Did you check the performance impact due to reduction of this PL1 and PL2 values ? […]
Yes, we have already check the performance test by WebGL aquarium, the spec is FPS:60 with 1000 fishes, and can meet target.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2: Code-Review-1
We have measured performance degradation with this CL. Updated detailed report on bug. Please, hold on this CL as of now till bug comments get addressed.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... PS2, Line 60: 12
Yes, we have already check the performance test by WebGL aquarium, the spec is FPS:60 with 1000 fish […]
We have observed performance impact on many benchmarks due to this change. All data results shared on bug.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2: Code-Review+1
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38658/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38658/2//COMMIT_MSG@10 PS2, Line 10: sensers sensors
https://review.coreboot.org/c/coreboot/+/38658/2//COMMIT_MSG@11 PS2, Line 11: 2. Change PL1 min value from 4w to 5w. SI unit for Watts is W.
Hello EricR Lai, 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/+/38658
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 fine tuning. 1. Update PSV values for sensors. 2. Change PL1 min value from 4W to 5W. 3. Change PL1 max value from 15W to 12W. 4. Change PL2 min value from 15W to 12W.
BUG=b:148627484 TEST=Built and tested on drallion
Change-Id: I957d41e3c14f6dbcec8c3555382895698beabe40 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, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/38658/3
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38658/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38658/2//COMMIT_MSG@10 PS2, Line 10: sensers
sensors
Done
https://review.coreboot.org/c/coreboot/+/38658/2//COMMIT_MSG@11 PS2, Line 11: 2. Change PL1 min value from 4w to 5w.
SI unit for Watts is W.
Done
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 3: -Code-Review
As per discussion on bug and data provided with this CL, removing -1 comment here.
Hello EricR Lai, 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/+/38658
to look at the new patch set (#4).
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 fine tuning. 1. Update PSV values for sensors. 2. Change PL1 min value from 4W to 5W. 3. Change PL1 max value from 15W to 12W. 4. Change PL2 min value from 15W to 12W.
BUG=b:148627484 TEST=Built and tested on drallion
Change-Id: I957d41e3c14f6dbcec8c3555382895698beabe40 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, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/38658/4
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 4: Code-Review+2
John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... PS2, Line 34: 90
Yes, for our test item request the thermal sensor trigger point is all in control.
Ack
https://review.coreboot.org/c/coreboot/+/38658/2/src/mainboard/google/dralli... PS2, Line 60: 12
We have observed performance impact on many benchmarks due to this change. […]
Ack
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
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 fine tuning. 1. Update PSV values for sensors. 2. Change PL1 min value from 4W to 5W. 3. Change PL1 max value from 15W to 12W. 4. Change PL2 min value from 15W to 12W.
BUG=b:148627484 TEST=Built and tested on drallion
Change-Id: I957d41e3c14f6dbcec8c3555382895698beabe40 Signed-off-by: John Su john_su@compal.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38658 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: EricR Lai ericr_lai@compal.corp-partner.google.com Reviewed-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/drallion/variants/drallion/include/variant/acpi/dptf.asl 1 file changed, 6 insertions(+), 6 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 4ecdf1a..6f114b7 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 @@ -19,19 +19,19 @@ /* Skin Sensor for CPU VR temperature monitor */ #define DPTF_TSR0_SENSOR_ID 1 #define DPTF_TSR0_SENSOR_NAME "Skin" -#define DPTF_TSR0_PASSIVE 64 +#define DPTF_TSR0_PASSIVE 67 #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 54 +#define DPTF_TSR1_PASSIVE 60 #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 40 +#define DPTF_TSR2_PASSIVE 90 #define DPTF_TSR2_CRITICAL 127
#undef DPTF_ENABLE_FAN_CONTROL @@ -56,15 +56,15 @@ 0x2, /* Revision */ Package () { /* Power Limit 1 */ 0, /* PowerLimitIndex, 0 for Power Limit 1 */ - 4000, /* PowerLimitMinimum */ - 15000, /* PowerLimitMaximum */ + 5000, /* PowerLimitMinimum */ + 12000, /* PowerLimitMaximum */ 100000, /* TimeWindowMinimum */ 100000, /* TimeWindowMaximum */ 100 /* StepSize */ }, Package () { /* Power Limit 2 */ 1, /* PowerLimitIndex, 1 for Power Limit 2 */ - 15000, /* PowerLimitMinimum */ + 12000, /* PowerLimitMinimum */ 51000, /* PowerLimitMaximum */ 280000, /* TimeWindowMinimum */ 280000, /* TimeWindowMaximum */
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38658 )
Change subject: mb/google/drallion/variants/drallion: Update thermal configuration for DPTF ......................................................................
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/655 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/654 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/653
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
EricR Lai (Code Review)
-
John Su (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Sumeet R Pawnikar (Code Review)