Consider running powerd in factory mode |
||||
Issue descriptionRight now powerd is stopped on Chrome OS devices in factory mode. Per Hung-Te at http://b/63878094#comment15: "We had some discussion with old powerd owner that we want lots of stuff, including no suspend (both idle and lid-close), no panel off, no sleep, .... etc, and in the end the owner gives up adding a new mode for us and said stop is probably the most easy solution." I've been the powerd owner for a while now and am happy to add this if it means we don't need workarounds like https://chromium-review.googlesource.com/c/581247. :-) If you give me a list of what you need, I'll add one or more powerd prefs for configuring it. Then it'd just be a matter of doing something like "echo 1>/var/lib/power_manager/factory_mode".
,
Jul 21 2017
Oh we also need to control charging/discharge. we do this by checking battery state and invoke ectool chargecontrol, but powerd should not change what we set.
,
Jul 21 2017
Do you want the screen to dim when the user is inactive? Do you want the brightness to be adjusted if the ambient light level changes? Alternately, let me know if you want the backlight to always remain at 100% brightness. What behavior do you want from the keyboard backlight? For the most part, powerd doesn't control charging; it just reports what's going on to Chrome. If the user selects a different Type-C charging source via Chrome, powerd passes the request on to the kernel, but I assume that that won't be happening in factory mode.
,
Jul 21 2017
> Do you want the screen to dim when the user is inactive? No because we have tests to verify specified screen backlight level. > Do you want the brightness to be adjusted if the ambient light level changes? No because the ALS is not calibrated yet during most of the manufacturing stage. > Alternately, let me know if you want the backlight to always remain at 100% brightness. We want it to keep 100% or some default value, and changes when we invoke the tool to change backlight (and keep changed value until we set it again). > What behavior do you want from the keyboard backlight? Just keep it in a default state and can be changed when we try to change it by commands. That's also a test item. > but I assume that that won't be happening in factory mode. The factory needs to test if both parts can charge, so we do ask op to attach different ports and verify the pd state, but no, we don't let OP change selection from Chrome. In short, we want everything to keep in default values but still change-able when we run related commands.
,
Jul 21 2017
>> Alternately, let me know if you want the backlight to always remain at 100% brightness. > > We want it to keep 100% or some default value, and changes when we invoke the tool to > change backlight (and keep changed value until we set it again). What tool do you use? Is it the backlight_tool program from the power_manager package? I'll probably look into just making powerd not touch the display or keyboard backlight devices.
,
Jul 21 2017
In current general implementation, sysfs: https://chromium.googlesource.com/chromiumos/platform/factory/+/master/py/device/display.py#45 We can use backlight_tool on cros if needed.
,
Jul 21 2017
Sticking with sysfs should be fine. (Calling backlight_tool instead of interacting directly with sysfs may simplify your code, though.) I've uploaded https://chromium-review.googlesource.com/c/582231/. Are you set up to give it a try? Writing "1" to /var/lib/power_manager/factory_mode and restarting powerd or rebooting should be all it takes. Are factory images still created by running a script that modifies a regular image? If so, you'll probably want to make it create /usr/share/power_manager/factory_mode. Note that powerd still inhibits input devices as needed for laptop, tablet, lid-closed, etc. modes. Let me know if you need it to leave all input devices active at all times instead (although I think the EC will still disable the keyboard when in tablet mode, for instance).
,
Jul 22 2017
@pihsun, can you try the CL in c#7 and see if that would work for tests like lid close / laptop mode switch? Re#7 factory images are currently using standard "test" image, plus additional files in /usr/local/factory. We can't create /usr/share/power_manager/factory_mode since that's in test image's rootfs (ro).
,
Jul 22 2017
Okay, got it. Using /var/lib/power_manager/factory_mode on the stateful partition should work, then.
,
Jul 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/b4e79726f0231b95a0aafdbbd98ab44c846d33d9 commit b4e79726f0231b95a0aafdbbd98ab44c846d33d9 Author: Daniel Erat <derat@chromium.org> Date: Sun Jul 23 16:26:55 2017 power: Add factory_mode pref. Add support for a new "factory_mode" powerd pref that can be used to request factory-specific behavior from powerd: - Don't change screen or keyboard brightness. - Do nothing when system is idle. - Do nothing when lid is closed. BUG= chromium:747501 TEST=added unit tests; also manually tested that system behaves as described after writing "1" to /var/lib/power_manager/factory_mode Change-Id: I3db3ed7188cc4ba487e4c0028c214a54a53a58cd Reviewed-on: https://chromium-review.googlesource.com/582231 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/b4e79726f0231b95a0aafdbbd98ab44c846d33d9/power_manager/common/power_constants.cc [modify] https://crrev.com/b4e79726f0231b95a0aafdbbd98ab44c846d33d9/power_manager/powerd/policy/state_controller_unittest.cc [modify] https://crrev.com/b4e79726f0231b95a0aafdbbd98ab44c846d33d9/power_manager/common/power_constants.h [modify] https://crrev.com/b4e79726f0231b95a0aafdbbd98ab44c846d33d9/power_manager/powerd/daemon.cc [modify] https://crrev.com/b4e79726f0231b95a0aafdbbd98ab44c846d33d9/power_manager/powerd/policy/state_controller.h [modify] https://crrev.com/b4e79726f0231b95a0aafdbbd98ab44c846d33d9/power_manager/powerd/daemon.h [modify] https://crrev.com/b4e79726f0231b95a0aafdbbd98ab44c846d33d9/power_manager/powerd/daemon_unittest.cc [modify] https://crrev.com/b4e79726f0231b95a0aafdbbd98ab44c846d33d9/power_manager/powerd/policy/state_controller.cc
,
Jul 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/6ddc2fded87c2762792ee4ce544b59ab767e75fa commit 6ddc2fded87c2762792ee4ce544b59ab767e75fa Author: Peter Shih <pihsun@chromium.org> Date: Mon Jul 24 13:27:56 2017 enable powerd with factory mode. powerd support factory mode after CL:582231, so we can enable powerd in factory, and don't need special hack to make pytest like tablet_rotation to work. BUG= chromium:747501 TEST=manually, test on DUT and check that there's no idle suspend. Change-Id: I85fc1ec850bb57a0ba0b54e2775b84215a9da0e5 Reviewed-on: https://chromium-review.googlesource.com/582252 Commit-Ready: Pi-Hsun Shih <pihsun@chromium.org> Tested-by: Pi-Hsun Shih <pihsun@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [delete] https://crrev.com/a6b560919185397e63d475badb7a5d9df5ee04f0/init/common.d/inhibit_jobs/powerd [add] https://crrev.com/6ddc2fded87c2762792ee4ce544b59ab767e75fa/init/common.d/powerd_factory_mode.sh
,
Jul 24 2017
https://chromium-review.googlesource.com/c/582252/3/init/common.d/inhibit_jobs/powerd says "we need to disable powerd to access power button". Is that still the case? If so, can you tell me the behavior that you need? Do you want powerd to not report power button presses and releases to Chrome (which makes decisions about when to lock the screen or shut down the system) while in factory mode?
,
Jul 24 2017
Re#12 oh that's right. In keyboard test we have to make sure all keys can be pressed properly, so we need to receive power button key down and up. We need to capture the key down / up events from evdev. We don't really care if Chrome received the key events or not, but pressing power button should not trigger Chrome to start logout / shutdown, so I think the answer is powerd should not report power button presses to Chrome.
,
Jul 24 2017
I've uploaded https://chromium-review.googlesource.com/c/582869 to make powerd ignore power button events when factory_mode is set. Let me know if anything else comes up.
,
Jul 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/c7cf62c9c507abc6852dddf74e092e3e2cf57044 commit c7cf62c9c507abc6852dddf74e092e3e2cf57044 Author: Daniel Erat <derat@chromium.org> Date: Tue Jul 25 05:54:05 2017 power: Ignore power button for factory mode. Make powerd ignore power button events when the factory_mode pref is set. This is needed for keyboard testing, in which all keys are pressed and evdev is used to watch for events. BUG= chromium:747501 TEST=added test Change-Id: I09c42ba288179d8032e16bed835c0f6464a81eba Reviewed-on: https://chromium-review.googlesource.com/582869 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> Reviewed-by: Pi-Hsun Shih <pihsun@chromium.org> [modify] https://crrev.com/c7cf62c9c507abc6852dddf74e092e3e2cf57044/power_manager/powerd/policy/input_event_handler.cc [modify] https://crrev.com/c7cf62c9c507abc6852dddf74e092e3e2cf57044/power_manager/powerd/policy/input_event_handler.h [modify] https://crrev.com/c7cf62c9c507abc6852dddf74e092e3e2cf57044/power_manager/powerd/policy/input_event_handler_unittest.cc
,
Jul 25 2017
Another small difference that I just thought of between powerd's factory_mode and not running powerd at all: right now, powerd shuts the system down when its battery is almost empty. The EC would do this anyway, but powerd may do it at around ~1% higher. Is this something that you test in the factory?
,
Jul 25 2017
Re#16 We do test battery level and charging, but it's using a higher minimal level, say 5% ( https://storage.googleapis.com/chromeos-factory-docs/sdk/pytests/battery_cycle.html ) - although the number may vary per project. If powerd will only increase that at 1% I think it's probably fine (but again, this depends on OEM's requirement per project) - but yes it would be even better if powerd won't add 1%.
,
Jul 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/1ffee1606dda97dc80b0fd957181956d4d811478 commit 1ffee1606dda97dc80b0fd957181956d4d811478 Author: Daniel Erat <derat@chromium.org> Date: Wed Jul 26 18:34:46 2017 power: Don't shut down for low battery in factory mode. Make powerd avoid shutting the system down in response to a low battery charge when the factory_mode pref is set to true. BUG= chromium:747501 TEST=added test Change-Id: Id0fc48fb2bc51758baf21cf7f96d88d444b4e1eb Reviewed-on: https://chromium-review.googlesource.com/585859 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/1ffee1606dda97dc80b0fd957181956d4d811478/power_manager/powerd/daemon.cc [modify] https://crrev.com/1ffee1606dda97dc80b0fd957181956d4d811478/power_manager/powerd/daemon.h [modify] https://crrev.com/1ffee1606dda97dc80b0fd957181956d4d811478/power_manager/powerd/daemon_unittest.cc
,
Jul 26 2017
Feel free to reopen this or file a new bug against me if you need anything else!
,
Jan 22 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by hungte@chromium.org
, Jul 21 2017