| Message ID | 20230822184940.31316-1-mudeshi1209@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | tools/thermal: tmon: Fix sample data update in PID | expand |
Hi Daniel, Wondering if this patch is being reviewed by you (you are delegated for this patch on Patchwork). I initially only sent this patch to the linux-pm mailing list, but I realised it may not have reached you. I am new to the Linux patch submission procedure so I apologize for any inconvenience. Regards, Meet. On Tue, 22 Aug 2023 at 14:50, Meet Udeshi <mudeshi1209@gmail.com> wrote: > Fixed the order of update for `xk_1` and `xk_2` in the PID controller in > function `controller_handler()`. > > The previous timestep data in the PID controller, `xk_1` and `xk_2`, > were updated in the wrong order. `xk_1` was overwritten before `xk_2` > was assigned. This caused both `xk_1` and `xk_2` to take the value of > `xk` when the function `controller_handler()` was called. > This means the D-term of the PID controller was simplified from > a second-order approximation using two previous timesteps to a > first-order approximation using one previous timestep. > > xk - 2 * xk_1 + xk_2 => xk - xk_1 > > This degraded the performance of the PID controller by making it more > noisy and less accurate. > > This bug was found by reverse engineering the tmon binary using the > [REMaQE tool](https://arxiv.org/abs/2305.06902). > > Signed-off-by: Meet Udeshi <mudeshi1209@gmail.com> > Fixes: 94f69966faf8 ("tools/thermal: Introduce tmon, a tool for thermal subsystem") > --- > tools/thermal/tmon/pid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/thermal/tmon/pid.c b/tools/thermal/tmon/pid.c > index da20088285bd..1c02eb675088 100644 > --- a/tools/thermal/tmon/pid.c > +++ b/tools/thermal/tmon/pid.c > @@ -103,8 +103,8 @@ void controller_handler(const double xk, double *yk) > /* compute output */ > *yk += p_term + i_term + d_term; > /* update sample data */ > - xk_1 = xk; > xk_2 = xk_1; > + xk_1 = xk; > > /* clamp output adjustment range */ > if (*yk < -LIMIT_HIGH) > -- > 2.34.1
diff --git a/tools/thermal/tmon/pid.c b/tools/thermal/tmon/pid.c index da20088285bd..1c02eb675088 100644 --- a/tools/thermal/tmon/pid.c +++ b/tools/thermal/tmon/pid.c @@ -103,8 +103,8 @@ void controller_handler(const double xk, double *yk) /* compute output */ *yk += p_term + i_term + d_term; /* update sample data */ - xk_1 = xk; xk_2 = xk_1; + xk_1 = xk; /* clamp output adjustment range */ if (*yk < -LIMIT_HIGH)
Fixed the order of update for `xk_1` and `xk_2` in the PID controller in function `controller_handler()`. The previous timestep data in the PID controller, `xk_1` and `xk_2`, were updated in the wrong order. `xk_1` was overwritten before `xk_2` was assigned. This caused both `xk_1` and `xk_2` to take the value of `xk` when the function `controller_handler()` was called. This means the D-term of the PID controller was simplified from a second-order approximation using two previous timesteps to a first-order approximation using one previous timestep. xk - 2 * xk_1 + xk_2 => xk - xk_1 This degraded the performance of the PID controller by making it more noisy and less accurate. This bug was found by reverse engineering the tmon binary using the [REMaQE tool](https://arxiv.org/abs/2305.06902). Signed-off-by: Meet Udeshi <mudeshi1209@gmail.com> Fixes: 94f69966faf8 ("tools/thermal: Introduce tmon, a tool for thermal subsystem") --- tools/thermal/tmon/pid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)