Bug 22574 - immediately suspends on startup when lid is closed
Summary: immediately suspends on startup when lid is closed
Status: RESOLVED FIXED
Alias: None
Product: upower
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Richard Hughes
QA Contact:
URL: https://launchpad.net/bugs/385135
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-01 01:28 UTC by Martin Pitt
Modified: 2009-07-06 10:24 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
suppress initial change event (1.35 KB, patch)
2009-07-01 02:59 UTC, Martin Pitt
Details | Splinter Review
git formatted patch of the above (1.57 KB, patch)
2009-07-01 04:03 UTC, Martin Pitt
Details | Splinter Review
Add a notify flag to set_lid_is_closed and use it only on input cold plug (2.38 KB, patch)
2009-07-06 08:20 UTC, Loïc Minier
Details | Splinter Review
git format patch of notify flag addition (2.81 KB, patch)
2009-07-06 08:24 UTC, Loïc Minier
Details | Splinter Review
log of dkp --verbose exposing the issue (before patch) (55.63 KB, text/x-log)
2009-07-06 08:36 UTC, Loïc Minier
Details
log of dkp --verbose once fixed (with the patch) (167.60 KB, text/x-log)
2009-07-06 08:36 UTC, Loïc Minier
Details

Description Martin Pitt 2009-07-01 01:28:01 UTC
This was originally filed against gnome-power-manager in http://bugzilla.gnome.org/show_bug.cgi?id=585228, but I think it belongs to dk-p.

2.27.1 now enables suspend on lid close by default. This is great in principle,
but it falls over if you keep your laptop in a docking station, and thus the
lid is closed all the time. When g-p-m starts up, the recent introduction of the "lid-is-closed" property into dk-p [1] and g-p-m [2] cause this:

 1. g-p-m starts the very first time (such as in gdm)
 2. This D-BUS activates devicekit-power-daemon
 3. dk-p sends a lid event:

TI:10:18:00     TH:0x17f6050    FI:dkp-input.c  FN:dkp_input_coldplug,255
 - using /sys/devices/LNXSYSTM:00/device:00/PNP0C0D:00/input/input0/event0 for lid event
TI:10:18:00     TH:0x17f6050    FI:dkp-daemon.c FN:dkp_daemon_set_lid_is_closed,108
 - lid_is_closed=1

 4. g-p-m picks it up:

TI:10:06:35     TH:0xcb3d00     FI:gpm-button.c
FN:gpm_button_client_changed_cb,554
 - ************* gpm_button_client_changed_cb: lid: 1
TI:10:06:35     TH:0xcb3d00     FI:gpm-button.c FN:gpm_button_emit_type,101
 - emitting button-pressed : lid-down
TI:10:06:35     TH:0xcb3d00     FI:gpm-manager.c       
FN:button_pressed_cb,737
 - Button press event type=lid-down
TI:10:06:35     TH:0xcb3d00     FI:gpm-manager.c       
FN:lid_button_pressed,645
 - **************** lid_button_pressed 1
TI:10:06:35     TH:0xcb3d00     FI:gpm-manager.c       
FN:lid_button_pressed,658
 - Performing AC policy
TI:10:06:35     TH:0xcb3d00     FI:gpm-manager.c       
FN:manager_policy_do,408
 - policy: /apps/gnome-power-manager/buttons/lid_ac

IMHO dk-p should not emit a lid event on startup, since g-p-m and other clients shoulnd't special-case dk-p activation. They need to work also on second login, etc., when dk-p is already running.


[1] http://cgit.freedesktop.org/DeviceKit/DeviceKit-power/commit/?id=7bd2dfefcb88d2e40402f4e1272dd70255d34b87
[2] http://git.gnome.org/cgit/gnome-power-manager/commit/?id=90000eb88dbc4ef404f6be0940a0b64572c588ff
Comment 1 Martin Pitt 2009-07-01 02:59:46 UTC
Created attachment 27290 [details] [review]
suppress initial change event

