HI,
On 12-11-16 12:52, Pali Rohár wrote:
On Friday 11 November 2016 19:40:51 Kevin Locke wrote:
On Fri, 2016-11-11 at 15:33 +0100, Hans de Goede wrote:
On 11-11-16 15:12, Pali Rohár wrote:
My question remains. Is this for Thinklight or keyboard backlight?
Because Thinklinght has led device "tpacpi_led_thinklight" and
keyboard backlight has led device "tpacpi_led_kbdlight".
I would say both, this matches with the pre-existing
TP_ACPI_HKEY_THNKLGHT_MASK (they have a 1:1 mapping),
keep in mind that there are no thinkpads with both
a thinklight and a backlit keyboard, as those both
serve the same purpose so it looks like the re-used
the scancode.
That's not entirely correct. The ThinkPad T430 has both a ThinkLight
and a keyboard backlight. Pressing Fn-Space toggles between 4
states:
Both Off -> Backlight Low -> Backlight High -> ThinkLight On (BL Off)
I tested out your patch and observed the following behavior (printing
brightness initially and on POLLPRI):
# Initial state with both lights off.
tpacpi::kbd_backlight/brightness: 0
tpacpi::thinklight/brightness: 0
# Press Fn-Space. KBD Backlight comes on low.
tpacpi::kbd_backlight/brightness: 1
# Press Fn-Space. KBD Backlight brightens to high.
tpacpi::kbd_backlight/brightness: 2
# Press Fn-Space. KBD Backlight turns off. ThinkLight turns on.
tpacpi::kbd_backlight/brightness: 0
# Press Fn-Space. ThinkLight turns off.
tpacpi::kbd_backlight/brightness: 0
It works, but the behavior is not quite what I would hope for. There
are no poll events for tpacpi::thinklight/brightness
Yes, this is what I already pointed... That we should send event also
for tpacpi::thinklight.
Agreed.
and when the
ThinkLight turns off it triggers an unnecessary
tpacpi::kbd_backlight/brightness poll event.
It is problem? Or are forced to send event only if brightness level is
really changed? I think that bigger problem is when brightness changes,
but we do not send event.
Should not be event treated as "hey, brightness level was probably
changed, read file to get current status"?
That is what I was thinking too, waking up userspace poll()
waiters while nothing has changed is not 100% ideal, but
is not a big deal (esp. given the frequency with which this
will happen).
I will do a new version renaming the events / mask to indicate
that they apply to both the thinklight and the keyboard backlight;
and to also notify the thinklight led_classdev of changes
(if it is present).
Note that the led-class changes this relies on are still being
discussed, I will do a new version once that is sorted out.
If there is anything else I can do to assist, let me know.
Great, this is really useful to see somebody who can test patches on
those machines with both Thinklight and keyboard backlight...
+1 thank you for testing and for the feedback.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
