J616s: drive the fans from the die temperatures (thermal cooling devices)

Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml

@@ -56,6 +56,15 @@ properties:
       The cell is the zero-based index of a temperature channel, counting
       the temperature-* child nodes in order.
 
+  "#cooling-cells":
+    const: 3
+    description:
+      Makes the fans cooling devices for thermal zones. The first cell is the
+      zero-based index of a fan, counting the fan-* child nodes in order; the
+      other two are the lower and upper cooling state. State 0 leaves the fan
+      to the SMC; states 1 to 8 run it at speeds spread evenly from its
+      minimum to its maximum. A fan needs all four of its speed and mode keys.
+
 patternProperties:
   "^current-[A-Za-z0-9]{4}$":
     $ref: "#/$defs/sensor"

arch/arm64/boot/dts/apple/t6040-j616s.dts

@@ -314,23 +314,59 @@
 	 * failure of that loop; they never act in normal use.
 	 *
 	 * The GPU has no cooling device of its own: its firmware sets the GPU
-	 * performance state and starts limiting at 101 C. The SMC runs the
-	 * fans in its own mode (3), only at their minimum speed in every load
-	 * measured under Linux, and refuses manual fan modes from the OS. So
-	 * when a GPU die sensor reaches the GPU firmware's limit, the seven
+	 * performance state and starts limiting at 101 C. So when a GPU die
+	 * sensor reaches the GPU firmware's limit, the seven
 	 * hottest ones (Tg05, Tg0S, Tg0Y, Tg0k, Tg0z, Tg1V, Tg1l) also shed
 	 * CPU power through the CPU clusters, from the same 101 C, leaving the
 	 * shared heat budget to the GPU. They shut down at 118 C. While the
 	 * GPU is powered off its sensors report placeholder values from -4 to
 	 * 4 C.
+	 *
+	 * The fans: left to themselves (SMC mode 3, with no OS asking for
+	 * cooling) they stay off, or switch on late and ramp slowly, while the
+	 * dies heat up under load. So the CPU die zone and the GPU zones also
+	 * run both fans through a ladder of active trips; a fan takes the
+	 * highest state any zone asks for and goes back to the SMC once no
+	 * zone asks for anything. States 1-8 of a fan run it from its minimum
+	 * (1350 rpm) to its maximum (5777 rpm) in even steps.
 	 */
