[phosphor-pid-control] scaling issue
Hank Liou (劉晉翰)
Hank.Liou at quantatw.com
Tue Sep 10 18:01:55 AEST 2019
>-----Original Message-----
>From: James Feist [mailto:james.feist at linux.intel.com]
>Sent: Tuesday, September 10, 2019 1:27 AM
>To: Patrick Venture <venture at google.com>; Hank Liou (劉晉翰)
><Hank.Liou at quantatw.com>
>Cc: openbmc at lists.ozlabs.org
>Subject: Re: [phosphor-pid-control] scaling issue
>
>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?
Yes, we want to ignore it.
>
>What is providing the dbus-based min/max?
We use dbus min/max for phosphor-sel-logger utility. The dbus min/max is provided by config file of phosphor-hwmon.
In our case, they are -128/127 respectively.
I have an observation:
I found that while fan readpath is of the form "/sys/...", the fan input would not be rescaled. The same is for other sensors since the input value would not enter rescaling function.
However, in our case we don't have a sys path for some thermal sensors.
>
>> 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"
>
I agree with the option to add a check since the min/max is only for fan now.
>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/fc2e803f5d9256
>>> 944e18c7c878a441606b1f121c
>>>
>>>
>>> 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/3d300fca24b30864b
>>> 3e9a4f5768cfe5e769458ae/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
>>>>> bounces+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/fancontro
>>>>> lle
>>>>> 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/fancontro
>>>>> lle
>>>>> r.cpp#L64
>>>>> [6] https://github.com/openbmc/phosphor-pid-
>>>>>
>control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/dbus/dbuspass
>>>>> i
>>>>> 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