improve ec power reporting |
|||||
Issue descriptionCurrently, when we measure power, in part we ask the ec what consumption it's seeing based on the battery's gas gauge. https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/master/servo/drv/ec.py#284 We'd like to improve this in 2 ways: (1) not involve the ec console, so that we don't wake the ec if it's in a low power state (2) get more than one sample in, and report the average consumption over the last x seconds (x being ~3-6s?) The idea is (1) to keep a running average and have a low priority task that periodically polls the battery for its power numbers, and maintains the average (2) I'm not sure how to do this. We'd want to have access to that information over servo. This bug is to get input on how to accomplish those improvements, and track work on this front.
,
Aug 29 2017
,
Oct 20 2017
I ran a couple tests on this and got the following results on soraka: - running a full get_battery_params every HOOK_TICK (200ms) costs about 5mW - running a voltage and current getter during every HOOK_TICK (200ms) costs about 1mW - running a voltage and current getter every 5 HOOK_TICK (1s) costs about 140uW (so within the noise). I think there are a couple options we have w/ polling every second: (1) keep running avg for a set window size (2) keep infinite running avg and reset the avg every time a user queries it (2.1) allow for an option to query without resetting the average. Since the use cases for this are to have secondary power number besides from ADCs, and allow for quick measurements without needing reworks (on boards without on-board adcs), I'm leaning toward option (1). Also it's much simpler. Thoughts?
,
Oct 21 2017
I'm not sure I understand your proposals, but I think it's OK to add a new feature, behind a CONFIG option, that takes power records in whichever way you think is most useful. Regarding the power impact of extra voltage / current polling, you can make it less intrusive by re-using the periodic battery measurements we take and store anyway -- of course, this makes your polling period less frequent.
,
Jan 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/51e9e69f386366256807e6f4ccdd258821cdcfe0 commit 51e9e69f386366256807e6f4ccdd258821cdcfe0 Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org> Date: Tue Jan 16 12:08:26 2018 power: introducing pwr_avg console command pwr_avg provides an average voltage, current, and power over the last 1 minute. It's up to the battery drivers to implement this functionality. This change allows us to have better power tracking while minimizing the power impact on the EC, because - the pwr_avg command only needs to be called once every minute, and is short, thus less expensive to parse on ECs without a UART buffer - the work done to keep the avg is partially done by the batteries already and it's just a question of retrieving it. undefined on wheatley since no power debugging planned on that board. usage: > pwr_avg mv = 7153 ma = -605 mw = -4327 BUG=chromium:752320 BRANCH=None TEST=make buildall -j Change-Id: Id1a3479d277aedf90dfa965afb4ee9136654b1cf Signed-off-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/823884 Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/51e9e69f386366256807e6f4ccdd258821cdcfe0/driver/battery/smart.c [modify] https://crrev.com/51e9e69f386366256807e6f4ccdd258821cdcfe0/common/charge_state_v2.c [modify] https://crrev.com/51e9e69f386366256807e6f4ccdd258821cdcfe0/include/config.h [modify] https://crrev.com/51e9e69f386366256807e6f4ccdd258821cdcfe0/board/wheatley/board.h [modify] https://crrev.com/51e9e69f386366256807e6f4ccdd258821cdcfe0/include/battery.h [modify] https://crrev.com/51e9e69f386366256807e6f4ccdd258821cdcfe0/driver/battery/max17055.c
,
Jan 26 2018
,
Jan 26 2018
,
Mar 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/1da8acbe375eba18ec0cf1a469312f77fbaf88c7 commit 1da8acbe375eba18ec0cf1a469312f77fbaf88c7 Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org> Date: Tue Mar 20 11:15:20 2018 power: introducing pwr_avg console command pwr_avg provides an average voltage, current, and power over the last 1 minute. It's up to the battery drivers to implement this functionality. This change allows us to have better power tracking while minimizing the power impact on the EC, because - the pwr_avg command only needs to be called once every minute, and is short, thus less expensive to parse on ECs without a UART buffer - the work done to keep the avg is partially done by the batteries already and it's just a question of retrieving it. undefined on wheatley since no power debugging planned on that board. usage: > pwr_avg mv = 7153 ma = -605 mw = -4327 BUG=chromium:752320 BRANCH=eve TEST=make BOARD=eve -j Signed-off-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/823884 Reviewed-by: Vincent Palatin <vpalatin@chromium.org> (cherry picked from commit 51e9e69f386366256807e6f4ccdd258821cdcfe0) Conflicts: board/wheatley/board.h undefine some tasks to make buildall -j work driver/battery/max17055.c did not exist in branch, not needed Change-Id: I5f37a6f6446fa0e4a205672cb3433827d5da948d Signed-off-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/966244 Reviewed-by: Duncan Laurie <dlaurie@google.com> [modify] https://crrev.com/1da8acbe375eba18ec0cf1a469312f77fbaf88c7/driver/battery/smart.c [modify] https://crrev.com/1da8acbe375eba18ec0cf1a469312f77fbaf88c7/board/wheatley/board.h [modify] https://crrev.com/1da8acbe375eba18ec0cf1a469312f77fbaf88c7/include/battery.h [modify] https://crrev.com/1da8acbe375eba18ec0cf1a469312f77fbaf88c7/common/charge_state_v2.c [modify] https://crrev.com/1da8acbe375eba18ec0cf1a469312f77fbaf88c7/include/config.h
,
Mar 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/8142d266fa53aa38e0106e06a8f7a21e9d83dfce commit 8142d266fa53aa38e0106e06a8f7a21e9d83dfce Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org> Date: Wed Mar 21 17:53:27 2018 servo: leverage pwr_avg where available pwr_avg was recently introduced as an ec command that provides a one minute averaged reading of current, voltage, and power. This change introduces the ec code and xml files required to leverage this. Now there is: - avg_ppvar_vbat_mw - avg_ppvar_vbat_ma - avg_ppvar_vbat_mv Since the command is fairly new, the code also explicitly checks for the command being available, and returns a more informative error if the failure is due to pwr_avg not being available on that ec image. Any code, such as power scripts, can then decide higher up in the stack if pwr_avg being unavailable is a deal breaker or if it's okay to default to ppvar_vbat_mw. BUG=chromium:752320, chromium:760267 TEST=manual $ dut-control avg_ppvar_vbat_mw avg_ppvar_vbat_mw:5572 (on dut without the command) $ dut-control avg_ppvar_vbat_mw Problem with ['avg_ppvar_vbat_mw'] :: cmd |pwr_avg| is not available on the ec. Change-Id: I4b9efeaa87a5c5f6ca932cfa2cec7cb135f7d388 Reviewed-on: https://chromium-review.googlesource.com/888281 Commit-Ready: Ruben Rodriguez Buchillon <coconutruben@chromium.org> Tested-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org> Reviewed-by: Todd Broch <tbroch@chromium.org> [modify] https://crrev.com/8142d266fa53aa38e0106e06a8f7a21e9d83dfce/servo/data/ec_common.xml [modify] https://crrev.com/8142d266fa53aa38e0106e06a8f7a21e9d83dfce/servo/drv/ec.py |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by sha...@chromium.org
, Aug 4 2017