Bug 288180 - Enhance backlight detection in powerdevil/backend/upower/backlighthelper.cpp
Summary: Enhance backlight detection in powerdevil/backend/upower/backlighthelper.cpp
Status: RESOLVED FIXED
Alias: None
Product: solid
Classification: Frameworks and Libraries
Component: powermanagement (show other bugs)
Version: unspecified
Platform: Unlisted Binaries Linux
: NOR wishlist
Target Milestone: ---
Assignee: Alex Fiestas
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-04 09:09 UTC by Corentin Chary
Modified: 2012-04-03 15:20 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.9


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Corentin Chary 2011-12-04 09:09:50 UTC
Version:           unspecified (using Devel) 
OS:                Linux

Currently the code maintain a whitelist of backlight:


    interfaces << "nv_backlight" << "intel_backlight" << "radeon_bl" << "mbp_backlight"
               << "asus_laptop" << "toshiba" << "eeepc" << "thinkpad_screen" << "acpi_video1"
               << "acpi_video0" << "apple_backlight" << "fujitsu-laptop" << "samsung"
               << "nvidia_backlight" << "dell_backlight" << "sony"
               ;

The issue is that this has to be updated each time a new driver is added to the kernel. For example the list lacks: "asus-nb-wmi" and "asus-wmi".

This whitelist also provides some priority information (use gpu first, then acpi, then vendor...).

Instead, we could use a new backlight attribute (if present):

What:           /sys/class/backlight/<backlight>/type
Date:           September 2010
KernelVersion:  2.6.37
Contact:        Matthew Garrett <mjg@redhat.com>
Description:
                The type of interface controlled by <backlight>.
                "firmware": The driver uses a standard firmware interface
                "platform": The driver uses a platform-specific interface
                "raw": The driver controls hardware registers directly

                In the general case, when multiple backlight
                interfaces are available for a single device, firmware
                control should be preferred to platform control should
                be preferred to raw control. Using a firmware
                interface reduces the probability of confusion with
                the hardware and the OS independently updating the
                backlight state. Platform interfaces are mostly a
                holdover from pre-standardisation of firmware
                interfaces.

As described, type can be used to describe the preference. However, using the gpu backlight (intel, radeon, nouveau, nvidia) provide a better control most of the time, so maybe an hybrid approach should be taken:
- Use a whitelist/priority list (containing only intel, radeon, etc...) ?
- If not found, use a firmware backlight
- If not found, use a platform backlight
- If not found, use a raw backlight

And maybe a settings should allow the user to specify a backlight (something hidden in a file, no need for a gui to set it).

Reproducible: Didn't try



Expected Results:  
-
Comment 1 Dario Freddi 2011-12-05 16:39:49 UTC
Lukas, Alex, can you please look into this?
Comment 2 Alex Fiestas 2011-12-09 12:09:23 UTC
Added to my TODO list, will take a look ASAP.
Comment 3 Lukáš Tinkl 2011-12-12 11:58:54 UTC
Would be nice, thx Alex, assigning  :)
Comment 4 Aaron Bauman 2012-01-30 14:52:48 UTC
Any updates on this?  I have the same issue on an HP laptop with intel_backlight.  I am not an active programmer but am dangerous enough to try and help out in any way possible.  If there is something I can assist with I would be more than happy to.
Comment 5 Alex Fiestas 2012-02-02 16:15:21 UTC
Just discussed this with Richard Hughsie and the way to go seems to be:

1-Use XRandR if possible
2-firmware->platform->raw

Going to work on it right now, though I'm a little bit afraid of putting this in 4.8.1 without testing. Maybe 4.8.2
Comment 6 Aaron Bauman 2012-02-02 16:44:09 UTC
I will be happy to test Alex whenever you like.
Comment 7 Aaron Bauman 2012-02-02 17:56:25 UTC
Alex, I think this bug I filed is some what related to this bug as well.