+#define J616S_FAN_TRIP(label, node, temp, hyst)	\
+	label: node {					\
+		temperature = <temp>;			\
+		hysteresis = <hyst>;			\
+		type = "active";			\
+	};
+
+#define J616S_FAN_TRIPS(zone)					\
+	J616S_FAN_TRIP(zone##_fan1, trip-fan1, 75000, 10000)	\
+	J616S_FAN_TRIP(zone##_fan2, trip-fan2, 80000, 5000)	\
+	J616S_FAN_TRIP(zone##_fan3, trip-fan3, 85000, 5000)	\
+	J616S_FAN_TRIP(zone##_fan4, trip-fan4, 90000, 5000)	\
+	J616S_FAN_TRIP(zone##_fan5, trip-fan5, 95000, 5000)
+
+#define J616S_FAN_MAP(trip_label, node, state)			\
+	node {							\
+		trip = <&trip_label>;				\
+		cooling-device = <&smc_hwmon 0 state state>,	\
+				 <&smc_hwmon 1 state state>;	\
+	};
+
+#define J616S_FAN_MAPS(zone)				\
+	J616S_FAN_MAP(zone##_fan1, map-fan1, 1)		\
+	J616S_FAN_MAP(zone##_fan2, map-fan2, 2)		\
+	J616S_FAN_MAP(zone##_fan3, map-fan3, 3)		\
+	J616S_FAN_MAP(zone##_fan4, map-fan4, 5)		\
+	J616S_FAN_MAP(zone##_fan5, map-fan5, 8)
+
 	thermal-zones {
 		cpu-die-thermal {
 			polling-delay-passive = <1000>;
 			polling-delay = <1000>;
 			thermal-sensors = <&smc_hwmon 0>;	/* TCMz */
 
 			trips {
+				J616S_FAN_TRIPS(cpu_die)
+
 				cpu_die_passive: cpu-die-passive {
 					temperature = <113000>;
 					hysteresis = <5000>;
@@ -345,6 +381,8 @@
 			};
 
 			cooling-maps {
+				J616S_FAN_MAPS(cpu_die)
+
 				map-e {
 					trip = <&cpu_die_passive>;
 					cooling-device =
@@ -890,6 +928,8 @@
 			thermal-sensors = <&smc_hwmon 17>;	/* Tg05 */
 
 			trips {
+				J616S_FAN_TRIPS(gpu0)
+
 				gpu0_passive: trip-passive {
 					temperature = <101000>;
 					hysteresis = <5000>;
@@ -904,6 +944,8 @@
 			};
 
 			cooling-maps {
+				J616S_FAN_MAPS(gpu0)
+
 				map-e {
 					trip = <&gpu0_passive>;
 					cooling-device =
@@ -941,6 +983,8 @@
 			thermal-sensors = <&smc_hwmon 18>;	/* Tg0S */
 
 			trips {
+				J616S_FAN_TRIPS(gpu1)
+
 				gpu1_passive: trip-passive {
 					temperature = <101000>;
 					hysteresis = <5000>;
@@ -955,6 +999,8 @@
 			};
 
 			cooling-maps {
+				J616S_FAN_MAPS(gpu1)
+
 				map-e {
 					trip = <&gpu1_passive>;
 					cooling-device =
@@ -992,6 +1038,8 @@
 			thermal-sensors = <&smc_hwmon 19>;	/* Tg0Y */
 
 			trips {
+				J616S_FAN_TRIPS(gpu2)
+
 				gpu2_passive: trip-passive {
 					temperature = <101000>;
 					hysteresis = <5000>;
@@ -1006,6 +1054,8 @@
 			};
 
 			cooling-maps {
+				J616S_FAN_MAPS(gpu2)
+
 				map-e {
 					trip = <&gpu2_passive>;
 					cooling-device =
@@ -1043,6 +1093,8 @@
 			thermal-sensors = <&smc_hwmon 20>;	/* Tg0k */
 
 			trips {
+				J616S_FAN_TRIPS(gpu3)
+
 				gpu3_passive: trip-passive {
 					temperature = <101000>;
 					hysteresis = <5000>;
@@ -1057,6 +1109,8 @@
 			};
 
 			cooling-maps {
+				J616S_FAN_MAPS(gpu3)
+
 				map-e {
 					trip = <&gpu3_passive>;
 					cooling-device =
@@ -1094,6 +1148,8 @@
 			thermal-sensors = <&smc_hwmon 21>;	/* Tg0z */
 
 			trips {
+				J616S_FAN_TRIPS(gpu4)
+
 				gpu4_passive: trip-passive {
 					temperature = <101000>;
 					hysteresis = <5000>;
@@ -1108,6 +1164,8 @@
 			};
 
 			cooling-maps {
+				J616S_FAN_MAPS(gpu4)
+
 				map-e {
 					trip = <&gpu4_passive>;
 					cooling-device =
@@ -1145,6 +1203,8 @@
 			thermal-sensors = <&smc_hwmon 22>;	/* Tg1V */
 
 			trips {
+				J616S_FAN_TRIPS(gpu5)
+
 				gpu5_passive: trip-passive {
 					temperature = <101000>;
 					hysteresis = <5000>;
@@ -1159,6 +1219,8 @@
 			};
 
 			cooling-maps {
+				J616S_FAN_MAPS(gpu5)
+
 				map-e {
 					trip = <&gpu5_passive>;
 					cooling-device =
@@ -1196,6 +1258,8 @@
 			thermal-sensors = <&smc_hwmon 23>;	/* Tg1l */
 
 			trips {
+				J616S_FAN_TRIPS(gpu6)
+
 				gpu6_passive: trip-passive {
 					temperature = <101000>;
 					hysteresis = <5000>;
@@ -1210,6 +1274,8 @@
 			};
 
 			cooling-maps {
+				J616S_FAN_MAPS(gpu6)
+
 				map-e {
 					trip = <&gpu6_passive>;
 					cooling-device =
@@ -3789,13 +3855,21 @@
  * Fan speeds, limits, targets and modes, and the common SMC sensors; every
  * key exists on this machine's SMC. Both fans list their target (F0Tg,
  * F1Tg) and mode (F0Md, F1Md) keys, so with the driver's fan_control
- * parameter fanN_target becomes writable; by default the SMC drives the
- * fans and the attributes are read-only.
+ * parameter fanN_target becomes writable; by default the attributes are
+ * read-only, and the thermal zones above drive the fans.
  */
 #include "hwmon-common.dtsi"
 #include "hwmon-fan-dual.dtsi"
 #include "hwmon-laptop.dtsi"
 
+/*
+ * Both fans are cooling devices of the CPU die and GPU zones, as
+ * <&smc_hwmon fan-index lower upper>.
+ */
+&smc_hwmon {
+	#cooling-cells = <3>;
+};
+
 &pcie_dart {
 	status = "okay";
 };

drivers/hwmon/macsmc-hwmon-test.c

@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only OR MIT
 /*
  * KUnit tests for the Apple SMC hwmon driver: channel numbering of the
- * temperature sensors, which thermal zones depend on, and manual fan control.
+ * temperature sensors, which thermal zones depend on, and manual fan control
+ * by userspace and by the thermal framework.
  *
  * Included by macsmc-hwmon.c.
  */
@@ -217,6 +218,9 @@ static int macsmc_fan_test_init(struct kunit *test)
 	t->hwmon.fan.count = 2;
 	t->hwmon.fan.unlock_key = SMC_KEY(Ftst);
 	mutex_init(&t->hwmon.fan.lock);
+	INIT_WORK(&t->hwmon.fan.cool_work, macsmc_hwmon_fans_cool_work);
+	for (i = 0; i < 2; i++)
+		t->fans[i].hwmon = &t->hwmon;
 	test->priv = t;
 	fan_control = true;
 	kunit_activate_static_stub(test, apple_smc_read, fan_test_read);
@@ -326,13 +330,111 @@ static void macsmc_fan_test_legacy(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, t->refused, 0);
 }
 
+/* Cooling states spread over [min, max]; 0 asks for nothing. */
+static void macsmc_fan_test_state_rpm(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777, 0), 0);
+	KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777, 1), 1350);
+	KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777, 4), 3247);
+	KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777,
+							 MACSMC_FAN_COOLING_STATES), 5777);
+	/* Never past the maximum, whatever the state. */
+	KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777,
+							 MACSMC_FAN_COOLING_STATES + 5), 5777);
+}
+
+/* Cooling needs no fan_control: unlock for a state, release at state 0. */
+static void macsmc_fan_test_cooling(struct kunit *test)
+{
+	struct macsmc_fan_test *t = test->priv;
+
+	fan_control = false;
+	t->fans[0].cool_state = 1;
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	KUNIT_EXPECT_EQ(test, t->ftst, 1);
+	KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_MANUAL);
+	KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F1350);
+	/* The other fan asks for nothing and stays with the firmware. */
+	KUNIT_EXPECT_EQ(test, t->mode[1], MACSMC_FAN_MODE_AUTO);
+	KUNIT_EXPECT_FALSE(test, t->fans[1].manual);
+
+	t->fans[0].cool_state = MACSMC_FAN_COOLING_STATES;
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F5777);
+	KUNIT_EXPECT_EQ(test, t->ftst_writes, 1);
+
+	t->fans[0].cool_state = 0;
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	fan_test_released(test, t);
+}
+
+/* A fan runs at the higher of the userspace target and the cooling state. */
+static void macsmc_fan_test_cooling_and_user(struct kunit *test)
+{
+	struct macsmc_fan_test *t = test->priv;
+
+	KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), 0);
+	t->fans[0].cool_state = 1;
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F2500);
+
+	t->fans[0].cool_state = MACSMC_FAN_COOLING_STATES;
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F5777);
+	/* Userspace cannot take the fan below what cooling asks for. */
+	KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 1350), 0);
+	KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F5777);
+	KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 0), 0);
+	KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_MANUAL);
+
+	t->fans[0].cool_state = 0;
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	fan_test_released(test, t);
+}
+
+/* Once stopped (suspend, shutdown, removal), no state takes a fan back. */
+static void macsmc_fan_test_cooling_stopped(struct kunit *test)
+{
+	struct macsmc_fan_test *t = test->priv;
+
+	t->fans[0].cool_state = 4;
+	t->fans[1].cool_state = 4;
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	KUNIT_EXPECT_EQ(test, t->hwmon.fan.manual, 2);
+
+	macsmc_hwmon_fans_stop(&t->hwmon);
+	fan_test_released(test, t);
+
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	fan_test_released(test, t);
+	KUNIT_EXPECT_EQ(test, t->ftst_writes, 2);
+}
+
+/* A failed unlock leaves the fan with the firmware. */
+static void macsmc_fan_test_cooling_unlock_fails(struct kunit *test)
+{
+	struct macsmc_fan_test *t = test->priv;
+
+	t->fail_key = SMC_KEY(Ftst);
+	t->fans[0].cool_state = MACSMC_FAN_COOLING_STATES;
+	macsmc_hwmon_fans_apply(&t->hwmon);
+	KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_SYSTEM);
+	KUNIT_EXPECT_EQ(test, t->hwmon.fan.manual, 0);
+	KUNIT_EXPECT_FALSE(test, t->hwmon.fan.unlocked);
+}
+
 static struct kunit_case macsmc_fan_test_cases[] = {
 	KUNIT_CASE(macsmc_fan_test_unlock_and_release),
 	KUNIT_CASE(macsmc_fan_test_clamp),
 	KUNIT_CASE(macsmc_fan_test_target_write_fails),
 	KUNIT_CASE(macsmc_fan_test_two_fans),
 	KUNIT_CASE(macsmc_fan_test_no_unlock_key),
 	KUNIT_CASE(macsmc_fan_test_legacy),
+	KUNIT_CASE(macsmc_fan_test_state_rpm),
+	KUNIT_CASE(macsmc_fan_test_cooling),
+	KUNIT_CASE(macsmc_fan_test_cooling_and_user),
+	KUNIT_CASE(macsmc_fan_test_cooling_stopped),
+	KUNIT_CASE(macsmc_fan_test_cooling_unlock_fails),
 	{}
 };