Bug 36818 - Wrong dock detection on hybrid graphics notebooks
Summary: Wrong dock detection on hybrid graphics notebooks
Status: RESOLVED FIXED
Alias: None
Product: upower
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: medium major
Assignee: Richard Hughes
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-03 19:25 UTC by wrobell
Modified: 2014-05-07 07:50 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments
intel card (1.45 KB, application/octet-stream)
2011-10-24 13:38 UTC, BDenis
Details
radeon card (2.15 KB, application/octet-stream)
2011-10-24 13:39 UTC, BDenis
Details
Patch to check 'dpms' attr in addition to 'status' attr (1.22 KB, patch)
2012-01-08 14:51 UTC, Alexei Colin
Details | Splinter Review
Attempt to detect docking stations by platform/dock_station (1.81 KB, patch)
2012-02-15 17:50 UTC, Armando Di Cianno
Details | Splinter Review
linux: Detect docked docking stations correctly (2.78 KB, patch)
2013-10-25 14:34 UTC, Bastien Nocera
Details | Splinter Review

Description wrobell 2011-05-03 19:25:04 UTC
in my /etc/UPower/UPower.conf there is

    PollDockDevices=false

but at the start the upowerd says: is_docked = yes

the machine is not docked and i am not sure what causes upower to report that it is docked.

