New Feature Request in phosphor-pid-control

[Patrick Venture]

On Fri, Jun 15, 2018 at 10:36 AM, Tanous, Ed <ed.tanous at intel.com> wrote:
> This seems like a bit of a corner case in the thermal realm.  Are you really sure that you want the same PID tuning values for the margin sensor as the absolute sensors?

We use margin values, so this use case isn't going away :D.

>
> Me and my team are actually trying to get away from margin sensors altogether.  Customers don't seem to like them, and question the fact that they're negative numbers.  It's  mostly a psychology thing, and usually after explaining it to them, they understand and can live with it, but it's a pain point all the same.  In lieu of margin sensors, we're trying an approach now where all sensors are absolute, but we dynamically adjust the thresholds based on what we find at runtime.  Most of the components expose their own temperature goal values through some kind of interface.  In the case of Intel processors, we would set our thresholds based on the information we read over PECI about the CPUs thermal characteristics, and report the absolute value back, then take our thermal limits as a percentage of the goal values.
>
> Back to your proposal, I'm not really clear on what the issue is with the existing solution.  If the "goal" is to hunt down for a temperature, doesn't that just mean that the proportional and integral components of the loop would have a negative coefficient in the tuning parameters?  It doesn't really matter what the sign is on the error calculation if you can simply reverse it in the PID values.
>
> I fully support adding multiple sensors to a single PID loop.  This was one of the things that we were planning on adding as soon as we got more time for fan control (hopefully next quarter) but I'm not really following the addition of a new parameter.

In order to choose whether to maximize or minimize the value, a goal
that varies based on the inputs, we need a method for specifying this.
The code won't care if it's a margin sensor input or absolute
temperature sensor input, but the human can control the behavior by
specifying the correct PID coefficients, as you said, but with
multiple inputs -- the code needs to know what to choose.

>
> -Ed
>
>> -----Original Message-----
>> From: openbmc [mailto:openbmc-
>> bounces+ed.tanous=intel.com at lists.ozlabs.org] On Behalf Of Patrick
>> Venture
>> Sent: Friday, June 15, 2018 8:39 AM
>> To: Lei YU <mine260309 at gmail.com>
>> Cc: OpenBMC Maillist <openbmc at lists.ozlabs.org>
>> Subject: New Feature Request in phosphor-pid-control
>>
>> The thermal pid controller object only supports one input.  The PID
>> parameters determine whether its goal is to increase or decrease the fan
>> speed given a set-point.  My primary usecase was to have the value as a
>> margin, if the sensor value is 15C, and the setpoint is 10C, then we're fine
>> because 15 > 10.  However, if it's an absolute temperature your goal would
>> be the opposite, to speed the fans up until 15 went down to 10.  Because of
>> these opposite use-cases it wasn't clear what behavior would be taken with
>> multiple inputs.  We have another program take dozens of inputs and make
>> one margin value given a configuration, and that's what is passed down to
>> phosphor-pid-control.
>>
>> My proposal is to support multiple inputs to the thermal controller and there
>> will be a new field in the configuration "min" "max" that will decide the
>> behavior.
>>
>> Patrick