LED devices & poll() for brightness attribute
Subsystems (2)
Hello,
now in linus tree appeared new support for brightness_hw_changed sysfs attribute which provides poll() for reporting brightness changes from hardware itself.
But this support is useful just for one single central userspace application which will control all leds in system other application which will change state by /sys/class/leds/ will de-synchronize that one central application.
On linux system it is normal that more userspace application could change brightness level and to make them happy and synchronized with kernel state all those application should know that there was change (by other application) of brightness level. Currently they will know only about change done by hardware itself.
So I think new ABI is not sufficient and I would propose to add poll() support also for changes done by userspace, write() to attribute /sys/class/leds/.../brightness.
Signature · 2 lines
Pali Rohár [email protected]
Hi!
Quoted 4 lines
But this support is useful just for one single central userspace application which will control all leds in system other application which will change state by
/sys/class/leds/will de-synchronize that one central application.
Yes. Does it matter for some real use case?
Quoted 3 lines
So I think new ABI is not sufficient and I would propose to add
poll()support also for changes done by userspace,write()to attribute/sys/class/leds/.../brightness.
Not easily possible, as we have triggers, and this was discussed in great great lengths before. Please go through that discussion.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 8 lines · On Wednesday 22 February 2017 13:25:57 Pavel Machek wrote:
Hi!
Quoted 4 lines
But this support is useful just for one single central userspace application which will control all leds in system other application which will change state by
/sys/class/leds/will de-synchronize that one central application.Yes. Does it matter for some real use case?
Yes. E.g. systemd has already some support for changing leds. And I do not want to be forced to use systemd (or any other specific application) just because it already controls leds.
And my own application for controlling leds should know current state of them if it is possible.
Quoted 6 lines
Quoted 3 lines
So I think new ABI is not sufficient and I would propose to add
poll()support also for changes done by userspace,write()to attribute/sys/class/leds/.../brightness.Not easily possible, as we have triggers, and this was discussed in great great lengths before. Please go through that discussion.
Without triggers and blinking it should be possible.
Just notify only when some application do echo > brightness.
Signature · 2 lines
Pali Rohár [email protected]
Quoted 7 lines · Pali Rohár
Quoted 6 lines · On Wednesday 22 February 2017 13:25:57 Pavel Machek wrote:
Hi!
Quoted 4 lines
But this support is useful just for one single central userspace application which will control all leds in system other application which will change state by
/sys/class/leds/will de-synchronize that one central application.
Actually this is wrong.
Quoted 5 lines
Quoted 1 line
Yes. Does it matter for some real use case?
Yes. E.g. systemd has already some support for changing leds. And I do not want to be forced to use systemd (or any other specific application) just because it already controls leds.
So... you don't want to use systemd, so don't use systemd. What is the problem? :-)
Quoted 2 lines
And my own application for controlling leds should know current state of them if it is possible.
If you have at most one application controlling each LED, you are fine. That's quite common situation, serial port can also be opened at most once. Do you actually have two applications accessing one led? If so, what applications?
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
So I think new ABI is not sufficient and I would propose to add
poll()support also for changes done by userspace,write()to attribute/sys/class/leds/.../brightness.Not easily possible, as we have triggers, and this was discussed in great great lengths before. Please go through that discussion.
Without triggers and blinking it should be possible.
Just notify only when some application do echo > brightness.
Yes, we _could_ do that. Would be slightly confusing w.r.t. triggers. But is it good idea?
Userspace could easily keep /var/run/led/XXX/brightness, and inotify works over regular files..
And yes, with three color LEDs, userspace daemon managing LED state will be pretty much mandatory...
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 20 lines · On Wednesday 22 February 2017 21:54:08 Pavel Machek wrote:
Quoted 7 lines · Pali Rohár
Quoted 6 lines · On Wednesday 22 February 2017 13:25:57 Pavel Machek wrote:
Hi!
Quoted 4 lines
But this support is useful just for one single central userspace application which will control all leds in system other application which will change state by
/sys/class/leds/will de-synchronize that one central application.Actually this is wrong.
Quoted 5 lines
Quoted 1 line
Yes. Does it matter for some real use case?
Yes. E.g. systemd has already some support for changing leds. And I do not want to be forced to use systemd (or any other specific application) just because it already controls leds.
So... you don't want to use systemd, so don't use systemd. What is the problem? :-)
Quoted 7 lines
Quoted 2 lines
And my own application for controlling leds should know current state of them if it is possible.
If you have at most one application controlling each LED, you are fine. That's quite common situation, serial port can also be opened at most once. Do you actually have two applications accessing one led? If so, what applications?
First application which controls leds is integrated into desktop environment. In basic/default settings it do nothing automatically, just show GUI bar to user and allow user to adjust LED level.
Second one is basic shell script executed at specific situation (e.g. power adapter was unplugged) which changes LED level.
Now you have two independent applications which both could change LED level and at least one of them needs to know if another one changed level so can adjust GUI bar correctly.
I think this starts to be real situation for lot of users as more desktops (gnome, kde, ...) have already GUI bars for setting keyboard backlight and more users writes own scripts which at specific time/condition call echo > brightness.
Yes, if somebody uses GUI bar for manual keyboard backlight probably does not use led trigger or blinking feature. Probably that GUI desktop application could turn off them (do not know if kde/gnome application really turn off them).
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
So I think new ABI is not sufficient and I would propose to add
poll()support also for changes done by userspace,write()to attribute/sys/class/leds/.../brightness.Not easily possible, as we have triggers, and this was discussed in great great lengths before. Please go through that discussion.
Quoted 6 lines
Quoted 3 lines
Without triggers and blinking it should be possible.
Just notify only when some application do echo > brightness.
Yes, we _could_ do that. Would be slightly confusing w.r.t. triggers. But is it good idea?
Above situation for setting manual level of led is normal. And in this situation setting trigger or blink support does not make sense. Specially when user want to manually set led level.
I think it is good idea to provide some poll-able sysfs for this change. And if brightness is used for writing, then I would expect that brightness also provide poll() state.
Yes, it could be little confusing due to triggers/blinking support, but I think it is better as providing new sysfs file and it does not make this interface worse...
Quoted 2 lines
Userspace could easily keep
/var/run/led/XXX/brightness, and inotify works over regular files..
That would need to force all application to decide on this specific path. And looks bad as kernel already provides brightness attribute, why it should not also notify for changes on it?
Quoted 2 lines
And yes, with three color LEDs, userspace daemon managing LED state will be pretty much mandatory...
Why it should be running daemon? Script which you start when you want change is also enough... no need to have permanently running process.
Signature · 2 lines
Pali Rohár [email protected]
Hi!
Quoted 6 lines
Quoted 6 lines
So... you don't want to use systemd, so don't use systemd. What is the problem? :-)
Quoted 2 lines
And my own application for controlling leds should know current state of them if it is possible.
Quoted 8 lines
Quoted 4 lines
If you have at most one application controlling each LED, you are fine. That's quite common situation, serial port can also be opened at most once. Do you actually have two applications accessing one led? If so, what applications?
First application which controls leds is integrated into desktop environment. In basic/default settings it do nothing automatically, just show GUI bar to user and allow user to adjust LED level.
Ok. Desktop wants to control the backlight, because it makes sense to turn it off when the screen is blanked, and dim it when screen is dimmed. Situation is quite similar to display backlight, actually.
Quoted 2 lines
Second one is basic shell script executed at specific situation (e.g. power adapter was unplugged) which changes LED level.
Ok, and this is where the problems start. You are not supposed to control keyboard backlight like that. (In the same way you can't control display backlight like that.)
There are numerous problems with the shell script:
1) how to identify the keyboard backlight LED
2) there may be more then one
3) synchronization problems
4) need to be root
Your shell script should talk to the desktop, because it already knows where the backlight is, and the problems disappear.
Quoted 7 lines
Quoted 2 lines
Yes, we _could_ do that. Would be slightly confusing w.r.t. triggers. But is it good idea?
Above situation for setting manual level of led is normal. And in this situation setting trigger or blink support does not make sense. Specially when user want to manually set led level.
Quoted 7 lines
I think it is good idea to provide some poll-able sysfs for this change. And if brightness is used for writing, then I would expect that brightness also provide
poll()state.Yes, it could be little confusing due to triggers/blinking support, but I think it is better as providing new sysfs file and it does not make this interface worse...
I believe the confusion is not worth it. Talk to the desktop people, there should be good way to control the backlight without reaching lowlevel interfaces directly.
Quoted 5 lines
Quoted 2 lines
And yes, with three color LEDs, userspace daemon managing LED state will be pretty much mandatory...
Why it should be running daemon? Script which you start when you want change is also enough... no need to have permanently running process.
Because you want to play multiple patterns on the LED.
You probably also want to automatically control the keyboard backlight LEDs using ambient light sensor. Shell should not be involved.
This is one example of automatic backlight / keyboard backlight / LED patterns. And yes, this should be integrated with desktop, not using /sys/class/leds directly.
https://gitlab.com/tui/tui/blob/master/ofone/mond
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 24 lines · On Thursday 23 February 2017 15:37:26 Pavel Machek wrote:
Hi!
Quoted 6 lines
Quoted 6 lines
So... you don't want to use systemd, so don't use systemd. What is the problem? :-)
Quoted 2 lines
And my own application for controlling leds should know current state of them if it is possible.
Quoted 8 lines
Quoted 4 lines
If you have at most one application controlling each LED, you are fine. That's quite common situation, serial port can also be opened at most once. Do you actually have two applications accessing one led? If so, what applications?
First application which controls leds is integrated into desktop environment. In basic/default settings it do nothing automatically, just show GUI bar to user and allow user to adjust LED level.
Ok. Desktop wants to control the backlight, because it makes sense to turn it off when the screen is blanked, and dim it when screen is dimmed. Situation is quite similar to display backlight, actually.
Quoted 2 lines
Second one is basic shell script executed at specific situation (e.g. power adapter was unplugged) which changes LED level.
Quoted 7 lines
Ok, and this is where the problems start. You are not supposed to control keyboard backlight like that. (In the same way you can't control display backlight like that.)
There are numerous problems with the shell script:
1) how to identify the keyboard backlight LED
Exactly same as it is doing desktop. It is possible to 100% reimplement desktop logic for identification of keyboard backlight LED
Quoted 1 line
2) there may be more then one
Yes and script can be adjusted to use specific one (config option for script).
Quoted 1 line
3) synchronization problems
This is reason why poll() is needed. So desktop GUI will receive information that somebody else changed LED.
Quoted 1 line
4) need to be root
Hm? Why should be this a problem? Anyway you can chmod brightness attribute... But this "root need" is not a problem.
Also your script can be spawned by some udev rule or some other program which detect e.g. power supply connection/disconnection.
Quoted 2 lines
Your shell script should talk to the desktop, because it already knows where the backlight is, and the problems disappear.
Sometime there does not have to be any desktop loaded. It could but it does not have to be.
Look at TLP scripts. They are very popular, and they have no problems with 1) (needs proper configuration) 2) (fixed by 1) 3) (no visible problem) 4) (spawned by root udev daemon or manually as root).
Quoted 11 lines
Quoted 11 lines
Quoted 2 lines
Yes, we _could_ do that. Would be slightly confusing w.r.t. triggers. But is it good idea?
Above situation for setting manual level of led is normal. And in this situation setting trigger or blink support does not make sense. Specially when user want to manually set led level.
I think it is good idea to provide some poll-able sysfs for this change. And if brightness is used for writing, then I would expect that brightness also provide
poll()state.
Quoted 7 lines
Quoted 3 lines
Yes, it could be little confusing due to triggers/blinking support, but I think it is better as providing new sysfs file and it does not make this interface worse...
I believe the confusion is not worth it. Talk to the desktop people, there should be good way to control the backlight without reaching lowlevel interfaces directly.
And probably every desktop will use own different method like before. No thanks. I do not want to depend on particular desktop or implement zillions of different desktop APIs. And even depend on desktop.
If I think about addition to TLP scripts they should work also without running desktop...
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
And yes, with three color LEDs, userspace daemon managing LED state will be pretty much mandatory...
Why it should be running daemon? Script which you start when you want change is also enough... no need to have permanently running process.
Because you want to play multiple patterns on the LED.
One script can set 3 LEDs too. I do not see any problem here.
Quoted 2 lines
You probably also want to automatically control the keyboard backlight LEDs using ambient light sensor. Shell should not be involved.
That is something different. For automatic configuration of LEDs some running daemon is needed. But for manual and static configuration it is not needed. So depends on what you want...
Quoted 7 lines
This is one example of automatic backlight / keyboard backlight / LED patterns. And yes, this should be integrated with desktop, not using
/sys/class/ledsdirectly.https://gitlab.com/tui/tui/blob/master/ofone/mond
Pavel
Signature · 2 lines
Pali Rohár [email protected]
Hi!
Quoted 10 lines
Quoted 7 lines
Ok, and this is where the problems start. You are not supposed to control keyboard backlight like that. (In the same way you can't control display backlight like that.)
There are numerous problems with the shell script:
1) how to identify the keyboard backlight LED
Exactly same as it is doing desktop. It is possible to 100% reimplement desktop logic for identification of keyboard backlight LED
Exactly! If you are reimplementing logic from another project, it is strong hint that you are doing something wrong.
Quoted 4 lines
Quoted 1 line
2) there may be more then one
Yes and script can be adjusted to use specific one (config option for script).
No, you should autoconfigure it, actually. See N900, there are 6 leds controlling one backlight.
Quoted 4 lines
Quoted 1 line
3) synchronization problems
This is reason why
poll()is needed. So desktop GUI will receive information that somebody else changed LED.
Actually, poll will not help you. If both your script and desktop access the LED at the same time, there will be races. ("Hmm. I just wrote to the brightness file, and received poll notification. What is the current brightness?")
Quoted 7 lines
Quoted 2 lines
Your shell script should talk to the desktop, because it already knows where the backlight is, and the problems disappear.
Sometime there does not have to be any desktop loaded. It could but it does not have to be.
Look at TLP scripts. They are very popular, and they have no
Do you have a link?
Quoted 7 lines
Quoted 3 lines
I believe the confusion is not worth it. Talk to the desktop people, there should be good way to control the backlight without reaching lowlevel interfaces directly.
And probably every desktop will use own different method like before. No thanks. I do not want to depend on particular desktop or implement zillions of different desktop APIs. And even depend on desktop.
And I don't want led/brightness file to be used for interprocess communication. I especially don't want to deal with _6_ keyboard led files to be used for interprocess communication -- what is desktop expected to do when you set different brightnesses to each of them?
You don't want to deal with desktop API, yet expect all the desktops to follow your prefferred API (poll on led/brightness?).
There should be one component controlling the keyboard backlight. Your scripts should talk to that component.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 8 lines · On Thursday 23 February 2017 21:23:33 Pavel Machek wrote:
Hi!
Quoted 6 lines
Quoted 6 lines
Ok, and this is where the problems start. You are not supposed to control keyboard backlight like that. (In the same way you can't control display backlight like that.)
There are numerous problems with the shell script:
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
1) how to identify the keyboard backlight LED
Exactly same as it is doing desktop. It is possible to 100% reimplement desktop logic for identification of keyboard backlight LED
Exactly! If you are reimplementing logic from another project, it is strong hint that you are doing something wrong.
Manual configuration is another option.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
2) there may be more then one
Yes and script can be adjusted to use specific one (config option for script).
No, you should autoconfigure it,
Why? Because you prefer autoconfiguration and all other people must use it only? Does not seems a good argument.
For machine which does not change, static policy file is enough too. And for this simple case it will work correctly.
Why everybody must be forced to use some autoconfiguration and does not allow user to use his own manual configuration?
Quoted 1 line
actually. See N900, there are 6 leds controlling one backlight.
I'm not saying that one script must be universal for everything. My script is enough for machine with single keyboard led. And for other people who have only one backlight keyboard it will work fine too.
For other cases something other could be written...
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
3) synchronization problems
This is reason why
poll()is needed. So desktop GUI will receive information that somebody else changed LED.Actually, poll will not help you. If both your script and desktop access the LED at the same time, there will be races.
But does not have to be. If your own scripts are under your control then you can use it race-free.
Quoted 3 lines
("Hmm. I just
wrote to the brightness file, and received poll notification. What is the current brightness?")
Value read *after* receiving poll notification. By poll notification we know that brightness was changed. And to get new up-to-date value, new read() must be done.
If value is changed again, you will receive another poll notification.
I do not see any race here. You will get poll notification about every change, so what is the problem? You do not miss any change for sure.
Quoted 9 lines
Quoted 7 lines
Quoted 2 lines
Your shell script should talk to the desktop, because it already knows where the backlight is, and the problems disappear.
Sometime there does not have to be any desktop loaded. It could but it does not have to be.
Look at TLP scripts. They are very popular, and they have no
Do you have a link?
https://github.com/linrunner/TLP
http://linrunner.de/en/tlp/docs/tlp-configuration.html
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
I believe the confusion is not worth it. Talk to the desktop people, there should be good way to control the backlight without reaching lowlevel interfaces directly.
And probably every desktop will use own different method like before. No thanks. I do not want to depend on particular desktop or implement zillions of different desktop APIs. And even depend on desktop.
And I don't want led/brightness file to be used for interprocess communication.
You are not forced to use something. I'm proposing extending current ABI which can be used for processes who want those notification.
If you do not want it, just do not use it.
Quoted 9 lines
I especially don't want to deal with _6_ keyboard led files to be used for interprocess communication -- what is desktop expected to do when you set different brightnesses to each of them?
You don't want to deal with desktop API, yet expect all the desktops to follow your prefferred API (poll on led/brightness?).
There should be one component controlling the keyboard backlight. Your scripts should talk to that component.
Yes, somebody creates some right central daemon and everybody who want to use LED kernel API will be forced to use that one "right" daemon?
And what happens if somebody write new daemon with new userspace ABI just because that previous was incorrect/broken/does not fit all needs?
No kernel must not force which userspace application will be used for some operation. This sounds like Microsoft product...
Signature · 2 lines
Pali Rohár [email protected]
Hi!
Quoted 7 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
2) there may be more then one
Yes and script can be adjusted to use specific one (config option for script).
No, you should autoconfigure it,
Quoted 8 lines
Why? Because you prefer autoconfiguration and all other people must use it only? Does not seems a good argument.
For machine which does not change, static policy file is enough too. And for this simple case it will work correctly.
Why everybody must be forced to use some autoconfiguration and does not allow user to use his own manual configuration?
Why everybody must be forced to poll() on brightness file?
Quoted 7 lines
Quoted 1 line
actually. See N900, there are 6 leds controlling one backlight.
I'm not saying that one script must be universal for everything. My script is enough for machine with single keyboard led. And for other people who have only one backlight keyboard it will work fine too.
For other cases something other could be written...
For other cases, you need to communicate with the desktop.
Quoted 5 lines
Quoted 2 lines
And I don't want led/brightness file to be used for interprocess communication.
You are not forced to use something. I'm proposing extending current ABI which can be used for processes who want those notification.
Your proposal does not help you with your problem, unless you get desktop people to poll() on the brightness files, and do something inteligent with the results.
Hmm. And doing something inteligent will be quite hard given that we have 6 of those files on N900.
So, if you seriously want to do this, can you
1) write specification explaining how desktops should poll() the brightness files
2) get someone to volunteer to implement that specification for at least GNOME and KDE?
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 7 lines · On Thursday 23 February 2017 22:03:21 Pavel Machek wrote:
So, if you seriously want to do this, can you
1) write specification explaining how desktops should
poll()the brightness files2) get someone to volunteer to implement that specification for at least GNOME and KDE?
Hm... Probably I should try to implement it myself :-) and look deeply what I'm proposing...
Hans, you probably already did some changes for hw brightness changed attribute... Do you have some links or application parts which handle this for GNOME, KDE or other DE? So I could look at correct application?
IIRC KDE4 uses upowerd for controlling keyboard backlight.
Signature · 2 lines
Pali Rohár [email protected]
Hi,
Quoted 9 lines · Pali Rohár
Quoted 7 lines · On Thursday 23 February 2017 22:03:21 Pavel Machek wrote:
So, if you seriously want to do this, can you
1) write specification explaining how desktops should
poll()the brightness files2) get someone to volunteer to implement that specification for at least GNOME and KDE?
Quoted 8 lines
Hm... Probably I should try to implement it myself :-) and look deeply what I'm proposing...
Hans, you probably already did some changes for hw brightness changed attribute... Do you have some links or application parts which handle this for GNOME, KDE or other DE? So I could look at correct application?
IIRC KDE4 uses upowerd for controlling keyboard backlight.
Gnome uses upower too, and current upower master has patches to support brightness_hw_changed which you could use as a start.
Regards,
Hans
Quoted 15 lines · On Friday 24 February 2017 09:52:11 Hans de Goede wrote:
Hi,
Quoted 12 lines · Pali Rohár
Quoted 7 lines · On Thursday 23 February 2017 22:03:21 Pavel Machek wrote:
So, if you seriously want to do this, can you
1) write specification explaining how desktops should
poll()the brightness files2) get someone to volunteer to implement that specification for at least GNOME and KDE?
Hm... Probably I should try to implement it myself :-) and look deeply what I'm proposing...
Quoted 8 lines
Quoted 5 lines
Hans, you probably already did some changes for hw brightness changed attribute... Do you have some links or application parts which handle this for GNOME, KDE or other DE? So I could look at correct application?
IIRC KDE4 uses upowerd for controlling keyboard backlight.
Gnome uses upower too, and current upower master has patches to support
brightness_hw_changedwhich you could use as a start.
Ok, thanks!
Signature · 2 lines
Pali Rohár [email protected]