From 5212ece6cf27fa91ffd07d97d4fbe171129cf1fa Mon Sep 17 00:00:00 2001 From: Tim Wawrzynczak Date: Thu, 16 Jul 2020 11:54:04 -0600 Subject: dptf: Fix scope of TCPU device In the initial DPTF refactor, the scope of the TCPU device was incorrectly set as \_SB, instead of \_SB.PCI0. However, because of the way that the acpi_inject_dsdt() callback currently works (it injects contents before the dsdt.aml file), the Scope where the TCPU device lives (\_SB.PCI0) doesn't exist yet. Therefore, to avoid playing games with *when* things are defined in the DSDT, switch to defining all of the DPTF devices in the SSDT. Signed-off-by: Tim Wawrzynczak Change-Id: Ia4922b4dc6544d79d44d39e6ad18c6ab9fee0fd7 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43529 Reviewed-by: Duncan Laurie Reviewed-by: Karthik Ramasubramanian Tested-by: build bot (Jenkins) --- src/acpi/acpigen_dptf.c | 34 +++--- src/drivers/intel/dptf/dptf.c | 222 ++++++++++++++++++++++------------------ src/include/acpi/acpigen_dptf.h | 4 + 3 files changed, 149 insertions(+), 111 deletions(-) (limited to 'src') diff --git a/src/acpi/acpigen_dptf.c b/src/acpi/acpigen_dptf.c index c8849261d6..0c44b8f8ba 100644 --- a/src/acpi/acpigen_dptf.c +++ b/src/acpi/acpigen_dptf.c @@ -3,9 +3,6 @@ #include #include -/* Hardcoded paths */ -#define TOPLEVEL_DPTF_SCOPE "\\_SB.DPTF" - /* Defaults */ #define DEFAULT_RAW_UNIT "ma" @@ -81,14 +78,27 @@ 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)); + snprintf(scope, sizeof(scope), TCPU_SCOPE ".%s", namestring_of(participant)); else - snprintf(scope, sizeof(scope), TOPLEVEL_DPTF_SCOPE ".%s", + snprintf(scope, sizeof(scope), DPTF_DEVICE_PATH ".%s", namestring_of(participant)); return scope; } +/* + * Most of the DPTF participants are underneath the \_SB.DPTF scope, so we can just get away + * with using the simple namestring for references, but the TCPU has a different scope, so + * either an absolute or relative path must be used instead. + */ +static const char *path_of(enum dptf_participant participant) +{ + if (participant == DPTF_CPU) + return scope_of(participant); + else + return namestring_of(participant); +} + /* Write out scope of a participant */ void dptf_write_scope(enum dptf_participant participant) { @@ -111,7 +121,7 @@ static void write_active_relationship_table(const struct dptf_active_policy *pol if (!max_count || policies[0].target == DPTF_NONE) return; - acpigen_write_scope(TOPLEVEL_DPTF_SCOPE); + acpigen_write_scope(DPTF_DEVICE_PATH); acpigen_write_method("_ART", 0); /* Return this package */ @@ -133,8 +143,8 @@ static void write_active_relationship_table(const struct dptf_active_policy *pol /* 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_emit_namestring(path_of(DPTF_FAN)); + acpigen_emit_namestring(path_of(policies[i].target)); acpigen_write_integer(DEFAULT_IF_0(policies[i].weight, DEFAULT_WEIGHT)); /* Write out fan %; corresponds with target's _ACx methods */ @@ -206,7 +216,7 @@ static void write_thermal_relationship_table(const struct dptf_passive_policy *p if (!max_count || policies[0].source == DPTF_NONE) return; - acpigen_write_scope(TOPLEVEL_DPTF_SCOPE); + acpigen_write_scope(DPTF_DEVICE_PATH); /* * A _TRT Revision (TRTR) of 1 means that the 'Priority' field is an arbitrary priority @@ -234,8 +244,8 @@ static void write_thermal_relationship_table(const struct dptf_passive_policy *p acpigen_write_package(8); /* Source, Target, Priority, Sampling Period */ - acpigen_emit_namestring(namestring_of(policies[i].source)); - acpigen_emit_namestring(namestring_of(policies[i].target)); + acpigen_emit_namestring(path_of(policies[i].source)); + acpigen_emit_namestring(path_of(policies[i].target)); acpigen_write_integer(DEFAULT_IF_0(policies[i].priority, DEFAULT_PRIORITY)); acpigen_write_integer(to_acpi_time(policies[i].period)); @@ -458,7 +468,7 @@ void dptf_write_enabled_policies(const struct dptf_active_policy *active_policie if (!pkg_count) return; - acpigen_write_scope(TOPLEVEL_DPTF_SCOPE); + acpigen_write_scope(DPTF_DEVICE_PATH); acpigen_write_name("IDSP"); acpigen_write_package(pkg_count); diff --git a/src/drivers/intel/dptf/dptf.c b/src/drivers/intel/dptf/dptf.c index 1e4063e40f..4bc6b082a4 100644 --- a/src/drivers/intel/dptf/dptf.c +++ b/src/drivers/intel/dptf/dptf.c @@ -58,52 +58,6 @@ static const char *dptf_acpi_name(const struct device *dev) return "DPTF"; } -/* Add custom tables and methods to SSDT */ -static void dptf_fill_ssdt(const struct device *dev) -{ - struct drivers_intel_dptf_config *config = config_of(dev); - enum dptf_participant p; - bool tsr_en[DPTF_MAX_TSR] = {false}; - int i; - - for (p = DPTF_TEMP_SENSOR_0, i = 0; p <= DPTF_TEMP_SENSOR_3; ++p, ++i) - tsr_en[i] = is_participant_used(config, p); - - /* Policies */ - dptf_write_enabled_policies(config->policies.active, DPTF_MAX_ACTIVE_POLICIES, - config->policies.passive, DPTF_MAX_PASSIVE_POLICIES, - config->policies.critical, DPTF_MAX_CRITICAL_POLICIES); - - dptf_write_active_policies(config->policies.active, - DPTF_MAX_ACTIVE_POLICIES); - - dptf_write_passive_policies(config->policies.passive, - DPTF_MAX_PASSIVE_POLICIES); - - dptf_write_critical_policies(config->policies.critical, - DPTF_MAX_CRITICAL_POLICIES); - - /* Controls */ - dptf_write_charger_perf(config->controls.charger_perf, DPTF_MAX_CHARGER_PERF_STATES); - dptf_write_fan_perf(config->controls.fan_perf, DPTF_MAX_FAN_PERF_STATES); - dptf_write_power_limits(&config->controls.power_limits); - - /* Fan options */ - dptf_write_fan_options(config->options.fan.fine_grained_control, - config->options.fan.step_size, - config->options.fan.low_speed_notify); - - /* TSR options */ - for (p = DPTF_TEMP_SENSOR_0, i = 0; p <= DPTF_TEMP_SENSOR_3; ++p, ++i) { - if (tsr_en[i]) { - dptf_write_tsr_hysteresis(config->options.tsr[i].hysteresis); - dptf_write_STR(config->options.tsr[i].desc); - } - } - - printk(BIOS_INFO, "\\_SB.DPTF: %s at %s\n", dev->chip_ops->name, dev_path(dev)); -} - static int get_STA_value(const struct drivers_intel_dptf_config *config, enum dptf_participant participant) { @@ -112,6 +66,7 @@ static int get_STA_value(const struct drivers_intel_dptf_config *config, ACPI_STATUS_DEVICE_ALL_OFF; } +/* Devices with GENERIC _HID (distinguished by PTYP) */ static void dptf_write_generic_participant(const char *name, enum dptf_generic_participant_type ptype, const char *str, int sta_val) @@ -120,13 +75,11 @@ static void dptf_write_generic_participant(const char *name, static int generic_uid = 0; acpigen_write_device(name); - - if (CONFIG(DPTF_USE_EISA_HID)) { - acpigen_write_name("_HID"); + acpigen_write_name("_HID"); + if (CONFIG(DPTF_USE_EISA_HID)) acpigen_emit_eisaid(GENERIC_HID_EISAID); - } else { - acpigen_write_name_string("_HID", GENERIC_HID); - } + else + acpigen_write_string(GENERIC_HID); acpigen_write_name_integer("_UID", generic_uid++); acpigen_write_STA(sta_val); @@ -139,62 +92,40 @@ static void dptf_write_generic_participant(const char *name, acpigen_pop_len(); /* Device */ } -/* Add static definitions of DPTF devices into the DSDT */ -static void dptf_inject_dsdt(const struct device *dev) +/* \_SB.PCI0.TCPU */ +static void write_tcpu(const struct device *pci_dev, + const struct drivers_intel_dptf_config *config) { - const struct drivers_intel_dptf_config *config; - enum dptf_participant participant; - struct device *parent; - char name[5]; - int i; - - /* The CPU device gets an _ADR that matches the ACPI PCI address for 00:04.00 */ - parent = dev && dev->bus ? dev->bus->dev : NULL; - if (!parent || parent->path.type != DEVICE_PATH_PCI) { - printk(BIOS_ERR, "%s: DPTF objects must live under 00:04.0 PCI device\n", - __func__); - return; - } - - config = config_of(dev); - acpigen_write_scope("\\_SB"); - - /* DPTF CPU device - \_SB.TCPU */ + /* DPTF CPU device - \_SB.PCI0.TCPU */ + acpigen_write_scope(TCPU_SCOPE); acpigen_write_device("TCPU"); - acpigen_write_ADR_pci_device(parent); + acpigen_write_ADR_pci_device(pci_dev); acpigen_write_STA(get_STA_value(config, DPTF_CPU)); acpigen_pop_len(); /* Device */ + acpigen_pop_len(); /* TCPU Scope */ +} - /* Toplevel DPTF device - \_SB.DPTF*/ - acpigen_write_device(acpi_device_name(dev)); - if (CONFIG(DPTF_USE_EISA_HID)) { - acpigen_write_name("_HID"); - acpigen_emit_eisaid(DPTF_DEVICE_HID_EISAID); - } else { - acpigen_write_name_string("_HID", DPTF_DEVICE_HID); - } - - acpigen_write_name_integer("_UID", 0); - acpigen_write_STA(ACPI_STATUS_DEVICE_ALL_ON); - - /* - * The following devices live underneath \_SB.DPTF: - * - Fan, \_SB.DPTF.TFN1 - * - Charger, \_SB.DPTF.TCHG - * - Temperature Sensors, \_SB.DPTF.TSRn - */ - +/* \_SB.DPTF.TFN1 */ +static void write_fan(const struct drivers_intel_dptf_config *config) +{ acpigen_write_device("TFN1"); - if (CONFIG(DPTF_USE_EISA_HID)) { - acpigen_write_name("_HID"); + acpigen_write_name("_HID"); + if (CONFIG(DPTF_USE_EISA_HID)) acpigen_emit_eisaid(FAN_HID_EISAID); - } else { - acpigen_write_name_string("_HID", FAN_HID); - } + else + acpigen_write_string(FAN_HID); acpigen_write_name_integer("_UID", 0); acpigen_write_STA(get_STA_value(config, DPTF_FAN)); acpigen_pop_len(); /* Device */ +} + +/* \_SB.DPTF.xxxx */ +static void write_generic_devices(const struct drivers_intel_dptf_config *config) +{ + enum dptf_participant participant; + char name[ACPI_NAME_BUFFER_SIZE]; + int i; dptf_write_generic_participant("TCHG", DPTF_GENERIC_PARTICIPANT_TYPE_CHARGER, DEFAULT_CHARGER_STR, get_STA_value(config, @@ -205,17 +136,110 @@ static void dptf_inject_dsdt(const struct device *dev) dptf_write_generic_participant(name, DPTF_GENERIC_PARTICIPANT_TYPE_TSR, NULL, get_STA_value(config, participant)); } +} + +/* \_SB.DPTF - note: leaves the Scope open for child devices*/ +static void write_open_dptf_device(const struct device *dev) +{ + acpigen_write_scope("\\_SB"); + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_name("_HID"); + if (CONFIG(DPTF_USE_EISA_HID)) + acpigen_emit_eisaid(DPTF_DEVICE_HID_EISAID); + else + acpigen_write_string(DPTF_DEVICE_HID); + + acpigen_write_name_integer("_UID", 0); + acpigen_write_STA(ACPI_STATUS_DEVICE_ALL_ON); +} + +/* Add minimal definitions of DPTF devices into the SSDT */ +static void write_device_definitions(const struct device *dev) +{ + const struct drivers_intel_dptf_config *config; + struct device *parent; + + /* The CPU device gets an _ADR that matches the ACPI PCI address for 00:04.00 */ + parent = dev && dev->bus ? dev->bus->dev : NULL; + if (!parent || parent->path.type != DEVICE_PATH_PCI) { + printk(BIOS_ERR, "%s: DPTF objects must live under 00:04.0 PCI device\n", + __func__); + return; + } - acpigen_pop_len(); /* DPTF Device */ + config = config_of(dev); + write_tcpu(parent, config); + write_open_dptf_device(dev); + write_fan(config); + write_generic_devices(config); + + acpigen_pop_len(); /* DPTF Device (write_open_dptf_device) */ acpigen_pop_len(); /* Scope */ } +/* Emites policy definitions for each policy type */ +static void write_policies(const struct drivers_intel_dptf_config *config) +{ + dptf_write_enabled_policies(config->policies.active, DPTF_MAX_ACTIVE_POLICIES, + config->policies.passive, DPTF_MAX_PASSIVE_POLICIES, + config->policies.critical, DPTF_MAX_CRITICAL_POLICIES); + + dptf_write_active_policies(config->policies.active, + DPTF_MAX_ACTIVE_POLICIES); + + dptf_write_passive_policies(config->policies.passive, + DPTF_MAX_PASSIVE_POLICIES); + + dptf_write_critical_policies(config->policies.critical, + DPTF_MAX_CRITICAL_POLICIES); +} + +/* Writes other static tables that are used by DPTF */ +static void write_controls(const struct drivers_intel_dptf_config *config) +{ + dptf_write_charger_perf(config->controls.charger_perf, DPTF_MAX_CHARGER_PERF_STATES); + dptf_write_fan_perf(config->controls.fan_perf, DPTF_MAX_FAN_PERF_STATES); + dptf_write_power_limits(&config->controls.power_limits); +} + +/* Options to control the behavior of devices */ +static void write_options(const struct drivers_intel_dptf_config *config) +{ + enum dptf_participant p; + int i; + + /* Fan options */ + dptf_write_fan_options(config->options.fan.fine_grained_control, + config->options.fan.step_size, + config->options.fan.low_speed_notify); + + /* TSR options */ + for (p = DPTF_TEMP_SENSOR_0, i = 0; p <= DPTF_TEMP_SENSOR_3; ++p, ++i) { + if (is_participant_used(config, p)) { + dptf_write_tsr_hysteresis(config->options.tsr[i].hysteresis); + dptf_write_STR(config->options.tsr[i].desc); + } + } +} + +/* Add custom tables and methods to SSDT */ +static void dptf_fill_ssdt(const struct device *dev) +{ + struct drivers_intel_dptf_config *config = config_of(dev); + + write_device_definitions(dev); + write_policies(config); + write_controls(config); + write_options(config); + + printk(BIOS_INFO, DPTF_DEVICE_PATH ": %s at %s\n", dev->chip_ops->name, dev_path(dev)); +} + static struct device_operations dptf_ops = { .read_resources = noop_read_resources, .set_resources = noop_set_resources, .acpi_name = dptf_acpi_name, .acpi_fill_ssdt = dptf_fill_ssdt, - .acpi_inject_dsdt = dptf_inject_dsdt, }; static void dptf_enable_dev(struct device *dev) diff --git a/src/include/acpi/acpigen_dptf.h b/src/include/acpi/acpigen_dptf.h index 89b64f3728..1790df77e8 100644 --- a/src/include/acpi/acpigen_dptf.h +++ b/src/include/acpi/acpigen_dptf.h @@ -9,6 +9,10 @@ /* 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_)) +/* Hardcoded paths */ +#define DPTF_DEVICE_PATH "\\_SB.DPTF" +#define TCPU_SCOPE "\\_SB.PCI0" + /* List of available participants (i.e., they can participate in policies) */ enum dptf_participant { DPTF_NONE, -- cgit v1.2.3