From 17d793f3ed53080dab6bbeabfc82de890c901001 Mon Sep 17 00:00:00 2001 From: Ping Cheng Date: Fri, 24 Feb 2023 08:26:43 -0800 Subject: [PATCH 1/4] HID: wacom: insert timestamp to packed Bluetooth (BT) events To fully utilize the BT polling/refresh rate, a few input events are sent together to reduce event delay. This causes issue to the timestamp generated by input_sync since all the events in the same packet would pretty much have the same timestamp. This patch inserts time interval to the events by averaging the total time used for sending the packet. This decision was mainly based on observing the actual time interval between each BT polling. The interval doesn't seem to be constant, due to the network and system environment. So, using solutions other than averaging doesn't end up with valid timestamps. Signed-off-by: Ping Cheng Reviewed-by: Jason Gerecke Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 26 ++++++++++++++++++++++++++ drivers/hid/wacom_wac.h | 1 + 2 files changed, 27 insertions(+) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 9312d611db8e..4cfa51416dbc 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -1308,6 +1308,9 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom) struct input_dev *pen_input = wacom->pen_input; unsigned char *data = wacom->data; + int number_of_valid_frames = 0; + int time_interval = 15000000; + ktime_t time_packet_received = ktime_get(); int i; if (wacom->features.type == INTUOSP2_BT || @@ -1328,12 +1331,30 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom) wacom->id[0] |= (wacom->serial[0] >> 32) & 0xFFFFF; } + /* number of valid frames */ for (i = 0; i < pen_frames; i++) { unsigned char *frame = &data[i*pen_frame_len + 1]; bool valid = frame[0] & 0x80; + + if (valid) + number_of_valid_frames++; + } + + if (number_of_valid_frames) { + if (wacom->hid_data.time_delayed) + time_interval = ktime_get() - wacom->hid_data.time_delayed; + time_interval /= number_of_valid_frames; + wacom->hid_data.time_delayed = time_packet_received; + } + + for (i = 0; i < number_of_valid_frames; i++) { + unsigned char *frame = &data[i*pen_frame_len + 1]; + bool valid = frame[0] & 0x80; bool prox = frame[0] & 0x40; bool range = frame[0] & 0x20; bool invert = frame[0] & 0x10; + int frames_number_reversed = number_of_valid_frames - i - 1; + int event_timestamp = time_packet_received - frames_number_reversed * time_interval; if (!valid) continue; @@ -1346,6 +1367,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom) wacom->tool[0] = 0; wacom->id[0] = 0; wacom->serial[0] = 0; + wacom->hid_data.time_delayed = 0; return; } @@ -1382,6 +1404,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom) get_unaligned_le16(&frame[11])); } } + if (wacom->tool[0]) { input_report_abs(pen_input, ABS_PRESSURE, get_unaligned_le16(&frame[5])); if (wacom->features.type == INTUOSP2_BT || @@ -1405,6 +1428,9 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom) wacom->shared->stylus_in_proximity = prox; + /* add timestamp to unpack the frames */ + input_set_timestamp(pen_input, event_timestamp); + input_sync(pen_input); } } diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h index 16f221388563..1a40bb8c5810 100644 --- a/drivers/hid/wacom_wac.h +++ b/drivers/hid/wacom_wac.h @@ -324,6 +324,7 @@ struct hid_data { int ps_connected; bool pad_input_event_flag; unsigned short sequence_number; + int time_delayed; }; struct wacom_remote_data { From 08a46b4190d345544d04ce4fe2e1844b772b8535 Mon Sep 17 00:00:00 2001 From: Ping Cheng Date: Sun, 9 Apr 2023 09:42:29 -0700 Subject: [PATCH 2/4] HID: wacom: Set a default resolution for older tablets Some older tablets may not report physical maximum for X/Y coordinates. Set a default to prevent undefined resolution. Signed-off-by: Ping Cheng Link: https://lore.kernel.org/r/20230409164229.29777-1-ping.cheng@wacom.com Signed-off-by: Benjamin Tissoires --- drivers/hid/wacom_wac.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 4cfa51416dbc..0c6a82c665c1 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -1921,6 +1921,7 @@ static void wacom_map_usage(struct input_dev *input, struct hid_usage *usage, int fmax = field->logical_maximum; unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid); int resolution_code = code; + int resolution = hidinput_calc_abs_res(field, resolution_code); if (equivalent_usage == HID_DG_TWIST) { resolution_code = ABS_RZ; @@ -1941,8 +1942,15 @@ static void wacom_map_usage(struct input_dev *input, struct hid_usage *usage, switch (type) { case EV_ABS: input_set_abs_params(input, code, fmin, fmax, fuzz, 0); - input_abs_set_res(input, code, - hidinput_calc_abs_res(field, resolution_code)); + + /* older tablet may miss physical usage */ + if ((code == ABS_X || code == ABS_Y) && !resolution) { + resolution = WACOM_INTUOS_RES; + hid_warn(input, + "Wacom usage (%d) missing resolution \n", + code); + } + input_abs_set_res(input, code, resolution); break; case EV_KEY: case EV_MSC: From 7fc68653fc2e1a3457bb886e10164119ac48734f Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Thu, 13 Apr 2023 11:17:42 -0700 Subject: [PATCH 3/4] HID: wacom: Lazy-init batteries Rather than creating batteries as part of the initial device probe, let's make the process lazy. This gives us the opportunity to prevent batteries from being created in situations where they are unnecessary. There are two cases in particular where batteries are being unnecessarily created at initialization. These are AES sensors (for which we don't know any battery status information until a battery-powered pen actually comes into prox) peripheral tablets which share HID descriptors between the wired-only and wireless-capable SKUs of a family of devices. This patch will delay battery initialization of the former until a pen actually comes into prox. It will delay battery initialization of the latter until either a pen comes into prox or a "heartbeat" packet is processed. Link: https://bugzilla.kernel.org/show_bug.cgi?id=217062 Link: https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/2354 Signed-off-by: Jason Gerecke Tested-by: Mario Limonciello Signed-off-by: Jiri Kosina --- drivers/hid/wacom_sys.c | 10 ---------- drivers/hid/wacom_wac.c | 13 ++++++------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index fb538a6c4add..8214896adada 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -2372,13 +2372,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless) if (error) goto fail; - if (!(features->device_type & WACOM_DEVICETYPE_WL_MONITOR) && - (features->quirks & WACOM_QUIRK_BATTERY)) { - error = wacom_initialize_battery(wacom); - if (error) - goto fail; - } - error = wacom_register_inputs(wacom); if (error) goto fail; @@ -2509,9 +2502,6 @@ static void wacom_wireless_work(struct work_struct *work) strscpy(wacom_wac->name, wacom_wac1->name, sizeof(wacom_wac->name)); - error = wacom_initialize_battery(wacom); - if (error) - goto fail; } return; diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 0c6a82c665c1..0351bce362d2 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -113,6 +113,11 @@ static void wacom_notify_battery(struct wacom_wac *wacom_wac, bool bat_connected, bool ps_connected) { struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac); + bool bat_initialized = wacom->battery.battery; + bool has_quirk = wacom_wac->features.quirks & WACOM_QUIRK_BATTERY; + + if (bat_initialized != has_quirk) + wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY); __wacom_notify_battery(&wacom->battery, bat_status, bat_capacity, bat_charging, bat_connected, ps_connected); @@ -3399,19 +3404,13 @@ static int wacom_status_irq(struct wacom_wac *wacom_wac, size_t len) int battery = (data[8] & 0x3f) * 100 / 31; bool charging = !!(data[8] & 0x80); + features->quirks |= WACOM_QUIRK_BATTERY; wacom_notify_battery(wacom_wac, WACOM_POWER_SUPPLY_STATUS_AUTO, battery, charging, battery || charging, 1); - - if (!wacom->battery.battery && - !(features->quirks & WACOM_QUIRK_BATTERY)) { - features->quirks |= WACOM_QUIRK_BATTERY; - wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY); - } } else if ((features->quirks & WACOM_QUIRK_BATTERY) && wacom->battery.battery) { features->quirks &= ~WACOM_QUIRK_BATTERY; - wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY); wacom_notify_battery(wacom_wac, POWER_SUPPLY_STATUS_UNKNOWN, 0, 0, 0, 0); } return 0; From bea407a427baa019758f29f4d31b26f008bb8cc6 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Thu, 13 Apr 2023 11:17:43 -0700 Subject: [PATCH 4/4] HID: wacom: generic: Set battery quirk only when we see battery data Some devices will include battery status usages in the HID descriptor but we won't see that battery data for one reason or another. For example, AES sensors won't send battery data unless an AES pen is in proximity. If a user does not have an AES pen but instead only interacts with the AES touchscreen with their fingers then there is no need for us to create a battery object. Similarly, if a family of peripherals shares the same HID descriptor between wired-only and wireless-capable SKUs, users of the former may never see a battery event and will not want a power_supply object created. Link: https://bugzilla.kernel.org/show_bug.cgi?id=217062 Link: https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/2354 Signed-off-by: Jason Gerecke Tested-by: Mario Limonciello Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 0351bce362d2..dc0f7d9a992c 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -1968,18 +1968,7 @@ static void wacom_map_usage(struct input_dev *input, struct hid_usage *usage, static void wacom_wac_battery_usage_mapping(struct hid_device *hdev, struct hid_field *field, struct hid_usage *usage) { - struct wacom *wacom = hid_get_drvdata(hdev); - struct wacom_wac *wacom_wac = &wacom->wacom_wac; - struct wacom_features *features = &wacom_wac->features; - unsigned equivalent_usage = wacom_equivalent_usage(usage->hid); - - switch (equivalent_usage) { - case HID_DG_BATTERYSTRENGTH: - case WACOM_HID_WD_BATTERY_LEVEL: - case WACOM_HID_WD_BATTERY_CHARGING: - features->quirks |= WACOM_QUIRK_BATTERY; - break; - } + return; } static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *field, @@ -2000,18 +1989,21 @@ static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *f wacom_wac->hid_data.bat_connected = 1; wacom_wac->hid_data.bat_status = WACOM_POWER_SUPPLY_STATUS_AUTO; } + wacom_wac->features.quirks |= WACOM_QUIRK_BATTERY; break; case WACOM_HID_WD_BATTERY_LEVEL: value = value * 100 / (field->logical_maximum - field->logical_minimum); wacom_wac->hid_data.battery_capacity = value; wacom_wac->hid_data.bat_connected = 1; wacom_wac->hid_data.bat_status = WACOM_POWER_SUPPLY_STATUS_AUTO; + wacom_wac->features.quirks |= WACOM_QUIRK_BATTERY; break; case WACOM_HID_WD_BATTERY_CHARGING: wacom_wac->hid_data.bat_charging = value; wacom_wac->hid_data.ps_connected = value; wacom_wac->hid_data.bat_connected = 1; wacom_wac->hid_data.bat_status = WACOM_POWER_SUPPLY_STATUS_AUTO; + wacom_wac->features.quirks |= WACOM_QUIRK_BATTERY; break; } } @@ -2027,18 +2019,15 @@ static void wacom_wac_battery_report(struct hid_device *hdev, { struct wacom *wacom = hid_get_drvdata(hdev); struct wacom_wac *wacom_wac = &wacom->wacom_wac; - struct wacom_features *features = &wacom_wac->features; - if (features->quirks & WACOM_QUIRK_BATTERY) { - int status = wacom_wac->hid_data.bat_status; - int capacity = wacom_wac->hid_data.battery_capacity; - bool charging = wacom_wac->hid_data.bat_charging; - bool connected = wacom_wac->hid_data.bat_connected; - bool powered = wacom_wac->hid_data.ps_connected; + int status = wacom_wac->hid_data.bat_status; + int capacity = wacom_wac->hid_data.battery_capacity; + bool charging = wacom_wac->hid_data.bat_charging; + bool connected = wacom_wac->hid_data.bat_connected; + bool powered = wacom_wac->hid_data.ps_connected; - wacom_notify_battery(wacom_wac, status, capacity, charging, - connected, powered); - } + wacom_notify_battery(wacom_wac, status, capacity, charging, + connected, powered); } static void wacom_wac_pad_usage_mapping(struct hid_device *hdev,