Add D-Bus interface to monitor and control individual PID loops

[Krellan]

Currently, phosphor-pid-control includes a control interface over D-Bus, but only for an entire PID zone as a whole:

• xyz.openbmc_project.State.FanCtrl service name
• /xyz/openbmc_project/settings/fanctrl/zoneX objects published under that service
• xyz.openbmc_project.Control.Mode interface added to those objects

This currently exposes 2 boolean values:

• FailSafe for seeing if a zone is in failsafe mode or not
• Manual for disabling PID control completely for this zone (so fan speed can be manually set by user)

It would be nice to have a similar control interface, but for an individual PID loop. Each PID loop managed by phosphor-pid-control would be allocated its own D-Bus object featuring this control interface, similar to how each thermal zone is currently given its own D-Bus object.

Things that would be nice to allow control of:

• Enable for enabling/disabling this PID loop's contribution to the zone's fan speed calculations, without affecting other PID loops in the zone. This would be nice for tuning. It would allow tuning of one PID loop at a time, by selectively disabling all the other PID loops.

Things that would be nice to reveal, in a read-only way, about the state of an individual PID loop:

• Leader to show the name (string) of the input to this PID loop that is currently the leader, driving the thermal PID loop, that is, the component with the highest temperature (or the lowest thermal safety margin).
• Input to show the input value of this PID loop, in other words, the temperature (or margin) of the leader.
• Output to show the output value of this PID loop, in other words, the RPM required of the fans, to keep the leader cool enough to achieve the desired setpoint.

These fields are also appropriate for the fan PID loop:

• Leader could show the name of the thermal (or margin) PID loop that is currently the leader, driving the fan PID loop, that is, the thermal PID loop that requested the highest fan RPM.
• Input to show the input value of this fan PID loop, in other words, the requested RPM of the fans.
• Output to show the output value of this fan PID loop, in other words, the PWM percentage value to be commanded to the fan hardware.

Any other things that would be nice to reveal, while we have the chance? It might be nice to clean up the mapping of zones to individual PID loops: as of now, the zones are nicely enumerated under their existing control interface, however, individual PID loops are not easily enumerated in any way except by exhaustive search within entity-manager inventory. It might be nice to add some cross-referencing, for example, each zone could contain an array of all the PID loops that comprise that zone.

[Krellan]

Harvey Wu bought up a good point in the Discord chat. The zone itself should also have a Leader field added, not just the individual PID loops within the zone. This would show which fan PID loop is currently driving this zone (that is, the fan PID loop that is currently outputting the highest RPM). For consistency, and to re-use the same interface, Input and Output fields could also be added.

[Krellan]

How should PID loops be addressed?

Should names or numbers be used? The phosphor-pid-control configuration in entity-manager has the feature of being able to give names to each of the thermal zones, and PID loops. I'm not sure if the old static JSON file configuration allows this, though.

As for /xyz/openbmc_project/settings/fanctrl/zoneX one proposal:

/xyz/openbmc_project/settings/fanctrl/zoneX/pidY

This is an extension of what is currently in the code as of now (just zoneX). X and Y would be numbers, starting at 0, and increasing. The number is arbitrary, based essentially on random loading order. If the user cares about a particular PID loop, they can drill down for details.

Alternatively, a more freeform naming scheme:

/xyz/openbmc_project/settings/fanctrl/zones/ZONE/pids/PID

ZONE and PID would be arbitrary strings (subject to D-Bus naming restrictions), chosen by the user, from the Name field of their configuration stanzas in their entity-manager JSON configuration files.

I'm adding the hardcoded names zones and pids to make it clear the purpose, even if the user chooses crazy misleading names for their thermal zones and PID loops, and also to allow for future expansion.

Which is preferred?

[Krellan]

Thought about it a little more.

Zone and PID numbers are mandatory, and will be auto-assigned in the sequence that phosphor-pid-control detects them in. Zone numbers, in particular, are user-visible already, as they are used for zone addressing in the IPMI extensions.

https://github.com/openbmc/phosphor-pid-control/blob/master/ipmi.md

Zone and PID names are optional, and are used mostly to ensure uniqueness when using the new entity-manager configuration method, as entity-manager requires each Inventory object to have a unique name.

