Drive Always-on-display wakeup events using a timerfd - #285
FlorentRevest wants to merge 1 commit into
Conversation
|
This goes hand in hand with AsteroidOS/meta-asteroid#296 |
| return; | ||
| } | ||
|
|
||
| // The CLOCK_REALTIME_ALARM RTC wakeup keeps the AP awake long enough to |
There was a problem hiding this comment.
Do we actually know how long it stays awake? Iirc in the previous approach MCE held a wakelock.
There was a problem hiding this comment.
Hopefully the new explanation is clearer. The kernel's alarmtimer mechanism seems to hold a wakeup source when an alarm-class timer fires, until the fd is serviced. As far as I could test on my aurora, the frame rendering seems to happen within that time frame so. Maybe there are some oddities left, we should keep an eye on it but that seems to work for me. At least better than the timed case which was broken on aurora for some reason
There was a problem hiding this comment.
Mh well, I've been arguing with my slopbot and it's actually unsure at this point. My personal take is: it seems to work without a wake lock at the moment and introducing so I'm tempted to go ahead with this and see what happens. If it turns out to be an issue, we can consider introducing a wakelock but that will require CAP_BLOCK_SUSPEND too and maybe exposing some sysfs files too so I'm not too keen right unless that's proven necessary.
There was a problem hiding this comment.
If this ends up needing a wakelock, can we please avoid using the wakelock kernel API directly, and instead talk to MCE over DBUS? Obviously this would be another dependency on MCE, but we have plenty of those.
The kernel wakelock API is just not very good (it relies on writing a string into a lock and unlock files). It feels a lot safer if we just have one daemon that does all the wakelocking.
| // Alarm-class timer for the ambient (AOD) minute tick. CLOCK_REALTIME_ALARM | ||
| // wakes the AP out of autosleep, so the watchface still updates off-charger, | ||
| // and it tracks wall-clock so ticks stay aligned to :00 across NTP/timezone | ||
| // changes. Requires CAP_WAKE_ALARM, granted to the launcher unit. |
There was a problem hiding this comment.
Have you checked that this is set in the systems service?
We have the service file also in the meta-smartwatch layer (should probably merge with meta-asteroid someday)
There was a problem hiding this comment.
Ah yeah, it's a little confusing because that's done in yet another pull request uh https://github.com/AsteroidOS/meta-asteroid/pull/290/commits
The whole timerfd conversion is to untangle both dsme, iphb and timed so currently it's a bit of a soup but eventually things will be more straightforward to understand hopefully!
|
Forgot to mention but this is great! Keep it up 💪 |
9789cf4 to
e00d802
Compare
…imerfd The always-on-display clock was repainted once a minute by scheduling a 'wakeup' event in timed, which timed re-broadcast over D-Bus and mce relayed back to the compositor as setAmbientUpdatesEnabled(true). That round-trip through two other daemons was fragile, and because timed's event did not wake the AP the clock froze whenever the device autosleeps (e.g. off-charger). Arm a CLOCK_REALTIME_ALARM timerfd for the next wall-clock minute instead, watched with a QSocketNotifier; on expiry re-enable ambient updates and let the existing re-arm path schedule the following minute. The RTC delivers the alarm through suspend, so the ambient watchface keeps ticking off-charger, and the mechanism is self-contained in the compositor. The launcher already holds CAP_WAKE_ALARM, and the kernel keeps the AP awake long enough after an RTC alarm to composite the frame, so no wakelock is required.
e00d802 to
e9c66b3
Compare
|
I feel a bit mixed on this. As I mentioned in a comment above, the timerfd and wakelock APIs are a bit austere, and they're not something that applications would want to use. Of course, the AOD usecase is a little bit special, as it needs to happen exactly once a minute and it happens more frequently than most other background tasks would. So maybe this solution is appropriate here, but some thought does need to be given to background tasks would work elsewhere. |
If your concern is API complexity, we could trivially have a C++ library that wraps timerfd's low level file descriptor APIs and what not with a higher level API (like QTimer). The part that matters to me is that we no longer need complex IPCs and a privileged daemon to do such simple things. I don't care what abstraction level we use to communicate with timerfd, what matters to me is that architecturally it's a lot simpler to have each process be able to directly ask the kernel for wake up events using a mainline API rather than have this crazy iphb custom socket protocol communicate with dsme which itself has crazy abstractions for legacy wakeup mechanisms that none of our watches have. |
|
@dodoradio Are you ok with the latest comments? |
|
I think AOD can be a good reason to talk to the kernel directly, since we can almost see it as a hardware feature. So yeah, I think this is good to merge! Regarding other usecases however, a response to kido's comment:
I think some level of abstraction is actually desirable. Not just an API wrapper, but something that abstracts some of the design decisions from application developers.
Obviously timed doesn't do all of this, and it has accumulated issues in both design and implementation, like the dependency on DSME, or the very rigid and tangled alarmclock system. But I don't think it's worth completely throwing out the idea of wakeup middleware, as it seems kinda necessary for more complex background usecases, and it's something that seems to be missing on other mobile linuxes. (Apparently systemd.timer hits some of these usecases, but I've not seen any well-developed implementations of power-aware background sync with it, so I'm not sure) |
No description provided.