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>

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();
 }