On 02/21/2017 04:13 PM, Pali Rohár wrote: > On Tuesday 21 February 2017 15:56:43 Hans de Goede wrote: >>> So do we really need this code which prevents update? >> >> Yes, because the ABI specification for the new brightness_hw_changed says >> that poll() listeners will only be woken up if the brightness is changed >> outside of the kernel and not when the kernel changes it itself. > > So in case there are two applications in userspace which want to monitor > brightness change and both of those application could change brightness > (via sysfs) then these two applications would not know about every > brightness change and would be out-of-sync of reality what is really > configured by kernel. > > This is one part which I did not liked in proposed ABI as it force > userspace to choose and use only one brightness monitoring application > at same time. > > Plus if ABI was specified that poll() could be used only for hardware > change (and not by software e.g. kernel/userspace) then such > functionality is not possible to implement for Dell machines. As it > looks like Dell firmware send event about every change of brightness and > we cannot distinguish if change was done by software (kernel) or by > hardware itself. > > I do not know now if you already accepted that ABI, I'm just saying that > I do not like it due to requirement for one userspace application as > listener and also because it is not what can be used for Dell machines. > > Jacek, was this requirement in ABI already accepted into stable? Yes, Linus today pulled that and it is in mainline now. From LED subsystem point of view I don't see any problem - the file is called brightness_hw_changed and its purpose is to report brightness changes occurred in the hardware, outside of kernel control. It also returns last brightness as was set by the hardware, which can be different from what old brightness file reports in the same time. -- Best regards, Jacek Anaszewski
