Bug 381199

Summary: Broken brightness control with kernel module (ex. nvidia-bl)
Product: [Plasma] Powerdevil Reporter: vsnrain.dev
Component: generalAssignee: Plasma Development Mailing List <plasma-devel>
Status: RESOLVED FIXED    
Severity: normal CC: bhush94
Priority: NOR    
Version: 5.10.0   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: https://commits.kde.org/powerdevil/5c57cf64b5e5c880b1a5f3a0177293f6958e1b9a Version Fixed In:
Sentry Crash Report:

Description vsnrain.dev 2017-06-14 12:12:11 UTC 
Overview:
Since 5.10, it is no longer possible to adjust brightness on my system, which requires special kernel module for brightness to work. I tried to look at what caused this, and found that in following commit 
https://cgit.kde.org/powerdevil.git/commit/?id=86c3548d103ff5966366788220796de629be9230
Was attempt to add feature that powerdevil will control only brightness of active device. Part of relevant code is following:

/daemon/backends/upower/backlighthelper.cpp
bool BacklightHelper::isRawBacklightEnabled(const QString &interface)
{
    QFile file(BACKLIGHT_SYSFS_PATH + interface + "/device/enabled");

    if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
        return false;
    }

    QByteArray buffer = file.readLine().trimmed();
    if (buffer == "enabled") {
        return true;
   }

    return false;
}

As you can see, it was implemented by skipping all brightness devices which in BACKLIGHT_SYSFS_PATH + interface + "/device/enabled" does not return "enabled".
The problem is, there is no property "device/enabled" in my nvidia_backlight device:

ls -la /sys/class/backlight/nvidia_backlight/                  
total 0
drwxr-xr-x 3 root root    0 Jun  2 22:50 .
drwxr-xr-x 3 root root    0 Jun  2 22:50 ..
-r--r--r-- 1 root root 4096 Jun  2 23:02 actual_brightness
-rw-r--r-- 1 root root 4096 Jun  2 23:02 bl_power
-rw-r--r-- 1 root root 4096 Jun  2 22:54 brightness
-r--r--r-- 1 root root 4096 Jun  2 22:50 max_brightness
drwxr-xr-x 2 root root    0 Jun  2 23:02 power
lrwxrwxrwx 1 root root    0 Jun  2 22:50 subsystem -> ../../../../class/backlight
-r--r--r-- 1 root root 4096 Jun  2 22:50 type
-rw-r--r-- 1 root root 4096 Jun  2 22:50 uevent

so powerdevil tries to open this file, fails, and always skips this brightness device. I recompiled powerdevil with "return true" in mentioned function, and brightness control works again, so it seems that problem in indeed in this part.

Steps to Reproduce:
use kernel module for brightness device, tested on Nvidia GF 9400M with nvidia-bl kernel module.

Actual Results:
powerdevil always skips brightness device, because it cannot open file /sys/class/backlight/nvidia_backlight/device/enabled, which does not exist.

Expected Results:
register and control brightness device.
Comment 1 Bhushan Shah 2017-06-14 12:27:29 UTC 
though This is done only for raw backlight type devices..

What is the backlight type for your device?

cat /sys/class/backlight/nvidia_backlight/type
Comment 2 vsnrain.dev 2017-06-14 12:29:01 UTC 
it returns "raw"
Comment 3 Bhushan Shah 2017-06-14 13:03:35 UTC 
Git commit 97f3900049aa657be5f4c124c9f992120f124944 by Bhushan Shah.
Committed on 14/06/2017 at 12:55.
Pushed by bshah into branch 'master'.

Revert "skip the disabled backlight device"

This reverts commits 86c3548d103ff5966366788220796de629be9230 and
5c0d35ca6caf7bb0a0e9897a1a868e8406f3b112.

Not all raw interfaces have device/enabled under the backlight sysfs
interface. Some examples are nvidia-bl and acpi_video0, they are raw
interfaces but doesn't have device/enabled properties.

Overall we can just return true even if file doesn't exist but then this
code serves no purpose.
Related: bug 381114

Ackd-by: Kai Uwe Broulik <kde@privat.broulik.de>
CCMAIL: AceLan Kao <acelan@acelan.idv.tw>

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

https://commits.kde.org/powerdevil/97f3900049aa657be5f4c124c9f992120f124944
Comment 4 Bhushan Shah 2017-06-14 13:05:02 UTC 
Git commit 5c57cf64b5e5c880b1a5f3a0177293f6958e1b9a by Bhushan Shah.
Committed on 14/06/2017 at 13:04.
Pushed by bshah into branch 'Plasma/5.10'.

Revert "skip the disabled backlight device"

This reverts commits 86c3548d103ff5966366788220796de629be9230 and
5c0d35ca6caf7bb0a0e9897a1a868e8406f3b112.

Not all raw interfaces have device/enabled under the backlight sysfs
interface. Some examples are nvidia-bl and acpi_video0, they are raw
interfaces but doesn't have device/enabled properties.

Overall we can just return true even if file doesn't exist but then this
code serves no purpose.
Related: bug 381114

Ackd-by: Kai Uwe Broulik <kde@privat.broulik.de>
CCMAIL: AceLan Kao <acelan@acelan.idv.tw>

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

https://commits.kde.org/powerdevil/5c57cf64b5e5c880b1a5f3a0177293f6958e1b9a
Comment 5 Jan Kundrát 2017-06-16 11:29:04 UTC 
Just FYI and as a random data point, my T460s with intel_backlight which once again has a type = raw contains "enabled" in its device/enabled property. Despite that, I was too hitting this bug.

However, it's possible that I might have always logged in with this laptop docked while I was debugging this. If that's the case, SDDM shows output on the laptop's internal screen as well as on some external one. kscreen switches the internal output off because that's what I want to have.

It's possible that there was a race between kscreen switching and powerdevil deciding to ignore stuff. Thanks bshah for fixing this.