After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 719949 - Set to 0 brightness will turn off the backlight
Set to 0 brightness will turn off the backlight
Status: RESOLVED DUPLICATE of bug 744278
Product: gnome-settings-daemon
Classification: Core
Component: power
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
: 719948 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-12-06 07:38 UTC by AceLan Kao
Modified: 2015-09-08 09:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't set the brightness to 0 (1.30 KB, patch)
2013-12-06 07:59 UTC, AceLan Kao
rejected Details | Review

Description AceLan Kao 2013-12-06 07:38:17 UTC
The machines ship with win8 have 101 levels of brightness, and the lowest one level will turn off the backlight.
User can't see anything on the screen, so in this case, we should not set the brightness to 0.
It won't be a problem on laptops, but on All-In-One machines.
There is no way for users to move their mouse and adjust the brightness.
Comment 1 AceLan Kao 2013-12-06 07:59:14 UTC
Created attachment 263638 [details] [review]
Don't set the brightness to 0
Comment 2 André Klapper 2013-12-06 09:30:24 UTC
*** Bug 719948 has been marked as a duplicate of this bug. ***
Comment 3 Bastien Nocera 2013-12-06 11:36:05 UTC
Review of attachment 263638 [details] [review]:

That's not good enough.

This only affects certain devices (my laptop which uses win8 brightness levels doesn't turn off the backlight), and there's no way to detect them.

Instead, this should be clamped at the kernel level which knows about different hardware and how it behaves.

::: plugins/power/gpm-common.c
@@ +597,3 @@
+	/* don't set to 0 brightness if it has more than 100 brightness degrees */
+	else if (max >= 100)
+		min = 5;

Why not 1?
Comment 4 AceLan Kao 2013-12-09 03:46:31 UTC
It's good that your laptop doesn't be affected by this issue, but it won't harm if the minimum brightness level is 5, instead of 0.

And why the value is 5, not 1, it's BIOS engineer told me.
BIOS engineer used to fix this issue by telling the OS the minimum brightness number is 5, so I use 5 here. They probably did some tests and found 5 is more feasible value.(We ask BIOS to fix this issue in BIOS code, but we only can fix very small amount of machines)

And I don't think this should be fixed in the kernel. Kernel does what you tell it, it shouldn't change the brightness value to 5 when you tell it set to 0.

There is a new kernel commit that tries to prevent the acpi_video0 from creating if its win8 system(after kernel 3.7, we will report us as win8), that means g-s-d will change the brightness through native graphic driver, such as intel_backlight. And it will turn off the backlight this time if the brightness value is 0.

commit fbc9fe1b4f222a7c575e3bd8e9defe59c6190a04
Author: Aaron Lu <aaron.lu@intel.com>
Date:   Fri Oct 11 21:27:45 2013 +0800

    ACPI / video: Do not register backlight if win8 and native interface exists
    
    According to Matthew Garrett, "Windows 8 leaves backlight control up
    to individual graphics drivers rather than making ACPI calls itself.
    There's plenty of evidence to suggest that the Intel driver for
    Windows [8] doesn't use the ACPI interface, including the fact that
    it's broken on a bunch of machines when the OS claims to support
    Windows 8.  The simplest thing to do appears to be to disable the
    ACPI backlight interface on these systems".
    
    So for Win8 systems, if there is native backlight control interface
    registered by GPU driver, ACPI video does not need to register its own.
    Since there are systems that don't work well with this approach, a
    parameter for video module named use_native_backlight is introduced and
    has the value of false by default. For users who have a broken ACPI
    video backlight interface, video.use_native_backlight=1 is needed in
    kernel cmdline.
    
    Signed-off-by: Aaron Lu <aaron.lu@intel.com>
    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Comment 5 Bastien Nocera 2013-12-10 16:26:01 UTC
I'm not interested in adding hardware specific hacks of that sort in gnome-settings-daemon. The driver for backlight on those devices should not allow the backlight to be turned off instead.
Comment 6 AceLan Kao 2013-12-11 09:29:47 UTC
You probably could think this way, this is not hardware specific hacks, this is to prevent from dimming to dark to read.
If it's not a good idea to set the minimum value to 5, how about set it to 1/10 max or 1/20 max according to how many brightness levels g-s-d provides.
Comment 7 Bastien Nocera 2013-12-11 11:06:05 UTC
(In reply to comment #6)
> You probably could think this way, this is not hardware specific hacks,

It is.

> this is
> to prevent from dimming to dark to read.

It can be both a hardware specific hack, and to prevent from dimming too dark. They're not mutually exclusive.

> If it's not a good idea to set the minimum value to 5, how about set it to 1/10
> max or 1/20 max according to how many brightness levels g-s-d provides.

That's pretty much the same hack.
Comment 8 AceLan Kao 2013-12-12 02:23:08 UTC
Please switch your position to kernel side and re-think if this kind of issue should be fixed in kernel.
Kernel accepts the brightness value from userspace, and it doesn't know what userspace really means by given that value, so it just does what it should do.
If userspace app would like to turn off the backlight, there is another interface for userspace app to call. Userspace app shouldn't try to turn off the backlight by setting 0 brightness.
This issue happens on many machines, the most suitable place to fix this issue is from g-s-d to not to set 0 brightness, for kernel doesn't know 0 brightness will switch off the backlight and kernel shouldn't change the brightness value passes from userspace.
Please re-consider it, it's not a big issue but more to user experience.
Comment 9 Bastien Nocera 2013-12-12 18:02:17 UTC
(In reply to comment #8)
> Please switch your position to kernel side and re-think if this kind of issue
> should be fixed in kernel.
> Kernel accepts the brightness value from userspace, and it doesn't know what
> userspace really means by given that value,

So make up a new value in the kernel that means "this is backlight off, not the lowest level of brightness with the backlight still on".

> so it just does what it should do.

It does so with ill-defined semantics. I don't find that useful.

> If userspace app would like to turn off the backlight, there is another
> interface for userspace app to call. Userspace app shouldn't try to turn off
> the backlight by setting 0 brightness.

It indeed shouldn't. Except that it has no way to know whether 0 means lowest level of backlight, or backlight off.

> This issue happens on many machines, the most suitable place to fix this issue
> is from g-s-d to not to set 0 brightness, for kernel doesn't know 0 brightness
> will switch off the backlight and kernel shouldn't change the brightness value
> passes from userspace.

Absolutely not. The best solution is to fix the semantics of the lowest brightness level in the kernel, not to work around it in every single desktop environment.

> Please re-consider it, it's not a big issue but more to user experience.

You should fix this in the kernel.
Comment 10 AceLan Kao 2013-12-13 07:45:51 UTC
First of all, kernel doesn't know the 0 brightness value will turn off the backlight. Second, kernel doesn't know the feasible minimum brightness value for users. And, according to the latest kernel patch I pasted above, there is no acpi_video0 anymore if there is intel_backlight directory(or such as radeon_backlight) in /sys/class/backlight/.

So, you will probably soon or later face this issue and get a lot of bug reports about this.

Please re-consider it, don't set the minimum brightness value to 0.
I'm not here to bother you and to waste your time. People who reports bugs and submit patches here are all hope gnome could become better.

Or if you have any idea where and how kernel could handle this, please let me know, I'm willing to bother kernel guys about this.
Comment 11 Bastien Nocera 2013-12-13 13:03:13 UTC
(In reply to comment #10)
> First of all, kernel doesn't know the 0 brightness value will turn off the
> backlight.

You can add that to the kernel through DMI matching. That's the whole point of doing it in the kernel, user-space doesn't need to do the checks itself.

> Second, kernel doesn't know the feasible minimum brightness value
> for users.

What does that mean? What's a minimum "feasible" brightness value?

> And, according to the latest kernel patch I pasted above, there is
> no acpi_video0 anymore if there is intel_backlight directory(or such as
> radeon_backlight) in /sys/class/backlight/.

Why does that matter? It's supposed to be the same (ill-defined) interface for both.

> So, you will probably soon or later face this issue and get a lot of bug
> reports about this.
> 
> Please re-consider it, don't set the minimum brightness value to 0.
> I'm not here to bother you and to waste your time. People who reports bugs and
> submit patches here are all hope gnome could become better.
> 
> Or if you have any idea where and how kernel could handle this, please let me
> know, I'm willing to bother kernel guys about this.

You need an interface that's better defined in:
Documentation/ABI/stable/sysfs-class-backlight

Maybe you could ensure that negative backlight levels will be used for "backlight off" levels.

Step 2 is modifying the various drivers to take this into account. The applesmc driver would need changing for example. You'd also need to add a DMI table for in the ACPI brightness section.

The remainder of the work in gnome-settings-daemon would simply be to ignore the negative values. So your example of 0 to 100 (101 values) would be -1 to 99, which gnome-settings-daemon would clamp to 0 to 99, avoiding turning the backlight off without any hardware specific hacks.
Comment 12 Bastien Nocera 2014-01-07 08:25:23 UTC
Let me know when there's been some movement about this in the kernel interface.
Comment 13 Unai Uribarri 2014-01-11 01:20:58 UTC
intel_backlight driver in linux 3.12, at least on my Toshiba Satellite P75-A, has 4882 brightness steps. Setting brightness to 0 is so soft than it's imposible to see anything.

I'm using Fedora 20 and, with Gnome 3.10, I have two issues:
 - GDM sets the brightness value to 0. If I replace gdb by lightdm, the brightness isn't changed.
 - Even with lightdm, brightness is reset to 0 when I login.

I see two problmes in Gnome 3.10:
 - Gnome should use a more sane default than the minimum brightness value. 30% or 40% would be much more appropiate.
 - The brightness setting must be saved (whenever changes or at logoff) and restored after login instead of using the minimum value.
Comment 14 Bastien Nocera 2014-01-13 08:52:18 UTC
(In reply to comment #13)
<snip>
> I see two problmes in Gnome 3.10:
>  - Gnome should use a more sane default than the minimum brightness value. 30%
> or 40% would be much more appropiate.
>  - The brightness setting must be saved (whenever changes or at logoff) and
> restored after login instead of using the minimum value.

GNOME doesn't change the backlight unless you're on low battery, and it certainly never changes it to zero in those cases.
Comment 15 Bastien Nocera 2015-09-08 09:16:08 UTC
This is now worked around in bug 744278.

*** This bug has been marked as a duplicate of bug 744278 ***