Changes related to PID controller and some man page updates

man/thermal-conf.xml.5

@@ -606,3 +606,73 @@ Example 7: Use RAPL power limits to control.
 </Platform>
 </ThermalConfiguration>
 .EE
+.PP
+.B Example 8:
+AMD Ryzen / k10temp configuration. On AMD platforms, the CPU temperature
+sensor (k10temp) is exposed via hwmon sysfs but does not have an
+associated thermal_zone in /sys/class/thermal. This means thermald
+cannot auto-discover it and the default CPU DTS monitoring will report
+"No coretemp sysfs found" and "No Zones present". A custom sensor must
+be defined with the hwmon path, and the CPU ID check must be bypassed
+using the \fB--ignore-cpuid-check\fP command line option.
+.PP
+To find the correct hwmon path, run:
+.RS 4
+.EX
+find /sys/class/hwmon/ -exec echo -n "{}: " \e; -exec cat {}/name \e;
+.EE
+.RE
+.PP
+Then identify the k10temp entry and its Tctl sensor. For example, if
+k10temp is hwmon2, the Tctl path is
+/sys/class/hwmon/hwmon2/temp1_input.
+.sp 1
+.EX
+<?xml version="1.0"?>
+<ThermalConfiguration>
+  <Platform>
+    <Name>AMD Ryzen (k10temp via hwmon)</Name>
+    <UUID>*</UUID>
+    <ProductName>*</ProductName>
+    <Preference>QUIET</Preference>
+    <ThermalSensors>
+      <ThermalSensor>
+        <!-- Define a custom sensor name and point it at the
+             k10temp hwmon path. The name must NOT match any
+             auto-discovered hwmon driver name (do not use
+             "k10temp" here); use a unique name instead. -->
+        <Type>amd_cpu_temp</Type>
+        <Path>/sys/class/hwmon/hwmon2/temp1_input</Path>
+        <AsyncCapable>0</AsyncCapable>
+      </ThermalSensor>
+    </ThermalSensors>
+    <ThermalZones>
+      <ThermalZone>
+        <Type>cpu</Type>
+        <TripPoints>
+          <TripPoint>
+            <SensorType>amd_cpu_temp</SensorType>
+            <Temperature>85000</Temperature>
+            <type>passive</type>
+            <CoolingDevice>
+              <type>cpufreq</type>
+              <influence>100</influence>
+              <SamplingPeriod>1</SamplingPeriod>
+            </CoolingDevice>
+          </TripPoint>
+          <TripPoint>
+            <SensorType>amd_cpu_temp</SensorType>
+            <Temperature>95000</Temperature>
+            <type>passive</type>
+            <CoolingDevice>
+              <type>cpufreq</type>
+              <influence>100</influence>
+              <SamplingPeriod>1</SamplingPeriod>
+            </CoolingDevice>
+          </TripPoint>
+        </TripPoints>
+      </ThermalZone>
+    </ThermalZones>
+  </Platform>
+</ThermalConfiguration>
+.EE

src/thd_cdev.cpp

