After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 659066 - Suspend when inactive doesn't work
Suspend when inactive doesn't work
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: power
3.1.x
Other Linux
: Normal major
: ---
Assigned To: Richard Hughes
gnome-settings-daemon-maint
: 660074 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-09-14 15:47 UTC by Ionut Biru
Modified: 2011-09-26 17:09 UTC
See Also:
GNOME target: 3.2
GNOME version: ---


Attachments
trivial patch to prevent debug spew (2.37 KB, patch)
2011-09-23 11:29 UTC, Richard Hughes
committed Details | Review
fix this bug (8.85 KB, patch)
2011-09-23 11:30 UTC, Richard Hughes
committed Details | Review

Description Ionut Biru 2011-09-14 15:47:58 UTC
Since the move from gnome-power-manager to g-s-d, the suspend when inactive feature doesn't work.

gnome-settings-daemon 3.1.91
upower 0.9.13

System is a laptop without a battery, always on AC
Comment 1 Matthias Clasen 2011-09-14 16:59:58 UTC
I'm seeing the same, but it is not suspending after the set time on battery either, it seems.
Comment 2 Matthias Clasen 2011-09-16 01:18:22 UTC
This is more crazy idle timeout mess.

there's a X idle timer that gets set for 'idle dim' - but has the side-effect of setting (or not) the x_idle variable. Which in turn gets checked when we get the session idle change event. On my system, idle dim seems turned off, so when the session goes idle, the suspend never happens.
Comment 3 Matthias Clasen 2011-09-16 01:29:30 UTC
So I guess you either need to set x_idle to TRUE unconditionally when idle dim is disabled, or remove the early exits from idle_evaluate
Comment 4 Richard Hughes 2011-09-16 16:17:50 UTC
commit dfd4505e4ebd21d5c6b06c039852a4c13896b8e8
Author: Richard Hughes <richard@hughsie.com>
Date:   Fri Sep 16 17:16:55 2011 +0100

    power: Do not use G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES when we want to read properties
    
    Hopefully resolves https://bugzilla.gnome.org/show_bug.cgi?id=659066
