summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRichard Yeh <rcy@google.com>2022-11-28 02:02:16 +0000
committerChromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com>2022-12-13 07:14:15 +0000
commit62c3a651befeb8d9b3ad03994b23b579d92c20d8 (patch)
treebfb65fa429c7a78fadfa5c968fc84dd351d703e2
parent802a90e2e991129c0d9ff2b4b1d8f6bd36f0a492 (diff)
downloadchrome-ec-62c3a651befeb8d9b3ad03994b23b579d92c20d8.tar.gz
fan: Rewrite and test the most common custom fan_percent_to_rpm.
temp_ratio_to_rpm_hysteresis uses a sorted fan_table containing a mapping of temp_ratio (percent, 0-100) to fan rpm. This clarifies the relationship of temperature to fan speed, while reducing the number of static variables. The existing temp_ratio expressing the percentage of cooling needed (temperature from temp_fan_off to temp_fan_max) suggests the possibility that a proportional-integral-derivative (PID) feedback controller might have been considered; but the default implementation is purely linear (mapping to fan speed from min to max) and this stepwise one just enables hysteresis --- different speeds when the board is warming up than when cooling down. The hysteresis attempts to avoid oscillations in temperature and fan speed. This refactoring is in preparation for reusing this implementation for {ambassador, genesis, moonbuggy, scout}. {kalista, berknip, chronicler, dewatt, endeavour, ezkinil, fizz} define CONFIG_FAN_RPM_CUSTOM and use this or nearly this implementation of a fan_percent_to_rpm with hysteresis. {chronicler} also performs boxcar smoothing of the input. {osiris}, which has two fans, and {jinlon, redrix, and other boards with multiple sensors}, use a similar but not exactly the same implementation, and would need additional abstraction. See: https://source.chromium.org/search?q=f:ec%2Fb%20fan_step%20-f:board.c BRANCH=refactor-fan-percent-to-rpm BUG=b:252966838,b:191187610,chromium:1383859 TEST=make -j run-fan && make -j buildall && make -j runhosttests Signed-off-by: Richard Yeh <rcy@google.com> Change-Id: I50ad4d78ac1145f92573a417646c1f57b8945463 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4021951 Reviewed-by: Jeremy Bettis <jbettis@chromium.org> Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org> Code-Coverage: Zoss <zoss-cl-coverage@prod.google.com> Reviewed-by: Abe Levkoy <alevkoy@chromium.org>
-rw-r--r--common/fan.c68
-rw-r--r--include/fan.h64
-rw-r--r--test/fan.c112
3 files changed, 234 insertions, 10 deletions
diff --git a/common/fan.c b/common/fan.c
index ddc15a4288..945c16952a 100644
--- a/common/fan.c
+++ b/common/fan.c
@@ -57,22 +57,80 @@ void fan_set_count(int count)
* the way down to zero because most fans won't turn that slowly, so
* we'll map [1,100] => [FAN_MIN,FAN_MAX], and [0] => "off".
*/
-int fan_percent_to_rpm(int fan, int pct)
+int fan_percent_to_rpm(int fan_index, int temp_ratio)
{
int rpm, max, min;
- if (!pct) {
+ if (temp_ratio <= 0) {
rpm = 0;
} else {
- min = fans[fan].rpm->rpm_min;
- max = fans[fan].rpm->rpm_max;
- rpm = ((pct - 1) * max + (100 - pct) * min) / 99;
+ min = fans[fan_index].rpm->rpm_min;
+ max = fans[fan_index].rpm->rpm_max;
+ rpm = ((temp_ratio - 1) * max + (100 - temp_ratio) * min) / 99;
}
return rpm;
}
#endif /* CONFIG_FAN_RPM_CUSTOM */
+int temp_ratio_to_rpm_hysteresis(const struct fan_step_1_1 *fan_table,
+ int num_fan_levels, int fan_index,
+ int temp_ratio, void (*on_change)(void))
+{
+ static int previous_temp_ratio;
+ const int previous_rpm = fan_get_rpm_target(FAN_CH(fan_index));
+ int rpm;
+
+ if (temp_ratio <= fan_table[0].decreasing_temp_ratio_threshold) {
+ rpm = 0;
+ } else if (previous_rpm == 0 &&
+ temp_ratio < fan_table[0].increasing_temp_ratio_threshold) {
+ rpm = 0;
+ } else {
+ /*
+ * Comparing temp_ratio and previous_temp_ratio, trichotomy:
+ * 1. decreasing path. (check the decreasing threshold)
+ * 2. increasing path. (check the increasing threshold)
+ * 3. invariant path. (return the current RPM)
+ */
+ if (temp_ratio < previous_temp_ratio) {
+ int i = num_fan_levels - 1;
+
+ while (i > 0 &&
+ fan_table[i].decreasing_temp_ratio_threshold >=
+ temp_ratio) {
+ i--;
+ }
+ rpm = fan_table[i].rpm;
+ } else if (temp_ratio > previous_temp_ratio) {
+ int i = 0;
+
+ while (i < num_fan_levels &&
+ fan_table[i].increasing_temp_ratio_threshold <=
+ temp_ratio) {
+ i++;
+ }
+ if (i > 0) {
+ i--;
+ }
+ rpm = fan_table[i].rpm;
+ } else {
+ rpm = previous_rpm;
+ }
+ }
+
+ previous_temp_ratio = temp_ratio;
+
+ if (rpm != previous_rpm) {
+ cprints(CC_THERMAL, "Setting fan %d RPM to %d", fan_index, rpm);
+ if (on_change) {
+ on_change();
+ }
+ }
+
+ return rpm;
+}
+
/* The thermal task will only call this function with pct in [0,100]. */
test_mockable void fan_set_percent_needed(int fan, int pct)
{
diff --git a/include/fan.h b/include/fan.h
index 7c64f4e31b..25068a653b 100644
--- a/include/fan.h
+++ b/include/fan.h
@@ -68,6 +68,18 @@ extern const struct fan_t fans[];
/* For convenience */
#define FAN_CH(fan) fans[fan].conf->ch
+/* Calculate temp_ratio as a macro. common/thermal.c defines the same
+ * function, but it cannot be used at file scope.
+ */
+#define THERMAL_FAN_PERCENT(low, high, cur) \
+ (((low) < (cur) && (cur) < (high)) ? \
+ (100 * ((cur) - (low)) / ((high) - (low))) : \
+ ((cur) <= (low) ? 0 : 100))
+/* Convert a temperature in centigrade to a temp_ratio, assuming constants
+ * temp_fan_off, temp_fan_max, already in Kelvin. Helpful for fan tables.
+ */
+#define TEMP_TO_RATIO(temp_c) \
+ (THERMAL_FAN_PERCENT((temp_fan_off), (temp_fan_max), (C_TO_K(temp_c))))
/**
* Set the amount of active cooling needed. The thermal control task will call
@@ -79,15 +91,57 @@ extern const struct fan_t fans[];
void fan_set_percent_needed(int fan, int pct);
/**
- * This function translates the percentage of cooling needed into a target RPM.
+ * Convert temp_ratio (temperature as a percentage of the ec_thermal_config
+ * .temp_fan_off to .temp_fan_max range, also cooling effort needed) into a
+ * target fan RPM.
* The default implementation should be sufficient for most needs, but
* individual boards may provide a custom version if needed (see config.h).
*
- * @param fan Fan number (index into fans[])
- * @param pct Percentage of cooling effort needed (always in [0,100])
- * Return Target RPM for fan
+ * @param fan Fan number (index into fans[])
+ * @param temp_ratio Temperature as fraction of temp_fan_off to
+ * temp_fan_max range, expressed as a percent ([0,100]).
+ * Return Target RPM for fan
+ */
+int fan_percent_to_rpm(int fan, int temp_ratio);
+/* Data structure to hold a tuple of parameters for one sensor and one fan. */
+struct fan_step_1_1 {
+ /* lowest temp_ratio (exclusive) to apply this rpm when decreasing.
+ * Use this rpm until temp_ratio falls to or below this threshold.
+ */
+ int decreasing_temp_ratio_threshold;
+ /* lowest temp_ratio (inclusive) to apply this rpm when increasing.
+ * Use this rpm when temp_ratio exceeds this threshold.
+ */
+ int increasing_temp_ratio_threshold;
+ int rpm;
+};
+/**
+ * Convert temp_ratio (temperature as a percentage of the ec_thermal_config
+ * .temp_fan_off to .temp_fan_max range) into a target fan RPM.
+ *
+ * This function adapts the most popular custom version of fan_percent_to_rpm,
+ * which provides hysteresis to reduce temperature/fan speed oscillations.
+ *
+ * To refactor to this, convert the fan_step-based fan_table to fan_step_1_1 by
+ * removing the first (.rpm = 0) element and using
+ * decreasing/increasing_temp_ratio_threshold for off/on respectively.
+ * See example in ../test/fan.c.
+ *
+ * @param fan_table Pointer to ordered array of fan_step_1_1 structs.
+ * There is no need to have any element with .rpm = 0.
+ * Function assumes 0 when temp_ratio is below the
+ * thresholds in the index-0 element.
+ * @param num_fan_levels Size of fan_table
+ * @param fan_index Fan number (index into fans[])
+ * @param temp_ratio Temperature as fraction of temp_fan_off to
+ * temp_fan_max range, expressed as a percent ([0,100]).
+ * @param on_change Pointer to function to be run when the target fan
+ * rpm changes, such as ezkinil board_print_temps().
+ * Return Target RPM for fan
*/
-int fan_percent_to_rpm(int fan, int pct);
+int temp_ratio_to_rpm_hysteresis(const struct fan_step_1_1 *fan_table,
+ int num_fan_levels, int fan_index,
+ int temp_ratio, void (*on_change)(void));
/**
* These functions require chip-specific implementations.
diff --git a/test/fan.c b/test/fan.c
index 76c3208cc6..48a019382b 100644
--- a/test/fan.c
+++ b/test/fan.c
@@ -104,9 +104,121 @@ static int test_fan(void)
return EC_SUCCESS;
}
+/* Provide a test driver to make test easier to read. */
+int temp_to_rpm(int temperature_c)
+{
+ const int temp_fan_off = C_TO_K(35);
+ const int temp_fan_max = C_TO_K(55);
+ const struct fan_step_1_1 fan_table[] = {
+ { .decreasing_temp_ratio_threshold = TEMP_TO_RATIO(35),
+ .increasing_temp_ratio_threshold = TEMP_TO_RATIO(41),
+ .rpm = 2500 },
+ { .decreasing_temp_ratio_threshold = TEMP_TO_RATIO(37),
+ .increasing_temp_ratio_threshold = TEMP_TO_RATIO(43),
+ .rpm = 3200 },
+ { .decreasing_temp_ratio_threshold = TEMP_TO_RATIO(42),
+ .increasing_temp_ratio_threshold = TEMP_TO_RATIO(45),
+ .rpm = 3500 },
+ { .decreasing_temp_ratio_threshold = TEMP_TO_RATIO(44),
+ .increasing_temp_ratio_threshold = TEMP_TO_RATIO(47),
+ .rpm = 3900 },
+ { .decreasing_temp_ratio_threshold = TEMP_TO_RATIO(46),
+ .increasing_temp_ratio_threshold = TEMP_TO_RATIO(49),
+ .rpm = 4500 },
+ { .decreasing_temp_ratio_threshold = TEMP_TO_RATIO(48),
+ .increasing_temp_ratio_threshold = TEMP_TO_RATIO(52),
+ .rpm = 5100 },
+ { .decreasing_temp_ratio_threshold = TEMP_TO_RATIO(51),
+ .increasing_temp_ratio_threshold = TEMP_TO_RATIO(55),
+ .rpm = 5400 },
+ };
+ const int num_fan_levels = ARRAY_SIZE(fan_table);
+ int temp_ratio = TEMP_TO_RATIO(temperature_c);
+
+ int rpm = temp_ratio_to_rpm_hysteresis(fan_table, num_fan_levels, 0,
+ temp_ratio, NULL);
+
+ fan_set_rpm_target(FAN_CH(0), rpm);
+ return rpm;
+}
+
+static int test_temp_ratio_to_rpm_hysteresis(void)
+{
+ const int ZERO = 0;
+ /* set initial value to be different so that a log message appears */
+ fan_set_rpm_target(FAN_CH(0), 5400);
+ /* initial turn-on behavior, ramp up. @ represents fan speed; + temp */
+ TEST_ASSERT(temp_to_rpm(30) == ZERO); /* @+. . 40 . 50 .60 */
+ TEST_ASSERT(temp_to_rpm(30) == ZERO); /* @+. . . . . . */
+ TEST_ASSERT(temp_to_rpm(35) == ZERO); /* @ + . . . . */
+ TEST_ASSERT(temp_to_rpm(37) == ZERO); /* @ . + . . . . */
+ TEST_ASSERT(temp_to_rpm(39) == ZERO); /* @ . +. . . . */
+ TEST_ASSERT(temp_to_rpm(40) == ZERO); /* @ . + . . . */
+ TEST_ASSERT(temp_to_rpm(41) == 2500); /* @. .+ . . . */
+ TEST_ASSERT(temp_to_rpm(36) == 2500); /* @.+ . . . . */
+ TEST_ASSERT(temp_to_rpm(42) == 2500); /* @. . + . . . */
+ TEST_ASSERT(temp_to_rpm(43) == 3200); /* @ . + . . . */
+ TEST_ASSERT(temp_to_rpm(38) == 3200); /* @ + . . . . */
+ TEST_ASSERT(temp_to_rpm(44) == 3200); /* @ . +. . . */
+ TEST_ASSERT(temp_to_rpm(45) == 3500); /* .@ . + . . */
+ TEST_ASSERT(temp_to_rpm(43) == 3500); /* .@ . + . . . */
+ TEST_ASSERT(temp_to_rpm(46) == 3500); /* .@ . .+ . . */
+ TEST_ASSERT(temp_to_rpm(47) == 3900); /* . @ . . + . . */
+ TEST_ASSERT(temp_to_rpm(45) == 3900); /* . @ . + . . */
+ TEST_ASSERT(temp_to_rpm(48) == 3900); /* . @ . . + . . */
+ TEST_ASSERT(temp_to_rpm(49) == 4500); /* . @ . . +. . */
+ TEST_ASSERT(temp_to_rpm(47) == 4500); /* . @ . . + . . */
+ TEST_ASSERT(temp_to_rpm(51) == 4500); /* . @ . . .+ . */
+ TEST_ASSERT(temp_to_rpm(52) == 5100); /* . @. . . + . */
+ TEST_ASSERT(temp_to_rpm(49) == 5100); /* . @. . +. . */
+ TEST_ASSERT(temp_to_rpm(54) == 5100); /* . @. . . +. */
+ TEST_ASSERT(temp_to_rpm(55) == 5400); /* . @ . . + */
+ TEST_ASSERT(temp_to_rpm(52) == 5400); /* . @ . . + . */
+ TEST_ASSERT(temp_to_rpm(60) == 5400); /* . @ . 50 ..+ */
+ /* cool-down */
+ TEST_ASSERT(temp_to_rpm(55) == 5400); /* . @ . . + */
+ TEST_ASSERT(temp_to_rpm(52) == 5400); /* . @ . . + . */
+ TEST_ASSERT(temp_to_rpm(51) == 5100); /* . @. . .+ . */
+ TEST_ASSERT(temp_to_rpm(54) == 5100); /* . @. . . +. */
+ TEST_ASSERT(temp_to_rpm(49) == 5100); /* . @. . +. . */
+ TEST_ASSERT(temp_to_rpm(48) == 4500); /* . @ . . + . . */
+ TEST_ASSERT(temp_to_rpm(51) == 4500); /* . @ . . .+ . */
+ TEST_ASSERT(temp_to_rpm(47) == 4500); /* . @ . . + . . */
+ TEST_ASSERT(temp_to_rpm(46) == 3900); /* . @ . .+ . . */
+ TEST_ASSERT(temp_to_rpm(48) == 3900); /* . @ . . + . . */
+ TEST_ASSERT(temp_to_rpm(45) == 3900); /* . @ . + . . */
+ TEST_ASSERT(temp_to_rpm(44) == 3500); /* .@ . +. . . */
+ TEST_ASSERT(temp_to_rpm(46) == 3500); /* .@ . .+ . . */
+ TEST_ASSERT(temp_to_rpm(43) == 3500); /* .@ . + . . . */
+ TEST_ASSERT(temp_to_rpm(42) == 3200); /* @ . + . . . */
+ TEST_ASSERT(temp_to_rpm(44) == 3200); /* @ . +. . . */
+ TEST_ASSERT(temp_to_rpm(38) == 3200); /* @ + . . . . */
+ TEST_ASSERT(temp_to_rpm(37) == 2500); /* @. + . . . . */
+ TEST_ASSERT(temp_to_rpm(42) == 2500); /* @. . + . . . */
+ TEST_ASSERT(temp_to_rpm(36) == 2500); /* @.+ . . . . */
+ TEST_ASSERT(temp_to_rpm(35) == ZERO); /* @ + 40 . 50 . */
+ /* warm up again */
+ TEST_ASSERT(temp_to_rpm(38) == ZERO); /* @ . + . . . . */
+ /* jumping */
+ TEST_ASSERT(temp_to_rpm(46) == 3500); /* .@ . .+ . . */
+ TEST_ASSERT(temp_to_rpm(36) == 2500); /* @.+ . . . . */
+ TEST_ASSERT(temp_to_rpm(35) == ZERO); /* @ + . . . . */
+ TEST_ASSERT(temp_to_rpm(37) == ZERO); /* @ . + . . . . */
+ TEST_ASSERT(temp_to_rpm(46) == 3500); /* .@ . .+ . . */
+ TEST_ASSERT(temp_to_rpm(54) == 5100); /* . @. . . +. */
+ TEST_ASSERT(temp_to_rpm(55) == 5400); /* . @ . . + */
+ TEST_ASSERT(temp_to_rpm(60) == 5400); /* . @ . . ..+ */
+ TEST_ASSERT(temp_to_rpm(53) == 5400); /* . @ . . + . */
+ TEST_ASSERT(temp_to_rpm(46) == 3900); /* . @ . .+ . . */
+ TEST_ASSERT(temp_to_rpm(30) == ZERO); /* @+. . 40 . 50 . */
+
+ return EC_SUCCESS;
+}
+
void run_test(int argc, const char **argv)
{
RUN_TEST(test_fan);
+ RUN_TEST(test_temp_ratio_to_rpm_hysteresis);
test_print_result();
}