@@ -475,10 +475,44 @@ int cthd_cdev::thd_cdev_set_state(int set_point, int target_temp,
 		ret = THD_SUCCESS;
 
 	} else if (pid_param && pid_param->valid) {
-		// Handle PID param unique to a trip
 		pid.set_target_temp(target_temp);
-		ret = pid.pid_output(temperature, get_curr_state(true) - get_min_state());
-		ret += get_min_state();
+		bool inverted = (get_min_state() > get_max_state());
+
+		if (pid_param->mode == PID_INCREMENTAL) {
+			/*
+			 * Incremental PID: same formula as absolute (Kp*e + Ki*∫e + Kd*de/dt)
+			 * but anchored to curr_state instead of min_state.
+			 *
+			 * Active (state=1):
+			 *   new_state = curr_state - pid_output   (inverted range)
+			 *   new_state = curr_state + pid_output   (normal range)
+			 *   → power limit keeps decreasing each poll while temp > target
+			 *
+			 * Deactivating (state=0):
+			 *   Restore to min_state (no restriction) and reset PID.
+			 *   This avoids leaving the power limit partially reduced
+			 *   after the trip threshold is no longer exceeded.
+			 */
+			if (state == 0) {
+				ret = get_min_state();
+			} else {
+				ret = pid.pid_output(temperature, 0);
+				if (inverted)
+					ret = -ret;
+				ret += get_curr_state(true);
+			}
+		} else {
+			/* Absolute PID: output is the desired offset from min_state */
+			int initial_val = get_curr_state(true) - get_min_state();
+			if (inverted)
+				initial_val = -initial_val;
+			ret = pid.pid_output(temperature, initial_val);
+			if (inverted)
+				ret = -ret;
+			ret += get_min_state();
+		}
+
+		/* Clamp to valid state range (handles both normal and inverted) */
 		if (get_min_state() < get_max_state()) {
 			if (ret > get_max_state())
 				ret = get_max_state();
@@ -491,18 +525,41 @@ int cthd_cdev::thd_cdev_set_state(int set_point, int target_temp,
 				ret = get_min_state();
 		}
 		set_curr_state_raw(ret, state);
-		thd_log_info("Set pid : %d, %d, %d, %d, %d\n", set_point, temperature,
-				index, get_curr_state(), max_state);
+		thd_log_info("Set pid(%s%s): set_pt:%d temp:%d cdev:%d(%s) state:%d max:%d\n",
+				pid_param->mode == PID_INCREMENTAL ? "inc" : "abs",
+				inverted ? ",inv" : "",
+				set_point, temperature, index, type_str.c_str(),
+				get_curr_state(), max_state);
 		ret = THD_SUCCESS;
 
 		if (state == 0)
 			pid.reset();
 
 	} else if (pid_enable) {
-		// Handle PID param common to whole cooling device
+		/* Cdev-level PID (enabled via enable_pid() / XML <PidControl> in
+		 * <CoolingDevice> section).  Same Fix 1 + incremental logic as
+		 * the trip-level PID branch above. */
 		pid_ctrl.set_target_temp(target_temp);
-		ret = pid_ctrl.pid_output(temperature);
-		ret += get_min_state();
+		bool inverted = (get_min_state() > get_max_state());
+
+		if (pid_ctrl.get_pid_mode() == PID_INCREMENTAL) {
+			if (state == 0) {
+				ret = get_min_state();
+			} else {
+				ret = pid_ctrl.pid_output(temperature, 0);
+				if (inverted)
+					ret = -ret;
+				ret += get_curr_state(true);
+			}
+		} else {
+			int initial_val = get_curr_state(true) - get_min_state();
+			if (inverted)
+				initial_val = -initial_val;
+			ret = pid_ctrl.pid_output(temperature, initial_val);
+			if (inverted)
+				ret = -ret;
+			ret += get_min_state();
+		}
 
 		if (get_min_state() < get_max_state()) {
 			if (ret > get_max_state())
@@ -517,8 +574,11 @@ int cthd_cdev::thd_cdev_set_state(int set_point, int target_temp,
 		}
 
 		set_curr_state_raw(ret, state);
-		thd_log_info("Set : %d, %d, %d, %d, %d\n", set_point, temperature,
-				index, get_curr_state(), max_state);
+		thd_log_info("Set pid_cdev(%s%s): set_pt:%d temp:%d cdev:%d(%s) state:%d max:%d\n",
+				pid_ctrl.get_pid_mode() == PID_INCREMENTAL ? "inc" : "abs",
+				inverted ? ",inv" : "",
+				set_point, temperature, index, type_str.c_str(),
+				get_curr_state(), max_state);
 		ret = THD_SUCCESS;
 	} else {
 		if (state)

src/thd_cdev.h

@@ -266,10 +266,16 @@ class cthd_cdev {
 		pid_ctrl.kd = kd;
 		thd_log_info("set_pid_param %d [%g.%g,%g]\n", index, kp, ki, kd);
 	}
+	void set_pid_mode(pid_mode_t m) {
+		pid_ctrl.set_pid_mode(m);
+		thd_log_info("set_pid_mode %d [%s]\n", index,
+				m == PID_INCREMENTAL ? "incremental" : "absolute");
+	}
 	void enable_pid() {
 		thd_log_info("PID control enabled %d\n", index);
 		pid_enable = true;
 	}
+	bool is_pid_enabled() const { return pid_enable; }
 
 	void thd_cdev_set_write_prefix(std::string prefix) {
 		write_prefix = std::move(prefix);

src/thd_engine_default.cpp

@@ -65,11 +65,11 @@ static const cooling_dev_t cpu_def_cooling_devices[] = {
 		{ true, CDEV_DEF_BIT_UNIT_VAL
 				| CDEV_DEF_BIT_READ_BACK | CDEV_DEF_BIT_MIN_STATE | CDEV_DEF_BIT_STEP,
 				0, ABSOULUTE_VALUE, 0, 0, 5, false, false, "intel_powerclamp", "", 4,
-				false, { 0.0, 0.0, 0.0 },"" },
+				false, { 0.0, 0.0, 0.0, PID_ABSOLUTE },"" },
 		{ true, CDEV_DEF_BIT_UNIT_VAL
 				| CDEV_DEF_BIT_READ_BACK | CDEV_DEF_BIT_MIN_STATE | CDEV_DEF_BIT_STEP,
 				0, ABSOULUTE_VALUE, 0, 100, 5, false, false, "LCD", "", 4, false, { 0.0,
-				0.0, 0.0 },"" } };
+				0.0, 0.0, PID_ABSOLUTE },"" } };
 
 cthd_engine_default::~cthd_engine_default() {
 }
