From:   Meet Udeshi <mudeshi1209@gmail.com>
To:     daniel.lezcano@linaro.org
Cc:     linux-pm@vger.kernel.org
Subject: Re: [PATCH] tools/thermal: tmon: Fix sample data update in PID
Date:   Mon, 11 Sep 2023 17:20:31 -0400
Message-Id: <20230911212031.46336-1-mudeshi1209@gmail.com>
In-Reply-To: <20230822184940.31316-1-mudeshi1209@gmail.com>
References: <20230822184940.31316-1-mudeshi1209@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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