| Message ID | 20181205004228.10714-8-peter.hutterer@who-t.net (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 4435ff2f09a2fc43d1201d732e6e606b4d4b1ad5 |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: MS and Logitech high-resolution scroll wheel support | expand |
Hi Peter, On Tue, 4 Dec 2018 at 16:43, Peter Hutterer <peter.hutterer@who-t.net> wrote: > Changes to v2: > - m560 now has REL_HWHEEL_HI_RES (untested, I don't have the device) I just tested with my M560, and it now reports REL_HWHEEL_HI_RES correctly. Verified-by: Harry Cutts <hcutts@chromium.org> Thanks, Harry Cutts Chrome OS Touch/Input team
Le mer. 5 déc. 2018 à 01:44, Peter Hutterer <peter.hutterer@who-t.net> a écrit : > > From: Harry Cutts <hcutts@chromium.org> > > There are three features used by various Logitech mice for > high-resolution scrolling: the scrolling acceleration bit in HID++ 1.0, > and the x2120 and x2121 features in HID++ 2.0 and above. This patch > supports all three, and uses the multiplier reported by the mouse for > the HID++ 2.0+ features. Hi, The G500s (and the G500 too, I think) does support the "scrolling acceleration" bit. If I set it, I get around 8 events for each wheel "click", this is what this driver expects, right? If I understood correctly, I should try this patch with the HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse.
Hi Clement, On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER <clement.vuchener@gmail.com> wrote: > Hi, The G500s (and the G500 too, I think) does support the "scrolling > acceleration" bit. If I set it, I get around 8 events for each wheel > "click", this is what this driver expects, right? If I understood > correctly, I should try this patch with the > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. Thanks for the info! Yes, that should work. Harry Cutts Chrome OS Touch/Input team
Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > Hi Clement, > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > <clement.vuchener@gmail.com> wrote: > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > acceleration" bit. If I set it, I get around 8 events for each wheel > > "click", this is what this driver expects, right? If I understood > > correctly, I should try this patch with the > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > Thanks for the info! Yes, that should work. Well, it is not that simple. I get "Device not connected" errors for both interfaces of the mouse. logitech-hidpp-device 0003:046D:C24E.000E: Device not connected logitech-hidpp-device 0003:046D:C24E.000F: Device not connected Here is the usbmon trace when connecting a G500s: 0cd42cc0 3.313741 C Ii:3:001:1 0:2048 1 = 04 0cd42cc0 3.313750 S Ii:3:001:1 -:2048 4 < 17b49a80 3.313764 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b49a80 3.313775 C Ci:3:001:0 0 4 = 01010100 17b49a80 3.313781 S Co:3:001:0 s 23 01 0010 0002 0000 0 17b49a80 3.313789 C Co:3:001:0 0 0 17b49a80 3.313792 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b49a80 3.313797 C Ci:3:001:0 0 4 = 01010000 17b49a80 3.340415 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b49a80 3.340438 C Ci:3:001:0 0 4 = 01010000 17b49a80 3.367434 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b49a80 3.367448 C Ci:3:001:0 0 4 = 01010000 17b49a80 3.394434 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b49a80 3.394449 C Ci:3:001:0 0 4 = 01010000 17b49a80 3.421416 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b49a80 3.421431 C Ci:3:001:0 0 4 = 01010000 17b49a80 3.421690 S Co:3:001:0 s 23 03 0004 0002 0000 0 17b49a80 3.421707 C Co:3:001:0 0 0 17b49a80 3.483421 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b49a80 3.483436 C Ci:3:001:0 0 4 = 11010000 17b499c0 3.545429 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b499c0 3.545442 C Ci:3:001:0 0 4 = 03011000 17b499c0 3.545448 S Co:3:001:0 s 23 01 0014 0002 0000 0 17b499c0 3.545456 C Co:3:001:0 0 0 17b499c0 3.597659 S Ci:3:000:0 s 80 06 0100 0000 0040 64 < 17b499c0 3.597851 C Ci:3:000:0 0 8 = 12010002 00000008 17b499c0 3.597870 S Co:3:001:0 s 23 03 0004 0002 0000 0 17b499c0 3.597884 C Co:3:001:0 0 0 17b499c0 3.659419 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b499c0 3.659434 C Ci:3:001:0 0 4 = 11010000 17b499c0 3.721421 S Ci:3:001:0 s a3 00 0000 0002 0004 4 < 17b499c0 3.721435 C Ci:3:001:0 0 4 = 03011000 17b499c0 3.721442 S Co:3:001:0 s 23 01 0014 0002 0000 0 17b499c0 3.721451 C Co:3:001:0 0 0 17b49600 3.788420 S Ci:3:007:0 s 80 06 0100 0000 0012 18 < 17b49600 3.788897 C Ci:3:007:0 0 18 = 12010002 00000008 6d044ec2 01840102 0301 . . . . . . . . m . N . . . . . . . 17b49600 3.788920 S Ci:3:007:0 s 80 06 0600 0000 000a 10 < 17b49600 3.789057 C Ci:3:007:0 -32 0 17b49600 3.789092 S Ci:3:007:0 s 80 06 0600 0000 000a 10 < 17b49600 3.789438 C Ci:3:007:0 -32 0 17b49600 3.789459 S Ci:3:007:0 s 80 06 0600 0000 000a 10 < 17b49600 3.789827 C Ci:3:007:0 -32 0 17b49600 3.789850 S Ci:3:007:0 s 80 06 0200 0000 0009 9 < 17b49600 3.790272 C Ci:3:007:0 0 9 = 09023b00 020104a0 31 . . ; . . . . . 1 17b49600 3.790285 S Ci:3:007:0 s 80 06 0200 0000 003b 59 < 17b49600 3.791004 C Ci:3:007:0 0 59 = 09023b00 020104a0 31090400 00010301 02000921 11010001 22430007 05810308 . . ; . . . . . 1 . . . . . . . . . . ! . . . . " C . . . . . . 17b49600 3.791021 S Ci:3:007:0 s 80 06 0300 0000 00ff 255 < 17b49600 3.791203 C Ci:3:007:0 0 4 = 04030904 17b49600 3.791212 S Ci:3:007:0 s 80 06 0302 0409 00ff 255 < 17b49600 3.791829 C Ci:3:007:0 0 50 = 32034700 35003000 30007300 20004c00 61007300 65007200 20004700 61006d00 2 . G . 5 . 0 . 0 . s . . L . a . s . e . r . . G . a . m . 17b49600 3.791844 S Ci:3:007:0 s 80 06 0301 0409 00ff 255 < 17b49600 3.792141 C Ci:3:007:0 0 18 = 12034c00 6f006700 69007400 65006300 6800 . . L . o . g . i . t . e . c . h . 17b49600 3.792154 S Ci:3:007:0 s 80 06 0303 0409 00ff 255 < 17b49600 3.792563 C Ci:3:007:0 0 30 = 1e033300 34004500 31003300 38003500 30003800 36003000 30003000 3900 . . 3 . 4 . E . 1 . 3 . 8 . 5 . 0 . 8 . 6 . 0 . 0 . 0 . 9 . 17b49300 3.795944 S Co:3:007:0 s 00 09 0001 0000 0000 0 17b49300 3.795998 C Co:3:007:0 0 0 17b49300 3.796025 S Ci:3:007:0 s 80 06 0304 0409 00ff 255 < 17b49300 3.796392 C Ci:3:007:0 0 26 = 1a035500 38003400 2e003000 31005f00 42003000 30003000 3900 . . U . 8 . 4 . . . 0 . 1 . _ . B . 0 . 0 . 0 . 9 . 17b49900 3.796473 S Ci:3:007:0 s 80 06 0303 0409 00ff 255 < 17b49900 3.796883 C Ci:3:007:0 0 30 = 1e033300 34004500 31003300 38003500 30003800 36003000 30003000 3900 . . 3 . 4 . E . 1 . 3 . 8 . 5 . 0 . 8 . 6 . 0 . 0 . 0 . 9 . 17b49900 3.796914 S Co:3:007:0 s 21 0a 0000 0000 0000 0 17b49900 3.797027 C Co:3:007:0 -32 0 17b49900 3.797056 S Ci:3:007:0 s 81 06 2200 0000 0043 67 < 17b49900 3.798055 C Ci:3:007:0 0 67 = 05010902 a1010901 a1000509 19012910 15002501 95107501 81020501 16018026 . . . . . . . . . . . . . . ) . . . % . . . u . . . . . . . . & 17b49840 3.798350 S Co:3:007:0 s 21 09 0211 0000 0014 20 = 11ff0011 00000000 00000000 00000000 00000000 17b49840 3.798500 C Co:3:007:0 0 20 > 17b49240 9.289560 S Ci:3:007:0 s 80 06 0303 0409 00ff 255 < 17b49240 9.289999 C Ci:3:007:0 0 30 = 1e033300 34004500 31003300 38003500 30003800 36003000 30003000 3900 . . 3 . 4 . E . 1 . 3 . 8 . 5 . 0 . 8 . 6 . 0 . 0 . 0 . 9 . 17b49240 9.290040 S Co:3:007:0 s 21 0a 0000 0001 0000 0 17b49240 9.290145 C Co:3:007:0 -32 0 17b49240 9.290155 S Ci:3:007:0 s 81 06 2200 0001 007a 122 < 17b49240 9.291756 C Ci:3:007:0 0 122 = 05010906 a1018501 050719e0 29e71500 25017501 95088102 95057508 150026a4 . . . . . . . . . . . . ) . . . % . u . . . . . . . u . . . & . 17b49c00 9.292167 S Co:3:007:0 s 21 09 0211 0001 0014 20 = 11ff0011 00000000 00000000 00000000 00000000 17b49c00 9.292628 C Co:3:007:0 0 20 > It looks like the mouse is not responding to the root_get_protocol_version packet. I don't know why, but even if it did, it would not work correctly. The HID++ reports will only work with one of the interfaces, the other should be ignored by the driver instead of being disabled because it failed to respond to the HID++ report. The G500s and G500 HID++ interface also use the device index 0 instead of 0xff. For comparison, if I use my distribution kernel (4.19.7-200.fc28.x86_64) and send reports from user-space (using hidraw) instead of for-4.21/highres-wheel kernel. I get this kind of trace: 592193c0 0.253975 S Co:3:003:0 s 21 09 0211 0001 0014 20 = 11ff0011 00000000 00000000 00000000 00000000 592193c0 0.254426 C Co:3:003:0 0 20 > 9ee5ff00 0.255859 C Ii:3:003:2 0:1 7 = 10ff8f00 110800 9ee5ff00 0.255867 S Ii:3:003:2 -:1 20 < 592193c0 11.116794 S Co:3:003:0 s 21 09 0211 0001 0014 20 = 11000011 00000000 00000000 00000000 00000000 592193c0 11.117258 C Co:3:003:0 0 20 > 9ee5ff00 11.118023 C Ii:3:003:2 0:1 7 = 10008f00 110100 9ee5ff00 11.118033 S Ii:3:003:2 -:1 20 < When using device index 0xff the mouse responds with a ERR_UNKNOWN_DEVICE (0x08) error, when using device index 0 it responds ERR_INVALID_SUBID (0x01), the expected error for a HID++1.0 device. Here is the diff from the for-4.21/highres-wheel branch, I only added the devices to the device list with the required quirk. diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index ed35c9a9a110..a1c431e7841c 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -724,6 +724,7 @@ #define USB_DEVICE_ID_LOGITECH_KEYBOARD_G710_PLUS 0xc24d #define USB_DEVICE_ID_LOGITECH_MOUSE_C01A 0xc01a #define USB_DEVICE_ID_LOGITECH_MOUSE_C05A 0xc05a +#define USB_DEVICE_ID_LOGITECH_MOUSE_G500 0xc068 #define USB_DEVICE_ID_LOGITECH_MOUSE_C06A 0xc06a #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD_CORD 0xc20a #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD 0xc211 @@ -731,6 +732,7 @@ #define USB_DEVICE_ID_LOGITECH_DUAL_ACTION 0xc216 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2 0xc218 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2 0xc219 +#define USB_DEVICE_ID_LOGITECH_MOUSE_G500S 0xc24e #define USB_DEVICE_ID_LOGITECH_G29_WHEEL 0xc24f #define USB_DEVICE_ID_LOGITECH_G920_WHEEL 0xc262 #define USB_DEVICE_ID_LOGITECH_WINGMAN_F3D 0xc283 diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 15ed6177a7a3..b9b34ce455a4 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -3416,6 +3416,10 @@ static const struct hid_device_id hidpp_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL), .driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS}, + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOUSE_G500), + .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOUSE_G500S), + .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, {} };
Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER <clement.vuchener@gmail.com> a écrit : > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > Hi Clement, > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > <clement.vuchener@gmail.com> wrote: > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > "click", this is what this driver expects, right? If I understood > > > correctly, I should try this patch with the > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > Thanks for the info! Yes, that should work. > > Well, it is not that simple. I get "Device not connected" errors for > both interfaces of the mouse. I suspect the device is not responding because the hid device is not started. When is hid_hw_start supposed to be called? It is called early for HID_QUIRK_CLASS_G920 but later for other device. So the device is not started when hidpp_is_connected is called. Is this because most of the device in this driver are not real HID devices but DJ devices? How should non-DJ devices be treated?
On Wed, Dec 19, 2018 at 11:57 AM Clément VUCHENER <clement.vuchener@gmail.com> wrote: > > Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER > <clement.vuchener@gmail.com> a écrit : > > > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > > > Hi Clement, > > > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > > <clement.vuchener@gmail.com> wrote: > > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > > "click", this is what this driver expects, right? If I understood > > > > correctly, I should try this patch with the > > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > > > Thanks for the info! Yes, that should work. > > > > Well, it is not that simple. I get "Device not connected" errors for > > both interfaces of the mouse. > > I suspect the device is not responding because the hid device is not > started. When is hid_hw_start supposed to be called? It is called > early for HID_QUIRK_CLASS_G920 but later for other device. So the > device is not started when hidpp_is_connected is called. Is this > because most of the device in this driver are not real HID devices but > DJ devices? How should non-DJ devices be treated? Hi Clement, I have a series I sent last September that allows to support non DJ devices on logitech-hidpp (https://patchwork.kernel.org/project/linux-input/list/?series=16359). In its current form, with the latest upstream kernel, the series will oops during the .event() callback, which is easy enough to fix. However, I am currently trying to make it better as a second or third reading made me realized that there was a bunch of non-sense in it and a proper support would require slightly more work for the non unifying receiver case. I hope I'll be able to send out something by the end of the week. Cheers, Benjamin
Hi Benjamin, I tried again to add hi-res wheel support for the G500 with Hans de Goede's latest patch series you've just merged in for-5.2/logitech, it is much better but there is still some issues. The first one is the device index, I need to use device index 0 instead 0xff. I added a quick and dirty quirk (stealing in the QUIRK_CLASS range since the normal quirk range looks full) to change the device index assigned in __hidpp_send_report. After that the device is correctly initialized and the wheel multiplier is set. The second issue is that wheel values are not actually scaled according to the multiplier. I get 7/8 full scroll event for each wheel step. I think it happens because the mouse is split in two devices. The first device has the wheel events, and the second device has the HID++ reports. The wheel multiplier is only set on the second device (where the hi-res mode is enabled) and does not affect the wheel events from the first one. Le mer. 19 déc. 2018 à 21:35, Benjamin Tissoires <benjamin.tissoires@redhat.com> a écrit : > > On Wed, Dec 19, 2018 at 11:57 AM Clément VUCHENER > <clement.vuchener@gmail.com> wrote: > > > > Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER > > <clement.vuchener@gmail.com> a écrit : > > > > > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > > > > > Hi Clement, > > > > > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > > > <clement.vuchener@gmail.com> wrote: > > > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > > > "click", this is what this driver expects, right? If I understood > > > > > correctly, I should try this patch with the > > > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > > > > > Thanks for the info! Yes, that should work. > > > > > > Well, it is not that simple. I get "Device not connected" errors for > > > both interfaces of the mouse. > > > > I suspect the device is not responding because the hid device is not > > started. When is hid_hw_start supposed to be called? It is called > > early for HID_QUIRK_CLASS_G920 but later for other device. So the > > device is not started when hidpp_is_connected is called. Is this > > because most of the device in this driver are not real HID devices but > > DJ devices? How should non-DJ devices be treated? > > Hi Clement, > > I have a series I sent last September that allows to support non DJ > devices on logitech-hidpp > (https://patchwork.kernel.org/project/linux-input/list/?series=16359). > > In its current form, with the latest upstream kernel, the series will > oops during the .event() callback, which is easy enough to fix. > However, I am currently trying to make it better as a second or third > reading made me realized that there was a bunch of non-sense in it and > a proper support would require slightly more work for the non unifying > receiver case. > > I hope I'll be able to send out something by the end of the week. > > Cheers, > Benjamin
Hi Clément, On Wed, Apr 24, 2019 at 5:34 PM Clément VUCHENER <clement.vuchener@gmail.com> wrote: > > Hi Benjamin, > > I tried again to add hi-res wheel support for the G500 with Hans de > Goede's latest patch series you've just merged in for-5.2/logitech, it > is much better but there is still some issues. > > The first one is the device index, I need to use device index 0 > instead 0xff. I added a quick and dirty quirk (stealing in the > QUIRK_CLASS range since the normal quirk range looks full) to change > the device index assigned in __hidpp_send_report. After that the > device is correctly initialized and the wheel multiplier is set. Hmm, maybe we should restrain a little bit the reserved quirks... But actually, .driver_data and .quirks are both unsigned long, so you should be able to use the 64 bits. > > The second issue is that wheel values are not actually scaled > according to the multiplier. I get 7/8 full scroll event for each > wheel step. I think it happens because the mouse is split in two > devices. The first device has the wheel events, and the second device > has the HID++ reports. The wheel multiplier is only set on the second > device (where the hi-res mode is enabled) and does not affect the > wheel events from the first one. I would think this have to do with the device not accepting the command instead. Can you share some raw logs of the events (ideally with hid-recorder from https://gitlab.freedesktop.org/libevdev/hid-tools)? Cheers, Benjamin > > Le mer. 19 déc. 2018 à 21:35, Benjamin Tissoires > <benjamin.tissoires@redhat.com> a écrit : > > > > On Wed, Dec 19, 2018 at 11:57 AM Clément VUCHENER > > <clement.vuchener@gmail.com> wrote: > > > > > > Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER > > > <clement.vuchener@gmail.com> a écrit : > > > > > > > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > > > > > > > Hi Clement, > > > > > > > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > > > > "click", this is what this driver expects, right? If I understood > > > > > > correctly, I should try this patch with the > > > > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > > > > > > > Thanks for the info! Yes, that should work. > > > > > > > > Well, it is not that simple. I get "Device not connected" errors for > > > > both interfaces of the mouse. > > > > > > I suspect the device is not responding because the hid device is not > > > started. When is hid_hw_start supposed to be called? It is called > > > early for HID_QUIRK_CLASS_G920 but later for other device. So the > > > device is not started when hidpp_is_connected is called. Is this > > > because most of the device in this driver are not real HID devices but > > > DJ devices? How should non-DJ devices be treated? > > > > Hi Clement, > > > > I have a series I sent last September that allows to support non DJ > > devices on logitech-hidpp > > (https://patchwork.kernel.org/project/linux-input/list/?series=16359). > > > > In its current form, with the latest upstream kernel, the series will > > oops during the .event() callback, which is easy enough to fix. > > However, I am currently trying to make it better as a second or third > > reading made me realized that there was a bunch of non-sense in it and > > a proper support would require slightly more work for the non unifying > > receiver case. > > > > I hope I'll be able to send out something by the end of the week. > > > > Cheers, > > Benjamin
Le jeu. 25 avr. 2019 à 09:40, Benjamin Tissoires <benjamin.tissoires@redhat.com> a écrit : > > Hi Clément, > > On Wed, Apr 24, 2019 at 5:34 PM Clément VUCHENER > <clement.vuchener@gmail.com> wrote: > > > > Hi Benjamin, > > > > I tried again to add hi-res wheel support for the G500 with Hans de > > Goede's latest patch series you've just merged in for-5.2/logitech, it > > is much better but there is still some issues. > > > > The first one is the device index, I need to use device index 0 > > instead 0xff. I added a quick and dirty quirk (stealing in the > > QUIRK_CLASS range since the normal quirk range looks full) to change > > the device index assigned in __hidpp_send_report. After that the > > device is correctly initialized and the wheel multiplier is set. > > Hmm, maybe we should restrain a little bit the reserved quirks... > But actually, .driver_data and .quirks are both unsigned long, so you > should be able to use the 64 bits. Only on 64 bits architectures, or is the kernel forcing long integers to be 64 bits everywhere? > > > > > The second issue is that wheel values are not actually scaled > > according to the multiplier. I get 7/8 full scroll event for each > > wheel step. I think it happens because the mouse is split in two > > devices. The first device has the wheel events, and the second device > > has the HID++ reports. The wheel multiplier is only set on the second > > device (where the hi-res mode is enabled) and does not affect the > > wheel events from the first one. > > I would think this have to do with the device not accepting the > command instead. Can you share some raw logs of the events (ideally > with hid-recorder from > https://gitlab.freedesktop.org/libevdev/hid-tools)? I already checked with usbmon and double-checked by querying the register. The flag is set and accepted by the device and it behaves accordingly: it sends about 8 wheel events per step. hid-recorder takes hidraw nodes as parameters, how do I use it to record the initialization by the driver? > > Cheers, > Benjamin > > > > > Le mer. 19 déc. 2018 à 21:35, Benjamin Tissoires > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > On Wed, Dec 19, 2018 at 11:57 AM Clément VUCHENER > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER > > > > <clement.vuchener@gmail.com> a écrit : > > > > > > > > > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > > > > > > > > > Hi Clement, > > > > > > > > > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > > > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > > > > > "click", this is what this driver expects, right? If I understood > > > > > > > correctly, I should try this patch with the > > > > > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > > > > > > > > > Thanks for the info! Yes, that should work. > > > > > > > > > > Well, it is not that simple. I get "Device not connected" errors for > > > > > both interfaces of the mouse. > > > > > > > > I suspect the device is not responding because the hid device is not > > > > started. When is hid_hw_start supposed to be called? It is called > > > > early for HID_QUIRK_CLASS_G920 but later for other device. So the > > > > device is not started when hidpp_is_connected is called. Is this > > > > because most of the device in this driver are not real HID devices but > > > > DJ devices? How should non-DJ devices be treated? > > > > > > Hi Clement, > > > > > > I have a series I sent last September that allows to support non DJ > > > devices on logitech-hidpp > > > (https://patchwork.kernel.org/project/linux-input/list/?series=16359). > > > > > > In its current form, with the latest upstream kernel, the series will > > > oops during the .event() callback, which is easy enough to fix. > > > However, I am currently trying to make it better as a second or third > > > reading made me realized that there was a bunch of non-sense in it and > > > a proper support would require slightly more work for the non unifying > > > receiver case. > > > > > > I hope I'll be able to send out something by the end of the week. > > > > > > Cheers, > > > Benjamin
On Thu, Apr 25, 2019 at 10:25 AM Clément VUCHENER <clement.vuchener@gmail.com> wrote: > > Le jeu. 25 avr. 2019 à 09:40, Benjamin Tissoires > <benjamin.tissoires@redhat.com> a écrit : > > > > Hi Clément, > > > > On Wed, Apr 24, 2019 at 5:34 PM Clément VUCHENER > > <clement.vuchener@gmail.com> wrote: > > > > > > Hi Benjamin, > > > > > > I tried again to add hi-res wheel support for the G500 with Hans de > > > Goede's latest patch series you've just merged in for-5.2/logitech, it > > > is much better but there is still some issues. > > > > > > The first one is the device index, I need to use device index 0 > > > instead 0xff. I added a quick and dirty quirk (stealing in the > > > QUIRK_CLASS range since the normal quirk range looks full) to change > > > the device index assigned in __hidpp_send_report. After that the > > > device is correctly initialized and the wheel multiplier is set. > > > > Hmm, maybe we should restrain a little bit the reserved quirks... > > But actually, .driver_data and .quirks are both unsigned long, so you > > should be able to use the 64 bits. > > Only on 64 bits architectures, or is the kernel forcing long integers > to be 64 bits everywhere? Damnit, you are correct, there is no such enforcement :/ > > > > > > > > > The second issue is that wheel values are not actually scaled > > > according to the multiplier. I get 7/8 full scroll event for each > > > wheel step. I think it happens because the mouse is split in two > > > devices. The first device has the wheel events, and the second device > > > has the HID++ reports. The wheel multiplier is only set on the second > > > device (where the hi-res mode is enabled) and does not affect the > > > wheel events from the first one. > > > > I would think this have to do with the device not accepting the > > command instead. Can you share some raw logs of the events (ideally > > with hid-recorder from > > https://gitlab.freedesktop.org/libevdev/hid-tools)? > > I already checked with usbmon and double-checked by querying the > register. The flag is set and accepted by the device and it behaves > accordingly: it sends about 8 wheel events per step. OK, that's what I wanted to see. > > hid-recorder takes hidraw nodes as parameters, how do I use it to > record the initialization by the driver? You can't. But it doesn't really matter here because I wanted to check what your mouse is actually sending after the initialization. So if I read correctly: the mouse is sending high res data but evemu-recorder shows one REL_WHEEL event every 7/8 clicks? Cheers, Benjamin > > > Le mer. 19 déc. 2018 à 21:35, Benjamin Tissoires > > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > > > On Wed, Dec 19, 2018 at 11:57 AM Clément VUCHENER > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > > Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER > > > > > <clement.vuchener@gmail.com> a écrit : > > > > > > > > > > > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > > > > > > > > > > > Hi Clement, > > > > > > > > > > > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > > > > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > > > > > > "click", this is what this driver expects, right? If I understood > > > > > > > > correctly, I should try this patch with the > > > > > > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > > > > > > > > > > > Thanks for the info! Yes, that should work. > > > > > > > > > > > > Well, it is not that simple. I get "Device not connected" errors for > > > > > > both interfaces of the mouse. > > > > > > > > > > I suspect the device is not responding because the hid device is not > > > > > started. When is hid_hw_start supposed to be called? It is called > > > > > early for HID_QUIRK_CLASS_G920 but later for other device. So the > > > > > device is not started when hidpp_is_connected is called. Is this > > > > > because most of the device in this driver are not real HID devices but > > > > > DJ devices? How should non-DJ devices be treated? > > > > > > > > Hi Clement, > > > > > > > > I have a series I sent last September that allows to support non DJ > > > > devices on logitech-hidpp > > > > (https://patchwork.kernel.org/project/linux-input/list/?series=16359). > > > > > > > > In its current form, with the latest upstream kernel, the series will > > > > oops during the .event() callback, which is easy enough to fix. > > > > However, I am currently trying to make it better as a second or third > > > > reading made me realized that there was a bunch of non-sense in it and > > > > a proper support would require slightly more work for the non unifying > > > > receiver case. > > > > > > > > I hope I'll be able to send out something by the end of the week. > > > > > > > > Cheers, > > > > Benjamin
Le jeu. 25 avr. 2019 à 10:45, Benjamin Tissoires <benjamin.tissoires@redhat.com> a écrit : > > On Thu, Apr 25, 2019 at 10:25 AM Clément VUCHENER > <clement.vuchener@gmail.com> wrote: > > > > Le jeu. 25 avr. 2019 à 09:40, Benjamin Tissoires > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > Hi Clément, > > > > > > On Wed, Apr 24, 2019 at 5:34 PM Clément VUCHENER > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > Hi Benjamin, > > > > > > > > I tried again to add hi-res wheel support for the G500 with Hans de > > > > Goede's latest patch series you've just merged in for-5.2/logitech, it > > > > is much better but there is still some issues. > > > > > > > > The first one is the device index, I need to use device index 0 > > > > instead 0xff. I added a quick and dirty quirk (stealing in the > > > > QUIRK_CLASS range since the normal quirk range looks full) to change > > > > the device index assigned in __hidpp_send_report. After that the > > > > device is correctly initialized and the wheel multiplier is set. > > > > > > Hmm, maybe we should restrain a little bit the reserved quirks... > > > But actually, .driver_data and .quirks are both unsigned long, so you > > > should be able to use the 64 bits. > > > > Only on 64 bits architectures, or is the kernel forcing long integers > > to be 64 bits everywhere? > > Damnit, you are correct, there is no such enforcement :/ > > > > > > > > > > > > > > The second issue is that wheel values are not actually scaled > > > > according to the multiplier. I get 7/8 full scroll event for each > > > > wheel step. I think it happens because the mouse is split in two > > > > devices. The first device has the wheel events, and the second device > > > > has the HID++ reports. The wheel multiplier is only set on the second > > > > device (where the hi-res mode is enabled) and does not affect the > > > > wheel events from the first one. > > > > > > I would think this have to do with the device not accepting the > > > command instead. Can you share some raw logs of the events (ideally > > > with hid-recorder from > > > https://gitlab.freedesktop.org/libevdev/hid-tools)? > > > > I already checked with usbmon and double-checked by querying the > > register. The flag is set and accepted by the device and it behaves > > accordingly: it sends about 8 wheel events per step. > > OK, that's what I wanted to see. > > > > > hid-recorder takes hidraw nodes as parameters, how do I use it to > > record the initialization by the driver? > > You can't. But it doesn't really matter here because I wanted to check > what your mouse is actually sending after the initialization. > > So if I read correctly: the mouse is sending high res data but > evemu-recorder shows one REL_WHEEL event every 7/8 clicks? It is HID++ 1.0, there is no special high res data report, it sends standard HID mouse wheel report, but more of them. To be clear, here is a recording using the modified kernel. I moved the wheel one step up (8 events are generated), then one step down (8 events again + a 2-event bump). # EVEMU 1.3 # Kernel: 5.0.0logitech+ # DMI: dmi:bvnAmericanMegatrendsInc.:bvr1.40:bd01/17/2019:svnMicro-StarInternationalCo.,Ltd.:pnMS-7B98:pvr1.0:rvnMicro-StarInternationalCo.,Ltd.:rnZ390-APRO(MS-7B98):rvr1.0:cvnMicro-StarInternationalCo.,Ltd.:ct3:cvr1.0: # Input device name: "Logitech G500" # Input device ID: bus 0x03 vendor 0x46d product 0xc068 version 0x111 # Supported events: # Event type 0 (EV_SYN) # Event code 0 (SYN_REPORT) # Event code 1 (SYN_CONFIG) # Event code 2 (SYN_MT_REPORT) # Event code 3 (SYN_DROPPED) # Event code 4 ((null)) # Event code 5 ((null)) # Event code 6 ((null)) # Event code 7 ((null)) # Event code 8 ((null)) # Event code 9 ((null)) # Event code 10 ((null)) # Event code 11 ((null)) # Event code 12 ((null)) # Event code 13 ((null)) # Event code 14 ((null)) # Event code 15 (SYN_MAX) # Event type 1 (EV_KEY) # Event code 272 (BTN_LEFT) # Event code 273 (BTN_RIGHT) # Event code 274 (BTN_MIDDLE) # Event code 275 (BTN_SIDE) # Event code 276 (BTN_EXTRA) # Event code 277 (BTN_FORWARD) # Event code 278 (BTN_BACK) # Event code 279 (BTN_TASK) # Event code 280 ((null)) # Event code 281 ((null)) # Event code 282 ((null)) # Event code 283 ((null)) # Event code 284 ((null)) # Event code 285 ((null)) # Event code 286 ((null)) # Event code 287 ((null)) # Event type 2 (EV_REL) # Event code 0 (REL_X) # Event code 1 (REL_Y) # Event code 6 (REL_HWHEEL) # Event code 8 (REL_WHEEL) # Event code 11 ((null)) # Event code 12 ((null)) # Event type 4 (EV_MSC) # Event code 4 (MSC_SCAN) # Properties: N: Logitech G500 I: 0003 046d c068 0111 P: 00 00 00 00 00 00 00 00 B: 00 0b 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 ff ff 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 01 00 00 00 00 00 00 00 00 B: 02 43 19 00 00 00 00 00 00 B: 03 00 00 00 00 00 00 00 00 B: 04 10 00 00 00 00 00 00 00 B: 05 00 00 00 00 00 00 00 00 B: 11 00 00 00 00 00 00 00 00 B: 12 00 00 00 00 00 00 00 00 B: 14 00 00 00 00 00 00 00 00 B: 15 00 00 00 00 00 00 00 00 B: 15 00 00 00 00 00 00 00 00 ################################ # Waiting for events # ################################ E: 0.000001 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 0.000001 0002 000b 0120 # EV_REL / (null) 120 E: 0.000001 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +0ms E: 0.042009 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 0.042009 0002 000b 0120 # EV_REL / (null) 120 E: 0.042009 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +42ms E: 0.075016 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 0.075016 0002 000b 0120 # EV_REL / (null) 120 E: 0.075016 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +33ms E: 0.107977 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 0.107977 0002 000b 0120 # EV_REL / (null) 120 E: 0.107977 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +32ms E: 0.124979 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 0.124979 0002 000b 0120 # EV_REL / (null) 120 E: 0.124979 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +17ms E: 0.141950 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 0.141950 0002 000b 0120 # EV_REL / (null) 120 E: 0.141950 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +17ms E: 0.152950 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 0.152950 0002 000b 0120 # EV_REL / (null) 120 E: 0.152950 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +11ms E: 0.170943 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 0.170943 0002 000b 0120 # EV_REL / (null) 120 E: 0.170943 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +18ms E: 1.452035 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.452035 0002 000b -120 # EV_REL / (null) -120 E: 1.452035 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +1282ms E: 1.535031 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.535031 0002 000b -120 # EV_REL / (null) -120 E: 1.535031 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +83ms E: 1.574981 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.574981 0002 000b -120 # EV_REL / (null) -120 E: 1.574981 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +39ms E: 1.702039 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.702039 0002 000b -120 # EV_REL / (null) -120 E: 1.702039 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +128ms E: 1.713031 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.713031 0002 000b -120 # EV_REL / (null) -120 E: 1.713031 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +11ms E: 1.724036 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.724036 0002 000b -120 # EV_REL / (null) -120 E: 1.724036 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +11ms E: 1.731006 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.731006 0002 000b -120 # EV_REL / (null) -120 E: 1.731006 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +7ms E: 1.740043 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.740043 0002 000b -120 # EV_REL / (null) -120 E: 1.740043 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +9ms E: 1.765013 0002 0008 -001 # EV_REL / REL_WHEEL -1 E: 1.765013 0002 000b -120 # EV_REL / (null) -120 E: 1.765013 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +25ms E: 1.975044 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 1.975044 0002 000b 0120 # EV_REL / (null) 120 E: 1.975044 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +210ms Here is the corresponding hid-recorder: D: 0 R: 67 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 10 15 00 25 01 95 10 75 01 81 02 05 01 16 01 80 26 ff 7f 75 10 95 02 09 30 09 31 81 06 15 81 25 7f 75 08 95 01 09 38 81 06 05 0c 0a 38 02 95 01 81 06 c0 c0 N: Logitech G500 P: usb-0000:00:14.0-2/input0 I: 3 046d c068 D: 0 E: 0.000000 8 00 00 00 00 00 00 01 00 E: 0.041957 8 00 00 00 00 00 00 01 00 E: 0.074966 8 00 00 00 00 00 00 01 00 E: 0.107932 8 00 00 00 00 00 00 01 00 E: 0.124933 8 00 00 00 00 00 00 01 00 E: 0.141897 8 00 00 00 00 00 00 01 00 E: 0.152900 8 00 00 00 00 00 00 01 00 E: 0.170892 8 00 00 00 00 00 00 01 00 E: 1.452000 8 00 00 00 00 00 00 ff 00 E: 1.535009 8 00 00 00 00 00 00 ff 00 E: 1.574928 8 00 00 00 00 00 00 ff 00 E: 1.702009 8 00 00 00 00 00 00 ff 00 E: 1.713009 8 00 00 00 00 00 00 ff 00 E: 1.724001 8 00 00 00 00 00 00 ff 00 E: 1.730963 8 00 00 00 00 00 00 ff 00 E: 1.740005 8 00 00 00 00 00 00 ff 00 E: 1.764977 8 00 00 00 00 00 00 ff 00 E: 1.975014 8 00 00 00 00 00 00 01 00 The hi-res events should +/-15 instead of +/-120, and less REL_WHEEL events should be generated. This looks like the multiplier is set to 1 instead of 8. kernel log shows the multiplier is set but only for one of the two devices: input: Logitech G500 as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C068.0006/input/input25 hid-generic 0003:046D:C068.0006: input,hidraw5: USB HID v1.11 Mouse [Logitech G500] on usb-0000:00:14.0-2/input0 input: Logitech G500 Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input26 input: Logitech G500 Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input27 hid-generic 0003:046D:C068.0007: input,hiddev97,hidraw6: USB HID v1.11 Keyboard [Logitech G500] on usb-0000:00:14.0-2/input1 input: Logitech G500 as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C068.0006/input/input30 logitech-hidpp-device 0003:046D:C068.0006: input,hidraw5: USB HID v1.11 Mouse [Logitech G500] on usb-0000:00:14.0-2/input0 logitech-hidpp-device 0003:046D:C068.0007: HID++ 1.0 device connected. logitech-hidpp-device 0003:046D:C068.0007: multiplier = 8 input: Logitech G500 as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input31 logitech-hidpp-device 0003:046D:C068.0007: input,hiddev97,hidraw6: USB HID v1.11 Keyboard [Logitech G500] on usb-0000:00:14.0-2/input1 > > Cheers, > Benjamin > > > > > Le mer. 19 déc. 2018 à 21:35, Benjamin Tissoires > > > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > > > > > On Wed, Dec 19, 2018 at 11:57 AM Clément VUCHENER > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > > > > Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER > > > > > > <clement.vuchener@gmail.com> a écrit : > > > > > > > > > > > > > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > > > > > > > > > > > > > Hi Clement, > > > > > > > > > > > > > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > > > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > > > > > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > > > > > > > "click", this is what this driver expects, right? If I understood > > > > > > > > > correctly, I should try this patch with the > > > > > > > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > > > > > > > > > > > > > Thanks for the info! Yes, that should work. > > > > > > > > > > > > > > Well, it is not that simple. I get "Device not connected" errors for > > > > > > > both interfaces of the mouse. > > > > > > > > > > > > I suspect the device is not responding because the hid device is not > > > > > > started. When is hid_hw_start supposed to be called? It is called > > > > > > early for HID_QUIRK_CLASS_G920 but later for other device. So the > > > > > > device is not started when hidpp_is_connected is called. Is this > > > > > > because most of the device in this driver are not real HID devices but > > > > > > DJ devices? How should non-DJ devices be treated? > > > > > > > > > > Hi Clement, > > > > > > > > > > I have a series I sent last September that allows to support non DJ > > > > > devices on logitech-hidpp > > > > > (https://patchwork.kernel.org/project/linux-input/list/?series=16359). > > > > > > > > > > In its current form, with the latest upstream kernel, the series will > > > > > oops during the .event() callback, which is easy enough to fix. > > > > > However, I am currently trying to make it better as a second or third > > > > > reading made me realized that there was a bunch of non-sense in it and > > > > > a proper support would require slightly more work for the non unifying > > > > > receiver case. > > > > > > > > > > I hope I'll be able to send out something by the end of the week. > > > > > > > > > > Cheers, > > > > > Benjamin
On Thu, Apr 25, 2019 at 11:29 AM Clément VUCHENER <clement.vuchener@gmail.com> wrote: > > Le jeu. 25 avr. 2019 à 10:45, Benjamin Tissoires > <benjamin.tissoires@redhat.com> a écrit : > > > > On Thu, Apr 25, 2019 at 10:25 AM Clément VUCHENER > > <clement.vuchener@gmail.com> wrote: > > > > > > Le jeu. 25 avr. 2019 à 09:40, Benjamin Tissoires > > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > > > Hi Clément, > > > > > > > > On Wed, Apr 24, 2019 at 5:34 PM Clément VUCHENER > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > > Hi Benjamin, > > > > > > > > > > I tried again to add hi-res wheel support for the G500 with Hans de > > > > > Goede's latest patch series you've just merged in for-5.2/logitech, it > > > > > is much better but there is still some issues. > > > > > > > > > > The first one is the device index, I need to use device index 0 > > > > > instead 0xff. I added a quick and dirty quirk (stealing in the > > > > > QUIRK_CLASS range since the normal quirk range looks full) to change > > > > > the device index assigned in __hidpp_send_report. After that the > > > > > device is correctly initialized and the wheel multiplier is set. > > > > > > > > Hmm, maybe we should restrain a little bit the reserved quirks... > > > > But actually, .driver_data and .quirks are both unsigned long, so you > > > > should be able to use the 64 bits. > > > > > > Only on 64 bits architectures, or is the kernel forcing long integers > > > to be 64 bits everywhere? > > > > Damnit, you are correct, there is no such enforcement :/ > > > > > > > > > > > > > > > > > > > The second issue is that wheel values are not actually scaled > > > > > according to the multiplier. I get 7/8 full scroll event for each > > > > > wheel step. I think it happens because the mouse is split in two > > > > > devices. The first device has the wheel events, and the second device > > > > > has the HID++ reports. The wheel multiplier is only set on the second > > > > > device (where the hi-res mode is enabled) and does not affect the > > > > > wheel events from the first one. > > > > > > > > I would think this have to do with the device not accepting the > > > > command instead. Can you share some raw logs of the events (ideally > > > > with hid-recorder from > > > > https://gitlab.freedesktop.org/libevdev/hid-tools)? > > > > > > I already checked with usbmon and double-checked by querying the > > > register. The flag is set and accepted by the device and it behaves > > > accordingly: it sends about 8 wheel events per step. > > > > OK, that's what I wanted to see. > > > > > > > > hid-recorder takes hidraw nodes as parameters, how do I use it to > > > record the initialization by the driver? > > > > You can't. But it doesn't really matter here because I wanted to check > > what your mouse is actually sending after the initialization. > > > > So if I read correctly: the mouse is sending high res data but > > evemu-recorder shows one REL_WHEEL event every 7/8 clicks? > > It is HID++ 1.0, there is no special high res data report, it sends > standard HID mouse wheel report, but more of them. > > To be clear, here is a recording using the modified kernel. I moved > the wheel one step up (8 events are generated), then one step down (8 > events again + a 2-event bump). > > # EVEMU 1.3 [snipped] > E: 1.975014 8 00 00 00 00 00 00 01 00 > > The hi-res events should +/-15 instead of +/-120, and less REL_WHEEL > events should be generated. This looks like the multiplier is set to 1 > instead of 8. > > kernel log shows the multiplier is set but only for one of the two devices: > > input: Logitech G500 as > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C068.0006/input/input25 > hid-generic 0003:046D:C068.0006: input,hidraw5: USB HID v1.11 Mouse > [Logitech G500] on usb-0000:00:14.0-2/input0 > input: Logitech G500 Keyboard as > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input26 > input: Logitech G500 Consumer Control as > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input27 > hid-generic 0003:046D:C068.0007: input,hiddev97,hidraw6: USB HID v1.11 > Keyboard [Logitech G500] on usb-0000:00:14.0-2/input1 > input: Logitech G500 as > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C068.0006/input/input30 > logitech-hidpp-device 0003:046D:C068.0006: input,hidraw5: USB HID > v1.11 Mouse [Logitech G500] on usb-0000:00:14.0-2/input0 > logitech-hidpp-device 0003:046D:C068.0007: HID++ 1.0 device connected. > logitech-hidpp-device 0003:046D:C068.0007: multiplier = 8 > input: Logitech G500 as > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input31 > logitech-hidpp-device 0003:046D:C068.0007: input,hiddev97,hidraw6: USB > HID v1.11 Keyboard [Logitech G500] on usb-0000:00:14.0-2/input1 > Oh, now I see. And yes you are correct. I wonder if we should not consider the mouse in hid-logitech-dj then. And let hid-logitech-dj merge the 2 nodes together into one hidpp device, and so we are facing a "regular" HID++ device which is consistent. Unfortunately, I am not sure I'll have the time to work on it this week :/ Cheers, Benjamin > > > > > > > Le mer. 19 déc. 2018 à 21:35, Benjamin Tissoires > > > > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > > > > > > > On Wed, Dec 19, 2018 at 11:57 AM Clément VUCHENER > > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > > > > > > Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER > > > > > > > <clement.vuchener@gmail.com> a écrit : > > > > > > > > > > > > > > > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > > > > > > > > > > > > > > > Hi Clement, > > > > > > > > > > > > > > > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > > > > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > > > > > > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > > > > > > > > "click", this is what this driver expects, right? If I understood > > > > > > > > > > correctly, I should try this patch with the > > > > > > > > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > > > > > > > > > > > > > > > Thanks for the info! Yes, that should work. > > > > > > > > > > > > > > > > Well, it is not that simple. I get "Device not connected" errors for > > > > > > > > both interfaces of the mouse. > > > > > > > > > > > > > > I suspect the device is not responding because the hid device is not > > > > > > > started. When is hid_hw_start supposed to be called? It is called > > > > > > > early for HID_QUIRK_CLASS_G920 but later for other device. So the > > > > > > > device is not started when hidpp_is_connected is called. Is this > > > > > > > because most of the device in this driver are not real HID devices but > > > > > > > DJ devices? How should non-DJ devices be treated? > > > > > > > > > > > > Hi Clement, > > > > > > > > > > > > I have a series I sent last September that allows to support non DJ > > > > > > devices on logitech-hidpp > > > > > > (https://patchwork.kernel.org/project/linux-input/list/?series=16359). > > > > > > > > > > > > In its current form, with the latest upstream kernel, the series will > > > > > > oops during the .event() callback, which is easy enough to fix. > > > > > > However, I am currently trying to make it better as a second or third > > > > > > reading made me realized that there was a bunch of non-sense in it and > > > > > > a proper support would require slightly more work for the non unifying > > > > > > receiver case. > > > > > > > > > > > > I hope I'll be able to send out something by the end of the week. > > > > > > > > > > > > Cheers, > > > > > > Benjamin
Le jeu. 25 avr. 2019 à 11:35, Benjamin Tissoires <benjamin.tissoires@redhat.com> a écrit : > > On Thu, Apr 25, 2019 at 11:29 AM Clément VUCHENER > <clement.vuchener@gmail.com> wrote: > > > > Le jeu. 25 avr. 2019 à 10:45, Benjamin Tissoires > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > On Thu, Apr 25, 2019 at 10:25 AM Clément VUCHENER > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > Le jeu. 25 avr. 2019 à 09:40, Benjamin Tissoires > > > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > > > > > Hi Clément, > > > > > > > > > > On Wed, Apr 24, 2019 at 5:34 PM Clément VUCHENER > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > > > > Hi Benjamin, > > > > > > > > > > > > I tried again to add hi-res wheel support for the G500 with Hans de > > > > > > Goede's latest patch series you've just merged in for-5.2/logitech, it > > > > > > is much better but there is still some issues. > > > > > > > > > > > > The first one is the device index, I need to use device index 0 > > > > > > instead 0xff. I added a quick and dirty quirk (stealing in the > > > > > > QUIRK_CLASS range since the normal quirk range looks full) to change > > > > > > the device index assigned in __hidpp_send_report. After that the > > > > > > device is correctly initialized and the wheel multiplier is set. > > > > > > > > > > Hmm, maybe we should restrain a little bit the reserved quirks... > > > > > But actually, .driver_data and .quirks are both unsigned long, so you > > > > > should be able to use the 64 bits. > > > > > > > > Only on 64 bits architectures, or is the kernel forcing long integers > > > > to be 64 bits everywhere? > > > > > > Damnit, you are correct, there is no such enforcement :/ > > > > > > > > > > > > > > > > > > > > > > > > The second issue is that wheel values are not actually scaled > > > > > > according to the multiplier. I get 7/8 full scroll event for each > > > > > > wheel step. I think it happens because the mouse is split in two > > > > > > devices. The first device has the wheel events, and the second device > > > > > > has the HID++ reports. The wheel multiplier is only set on the second > > > > > > device (where the hi-res mode is enabled) and does not affect the > > > > > > wheel events from the first one. > > > > > > > > > > I would think this have to do with the device not accepting the > > > > > command instead. Can you share some raw logs of the events (ideally > > > > > with hid-recorder from > > > > > https://gitlab.freedesktop.org/libevdev/hid-tools)? > > > > > > > > I already checked with usbmon and double-checked by querying the > > > > register. The flag is set and accepted by the device and it behaves > > > > accordingly: it sends about 8 wheel events per step. > > > > > > OK, that's what I wanted to see. > > > > > > > > > > > hid-recorder takes hidraw nodes as parameters, how do I use it to > > > > record the initialization by the driver? > > > > > > You can't. But it doesn't really matter here because I wanted to check > > > what your mouse is actually sending after the initialization. > > > > > > So if I read correctly: the mouse is sending high res data but > > > evemu-recorder shows one REL_WHEEL event every 7/8 clicks? > > > > It is HID++ 1.0, there is no special high res data report, it sends > > standard HID mouse wheel report, but more of them. > > > > To be clear, here is a recording using the modified kernel. I moved > > the wheel one step up (8 events are generated), then one step down (8 > > events again + a 2-event bump). > > > > # EVEMU 1.3 > [snipped] > > E: 1.975014 8 00 00 00 00 00 00 01 00 > > > > The hi-res events should +/-15 instead of +/-120, and less REL_WHEEL > > events should be generated. This looks like the multiplier is set to 1 > > instead of 8. > > > > kernel log shows the multiplier is set but only for one of the two devices: > > > > input: Logitech G500 as > > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C068.0006/input/input25 > > hid-generic 0003:046D:C068.0006: input,hidraw5: USB HID v1.11 Mouse > > [Logitech G500] on usb-0000:00:14.0-2/input0 > > input: Logitech G500 Keyboard as > > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input26 > > input: Logitech G500 Consumer Control as > > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input27 > > hid-generic 0003:046D:C068.0007: input,hiddev97,hidraw6: USB HID v1.11 > > Keyboard [Logitech G500] on usb-0000:00:14.0-2/input1 > > input: Logitech G500 as > > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C068.0006/input/input30 > > logitech-hidpp-device 0003:046D:C068.0006: input,hidraw5: USB HID > > v1.11 Mouse [Logitech G500] on usb-0000:00:14.0-2/input0 > > logitech-hidpp-device 0003:046D:C068.0007: HID++ 1.0 device connected. > > logitech-hidpp-device 0003:046D:C068.0007: multiplier = 8 > > input: Logitech G500 as > > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C068.0007/input/input31 > > logitech-hidpp-device 0003:046D:C068.0007: input,hiddev97,hidraw6: USB > > HID v1.11 Keyboard [Logitech G500] on usb-0000:00:14.0-2/input1 > > > > Oh, now I see. And yes you are correct. > > I wonder if we should not consider the mouse in hid-logitech-dj then. > And let hid-logitech-dj merge the 2 nodes together into one hidpp > device, and so we are facing a "regular" HID++ device which is > consistent. This would change how userspace see the devices, isn't it? And I don't like the dj hidraw nodes, they mess with the device index: you get answers with a device index different from the one used in the corresponding request. > > Unfortunately, I am not sure I'll have the time to work on it this week :/ > > Cheers, > Benjamin > > > > > > > > > > Le mer. 19 déc. 2018 à 21:35, Benjamin Tissoires > > > > > > <benjamin.tissoires@redhat.com> a écrit : > > > > > > > > > > > > > > On Wed, Dec 19, 2018 at 11:57 AM Clément VUCHENER > > > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > > > > > > > > Le sam. 15 déc. 2018 à 15:45, Clément VUCHENER > > > > > > > > <clement.vuchener@gmail.com> a écrit : > > > > > > > > > > > > > > > > > > Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@chromium.org> a écrit : > > > > > > > > > > > > > > > > > > > > Hi Clement, > > > > > > > > > > > > > > > > > > > > On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER > > > > > > > > > > <clement.vuchener@gmail.com> wrote: > > > > > > > > > > > Hi, The G500s (and the G500 too, I think) does support the "scrolling > > > > > > > > > > > acceleration" bit. If I set it, I get around 8 events for each wheel > > > > > > > > > > > "click", this is what this driver expects, right? If I understood > > > > > > > > > > > correctly, I should try this patch with the > > > > > > > > > > > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse. > > > > > > > > > > > > > > > > > > > > Thanks for the info! Yes, that should work. > > > > > > > > > > > > > > > > > > Well, it is not that simple. I get "Device not connected" errors for > > > > > > > > > both interfaces of the mouse. > > > > > > > > > > > > > > > > I suspect the device is not responding because the hid device is not > > > > > > > > started. When is hid_hw_start supposed to be called? It is called > > > > > > > > early for HID_QUIRK_CLASS_G920 but later for other device. So the > > > > > > > > device is not started when hidpp_is_connected is called. Is this > > > > > > > > because most of the device in this driver are not real HID devices but > > > > > > > > DJ devices? How should non-DJ devices be treated? > > > > > > > > > > > > > > Hi Clement, > > > > > > > > > > > > > > I have a series I sent last September that allows to support non DJ > > > > > > > devices on logitech-hidpp > > > > > > > (https://patchwork.kernel.org/project/linux-input/list/?series=16359). > > > > > > > > > > > > > > In its current form, with the latest upstream kernel, the series will > > > > > > > oops during the .event() callback, which is easy enough to fix. > > > > > > > However, I am currently trying to make it better as a second or third > > > > > > > reading made me realized that there was a bunch of non-sense in it and > > > > > > > a proper support would require slightly more work for the non unifying > > > > > > > receiver case. > > > > > > > > > > > > > > I hope I'll be able to send out something by the end of the week. > > > > > > > > > > > > > > Cheers, > > > > > > > Benjamin
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 95ba9db6e613..a66daf8acd87 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -21,6 +21,7 @@ #include <linux/module.h> #include <linux/slab.h> #include <linux/sched.h> +#include <linux/sched/clock.h> #include <linux/kfifo.h> #include <linux/input/mt.h> #include <linux/workqueue.h> @@ -64,6 +65,14 @@ MODULE_PARM_DESC(disable_tap_to_click, #define HIDPP_QUIRK_NO_HIDINPUT BIT(23) #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS BIT(24) #define HIDPP_QUIRK_UNIFYING BIT(25) +#define HIDPP_QUIRK_HI_RES_SCROLL_1P0 BIT(26) +#define HIDPP_QUIRK_HI_RES_SCROLL_X2120 BIT(27) +#define HIDPP_QUIRK_HI_RES_SCROLL_X2121 BIT(28) + +/* Convenience constant to check for any high-res support. */ +#define HIDPP_QUIRK_HI_RES_SCROLL (HIDPP_QUIRK_HI_RES_SCROLL_1P0 | \ + HIDPP_QUIRK_HI_RES_SCROLL_X2120 | \ + HIDPP_QUIRK_HI_RES_SCROLL_X2121) #define HIDPP_QUIRK_DELAYED_INIT HIDPP_QUIRK_NO_HIDINPUT @@ -128,6 +137,25 @@ struct hidpp_battery { bool online; }; +/** + * struct hidpp_scroll_counter - Utility class for processing high-resolution + * scroll events. + * @dev: the input device for which events should be reported. + * @wheel_multiplier: the scalar multiplier to be applied to each wheel event + * @remainder: counts the number of high-resolution units moved since the last + * low-resolution event (REL_WHEEL or REL_HWHEEL) was sent. Should + * only be used by class methods. + * @direction: direction of last movement (1 or -1) + * @last_time: last event time, used to reset remainder after inactivity + */ +struct hidpp_scroll_counter { + struct input_dev *dev; + int wheel_multiplier; + int remainder; + int direction; + unsigned long long last_time; +}; + struct hidpp_device { struct hid_device *hid_dev; struct mutex send_mutex; @@ -149,6 +177,7 @@ struct hidpp_device { unsigned long capabilities; struct hidpp_battery battery; + struct hidpp_scroll_counter vertical_wheel_counter; }; /* HID++ 1.0 error codes */ @@ -391,6 +420,67 @@ static void hidpp_prefix_name(char **name, int name_length) *name = new_name; } +/** + * hidpp_scroll_counter_handle_scroll() - Send high- and low-resolution scroll + * events given a high-resolution wheel + * movement. + * @counter: a hid_scroll_counter struct describing the wheel. + * @hi_res_value: the movement of the wheel, in the mouse's high-resolution + * units. + * + * Given a high-resolution movement, this function converts the movement into + * fractions of 120 and emits high-resolution scroll events for the input + * device. It also uses the multiplier from &struct hid_scroll_counter to + * emit low-resolution scroll events when appropriate for + * backwards-compatibility with userspace input libraries. + */ +static void hidpp_scroll_counter_handle_scroll(struct hidpp_scroll_counter *counter, + int hi_res_value) +{ + int low_res_value, remainder, direction; + unsigned long long now, previous; + + hi_res_value = hi_res_value * 120/counter->wheel_multiplier; + input_report_rel(counter->dev, REL_WHEEL_HI_RES, hi_res_value); + + remainder = counter->remainder; + direction = hi_res_value > 0 ? 1 : -1; + + now = sched_clock(); + previous = counter->last_time; + counter->last_time = now; + /* + * Reset the remainder after a period of inactivity or when the + * direction changes. This prevents the REL_WHEEL emulation point + * from sliding for devices that don't always provide the same + * number of movements per detent. + */ + if (now - previous > 1000000000 || direction != counter->direction) + remainder = 0; + + counter->direction = direction; + remainder += hi_res_value; + + /* Some wheels will rest 7/8ths of a detent from the previous detent + * after slow movement, so we want the threshold for low-res events to + * be in the middle between two detents (e.g. after 4/8ths) as + * opposed to on the detents themselves (8/8ths). + */ + if (abs(remainder) >= 60) { + /* Add (or subtract) 1 because we want to trigger when the wheel + * is half-way to the next detent (i.e. scroll 1 detent after a + * 1/2 detent movement, 2 detents after a 1 1/2 detent movement, + * etc.). + */ + low_res_value = remainder / 120; + if (low_res_value == 0) + low_res_value = (hi_res_value > 0 ? 1 : -1); + input_report_rel(counter->dev, REL_WHEEL, low_res_value); + remainder -= low_res_value * 120; + } + counter->remainder = remainder; +} + /* -------------------------------------------------------------------------- */ /* HIDP++ 1.0 commands */ /* -------------------------------------------------------------------------- */ @@ -1157,6 +1247,99 @@ static int hidpp_battery_get_property(struct power_supply *psy, return ret; } +/* -------------------------------------------------------------------------- */ +/* 0x2120: Hi-resolution scrolling */ +/* -------------------------------------------------------------------------- */ + +#define HIDPP_PAGE_HI_RESOLUTION_SCROLLING 0x2120 + +#define CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE 0x10 + +static int hidpp_hrs_set_highres_scrolling_mode(struct hidpp_device *hidpp, + bool enabled, u8 *multiplier) +{ + u8 feature_index; + u8 feature_type; + int ret; + u8 params[1]; + struct hidpp_report response; + + ret = hidpp_root_get_feature(hidpp, + HIDPP_PAGE_HI_RESOLUTION_SCROLLING, + &feature_index, + &feature_type); + if (ret) + return ret; + + params[0] = enabled ? BIT(0) : 0; + ret = hidpp_send_fap_command_sync(hidpp, feature_index, + CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE, + params, sizeof(params), &response); + if (ret) + return ret; + *multiplier = response.fap.params[1]; + return 0; +} + +/* -------------------------------------------------------------------------- */ +/* 0x2121: HiRes Wheel */ +/* -------------------------------------------------------------------------- */ + +#define HIDPP_PAGE_HIRES_WHEEL 0x2121 + +#define CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY 0x00 +#define CMD_HIRES_WHEEL_SET_WHEEL_MODE 0x20 + +static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp, + u8 *multiplier) +{ + u8 feature_index; + u8 feature_type; + int ret; + struct hidpp_report response; + + ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL, + &feature_index, &feature_type); + if (ret) + goto return_default; + + ret = hidpp_send_fap_command_sync(hidpp, feature_index, + CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY, + NULL, 0, &response); + if (ret) + goto return_default; + + *multiplier = response.fap.params[0]; + return 0; +return_default: + hid_warn(hidpp->hid_dev, + "Couldn't get wheel multiplier (error %d)\n", ret); + return ret; +} + +static int hidpp_hrw_set_wheel_mode(struct hidpp_device *hidpp, bool invert, + bool high_resolution, bool use_hidpp) +{ + u8 feature_index; + u8 feature_type; + int ret; + u8 params[1]; + struct hidpp_report response; + + ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL, + &feature_index, &feature_type); + if (ret) + return ret; + + params[0] = (invert ? BIT(2) : 0) | + (high_resolution ? BIT(1) : 0) | + (use_hidpp ? BIT(0) : 0); + + return hidpp_send_fap_command_sync(hidpp, feature_index, + CMD_HIRES_WHEEL_SET_WHEEL_MODE, + params, sizeof(params), &response); +} + /* -------------------------------------------------------------------------- */ /* 0x4301: Solar Keyboard */ /* -------------------------------------------------------------------------- */ @@ -2408,10 +2591,15 @@ static int m560_raw_event(struct hid_device *hdev, u8 *data, int size) input_report_key(mydata->input, BTN_RIGHT, !!(data[1] & M560_MOUSE_BTN_RIGHT)); - if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT) + if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT) { input_report_rel(mydata->input, REL_HWHEEL, -1); - else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT) + input_report_rel(mydata->input, REL_HWHEEL_HI_RES, + -120); + } else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT) { input_report_rel(mydata->input, REL_HWHEEL, 1); + input_report_rel(mydata->input, REL_HWHEEL_HI_RES, + 120); + } v = hid_snto32(hid_field_extract(hdev, data+3, 0, 12), 12); input_report_rel(mydata->input, REL_X, v); @@ -2420,7 +2608,8 @@ static int m560_raw_event(struct hid_device *hdev, u8 *data, int size) input_report_rel(mydata->input, REL_Y, v); v = hid_snto32(data[6], 8); - input_report_rel(mydata->input, REL_WHEEL, v); + hidpp_scroll_counter_handle_scroll( + &hidpp->vertical_wheel_counter, v); input_sync(mydata->input); } @@ -2447,6 +2636,8 @@ static void m560_populate_input(struct hidpp_device *hidpp, __set_bit(REL_Y, mydata->input->relbit); __set_bit(REL_WHEEL, mydata->input->relbit); __set_bit(REL_HWHEEL, mydata->input->relbit); + __set_bit(REL_WHEEL_HI_RES, mydata->input->relbit); + __set_bit(REL_HWHEEL_HI_RES, mydata->input->relbit); } static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi, @@ -2548,6 +2739,37 @@ static int g920_get_config(struct hidpp_device *hidpp) return 0; } +/* -------------------------------------------------------------------------- */ +/* High-resolution scroll wheels */ +/* -------------------------------------------------------------------------- */ + +static int hi_res_scroll_enable(struct hidpp_device *hidpp) +{ + int ret; + u8 multiplier = 1; + + if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) { + ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false); + if (ret == 0) + ret = hidpp_hrw_get_wheel_capability(hidpp, &multiplier); + } else if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2120) { + ret = hidpp_hrs_set_highres_scrolling_mode(hidpp, true, + &multiplier); + } else /* if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) */ { + ret = hidpp10_enable_scrolling_acceleration(hidpp); + multiplier = 8; + } + if (ret) + return ret; + + if (multiplier == 0) + multiplier = 1; + + hidpp->vertical_wheel_counter.wheel_multiplier = multiplier; + hid_info(hidpp->hid_dev, "multiplier = %d\n", multiplier); + return 0; +} + /* -------------------------------------------------------------------------- */ /* Generic HID++ devices */ /* -------------------------------------------------------------------------- */ @@ -2593,6 +2815,9 @@ static void hidpp_populate_input(struct hidpp_device *hidpp, wtp_populate_input(hidpp, input, origin_is_hid_core); else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) m560_populate_input(hidpp, input, origin_is_hid_core); + + if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) + hidpp->vertical_wheel_counter.dev = input; } static int hidpp_input_configured(struct hid_device *hdev, @@ -2711,6 +2936,27 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report, return 0; } +static int hidpp_event(struct hid_device *hdev, struct hid_field *field, + struct hid_usage *usage, __s32 value) +{ + /* This function will only be called for scroll events, due to the + * restriction imposed in hidpp_usages. + */ + struct hidpp_device *hidpp = hid_get_drvdata(hdev); + struct hidpp_scroll_counter *counter = &hidpp->vertical_wheel_counter; + /* A scroll event may occur before the multiplier has been retrieved or + * the input device set, or high-res scroll enabling may fail. In such + * cases we must return early (falling back to default behaviour) to + * avoid a crash in hidpp_scroll_counter_handle_scroll. + */ + if (!(hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) || value == 0 + || counter->dev == NULL || counter->wheel_multiplier == 0) + return 0; + + hidpp_scroll_counter_handle_scroll(counter, value); + return 1; +} + static int hidpp_initialize_battery(struct hidpp_device *hidpp) { static atomic_t battery_no = ATOMIC_INIT(0); @@ -2922,6 +3168,9 @@ static void hidpp_connect_event(struct hidpp_device *hidpp) if (hidpp->battery.ps) power_supply_changed(hidpp->battery.ps); + if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) + hi_res_scroll_enable(hidpp); + if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input) /* if the input nodes are already created, we can stop now */ return; @@ -3107,6 +3356,10 @@ static void hidpp_remove(struct hid_device *hdev) mutex_destroy(&hidpp->send_mutex); } +#define LDJ_DEVICE(product) \ + HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, \ + USB_VENDOR_ID_LOGITECH, (product)) + static const struct hid_device_id hidpp_devices[] = { { /* wireless touchpad */ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, @@ -3121,10 +3374,39 @@ static const struct hid_device_id hidpp_devices[] = { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_T651), .driver_data = HIDPP_QUIRK_CLASS_WTP }, + { /* Mouse Logitech Anywhere MX */ + LDJ_DEVICE(0x1017), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, + { /* Mouse Logitech Cube */ + LDJ_DEVICE(0x4010), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2120 }, + { /* Mouse Logitech M335 */ + LDJ_DEVICE(0x4050), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { /* Mouse Logitech M515 */ + LDJ_DEVICE(0x4007), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2120 }, { /* Mouse logitech M560 */ - HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, - USB_VENDOR_ID_LOGITECH, 0x402d), - .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 }, + LDJ_DEVICE(0x402d), + .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 + | HIDPP_QUIRK_HI_RES_SCROLL_X2120 }, + { /* Mouse Logitech M705 (firmware RQM17) */ + LDJ_DEVICE(0x101b), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, + { /* Mouse Logitech M705 (firmware RQM67) */ + LDJ_DEVICE(0x406d), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { /* Mouse Logitech M720 */ + LDJ_DEVICE(0x405e), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { /* Mouse Logitech MX Anywhere 2 */ + LDJ_DEVICE(0x404a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { LDJ_DEVICE(0xb013), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { LDJ_DEVICE(0xb018), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { LDJ_DEVICE(0xb01f), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { /* Mouse Logitech MX Anywhere 2S */ + LDJ_DEVICE(0x406a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { /* Mouse Logitech MX Master */ + LDJ_DEVICE(0x4041), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { LDJ_DEVICE(0x4060), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { LDJ_DEVICE(0x4071), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { /* Mouse Logitech MX Master 2S */ + LDJ_DEVICE(0x4069), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, + { /* Mouse Logitech Performance MX */ + LDJ_DEVICE(0x101a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, { /* Keyboard logitech K400 */ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, USB_VENDOR_ID_LOGITECH, 0x4024), @@ -3144,12 +3426,19 @@ static const struct hid_device_id hidpp_devices[] = { MODULE_DEVICE_TABLE(hid, hidpp_devices); +static const struct hid_usage_id hidpp_usages[] = { + { HID_GD_WHEEL, EV_REL, REL_WHEEL_HI_RES }, + { HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1} +}; + static struct hid_driver hidpp_driver = { .name = "logitech-hidpp-device", .id_table = hidpp_devices, .probe = hidpp_probe, .remove = hidpp_remove, .raw_event = hidpp_raw_event, + .usage_table = hidpp_usages, + .event = hidpp_event, .input_configured = hidpp_input_configured, .input_mapping = hidpp_input_mapping, .input_mapped = hidpp_input_mapped,