Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c M src/include/acpi/acpigen_dptf.h 4 files changed, 204 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/1
diff --git a/src/acpi/acpigen_dptf.c b/src/acpi/acpigen_dptf.c index d0827b6..dfe5337 100644 --- a/src/acpi/acpigen_dptf.c +++ b/src/acpi/acpigen_dptf.c @@ -13,6 +13,70 @@ #define DPTF_CRITICAL_POLICY_UUID "97C68AE7-15FA-499c-B8C9-5DA81D606E0A" #define DPTF_ACTIVE_POLICY_UUID "3A95C389-E4B8-4629-A526-C52C88626BAE"
+/* Defaults */ +enum { + ART_REVISION = 0, + DEFAULT_WEIGHT = 100, + DPTF_MAX_ART_THRESHOLDS = 10, +}; + +/* Convert degrees C to 1/10 degree Kelvin for ACPI */ +static int to_acpi_temp(int deg_c) +{ + return deg_c * 10 + 2732; +} + +/* Writes out a 0-argument non-Serialized Method that returns an Integer */ +static void write_simple_return_method(const char *name, int value) +{ + acpigen_write_method(name, 0); + acpigen_write_return_integer(value); + acpigen_pop_len(); /* Method */ +} + +/* Return the assigned namestring of any participant */ +static const char *namestring_of(enum dptf_participant participant) +{ + switch (participant) { + case DPTF_CPU: + return "TCPU"; + case DPTF_CHARGER: + return "TCHG"; + case DPTF_FAN: + return "TFN1"; + case DPTF_TSR0: + return "TSR0"; + case DPTF_TSR1: + return "TSR1"; + case DPTF_TSR2: + return "TSR2"; + case DPTF_TSR3: + return "TSR3"; + default: + return ""; + } +} + +/* Helper to get Scope for participants underneath _SB.DPTF */ +static const char *scope_of(enum dptf_participant participant) +{ + static char scope[16]; + + if (participant == DPTF_CPU) + snprintf(scope, sizeof(scope), "\_SB.%s", namestring_of(participant)); + else + snprintf(scope, sizeof(scope), TOPLEVEL_DPTF_SCOPE ".%s", + namestring_of(participant)); + + return scope; +} + +/* Write out scope of a participant */ +void dptf_write_scope(enum dptf_participant participant) +{ + acpigen_write_scope(scope_of(participant)); +} + /* * Almost all Intel boards use a Global NVS field named DPTE. * Writes out a _STA that checks if \DPTE is 1, and returns 0xF if true, 0x0 otherwise. @@ -68,3 +132,98 @@ acpigen_pop_len(); /* Package */ acpigen_pop_len(); /* Scope */ } + +/* + * This table describes active cooling relationships between the system's fan and the + * temperature sensors that it can have an effect on. As ever-increasing temperature thresholds + * are crossed (_AC9.._AC0, low to high), the corresponding fan percentages listed in this table + * are used to increase the speed of the fan in order to speed up cooling. + */ +static void write_active_relationship_table(const struct dptf_active_policy *policies, int max_count) +{ + char *pkg_count; + int i, j; + + /* Nothing to do */ + if (!max_count || policies[0].target == DPTF_NONE) + return; + + acpigen_write_scope(TOPLEVEL_DPTF_SCOPE); + acpigen_write_method("_ART", 0); + + /* Return this package */ + acpigen_emit_byte(RETURN_OP); + + /* Keep track of items added to the package */ + pkg_count = acpigen_write_package(1); /* The '1' here is for the revision */ + acpigen_write_integer(ART_REVISION); + + for (i = 0; i < max_count; ++i) + { + /* + * These have to be filled out from AC0 down to AC9, filling in only as many + * as are used. As soon as one isn't filled in, we're done. + */ + if (policies[i].target == DPTF_NONE) + break; + + (*pkg_count)++; + + /* Source, Target, Percent, Fan % for each of _AC0 ... _AC9 */ + acpigen_write_package(13); + acpigen_emit_namestring(namestring_of(DPTF_FAN)); + acpigen_emit_namestring(namestring_of(policies[i].target)); + acpigen_write_integer(DEFAULT_IF_0(policies[i].weight, DEFAULT_WEIGHT)); + + /* Write out fan %; corresponds with target's _ACx methods */ + for (j = 0; j < DPTF_MAX_ART_THRESHOLDS; ++j) + acpigen_write_integer(policies[i].thresholds[j].fan_pct); + + acpigen_pop_len(); /* inner Package */ + } + + acpigen_pop_len(); /* outer Package */ + acpigen_pop_len(); /* Method _ART */ + acpigen_pop_len(); /* Scope */ +} + +/* + * _AC9 through _AC0 represent temperature thresholds, in increasing order, defined from _AC0 + * down, that, when reached, DPTF will activate TFN1 in order to actively cool the temperature + * sensor(s). As increasing thresholds are reached, the fan is spun faster. + */ +static void write_active_cooling_methods(const struct dptf_active_policy *policies, + int max_count) +{ + char name[5]; + int i, j; + + /* Nothing to do */ + if (!max_count || policies[0].target == DPTF_NONE) + return; + + for (i = 0; i < max_count; ++i) { + if (policies[i].target == DPTF_NONE) + break; + + dptf_write_scope(policies[i].target); + + /* Write out as many of _AC0 through _AC9 that are applicable */ + for (j = 0; j < DPTF_MAX_ACX; ++j) { + if (!policies[i].thresholds[j].temp) + break; + + snprintf(name, sizeof(name), "_AC%1X", j); + write_simple_return_method(name, to_acpi_temp( + policies[i].thresholds[j].temp)); + } + + acpigen_pop_len(); /* Scope */ + } +} + +void dptf_write_active_policies(const struct dptf_active_policy *policies, int max_count) +{ + write_active_relationship_table(policies, max_count); + write_active_cooling_methods(policies, max_count); +} diff --git a/src/drivers/intel/dptf/chip.h b/src/drivers/intel/dptf/chip.h index 9ab7963..bc4235e 100644 --- a/src/drivers/intel/dptf/chip.h +++ b/src/drivers/intel/dptf/chip.h @@ -9,6 +9,7 @@ struct drivers_intel_dptf_config { struct { struct dptf_enabled_policies enabled; + struct dptf_active_policy active[DPTF_MAX_ACTIVE_POLICIES]; } policies; };
diff --git a/src/drivers/intel/dptf/dptf.c b/src/drivers/intel/dptf/dptf.c index e3e4efd..a3764fa 100644 --- a/src/drivers/intel/dptf/dptf.c +++ b/src/drivers/intel/dptf/dptf.c @@ -23,6 +23,17 @@ static bool is_participant_used(const struct drivers_intel_dptf_config *config, enum dptf_participant participant) { + int i; + + /* Active? */ + for (i = 0; i < DPTF_MAX_ACTIVE_POLICIES; ++i) + if (config->policies.active[i].target == participant) + return true; + + /* Check fan as well (it's use is implicit in the Active policy) */ + if (participant == DPTF_FAN && config->policies.enabled.active) + return true; + return false; }
@@ -39,6 +50,10 @@ /* Policies */ dptf_write_enabled_policies(&config->policies.enabled);
+ if (config->policies.enabled.active) + dptf_write_active_policies(config->policies.active, + DPTF_MAX_ACTIVE_POLICIES); + printk(BIOS_INFO, "\_SB.DPTF: %s at %s\n", dev->chip_ops->name, dev_path(dev)); }
diff --git a/src/include/acpi/acpigen_dptf.h b/src/include/acpi/acpigen_dptf.h index f717296..24dc279 100644 --- a/src/include/acpi/acpigen_dptf.h +++ b/src/include/acpi/acpigen_dptf.h @@ -22,6 +22,13 @@ DPTF_PARTICIPANT_COUNT, };
+/* DPTF compile-time constants */ +enum { + /* A device can only define _AC0 .. _AC9 (i.e., 10) Active Cooling Methods */ + DPTF_MAX_ACX = 10, + DPTF_MAX_ACTIVE_POLICIES = (DPTF_PARTICIPANT_COUNT-1), +}; + /* Which policies are enabled? */ struct dptf_enabled_policies { bool passive; @@ -29,12 +36,34 @@ bool active; };
+/* Active Policy */ +struct dptf_active_policy { + /* Device capable of being affected by the fan */ + enum dptf_participant target; + /* */ + uint8_t weight; + /* When target reaches temperature 'temp', the source will turn on at 'fan_pct' % */ + struct { + /* (degrees C) */ + uint8_t temp; + /* 0 - 100 */ + uint8_t fan_pct; + } thresholds [DPTF_MAX_ACX]; +}; + /* * This function writes out _SB.DPTF.IDSP, which describes the different DPTF policies that * this implementation is using. */ void dptf_write_enabled_policies(const struct dptf_enabled_policies *policies);
+/* + * This function provides tables of temperature and corresponding fan or percent. When the + * temperature thresholds are met (_AC0 - _AC9), the fan is driven to corresponding percentage + * of full speed. + */ +void dptf_write_active_policies(const struct dptf_active_policy *policies, int max_count); + /* Helper method to open the scope for a given participant. */ void dptf_write_scope(enum dptf_participant participant);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41885/1/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/1/src/acpi/acpigen_dptf.c@142 PS1, Line 142: static void write_active_relationship_table(const struct dptf_active_policy *policies, int max_count) line over 96 characters
https://review.coreboot.org/c/coreboot/+/41885/1/src/acpi/acpigen_dptf.c@161 PS1, Line 161: for (i = 0; i < max_count; ++i) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/41885/1/src/include/acpi/acpigen_dp... File src/include/acpi/acpigen_dptf.h:
https://review.coreboot.org/c/coreboot/+/41885/1/src/include/acpi/acpigen_dp... PS1, Line 51: } thresholds [DPTF_MAX_ACX]; space prohibited before open square bracket '['
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41885
to look at the new patch set (#2).
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c M src/include/acpi/acpigen_dptf.h 4 files changed, 204 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/2
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41885
to look at the new patch set (#4).
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c M src/include/acpi/acpigen_dptf.h 4 files changed, 204 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/4
Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Sumeet R Pawnikar, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41885
to look at the new patch set (#6).
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c M src/include/acpi/acpigen_dptf.h 4 files changed, 204 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/6
Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Sumeet R Pawnikar, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41885
to look at the new patch set (#7).
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/Makefile.inc A src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c A src/include/acpi/acpigen_dptf.h 5 files changed, 266 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/7
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41885/7/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/7/src/acpi/acpigen_dptf.c@39 PS7, Line 39: TFN1 could these be more descriptive (like DPTF_CHARGER) since the ACPI name is hidden?
https://review.coreboot.org/c/coreboot/+/41885/7/src/acpi/acpigen_dptf.c@75 PS7, Line 75: Almost all Intel boards use a Global NVS field named DPTE. Should we kill this? can just turn off the device in the devicetree to prevent the ACPI from being generated.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41885/7/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/7/src/acpi/acpigen_dptf.c@39 PS7, Line 39: TFN1
could these be more descriptive (like DPTF_CHARGER) since the ACPI name is hidden?
Yeah I waffled on that, should have stuck with longer names, will go back to that.
https://review.coreboot.org/c/coreboot/+/41885/7/src/acpi/acpigen_dptf.c@75 PS7, Line 75: Almost all Intel boards use a Global NVS field named DPTE.
Should we kill this? can just turn off the device in the devicetree to prevent the ACPI from being […]
SGTM.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Duncan Laurie, Sumeet R Pawnikar, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41885
to look at the new patch set (#8).
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/Makefile.inc A src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c A src/include/acpi/acpigen_dptf.h 5 files changed, 250 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/8
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Duncan Laurie, Sumeet R Pawnikar, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41885
to look at the new patch set (#9).
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/Makefile.inc A src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c A src/include/acpi/acpigen_dptf.h 5 files changed, 250 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/9
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41885/7/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/7/src/acpi/acpigen_dptf.c@75 PS7, Line 75: Almost all Intel boards use a Global NVS field named DPTE.
SGTM.
Done
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41885/10/src/include/acpi/acpigen_d... File src/include/acpi/acpigen_dptf.h:
https://review.coreboot.org/c/coreboot/+/41885/10/src/include/acpi/acpigen_d... PS10, Line 36: /* */ Nit: empty comment. Probably missing explanation for weight.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c@17 PS10, Line 17: 1/10 degree Kelvin I am confused by the calculation/comment here:
Shouldn't 1/10th of degree Kelvin = (deg_c + 273) / 10 ?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c@17 PS10, Line 17: 1/10 degree Kelvin
I am confused by the calculation/comment here: […]
That would be 10s of degrees Kelvin (decadegrees K). ACPI uses decidegrees Kelvin. Example: 50 degrees C would be 3232 (323.2 degrees K, or 3232 decidegrees K). Your calculation would make that 32.3 (decadegrees K).
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c@17 PS10, Line 17: 1/10 degree Kelvin
That would be 10s of degrees Kelvin (decadegrees K). ACPI uses decidegrees Kelvin. […]
Ack. Thanks for the explanation.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c@8 PS10, Line 8: #define GNVS_DPTE "\DPTE" I think this should be unused now.
https://review.coreboot.org/c/coreboot/+/41885/10/src/drivers/intel/dptf/dpt... File src/drivers/intel/dptf/dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/10/src/drivers/intel/dptf/dpt... PS10, Line 36: it's its?
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Duncan Laurie, Sumeet R Pawnikar, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41885
to look at the new patch set (#11).
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/Makefile.inc A src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c A src/include/acpi/acpigen_dptf.h 5 files changed, 249 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/11
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 11:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c File src/acpi/acpigen_dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/10/src/acpi/acpigen_dptf.c@8 PS10, Line 8: #define GNVS_DPTE "\DPTE"
I think this should be unused now.
Done
https://review.coreboot.org/c/coreboot/+/41885/10/src/drivers/intel/dptf/dpt... File src/drivers/intel/dptf/dptf.c:
https://review.coreboot.org/c/coreboot/+/41885/10/src/drivers/intel/dptf/dpt... PS10, Line 36: it's
its?
Done
https://review.coreboot.org/c/coreboot/+/41885/10/src/include/acpi/acpigen_d... File src/include/acpi/acpigen_dptf.h:
https://review.coreboot.org/c/coreboot/+/41885/10/src/include/acpi/acpigen_d... PS10, Line 36: /* */
Nit: empty comment. Probably missing explanation for weight.
Oops yep, that's it. Will fix.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41885/11/src/include/acpi/acpigen_d... File src/include/acpi/acpigen_dptf.h:
https://review.coreboot.org/c/coreboot/+/41885/11/src/include/acpi/acpigen_d... PS11, Line 27: /* A device can only define _AC0 .. _AC9 (i.e., 10) Active Cooling Methods */ 0 .. 10
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41885/11/src/include/acpi/acpigen_d... File src/include/acpi/acpigen_dptf.h:
https://review.coreboot.org/c/coreboot/+/41885/11/src/include/acpi/acpigen_d... PS11, Line 27: /* A device can only define _AC0 .. _AC9 (i.e., 10) Active Cooling Methods */
0 .. […]
Ack
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Duncan Laurie, Sumeet R Pawnikar, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41885
to look at the new patch set (#12).
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/Makefile.inc A src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c A src/include/acpi/acpigen_dptf.h 5 files changed, 249 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41885/12
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 13: Code-Review+1
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
Patch Set 13: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41885 )
Change subject: dptf: Add support for generation of Active Policies ......................................................................
dptf: Add support for generation of Active Policies
This change adds support for generating the different pieces of DPTF Active Policies. This includes the Active Relationship Table, in addition to _ACx methods.
BUG=b:143539650 TEST=compiles
Change-Id: Iea0ccbd96f88d0f3a8f2c77a7d0f3a284e5ee463 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/41885 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Reviewed-by: Duncan Laurie dlaurie@chromium.org --- M src/acpi/Makefile.inc A src/acpi/acpigen_dptf.c M src/drivers/intel/dptf/chip.h M src/drivers/intel/dptf/dptf.c A src/include/acpi/acpigen_dptf.h 5 files changed, 249 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved Sumeet R Pawnikar: Looks good to me, but someone else must approve
diff --git a/src/acpi/Makefile.inc b/src/acpi/Makefile.inc index 29ddc08..f70b23f 100644 --- a/src/acpi/Makefile.inc +++ b/src/acpi/Makefile.inc @@ -4,6 +4,7 @@
ramstage-y += acpi.c ramstage-y += acpigen.c +ramstage-y += acpigen_dptf.c ramstage-y += acpigen_dsm.c ramstage-y += acpigen_ps2_keybd.c ramstage-y += acpigen_usb.c diff --git a/src/acpi/acpigen_dptf.c b/src/acpi/acpigen_dptf.c new file mode 100644 index 0000000..74e9191 --- /dev/null +++ b/src/acpi/acpigen_dptf.c @@ -0,0 +1,166 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <acpi/acpigen.h> +#include <acpi/acpigen_dptf.h> + +/* Hardcoded paths */ +#define TOPLEVEL_DPTF_SCOPE "\_SB.DPTF" + +/* Defaults */ +enum { + ART_REVISION = 0, + DEFAULT_WEIGHT = 100, + DPTF_MAX_ART_THRESHOLDS = 10, +}; + +/* Convert degrees C to 1/10 degree Kelvin for ACPI */ +static int to_acpi_temp(int deg_c) +{ + return deg_c * 10 + 2732; +} + +/* Writes out a 0-argument non-Serialized Method that returns an Integer */ +static void write_simple_return_method(const char *name, int value) +{ + acpigen_write_method(name, 0); + acpigen_write_return_integer(value); + acpigen_pop_len(); /* Method */ +} + +/* Return the assigned namestring of any participant */ +static const char *namestring_of(enum dptf_participant participant) +{ + switch (participant) { + case DPTF_CPU: + return "TCPU"; + case DPTF_CHARGER: + return "TCHG"; + case DPTF_FAN: + return "TFN1"; + case DPTF_TEMP_SENSOR_0: + return "TSR0"; + case DPTF_TEMP_SENSOR_1: + return "TSR1"; + case DPTF_TEMP_SENSOR_2: + return "TSR2"; + case DPTF_TEMP_SENSOR_3: + return "TSR3"; + default: + return ""; + } +} + +/* Helper to get Scope for participants underneath _SB.DPTF */ +static const char *scope_of(enum dptf_participant participant) +{ + static char scope[16]; + + if (participant == DPTF_CPU) + snprintf(scope, sizeof(scope), "\_SB.%s", namestring_of(participant)); + else + snprintf(scope, sizeof(scope), TOPLEVEL_DPTF_SCOPE ".%s", + namestring_of(participant)); + + return scope; +} + +/* Write out scope of a participant */ +void dptf_write_scope(enum dptf_participant participant) +{ + acpigen_write_scope(scope_of(participant)); +} + +/* + * This table describes active cooling relationships between the system's fan and the + * temperature sensors that it can have an effect on. As ever-increasing temperature thresholds + * are crossed (_AC9.._AC0, low to high), the corresponding fan percentages listed in this table + * are used to increase the speed of the fan in order to speed up cooling. + */ +static void write_active_relationship_table(const struct dptf_active_policy *policies, + int max_count) +{ + char *pkg_count; + int i, j; + + /* Nothing to do */ + if (!max_count || policies[0].target == DPTF_NONE) + return; + + acpigen_write_scope(TOPLEVEL_DPTF_SCOPE); + acpigen_write_method("_ART", 0); + + /* Return this package */ + acpigen_emit_byte(RETURN_OP); + + /* Keep track of items added to the package */ + pkg_count = acpigen_write_package(1); /* The '1' here is for the revision */ + acpigen_write_integer(ART_REVISION); + + for (i = 0; i < max_count; ++i) { + /* + * These have to be filled out from AC0 down to AC9, filling in only as many + * as are used. As soon as one isn't filled in, we're done. + */ + if (policies[i].target == DPTF_NONE) + break; + + (*pkg_count)++; + + /* Source, Target, Percent, Fan % for each of _AC0 ... _AC9 */ + acpigen_write_package(13); + acpigen_emit_namestring(namestring_of(DPTF_FAN)); + acpigen_emit_namestring(namestring_of(policies[i].target)); + acpigen_write_integer(DEFAULT_IF_0(policies[i].weight, DEFAULT_WEIGHT)); + + /* Write out fan %; corresponds with target's _ACx methods */ + for (j = 0; j < DPTF_MAX_ART_THRESHOLDS; ++j) + acpigen_write_integer(policies[i].thresholds[j].fan_pct); + + acpigen_pop_len(); /* inner Package */ + } + + acpigen_pop_len(); /* outer Package */ + acpigen_pop_len(); /* Method _ART */ + acpigen_pop_len(); /* Scope */ +} + +/* + * _AC9 through _AC0 represent temperature thresholds, in increasing order, defined from _AC0 + * down, that, when reached, DPTF will activate TFN1 in order to actively cool the temperature + * sensor(s). As increasing thresholds are reached, the fan is spun faster. + */ +static void write_active_cooling_methods(const struct dptf_active_policy *policies, + int max_count) +{ + char name[5]; + int i, j; + + /* Nothing to do */ + if (!max_count || policies[0].target == DPTF_NONE) + return; + + for (i = 0; i < max_count; ++i) { + if (policies[i].target == DPTF_NONE) + break; + + dptf_write_scope(policies[i].target); + + /* Write out as many of _AC0 through _AC9 that are applicable */ + for (j = 0; j < DPTF_MAX_ACX; ++j) { + if (!policies[i].thresholds[j].temp) + break; + + snprintf(name, sizeof(name), "_AC%1X", j); + write_simple_return_method(name, to_acpi_temp( + policies[i].thresholds[j].temp)); + } + + acpigen_pop_len(); /* Scope */ + } +} + +void dptf_write_active_policies(const struct dptf_active_policy *policies, int max_count) +{ + write_active_relationship_table(policies, max_count); + write_active_cooling_methods(policies, max_count); +} diff --git a/src/drivers/intel/dptf/chip.h b/src/drivers/intel/dptf/chip.h index 704b83e..730d23e 100644 --- a/src/drivers/intel/dptf/chip.h +++ b/src/drivers/intel/dptf/chip.h @@ -3,7 +3,12 @@ #ifndef _DRIVERS_INTEL_DPTF_CHIP_H_ #define _DRIVERS_INTEL_DPTF_CHIP_H_
+#include <acpi/acpigen_dptf.h> + struct drivers_intel_dptf_config { + struct { + struct dptf_active_policy active[DPTF_MAX_ACTIVE_POLICIES]; + } policies; };
#endif /* _DRIVERS_INTEL_DPTF_CHIP_H_ */ diff --git a/src/drivers/intel/dptf/dptf.c b/src/drivers/intel/dptf/dptf.c index 060864a..20f8d9b 100644 --- a/src/drivers/intel/dptf/dptf.c +++ b/src/drivers/intel/dptf/dptf.c @@ -5,18 +5,6 @@ #include <device/device.h> #include "chip.h"
-enum dptf_participant { - DPTF_NONE, - DPTF_CPU, - DPTF_CHARGER, - DPTF_FAN, - DPTF_TEMP_SENSOR_0, - DPTF_TEMP_SENSOR_1, - DPTF_TEMP_SENSOR_2, - DPTF_TEMP_SENSOR_3, - DPTF_PARTICIPANT_COUNT, -}; - /* Generic DPTF participants have a PTYP field to distinguish them */ enum dptf_generic_participant_type { DPTF_GENERIC_PARTICIPANT_TYPE_TSR = 0x3, @@ -40,6 +28,17 @@ static bool is_participant_used(const struct drivers_intel_dptf_config *config, enum dptf_participant participant) { + int i; + + /* Active? */ + for (i = 0; i < DPTF_MAX_ACTIVE_POLICIES; ++i) + if (config->policies.active[i].target == participant) + return true; + + /* Check fan as well (its use is implicit in the Active policy) */ + if (participant == DPTF_FAN && config->policies.active[0].target != DPTF_NONE) + return true; + return false; }
@@ -53,6 +52,9 @@ { struct drivers_intel_dptf_config *config = config_of(dev);
+ dptf_write_active_policies(config->policies.active, + DPTF_MAX_ACTIVE_POLICIES); + printk(BIOS_INFO, "\_SB.DPTF: %s at %s\n", dev->chip_ops->name, dev_path(dev)); }
diff --git a/src/include/acpi/acpigen_dptf.h b/src/include/acpi/acpigen_dptf.h new file mode 100644 index 0000000..a082b62 --- /dev/null +++ b/src/include/acpi/acpigen_dptf.h @@ -0,0 +1,63 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef ACPI_ACPIGEN_DPTF_H +#define ACPI_ACPIGEN_DPTF_H + +#include <device/device.h> +#include <stdbool.h> + +/* A common idiom is to use a default value if none is provided (i.e., == 0) */ +#define DEFAULT_IF_0(thing, default_) ((thing) ? (thing) : (default_)) + +/* List of available participants (i.e., they can participate in policies) */ +enum dptf_participant { + DPTF_NONE, + DPTF_CPU, + DPTF_CHARGER, + DPTF_FAN, + DPTF_TEMP_SENSOR_0, + DPTF_TEMP_SENSOR_1, + DPTF_TEMP_SENSOR_2, + DPTF_TEMP_SENSOR_3, + DPTF_PARTICIPANT_COUNT, +}; + +/* DPTF compile-time constants */ +enum { + /* A device can only define _AC0 .. _AC9 i.e. between 0 and 10 Active Cooling Methods */ + DPTF_MAX_ACX = 10, + DPTF_MAX_ACTIVE_POLICIES = (DPTF_PARTICIPANT_COUNT-1), +}; + +/* Active Policy */ +struct dptf_active_policy { + /* Device capable of being affected by the fan */ + enum dptf_participant target; + /* Source's contribution to the Target's cooling capability as a percentage */ + uint8_t weight; + /* When target reaches temperature 'temp', the source will turn on at 'fan_pct' % */ + struct { + /* (degrees C) */ + uint8_t temp; + /* 0 - 100 */ + uint8_t fan_pct; + } thresholds[DPTF_MAX_ACX]; +}; + +/* + * This function provides tables of temperature and corresponding fan or percent. When the + * temperature thresholds are met (_AC0 - _AC9), the fan is driven to corresponding percentage + * of full speed. + */ +void dptf_write_active_policies(const struct dptf_active_policy *policies, int max_count); + +/* Helper method to open the scope for a given participant. */ +void dptf_write_scope(enum dptf_participant participant); + +/* + * Write out a _STA that will check the value of the DPTE field in GNVS, and return 0xF if DPTE + * is 1, otherwise it will return 0. + */ +void dptf_write_STA(void); + +#endif /* ACPI_ACPIGEN_DPTF_H */