Bug 10139 - fujitsu-laptop inconsistency due to hardware controlled fn-keys
Summary: fujitsu-laptop inconsistency due to hardware controlled fn-keys
Status: CLOSED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Platform (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Jonathan Woithe
URL:
Keywords:
Depends on:
Blocks: 56331
  Show dependency tree
 
Reported: 2008-03-01 04:03 UTC by Khashayar Naderehvandi
Modified: 2013-04-09 06:23 UTC (History)
2 users (show)

See Also:
Kernel Version: 2.6.24
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
output of acpidump (114.41 KB, text/plain)
2008-04-24 09:35 UTC, Khashayar Naderehvandi
Details
output of lspci -vvv (11.41 KB, text/plain)
2008-04-24 09:36 UTC, Khashayar Naderehvandi
Details

Description Khashayar Naderehvandi 2008-03-01 04:03:08 UTC
Using the new fujitsu-laptop module on my lifebook p7010 I have encountered a peculiar problem that confuses user space tools such as xbacklight which can set the brightness of the panel.

For example, if brightness is set 100% cat /sys/class/backlight/fujitsu-laptop/brightness returns 7. As does cat /sys/class/backlight/fujitsu-laptop/actual_brightness. Decreasing the the brightness through the sysfs interface by echoing, say, 4 to fujitsu-laptop/brightness, also changes the value that resides in actual_brightness.

However (and this is the problem), changing the brightness  by means of the Fn-keys, 'actual_brightness' is changed but not 'brightness'. This causes tools such as the previously mentioned xbacklight to believe that the brightness is still 100%.

I'm not sure whether this bug in fact should be filed against those tools, or if it should be handled here.

Please let me know what information to attach here.
Comment 1 Jonathan Woithe 2008-03-02 14:47:33 UTC
The driver does not currently handle the function keys in any way.  Therefore the driver never gets to know about the operation of the function keys and so doesn't know that the value in "brightness" needs to be changed.  The reason actual_brightness changes after using the function keys is that a request to read actual_brightness causes the kernel to query the hardware, so it is always in sync with reality.  Reading "brightness" doesn't - as far as I know that is a value cached by the kernel and only gets updated when the backlight class driver is used to change the brightness.

Note that the interface in backlight/fujitsu-laptop is provided by the backlight class driver using get/set functions provided by fujitsu-laptop.  In the first instance the behaviour of "brightness" is probably more a result of the backlight class driver than fujitsu-laptop.  However, I am by no means an expert in this area and are willing to be proven wrong.

You may have noticed that the function key brightness control works even without fujitsu-laptop loaded.  It seems that these keys are handled by the BIOS in this laptop which is why they always work.  

As to whether Linux can even get ACPI notification of the brightness control keys on the S7020/S7010 is something I don't know - it's not something I've looked into.  I could investigate this in a few weeks if someone can point me to information about how to do it.

xbacklight probably uses brightness instead of actual_brightness to cut down the overhead of brightness polling.  I've not used xbacklight myself though, nor have I looked into its code, so this is purely speculation on my part.

I'm not sure of the way forward here and would be interested to hear the views of others.
Comment 2 Zhang Rui 2008-03-05 21:56:45 UTC
Please attach the lspci output.

If you are using a Intel graphic chipset, I think (In reply to comment #0)
> Using the new fujitsu-laptop module on my lifebook p7010 I have encountered a
> peculiar problem that confuses user space tools such as xbacklight which can
> set the brightness of the panel.
Which mode of xbacklight are you using?
please refer to http://bugzilla.kernel.org/show_bug.cgi?id=9625#c7

> For example, if brightness is set 100% cat
> /sys/class/backlight/fujitsu-laptop/brightness returns 7. As does cat
> /sys/class/backlight/fujitsu-laptop/actual_brightness. Decreasing the the
> brightness through the sysfs interface by echoing, say, 4 to
> fujitsu-laptop/brightness, also changes the value that resides in
> actual_brightness.
Hmm, xbacklight will not looking for /sys/class/backlight/fujitsu-laptop/ dir at all. So the invalid value from "brightness" has no impact to xbacklight.


> However (and this is the problem), changing the brightness  by means of the
> Fn-keys, 'actual_brightness' is changed but not 'brightness'. 
In fact, the xbacklight looks for the "actual_brightness" rather than the "brightness" file.

Khashayar, could you give a more detailed description of the problem you get please? i.e. why you think the xbacklight is confused?
Comment 3 Zhang Rui 2008-03-06 22:52:58 UTC
Please attach the acpidump output as well. thanks
Comment 4 Zhang Rui 2008-04-24 00:50:12 UTC
any updates on this?
Comment 5 Khashayar Naderehvandi 2008-04-24 09:34:33 UTC
I'm sorry for being really slow with this, a lot of real life things have taken up my time lately, and I've only had sporadic internet access. I'll attach the output of acpidump and lspci -vvv. 

In the meantime, this is the most elaborate description I can give (I'll get back later if need be): Say, I run xbacklight -set 100. This will maximize backlight. Running xbacklight -get, correctly says 100. If I after that decrease backlight to minimum with the Fn-buttons, and run xbacklight -get again, it will still say 100, instead of saying 0. xbacklight is in KERNEL mode.
Comment 6 Khashayar Naderehvandi 2008-04-24 09:35:23 UTC
Created attachment 15892 [details]
output of acpidump
Comment 7 Khashayar Naderehvandi 2008-04-24 09:36:06 UTC
Created attachment 15893 [details]
output of lspci -vvv
Comment 8 Jonathan Woithe 2008-04-27 17:14:24 UTC
On my test machine the function keys do correctly cause actual_brightness to be altered.  If xbacklight is referencing that file, why is it getting the "old" value?

Note too that the fujitsu driver hooks into the standard backlight subsystem, so in addition to sys/class/backlight/fujitsu-laptop/ functioning backlight controls should also show up in the standard location (I forget where that is off-hand).  Anyway, maybe it's the files in the standard location which are reporting the wrong values.  I'll see if I can find time tonight to have a quick look into this.

FYI I am currently working on a patch with another user which, among other things, includes a handler for the keypress events which are then passed to userspace in the normal way.  Any persistent brightness applet which watches for the standard brightness change events should therefore be able to track the hardware status.  Hopefully we'll have something publishable within a week or so.  Watch this space.
Comment 9 Khashayar Naderehvandi 2008-05-11 03:19:04 UTC
(In reply to comment #8)

> 
> FYI I am currently working on a patch with another user which, among other
> things, includes a handler for the keypress events which are then passed to
> userspace in the normal way.  Any persistent brightness applet which watches
> for the standard brightness change events should therefore be able to track
> the
> hardware status.  Hopefully we'll have something publishable within a week or
> so.  Watch this space.
> 

This would effectively solve the problem reported here. Looking forward to try it out when it's finished.
Comment 10 Zhang Rui 2008-07-14 20:38:12 UTC
Hi, jonathan,
any updates?
Comment 11 Jonathan Woithe 2008-07-14 21:35:20 UTC
Yes - I've been rather busy lately. :-(

Seriously, the work referred to by me in comment #9 has been merged into the ACPI subsystem and is in the ACPI patch queue for 2.6.27.  I'm not sure this will completely cure the problem mentioned at the start of this report since I'm still a little unsure what that problem was.  However, since ACPI brightness events are now passed to userspace, userspace tools have a greater chance of staying in sync with the hardware which seems to be the issue referred to in comment #5.
Comment 12 Zhang Rui 2008-07-14 22:40:15 UTC
so 2.6.27-rc1 is a good candidate for khashayar to try, right?
Comment 13 Jonathan Woithe 2008-07-14 23:00:06 UTC
I would expect so, yes.
Comment 14 Khashayar Naderehvandi 2008-07-15 14:02:54 UTC
(In reply to comment #12)
> so 2.6.27-rc1 is a good candidate for khashayar to try, right?
> 

A lot of real life things have been piling up lately. I will try the 2.6.27-rc1 as soon as I find the time.

(In reply to comment #11)
@Jonathan: The original problem is exactly what you seem to believe it is, namely that userspace tools get confused when reading the brightness level through the kernel interface in case the brightness levels are changed using fn-keys. Considering how you describe the fujitsu module now works, this bug should be closed. I guess someone should just test it first. Me, for example (and I will if I find the time to compile a kernel).
Comment 15 Jonathan Woithe 2008-07-15 17:01:40 UTC
You will of course have to wait for 2.6.27-rc1 to be released before testing it. :-)

In terms of closing the bug we should wait until you have had a chance to test 2.6.27-rc1 (or a later version).  I can't easily test it because I never saw the symptoms in the first place (in part because my system's configuration is very different from yours).
Comment 16 Zhang Rui 2008-08-28 01:30:43 UTC
Hi, Khashayar,

Does the problem still exists in the latest kernel, say 2.6.27-rc4?
Comment 17 Khashayar Naderehvandi 2008-08-28 02:03:54 UTC
Hi Zhang,

I don't seem to find the time to compile a kernel right now, but I did test a run with ubuntu's latest build of the 2.6.27 kernel, and with that kernel the problem is solved. I know ubuntu apply a bunch of patches to their kernels, but I'm quite sure they haven't done anything weird with the fujitsu module, so I'd suggest we mark this bug as fixed.

from dmesg:
[   30.404007] fujitsu-laptop: driver 0.4.2 successfully loaded.

Thanks to all involved!

Note You need to log in before you can comment on or make changes to this bug.