| Message ID | 20241210210059.87780-1-jason.gerecke@wacom.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] HID: wacom: Improve behavior of non-standard LED brightness values | expand |
On Tue, 10 Dec 2024, Gerecke, Jason wrote: > From: Jason Gerecke <jason.gerecke@wacom.com> > > Assigning a non-standard brightness value to an LED can cause the value > to slowly drift downward over time as the effects of integer division > accumulate. Each time that an LED is triggered, a series of set and get > calls occur. For example, if we assume a tablet with max_hlv = 100, then > when brightness is set to "200" through sysfs, the hlv value written to > hardware will be `200*100/255 = 78`. If the LED trigger is later activated, > the hlv value will be used to determine the brightness: `78*255/100 = 198`. > This lower brightness then used to set the brightness of the next LED. > However, `198*100/255 = 77`, so the next LED ends up slightly dimmer. > Each subsequent trigger activation will cause the brightness to continue > drifting down until we reach a point where the result of integer divsion > does not introduce any new error. > > This commit corrects the issue by being more careful about how we handle > scaling between the two ranges (0..max_{h,l}lv) and (0..LED_FULL). > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> Both applied, thanks.
diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h index 6f1443999d1d9..1deacb4568cb9 100644 --- a/drivers/hid/wacom.h +++ b/drivers/hid/wacom.h @@ -218,6 +218,14 @@ static inline __u32 wacom_s32tou(s32 value, __u8 n) return value & (1 << (n - 1)) ? value & (~(~0U << n)) : value; } +static inline u32 wacom_rescale(u32 value, u32 in_max, u32 out_max) +{ + if (in_max == 0 || out_max == 0) + return 0; + value = clamp(value, 0, in_max); + return DIV_ROUND_CLOSEST(value * out_max, in_max); +} + extern const struct hid_device_id wacom_ids[]; void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len); diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index 34428349fa311..5689bb6fcb264 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -1302,10 +1302,10 @@ enum led_brightness wacom_leds_brightness_get(struct wacom_led *led) struct wacom *wacom = led->wacom; if (wacom->led.max_hlv) - return led->hlv * LED_FULL / wacom->led.max_hlv; + return wacom_rescale(led->hlv, wacom->led.max_hlv, LED_FULL); if (wacom->led.max_llv) - return led->llv * LED_FULL / wacom->led.max_llv; + return wacom_rescale(led->llv, wacom->led.max_llv, LED_FULL); /* device doesn't support brightness tuning */ return LED_FULL; @@ -1337,8 +1337,8 @@ static int wacom_led_brightness_set(struct led_classdev *cdev, goto out; } - led->llv = wacom->led.llv = wacom->led.max_llv * brightness / LED_FULL; - led->hlv = wacom->led.hlv = wacom->led.max_hlv * brightness / LED_FULL; + led->llv = wacom->led.llv = wacom_rescale(brightness, LED_FULL, wacom->led.max_llv); + led->hlv = wacom->led.hlv = wacom_rescale(brightness, LED_FULL, wacom->led.max_hlv); wacom->led.groups[led->group].select = led->id;