[PATCH 1/2] HID: wacom: Improve behavior of non-standard LED brightness values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Jason Gerecke <jason.gerecke@xxxxxxxxx>

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@xxxxxxxxx>
---
 drivers/hid/wacom.h     | 8 ++++++++
 drivers/hid/wacom_sys.c | 8 ++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

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;
 
-- 
2.47.1





[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux