HI,
On 06-03-17 14:39, Hans de Goede wrote:
Hi,
On 03-03-17 13:00, Pali Rohár wrote:
On Wednesday 01 March 2017 14:58:45 Hans de Goede wrote:
In my idea I do not add any additional information to event. Which means
all received events are same in current driver state and I can swap
them. So I can decide to drop one event when driver is changing
backlight level. And because I can swap received events it does not
matter if I drop "correct" one (that which was really generated by my
backlight level change) or any other before or after (as they are same).
The only important is that kernel use correct number of firmware events.
Idea for time interval of 2-3 seconds is that if event is delivered
later as after 3 seconds it is probably useless as driver application
code can be already out-of-sync. But this time interval is just another
idea and we do not need to use it.
I really don't like the 2-3 seconds stuff, but I'm fine with going
with your idea of just swallowing one event after a sysfs-write
without the timeout stuff.
Ok. I'm fine with just dropping exactly one event after setting new
value via sysfs. It is OK for you?
Yes that will work for me. I will prepare a new version. Note I probably
will not have time for this till the end of this week.
One thing I was wondering, is what will happen if we write the
same value to the sysfs brightness file twice, do we get events
for both writes? One way to avoid this problem would be to compare
the written value to the last known value and discard the write
(avoiding an expensive SMM call) if they are equal.
Discarding double writes does introduce a race between hotkey changes
vs sysfs writes, if a hotkey change has been made but the event has
not yet been handled then a sysfs write may be seen as being double
and get discarded even though it is not double. OTOH if the hotkey
press wins the race then the result would have been the write being
discarded anyways, so I don't think this is a real problem.
You're input on this would be appreciated. If writing the same value
twice does not generate events then we must filter out the double
writes. If it does drop events we can choose, the simplest would be
to not filter in that case, avoiding any races (and userspace normally
should not write the same value twice, but we cannot assume that).
Ok, I've spend some time on this today. As I was afraid no new wmi
events are generated when writing the same value to sysfs twice,
making the ignore one event after write approach tricky.
But luckily I've found a better fix, the kbd led related wmi events
with a type of 0x0010 are only send when using the hotkeys.
So if we ignore the events with a type of 0x0011, by dropping the
part of the patch which was mapping those to KEY_KBDILLUMTOGGLE
instead of to KEY_RESERVED, we only end up calling
dell_laptop_call_notifier from dell-wmi.c on hotkey presses and
we can drop the current vs new brightness comparison which
you disliked from dell-laptop.c.
So we can fix this by making the patch smaller :)
I will prepare a v9 with these changes and send that out later
today.
Regards,
Hans
