GNOME Bugzilla – Bug 729331
power: Fix restarting of the lid inhibitor check timer
Last modified: 2014-05-05 14:31:52 UTC
Ok, I think I got to the bottom of this now. The patch in bug 698132 introduced a regression by making us always uninhibit on lid close. Then the patch in bug 708160 fixed that but instead made us never unhibit since it just stops the timer and then on randr events we were not able to restart the timer due to a logic bug in restart_inhibit_lid_switch_timer(). The patch in bug 719975 also partially undid the intention of the patch in bug 698132 which was to stop polling for external monitors. The patch I'm attaching makes it all work again, including not having to poll, and hopefully makes the logic simpler and clearer. Applies to 3.10, 3.12 and master.
Created attachment 275543 [details] [review] power: Fix restarting of the lid inhibitor check timer We were trying to restart the timer when randr events tell us that there are no longer any external monitors connected. But we were only actually restarting if there was a timer scheduled already, i.e. in case that there was no previous timer we wouldn't start a new one either and thus we'd never clear the inhibitor. This would happen when closing the lid, since we'd stop the timer on do_lid_closed_action(). This commit simplifies the logic and makes the timer be one shot again as was intended in 1e14c67 . The logic now is: 1. Only take the inhibitor and start the timer on randr events, unconditionally. If there was a timer running from a previous randr event we stop it and start a new one so that the "grace period" is always counted since the last randr event. 2. On the timer callback we check whether we should keep the inhibitor and thus we either suspend at that point or don't. In any case, we stop checking until we have further randr events. 3. When the lid closes, we only have to check if we should lock the session. If the timer has elapsed already and we should suspend, then the inhibitor is already gone and logind starts suspending right away. Otherwise we suspend when the timer elapses and we remove the inhibitor, unless we see an external monitor at that point.
Review of attachment 275543 [details] [review]: Looks correct. ::: plugins/power/gsd-power-manager.c @@ -1138,3 @@ - if (manager->priv->inhibit_lid_switch_timer_id != 0) { - stop_inhibit_lid_switch_timer (manager); - g_debug ("restarting lid close safety timer"); Can you leave the debug output for that one as well?
Pushed with the debug output. Attachment 275543 [details] pushed as bb47f3d - power: Fix restarting of the lid inhibitor check timer