Bug 399646

I can confirm with a Thinkpad W530.  My situation is exactly the same.  I'm running Fedora 29 which has powerdevil 5.14.4 currently.  I haven't previously tried to run Linux on one of these laptops so I have no idea if this worked at some point in the past.

Supposedly (from a quick chat with Matthew Garrett on IRC) the desktop environment needs to figure out which output port needs to be adjusted, follow that to the device (graphics card) and then look for the backlight in /sys/class/backlight which corresponds to the device.

I have to give this laptop to a user, but I think I have another on hand to test if there's a patch I can try.

I'm also affected by this. I have a T530 with multiple GPUs.

Just for fun, I read the source of gnome-power-manager which is the only one that gets this right. They seem to be checking if *_backlight/device/enabled is actually reading "enabled" and using that one.

At one point, I found the equivalent code in Powerdevil and Solid and for some reason I can't find where that file is right now.

I'm almost certain that would fix it if someone that (a) knows where that file is and (b) knows enough Qt (I don't, unless I spend a lot of time studying up) wants to write a patch. Or if you can point me in the right direction I'm happy to take a stab at it but it might be a while.

The relevant PowerDevil code is at https://cgit.kde.org/powerdevil.git/tree/daemon/backends/upower/backlighthelper.cpp?id=5602920aeb852b3703ba357394e43454820460f3#n80

The GNOME backlight code has been moved from gnome-power-manager to https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/blob/523ffa148ce16773c2d9f711edb2c0424411d7ac/plugins/power/gsd-backlight.c#L119

An alternative code solution would be to affect *all* of the backlights at once.

Hi.
Bug still present in KDE 5.78.0 / Powerdevil 5.20.5.
I have a laptop with just one video card, but with 2 "backends"/providers/drivers found by the system: /sys/class/backlight/dell_backlight and /sys/class/backlight/intel_backlight (yes, it's a Dell laptop with an Intel video card :) ).
As Bastian says, KDE picks whatever backend is returned first by scanning the /sys/class/backlight directory. For me, it always seems to be the wrong one :). I also found (although in some other place) the same piece of code that does this.
I partially agree that KDE should check which backend is enabled and use that one (as Dan says). Perhaps this will be enough. Unless there really are 2 graphic cards installed, both enabled, and the user would like to adjust the brightness on all screens at once. Or maybe just some of them (e.g. because it works on one, but breaks on the other).
That's why I agree more to a configurable setting - letting the user choose is, in my opinion, always best.
Should it be a single-select or multi-select setting, should it be in ~/.config and only there or also on the KDE's systemsettings GUI is another question.
I'd opt for a multi-select setting and a list of check-boxes in systemsettings with all the found backends (found by listing the directory, just like it's done now), letting the user select which ones to use and then make Powerdevil iterate through the selected ones when the brightness action is called (button press, power level change, whatever). If none selected, change none. Or just the default. Or maybe introduce another setting - "Use default". If selected (this would be the selected by default on fresh installations), use all the found and enabled backends. When deselected (meaning: on purpose), use just the ones the user selected (including "none"). Something like:

Choose the provider/backend/driver/graphic card for screen brightness:
(o) Use default (all enabled providers)
( ) Choose the provider:
    [ ] dell
    [x] intel

(2 radiobuttons, 2 check-boxes enabled only when the second radiobutton is selected).
My C++ knowledge is too poor to fix this myself, and my Qt knowledge is zero, not to mention making a decision which path to take, doing it and then compiling half of KDE to do testing :). So, I'm just "keeping this bug alive" to show that it's still relevant (and help make KDE better than Windows ;) where the keyboard buttons to change the brightness do actually work on the same laptop...).

I'm not sure controlling all of them is the right answer either unless there's an option to disable that, on the off chance there's hardware where both works and it causes either 2 steps of change per press, or a conflict.

