phosphor-pid-control: dbus tuning interface

[Ed Tanous]

On Tue, Jul 7, 2020 at 4:51 PM Patrick Venture <venture at google.com> wrote:
>
> On Tue, Jul 7, 2020 at 4:35 PM Jason Ling <jasonling at google.com> wrote:
> >
> > Apologies if this has already been discussed but..
> > Would the community benefit from the addition of a dbus tuning interface for phosphor-pid-control?

This capability already exists if you're using phosphor-pid-control
with entity manager.  There is a redfish interface into the thermal
control that allows modification of tuning parameters at runtime
through PATCH calls to the Managers API.  For example, the REST model
for a Pid controller is here.
https://github.com/openbmc/bmcweb/blob/07941a881d1d07fd8cbe2ac0db88b3c96deab4c7/static/redfish/v1/schema/OemManager_v1.xml#L197

Entity manager essentially will trigger a reconfigure of
phosphor-pid-control on every change, so new parameters can be tested.
It's worked quite well in the past when I've worked with thermal teams
who don't want to edit config files, recompile, and reload by hand, or
want to do things like connect the fan outputs directly to a simulink
model to test some new tuning types.

I'm guessing you didn't know that existed, and clearly Patrick didn't
either.  So the next question is, where did you go to look for this
kind of thing (ie, where should we document this)?  There might not be
a perfect place, but hopefully we can make this more clear in the
future when people have these needs.

> >
> > To get things going:
> >
> > initial thoughts would be to have the details of each thermal/fan PID control pushed onto dbus with loop parameters being r/w.

In the currently existing one, these parameters are modeled more as
phosphor-pid-control pulling parameters, rather than config changes
being pushed, as technically we're modifying a system config, and the
configuration manager needs to know about it, otherwise it will simply
override it again on the next configuration change.  Said another way,
you want to be able to add/remove drives while running under
non-standard tunings, and not have it override what you're trying to
test.
> >
> > I'm not sure about whether the output or input to the loops should be published as properties.

In what exists today, the inputs and outputs can be overridden by a
PATCH to the relevant Redfish sensor.  This gets translated into a
Dbus SetProperties call to the Value interface.  The PWM outputs are
modeled as sensors to allow this to be done (and to track their
values).

> >
> > Have this capability compiled out or runtime disabled by default

I believe this is compiled in by default today if you're using the
right combination of daemons.

> >
> > Things I worry about:
> >
> > If the phosphor-pid-loop updates the PID output/input/error to dbus on every evaluation cycle will it bring adversely affect dbus performance or bring a system to its knees? is it better to just implement the querying of pid loop updates as a dbus method?

You could be right, although it's mostly dependent on how many sensor
updates happen, and what hysteresis settings you have.  In practice,
it doesn't tend to be a lot more traffic if the loops are tuned
reasonably, and aren't cycling.


>
> James and I have talked about this type of feature in the past.
> Effectively making the PID control object a dbus object.  I think it
> would be fine if it when it updated its own values, they weren't
> broadcast - IIRC< that's an easy thing to implement, there is a
> separate parameter for this.  Then it would be queryable and somewhat
> tunable.  Currently the tuning is done by writing to a file, which is
> how the scripts are set up - but presumably something more
> programmatic or thorough could provide some value.
>