From 2f7fa5543308e83787b7cef6e83e20e1e3f379d9 Mon Sep 17 00:00:00 2001 From: Sumeet Pawnikar Date: Wed, 8 Jun 2022 17:43:36 +0530 Subject: [PATCH] Reland "drivers/intel/dptf: Add multiple fan support under dptf" This reverts commit 4dba71fd25c91a9e610287c61238a8fe24452e4e. Add multiple fan support for dptf policies. This also fixes the Google Meet resolution drop issue as per b:246535768 comment#12. When system starts Google Meet video call, it uses the hardware accelerated encoder as expected. But, as soon as another system connects to the call, an immediate fallback is observed from hardware to software encoder. Due to this, Google Meet resolution dropped from 720p to 180p. This issue is observed on Alder Lake-N SoC based fanless platforms. This same issue was not seen on fan based systems. With the fix in dptf driver where fan configures appropriate setting for only fan participant, not for other device participants, able to see consistent 720p resolution. BUG=b:246535768,b:235254828 BRANCH=None TEST=Built and tested on Alder Lake-P Redrix system for two fans support and on Alder Lake-N fanless systems. With this code change Google Meet resolution drop not observed. Signed-off-by: Sumeet Pawnikar Change-Id: Id07d279ff962253c22be9d395ed7be0d732aeaa7 Reviewed-on: https://review.coreboot.org/c/coreboot/+/73249 Reviewed-by: Lean Sheng Tan Reviewed-by: Paul Menzel Reviewed-by: Reka Norman Tested-by: build bot (Jenkins) --- src/acpi/acpigen_dptf.c | 89 ++++++++++++++++++------ src/drivers/intel/dptf/chip.h | 12 ++++ src/drivers/intel/dptf/dptf.c | 83 +++++++++++++++++----- src/ec/google/chromeec/acpi/emem.asl | 1 + src/ec/google/chromeec/chip.h | 1 + src/ec/google/chromeec/ec_acpi.c | 2 +- src/ec/google/chromeec/ec_dptf_helpers.c | 35 ++++++++-- src/ec/google/common/dptf.h | 2 +- src/include/acpi/acpigen_dptf.h | 31 ++++++++- 9 files changed, 209 insertions(+), 47 deletions(-) diff --git a/src/acpi/acpigen_dptf.c b/src/acpi/acpigen_dptf.c index a458a5599b..d1a79d18e2 100644 --- a/src/acpi/acpigen_dptf.c +++ b/src/acpi/acpigen_dptf.c @@ -62,6 +62,8 @@ static const char *namestring_of(enum dptf_participant participant) return "TCHG"; case DPTF_FAN: return "TFN1"; + case DPTF_FAN_2: + return "TFN2"; case DPTF_TEMP_SENSOR_0: return "TSR0"; case DPTF_TEMP_SENSOR_1: @@ -123,7 +125,7 @@ void dptf_write_scope(enum dptf_participant participant) * 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) + int max_count, bool dptf_multifan_support) { char *pkg_count; int i, j; @@ -154,7 +156,11 @@ 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(path_of(DPTF_FAN)); + if (dptf_multifan_support) + acpigen_emit_namestring(path_of(policies[i].source)); + else + 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)); @@ -205,9 +211,10 @@ static void write_active_cooling_methods(const struct dptf_active_policy *polici } } -void dptf_write_active_policies(const struct dptf_active_policy *policies, int max_count) +void dptf_write_active_policies(const struct dptf_active_policy *policies, + int max_count, bool dptf_multifan_support) { - write_active_relationship_table(policies, max_count); + write_active_relationship_table(policies, max_count, dptf_multifan_support); write_active_cooling_methods(policies, max_count); } @@ -352,7 +359,29 @@ void dptf_write_charger_perf(const struct dptf_charger_perf *states, int max_cou acpigen_pop_len(); /* Scope */ } -void dptf_write_fan_perf(const struct dptf_fan_perf *states, int max_count) +int dptf_write_fan_perf_fps(uint8_t percent, uint16_t power, uint16_t speed, + uint16_t noise_level) +{ + /* + * Some _FPS tables do include a last entry where Percent is 0, but Power is + * called out, so this table is finished when both are zero. + */ + if (!percent && !power) + return 1; + + acpigen_write_package(5); + acpigen_write_integer(percent); + acpigen_write_integer(DEFAULT_TRIP_POINT); + acpigen_write_integer(speed); + acpigen_write_integer(noise_level); + acpigen_write_integer(power); + acpigen_pop_len(); /* inner Package */ + + return 0; +} + +void dptf_write_fan_perf(const struct dptf_fan_perf *states, int max_count, + enum dptf_participant participant) { char *pkg_count; int i; @@ -360,29 +389,49 @@ void dptf_write_fan_perf(const struct dptf_fan_perf *states, int max_count) if (!max_count || !states[0].percent) return; - dptf_write_scope(DPTF_FAN); + dptf_write_scope(participant); /* _FPS - Fan Performance States */ acpigen_write_name("_FPS"); + pkg_count = acpigen_write_package(1); /* 1 for Revision */ acpigen_write_integer(FPS_REVISION); /* revision */ for (i = 0; i < max_count; ++i) { - /* - * Some _FPS tables do include a last entry where Percent is 0, but Power is - * called out, so this table is finished when both are zero. - */ - if (!states[i].percent && !states[i].power) - break; - (*pkg_count)++; - acpigen_write_package(5); - acpigen_write_integer(states[i].percent); - acpigen_write_integer(DEFAULT_TRIP_POINT); - acpigen_write_integer(states[i].speed); - acpigen_write_integer(states[i].noise_level); - acpigen_write_integer(states[i].power); - acpigen_pop_len(); /* inner Package */ + if (dptf_write_fan_perf_fps(states[i].percent, states[i].power, + states[i].speed, states[i].noise_level)) + break; + } + + acpigen_pop_len(); /* Package */ + acpigen_pop_len(); /* Scope */ +} + +void dptf_write_multifan_perf( + const struct dptf_multifan_perf + states[DPTF_MAX_FAN_PARTICIPANTS][DPTF_MAX_FAN_PERF_STATES], + int max_count, enum dptf_participant participant, int fan_num) +{ + char *pkg_count; + int i; + + if (!max_count || !states[fan_num][0].percent) + return; + + dptf_write_scope(participant); + + /* _FPS - Fan Performance States */ + acpigen_write_name("_FPS"); + + pkg_count = acpigen_write_package(1); /* 1 for Revision */ + acpigen_write_integer(FPS_REVISION); /* revision */ + + for (i = 0; i < max_count; ++i) { + (*pkg_count)++; + if (dptf_write_fan_perf_fps(states[fan_num][i].percent, states[fan_num][i].power, + states[fan_num][i].speed, states[fan_num][i].noise_level)) + break; } acpigen_pop_len(); /* Package */ diff --git a/src/drivers/intel/dptf/chip.h b/src/drivers/intel/dptf/chip.h index 060f19699d..9bbf11f2f7 100644 --- a/src/drivers/intel/dptf/chip.h +++ b/src/drivers/intel/dptf/chip.h @@ -25,6 +25,8 @@ struct drivers_intel_dptf_config { struct { struct dptf_charger_perf charger_perf[DPTF_MAX_CHARGER_PERF_STATES]; struct dptf_fan_perf fan_perf[DPTF_MAX_FAN_PERF_STATES]; + struct dptf_multifan_perf + multifan_perf[DPTF_MAX_FAN_PARTICIPANTS][DPTF_MAX_FAN_PERF_STATES]; struct dptf_power_limits power_limits; } controls; @@ -44,6 +46,14 @@ struct drivers_intel_dptf_config { */ bool low_speed_notify; } fan; + + /* For multiple TFN fan options */ + struct { + bool fine_grained_control; + uint8_t step_size; + bool low_speed_notify; + } multifan_options[DPTF_MAX_FAN_PARTICIPANTS]; + struct { /* * The amount of hysteresis implemented in circuitry or in the platform @@ -62,6 +72,8 @@ struct drivers_intel_dptf_config { /* Rest of platform Power */ uint32_t prop; + + bool dptf_multifan_support; }; #endif /* _DRIVERS_INTEL_DPTF_CHIP_H_ */ diff --git a/src/drivers/intel/dptf/dptf.c b/src/drivers/intel/dptf/dptf.c index 886093e26d..fb7cb85e75 100644 --- a/src/drivers/intel/dptf/dptf.c +++ b/src/drivers/intel/dptf/dptf.c @@ -13,6 +13,7 @@ /* Generic DPTF participants have a PTYP field to distinguish them */ enum dptf_generic_participant_type { DPTF_GENERIC_PARTICIPANT_TYPE_TSR = 0x3, + DPTF_GENERIC_PARTICIPANT_TYPE_FAN = 0x4, DPTF_GENERIC_PARTICIPANT_TYPE_TPCH = 0x5, DPTF_GENERIC_PARTICIPANT_TYPE_CHARGER = 0xB, DPTF_GENERIC_PARTICIPANT_TYPE_BATTERY = 0xC, @@ -23,6 +24,7 @@ enum dptf_generic_participant_type { #define DEFAULT_TPCH_STR "Intel PCH FIVR Participant" #define DEFAULT_POWER_STR "Power Participant" #define DEFAULT_BATTERY_STR "Battery Participant" +#define DEFAULT_FAN_STR "Fan Participant" #define PMC_IPC_COMMAND_FIVR_SIZE 0x8 @@ -52,12 +54,26 @@ static bool is_participant_used(const struct drivers_intel_dptf_config *config, 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) + if ((participant == DPTF_FAN || participant == DPTF_FAN_2) && + config->policies.active[0].target != DPTF_NONE) return true; return false; } +/* Return the assigned namestring of the FAN participant */ +static const char *fan_namestring_of(enum dptf_participant participant) +{ + switch (participant) { + case DPTF_FAN: + return "TFN1"; + case DPTF_FAN_2: + return "TFN2"; + default: + return ""; + } +} + static const char *dptf_acpi_name(const struct device *dev) { return "DPTF"; @@ -116,15 +132,20 @@ static void write_tcpu(const struct device *pci_dev, acpigen_pop_len(); /* TCPU Scope */ } -/* \_SB.DPTF.TFN1 */ +/* \_SB.DPTF.TFNx */ static void write_fan(const struct drivers_intel_dptf_config *config, - const struct dptf_platform_info *platform_info) + const struct dptf_platform_info *platform_info, + enum dptf_participant participant) { - acpigen_write_device("TFN1"); + static int fan_uid = 0; + + acpigen_write_device(fan_namestring_of(participant)); acpigen_write_name("_HID"); dptf_write_hid(platform_info->use_eisa_hids, platform_info->fan_hid); - acpigen_write_name_integer("_UID", 0); - acpigen_write_STA(get_STA_value(config, DPTF_FAN)); + acpigen_write_name_integer("_UID", fan_uid++); + acpigen_write_name_string("_STR", DEFAULT_FAN_STR); + acpigen_write_name_integer("PTYP", DPTF_GENERIC_PARTICIPANT_TYPE_FAN); + acpigen_write_STA(get_STA_value(config, participant)); acpigen_pop_len(); /* Device */ } @@ -479,6 +500,7 @@ static void write_device_definitions(const struct device *dev) const struct dptf_platform_info *platform_info = get_dptf_platform_info(); const struct drivers_intel_dptf_config *config; struct device *parent; + enum dptf_participant p; /* The CPU device gets an _ADR that matches the ACPI PCI address for 00:04.00 */ parent = dev && dev->bus ? dev->bus->dev : NULL; @@ -491,7 +513,13 @@ static void write_device_definitions(const struct device *dev) config = config_of(dev); write_tcpu(parent, config); write_open_dptf_device(dev, platform_info); - write_fan(config, platform_info); + + if (config->dptf_multifan_support) { + for (p = DPTF_FAN; p <= DPTF_FAN_2; ++p) + write_fan(config, platform_info, p); + } else + write_fan(config, platform_info, DPTF_FAN); + write_oem_variables(config); write_imok(); write_generic_devices(config, platform_info); @@ -517,7 +545,7 @@ static void write_policies(const struct drivers_intel_dptf_config *config) config->policies.critical, DPTF_MAX_CRITICAL_POLICIES); dptf_write_active_policies(config->policies.active, - DPTF_MAX_ACTIVE_POLICIES); + DPTF_MAX_ACTIVE_POLICIES, config->dptf_multifan_support); dptf_write_passive_policies(config->policies.passive, DPTF_MAX_PASSIVE_POLICIES); @@ -529,8 +557,20 @@ static void write_policies(const struct drivers_intel_dptf_config *config) /* Writes other static tables that are used by DPTF */ static void write_controls(const struct drivers_intel_dptf_config *config) { + enum dptf_participant p; + int fan_num; + 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); + + /* Write TFN perf states based on the number of fans on the platform */ + if (config->dptf_multifan_support) { + for (p = DPTF_FAN, fan_num = 0; p <= DPTF_FAN_2; ++p, ++fan_num) + dptf_write_multifan_perf(config->controls.multifan_perf, + DPTF_MAX_FAN_PERF_STATES, p, fan_num); + } else + dptf_write_fan_perf(config->controls.fan_perf, DPTF_MAX_FAN_PERF_STATES, + DPTF_FAN); + dptf_write_power_limits(&config->controls.power_limits); } @@ -538,14 +578,25 @@ static void write_controls(const struct drivers_intel_dptf_config *config) static void write_options(const struct drivers_intel_dptf_config *config) { enum dptf_participant p; - int i; + int i, fan_num; - /* Fan options */ - dptf_write_scope(DPTF_FAN); - dptf_write_fan_options(config->options.fan.fine_grained_control, - config->options.fan.step_size, - config->options.fan.low_speed_notify); - acpigen_pop_len(); /* Scope */ + /* Configure Fan options based on the number of fans on the platform */ + if (config->dptf_multifan_support) { + for (p = DPTF_FAN, fan_num = 0; p <= DPTF_FAN_2; ++p, ++fan_num) { + dptf_write_scope(p); + dptf_write_fan_options( + config->options.multifan_options[fan_num].fine_grained_control, + config->options.multifan_options[fan_num].step_size, + config->options.multifan_options[fan_num].low_speed_notify); + acpigen_pop_len(); /* Scope */ + } + } else { + dptf_write_scope(DPTF_FAN); + dptf_write_fan_options(config->options.fan.fine_grained_control, + config->options.fan.step_size, + config->options.fan.low_speed_notify); + acpigen_pop_len(); /* Scope */ + } /* TSR options */ for (p = DPTF_TEMP_SENSOR_0, i = 0; p <= DPTF_TEMP_SENSOR_4; ++p, ++i) { diff --git a/src/ec/google/chromeec/acpi/emem.asl b/src/ec/google/chromeec/acpi/emem.asl index 3f9a457e18..59395f3bc5 100644 --- a/src/ec/google/chromeec/acpi/emem.asl +++ b/src/ec/google/chromeec/acpi/emem.asl @@ -16,6 +16,7 @@ TIN8, 8, // Temperature 8 TIN9, 8, // Temperature 9 Offset (0x10), FAN0, 16, // Fan Speed 0 +FAN1, 16, // Fan Speed 1 Offset (0x24), BTVR, 8, // Battery structure version Offset (0x30), diff --git a/src/ec/google/chromeec/chip.h b/src/ec/google/chromeec/chip.h index bb03e5731d..77851d1d3d 100644 --- a/src/ec/google/chromeec/chip.h +++ b/src/ec/google/chromeec/chip.h @@ -12,6 +12,7 @@ struct ec_google_chromeec_config { /* Pointer to PMC Mux connector for each Type-C port */ DEVTREE_CONST struct device *mux_conn[MAX_TYPEC_PORTS]; DEVTREE_CONST struct device *retimer_conn[MAX_TYPEC_PORTS]; + bool ec_multifan_support; }; #endif /* EC_GOOGLE_CHROMEEC_CHIP_H */ diff --git a/src/ec/google/chromeec/ec_acpi.c b/src/ec/google/chromeec/ec_acpi.c index 02b6394f04..53a4e75e59 100644 --- a/src/ec/google/chromeec/ec_acpi.c +++ b/src/ec/google/chromeec/ec_acpi.c @@ -276,7 +276,7 @@ void google_chromeec_fill_ssdt_generator(const struct device *dev) ec->ops = &ec_ops; if (CONFIG(DRIVERS_INTEL_DPTF)) - ec_fill_dptf_helpers(ec); + ec_fill_dptf_helpers(ec, dev); fill_ssdt_typec_device(dev); fill_ssdt_ps2_keyboard(dev); diff --git a/src/ec/google/chromeec/ec_dptf_helpers.c b/src/ec/google/chromeec/ec_dptf_helpers.c index 1238bcfbec..c44138f587 100644 --- a/src/ec/google/chromeec/ec_dptf_helpers.c +++ b/src/ec/google/chromeec/ec_dptf_helpers.c @@ -3,7 +3,9 @@ #include #include #include +#include +#include "chip.h" /* * The Chrome EC is typically in charge of many system functions, including battery charging and * fan PWM control. This places it in the middle of a DPTF implementation and therefore, many of @@ -22,6 +24,19 @@ enum { EC_FAN_DUTY_AUTO = 0xFF, }; +/* Return the fan number as a string for the FAN participant */ +static const char *fan_num_namestring_of(enum dptf_participant participant) +{ + switch (participant) { + case DPTF_FAN: + return "FAN0"; + case DPTF_FAN_2: + return "FAN1"; + default: + return ""; + } +} + static void write_charger_PPPC(const struct device *ec) { acpigen_write_method_serialized("PPPC", 0); @@ -91,7 +106,7 @@ static void write_charger_SPPC(const struct device *ec) acpigen_pop_len(); /* Method */ } -static void write_fan_fst(const struct device *ec) +static void write_fan_fst(const struct device *ec, int participant) { /* TFST is a package that is used to store data from FAND */ acpigen_write_name("TFST"); @@ -110,7 +125,7 @@ static void write_fan_fst(const struct device *ec) acpigen_write_integer(1); acpigen_emit_byte(ZERO_OP); /* 3rd arg to Index */ acpigen_write_store(); - acpigen_emit_namestring(acpi_device_path_join(ec, "FAN0")); + acpigen_emit_namestring(acpi_device_path_join(ec, fan_num_namestring_of(participant))); acpigen_emit_byte(INDEX_OP); acpigen_emit_namestring("TFST"); acpigen_write_integer(2); @@ -300,11 +315,11 @@ static void write_charger_methods(const struct device *ec) acpigen_pop_len(); /* Scope */ } -static void write_fan_methods(const struct device *ec) +static void write_fan_methods(const struct device *ec, int participant) { - dptf_write_scope(DPTF_FAN); + dptf_write_scope(participant); write_fan_fsl(ec); - write_fan_fst(ec); + write_fan_fst(ec, participant); acpigen_pop_len(); /* Scope */ } @@ -352,14 +367,20 @@ static void write_thermal_methods(const struct device *ec, enum dptf_participant acpigen_pop_len(); /* Scope */ } -void ec_fill_dptf_helpers(const struct device *ec) +void ec_fill_dptf_helpers(const struct device *ec, const struct device *fan_dev) { enum dptf_participant p; int i; + struct ec_google_chromeec_config *config = fan_dev->chip_info; write_dppm_methods(ec); write_charger_methods(ec); - write_fan_methods(ec); + + if (config->ec_multifan_support) { + for (p = DPTF_FAN; p <= DPTF_FAN_2; ++p) + write_fan_methods(ec, p); + } else + write_fan_methods(ec, DPTF_FAN); for (p = DPTF_TEMP_SENSOR_0, i = 0; p <= DPTF_TEMP_SENSOR_4; ++p, ++i) write_thermal_methods(ec, p, i); diff --git a/src/ec/google/common/dptf.h b/src/ec/google/common/dptf.h index a59ee0b6bb..b752a83549 100644 --- a/src/ec/google/common/dptf.h +++ b/src/ec/google/common/dptf.h @@ -6,6 +6,6 @@ #include /* Called by google_chromeec_fill_ssdt_generator */ -void ec_fill_dptf_helpers(const struct device *dev); +void ec_fill_dptf_helpers(const struct device *dev, const struct device *fan_dev); #endif /* EC_GOOGLE_COMMON_DPTF_H */ diff --git a/src/include/acpi/acpigen_dptf.h b/src/include/acpi/acpigen_dptf.h index 6ef87a4ee6..31164bfa91 100644 --- a/src/include/acpi/acpigen_dptf.h +++ b/src/include/acpi/acpigen_dptf.h @@ -14,12 +14,15 @@ #define DPTF_DEVICE_PATH "\\_SB.DPTF" #define TCPU_SCOPE "\\_SB.PCI0" +#define DPTF_MAX_FAN_PARTICIPANTS 2 + /* List of available participants (i.e., they can participate in policies) */ enum dptf_participant { DPTF_NONE, DPTF_CPU, DPTF_CHARGER, DPTF_FAN, + DPTF_FAN_2, DPTF_TEMP_SENSOR_0, DPTF_TEMP_SENSOR_1, DPTF_TEMP_SENSOR_2, @@ -52,6 +55,8 @@ enum { /* Active Policy */ struct dptf_active_policy { + /* The device that can be throttled */ + enum dptf_participant source; /* Device capable of being affected by the fan */ enum dptf_participant target; /* Source's contribution to the Target's cooling capability as a percentage */ @@ -115,6 +120,18 @@ struct dptf_fan_perf { uint16_t power; }; +/* Different levels of fan activity, chosen by active policies */ +struct dptf_multifan_perf { + /* Fan percentage level */ + uint8_t percent; + /* Fan speed, in RPM */ + uint16_t speed; + /* Noise level, in 0.1 dBs */ + uint16_t noise_level; + /* Power in mA */ + uint16_t power; +}; + /* Running Average Power Limits (RAPL) */ struct dptf_power_limit_config { /* Minimum level of power limit, in mW */ @@ -151,7 +168,8 @@ void dptf_write_enabled_policies(const struct dptf_active_policy *active_policie * 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); +void dptf_write_active_policies(const struct dptf_active_policy *policies, int max_count, + bool dptf_multifan_support); /* * This function uses the definition of the passive policies to write out _PSV Methods on all @@ -183,7 +201,16 @@ void dptf_write_charger_perf(const struct dptf_charger_perf *perf, int max_count * 4) The corresponding active cooling trip point (from _ART) (typically left as * DPTF_FIELD_UNUSED). */ -void dptf_write_fan_perf(const struct dptf_fan_perf *perf, int max_count); +void dptf_write_fan_perf(const struct dptf_fan_perf *perf, int max_count, + enum dptf_participant participant); + +void dptf_write_multifan_perf( + const struct dptf_multifan_perf + states[DPTF_MAX_FAN_PARTICIPANTS][DPTF_MAX_FAN_PERF_STATES], + int max_count, enum dptf_participant participant, int fan_num); + +int dptf_write_fan_perf_fps(uint8_t percent, uint16_t power, uint16_t speed, + uint16_t noise_level); /* * This function writes out a PPCC table, which indicates power ranges that different Intel