Having an option isn't a bad idea either, but that's way over my head on the coding stand point (although real life has taken over since reporting this so I haven't had time to look at a patch at all).

A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/69

Git commit e95924f499eb3f8ca916e617e4c9e70aefb8669c by Nate Graham, on behalf of Dan Robinson.
Committed on 03/12/2021 at 17:19.
Pushed by ngraham into branch 'master'.

Check if backlight device is enabled

Check the enabled property on each backlight device before using
it as a backlight device, so the correct device is used in multi
GPU setups
FIXED-IN: 5.24

M  +10   -0    daemon/backends/upower/backlighthelper.cpp

https://invent.kde.org/plasma/powerdevil/commit/e95924f499eb3f8ca916e617e4c9e70aefb8669c

Although this was fixed in https://invent.kde.org/plasma/powerdevil/-/commit/e95924f499eb3f8ca916e617e4c9e70aefb8669c, a new commit https://invent.kde.org/plasma/powerdevil/-/commit/8e5c131d2050cc36a794d9e52fbd8cb762be2c6f make this bug appears again. So, version 5.24.0 and 5.24.1 still have this BUG.

A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/78

Git commit 401e3ed8999e22d5653fac9cb3d78a638c742825 by Xingang Li.
Committed on 23/02/2022 at 13:41.
Pushed by bshah into branch 'master'.

Improved backlight devices selection

1. Change backlight devices selection order, return more early;
2. Only check raw type devices's enable attribute. If no raw
device available, we will search again without above checking.

M  +34   -22   daemon/backends/upower/backlighthelper.cpp

https://invent.kde.org/plasma/powerdevil/commit/401e3ed8999e22d5653fac9cb3d78a638c742825

Git commit c6b7c714af1d25d04bc820bc5ce554241999901f by Nate Graham, on behalf of Xingang Li.
Committed on 24/02/2022 at 17:00.
Pushed by ngraham into branch 'Plasma/5.24'.

Improved backlight devices selection

1. Change backlight devices selection order, return more early;
2. Only check raw type devices's enable attribute. If no raw
device available, we will search again without above checking.


(cherry picked from commit 401e3ed8999e22d5653fac9cb3d78a638c742825)

M  +34   -22   daemon/backends/upower/backlighthelper.cpp

https://invent.kde.org/plasma/powerdevil/commit/c6b7c714af1d25d04bc820bc5ce554241999901f

Git commit 761fc8a4bf4bd70bcd9aca63fc67382c94ecf884 by Nate Graham, on behalf of Max Ramanouski.
Committed on 23/03/2022 at 19:00.
Pushed by ngraham into branch 'master'.

Write brightness to all raw devices

On some laptops, there are multiple enabled raw backlight devices, and
backlighthelper can't choose right device.

For example: NVIDIA Optimus laptop, Lenovo 15ARH05. It has two GPUs: AMD
and NVIDIA, both of them provide their backlight device, both backlight
devices are raw type and enabled(in case prime is used). Backlighthelper
without this fix selects NVIDIA device for backlight, but on this laptop
brightness is controlled by AMD gpu, so user can't control brightness.

This fix implements additional list for all raw devices. This list is
loaded on initUsingBacklightType. List contains pairs of sysfs path and
max brightness of device. When brightness is changed, backlighthelper goes
through entries starting from second(first device is written by data from
m_dirname) and writes brightness with correcting difference between
max_brightness in different devices. For all other cases(read brightness
and other) first raw device is used.
FIXED-IN: 5.25

M  +76   -71   daemon/backends/upower/backlighthelper.cpp
M  +5    -1    daemon/backends/upower/backlighthelper.h

https://invent.kde.org/plasma/powerdevil/commit/761fc8a4bf4bd70bcd9aca63fc67382c94ecf884

This is happening again in Plasma 5.27.1

Here are my hardware/software infos:
Operating System: Manjaro Linux 
KDE Plasma Version: 5.27.1
KDE Frameworks Version: 5.103.0
Qt Version: 5.15.8
Kernel Version: 6.1.13-1-MANJARO (64-bit)
Graphics Platform: Wayland
Processors: 16 × AMD Ryzen 9 5900HX with Radeon Graphics
Memory: 62.2 Gio of RAM
Graphics Processor: AMD Radeon Graphics
Manufacturer: TUXEDO
Product Name: TUXEDO Stellaris AMD Gen3 (CZN)

Here is a log of some values of the sys providers concerned. The backlight value changes for acpi_video0 but amdgpu_bl1 is the one controlling the screen.

$ pwd
/sys/class/backlight
$ ls
acpi_video0  amdgpu_bl1
$ cat acpi_video0/device/enable
1
$ cat amdgpu_bl1/device/enable
1
$ cat acpi_video0/max_brightness
49
$ cat amdgpu_bl1/max_brightness
255

(In reply to Nils from comment #13)
> Here is a log of some values of the sys providers concerned. The backlight
> value changes for acpi_video0 but amdgpu_bl1 is the one controlling the
> screen.

Nils, what are the values for /sys/class/backlight/acpi_video0/type and /sys/class/backlight/amdgpu_bl1/type on your system? If they're different, PowerDevil currently would only pick the one(s) with the highest-ranked type.

(In reply to Jakob Petsovits from comment #14)
> (In reply to Nils from comment #13)
> > Here is a log of some values of the sys providers concerned. The backlight
> > value changes for acpi_video0 but amdgpu_bl1 is the one controlling the
> > screen.
> 
> Nils, what are the values for /sys/class/backlight/acpi_video0/type and
> /sys/class/backlight/amdgpu_bl1/type on your system? If they're different,
> PowerDevil currently would only pick the one(s) with the highest-ranked type.

acpi_video0/type -> firmware
amdgpu_bl2/type -> raw

(In reply to Nils from comment #15)
> acpi_video0/type -> firmware
> amdgpu_bl2/type -> raw

Hm, that could be tricky to fix without potentially breaking other systems. Reading material such as https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-class-backlight or the more in-depth write-up at https://mjg59.livejournal.com/127103.html suggest that it's not necessarily safe to change both at once.

Maybe we can learn something from https://github.com/systemd/systemd/blob/main/src/backlight/backlight.c which enumerates backlight devices to save/restore brightness at startup/shutdown time. Based on a quick initial reading, it seems to allow both firmware/platform devices and raw devices next to each other, but filters out any raw backlight device that points to the same PCI device parent as another firmware/platform backlight device. I think doing the same in powerdevil could fix this remaining use case... while also making the code more fragile :-S