Comment 5 Richard Hughes 2011-09-16 16:23:16 UTC
(In reply to comment #3)
> So I guess you either need to set x_idle to TRUE unconditionally when idle dim
> is disabled, or remove the early exits from idle_evaluate

Ahh, I've got dimming enabled on AC, so I didn't see this. I'll debug more tomorrow.
Comment 6 Ionut Biru 2011-09-19 18:46:21 UTC
can a possible reason be that /usr/lib/gnome-settings-daemon-3.0/libpower.so doesn't link to libxext?
Comment 7 Matthias Clasen 2011-09-19 19:37:40 UTC
unlikely
Comment 8 Matthias Clasen 2011-09-22 20:54:04 UTC
Richard, any progress on this ?
Comment 9 Richard Hughes 2011-09-23 11:29:15 UTC
Created attachment 197343 [details] [review]
trivial patch to prevent debug spew
Comment 10 Richard Hughes 2011-09-23 11:30:00 UTC
Created attachment 197344 [details] [review]
fix this bug

Fix this bug. See commit message. Thanks.
Comment 11 Matthias Clasen 2011-09-23 12:27:55 UTC
Review of attachment 197343 [details] [review]:

::: plugins/power/gsd-power-manager.c
@@ +2731,3 @@
 
+                /* reset brightness if we dimmed */
+                if (manager->priv->pre_dim_brightness > 0) {

Below you initialize to -1. Should this check be >= 0 ?
Admittedly brightness of 0 is maybe not that useful. But still an allowed value ?
Comment 12 Richard Hughes 2011-09-23 12:33:31 UTC
(In reply to comment #11)
> Review of attachment 197343 [details] [review]:
> 
> ::: plugins/power/gsd-power-manager.c
> @@ +2731,3 @@
> 
> +                /* reset brightness if we dimmed */
> +                if (manager->priv->pre_dim_brightness > 0) {
> 
> Below you initialize to -1. Should this check be >= 0 ?
> Admittedly brightness of 0 is maybe not that useful. But still an allowed value
> ?

Yes, i pondered that. It's still technically allowed, but it's not useful to set. Using >= 0 would be more correct I suppose. Did the rest of the patch work?
Comment 13 Matthias Clasen 2011-09-23 12:35:10 UTC
Review of attachment 197344 [details] [review]:

::: plugins/power/gsd-power-manager.c
@@ +144,3 @@
 typedef enum {
         GSD_POWER_IDLE_MODE_NORMAL,
+        GSD_POWER_IDLE_MODE_DIM,        /* set even if no dim action */

Tbh, I don't think this comment helps.
If these modes don't mean what their name says, they should be renamed.
And it would be good to have a comment explaining them all.

@@ +2662,3 @@
+                }
+                if (!ret) {
+                        g_debug ("not dimming due to policy");

Seeing this g_debug here, I realize that we should set up per-plugin log domains, maybe.

@@ +3267,3 @@
                 return;
         }
+        if (g_strcmp0 (key, "idle-dim-time") == 0) {

keys going away ? obsolete keys ? this should probably be a separate commit.
Comment 14 Ionut Biru 2011-09-23 13:25:42 UTC
i've tried this patches and still doesn't work.

nome-settings-daemon:11090): power-plugin-DEBUG: idletime alarm: 1
(gnome-settings-daemon:11090): power-plugin-DEBUG: normal to dim
(gnome-settings-daemon:11090): power-plugin-DEBUG: Doing a state transition: dim
(gnome-settings-daemon:11090): power-plugin-DEBUG: not dimming due to policy
(gnome-settings-daemon:11090): power-plugin-DEBUG: setting up blank callback for 600s
(gnome-settings-daemon:11090): power-plugin-DEBUG: setting up sleep callback 300s
(gnome-settings-daemon:11090): housekeeping-plugin-DEBUG: housekeeping: checking thumbnail cache size and freshness
(gnome-settings-daemon:11090): power-plugin-DEBUG: session is not idle, cannot SLEEP
(gnome-settings-daemon:11090): power-plugin-DEBUG: Received gnome session status change
(gnome-settings-daemon:11090): power-plugin-DEBUG: Received gnome session status change
(gnome-settings-daemon:11090): power-plugin-DEBUG: idletime reset
(gnome-settings-daemon:11090): power-plugin-DEBUG: Doing a state transition: normal
(gnome-settings-daemon:11090): power-plugin-DEBUG: X not idle


note that i set up suspend on idle 5 minutes and screensaver at 10 minutes.
the screensaver kicks on time
Comment 15 Bastien Nocera 2011-09-23 13:29:02 UTC
(In reply to comment #13)
<snip>
> @@ +2662,3 @@
> +                }
> +                if (!ret) {
> +                        g_debug ("not dimming due to policy");
> 
> Seeing this g_debug here, I realize that we should set up per-plugin log
> domains, maybe.

We already do.
PLUGIN_CFLAGS="-DG_LOG_DOMAIN=\"\\\"\$(plugin_name)-plugin\\\"\""
in configure.ac
Comment 16 Richard Hughes 2011-09-23 14:41:48 UTC
(In reply to comment #13)
> Review of attachment 197344 [details] [review]:
> 
> ::: plugins/power/gsd-power-manager.c
> @@ +144,3 @@
>  typedef enum {
>          GSD_POWER_IDLE_MODE_NORMAL,
> +        GSD_POWER_IDLE_MODE_DIM,        /* set even if no dim action */
> 
> Tbh, I don't think this comment helps.
> If these modes don't mean what their name says, they should be renamed.
> And it would be good to have a comment explaining them all.

Sure, valid. What about s\GSD_POWER_IDLE_MODE_DIM\GSD_POWER_IDLE_MODE_XIDLE?

> @@ +3267,3 @@
>                  return;
>          }
> +        if (g_strcmp0 (key, "idle-dim-time") == 0) {
> 
> keys going away ? obsolete keys ? this should probably be a separate commit.

Well, it's the fact that there's no point them being there with this reshuffle.
Comment 17 Richard Hughes 2011-09-23 14:42:57 UTC
(In reply to comment #14)
> i've tried this patches and still doesn't work.
> (gnome-settings-daemon:11090): power-plugin-DEBUG: session is not idle, cannot
> SLEEP

That's a different issue. We check if the session is really idle before actually doing the suspend, so it's likely there's a sanity check there.
Comment 18 Matthias Clasen 2011-09-23 17:22:32 UTC
With these patches, I've actually seen my machine suspend when idle for a while.
What makes testing all this horribly confusing is that it is all set off by the session idle-delay, which is usually set to some largish value (600s).

Setting 

org.gnome.desktop.session idle-delay 30
org.gnome.settings-daemon.plugins.power idle-sleep-ac-timeout 30

Make my machine suspend after a short wait of ~ a minute.
Comment 19 Ionut Biru 2011-09-23 17:41:33 UTC
it's idle-sleep-ac-timeout  because i don't have that. i do have sleep-inactive-ac-timeout

gsettings get org.gnome.desktop.session idle-delay 
uint32 600

gsettings get org.gnome.settings-daemon.plugins.power sleep-inactive-ac-timeout
1800

i'll try tonight to see if it's sleeping after screensaver is started
Comment 20 Ionut Biru 2011-09-24 05:47:42 UTC
suspend works for me
Comment 21 Richard Hughes 2011-09-26 08:52:33 UTC
*** Bug 660074 has been marked as a duplicate of this bug. ***
Comment 22 Richard Hughes 2011-09-26 08:57:04 UTC
Comment on attachment 197343 [details] [review]
trivial patch to prevent debug spew

I've committed this to master.
Comment 23 Richard Hughes 2011-09-26 09:01:48 UTC
(In reply to comment #18)
> What makes testing all this horribly confusing is that it is all set off by the
> session idle-delay, which is usually set to some largish value (600s).

This is basically a sanity check, as the session idle is controlled by things like the inhibits available. We're never going to want to set the suspend-on-idle value in the power control panel to less than 10 minutes, and so I don't think there is a problem here.
Comment 24 Richard Hughes 2011-09-26 09:03:41 UTC
Comment on attachment 197344 [details] [review]
fix this bug

I've committed this to master with minor fixes.

I think it's a bit late for gnome 3.2, but we can push this to gnome-3-2 after the hard code freeze opens so it's fixed in 3.2.1.