Skip to content

Commit

Permalink
Revert "drivers/intel/dptf: Add multiple fan support under dptf"
Browse files Browse the repository at this point in the history
This reverts commit 672bd9b.

Reason for revert: Gmeet resolution dropped. When system starts
Gmeet video call, it uses the hardware accelerated encoder as per
the expectation. But, as soon as another system connects to the call,
the immediate fallback observed from hardware to software encoder.
Due to this, Gmeet resolution dropped from 720p to 180p.
Currently, this issue observed on AlderLake-N SoC based fanless
platforms. This issue is not seen on fan based systems.

BUG=b:246535768,b:235254828
BRANCH=None
TEST=Built and tested on Alderlake-N systems. With this revert
Gmeet resolution drop not observed.

Change-Id: Idaeaeaed47be44166a7cba9a0a1fac50d2688e50
Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/68568
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Baieswara Reddy Sagili <baieswara.reddy.sagili@intel.com>
Reviewed-by: V Sowmya <v.sowmya@intel.com>
  • Loading branch information
sumeetpawnikar authored and felixheld committed Oct 20, 2022
1 parent 58a38af commit 4dba71f
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 205 deletions.
84 changes: 18 additions & 66 deletions src/acpi/acpigen_dptf.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ 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:
Expand Down Expand Up @@ -125,7 +123,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, bool dptf_multifan_support)
int max_count)
{
char *pkg_count;
int i, j;
Expand Down Expand Up @@ -156,11 +154,7 @@ 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);
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(DPTF_FAN));
acpigen_emit_namestring(path_of(policies[i].target));
acpigen_write_integer(DEFAULT_IF_0(policies[i].weight, DEFAULT_WEIGHT));

Expand Down Expand Up @@ -211,10 +205,9 @@ 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, bool dptf_multifan_support)
void dptf_write_active_policies(const struct dptf_active_policy *policies, int max_count)
{
write_active_relationship_table(policies, max_count, dptf_multifan_support);
write_active_relationship_table(policies, max_count);
write_active_cooling_methods(policies, max_count);
}

Expand Down Expand Up @@ -359,78 +352,37 @@ void dptf_write_charger_perf(const struct dptf_charger_perf *states, int max_cou
acpigen_pop_len(); /* Scope */
}

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)
void dptf_write_fan_perf(const struct dptf_fan_perf *states, int max_count)
{
char *pkg_count;
int i;

if (!max_count || !states[0].percent)
return;

dptf_write_scope(participant);
dptf_write_scope(DPTF_FAN);

/* _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[i].percent, states[i].power,
states[i].speed, states[i].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 (!states[i].percent && !states[i].power)
break;
}

acpigen_pop_len(); /* Package */
acpigen_pop_len(); /* Scope */
}

void dptf_write_multifan_perf(
const struct dptf_multifan_perf (*states)[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_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 */
}

acpigen_pop_len(); /* Package */
Expand Down
12 changes: 0 additions & 12 deletions src/drivers/intel/dptf/chip.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ 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;

Expand All @@ -46,14 +44,6 @@ 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
Expand All @@ -72,8 +62,6 @@ struct drivers_intel_dptf_config {

/* Rest of platform Power */
uint32_t prop;

bool dptf_multifan_support;
};

#endif /* _DRIVERS_INTEL_DPTF_CHIP_H_ */
83 changes: 16 additions & 67 deletions src/drivers/intel/dptf/dptf.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
/* 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,
Expand All @@ -23,7 +22,6 @@ 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

Expand Down Expand Up @@ -53,26 +51,12 @@ 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 || participant == DPTF_FAN_2) &&
config->policies.active[0].target != DPTF_NONE)
if (participant == DPTF_FAN && 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";
Expand Down Expand Up @@ -131,20 +115,15 @@ static void write_tcpu(const struct device *pci_dev,
acpigen_pop_len(); /* TCPU Scope */
}

/* \_SB.DPTF.TFNx */
/* \_SB.DPTF.TFN1 */
static void write_fan(const struct drivers_intel_dptf_config *config,
const struct dptf_platform_info *platform_info,
enum dptf_participant participant)
const struct dptf_platform_info *platform_info)
{
static int fan_uid = 0;

acpigen_write_device(fan_namestring_of(participant));
acpigen_write_device("TFN1");
acpigen_write_name("_HID");
dptf_write_hid(platform_info->use_eisa_hids, platform_info->fan_hid);
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(ACPI_STATUS_DEVICE_ALL_ON);
acpigen_write_name_integer("_UID", 0);
acpigen_write_STA(get_STA_value(config, DPTF_FAN));
acpigen_pop_len(); /* Device */
}

Expand Down Expand Up @@ -459,7 +438,6 @@ 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;
Expand All @@ -472,13 +450,7 @@ static void write_device_definitions(const struct device *dev)
config = config_of(dev);
write_tcpu(parent, config);
write_open_dptf_device(dev, 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_fan(config, platform_info);
write_oem_variables(config);
write_imok();
write_generic_devices(config, platform_info);
Expand All @@ -504,7 +476,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, config->dptf_multifan_support);
DPTF_MAX_ACTIVE_POLICIES);

dptf_write_passive_policies(config->policies.passive,
DPTF_MAX_PASSIVE_POLICIES);
Expand All @@ -516,46 +488,23 @@ 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);

/* 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_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_num;
int i;

/* 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 */
}
/* 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 */

/* TSR options */
for (p = DPTF_TEMP_SENSOR_0, i = 0; p <= DPTF_TEMP_SENSOR_4; ++p, ++i) {
Expand Down
1 change: 0 additions & 1 deletion src/ec/google/chromeec/acpi/emem.asl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ 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),
Expand Down
1 change: 0 additions & 1 deletion src/ec/google/chromeec/chip.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ 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 */
2 changes: 1 addition & 1 deletion src/ec/google/chromeec/ec_acpi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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, dev);
ec_fill_dptf_helpers(ec);

fill_ssdt_typec_device(dev);
fill_ssdt_ps2_keyboard(dev);
Expand Down

0 comments on commit 4dba71f

Please sign in to comment.