Decreases allowed based on all groups
For speed decreases to occur, all sensor groups involved with setting a
net decrease delta must be below their given t-control low values for
the associated zone. This handles the case where one or more sensor
groups' temperatures stabilize above their t-control low value, but one
or more other sensor groups' temperatures are below their t-control low
value which would result in speed decrease requests that could
eventually lead to fan speed oscillations.
Resolves openbmc/openbmc#2710
Change-Id: I382de5d3f9c3e631a332d49dfcb06e705ff6fc17
Signed-off-by: Matthew Barth <msbarth@us.ibm.com>
diff --git a/control/actions.hpp b/control/actions.hpp
index 55c8af8..c7356ab 100644
--- a/control/actions.hpp
+++ b/control/actions.hpp
@@ -230,41 +230,45 @@
state = std::forward<T>(state)](auto& zone, auto& group)
{
auto netDelta = zone.getDecSpeedDelta();
- std::for_each(
- group.begin(),
- group.end(),
- [&zone, &state, &factor, &speedDelta, &netDelta](auto const& entry)
+ for (auto& entry : group)
+ {
+ try
{
- try
+ T value = zone.template getPropertyValue<T>(
+ entry.first,
+ std::get<intfPos>(entry.second),
+ std::get<propPos>(entry.second));
+ // TODO openbmc/phosphor-fan-presence#7 - Support possible
+ // state types for comparison
+ if (value < state)
{
- T value = zone.template getPropertyValue<T>(
- entry.first,
- std::get<intfPos>(entry.second),
- std::get<propPos>(entry.second));
- // TODO openbmc/phosphor-fan-presence#7 - Support possible
- // state types for comparison
- if (value < state)
+ if (netDelta == 0)
{
- if (netDelta == 0)
- {
- netDelta = ((state - value)/factor) * speedDelta;
- }
- else
- {
- // Decrease is the factor applied to the
- // difference times the given speed delta
- netDelta = std::min(
- netDelta,
- ((state - value)/factor) * speedDelta);
- }
+ netDelta = ((state - value)/factor) * speedDelta;
+ }
+ else
+ {
+ // Decrease is the factor applied to the
+ // difference times the given speed delta
+ netDelta = std::min(
+ netDelta,
+ ((state - value)/factor) * speedDelta);
}
}
- catch (const std::out_of_range& oore)
+ else
{
- // Property value not found, netDelta unchanged
+ // No decrease allowed for this group
+ netDelta = 0;
+ break;
}
}
- );
+ catch (const std::out_of_range& oore)
+ {
+ // Property value not found, netDelta unchanged
+ }
+ }
+ // Update group's decrease allowed state
+ zone.setDecreaseAllow(&group, !(netDelta == 0));
// Request speed decrease to occur on decrease interval
zone.requestSpeedDecrease(netDelta);
};
diff --git a/control/zone.cpp b/control/zone.cpp
index ffde46a..209d773 100644
--- a/control/zone.cpp
+++ b/control/zone.cpp
@@ -270,9 +270,17 @@
void Zone::decTimerExpired()
{
- // Only decrease speeds when no requested increases exist and
- // the increase timer is not running (i.e. not in the middle of increasing)
- if (_incSpeedDelta == 0 && !_incTimer.running())
+ // Check all entries are set to allow a decrease
+ auto pred = [](auto const& entry) {return entry.second;};
+ auto decAllowed = std::all_of(_decAllowed.begin(),
+ _decAllowed.end(),
+ pred);
+
+ // Only decrease speeds when allowed,
+ // where no requested increases exist and
+ // the increase timer is not running
+ // (i.e. not in the middle of increasing)
+ if (decAllowed && _incSpeedDelta == 0 && !_incTimer.running())
{
auto requestTarget = getRequestSpeedBase();
// Request target speed should not start above ceiling
diff --git a/control/zone.hpp b/control/zone.hpp
index bc08f48..406af13 100644
--- a/control/zone.hpp
+++ b/control/zone.hpp
@@ -89,6 +89,17 @@
}
/**
+ * @brief Sets the decrease allowed state of a group
+ *
+ * @param[in] group - A group that affects speed decreases
+ * @param[in] isAllow - Allow state according to group
+ */
+ inline void setDecreaseAllow(const Group* group, bool isAllow)
+ {
+ _decAllowed[*(group)] = isAllow;
+ }
+
+ /**
* @brief Sets a given object's property value
*
* @param[in] object - Name of the object containing the property
@@ -487,6 +498,11 @@
std::map<const Group, bool> _floorChange;
/**
+ * @brief Map of groups controlling decreases allowed
+ */
+ std::map<const Group, bool> _decAllowed;
+
+ /**
* @brief Map of group service names
*/
std::map<const Group, std::vector<Service>> _services;