So, we can't address by zone name or PID name, because names won't be there always.

So, let's use numbers, like zoneX and pidY mentioned earlier.

I propose adding name as a new field, Name, a string, within this new. interface. That will allow callers to map zone numbers to zone names, and vice versa, and iteratively look up a zone by name if interested.

[huangalang]

@Krellan setting a zone to manual mode , is simply skip the pidcontrol loop
as following line
https://github.com/openbmc/phosphor-pid-control/blob/master/pid/pidloop.cpp#L117

but for pidloop , it can be thermal controller type or fan controller type
when you are talking about set one pid loop to manual mode
you mean skip the process simply no mater what type it's , right?

thanks

[Krellan]

Manual mode is something that can already be set. There's already an existing D-Bus interface to control that. Manual mode has to be set on a per-zone basis, because it needs to disable all PID loops in the zone, as you pointed out in the line of code you linked above.

This is a proposal for exposing a new D-Bus interface, for use by both PID loops and by thermal zones. No changes to the existing thermal zone D-Bus interface would be made.

Other than that, sorry, I don't fully understand your question.

[huangalang]

hello Krellan:
sorry to mislead you
I think we can design the path look like
/xyz/openbmc_project/settings/fanctrl/zoneX/<pid_loopname>
pid_loopname can be obtained from the config
we are building the prototype based on that

[Krellan]

The monitoring interface should, perhaps, be split up from the control interface. This might make upstream acceptance easier to achieve. The monitoring interface can also then have wider permissions than the control interface.

[huangalang]

@Krellan
we implemented this idea and submitted for code review
https://gerrit.openbmc.org/c/openbmc/phosphor-pid-control/+/63214

please go check out

[Krellan]

I forgot to mention Setpoint above. If we are exposing Input and Output, we should also be exposing Setpoint for each PID loop.

[Krellan]

There are really 3 pieces to this puzzle, each of which I should have probably split out as a separate GitHub issue number earlier, but unfortunately, did not:

1. Determine, and agree upon, an addressing scheme for addressing each PID loop in D-Bus. This is non-trivial, as allowable PID loop names are a superset of allowable D-Bus object names, and PID loop names might be optional (the original hardcoded config.json implementation did not require each PID loop to be named, I believe). An acceptable solution needs to take each of these into account. PID loops should also be nested underneath the thermal zone they are a member of.
2. Under these D-Bus objects, create a monitoring interface (read-only). Include, at a minimum,Input, Output, Setpoint, and Leader. All are numbers, except Leader which is a string.
3. Under the D-Bus object for each PID loop, create a control interface (read-write). Include Enable, which is a boolean, true by default. When false, this PID loop is disconnected from contributing to any further outputs or computations, as if it did not exist. This will be useful for disabling an unwanted thermal loop during tuning, or disconnecting an unwanted fan loop from the underlying fan hardware (to effectively allow a single fan to be set to manual mode even though the rest of the thermal zone is still in automatic mode).

[harveyquta]

Hi Krellan, I have some questions.
Do the /xyz/openbmc_project/settings/fanctrl/zoneX and /xyz/openbmc_project/settings/fanctrl/zoneX/<pid_loopname> have all these properties?(Input, Output, Setpoint, and Leader)
If so, what are the Input/Output/Setpoint meaning in /xyz/openbmc_project/settings/fanctrl/zoneX. Because there have thermal pid and fan pid in a zone.

[Krellan]

That is a good question.

(Input, Output, Setpoint, Leader) = These are best for a thermal PID loop or a fan PID loop.

I can see how they would be confusing to use for a thermal zone as a whole, though, especially if the zone has more than one fan PID loop. Are there any systems like this in the field? All thermal zones that I have worked with, have multiple thermal PID loops, but only one fan PID loop.

If the thermal zone has only one fan PID loop, then (Input, Output, Setpoint, Leader) for the thermal zone as a whole can mirror the information from the fan PID loop. That will save the user the trouble of having to iterate through all of the loops to figure out which one was the fan loop.

However, if the thermal zone has more than one fan PID loop, then this would not be practical. Any thoughts?