| Message ID | 20240903182633.870892-1-jason.gerecke@wacom.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [RFC] HID: wacom: Stop mangling tool IDs | expand |
On Tue, Sep 03, 2024 at 11:26:33AM -0700, Gerecke, Jason wrote: > From: Jason Gerecke <jason.gerecke@wacom.com> > > In ancient times, an off-by-one-nybble error resulted in the Wacom > driver sending "mangled" tool IDs to userspace. This mangling behavior > was later enshrined into a function so that devices using the then-new > generic codepath could share the same broken behavior. The mangled IDs > have not historically been a major problem for userspace since few > applications care about the exact numeric value of the IDs. As long as > the IDs were unique it didn't much matter. Some programs (cross- > platform and remote-desktop applications in particular) /do/ rely on > the IDs being correct, however. > > This patch rids the driver of the mangling behavior. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2") > References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs") > --- > I'd like to get the opinion of the kernel maintainers on making a > change like this at some point in the future. There are _very_ few > userspace uses of these IDs (primarily: drivers, compositors, and > tablet control panels) and my plan is to update those bits and then > give some time for the changes to roll out to users before re- > submitting this for real. I don't expect any kind of breakage since > we'll be taking our time with the rollout and userspace needs to > have handling for "unknown" IDs anyway (since Wacom periodically > releases new pens). I think if you take care of users of this data (it is Wacom-specific anyway because ABS_MISC does not have a defined behavior) I'd be fine changing the kernel. Up to Jiri/Benjamin though. Thanks.
On Tue, Sep 03, 2024 at 11:26:33AM -0700, Gerecke, Jason wrote: > From: Jason Gerecke <jason.gerecke@wacom.com> > > In ancient times, an off-by-one-nybble error resulted in the Wacom > driver sending "mangled" tool IDs to userspace. This mangling behavior > was later enshrined into a function so that devices using the then-new > generic codepath could share the same broken behavior. The mangled IDs > have not historically been a major problem for userspace since few > applications care about the exact numeric value of the IDs. As long as > the IDs were unique it didn't much matter. Some programs (cross- > platform and remote-desktop applications in particular) /do/ rely on > the IDs being correct, however. > > This patch rids the driver of the mangling behavior. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2") > References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs") > --- > I'd like to get the opinion of the kernel maintainers on making a > change like this at some point in the future. There are _very_ few > userspace uses of these IDs (primarily: drivers, compositors, and > tablet control panels) and my plan is to update those bits and then > give some time for the changes to roll out to users before re- > submitting this for real. I don't expect any kind of breakage since > we'll be taking our time with the rollout and userspace needs to > have handling for "unknown" IDs anyway (since Wacom periodically > releases new pens). Just to show my support :) Acked-by: Peter Hutterer <peter.hutterer@who-t.net> Cheers, Peter
On Tue, 3 Sep 2024, Gerecke, Jason wrote: > From: Jason Gerecke <jason.gerecke@wacom.com> > > In ancient times, an off-by-one-nybble error resulted in the Wacom > driver sending "mangled" tool IDs to userspace. This mangling behavior > was later enshrined into a function so that devices using the then-new > generic codepath could share the same broken behavior. The mangled IDs > have not historically been a major problem for userspace since few > applications care about the exact numeric value of the IDs. As long as > the IDs were unique it didn't much matter. Some programs (cross- > platform and remote-desktop applications in particular) /do/ rely on > the IDs being correct, however. > > This patch rids the driver of the mangling behavior. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2") > References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs") > --- > I'd like to get the opinion of the kernel maintainers on making a > change like this at some point in the future. There are _very_ few > userspace uses of these IDs (primarily: drivers, compositors, and > tablet control panels) and my plan is to update those bits and then > give some time for the changes to roll out to users before re- > submitting this for real. I don't expect any kind of breakage since > we'll be taking our time with the rollout and userspace needs to > have handling for "unknown" IDs anyway (since Wacom periodically > releases new pens). As you are in control of the whole ecosystem anyway (because it's Wacom specific), I don't see it as particularly problematic. Thanks,
On Sep 05 2024, Jiri Kosina wrote: > On Tue, 3 Sep 2024, Gerecke, Jason wrote: > > > From: Jason Gerecke <jason.gerecke@wacom.com> > > > > In ancient times, an off-by-one-nybble error resulted in the Wacom > > driver sending "mangled" tool IDs to userspace. This mangling behavior > > was later enshrined into a function so that devices using the then-new > > generic codepath could share the same broken behavior. The mangled IDs > > have not historically been a major problem for userspace since few > > applications care about the exact numeric value of the IDs. As long as > > the IDs were unique it didn't much matter. Some programs (cross- > > platform and remote-desktop applications in particular) /do/ rely on > > the IDs being correct, however. > > > > This patch rids the driver of the mangling behavior. > > > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > > References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2") > > References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs") > > --- > > I'd like to get the opinion of the kernel maintainers on making a > > change like this at some point in the future. There are _very_ few > > userspace uses of these IDs (primarily: drivers, compositors, and > > tablet control panels) and my plan is to update those bits and then > > give some time for the changes to roll out to users before re- > > submitting this for real. I don't expect any kind of breakage since > > we'll be taking our time with the rollout and userspace needs to > > have handling for "unknown" IDs anyway (since Wacom periodically > > releases new pens). > > As you are in control of the whole ecosystem anyway (because it's Wacom > specific), I don't see it as particularly problematic. > Yeah, same here. And that change doesn't impact the basic functionality of the styluses: they'll still be handled properly as pointers, but only the configuration panel IIUC. Peter mentioned about some slight differences in libinput which shouldn't impact the users as well IIRC. Anyway, that's an OK from me too. Cheers, Benjamin
On Thu, Sep 05, 2024 at 11:01:58AM +0200, Benjamin Tissoires wrote: > On Sep 05 2024, Jiri Kosina wrote: > > On Tue, 3 Sep 2024, Gerecke, Jason wrote: > > > > > From: Jason Gerecke <jason.gerecke@wacom.com> > > > > > > In ancient times, an off-by-one-nybble error resulted in the Wacom > > > driver sending "mangled" tool IDs to userspace. This mangling behavior > > > was later enshrined into a function so that devices using the then-new > > > generic codepath could share the same broken behavior. The mangled IDs > > > have not historically been a major problem for userspace since few > > > applications care about the exact numeric value of the IDs. As long as > > > the IDs were unique it didn't much matter. Some programs (cross- > > > platform and remote-desktop applications in particular) /do/ rely on > > > the IDs being correct, however. > > > > > > This patch rids the driver of the mangling behavior. > > > > > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > > > References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2") > > > References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs") > > > --- > > > I'd like to get the opinion of the kernel maintainers on making a > > > change like this at some point in the future. There are _very_ few > > > userspace uses of these IDs (primarily: drivers, compositors, and > > > tablet control panels) and my plan is to update those bits and then > > > give some time for the changes to roll out to users before re- > > > submitting this for real. I don't expect any kind of breakage since > > > we'll be taking our time with the rollout and userspace needs to > > > have handling for "unknown" IDs anyway (since Wacom periodically > > > releases new pens). > > > > As you are in control of the whole ecosystem anyway (because it's Wacom > > specific), I don't see it as particularly problematic. > > > > Yeah, same here. And that change doesn't impact the basic functionality > of the styluses: they'll still be handled properly as pointers, but only > the configuration panel IIUC. Peter mentioned about some slight > differences in libinput which shouldn't impact the users as well IIRC. Assuming libwacom is updated (and used) before this happens in the kernel, then there won't be a difference in libinput behaviour at all. And that's the plan for this anyway, have libwacom updated well before the kernel update ships. Otherwise, the only difference in libinput will be that it is treated as an unknown stylus. Those default to capabilities matching the kernel evdev codes (i.e. "everything") whereas a known stylus will only initialize capabilities that the stylus actually has. IOW you may get a libinput_tablet_tool that e.g. claims to support rotation and has a wheel when it doesn't. Not the end of the world either. Cheers, Peter
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 2541fa2e0fa3b..facab5bd37e41 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -671,11 +671,6 @@ static int wacom_intuos_pad(struct wacom_wac *wacom) return 1; } -static int wacom_intuos_id_mangle(int tool_id) -{ - return (tool_id & ~0xFFF) << 4 | (tool_id & 0xFFF); -} - static bool wacom_is_art_pen(int tool_id) { bool is_art_pen = false; @@ -1010,8 +1005,7 @@ static int wacom_intuos_general(struct wacom_wac *wacom) break; } - input_report_abs(input, ABS_MISC, - wacom_intuos_id_mangle(wacom->id[idx])); /* report tool id */ + input_report_abs(input, ABS_MISC, wacom->id[idx]); /* report tool id */ input_report_key(input, wacom->tool[idx], 1); input_event(input, EV_MSC, MSC_SERIAL, wacom->serial[idx]); wacom->reporting_data = true; @@ -1379,8 +1373,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom) input_report_key(pen_input, wacom->tool[0], prox); input_event(pen_input, EV_MSC, MSC_SERIAL, wacom->serial[0]); - input_report_abs(pen_input, ABS_MISC, - wacom_intuos_id_mangle(wacom->id[0])); /* report tool id */ + input_report_abs(pen_input, ABS_MISC, wacom->id[0]); /* report tool id */ } wacom->shared->stylus_in_proximity = prox; @@ -2514,13 +2507,6 @@ static void wacom_wac_pen_report(struct hid_device *hdev, input_report_key(input, BTN_STYLUS2, wacom_wac->hid_data.barrelswitch2); input_report_key(input, BTN_STYLUS3, wacom_wac->hid_data.barrelswitch3); - /* - * Non-USI EMR tools should have their IDs mangled to - * match the legacy behavior of wacom_intuos_general - */ - if (wacom_wac->serial[0] >> 52 == 1) - id = wacom_intuos_id_mangle(id); - /* * To ensure compatibility with xf86-input-wacom, we should * report the BTN_TOOL_* event prior to the ABS_MISC or