https://bugs.kde.org/show_bug.cgi?id=292873
Comment 8 Alex Fiestas 2012-02-03 01:06:48 UTC
Git commit 018679ecdd07a2fd8dfb27df79355465252e8bc2 by Alex Fiestas.
Committed on 02/02/2012 at 23:59.
Pushed by afiestas into branch 'KDE/4.8'.

Put acpi_video1/0 before intel_backlight

Now we know that firmware interface should be put BEFORE the raw ones
so in this case, acpi_video1/0 are firmware and intel_backlight is RAW.
Related: bug 292873

M  +4    -3    powerdevil/daemon/backends/upower/backlighthelper.cpp

http://commits.kde.org/kde-workspace/018679ecdd07a2fd8dfb27df79355465252e8bc2
Comment 9 Alex Fiestas 2012-02-03 01:29:46 UTC
Git commit 5257ff19aa3e779a9e07cc49ee82cca7a051b558 by Alex Fiestas.
Committed on 03/02/2012 at 02:12.
Pushed by afiestas into branch 'master'.

If kernel newer than 2.6.36 use the backlight type instead of whitelist

If the kernel is newer than 2.6.36 it indicates the type of the
backlight wether it is a firmware/platform or raw. We should
attempt to connect to these interfaces in this order.

This should fix any compatibility issue we have right now and make this
code comaptible with drivers that may appear in the future.
Related: bug 292873
CCMAIL: lukas@kde.org
FIXED-IN: 4.9

M  +97   -5    powerdevil/daemon/backends/upower/backlighthelper.cpp
M  +19   -0    powerdevil/daemon/backends/upower/backlighthelper.h

http://commits.kde.org/kde-workspace/5257ff19aa3e779a9e07cc49ee82cca7a051b558
Comment 10 Alex Fiestas 2012-02-03 01:31:34 UTC
Will backport to KDE 4.8.1 or 4.8.2 if it is demonstrate stable enough.
Comment 11 Aaron Bauman 2012-02-03 02:15:32 UTC
On Friday 03 February 2012 01:31:35 Alex Fiestas wrote:
> https://bugs.kde.org/show_bug.cgi?id=288180
> 
> 
> 
> 
> 
> --- Comment #10 from Alex Fiestas <afiestas kde org>  2012-02-03 01:31:34
> --- Will backport to KDE 4.8.1 or 4.8.2 if it is demonstrate stable enough.

Alex,
  I am on Gentoo and learning a lot but am curious how I could implement your 
fix to test?  We have access to developer sources but I would rather not run 
that on my laptop.  Is there any way to implement this outside of running 
unstable sources?  I have the source for kde-workspace-4.8.0 so I suppose I 
could just patch that?

If you have time any advice would be great.
Comment 12 Corentin Chary 2012-02-03 07:53:30 UTC
Alex, two comments on your patch:
- I'd check if the backlight have a "type" file directly instead of checking the kernel version (a kernel patch can be backported).
- Could you add a setting to "force" to use a specific backlight ? In some cases acpi_video is available but non-functional for example.

Aaron: the patch probably cleanly apply on 4.7 two (or is easy to adapt to). So patching your ebuild to apply thoses two patches should be enought to test without breaking your system with a make / make install.
Comment 13 Corentin Chary 2012-04-03 13:46:50 UTC
Ping.

Would it be possible for force the backlight (or to override the whitelist) with a user-defined setting ? Another user asked that to me again today.

Thanks,
Comment 14 darkbasic 2012-04-03 14:06:41 UTC
Yes it would be nice...

Thanks,
Niccolò
Comment 15 Aaron Bauman 2012-04-03 15:11:17 UTC
I was able to resolve my issue with the following grub settings:

kernel /boot/bzImage root=/dev/sda3 acpi_osi=Linux acpi_backlight=vendor
Comment 16 darkbasic 2012-04-03 15:20:22 UTC
Thanks, it did work for me.