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 751794 - Add "power-save-mode" property
Add "power-save-mode" property
Status: RESOLVED FIXED
Product: gnome-desktop
Classification: Core
Component: libgnome-desktop
3.17.x
Other Linux
: Normal normal
: ---
Assigned To: Desktop Maintainers
Desktop Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-07-01 17:42 UTC by Bastien Nocera
Modified: 2015-07-02 17:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gnome-rr: Add dpms-mode property (6.37 KB, patch)
2015-07-01 18:33 UTC, Bastien Nocera
none Details | Review
gnome-rr: Remove unused GNOME_RR_DPMS_DISABLED (1.23 KB, patch)
2015-07-02 14:41 UTC, Bastien Nocera
committed Details | Review
gnome-rr: Add dpms-mode property (5.33 KB, patch)
2015-07-02 14:41 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2015-07-01 17:42:05 UTC
This would allow the orientation plugin to stop monitoring the accelerometer when the tablet's screen is off, and start again when it's on.
Comment 1 Bastien Nocera 2015-07-01 18:33:22 UTC
Created attachment 306553 [details] [review]
gnome-rr: Add dpms-mode property

So that other parts of the desktop environment can monitor the state of
the power saving mode, and disable their own power consuming tasks when
the screens are off.
Comment 2 Rui Matos 2015-07-02 14:32:34 UTC
Review of attachment 306553 [details] [review]:

::: libgnome-desktop/gnome-rr-private.h
@@ +47,3 @@
     GdkWindow *			gdk_root;
     ScreenInfo *		info;
+    GnomeRRDpmsMode		dpms_mode;

Looks like we don't need this since the value is always cached in the MetaDBusDisplayConfig proxy

::: libgnome-desktop/gnome-rr.c
@@ +679,3 @@
+                         GnomeRRScreen *self)
+{
+        g_object_notify (object, "dpms-mode");

s/object/self

@@ +899,3 @@
+                    GNOME_RR_DPMS_UNKNOWN,
+                    G_PARAM_READWRITE |
+                    G_PARAM_CONSTRUCT_ONLY |

not construct only, copy/paste ?

@@ +1184,2 @@
     meta_dbus_display_config_set_power_save_mode (screen->priv->proxy, power_save);
+    screen->priv->dpms_mode = power_save;

not needed

@@ +2240,3 @@
+                        { GNOME_RR_DPMS_SUSPEND, "GNOME_RR_DPMS_SUSPEND", "suspend" },
+                        { GNOME_RR_DPMS_OFF, "GNOME_RR_DPMS_OFF", "off" },
+                        { GNOME_RR_DPMS_DISABLED, "GNOME_RR_DPMS_DISABLED", "disabled" },

I had never noticed we had this DISABLED value. And we don't use it anywhere, I think it's better to remove it in a patch before this one.

If it gets passed in to _set_dpms_mode() for instance, it will trigger an assert.
Comment 3 Bastien Nocera 2015-07-02 14:41:20 UTC
Created attachment 306627 [details] [review]
gnome-rr: Remove unused GNOME_RR_DPMS_DISABLED

GNOME_RR_DPMS_DISABLED is one of the possible values of GnomeRRDpmsMode
that's not currently used, and would trigger an assertion if passed to
gnome_rr_screen_set_dpms_mode().
Comment 4 Bastien Nocera 2015-07-02 14:41:24 UTC
Created attachment 306628 [details] [review]
gnome-rr: Add dpms-mode property

So that other parts of the desktop environment can monitor the state of
the power saving mode, and disable their own power consuming tasks when
the screens are off.
Comment 5 Rui Matos 2015-07-02 16:06:18 UTC
Review of attachment 306627 [details] [review]:

Anyway, sure

::: configure.ac
@@ +29,3 @@
 # - If the interface is the same as the previous version, change to C:R+1:A
 
+LT_VERSION=12:0:0

We just bumped this for tiled monitor support and there hasn't been a release yet, do we really need this?
Comment 6 Rui Matos 2015-07-02 16:07:04 UTC
Review of attachment 306628 [details] [review]:

++
Comment 7 Bastien Nocera 2015-07-02 17:04:47 UTC
(In reply to Rui Matos from comment #5)
> Review of attachment 306627 [details] [review] [review]:
> 
> Anyway, sure
> 
> ::: configure.ac
> @@ +29,3 @@
>  # - If the interface is the same as the previous version, change to C:R+1:A
>  
> +LT_VERSION=12:0:0
> 
> We just bumped this for tiled monitor support and there hasn't been a
> release yet, do we really need this?

We bumped C and A for the tiled monitor support, because it's binary compatible (just new symbols). This breaks ABI.
Comment 8 Bastien Nocera 2015-07-02 17:13:00 UTC
Added some code to gnome-rr-debug.c as well, and fixed a compilation warning.

Attachment 306627 [details] pushed as cb29d64 - gnome-rr: Remove unused GNOME_RR_DPMS_DISABLED
Attachment 306628 [details] pushed as 367bfb7 - gnome-rr: Add dpms-mode property