GNOME Bugzilla – Bug 777538
gnome_rr_output_is_builtin_display incorrectly returns false for Apple laptops with nvidia driver
Last modified: 2017-11-15 18:40:44 UTC
Created attachment 343907 [details] [review] Patch to fix apple built-in monitor detection gnome_rr_output_is_builtin_display() uses the output name to determine whether or not a display is built-in. With the nouveau driver, the built-in display on an Apple Macbook Pro is named "eDP-1", and it's correctly detected as built-in. With the proprietary nvidia driver, however, the display is simply named "DP-2" and gnome_rr_output_is_builtin_display() returns false. This causes issues with sleeping, because suspend is inhibited when the computer has an external display attached. The attached patch changes gnome_rr_output_is_builtin_display() to include outputs whose vendor is "APP". I'm not sure if this would cause any issues with Apple-branded external monitors. If it would, then perhaps a better, though more far-reaching, change would be to change gnome-settings-daemon to inhibit suspend based on the number of monitors, rather than their type. (Is there a situation where a computer has a lid and no built-in monitor? If not, then we can safely assume that having 2 or more displays is equivalent to having 1 or more external display.) Thanks! Tony
Review of attachment 343907 [details] [review]: This really needs fixing in the driver, as with your patch, there'd be no way to differentiate between a builtin monitor, and an Apple display connected over DisplayPort.
I agree with rejecting the patch as-is because of the possibility of issues with external Apple monitors. But I'm not sure this is a bug in the driver: from what I understand, the driver can call its output eDP-1, or DP-2, or POTATO-3; there's no semantic meaning associated with the display name. Right? I wonder why we're using the name and not the ConnectorType property (which is correctly reported as Panel when I run 'xrandr')? I wrote a patch to do exactly this; see attached. But then I found a strange comment in the code which led me to dig back in the git history. It looks like gnome-desktop USED to use the connector type until connector_type -- and all references to it -- was removed in 632dffb7887c8650154670ae0e76822ae11e22f3. That said, a lot has changed in the meantime. Let me know if this patch looks OK. In particular, I'm wondering if it would be possible to use ONLY the connector type, or if we still need to fall back on the name. Thanks again! Tony
Created attachment 343924 [details] [review] Use connector type to test if monitor is builtin
Has anyone had a chance to look over the updated patch? Thanks! Tony
I had a similar problem and ended up writing a very similar patch, whic
Created attachment 363308 [details] [review] nvidia-builtin-display.patch I had a very similar problem on NVIDIA laptops from other vendors. The NVIDIA driver returns "DP-0" instead of "eDP-0" for internal displays. This is a problem, but not one that needs to be fixed immediately. The patch shown above is very similar to one I created independently, which I am attaching here for more validation of the above patch, which uses connector-type from Mutter to determine if the output is a builtin display. Feel free to use my patch instead, as it is based on the newest gnome-desktop3 git master. The two are functionally equivalent.
Most of this code isn't used anymore, as mutter got a new display configuration API, and I believe the Display panel uses it directly. Jonas, this code is still called from a couple of place in gnome-settings-daemon (the power and color plugins, if I remember correctly), could we port those users and remove this code?
(In reply to Bastien Nocera from comment #7) > Most of this code isn't used anymore, as mutter got a new display > configuration API, and I believe the Display panel uses it directly. > > Jonas, this code is still called from a couple of place in > gnome-settings-daemon (the power and color plugins, if I remember > correctly), could we port those users and remove this code? I intend to fix that soon
This is good to hear. It is best if gnome-settings-daemon gets its information from mutter like everything else.