What do you think about this patch? It works for me.
Comment 2 Richard Hughes 2009-07-01 03:21:14 UTC
(In reply to comment #1)
> What do you think about this patch? It works for me.

Yup, please apply. Looks fine to me.

Comment 3 Martin Pitt 2009-07-01 03:26:22 UTC
> Yup, please apply. Looks fine to me.

Was that for me? (-EPERM)
Comment 4 Richard Hughes 2009-07-01 03:30:13 UTC
(In reply to comment #3)
> Was that for me? (-EPERM)

Sure. If you want commit then open bug in fd.o, cc me, and I'll give my approval.

Are you sure that you've not got commit? -- I seem to remember that you have commit on HAL, and the group list is the same as that IIRC.

If you can't be arsed, say so and I'll commit on your behalf :-)
Comment 5 Martin Pitt 2009-07-01 03:58:20 UTC
Yes, I already tried that a while ago:

$ ls -ld  /git/DeviceKit/DeviceKit-power.git/
drwxrwsr-x 7 david devicekit 4096 2008-08-01 03:12 /git/DeviceKit/DeviceKit-power.git/

$ groups
freedesktop hal

So it's a different group.

I requested upload privs in bug 22578.

Thanks!
Comment 6 Martin Pitt 2009-07-01 04:03:54 UTC
Created attachment 27294 [details] [review]
git formatted patch of the above

For your convenience. :-)
Comment 7 Richard Hughes 2009-07-01 04:22:11 UTC
(In reply to comment #6)
> Created an attachment (id=27294) [details]
> git formatted patch of the above

Committed. Thanks dude.
Comment 8 Loïc Minier 2009-07-06 08:18:02 UTC
Hi folks

This breaks the first suspend resume for me when I close the lid.

What happens is that:
1. on startup dkp assumes lid is closed
2. on input coldplug it detects lid as truly closed; this doesn't set initialized though
3. on the first lid event (lid close to suspend in my case) the event is swallowed because of this patch

I could offer a fix to set initialized in case 2. as well, but I don't like the general approach very much.  Instead I'd like to offer a different approach which is to add a flag to set_lid_is_closed to disable notifications and use that only on input coldplug.
Comment 9 Loïc Minier 2009-07-06 08:20:18 UTC
Created attachment 27417 [details] [review]
Add a notify flag to set_lid_is_closed and use it only on input cold plug
Comment 10 Loïc Minier 2009-07-06 08:24:23 UTC
Created attachment 27418 [details] [review]
git format patch of notify flag addition
Comment 11 Loïc Minier 2009-07-06 08:36:11 UTC
Created attachment 27419 [details]
log of dkp --verbose exposing the issue (before patch)
Comment 12 Loïc Minier 2009-07-06 08:36:50 UTC
Created attachment 27420 [details]
log of dkp --verbose once fixed (with the patch)
Comment 13 Martin Pitt 2009-07-06 09:22:20 UTC
Thanks, Loic. I like this approach better. I tried to apply it to 009, but unfortunately I can't really test it since g-p-m bails out with

$ gnome-power-manager --debug

(gnome-power-manager:4054): devkit-power-gobject-WARNING **: unhandled property 'recall-vendor'
**
devkit-power-gobject:ERROR:dkp-device.c:193:dkp_device_collect_props_cb: code should not be reached
Aborted (core dumped)

I'll investigate this, and report back about this patch later here.
Comment 14 Richard Hughes 2009-07-06 09:28:45 UTC
(In reply to comment #13)
> Thanks, Loic. I like this approach better. I tried to apply it to 009, but
> unfortunately I can't really test it since g-p-m bails out with

You need to grab a patch for gnome-power-manager:

commit 655dc92e5b2259d008021d5258e77b00e3d5bfad
Author: Richard Hughes <richard@hughsie.com>
Date:   Fri Jul 3 10:11:09 2009 +0100

    Be less asserty if newer enums get added to DeviceKit-power

If you're using a new DKP with an old g-p-m.
Comment 15 Richard Hughes 2009-07-06 09:30:34 UTC
(In reply to comment #10)
> Created an attachment (id=27418) [details]
> git format patch of notify flag addition

I've applied this, thanks.

Richard.
Comment 16 Martin Pitt 2009-07-06 10:24:51 UTC
Ah, apparently 009 broke ABI without bumping shlibs. A mere g-p-m rebuild was enough.

I tested the patch now and confirm that it works just fine in both cases (lid closed in docking, and lid open/first suspend).

Thanks!


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.