Hi,
On 21-02-17 18:08, Pali Rohár wrote:
On Tuesday 21 February 2017 17:14:06 Hans de Goede wrote:
Hi,
On 21-02-17 16:13, 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.
Yes, because with triggers and blinking etc. it is impossible for
userspace to continuously monitor brigthness in a way which does not
cause a high system load.
Triggers and blinking features are out due to high cpu load. Fine.
But why also manual writes to /sys/class/leds/... by userspace
applications is filtered and not reported via poll()?
I agree that having a way for interested userspace to detect those
would be good, but that would need to be another API, may poll()
on the brightness attribute itself while excluding triggers / blinking
changes from wakeup ?
Anyways that is something to discuss in a thread specific to the
LED subsystem and somewhat orthogonal to this patch.
One disadvantage of poll() is that it does not give the source of
the change, so in retrospect I actually like the new brightness_hw_changed
attribute as that does give the source, which is something which we need
to know in userspace. In previous versions of the ABI I had to do
the same brightness comparison I'm doing in the dell-laptop driver
now in userspace, where it can never be done safely as userspace does
not know about other userspace.
Since the Dell smbios events don't provide us with a source of the
change, we need to compare the brightness to the last set brightness
somewhere and IMHO the kernel is the best (least bad) place to do
that.
Due to fundamental reasons we ignore all race condition between
libsmbios and kernel (they are basically not possible to solve). I'm
fine with this.
But why should setting keyboard backlight via smbios-keyboard-ctl and
via echo > /sys/class/leds/ behave differently?
Because we cannot solve the smbios-keyboard-ctl case, but we can solve
the echo case, as said we could probably use a new kernel ABI to allow
userspace to detect changes caused by the echo example, but that
really is a whole new discussion.
Regards,
Hans
