Change in coreboot[master]: mb/google/volteer: Convert static ASL files to new DPTF implementation
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 8 files changed, 93 insertions(+), 151 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/1
diff --git a/src/mainboard/google/volteer/Kconfig b/src/mainboard/google/volteer/Kconfig index 63e70c5..7bb6d64 100644 --- a/src/mainboard/google/volteer/Kconfig +++ b/src/mainboard/google/volteer/Kconfig @@ -5,6 +5,7 @@ select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_I2C_SX9310 + select DRIVERS_INTEL_DPTF select DRIVERS_INTEL_PMC select DRIVERS_INTEL_SOUNDWIRE select DRIVERS_SPI_ACPI diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index c6a2e8b..a02d30f 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -224,7 +224,98 @@ #From EDS(575683) device pci 00.0 on end # Host Bridge 0x9A14:U/0x9A12:Y device pci 02.0 on end # Graphics - device pci 04.0 on end # DPTF 0x9A03 + device pci 04.0 on + # Default DPTF Policy for all Volteer boards if not overridden + chip drivers/intel/dptf + register "policies.enabled.passive" = "1" + register "policies.enabled.critical" = "1" + register "policies.enabled.active" = "1" + + ## Active Policy + register "policies.active[0]" = "{.target=DPTF_CPU, .thresholds={ + {.temp=85, .fan_pct=90}, + {.temp=80, .fan_pct=69}, + {.temp=75, .fan_pct=56}, + {.temp=70, .fan_pct=46}, + {.temp=65, .fan_pct=36}}}" + register "policies.active[1]" = "{.target=DPTF_TSR0, .thresholds={ + {.temp=50, .fan_pct=90}, + {.temp=47, .fan_pct=69}, + {.temp=45, .fan_pct=56}, + {.temp=42, .fan_pct=46}, + {.temp=39, .fan_pct=36}}}" + register "policies.active[2]" = "{.target=DPTF_TSR1, .thresholds={ + {.temp=50, .fan_pct=90}, + {.temp=47, .fan_pct=69}, + {.temp=45, .fan_pct=56}, + {.temp=42, .fan_pct=46}, + {.temp=39, .fan_pct=36}}}" + register "policies.active[3]" = "{.target=DPTF_TSR2, .thresholds={ + {.temp=50, .fan_pct=90}, + {.temp=47, .fan_pct=69}, + {.temp=45, .fan_pct=56}, + {.temp=42, .fan_pct=46}, + {.temp=39, .fan_pct=36}}}" + + ## Passive Policy + register "policies.passive[0]" = "{.source=DPTF_CPU, .target=DPTF_CPU, .temp=95, .period=5000}" + register "policies.passive[1]" = "{.source=DPTF_CPU, .target=DPTF_TSR0, .temp=65, .period=6000}" + register "policies.passive[2]" = "{.source=DPTF_CHARGER, .target=DPTF_TSR1, .temp=65, .period=6000}" + register "policies.passive[3]" = "{.source=DPTF_CPU, .target=DPTF_TSR2, .temp=65, .period=6000}" + + ## Critical Policy + register "policies.critical[0]" = "{.source=DPTF_CPU, .temp=105, .type=DPTF_CRITICAL_SUSPEND}" + register "policies.critical[1]" = "{.source=DPTF_TSR0, .temp=75, .type=DPTF_CRITICAL_SUSPEND}" + register "policies.critical[2]" = "{.source=DPTF_TSR1, .temp=75, .type=DPTF_CRITICAL_SUSPEND}" + register "policies.critical[3]" = "{.source=DPTF_TSR2, .temp=75, .type=DPTF_CRITICAL_SUSPEND}" + + ## Power Limits Control + # 10-15W PL1 in 200mW increments, avg over 28s interval + # PL2 is fixed at 64W, avg over 28s interval + register "controls.power_limits.pl1" = "{ + .min_power = 3000, + .max_power = 15000, + .time_window_min = 28000, + .time_window_max = 32000, + .granularity = 200,}" + register "controls.power_limits.pl2" = "{ + .min_power = 15000, + .max_power = 60000, + .time_window_min = 28000, + .time_window_max = 32000, + .granularity = 1000,}" + + ## Charger Performance Control (Control, mA) + register "controls.charger_perf[0]" = "{ 255, 1700 }" + register "controls.charger_perf[1]" = "{ 24, 1500 }" + register "controls.charger_perf[2]" = "{ 16, 1000 }" + register "controls.charger_perf[3]" = "{ 8, 500 }" + + ## Fan Performance Control (Percent, Speed, Noise, Power) + register "controls.fan_perf[0]" = "{ 90, 6700, 220, 2200, }" + register "controls.fan_perf[1]" = "{ 80, 5800, 180, 1800, }" + register "controls.fan_perf[2]" = "{ 70, 5000, 145, 1450, }" + register "controls.fan_perf[3]" = "{ 60, 4900, 115, 1150, }" + register "controls.fan_perf[4]" = "{ 50, 3838, 90, 900, }" + register "controls.fan_perf[5]" = "{ 40, 2904, 55, 550, }" + register "controls.fan_perf[6]" = "{ 30, 2337, 30, 300, }" + register "controls.fan_perf[7]" = "{ 20, 1608, 15, 150, }" + register "controls.fan_perf[8]" = "{ 10, 800, 10, 100, }" + register "controls.fan_perf[9]" = "{ 0, 0, 0, 50, }" + + # Fan options + register "options.fan.fine_grained_control" = "1" + register "options.fan.step_size" = "2" + + # TSR hysteresis + register "options.tsr[0].hysteresis" = "2" + register "options.tsr[1].hysteresis" = "2" + register "options.tsr[2].hysteresis" = "2" + register "options.tsr[3].hysteresis" = "2" + + device generic 0 on end + end + end # DPTF 0x9A03 device pci 05.0 off end # IPU 0x9A19 device pci 06.0 off end # PEG60 0x9A09 device pci 07.0 on end # TBT_PCIe0 0x9A23 diff --git a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl b/src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl deleted file mode 100644 index cef895b..0000000 --- a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl +++ /dev/null @@ -1,131 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#define DPTF_CPU_PASSIVE 95 -#define DPTF_CPU_CRITICAL 105 -#define DPTF_CPU_ACTIVE_AC0 85 -#define DPTF_CPU_ACTIVE_AC1 80 -#define DPTF_CPU_ACTIVE_AC2 75 -#define DPTF_CPU_ACTIVE_AC3 70 -#define DPTF_CPU_ACTIVE_AC4 65 - -#define DPTF_TSR0_SENSOR_ID 0 -#define DPTF_TSR0_SENSOR_NAME "Thermal Sensor 1" -#define DPTF_TSR0_PASSIVE 65 -#define DPTF_TSR0_CRITICAL 75 -#define DPTF_TSR0_ACTIVE_AC0 50 -#define DPTF_TSR0_ACTIVE_AC1 47 -#define DPTF_TSR0_ACTIVE_AC2 45 -#define DPTF_TSR0_ACTIVE_AC3 42 -#define DPTF_TSR0_ACTIVE_AC4 39 - -#define DPTF_TSR1_SENSOR_ID 1 -#define DPTF_TSR1_SENSOR_NAME "Thermal Sensor 2" -#define DPTF_TSR1_PASSIVE 65 -#define DPTF_TSR1_CRITICAL 75 -#define DPTF_TSR1_ACTIVE_AC0 50 -#define DPTF_TSR1_ACTIVE_AC1 47 -#define DPTF_TSR1_ACTIVE_AC2 45 -#define DPTF_TSR1_ACTIVE_AC3 42 -#define DPTF_TSR1_ACTIVE_AC4 39 - -#define DPTF_TSR2_SENSOR_ID 1 -#define DPTF_TSR2_SENSOR_NAME "Thermal Sensor 3" -#define DPTF_TSR2_PASSIVE 65 -#define DPTF_TSR2_CRITICAL 75 -#define DPTF_TSR2_ACTIVE_AC0 50 -#define DPTF_TSR2_ACTIVE_AC1 47 -#define DPTF_TSR2_ACTIVE_AC2 45 -#define DPTF_TSR2_ACTIVE_AC3 42 -#define DPTF_TSR2_ACTIVE_AC4 39 - -#define DPTF_ENABLE_CHARGER -#define DPTF_ENABLE_FAN_CONTROL - -/* Charger performance states, board-specific values from charger and EC */ -Name (CHPS, Package () { - Package () { 0, 0, 0, 0, 255, 0x6a4, "mA", 0 }, /* 1.7A (MAX) */ - Package () { 0, 0, 0, 0, 24, 0x600, "mA", 0 }, /* 1.5A */ - Package () { 0, 0, 0, 0, 16, 0x400, "mA", 0 }, /* 1.0A */ - Package () { 0, 0, 0, 0, 8, 0x200, "mA", 0 }, /* 0.5A */ -}) - -/* DFPS: Fan Performance States */ -Name (DFPS, Package () { - 0, // Revision - /* - * TODO : Need to update this Table after characterization. - * These are initial reference values. - */ - /* Control, Trip Point, Speed, NoiseLevel, Power */ - Package () {90, 0xFFFFFFFF, 6700, 220, 2200}, - Package () {80, 0xFFFFFFFF, 5800, 180, 1800}, - Package () {70, 0xFFFFFFFF, 5000, 145, 1450}, - Package () {60, 0xFFFFFFFF, 4900, 115, 1150}, - Package () {50, 0xFFFFFFFF, 3838, 90, 900}, - Package () {40, 0xFFFFFFFF, 2904, 55, 550}, - Package () {30, 0xFFFFFFFF, 2337, 30, 300}, - Package () {20, 0xFFFFFFFF, 1608, 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, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - }, - Package () { - _SB.DPTF.TFN1, _SB.DPTF.TSR0, 100, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - }, - Package () { - _SB.DPTF.TFN1, _SB.DPTF.TSR1, 100, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - }, - Package () { - _SB.DPTF.TFN1, _SB.DPTF.TSR2, 100, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - } -}) - - -Name (DTRT, Package () { - /* CPU Throttle Effect on CPU */ - Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 100, 50, 0, 0, 0, 0 }, - - /* CPU Throttle Effect on TSR0 sensor */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR0, 100, 60, 0, 0, 0, 0 }, - - /* Charger Throttle Effect on Charger (TSR1) */ - Package () { _SB.DPTF.TCHG, _SB.DPTF.TSR1, 100, 60, 0, 0, 0, 0 }, - - /* CPU Throttle Effect on TSR2 sensor */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 100, 60, 0, 0, 0, 0 }, -}) - -Name (MPPC, Package () -{ - 0x2, /* Revision */ - Package () { /* Power Limit 1 */ - 0, /* PowerLimitIndex, 0 for Power Limit 1 */ - 3000, /* PowerLimitMinimum */ - 15000, /* PowerLimitMaximum */ - 28000, /* TimeWindowMinimum */ - 32000, /* TimeWindowMaximum */ - 200 /* StepSize */ - }, - Package () { /* Power Limit 2 */ - 1, /* PowerLimitIndex, 1 for Power Limit 2 */ - 15000, /* PowerLimitMinimum */ - 60000, /* PowerLimitMaximum */ - 28000, /* TimeWindowMinimum */ - 32000, /* TimeWindowMaximum */ - 1000 /* StepSize */ - } -}) diff --git a/src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl deleted file mode 100644 index 189cafe..0000000 --- a/src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,3 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <baseboard/acpi/dptf.asl> diff --git a/src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl deleted file mode 100644 index 189cafe..0000000 --- a/src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,3 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <baseboard/acpi/dptf.asl> diff --git a/src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl deleted file mode 100644 index 189cafe..0000000 --- a/src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,3 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <baseboard/acpi/dptf.asl> diff --git a/src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl deleted file mode 100644 index 189cafe..0000000 --- a/src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,3 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <baseboard/acpi/dptf.asl> diff --git a/src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl deleted file mode 100644 index 726e381..0000000 --- a/src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,7 +0,0 @@ -/* - * - * - * SPDX-License-Identifier: GPL-2.0-or-later - */ - -#include <baseboard/acpi/dptf.asl>
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#3).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 9 files changed, 93 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/3
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#4).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
TEST=Manually verified (using decompiled ASL) that both DSDT & SSDT contain effectively the same content as before, although with slight organizational differences.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 9 files changed, 93 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/4
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#6).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 9 files changed, 93 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/6
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 6:
(5 comments)
I kinda went out of order in the patch stack because I wanted to focus on the devicetree view first.
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 230: register "policies.enabled.passive" = "1" : register "policies.enabled.critical" = "1" : register "policies.enabled.active" = "1" could these be auto-detected by non-zero values in the different policy structures?
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 261: .source=DPTF_CPU, .target=DPTF_CPU, .temp=95, .period=5000}" you could define a macro for generating these:
DPTF_PASSIVE(CPU, CPU, 95, 5000) //or something similar...
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 284: 28000 might be able to use "28 * MSECS_PER_SEC" in here
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 294: Fan Performance Control (Percent, Speed, Noise, Power) Not sure if you want to go there but it seems like this table could be calculated with some input values like number of steps and some fan characteristics. It looks like most boards use the same table for noise/power even if they change the speed values which makes me wonder if/how DPTF actually uses those values.
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 311: 2 this seems to be hardcoded in the ASL, maybe it should just default to '2' and allow overriding?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 230: register "policies.enabled.passive" = "1" : register "policies.enabled.critical" = "1" : register "policies.enabled.active" = "1"
could these be auto-detected by non-zero values in the different policy structures?
Good idea.
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 261: .source=DPTF_CPU, .target=DPTF_CPU, .temp=95, .period=5000}"
you could define a macro for generating these: […]
Oooh, now that's thinking with your noggin, that's a great idea!
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 294: Fan Performance Control (Percent, Speed, Noise, Power)
Not sure if you want to go there but it seems like this table could be calculated with some input va […]
For this table, RPM is nearly perfectly linear with %, although noise and power are not quite as linear...
My guess is that, because we have fine-grained control on, it probably linearly interpolates between each setpoint.
Also I would guess that power could be used if you use one of the more advanced policies, but I'm guessing noise level & power are ignored in our implementation.
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 311: 2
this seems to be hardcoded in the ASL, maybe it should just default to '2' and allow overriding?
This is supposed to be the hysteresis inherent in the system, so either analog hysteresis on a thermocouple or digital hysteresis in the firmware. Also the entire DPTF implementation was "customized" for ChromeOS. What if I shove generating these in the vendorcode CL instead and leave them out here?
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#7).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/intel/dptf/chip.h M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 10 files changed, 90 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/7
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 284: 28000
might be able to use "28 * MSECS_PER_SEC" in here
Ack
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 7:
(15 comments)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... File src/drivers/intel/dptf/chip.h:
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 9: #define DPTF_PASSIVE(src,tgt,tmp,prd) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 9: #define DPTF_PASSIVE(src,tgt,tmp,prd) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 9: #define DPTF_PASSIVE(src,tgt,tmp,prd) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 10: {.source=DPTF_##src, .target=DPTF_##tgt, .temp=tmp, .period=prd} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 10: {.source=DPTF_##src, .target=DPTF_##tgt, .temp=tmp, .period=prd} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 10: {.source=DPTF_##src, .target=DPTF_##tgt, .temp=tmp, .period=prd} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 10: {.source=DPTF_##src, .target=DPTF_##tgt, .temp=tmp, .period=prd} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 11: #define DPTF_CRITICAL(src,tmp,typ) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 11: #define DPTF_CRITICAL(src,tmp,typ) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 12: {.source=DPTF_##src, .temp=tmp, .type=DPTF_CRITICAL_##typ} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 12: {.source=DPTF_##src, .temp=tmp, .type=DPTF_CRITICAL_##typ} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 12: {.source=DPTF_##src, .temp=tmp, .type=DPTF_CRITICAL_##typ} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 13: #define TEMP_PCT(t,r) {.temp=t, .fan_pct=r} space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 13: #define TEMP_PCT(t,r) {.temp=t, .fan_pct=r} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/7/src/drivers/intel/dptf/chip... PS7, Line 13: #define TEMP_PCT(t,r) {.temp=t, .fan_pct=r} spaces required around that '=' (ctx:VxV)
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#8).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
TEST=Dumped SSDT on Volteer and confirmed that the AML/ASL is as expected and is similar on all expected ways to the previous static ASL.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/intel/dptf/chip.h M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 10 files changed, 90 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/8
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#9).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/intel/dptf/chip.h M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 10 files changed, 90 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 9:
(15 comments)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... File src/drivers/intel/dptf/chip.h:
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 9: #define DPTF_PASSIVE(src,tgt,tmp,prd) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 9: #define DPTF_PASSIVE(src,tgt,tmp,prd) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 9: #define DPTF_PASSIVE(src,tgt,tmp,prd) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 10: {.source=DPTF_##src, .target=DPTF_##tgt, .temp=tmp, .period=prd} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 10: {.source=DPTF_##src, .target=DPTF_##tgt, .temp=tmp, .period=prd} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 10: {.source=DPTF_##src, .target=DPTF_##tgt, .temp=tmp, .period=prd} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 10: {.source=DPTF_##src, .target=DPTF_##tgt, .temp=tmp, .period=prd} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 11: #define DPTF_CRITICAL(src,tmp,typ) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 11: #define DPTF_CRITICAL(src,tmp,typ) \ space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 12: {.source=DPTF_##src, .temp=tmp, .type=DPTF_CRITICAL_##typ} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 12: {.source=DPTF_##src, .temp=tmp, .type=DPTF_CRITICAL_##typ} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 12: {.source=DPTF_##src, .temp=tmp, .type=DPTF_CRITICAL_##typ} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 13: #define TEMP_PCT(t,r) {.temp=t, .fan_pct=r} space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 13: #define TEMP_PCT(t,r) {.temp=t, .fan_pct=r} spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/41895/9/src/drivers/intel/dptf/chip... PS9, Line 13: #define TEMP_PCT(t,r) {.temp=t, .fan_pct=r} spaces required around that '=' (ctx:VxV)
Hello build bot (Jenkins), Sumeet R Pawnikar, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#10).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/intel/dptf/chip.h M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 10 files changed, 90 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/10
Hello build bot (Jenkins), Furquan Shaikh, Sumeet R Pawnikar, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#12).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/intel/dptf/chip.h M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 10 files changed, 90 insertions(+), 179 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/12
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41895/12/src/drivers/intel/dptf/chi... File src/drivers/intel/dptf/chip.h:
https://review.coreboot.org/c/coreboot/+/41895/12/src/drivers/intel/dptf/chi... PS12, Line 7: MSECS_PER_SEC This chip.h header file doesn't appear to use MSECS_PER_SEC, but I do see that devicetree.cb now does. Does including this here make it available to the devicetree.cb parser somehow, or is this perhaps a missed left-over from a previous revision of your change?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41895/12/src/drivers/intel/dptf/chi... File src/drivers/intel/dptf/chip.h:
https://review.coreboot.org/c/coreboot/+/41895/12/src/drivers/intel/dptf/chi... PS12, Line 7: MSECS_PER_SEC
This chip.h header file doesn't appear to use MSECS_PER_SEC, but I do see that devicetree. […]
Yes, that is included solely devicetree.cb has access to MSECS_PER_SEC, because chip.h is automatically included for the struct drivers_intel_dptf_config definition.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 13: Code-Review+1
LGTM, but I'm not yet well-versed enough in ASL to give this a +2.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 14:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte... PS14, Line 282: ## Active Policy We need to add Weight parameter as well, which decides the priority between two sensors when they hit the threshold at the same time to avoid any kind of race conditions. This Weight works similar like priority between two sensors, reference as https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte...
https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte... PS14, Line 308: Passive Policy same as above active policy comment, we need Weight here as well to avoid any kind of race conditions between sensors. Previous asl code for reference, https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte...
https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte... PS14, Line 314: ## Critical Policy In Critical policy, once the sensor hits the temperature threshold, DPTF initiates the graceful system shutdown. I am not sure on SUSPEND srting below. Please, share details.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 14:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte... PS14, Line 282: ## Active Policy
We need to add Weight parameter as well, which decides the priority between two sensors when they hi […]
Weight is written out, if it is not specified in the devicetree, then it defaults to 100. See the Active Policy patchset (the line looks like:
acpigen_write_integer(DEFAULT_IF_0(policies[i].weight, DEFAULT_WEIGHT));
IOW, if the user doesn't specify a weight (i.e., it's 0), then it will write out 100 as the default instead.
If it's required, then it can be added here:
policies.active[0] = "{.target=DPTF_CPU, .weight=90, .... }"
https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte... PS14, Line 308: Passive Policy
same as above active policy comment, we need Weight here as well to avoid any kind of race condition […]
The weight parameter can be written out as well, it defaults to 100, same as for Active Policies, see the Passive Policy patchset (although it's called Priority there):
acpigen_write_integer(DEFAULT_IF_0(policies[i].priority, DEFAULT_PRIORITY));
Similar to above, if no priority is specified, it defaults to writing out 100, otherwise it can be added. I could add macros though for the cases when that is desired to be specified: DPTF_PASSIVE_PRIORITY(CPU, CPU, 95, 5000, 90) for example.
https://review.coreboot.org/c/coreboot/+/41895/14/src/mainboard/google/volte... PS14, Line 314: ## Critical Policy
In Critical policy, once the sensor hits the temperature threshold, DPTF initiates the graceful syst […]
Good catch, Sumeet I think I had it backwards. _CRT "Returns critical trip point in tenths of degrees where DPPM must perform a critical shutdown." _HOT defines a threshold for transition to S4. Will revise the naming.
Hello build bot (Jenkins), Furquan Shaikh, Sumeet R Pawnikar, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41895
to look at the new patch set (#15).
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/intel/dptf/chip.h M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/ripto/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 10 files changed, 90 insertions(+), 179 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/41895/15
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 16:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 261: .source=DPTF_CPU, .target=DPTF_CPU, .temp=95, .period=5000}"
Oooh, now that's thinking with your noggin, that's a great idea!
Done
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 294: Fan Performance Control (Percent, Speed, Noise, Power)
For this table, RPM is nearly perfectly linear with %, although noise and power are not quite as lin […]
Done
https://review.coreboot.org/c/coreboot/+/41895/5/src/mainboard/google/voltee... PS5, Line 311: 2
This is supposed to be the hysteresis inherent in the system, so either analog hysteresis on a therm […]
Done
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 17: Code-Review+2
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 17: Code-Review+2
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
Patch Set 17: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41895 )
Change subject: mb/google/volteer: Convert static ASL files to new DPTF implementation ......................................................................
mb/google/volteer: Convert static ASL files to new DPTF implementation
This patch converts the current DPTF policies from static ASL files into the new SSDT-based DPTF implementation. All settings are intended to be copied exactly.
Change-Id: I964c53afbd503d47a07b982672425f0e7a986a3f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/41895 Reviewed-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Reviewed-by: Nick Vaccaro nvaccaro@google.com Reviewed-by: Duncan Laurie dlaurie@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/dptf/chip.h M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/dsdt.asl M src/mainboard/google/volteer/variants/baseboard/devicetree.cb D src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl D src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl D src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl 9 files changed, 90 insertions(+), 176 deletions(-)
Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved Sumeet R Pawnikar: Looks good to me, approved Nick Vaccaro: Looks good to me, approved
diff --git a/src/drivers/intel/dptf/chip.h b/src/drivers/intel/dptf/chip.h index 28403a8..db4c3ae 100644 --- a/src/drivers/intel/dptf/chip.h +++ b/src/drivers/intel/dptf/chip.h @@ -4,6 +4,13 @@ #define _DRIVERS_INTEL_DPTF_CHIP_H_
#include <acpi/acpigen_dptf.h> +#include <timer.h> /* for MSECS_PER_SEC */ + +#define DPTF_PASSIVE(src, tgt, tmp, prd) \ + {.source = DPTF_##src, .target = DPTF_##tgt, .temp = (tmp), .period = (prd)} +#define DPTF_CRITICAL(src, tmp, typ) \ + {.source = DPTF_##src, .temp = (tmp), .type = DPTF_CRITICAL_##typ} +#define TEMP_PCT(t, p) {.temp = (t), .fan_pct = (p)}
struct drivers_intel_dptf_config { struct { diff --git a/src/mainboard/google/volteer/Kconfig b/src/mainboard/google/volteer/Kconfig index 5c6f13c..16834ae 100644 --- a/src/mainboard/google/volteer/Kconfig +++ b/src/mainboard/google/volteer/Kconfig @@ -5,6 +5,7 @@ select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_I2C_SX9310 + select DRIVERS_INTEL_DPTF select DRIVERS_INTEL_PMC select DRIVERS_I2C_MAX98373 select DRIVERS_INTEL_SOUNDWIRE diff --git a/src/mainboard/google/volteer/dsdt.asl b/src/mainboard/google/volteer/dsdt.asl index 31897674..ddbc10f 100644 --- a/src/mainboard/google/volteer/dsdt.asl +++ b/src/mainboard/google/volteer/dsdt.asl @@ -52,17 +52,6 @@ #include <ec/google/chromeec/acpi/ec.asl> }
- /* Dynamic Platform Thermal Framework */ - Scope (_SB) - { - /* Per board variant specific definitions. */ - #include <variant/acpi/dptf.asl> - /* Include Tiger Lake soc specific DPTF changes */ - #include <soc/intel/tigerlake/acpi/dptf.asl> - /* Include common dptf ASL files */ - #include <soc/intel/common/acpi/dptf/dptf.asl> - } - #include <southbridge/intel/common/acpi/sleepstates.asl>
#if CONFIG(VARIANT_HAS_MIPI_CAMERA) diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 88aff01..322389d 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -282,7 +282,88 @@ #From EDS(575683) device pci 00.0 on end # Host Bridge 0x9A14:U/0x9A12:Y device pci 02.0 on end # Graphics - device pci 04.0 on end # DPTF 0x9A03 + device pci 04.0 on + # Default DPTF Policy for all Volteer boards if not overridden + chip drivers/intel/dptf + ## Active Policy + register "policies.active[0]" = "{.target=DPTF_CPU, + .thresholds={TEMP_PCT(85, 90), + TEMP_PCT(80, 69), + TEMP_PCT(75, 56), + TEMP_PCT(70, 46), + TEMP_PCT(65, 36),}}" + register "policies.active[1]" = "{.target=DPTF_TEMP_SENSOR_0, + .thresholds={TEMP_PCT(50, 90), + TEMP_PCT(47, 69), + TEMP_PCT(45, 56), + TEMP_PCT(42, 46), + TEMP_PCT(39, 36),}}" + register "policies.active[2]" = "{.target=DPTF_TEMP_SENSOR_1, + .thresholds={TEMP_PCT(50, 90), + TEMP_PCT(47, 69), + TEMP_PCT(45, 56), + TEMP_PCT(42, 46), + TEMP_PCT(39, 36),}}" + register "policies.active[3]" = "{.target=DPTF_TEMP_SENSOR_2, + .thresholds={TEMP_PCT(50, 90), + TEMP_PCT(47, 69), + TEMP_PCT(45, 56), + TEMP_PCT(42, 46), + TEMP_PCT(39, 36),}}" + + ## Passive Policy + register "policies.passive[0]" = "DPTF_PASSIVE(CPU, CPU, 95, 5000)" + register "policies.passive[1]" = "DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 65, 6000)" + register "policies.passive[2]" = "DPTF_PASSIVE(CHARGER, TEMP_SENSOR_1, 65, 6000)" + register "policies.passive[3]" = "DPTF_PASSIVE(CPU, TEMP_SENSOR_2, 65, 6000)" + + ## Critical Policy + register "policies.critical[0]" = "DPTF_CRITICAL(CPU, 105, SHUTDOWN)" + register "policies.critical[1]" = "DPTF_CRITICAL(TEMP_SENSOR_0, 75, SHUTDOWN)" + register "policies.critical[2]" = "DPTF_CRITICAL(TEMP_SENSOR_1, 75, SHUTDOWN)" + register "policies.critical[3]" = "DPTF_CRITICAL(TEMP_SENSOR_2, 75, SHUTDOWN)" + + ## Power Limits Control + # 10-15W PL1 in 200mW increments, avg over 28-32s interval + # PL2 is fixed at 64W, avg over 28-32s interval + register "controls.power_limits.pl1" = "{ + .min_power = 3000, + .max_power = 15000, + .time_window_min = 28 * MSECS_PER_SEC, + .time_window_max = 32 * MSECS_PER_SEC, + .granularity = 200,}" + register "controls.power_limits.pl2" = "{ + .min_power = 15000, + .max_power = 60000, + .time_window_min = 28 * MSECS_PER_SEC, + .time_window_max = 32 * MSECS_PER_SEC, + .granularity = 1000,}" + + ## Charger Performance Control (Control, mA) + register "controls.charger_perf[0]" = "{ 255, 1700 }" + register "controls.charger_perf[1]" = "{ 24, 1500 }" + register "controls.charger_perf[2]" = "{ 16, 1000 }" + register "controls.charger_perf[3]" = "{ 8, 500 }" + + ## Fan Performance Control (Percent, Speed, Noise, Power) + register "controls.fan_perf[0]" = "{ 90, 6700, 220, 2200, }" + register "controls.fan_perf[1]" = "{ 80, 5800, 180, 1800, }" + register "controls.fan_perf[2]" = "{ 70, 5000, 145, 1450, }" + register "controls.fan_perf[3]" = "{ 60, 4900, 115, 1150, }" + register "controls.fan_perf[4]" = "{ 50, 3838, 90, 900, }" + register "controls.fan_perf[5]" = "{ 40, 2904, 55, 550, }" + register "controls.fan_perf[6]" = "{ 30, 2337, 30, 300, }" + register "controls.fan_perf[7]" = "{ 20, 1608, 15, 150, }" + register "controls.fan_perf[8]" = "{ 10, 800, 10, 100, }" + register "controls.fan_perf[9]" = "{ 0, 0, 0, 50, }" + + # Fan options + register "options.fan.fine_grained_control" = "1" + register "options.fan.step_size" = "2" + + device generic 0 on end + end + end # DPTF 0x9A03 device pci 05.0 off end # IPU 0x9A19 device pci 06.0 off end # PEG60 0x9A09 device pci 07.0 on end # TBT_PCIe0 0x9A23 diff --git a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl b/src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl deleted file mode 100644 index ddf8814..0000000 --- a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/acpi/dptf.asl +++ /dev/null @@ -1,148 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#define DPTF_CPU_PASSIVE 95 -#define DPTF_CPU_CRITICAL 105 -#define DPTF_CPU_ACTIVE_AC0 85 -#define DPTF_CPU_ACTIVE_AC1 80 -#define DPTF_CPU_ACTIVE_AC2 75 -#define DPTF_CPU_ACTIVE_AC3 70 -#define DPTF_CPU_ACTIVE_AC4 65 - -#define DPTF_TSR0_SENSOR_ID 0 -#define DPTF_TSR0_SENSOR_NAME "Thermal Sensor 1" -#define DPTF_TSR0_PASSIVE 65 -#define DPTF_TSR0_CRITICAL 75 -#define DPTF_TSR0_ACTIVE_AC0 50 -#define DPTF_TSR0_ACTIVE_AC1 47 -#define DPTF_TSR0_ACTIVE_AC2 45 -#define DPTF_TSR0_ACTIVE_AC3 42 -#define DPTF_TSR0_ACTIVE_AC4 39 - -#define DPTF_TSR1_SENSOR_ID 1 -#define DPTF_TSR1_SENSOR_NAME "Thermal Sensor 2" -#define DPTF_TSR1_PASSIVE 65 -#define DPTF_TSR1_CRITICAL 75 -#define DPTF_TSR1_ACTIVE_AC0 50 -#define DPTF_TSR1_ACTIVE_AC1 47 -#define DPTF_TSR1_ACTIVE_AC2 45 -#define DPTF_TSR1_ACTIVE_AC3 42 -#define DPTF_TSR1_ACTIVE_AC4 39 - -#define DPTF_TSR2_SENSOR_ID 2 -#define DPTF_TSR2_SENSOR_NAME "Thermal Sensor 3" -#define DPTF_TSR2_PASSIVE 65 -#define DPTF_TSR2_CRITICAL 75 -#define DPTF_TSR2_ACTIVE_AC0 50 -#define DPTF_TSR2_ACTIVE_AC1 47 -#define DPTF_TSR2_ACTIVE_AC2 45 -#define DPTF_TSR2_ACTIVE_AC3 42 -#define DPTF_TSR2_ACTIVE_AC4 39 - -#define DPTF_TSR3_SENSOR_ID 3 -#define DPTF_TSR3_SENSOR_NAME "Thermal Sensor 4" -#define DPTF_TSR3_PASSIVE 65 -#define DPTF_TSR3_CRITICAL 75 -#define DPTF_TSR3_ACTIVE_AC0 50 -#define DPTF_TSR3_ACTIVE_AC1 47 -#define DPTF_TSR3_ACTIVE_AC2 45 -#define DPTF_TSR3_ACTIVE_AC3 42 -#define DPTF_TSR3_ACTIVE_AC4 39 - -#define DPTF_ENABLE_CHARGER -#define DPTF_ENABLE_FAN_CONTROL - -/* Charger performance states, board-specific values from charger and EC */ -Name (CHPS, Package () { - Package () { 0, 0, 0, 0, 255, 0x6a4, "mA", 0 }, /* 1.7A (MAX) */ - Package () { 0, 0, 0, 0, 24, 0x600, "mA", 0 }, /* 1.5A */ - Package () { 0, 0, 0, 0, 16, 0x400, "mA", 0 }, /* 1.0A */ - Package () { 0, 0, 0, 0, 8, 0x200, "mA", 0 }, /* 0.5A */ -}) - -/* DFPS: Fan Performance States */ -Name (DFPS, Package () { - 0, // Revision - /* - * TODO : Need to update this Table after characterization. - * These are initial reference values. - */ - /* Control, Trip Point, Speed, NoiseLevel, Power */ - Package () {90, 0xFFFFFFFF, 6700, 220, 2200}, - Package () {80, 0xFFFFFFFF, 5800, 180, 1800}, - Package () {70, 0xFFFFFFFF, 5000, 145, 1450}, - Package () {60, 0xFFFFFFFF, 4900, 115, 1150}, - Package () {50, 0xFFFFFFFF, 3838, 90, 900}, - Package () {40, 0xFFFFFFFF, 2904, 55, 550}, - Package () {30, 0xFFFFFFFF, 2337, 30, 300}, - Package () {20, 0xFFFFFFFF, 1608, 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, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - }, - Package () { - _SB.DPTF.TFN1, _SB.DPTF.TSR0, 100, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - }, - Package () { - _SB.DPTF.TFN1, _SB.DPTF.TSR1, 100, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - }, - Package () { - _SB.DPTF.TFN1, _SB.DPTF.TSR2, 100, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - }, - Package () { - _SB.DPTF.TFN1, _SB.DPTF.TSR3, 100, 90, 69, 56, 46, 36, 0, 0, - 0, 0, 0 - } -}) - - -Name (DTRT, Package () { - /* CPU Throttle Effect on CPU */ - Package () { _SB.PCI0.TCPU, _SB.PCI0.TCPU, 100, 50, 0, 0, 0, 0 }, - - /* CPU Throttle Effect on TSR1 sensor */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR1, 100, 60, 0, 0, 0, 0 }, - - /* Charger Throttle Effect on Charger (TSR0) */ - Package () { _SB.DPTF.TCHG, _SB.DPTF.TSR0, 100, 60, 0, 0, 0, 0 }, - - /* CPU Throttle Effect on TSR2 sensor */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR2, 100, 60, 0, 0, 0, 0 }, - - /* CPU Throttle Effect on TSR3 sensor */ - Package () { _SB.PCI0.TCPU, _SB.DPTF.TSR3, 100, 60, 0, 0, 0, 0 }, -}) - -Name (MPPC, Package () -{ - 0x2, /* Revision */ - Package () { /* Power Limit 1 */ - 0, /* PowerLimitIndex, 0 for Power Limit 1 */ - 3000, /* PowerLimitMinimum */ - 15000, /* PowerLimitMaximum */ - 28000, /* TimeWindowMinimum */ - 32000, /* TimeWindowMaximum */ - 200 /* StepSize */ - }, - Package () { /* Power Limit 2 */ - 1, /* PowerLimitIndex, 1 for Power Limit 2 */ - 15000, /* PowerLimitMinimum */ - 60000, /* PowerLimitMaximum */ - 28000, /* TimeWindowMinimum */ - 32000, /* TimeWindowMaximum */ - 1000 /* StepSize */ - } -}) diff --git a/src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl deleted file mode 100644 index 189cafe..0000000 --- a/src/mainboard/google/volteer/variants/halvor/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,3 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <baseboard/acpi/dptf.asl> diff --git a/src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl deleted file mode 100644 index 189cafe..0000000 --- a/src/mainboard/google/volteer/variants/malefor/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,3 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <baseboard/acpi/dptf.asl> diff --git a/src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl deleted file mode 100644 index 189cafe..0000000 --- a/src/mainboard/google/volteer/variants/trondo/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,3 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <baseboard/acpi/dptf.asl> diff --git a/src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl b/src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl deleted file mode 100644 index 726e381..0000000 --- a/src/mainboard/google/volteer/variants/volteer/include/variant/acpi/dptf.asl +++ /dev/null @@ -1,7 +0,0 @@ -/* - * - * - * SPDX-License-Identifier: GPL-2.0-or-later - */ - -#include <baseboard/acpi/dptf.asl>
-
build bot (Jenkins) (Code Review) -
Duncan Laurie (Code Review)
-
Nick Vaccaro (Code Review)
-
Patrick Georgi (Code Review)
-
Sumeet R Pawnikar (Code Review)
-
Tim Wawrzynczak (Code Review)