Re: [PATCH 1/2] platform/x86: uniwill-laptop: Fix brightness notify for 3 level keyboards

[Armin Wolf]

Am 25.07.26 um 01:00 schrieb Werner Sembach:

> The devices with 3 brightness level keyboards seem to send
> UNIWILL_OSD_KB_LED_LEVEL0, UNIWILL_OSD_KB_LED_LEVEL2, and
> UNIWILL_OSD_KB_LED_LEVEL4 and not UNIWILL_OSD_KB_LED_LEVEL0,
> UNIWILL_OSD_KB_LED_LEVEL1, and UNIWILL_OSD_KB_LED_LEVEL2 so this patch does
> not return the brightness based on one of these events, but the brightness
> read from hardware.
>
> Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
> ---
>   drivers/platform/x86/uniwill/uniwill-acpi.c | 22 +++------------------
>   1 file changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
> index 7a2eeaec4c961..d9a33be4f4a37 100644
> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
> @@ -1626,7 +1626,7 @@ static int uniwill_led_init(struct uniwill_data *data)
>   							 &init_data);
>   }
>   
> -static int uniwill_notify_kbd_led(struct uniwill_data *data, int brightness)
> +static int uniwill_notify_kbd_led(struct uniwill_data *data)
>   {
>   	struct led_classdev *led_cdev;
>   	int ret;
> @@ -1643,7 +1643,7 @@ static int uniwill_notify_kbd_led(struct uniwill_data *data, int brightness)
>   	if (ret < 0)
>   		return ret;
>   
> -	led_classdev_notify_brightness_hw_changed(led_cdev, brightness);
> +	led_classdev_notify_brightness_hw_changed(led_cdev, led_cdev->brightness);

WMI events are usually delivered with a large latency, a userspace application could
have changed the LED brightness value in the meantime.

Please check the maximum keyboard backlight brightness inside uniwill_notify_kbd_led()
and translate the brightness values instead:

/* Comment here */
switch (data->kbd_led_max_brightness) {
case 4:
	break;
case 2:
	switch (brightness) {
	case 0:
		break;
	case 2:
		brightness = 1;
		break;
	case 4:
		brightness = 2;
		break;
	default:
		return -EINVAL;
	}
	break;
default:
	return -EINVAL;
}

Thanks,
Armin Wolf

>   
>   	return 0;
>   }
> @@ -2215,30 +2215,14 @@ static int uniwill_notifier_call(struct notifier_block *nb, unsigned long action
>   
>   		return NOTIFY_OK;
>   	case UNIWILL_OSD_KB_LED_LEVEL0:
> -		if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
> -			return NOTIFY_DONE;
> -
> -		return notifier_from_errno(uniwill_notify_kbd_led(data, 0));
>   	case UNIWILL_OSD_KB_LED_LEVEL1:
> -		if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
> -			return NOTIFY_DONE;
> -
> -		return notifier_from_errno(uniwill_notify_kbd_led(data, 1));
>   	case UNIWILL_OSD_KB_LED_LEVEL2:
> -		if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
> -			return NOTIFY_DONE;
> -
> -		return notifier_from_errno(uniwill_notify_kbd_led(data, 2));
>   	case UNIWILL_OSD_KB_LED_LEVEL3:
> -		if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
> -			return NOTIFY_DONE;
> -
> -		return notifier_from_errno(uniwill_notify_kbd_led(data, 3));
>   	case UNIWILL_OSD_KB_LED_LEVEL4:
>   		if (!uniwill_device_supports(data, UNIWILL_FEATURE_KEYBOARD_BACKLIGHT))
>   			return NOTIFY_DONE;
>   
> -		return notifier_from_errno(uniwill_notify_kbd_led(data, 4));
> +		return notifier_from_errno(uniwill_notify_kbd_led(data));
>   	default:
>   		mutex_lock(&data->input_lock);
>   		sparse_keymap_report_event(data->input_device, action, 1, true);
>
> base-commit: caf8342512c3056005f475d350eeca089c3c6623