imho, if PollDockDevices is false then upowerd should always report that machine is not docked.
Comment 1 Richard Hughes 2011-05-17 03:17:38 UTC
(In reply to comment #0)
> in my /etc/UPower/UPower.conf there is
> 
>     PollDockDevices=false

Right this enables the fallback polling to the kernel events.

> but at the start the upowerd says: is_docked = yes
> 
> the machine is not docked and i am not sure what causes upower to report that
> it is docked.

Docked, in upower, is where you have more than one active display attached.

> imho, if PollDockDevices is false then upowerd should always report that
> machine is not docked.

No, that's not how PollDockDevices works. Do you have an external monitor or projector installed?
Comment 2 BDenis 2011-10-24 13:37:46 UTC
I can confirm this bug.
Bug affected on notebook with hybrid graphics. On such systems upower see one physical notebook matrix as two logical video device, ATTR "status" both of them is "connected" and as result notebook always seen as docked -> lid switch not functional. (https://bugs.launchpad.net/ubuntu/+source/linux/+bug/854404)

Example.
I have HP Envy 14 with radeon and intel videocards. Look in attachment. In that time I work on intel card, no another monitor been attached. Radeon card was powered but not active.

As you see both logical device status was "connected". But on intel card we can see "modes" ATTR and i think when we determine "is notebook docket" we must check not only "status" ATTR but also "dpms" ATTR and if it "on" - "modes" ATTR (not empty). Or please make config parameter that turn dock detection off.

ps. Sorry for bad english.
Comment 3 BDenis 2011-10-24 13:38:32 UTC
Created attachment 52706 [details]
intel card
Comment 4 BDenis 2011-10-24 13:39:09 UTC
Created attachment 52707 [details]
radeon card
Comment 5 Sebastiano Ternullo 2011-11-14 03:09:03 UTC
i can confirm this bug, everything as BDenis
Comment 6 Alexei Colin 2012-01-08 14:51:04 UTC
Created attachment 55316 [details] [review]
Patch to check 'dpms' attr in addition to 'status' attr

I can observe the issue of upower returning a false positive for is-docked state on my system *with a single video card* (Thinkpad T60 with Radeon X1400). The 'status' is 'connected' for both .../drm/card0/card0-LVDS-1 and .../drm/card0/card0-VGA-1 regardless of whether the respective screen is on or off. As BDenis suggested, 'dpms' attribute can be used to distinguish the state of the screen (it is 'On'/'Off' as expected). The attached patch does just that (a patched package for Ubuntu Oneiric is in ppa:alexei.colin/upower).

NOTE re BDenis's situation+patch posted to LP: on my system, the 'modes' attribute has the same value regardless of whether screen is on or off. Hence, his patch is not applicable (it always claims is-docked=false). That said, the dpms patch that works for me would not work for him (judging from his udevadm output attached to this ticket). 

RANT:
(1) Upower should probably report system as docked when *an external screen is active* instead of when "more than one screen is active" (ie. switching off the laptop screen while having the external monitor on is not "undocking"). The former is probably the true intention of the Gnome3 behavior for auto-disabling suspend (see [1]).
(2) Is display state the best definition of "docked"? What happens when hooking up to a docking station without a screen attached -- does upower still see that as undocked? Maybe upower could expose two separate flags: 'is-external-screen-present' and 'is-docked'?

Thank you for looking at this bug.

[1] http://blogs.gnome.org/hughsie/2011/02/02/is-gnome-3-going-to-melt-your-laptop/
Comment 7 Armando Di Cianno 2012-02-15 16:57:49 UTC
Confirming this bug here on 3xxx series (i.e. late 2011) HP Envy 17 (non-3D) with Intel + AMD Hybrid graphics. Suspend works, but the lid-close action configured through GNOME 3 power settings will not activate suspend.

upower --dump reports that the laptop is docked, when there is no dock installed.

My ACPI firmware may be reporting a dock (I think via hp_wmi module), as I see an entry in /sys, but the state is reported correctly as off:

# cat /sys/devices/platform/dock.0/docked 
0

As Richard noted, upower is assuming that if two cards are installed, the machine is docked (and the NEWS file notes this as well).

Twiddling the discrete card to OFF with vgaswitcheroo doesn't effect upower.

The DRM connected >= 2 assumption is invalid. upower should support docks as the kernel sees them (as entries in /sys like above), thereby not assuming the behavior the user desires. If the user wishes to close the lid, and use the machine with an external display, they can change the behavior of the lid-close action.
Comment 8 Armando Di Cianno 2012-02-15 17:50:16 UTC
Created attachment 57127 [details] [review]
Attempt to detect docking stations by platform/dock_station

This changes the assumption that >=2 graphics cards means "docked". The patch explicitly checks for platform/dock_station /sys entries, and then checks if they are docked or not.
Comment 9 Bastien Nocera 2013-10-21 10:31:56 UTC
Comment on attachment 57127 [details] [review]
Attempt to detect docking stations by platform/dock_station

Review of attachment 57127 [details] [review]:
-----------------------------------------------------------------

::: upower-0.9.15/src/linux/up-dock.c
@@ +51,5 @@
>  	gboolean ret = FALSE;
>  
> +	/* Get the boolean state from the kernel */
> +	docked = g_udev_device_get_sysfs_attr_as_int (d, "docked");
> +	if (docked == NULL)

How can an int be NULL?
You probably want to get it as a string, check if it's NULL and bail if so, and then use atoi or a string comparison to check the actul value.
Comment 10 Bastien Nocera 2013-10-25 14:34:27 UTC
Created attachment 88109 [details] [review]
linux: Detect docked docking stations correctly

Instead of counting the number of graphics outputs, check
all the devices the platform/dock_station subsystem that
export a "dock_station" type.

Based on patch by Armando Di Cianno <armando@goodship.net>
Comment 11 Bastien Nocera 2013-10-25 14:36:24 UTC
Can you please all test this last patch?

I would also need somebody with an actual dock to test this...
Comment 12 Matthew Garrett 2014-05-06 20:47:51 UTC
This will work, but will miss many (most?) docks - not all are exposed as ACPI devices.
Comment 13 Bastien Nocera 2014-05-07 07:50:11 UTC
commit e8beb269dffd7957a1faefa9794a6cbc1a23d40e
Author: Bastien Nocera <hadess@hadess.net>
Date:   Wed May 7 09:48:44 2014 +0200

    tools: Remove is-docked from up-tool

commit cfbd05deb30f722e25707fdf28d880fce1c908fc
Author: Bastien Nocera <hadess@hadess.net>
Date:   Wed May 7 09:47:45 2014 +0200

    linux: Remove is_docked from the integration test

commit 54a030a38d3451b90cc6552b15555085bfc215ca
Author: Bastien Nocera <hadess@hadess.net>
Date:   Wed May 7 09:46:08 2014 +0200

    lib: Hard-code the IsDocked value

commit 135339acc197f0d6a92b71c85048c5a7cb6ce9d6
Author: Bastien Nocera <hadess@hadess.net>
Date:   Wed May 7 09:27:24 2014 +0200

    daemon: Deprecate "IsDocked" property
    
    The IsDocked property has been incorrect for a number of laptops for a
    while, as it thought that laptops with hybrid graphics cards were always
    docked.
    
    The alternative would have been to use the platform/dock_station
    devices, but those are only exported for ACPI docking stations.
    
    Instead, whether an external display is attached (which isn't really
    docking) should be checked in the same place where the policy depending
    on the value should be applied, such as gnome-settings-daemon.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=36818


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.