GNOME Bugzilla – Bug 668703
Blanking only works right when gsettings align
Last modified: 2012-11-14 11:48:32 UTC
For the screen to blank at the right time org.gnome.settings-daemon.plugins.power.idle-dim-time + (org.gnome.settings-daemon.plugins.power.sleep-display-ac or org.gnome.settings-daemon.plugins.power.idle-dim-time.sleep-display-battery) has to be the same as org.gnome.desktop.session.idle-delay + FADE_TIMEOUT defined in gnome-screensaver/src/gs-monitor.c The first set of values is used by gnome-settings-daemon/plugins/power code and the second set of values is used by gnome-screensaver code. The power plugin code sets up its own alarms based on the values, with the 'sleep-display-ac' or 'sleep-display-battery' timeout being set in gsd-power-manager.c::idle_evaluate() only after 'idle-dim-time' is reached and manager->priv->x_idle is set to TRUE in gsd-power-manager.c::idle_idletime_alarm_expired_cb() . The screensaver code relies on the session's StatusChanged signal that changes the session's status to idle after 'idle-delay' and then uses 10 seconds FADE_TIMEOUT to fade out the screen. 'sleep-display-ac', 'sleep-display-battery', and 'idle-delay' are all updated when System Settings -> Screen -> "Turn screen off when inactive for:" value is updated. There is no control in System Settings to change the 'idle-dim-time' whose default value is and has to be 10 seconds, but there is a control to disable it - System Settings -> Screen -> "Dim screen to save power". If 'idle-dim-time' is not changed directly, things work fine whether it is disabled or enabled. However, the way things are implemented now means that blanking the screen stops happening at the right time if any of the gsetting values are changed directly.
After testing, not only manual setting is affected. Updating is wrong (sleep-display-* are set to idle-delay, rather than idle-delay + FADE_TIMEOUT - idle-dim-time), and defaults are wrong too: session.idle-delay: 600 (ok), power.idle-dim-time: 90 (ok), power.sleep-display-*: 600, should be 600 + 10 - 90 = 220 otherwise by default you get 80 seconds of "screensaver" (btw, idle-brightness should also be changed to 10 or a fraction of current brightness, otherwise you risk an "idle" brightness higher than normal...)
Looking at it again, having the blanking timeout depend on idle-dim-time is soooo broken, so I prepared a patch that reworks the blank/sleep timeouts in a way that current control center code becomes fine, and so does the 90s default for idle-dim-time. This could still break if sleep-display-* is set to to anything but idle-delay, but I'd say this is OK, as there is no way to screw that with the UI, and if you go down to GSettings you're supposed to know what you're doing.
Created attachment 221373 [details] [review] power: fix idle blank and sleep timeouts The previous logic for blank and sleep timeouts was broken, as it setup the timeouts only after the timeout for brightness dimming (which is logically unrelated). Refactor it to use GpmIdletime instead of GTimeout for blank and sleep (so that actual idle times are considered), and to take in account screensaver fading before blanking. The net effect of this patch is that the idle-dim-time has now no effect on blank or sleep, and that setting session idleness to the same time as sleep-display-* causes the screen to power off at the end of the screensaver animation (which is the desired behaviour)
ping - can somebody review Giovanni's patch please?
Review of attachment 221373 [details] [review]: Looks good to me, and seems to work fine in my limited testing. Thanks for looking at this.
Attachment 221373 [details] pushed as b081f0a - power: fix idle blank and sleep timeouts
The patch works great if sleep-inactive-*-timeout > sleep-display-* but it has a weird result if sleep-display-* >= sleep-inactive-*-timeout If I set brightness settings "Turn screen off when inactive for" as 10 minutes (which implies sleep-display-ac, sleep-display-battery, idle-delay is 600) and power settings "Suspend when inactive for " as 5 minutes, The actual result of the screen off time is correct as 10 minutes, but the suspend time is 20 minutes, not 5 or 10 or even 15 mins. The log is attached below: ----TIME at IDLE+5 min (gnome-settings-daemon:4344): power-plugin-DEBUG: idletime alarm: 3 (gnome-settings-daemon:4344): power-plugin-DEBUG: session is not idle, cannot SLEEP ----TIME at IDLE+10 min (gnome-settings-daemon:4344): power-plugin-DEBUG: Received gnome session status change (gnome-settings-daemon:4344): power-plugin-DEBUG: setting up blank callback for 600s (gnome-settings-daemon:4344): power-plugin-DEBUG: setting up sleep callback 300s (gnome-settings-daemon:4344): power-plugin-DEBUG: idletime alarm: 2 (gnome-settings-daemon:4344): power-plugin-DEBUG: Doing a state transition: blank (gnome-settings-daemon:4344): power-plugin-DEBUG: keyboard toggle on ----TIME at IDLE+20 min (gnome-settings-daemon:4344): power-plugin-DEBUG: idletime alarm: 3 (gnome-settings-daemon:4344): power-plugin-DEBUG: sending to SLEEP (gnome-settings-daemon:4344): power-plugin-DEBUG: Doing a state transition: sleep (gnome-settings-daemon:4344): power-plugin-DEBUG: gnome-screensaver activated, doing gnome-screensaver lock
The reason of this weird behavior is in line 3150 of gsd-power-manager.c: when setting the timeout, it gets doubled (or quadrupled, in this case) if it would be already expired at the time it is configured. A fix would be either to avoid this doubling (since I find it unlikely that by the time we set up the alarms the first time the machine has been idle for more than 1 minute), or not to reconfigure alarms when the session goes idle. Still, this would not allow for a sleep-inactive-* smaller than idle-delay, since current code requires the screensaver to be already active before suspending. I'm not sure that's right, I think it was added to account for gnome-sessions inhibitors, but we already check them before going to sleep.
Created attachment 226100 [details] [review] When timeout_blank >= timeout_sleep, adjust sleep timer to blank timer with slightly delay. The previous behavior is that when sleep-inactive-* smaller or equal than idle-delay/sleep-display-*, system will sleep at idle-delay/sleep-display-* time. So system will sleep at timing which is the bigger one of idle-delay/sleep-display-* or sleep-inactive-*. To mimic the behavior, I wrote a patch which adjust the sleep timer to blank timer when sleep-display-* >= sleep-inactive-*-timeout, just add a slightly delay and add SCREENSAVER_FADE_TIME into account should be enough.
(driveby reopen, since i just happened to run across this bug report and noticed there's a new patch on it)
Comment on attachment 226100 [details] [review] When timeout_blank >= timeout_sleep, adjust sleep timer to blank timer with slightly delay. Looks sane to me, thanks.