thermald : PID fix inverted-range control and add incremental PID mode

[priyjain1]

Power-limit devices (RAPL, SPEL) use an inverted state range where min_state is the maximum allowed power and max_state is the minimum. This patch fixes PID control for such devices and adds an incremental PID mode.

• Negate PID output for inverted-range devices so a positive error (temp > target) reduces the power limit (thd_cdev.cpp)
• Bypass in_max_state() guard for PID on inverted-range devices so the controller keeps running after hitting max restriction (thd_trip_point.cpp)
• Fix unsigned subtraction wrap-around in pid_output() (thd_pid.cpp)
• Add PID_INCREMENTAL mode: same formula as absolute but applied to curr_state instead of min_state, so the power limit keeps decreasing each poll while temperature stays above the trip threshold
• Extend XML PID gain range from [0, 100] to [0, 1000] to support temperature-to-power mappings (thd_parse.cpp)
• Add incremental XML element to select the mode

[Copilot]

🟡 Not ready to approve

The new inverted-range max-state bypass only covers per-trip PID and the incremental PID anchoring uses potentially-stale get_curr_state() instead of the codebase’s “readback” path, which can break PID behavior for some devices.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR adjusts thermald’s PID control to correctly handle cooling devices whose control ranges are inverted (e.g., power-limit devices where “min_state” is least-restrictive and “max_state” is most-restrictive), and introduces an “incremental” PID mode intended to keep applying corrections while a trip remains active.

Changes:

• Adds PID mode plumbing (absolute vs incremental) through PID controller, XML parsing, and default cooling-device configuration.
• Fixes PID math for temperature below target (avoid unsigned wrap-around) and adds inverted-range handling (including output sign inversion and clamp logic).
• Adjusts trip-point max-state skipping logic to keep PID running for inverted-range devices.

File summaries

File	Description
src/thd_trip_point.h	Initializes per-trip PID mode default (absolute).
src/thd_trip_point.cpp	Keeps PID running at max_state for inverted-range devices; wires PID mode into trip cdev setup/logging.
src/thd_pid.h	Introduces pid_mode_t and stores PID mode in controller/params.
src/thd_pid.cpp	Fixes unsigned wrap-around and implements mode-aware first-call behavior/logging.
src/thd_parse.h	Extends parsed PID config struct to include PID mode.
src/thd_parse.cpp	Parses <PidMode> and widens PID gain range to 0–1000.
src/thd_engine_default.cpp	Updates default cooling-device PID struct initialization; wires pid mode into cdev creation.
src/thd_cdev.h	Adds set_pid_mode() for cdev-level PID.
src/thd_cdev.cpp	Implements inverted-range PID output sign handling + incremental PID application for trip-level and cdev-level PID.

Review details

Suppressed comments (1)

src/thd_cdev.cpp:553

• Same issue as above for cdev-level incremental PID: anchoring the delta to get_curr_state() can be stale for devices that implement get_curr_state(true) as a readback-from-hardware path.

				ret = pid_ctrl.pid_output(temperature, 0);
				if (inverted)
					ret = -ret;
				ret += get_curr_state();
			}

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

src/thd_pid.cpp:88

• pid_output() logs and returns (int)output for steady-state calls as well. Converting an out-of-range double to int is undefined behavior in C++; clamp to INT_MIN/INT_MAX once and reuse the clamped value for logs + return.

	thd_log_debug("pid_%s e:%d kp:%g ki_sum:%g kd:%g out:%d\n",
			mode == PID_INCREMENTAL ? "inc" : "abs",
			error, kp * error, ki * err_sum, kd * d_err, (int)output);

	last_err = error;

src/thd_pid.h:27

• pid_output() will be clamping the computed PID output to an int range (to avoid undefined behavior when casting large doubles). Please add an explicit header for INT_MIN/INT_MAX (or equivalent) so the saturation logic is well-defined across toolchains.

#include "thermald.h"
#include <cstdint>
#include <time.h>

src/thd_engine_default.cpp:68

• There’s a commented-out example initializer for cpu_def_cooling_devices above this array that still uses .pid = {0.0, 0.0, 0.0}. Now that pid_control_t includes a PID mode, that example is stale/misleading; update it to include the default mode for consistency with the real initializer below.

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, PID_ABSOLUTE },"" },