powerd should emit DarkSuspendImminent for non-user-initiated canceled suspend attempts |
||||
Issue descriptionAs discussed at https://crrev.com/c/798640/1/power_manager/powerd/policy/suspender.cc#b551, powerd's Suspender class currently contains code to emit a DarkSuspendImminent signal after a failed suspend attempt, even when the system isn't actually in dark resume. It sounds like the reason this is present is because it's possible that the attempt was canceled as a result of a non-user-initiated wake event (e.g. WoWLAN). In this case, we'd want other processes on the system to do their usual dark resume preparations to handle the wakeup. The current implementation still seems a bit wonky to me, in that the code doesn't check that DarkResume::IsEnabled() is enabled before sending the signal. It does call CanSafelyExitDarkResume() first, though. Ravi is making changes for (I think) http://b/67747385 that should make it possible for powerd to identify the wake source, which should make it possible for us to follow the normal dark resume code path in this scenario. In the meantime, I think I'm going to drop this unused (and untested) code from the normal-resume path in my refactoring change, while adding a TODO pointing at this bug to make sure that it's handled properly when possible.
,
Dec 8 2017
,
Jul 16
Hi,
Nomenclature:
suspend cancel : kernel bailed out of suspend attempt due to a increment in the wakeup count
suspend failure : kernel bailed out of suspend as one of the driver's callback returned -1 (failure).
Current way of handling wake events seem to be broken in the following remote case.
1. System tries to suspend (powerd_dbus_suspend with wakeup_count).
2. In the path to suspend (when the kernel is calling the driver callbacks), user presses a key.
3. Currently we wake up, see that wake up count incremented and suspend again without turning the screen on.
4. Thus a wake up event when suspending is always lost (unless external wake up count is provided which happens only in autotests).
Possible Fix:
when suspend is cancelled (not failure) do a full resume (turns the screen on too).
Side effects of the fix:
1. In case if any of the drivers are malfunctioning (increments wakeup count on suspend path) we would see device going through the full idle cycle (current default timeout is 6 mins 30 secs) before retrying.
2. Might annoy the user in case of a malfunctioning driver.
Note that if dark resume is enabled, we will turn the screen only for user initiated (input devices) resumes.
,
Jul 16
#3: I thought that it at least used to be the case that userspace would receive the key event that canceled suspend, and that when Chrome reported it to powerd, powerd would abort the suspend request instead of trying to suspend again. Does that not work in some cases? If you're typing "powerd_dbus_suspend" directly into a shell on your test device, it's possible that the canceling key event might not be reported to powerd immediately if a report of user activity was already sent within the previous five seconds.
,
Aug 2
|
||||
►
Sign in to add a comment |
||||
Comment 1 by bugdroid1@chromium.org
, Dec 2 2017