On Thu, Jan 4, 2018 at 10:52 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > On some Dell XPS models WMI events of type 0x0000 reporting a keycode of > 0xe00c get reported when the brightness of the LCD panel changes. > > This leads to us reporting false-positive kbd_led change events to > userspace which in turn leads to the kbd backlight OSD showing when it > should not. > > We already read the current keyboard backlight brightness value when > reporting events because the led_classdev_notify_brightness_hw_changed > API requires this. Compare this value to the last known value and filter > out duplicate events, fixing this. > > Note the fixed issue is esp. a problem on XPS models with an ambient light > sensor and automatic brightness adjustments turned on, this causes the kbd > backlight OSD to show all the time there. > Should it have Fixes tag? Pali, any comment on this? > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1514969 > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > --- > drivers/platform/x86/dell-laptop.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c > index cd4725e7e0b5..2ef3297a9efc 100644 > --- a/drivers/platform/x86/dell-laptop.c > +++ b/drivers/platform/x86/dell-laptop.c > @@ -1133,6 +1133,7 @@ static u8 kbd_previous_mode_bit; > > static bool kbd_led_present; > static DEFINE_MUTEX(kbd_led_mutex); > +static enum led_brightness kbd_led_level; > > /* > * NOTE: there are three ways to set the keyboard backlight level. > @@ -1947,6 +1948,7 @@ static enum led_brightness kbd_led_level_get(struct led_classdev *led_cdev) > static int kbd_led_level_set(struct led_classdev *led_cdev, > enum led_brightness value) > { > + enum led_brightness new_value = value; > struct kbd_state state; > struct kbd_state new_state; > u16 num; > @@ -1976,6 +1978,9 @@ static int kbd_led_level_set(struct led_classdev *led_cdev, > } > > out: > + if (ret == 0) > + kbd_led_level = new_value; > + > mutex_unlock(&kbd_led_mutex); > return ret; > } > @@ -2003,6 +2008,9 @@ static int __init kbd_led_init(struct device *dev) > if (kbd_led.max_brightness) > kbd_led.max_brightness--; > } > + > + kbd_led_level = kbd_led_level_get(NULL); > + > ret = led_classdev_register(dev, &kbd_led); > if (ret) > kbd_led_present = false; > @@ -2027,13 +2035,25 @@ static void kbd_led_exit(void) > static int dell_laptop_notifier_call(struct notifier_block *nb, > unsigned long action, void *data) > { > + bool changed = false; > + enum led_brightness new_kbd_led_level; > + > switch (action) { > case DELL_LAPTOP_KBD_BACKLIGHT_BRIGHTNESS_CHANGED: > if (!kbd_led_present) > break; > > - led_classdev_notify_brightness_hw_changed(&kbd_led, > - kbd_led_level_get(&kbd_led)); > + mutex_lock(&kbd_led_mutex); > + new_kbd_led_level = kbd_led_level_get(&kbd_led); > + if (kbd_led_level != new_kbd_led_level) { > + kbd_led_level = new_kbd_led_level; > + changed = true; > + } > + mutex_unlock(&kbd_led_mutex); > + > + if (changed) > + led_classdev_notify_brightness_hw_changed(&kbd_led, > + kbd_led_level); > break; > } > > -- > 2.14.3 > -- With Best Regards, Andy Shevchenko
