GNOME Bugzilla – Bug 782211
GSD may pick wrong backlight device on some hybrid graphics machine
Last modified: 2017-05-09 16:29:52 UTC
The Dell E6430 with Nvidia Optimus enables in BIOS, using the Open Source Nouveau driver shows the brightness slider moving but does not affect the brightness. https://bugs.launchpad.net/bugs/1683445
Created attachment 351170 [details] [review] A patch to let GSD pick the correct backlight device
Hi Kai-Heng, Bastien, Small world :) The somebody in "I had a chat with somebody with some experience" in https://bugs.launchpad.net/bugs/1683445 was me. I actually also have an E6430 and had fixing this on my TODO already after Vincent pointed the bug out to me. As for the patch, it is not entirely correct, you check for an enabled attribute for all backlight types and skip any backlights without it, the get_parent to get the actual drm-connector will only work on raw interfaces registered by the drm driver itself, so you are now skipping any non-raw and non-drm-driver-registered backlights. Also you should check that enabled != NULL before dereferencing it. I've written a new patch which only checks the parents "enabled" sysfs attribute for raw interfaces and falls back to just using the first raw interface if no raw interface with enabled == "enabled" is found to avoid regressions. I'll attach the patch here. Regards, Hans
Created attachment 351354 [details] [review] [PATCH] power: Choose correct backlight device on laptops with hybrid graphics
(In reply to Hans de Goede from comment #2) > Hi Kai-Heng, Bastien, > > Small world :) The somebody in "I had a chat with somebody with some > experience" in https://bugs.launchpad.net/bugs/1683445 was me. I actually > also have an E6430 and had fixing this on my TODO already after Vincent > pointed the bug out to me. > > As for the patch, it is not entirely correct, you check for an enabled > attribute for all backlight types and skip any backlights without it, the > get_parent to get the actual drm-connector will only work on raw interfaces > registered by the drm driver itself, so you are now skipping any non-raw and > non-drm-driver-registered backlights. Also you should check that enabled != > NULL before dereferencing it. Thanks! Didn't know that the DRM and backlight worked this way. > > I've written a new patch which only checks the parents "enabled" sysfs > attribute for raw interfaces and falls back to just using the first raw > interface if no raw interface with enabled == "enabled" is found to avoid > regressions. > > I'll attach the patch here. Looks great, I'll cherry-pick it into unity-settings-daemon when it's being merged. Thanks for the review and all the info. > > Regards, > > Hans
Review of attachment 351354 [details] [review]: looks good, thanks
00d1a8a..2ff6738 gnome-3-24 -> gnome-3-24
1dffcf5..ed7c274 master -> master