[phosphor-pid-control] scaling issue

James Feist james.feist at linux.intel.com
Tue Sep 10 03:27:08 AEST 2019 

On 9/9/19 9:57 AM, Patrick Venture wrote:
> On Thu, Sep 5, 2019 at 12:25 AM Hank Liou (劉晉翰) <Hank.Liou at quantatw.com> wrote:
>>
>> Hi Patrick,
>>
>>
>> Sorry for not clearly stating our problem. We have the following issue:
>>
>>
>> temp sensor gets 31(C) -> 31 / 0.255 = 121.57 -> pid uses 127.57 as input for temp sensor ->
>>
>> stepwise fan table output 100 duty -> full speed fan
> 
> Ok, so you're getting a dbus-based min/max value and you want to
> ignore it?  

What is providing the dbus-based min/max?

> In the json configuration, if you set the values to 0,
> they are set to the default (0), so there'd be no clean way to know to
> ignore dbus in this case, without adding a small check to only
> consider min/max from dbus when sensor is not temp/margin.  Basically,
> only care about min/max on dbus if type is "fan"

Why is the dbus-based min/max even there if you don't plan on using it?

> 
> If that's right, James do you have a cycle to look at this one-liner?
> 
>>
>>
>> As a result, fan will be at full speed while temp is low. Before the commit fc2e803 [1], this won't happen. The root cause is that, before fc2e803, pid will use config min/max, which is 0 in our case. This would not trigger scaling function, namely scaleSensorReading, in util.cpp. However, after such commit, min/max would be non-zero (-128/127 from DBus). This will trigger scaling function.
>>
>>
>> [1] https://github.com/openbmc/phosphor-pid-control/commit/fc2e803f5d9256944e18c7c878a441606b1f121c
>>
>>
>> Hank
>>
>>
>> ________________________________
>> From: Hank Liou (劉晉翰)
>> Sent: Monday, September 2, 2019 4:52 PM
>> To: Patrick Venture
>> Cc: James Feist; openbmc at lists.ozlabs.org
>> Subject: Re: [phosphor-pid-control] scaling issue
>>
>>
>> Hi Patrick,
>>
>>
>> Since we use phosphor-sel-logger [1] at the same time, we do assign min and max of temp sensors to Dbus (max: 127, min: -128). So in the present case, our temp value will be divided by 0.255 (also due to exponent is -3 here). This will cause re-scaling problem. Therefore there should be an statement to distinguish sensor types. If it is "temp", then one assigns 0 to the min and max from Dbus.
>>
>>
>> [1] https://github.com/openbmc/phosphor-sel-logger/blob/3d300fca24b30864b3e9a4f5768cfe5e769458ae/include/sensorutils.hpp#L59
>>
>>
>> Hank
>>
>>
>> ________________________________
>> From: Patrick Venture <venture at google.com>
>> Sent: Friday, August 30, 2019 1:47 AM
>> To: Hank Liou (劉晉翰)
>> Cc: James Feist; openbmc at lists.ozlabs.org
>> Subject: Re: [phosphor-pid-control] scaling issue
>>
>> PTAL - https://gerrit.openbmc-project.xyz/24827 - this is merged, and
>> the srcrev bump should propagate into openbmc/openbmc in a day or two.
>>
>> On Wed, Aug 28, 2019 at 11:00 PM Hank Liou (劉晉翰) <Hank.Liou at quantatw.com> wrote:
>>>
>>> Hi Patrick,
>>>
>>> I think it's OK to parse the config min&max for temp sensors.
>>>
>>> Any suggestion?
>>>
>>> Thanks,
>>> Hank
>>>
>>>> -----Original Message-----
>>>> From: openbmc [mailto:openbmc-
>>>> bounces+hank.liou=quantatw.com at lists.ozlabs.org] On Behalf Of Hank Liou
>>>> (劉晉翰)
>>>> Sent: Friday, August 23, 2019 4:31 PM
>>>> To: Patrick Venture <venture at google.com>; James Feist
>>>> <james.feist at linux.intel.com>
>>>> Cc: openbmc at lists.ozlabs.org
>>>> Subject: RE: [phosphor-pid-control] scaling issue
>>>>
>>>> Hi Patrick,
>>>>
>>>>> -----Original Message-----
>>>>> From: Patrick Venture [mailto:venture at google.com]
>>>>> Sent: Wednesday, August 21, 2019 10:32 PM
>>>>> To: Hank Liou (劉晉翰) <Hank.Liou at quantatw.com>; James Feist
>>>>> <james.feist at linux.intel.com>
>>>>> Cc: openbmc at lists.ozlabs.org
>>>>> Subject: Re: [phosphor-pid-control] scaling issue
>>>>>
>>>>> On Wed, Aug 21, 2019 at 1:11 AM Hank Liou (劉晉翰)
>>>>> <Hank.Liou at quantatw.com> wrote:
>>>>>>
>>>>>> Hi All,
>>>>>>
>>>>>>
>>>>>> After commit [1], I found my temp sensor reading would be re-scaled
>>>>>> by
>>>>> multiplying 1 over 255, making temperature into unfamiliar unit. Also
>>>>> the fan rpm reading would lie in [0,1] interval, letting the fan input
>>>>> to be 0 (since the input value of fan is from an integer array [2]). Are these
>>>> normal behaviors?
>>>>> Or do I miss something?
>>>>>
>>>>> Are you using dbus configuration or json?  If json, can you attach your config.
>>>>> Since you're saying it was working and now isn't, I'm assuming there's
>>>>> something about the config being treated differently with the code
>>>>> changes in an unexpected way.
>>>>
>>>> I found pid control will first read min and max from dbus and then (before
>>>> commit [1]) revise them if
>>>>
>>>> info->min != conf::inheritValueFromDbus (in dbus/dbuspassive.cpp)
>>>>
>>>> After value initialization, the min and max would be the ones in json file (Json
>>>> file [3]). However, after commit [1] the min and max values of config would
>>>> not be fed into the fan control process. The scaling issue is originated from
>>>> commit [4] with the aim to treat fan rpm as percentage. It seems that commit
>>>> [1] unexpectedly affects temp sensors in the sense that the temp is the
>>>> integer reading not percentage. Before commit [1], it would not re-scale the
>>>> temp reading since my min and max are 0 [6].
>>>>
>>>>
>>>>
>>>> There is another issue with commit [1]. Now the fan rpm would be something
>>>> like
>>>>
>>>> 1500 / 20000 = 0.075
>>>>
>>>> where rpm max 20000 is from dbus. However the fan input function would
>>>> transfer double into int, which is 0 for 0.075 (see [5]). Hence the fan input is 0
>>>> not percentage. Is there something wrong?
>>>>
>>>> [1] https://github.com/openbmc/phosphor-pid-
>>>> control/commit/fc2e803f5d9256944e18c7c878a441606b1f121c
>>>> [2] https://github.com/openbmc/phosphor-pid-
>>>> control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/pid/fancontrolle
>>>> r.cpp#L86
>>>> [3]
>>>>        {
>>>>             "name": "fan1",
>>>>             "type": "fan",
>>>>             "readPath": "/sys/class/hwmon/hwmon1/fan1_input",
>>>>             "writePath": "/sys/class/hwmon/hwmon1/pwm1",
>>>>             "min": 0,
>>>>             "max": 255
>>>>         },
>>>>         {
>>>>             "name": "temp1",
>>>>             "type": "temp",
>>>>             "readPath": "/xyz/openbmc_project/sensors/temperature/temp1",
>>>>             "writePath": "",
>>>>             "min": 0,
>>>>             "max": 0
>>>>         }
>>>> [4] https://github.com/openbmc/phosphor-pid-
>>>> control/commit/75eb769d351434547899186f73ff70ae00d7934a
>>>> [5] https://github.com/openbmc/phosphor-pid-
>>>> control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/pid/fancontrolle
>>>> r.cpp#L64
>>>> [6] https://github.com/openbmc/phosphor-pid-
>>>> control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/dbus/dbuspassi
>>>> ve.cpp#L158
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>> [1]
>>>>>> https://github.com/openbmc/phosphor-pid-
>>>>> control/commit/fc2e803f5d92569
>>>>>> 44e18c7c878a441606b1f121c
>>>>>>
>>>>>> [2]
>>>>>> https://github.com/openbmc/phosphor-pid-
>>>>> control/blob/a7ec8350d17b70153
>>>>>> cebe666d3fbe88bddd02a1a/pid/fancontroller.cpp#L86
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>
>>>>>> Hank Liou
>>>>>>
>>>>>> Quanta Computer Inc.
>>>>>>
>>>>>>
>>>>
>>>> Sincerely,
>>>> Hank

More information about the openbmc mailing list