@@ -688,6 +688,7 @@ int cthd_engine_default::add_replace_cdev(const cooling_dev_t *config) {
 	if (config->mask & CDEV_DEF_BIT_PID_PARAMS) {
 		cdev->enable_pid();
 		cdev->set_pid_param(config->pid.Kp, config->pid.Ki, config->pid.Kd);
+		cdev->set_pid_mode(config->pid.mode);
 	}
 
 	if (config->mask & CDEV_DEF_BIT_WRITE_PREFIX)

src/thd_gddv.cpp

@@ -1358,8 +1358,8 @@ int cthd_gddv::verify_condition(const struct condition& condition) {
 
 	if (condition.condition >= Oem0 && condition.condition <= Oem5)
 		return 0;
-	if (condition.condition >= adaptive_condition(0x1000)
-			&& condition.condition < adaptive_condition(0x10000))
+	if (condition.condition >= adaptive_condition(OEM_CONDITION_BASE_ID)
+			&& condition.condition < adaptive_condition(SW_OEM_CONDITION_BASE_ID))
 		return 0;
 	if (condition.condition == Default)
 		return 0;
@@ -1383,6 +1383,26 @@ int cthd_gddv::verify_condition(const struct condition& condition) {
 	if (condition.condition == OS_type)
 		return 0;
 
+	/*
+	 * Software-OEM conditions are set at runtime by OEM software through
+	 * the DPTF/ESIF interface, which has no equivalent on Linux, so they
+	 * can never be satisfied here. Don't fail verify_conditions() for
+	 * them: that would change engine startup behavior (unsupported
+	 * condition fallback). They are simply excluded from the ODVP
+	 * mapping, so evaluation fails quietly and the containing condition
+	 * set never matches - the same outcome as when they were misread as
+	 * ODVP variables, without the per-poll odvpN read errors.
+	 */
+	if (condition.condition >= adaptive_condition(SW_OEM_CONDITION_BASE_ID)
+			&& condition.condition < adaptive_condition(PARTICIPANT_CONDITION_BASE_ID)) {
+		thd_log_info(
+				"Software-OEM condition %" PRIu64
+				" (SwOem%" PRIu64 ") is set by OEM software via DPTF, not available on Linux; the condition set using it will never match\n",
+				condition.condition,
+				condition.condition - SW_OEM_CONDITION_BASE_ID);
+		return 0;
+	}
+
 	if ( condition.condition >=  ARRAY_SIZE(condition_names))
 		cond_name = "UNKNOWN";
 	else
@@ -1500,9 +1520,9 @@ int cthd_gddv::evaluate_oem_condition(const struct condition& condition) {
 
 	if (condition.condition >= Oem0 && condition.condition <= Oem5)
 		oem_condition = (int) condition.condition - Oem0;
-	else if (condition.condition >= (adaptive_condition) 0x1000
-			&& condition.condition < (adaptive_condition) 0x10000)
-		oem_condition = (int) condition.condition - 0x1000 + 6;
+	else if (condition.condition >= (adaptive_condition) OEM_CONDITION_BASE_ID
+			&& condition.condition < (adaptive_condition) SW_OEM_CONDITION_BASE_ID)
+		oem_condition = (int) condition.condition - OEM_CONDITION_BASE_ID + 6;
 
 	if (oem_condition != -1) {
 		std::string filename = "odvp" + std::to_string(oem_condition);
@@ -1673,8 +1693,8 @@ int cthd_gddv::evaluate_condition(struct condition& condition) {
 	}
 
 	if ((condition.condition >= Oem0 && condition.condition <= Oem5)
-			|| (condition.condition >= (adaptive_condition) 0x1000
-					&& condition.condition < (adaptive_condition) 0x10000))
+			|| (condition.condition >= (adaptive_condition) OEM_CONDITION_BASE_ID
+					&& condition.condition < (adaptive_condition) SW_OEM_CONDITION_BASE_ID))
 		ret = evaluate_oem_condition(condition);
 
 	if (condition.condition == Temperature

src/thd_gddv.h

@@ -100,6 +100,21 @@ enum adaptive_condition : uint32_t { // NOLINT(performance-enum-size)
 	OS_type = 86
 };
 
+/*
+ * Adaptive condition ID ranges, matching Intel DPTF's ConditionType
+ * (OemConditionBaseId / SwOemConditionBaseId / ParticipantConditionBaseId).
+ *
+ * [OEM_CONDITION_BASE_ID, SW_OEM_CONDITION_BASE_ID)
+ *     OEM variables exported by the firmware as odvpN sysfs entries.
+ * [SW_OEM_CONDITION_BASE_ID, PARTICIPANT_CONDITION_BASE_ID)
+ *     Software-OEM conditions set at runtime by OEM software through the
+ *     DPTF/ESIF interface. These have no equivalent on Linux, so thermald
+ *     never matches them instead of misreading them as ODVP variables.
+ */
+#define OEM_CONDITION_BASE_ID		0x1000
+#define SW_OEM_CONDITION_BASE_ID	0x2000
+#define PARTICIPANT_CONDITION_BASE_ID	0x10000
+
 enum adaptive_comparison : uint8_t {
 	ADAPTIVE_EQUAL = 0x01, ADAPTIVE_LESSER_OR_EQUAL, ADAPTIVE_GREATER_OR_EQUAL, ADAPTIVE_NOT_EQUAL,
 };

src/thd_parse.cpp

@@ -246,6 +246,7 @@ int cthd_parse::parse_new_trip_cdev(xmlNode * a_node, xmlDoc *doc,
 					trip_cdev->pid_param.kp = pid_params.Kp;
 					trip_cdev->pid_param.ki = pid_params.Ki;
 					trip_cdev->pid_param.kd = pid_params.Kd;
+					trip_cdev->pid_param.mode = pid_params.mode;
 					trip_cdev->pid_param.valid = 1;
 				}
 				xmlFree(tmp_value);
@@ -309,6 +310,7 @@ int cthd_parse::parse_new_trip_point(xmlNode * a_node, xmlDoc *doc,
 				trip_cdev.pid_param.kp = 0.0;
 				trip_cdev.pid_param.ki = 0.0;
 				trip_cdev.pid_param.kd = 0.0;
+				trip_cdev.pid_param.mode = PID_ABSOLUTE;
 
 				parse_new_trip_cdev(cur_node->children, doc, &trip_cdev);
 				trip_pt->cdev_trips.push_back(trip_cdev);
@@ -380,6 +382,7 @@ int cthd_parse::parse_pid_values(xmlNode * a_node, xmlDoc *doc,
 	pid_ptr->Kp = 0.0005;
 	pid_ptr->Ki = 0.0001;
 	pid_ptr->Kd = 0.0001;
+	pid_ptr->mode = PID_ABSOLUTE;   /* default */
 
 	for (cur_node = a_node; cur_node; cur_node = cur_node->next) {
 		if (cur_node->type == XML_ELEMENT_NODE) {
@@ -389,22 +392,33 @@ int cthd_parse::parse_pid_values(xmlNode * a_node, xmlDoc *doc,
 			if (tmp_value) {
 				if (!thd_strcasecmp_n((const char*) cur_node->name, "Kp")) {
 					double val;
-
-					if (parse_double_value(tmp_value, &val, 0.0, 100.0) == THD_SUCCESS) {
+					/* Extended range: 0–1000 to support power-limit control
+					 * (temperature in millidegrees, power in microwatts). */
+					if (parse_double_value(tmp_value, &val, 0.0, 1000.0) == THD_SUCCESS) {
 						pid_ptr->Kp = val;
 					}
 				} else if (!thd_strcasecmp_n((const char*) cur_node->name, "Kd")) {
 					double val;
-
-					if (parse_double_value(tmp_value, &val, 0.0, 100.0) == THD_SUCCESS) {
+					if (parse_double_value(tmp_value, &val, 0.0, 1000.0) == THD_SUCCESS) {
 						pid_ptr->Kd = val;
 					}
 				} else if (!thd_strcasecmp_n((const char*) cur_node->name, "Ki")) {
 					double val;
-
-					if (parse_double_value(tmp_value, &val, 0.0, 100.0) == THD_SUCCESS) {
+					if (parse_double_value(tmp_value, &val, 0.0, 1000.0) == THD_SUCCESS) {
 						pid_ptr->Ki = val;
 					}
+				} else if (!thd_strcasecmp_n((const char*) cur_node->name,
+						"PidMode")) {
+					/*
+					 * <PidMode>absolute</PidMode>     — absolute PID (default)
+					 * <PidMode>incremental</PidMode>  — incremental PID
+					 */
+					char *mode_val = char_trim(tmp_value);
+					if (mode_val &&
+					    !thd_strcasecmp_n(mode_val, "incremental"))
+						pid_ptr->mode = PID_INCREMENTAL;
+					else
+						pid_ptr->mode = PID_ABSOLUTE;
 				}
 				xmlFree(tmp_value);
 			}

src/thd_parse.h

@@ -51,6 +51,7 @@ typedef struct {
 	double Kp;
 	double Ki;
 	double Kd;
+	pid_mode_t mode;   /* PID_ABSOLUTE (default) or PID_INCREMENTAL */
 } pid_control_t;
 
 typedef struct {