Support for Logitech MX Anywhere 2

Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Hi,

The MX Anywhere 2 mouse has a high-resolution wheel that allows sending
wheel events together with mouse movements or in separate, via HID++.
By default, it starts in low-resolution mode, reporting events via HID.

The documentation for it was recently added[1]:
	https://lekensteyn.nl/files/logitech/x2121_hires_wheel.pdf

[1] there's a small error at the documentation, though:
    On page 5, at "[event0] wheelMovement() → resolution, periods, deltaV"
    chapter, it says that the "periods" field can be up to 15, but it shows
    only 3 bits for it table 9 "wheelMovement() report packet", placing
    the high-resolution bit as bit 3.

    After investigating it in practice, the actual bit that changes when
    the wheel is in high-resolution mode is bit 4. When bit = 1, the mouse
    is using high-resolution; when 0, it is using low resolution.

    So, in summary, the correct information there at the table should be,
    instead:
	- bits 0-3: periods
	- bit 4: resolution

There is a BZ for Solaar userspace application, opened in Aug, 2016
requesting support for it:
	https://github.com/pwr/Solaar/issues/283

With the new documentation, released yesterday, I added support for it today,
on a set of patches at:
	https://github.com/mchehab/Solaar/tree/mx_anywhere2-v3

With those patches, Solaar, in CLI, mode, shows the status of the
high-res wheel:

        11: HIRES WHEEL            {2121}   
            Multiplier: 8
            Has invert
              Normal wheel motion
            Has ratchet switch
              Free wheel mode
            Low resolution mode
            HID notification

And allows to toggle at the 3 possible switches:
	- Invert wheel movement;
	- Change to high-resolution mode - with makes the wheel to
	  scroll 8 times faster;
	- Select between HID or HID++ notifications.

It also generate an event when the ratchet button is clicked
(changing from ratchet mode to freewheel). Solaar now handle those
events using hiraw interface.

When the notification is set to HID mode, evdev properly
generates Wheel events:

1490181781.995144: event type EV_REL(0x02): REL_WHEEL (0x0002) value=-1
1490181781.995144: event type EV_SYN(0x00).
1490181782.211146: event type EV_REL(0x02): REL_WHEEL (0x0002) value=1
1490181782.211146: event type EV_SYN(0x00).

But it doesn't generate events when ratchet switch button is
pressed.

When the notification is changed to HID++ mode, it also doesn't
generate events.

I suspect that the right thing to do would be to add a handler for
it, probably at:

	drivers/hid/hid-logitech-hidpp.c

On Solaar, with my patches, we can receive such messages via
hidraw interface:

HID++ wheel movement event:

	07:37:22,846    DEBUG [ReceiverListener:hidraw1] logitech_receiver.base: (13) => r[11 03 0B00 01FFFF00000000000000000000000000]
	07:37:22,847     INFO [ReceiverListener:hidraw1] logitech_receiver.notifications: <PairedDevice(3,404A,Anywhere MX 2)>: WHEEL: res: 0 periods: 1 delta V:-1 

Ratchet event (generated on both HID and HID++ modes):

	08:26:12,455    DEBUG [ReceiverListener:hidraw2] logitech_receiver.base: (13) => r[11 03 0B10 01000000000000000000000000000000]
	08:26:12,456     INFO [ReceiverListener:hidraw2] logitech_receiver.notifications: <PairedDevice(3,404A,Anywhere MX 2)>: WHEEL: ratchet: 1

With regards to ratchet, it probably makes sense to query its state
when the driver starts, as IMHO, it should work on a way similar to
<CAPS LOCK>. Btw, are there any event already defined for ratchet mode?

As I never touched on those HID drivers, could someone either add support
for it or shed some light about what would be the proper way to add support
for it?

Thanks!
Mauro

Re: Support for Logitech MX Anywhere 2

[Benjamin Tissoires]

Hi,

On Mar 22 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> Hi,
> 
> The MX Anywhere 2 mouse has a high-resolution wheel that allows sending
> wheel events together with mouse movements or in separate, via HID++.
> By default, it starts in low-resolution mode, reporting events via HID.
> 
> The documentation for it was recently added[1]:
> 	https://lekensteyn.nl/files/logitech/x2121_hires_wheel.pdf
> 
> [1] there's a small error at the documentation, though:
>     On page 5, at "[event0] wheelMovement() → resolution, periods, deltaV"
>     chapter, it says that the "periods" field can be up to 15, but it shows
>     only 3 bits for it table 9 "wheelMovement() report packet", placing
>     the high-resolution bit as bit 3.
> 
>     After investigating it in practice, the actual bit that changes when
>     the wheel is in high-resolution mode is bit 4. When bit = 1, the mouse
>     is using high-resolution; when 0, it is using low resolution.
> 
>     So, in summary, the correct information there at the table should be,
>     instead:
> 	- bits 0-3: periods
> 	- bit 4: resolution
> 
> There is a BZ for Solaar userspace application, opened in Aug, 2016
> requesting support for it:
> 	https://github.com/pwr/Solaar/issues/283
> 
> With the new documentation, released yesterday, I added support for it today,
> on a set of patches at:
> 	https://github.com/mchehab/Solaar/tree/mx_anywhere2-v3
> 
> With those patches, Solaar, in CLI, mode, shows the status of the
> high-res wheel:
> 
>         11: HIRES WHEEL            {2121}   
>             Multiplier: 8
>             Has invert
>               Normal wheel motion
>             Has ratchet switch
>               Free wheel mode
>             Low resolution mode
>             HID notification
> 
> And allows to toggle at the 3 possible switches:
> 	- Invert wheel movement;
> 	- Change to high-resolution mode - with makes the wheel to
> 	  scroll 8 times faster;
> 	- Select between HID or HID++ notifications.
> 
> It also generate an event when the ratchet button is clicked
> (changing from ratchet mode to freewheel). Solaar now handle those
> events using hiraw interface.
> 
> When the notification is set to HID mode, evdev properly
> generates Wheel events:
> 
> 1490181781.995144: event type EV_REL(0x02): REL_WHEEL (0x0002) value=-1
> 1490181781.995144: event type EV_SYN(0x00).
> 1490181782.211146: event type EV_REL(0x02): REL_WHEEL (0x0002) value=1
> 1490181782.211146: event type EV_SYN(0x00).
> 
> But it doesn't generate events when ratchet switch button is
> pressed.
> 
> When the notification is changed to HID++ mode, it also doesn't
> generate events.
> 
> I suspect that the right thing to do would be to add a handler for
> it, probably at:
> 
> 	drivers/hid/hid-logitech-hidpp.c

Yes, that would be the place. The M560 could be used as an example as it
requires also some commands to be sent at connect and needs to treat raw
incoming events.

> 
> On Solaar, with my patches, we can receive such messages via
> hidraw interface:
> 
> HID++ wheel movement event:
> 
> 	07:37:22,846    DEBUG [ReceiverListener:hidraw1] logitech_receiver.base: (13) => r[11 03 0B00 01FFFF00000000000000000000000000]
> 	07:37:22,847     INFO [ReceiverListener:hidraw1] logitech_receiver.notifications: <PairedDevice(3,404A,Anywhere MX 2)>: WHEEL: res: 0 periods: 1 delta V:-1 
> 
> Ratchet event (generated on both HID and HID++ modes):
> 
> 	08:26:12,455    DEBUG [ReceiverListener:hidraw2] logitech_receiver.base: (13) => r[11 03 0B10 01000000000000000000000000000000]
> 	08:26:12,456     INFO [ReceiverListener:hidraw2] logitech_receiver.notifications: <PairedDevice(3,404A,Anywhere MX 2)>: WHEEL: ratchet: 1

In hid-logitech-hidpp, you can simply parse the incoming raw events and
handle the events as they should (see wtp_raw_events for handling the
HID++ specifics related to a feature).

> 
> With regards to ratchet, it probably makes sense to query its state
> when the driver starts, as IMHO, it should work on a way similar to
> <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?

There is not. And that's where the problem goes a little bit beyond just
enabling the feature, we need to forward this info to userspace.

There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
a state, and we should be able to forward this state with such a new
event.

The thing I am more worried is how can we report the high-res wheel
events. I know Peter has a DB of wheel resolution, but in that case, we
can switch to high-res or not, so a static hwdb entry won't help.

> 
> As I never touched on those HID drivers, could someone either add support
> for it or shed some light about what would be the proper way to add support
> for it?

If we can agree on a proper evdev API for this (either using ABS event
or something else), then I should be able to implement this given that
the MX Master also exports the same feature. But really, the code should
not be that much of an issue given that everything is already in place
in hid-logitech-hidpp.c.

Cheers,
Benjamin

> 
> Thanks!
> Mauro

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Thu, 23 Mar 2017 11:59:56 +0100
Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:

> Hi,
> 
> On Mar 22 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > Hi,
> > 
> > The MX Anywhere 2 mouse has a high-resolution wheel that allows sending
> > wheel events together with mouse movements or in separate, via HID++.
> > By default, it starts in low-resolution mode, reporting events via HID.
> > 
> > The documentation for it was recently added[1]:
> > 	https://lekensteyn.nl/files/logitech/x2121_hires_wheel.pdf
> > 
> > [1] there's a small error at the documentation, though:
> >     On page 5, at "[event0] wheelMovement() → resolution, periods, deltaV"
> >     chapter, it says that the "periods" field can be up to 15, but it shows
> >     only 3 bits for it table 9 "wheelMovement() report packet", placing
> >     the high-resolution bit as bit 3.
> > 
> >     After investigating it in practice, the actual bit that changes when
> >     the wheel is in high-resolution mode is bit 4. When bit = 1, the mouse
> >     is using high-resolution; when 0, it is using low resolution.
> > 
> >     So, in summary, the correct information there at the table should be,
> >     instead:
> > 	- bits 0-3: periods
> > 	- bit 4: resolution

Btw, the documentation there at:
	https://lekensteyn.nl/files/logitech/x2121_hires_wheel.pdf

got updated to fix the above errata.

> > 
> > There is a BZ for Solaar userspace application, opened in Aug, 2016
> > requesting support for it:
> > 	https://github.com/pwr/Solaar/issues/283
> > 
> > With the new documentation, released yesterday, I added support for it today,
> > on a set of patches at:
> > 	https://github.com/mchehab/Solaar/tree/mx_anywhere2-v3
> > 
> > With those patches, Solaar, in CLI, mode, shows the status of the
> > high-res wheel:
> > 
> >         11: HIRES WHEEL            {2121}   
> >             Multiplier: 8
> >             Has invert
> >               Normal wheel motion
> >             Has ratchet switch
> >               Free wheel mode
> >             Low resolution mode
> >             HID notification
> > 
> > And allows to toggle at the 3 possible switches:
> > 	- Invert wheel movement;
> > 	- Change to high-resolution mode - with makes the wheel to
> > 	  scroll 8 times faster;
> > 	- Select between HID or HID++ notifications.
> > 
> > It also generate an event when the ratchet button is clicked
> > (changing from ratchet mode to freewheel). Solaar now handle those
> > events using hiraw interface.
> > 
> > When the notification is set to HID mode, evdev properly
> > generates Wheel events:
> > 
> > 1490181781.995144: event type EV_REL(0x02): REL_WHEEL (0x0002) value=-1
> > 1490181781.995144: event type EV_SYN(0x00).
> > 1490181782.211146: event type EV_REL(0x02): REL_WHEEL (0x0002) value=1
> > 1490181782.211146: event type EV_SYN(0x00).
> > 
> > But it doesn't generate events when ratchet switch button is
> > pressed.
> > 
> > When the notification is changed to HID++ mode, it also doesn't
> > generate events.
> > 
> > I suspect that the right thing to do would be to add a handler for
> > it, probably at:
> > 
> > 	drivers/hid/hid-logitech-hidpp.c  
> 
> Yes, that would be the place. The M560 could be used as an example as it
> requires also some commands to be sent at connect and needs to treat raw
> incoming events.

Ok.

> > 
> > On Solaar, with my patches, we can receive such messages via
> > hidraw interface:
> > 
> > HID++ wheel movement event:
> > 
> > 	07:37:22,846    DEBUG [ReceiverListener:hidraw1] logitech_receiver.base: (13) => r[11 03 0B00 01FFFF00000000000000000000000000]
> > 	07:37:22,847     INFO [ReceiverListener:hidraw1] logitech_receiver.notifications: <PairedDevice(3,404A,Anywhere MX 2)>: WHEEL: res: 0 periods: 1 delta V:-1 
> > 
> > Ratchet event (generated on both HID and HID++ modes):
> > 
> > 	08:26:12,455    DEBUG [ReceiverListener:hidraw2] logitech_receiver.base: (13) => r[11 03 0B10 01000000000000000000000000000000]
> > 	08:26:12,456     INFO [ReceiverListener:hidraw2] logitech_receiver.notifications: <PairedDevice(3,404A,Anywhere MX 2)>: WHEEL: ratchet: 1  
> 
> In hid-logitech-hidpp, you can simply parse the incoming raw events and
> handle the events as they should (see wtp_raw_events for handling the
> HID++ specifics related to a feature).

Ok. Seems simple enough. The events start with REPORT_ID_HIDPP_LONG (0x11):

That's what I captured from usbmon driver, on high-res mode:

000025675 ms 000042 ms (241974 us EP=83, Dev=68) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
000025917 ms 000242 ms (215972 us EP=83, Dev=68) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000026133 ms 000216 ms (281975 us EP=83, Dev=68) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00

and this is on Low-res mode:

000030905 ms 000000 ms (017904 us EP=83, Dev=68) >>> 11 03 0b 2d 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000030923 ms 000018 ms (596012 us EP=83, Dev=68) >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000031519 ms 000596 ms (512004 us EP=83, Dev=68) >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000032031 ms 000512 ms (211972 us EP=83, Dev=68) >>> 11 03 0b 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
000032243 ms 000212 ms (261975 us EP=83, Dev=68) >>> 11 03 0b 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00

If I understood it right, it will need to check if it is a MX2, for example
by adding a quirk for it, and if feature_index is 0x0b, parse the
events at wtp_raw_event(), something like:

...
	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_MX2)
		return mx2_raw_event(hdev, data, size);
...

Sounds easy enough.

> > With regards to ratchet, it probably makes sense to query its state
> > when the driver starts, as IMHO, it should work on a way similar to
> > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?  
> 
> There is not. And that's where the problem goes a little bit beyond just
> enabling the feature, we need to forward this info to userspace.
> 
> There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> a state, and we should be able to forward this state with such a new
> event.
> 
> The thing I am more worried is how can we report the high-res wheel
> events. I know Peter has a DB of wheel resolution, but in that case, we
> can switch to high-res or not, so a static hwdb entry won't help.

Yes. In the case of high-res, there are actually two ways for those
events to arrive:

In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
isn't any events reporting if the mode changed, nor the event with
gets wheel axes movement tell if the mouse is in low or high res.

So, in HID mode, identifying if the wheel is in low or high res
is not direct.

When the device is in HID+ mode, the resolution mode comes together with
axes changes. So, it should be possible to define a different event
for high-res wheel.

E. g. if the event reports high-res, it would be generating:
EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
it will keep generating EV_REL / REL_WHEEL.

This way, Peter's code (libinput, I guess?) could handle it without
requiring any DB for the devices that allow switching the mode.

Another alternative would be to have a EV_SWITCH SW_HIRES event that
would signal if the driver detects that the resolution switched.
IMHO, this would be more generic, but would work on HID mode only
if the driver would have some sort of check if the user commanded a
resolution change, or do periodic polls.

> > 
> > As I never touched on those HID drivers, could someone either add support
> > for it or shed some light about what would be the proper way to add support
> > for it?  
> 
> If we can agree on a proper evdev API for this (either using ABS event
> or something else),

Using ABS event could work, but seems odd ;)

> then I should be able to implement this given that
> the MX Master also exports the same feature.

Ah! good to know!

> But really, the code should
> not be that much of an issue given that everything is already in place
> in hid-logitech-hidpp.c.

Yeah, it seems that the changes aren't hard.

Thanks,
Mauro

Re: Support for Logitech MX Anywhere 2

[Peter Hutterer]

On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 23 Mar 2017 11:59:56 +0100
> Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:
> > > With regards to ratchet, it probably makes sense to query its state
> > > when the driver starts, as IMHO, it should work on a way similar to
> > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?  
> > 
> > There is not. And that's where the problem goes a little bit beyond just
> > enabling the feature, we need to forward this info to userspace.
> > 
> > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > a state, and we should be able to forward this state with such a new
> > event.
> > 
> > The thing I am more worried is how can we report the high-res wheel
> > events. I know Peter has a DB of wheel resolution, but in that case, we
> > can switch to high-res or not, so a static hwdb entry won't help.
> 
> Yes. In the case of high-res, there are actually two ways for those
> events to arrive:
> 
> In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> isn't any events reporting if the mode changed, nor the event with
> gets wheel axes movement tell if the mouse is in low or high res.
> 
> So, in HID mode, identifying if the wheel is in low or high res
> is not direct.
> 
> When the device is in HID+ mode, the resolution mode comes together with
> axes changes. So, it should be possible to define a different event
> for high-res wheel.
> 
> E. g. if the event reports high-res, it would be generating:
> EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> it will keep generating EV_REL / REL_WHEEL.
> 
> This way, Peter's code (libinput, I guess?) could handle it without
> requiring any DB for the devices that allow switching the mode.

sort-of. The main problem with relative axes is that we don't have a
resolution info. The reason we have a hwdb for wheels is that libinput
converts kernel data to physical dimensions so that callers can use the data
in a reliable manner. Switching to a hi-res-wheel just moves the problem
around a bit.

Using ABS events simply gives us the resolution in the inital description.
That's (I suspect) the only reason Benjamin suggested it. This isn't the
first time it has come up, it would be interesting to add something like
EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
resolution. But I also suspect that all but this use-case would have the
kernel return a digital shrug anyway, so I'm not sure it's worth the effort.

Cheers,
   Peter

> Another alternative would be to have a EV_SWITCH SW_HIRES event that
> would signal if the driver detects that the resolution switched.
> IMHO, this would be more generic, but would work on HID mode only
> if the driver would have some sort of check if the user commanded a
> resolution change, or do periodic polls.
> 
> > > 
> > > As I never touched on those HID drivers, could someone either add support
> > > for it or shed some light about what would be the proper way to add support
> > > for it?  
> > 
> > If we can agree on a proper evdev API for this (either using ABS event
> > or something else),
> 
> Using ABS event could work, but seems odd ;)
> 
> > then I should be able to implement this given that
> > the MX Master also exports the same feature.
> 
> Ah! good to know!
> 
> > But really, the code should
> > not be that much of an issue given that everything is already in place
> > in hid-logitech-hidpp.c.
> 
> Yeah, it seems that the changes aren't hard.
> 
> Thanks,
> Mauro
> 

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Fri, 24 Mar 2017 15:22:20 +1000
Peter Hutterer <peter.hutterer@who-t.net> escreveu:

> On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:
> > Em Thu, 23 Mar 2017 11:59:56 +0100
> > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:  
> > > > With regards to ratchet, it probably makes sense to query its state
> > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?    
> > > 
> > > There is not. And that's where the problem goes a little bit beyond just
> > > enabling the feature, we need to forward this info to userspace.
> > > 
> > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > a state, and we should be able to forward this state with such a new
> > > event.
> > > 
> > > The thing I am more worried is how can we report the high-res wheel
> > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > can switch to high-res or not, so a static hwdb entry won't help.  
> > 
> > Yes. In the case of high-res, there are actually two ways for those
> > events to arrive:
> > 
> > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > isn't any events reporting if the mode changed, nor the event with
> > gets wheel axes movement tell if the mouse is in low or high res.
> > 
> > So, in HID mode, identifying if the wheel is in low or high res
> > is not direct.
> > 
> > When the device is in HID+ mode, the resolution mode comes together with
> > axes changes. So, it should be possible to define a different event
> > for high-res wheel.
> > 
> > E. g. if the event reports high-res, it would be generating:
> > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > it will keep generating EV_REL / REL_WHEEL.
> > 
> > This way, Peter's code (libinput, I guess?) could handle it without
> > requiring any DB for the devices that allow switching the mode.  
> 
> sort-of. The main problem with relative axes is that we don't have a
> resolution info. The reason we have a hwdb for wheels is that libinput
> converts kernel data to physical dimensions so that callers can use the data
> in a reliable manner. Switching to a hi-res-wheel just moves the problem
> around a bit.
> 
> Using ABS events simply gives us the resolution in the inital description.
> That's (I suspect) the only reason Benjamin suggested it. This isn't the
> first time it has come up, it would be interesting to add something like
> EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> resolution. But I also suspect that all but this use-case would have the
> kernel return a digital shrug anyway, so I'm not sure it's worth the effort.

I see. Well, at least in the case of the feature supported by this
mouse, there are just two possible resolutions: low-res and high-res.
The high-res resolution is fixed[1].

As the multiplier has a fixed value per device, a hwdb could still
work, provided that high-res wheel events would produce a different
event code than low-res.

[1] there's a USB message that can be used to query the multiplier,
with is always equal to 8 for MX Anywhere 2. No idea if other
devices with this feature use the same multiplier.

Regards,
Mauro

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Fri, 24 Mar 2017 06:57:00 -0300
Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:

> Em Fri, 24 Mar 2017 15:22:20 +1000
> Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> 
> > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:  
> > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:    
> > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?      
> > > > 
> > > > There is not. And that's where the problem goes a little bit beyond just
> > > > enabling the feature, we need to forward this info to userspace.
> > > > 
> > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > a state, and we should be able to forward this state with such a new
> > > > event.
> > > > 
> > > > The thing I am more worried is how can we report the high-res wheel
> > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > can switch to high-res or not, so a static hwdb entry won't help.    
> > > 
> > > Yes. In the case of high-res, there are actually two ways for those
> > > events to arrive:
> > > 
> > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > isn't any events reporting if the mode changed, nor the event with
> > > gets wheel axes movement tell if the mouse is in low or high res.
> > > 
> > > So, in HID mode, identifying if the wheel is in low or high res
> > > is not direct.
> > > 
> > > When the device is in HID+ mode, the resolution mode comes together with
> > > axes changes. So, it should be possible to define a different event
> > > for high-res wheel.
> > > 
> > > E. g. if the event reports high-res, it would be generating:
> > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > it will keep generating EV_REL / REL_WHEEL.
> > > 
> > > This way, Peter's code (libinput, I guess?) could handle it without
> > > requiring any DB for the devices that allow switching the mode.    
> > 
> > sort-of. The main problem with relative axes is that we don't have a
> > resolution info. The reason we have a hwdb for wheels is that libinput
> > converts kernel data to physical dimensions so that callers can use the data
> > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > around a bit.
> > 
> > Using ABS events simply gives us the resolution in the inital description.
> > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > first time it has come up, it would be interesting to add something like
> > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > resolution. But I also suspect that all but this use-case would have the
> > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.  
> 
> I see. Well, at least in the case of the feature supported by this
> mouse, there are just two possible resolutions: low-res and high-res.
> The high-res resolution is fixed[1].
> 
> As the multiplier has a fixed value per device, a hwdb could still
> work, provided that high-res wheel events would produce a different
> event code than low-res.
> 
> [1] there's a USB message that can be used to query the multiplier,
> with is always equal to 8 for MX Anywhere 2. No idea if other
> devices with this feature use the same multiplier.

What I'm proposing is basically something like what's in the patch
below (for now, just compile-tested).

So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
switch to the HID++ report mode at device connect and handle the Wheel
events. If the wheel event is low res, will generate
REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.

This way, at libinput, from a logitech mouse with either 0x4041 or 0x404a
IDs, if it receives a REL_HWHEEL event, it will consider a high-resolution
event, with has 8 times the resolution of a low-res one.

See the enclosed (for now, compile-tested only) patch.

Thanks,
Mauro


[PATCH RFC] hid-logitech-hidpp: add support for high res wheel
    
Some Logitech mouses (MX Anyware 2 and MX Master) have support
for a high-resolution wheel.

This wheel can work in backward-compatible mode, generating
wheel events via HID normal events, or it can use new
HID++ events that report not only the wheel movement, but also
the resolution.

Add support for it.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 2e2515a4c070..d4430110ee06 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -62,6 +62,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
 #define HIDPP_QUIRK_NO_HIDINPUT			BIT(23)
 #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS	BIT(24)
+#define HIDPP_QUIRK_HIRES_SCROLL		BIT(25)
 
 #define HIDPP_QUIRK_DELAYED_INIT		(HIDPP_QUIRK_NO_HIDINPUT | \
 						 HIDPP_QUIRK_CONNECT_EVENTS)
@@ -1361,6 +1362,65 @@ static int hidpp_ff_deinit(struct hid_device *hid)
 	return 0;
 }
 
+/* -------------------------------------------------------------------------- */
+/* 0x2121: High Resolution Wheel                                              */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_HIGH_RES_WHEEL		0x2121
+
+#define CMD_MOUSE_SET_WHEEL_MODE	0x20
+
+struct high_res_wheel_data {
+	u8 feature_index;
+	struct input_dev *input;
+};
+
+/**
+ * hidpp_mouse_set_wheel_mode - Sets high resolution wheel mode
+ *
+ * @invert:	if true, inverts wheel movement
+ * @high_res:	if true, wheel is in high-resolution mode. Otherwise, low res
+ * @hidpp:	if true, report wheel events via HID++ notification. If false,
+ *		use standard HID events
+ */
+static int hidpp_mouse_set_wheel_mode(struct hidpp_device *hidpp,
+				      bool invert,
+				      bool high_res,
+				      bool hidpp_mode)
+{
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+	u8 feature_type;
+	struct hidpp_report response;
+	int ret;
+	u8 params[16] = { 0 };
+
+	params[0] = invert     ? 0x4 : 0  |
+		    high_res   ? 0x2 : 0  |
+		    hidpp_mode ? 0x1 : 0;
+
+	if (!hrd->feature_index) {
+		ret = hidpp_root_get_feature(hidpp,
+					    HIDPP_HIGH_RES_WHEEL,
+					    &hrd->feature_index,
+					    &feature_type);
+		if (ret)
+			/* means that the device is not powered up */
+			return ret;
+	}
+
+	ret = hidpp_send_fap_command_sync(hidpp, hrd->feature_index,
+					  CMD_MOUSE_SET_WHEEL_MODE,
+					  params, 16, &response);
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
+	if (ret)
+		return ret;
+
+	return 0;
+}
 
 /* ************************************************************************** */
 /*                                                                            */
@@ -1816,6 +1876,121 @@ static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 }
 
 /* ------------------------------------------------------------------------- */
+/* Logitech mouse devices with high resolution wheel                         */
+/* ------------------------------------------------------------------------- */
+
+static int high_res_raw_event(struct hid_device *hdev, u8 *data, int size)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+
+	/* sanity check */
+	if (!hrd) {
+		hid_err(hdev, "error in parameter\n");
+		return -EINVAL;
+	}
+
+	if (size < 19) {
+		hid_err(hdev, "error in report\n");
+		return 0;
+	}
+
+	if (data[0] != REPORT_ID_HIDPP_LONG ||
+	    data[2] != hrd->feature_index)
+		return 1;
+
+	/*
+	 * high res wheel mouse events
+	 *
+	 * Wheel movement events are like:
+	 *
+	 * 11 03 0b 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
+	 *
+	 * data[0] = 0x11
+	 * data[1] = device-id
+	 * data[2] = feature index (0b)
+	 * data[3] = event type: 0x00 - wheel movement
+	 * data[4] = bitmask:
+	 *		bits 0-3: number of sampling periods combined
+	 *		bit 4:
+	 *			0 = low resolution
+	 *			1 = high resolution
+	 * data[5] - deltaV MSB
+	 * data[6] = deltaV LSB
+	 * Remaining payload is reserved
+	 *
+	 * Ratchet events are like:
+	 * 11 03 0b 10 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+	 *
+	 * data[0] = 0x11
+	 * data[1] = device-id
+	 * data[2] = feature index
+	 * data[3] = event type: 0x10 - ratchet state
+	 * data[4] = bit 0:
+	 *		1 = ratchet
+	 *		0 = free wheel
+	 * Remaining payload is reserved
+	 */
+
+	if (data[3] == 0) {
+		s16 delta = data[6] | data[5] << 8;
+		bool res = data[4] & 0x10;
+
+		/*
+		 * Report high-resolution events as REL_HWHEEL and
+		 * low-resolution events as REL_WHEEL.
+		 */
+
+		if (res)
+			input_report_rel(hrd->input, REL_HWHEEL, delta);
+		else
+			input_report_rel(hrd->input, REL_WHEEL, delta);
+	}
+
+	/* FIXME: also report ratchet events to userspace */
+
+	return 1;
+}
+
+static void high_res_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input_dev, bool origin_is_hid_core)
+{
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+
+	hrd->input = input_dev;
+
+	__set_bit(REL_WHEEL, hrd->input->relbit);
+	__set_bit(REL_HWHEEL, hrd->input->relbit);
+}
+
+
+static int high_res_allocate(struct hid_device *hdev)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct high_res_wheel_data *hrd;
+
+	hrd = devm_kzalloc(&hdev->dev, sizeof(struct high_res_wheel_data),
+			GFP_KERNEL);
+	if (!hrd)
+		return -ENOMEM;
+
+	hidpp->private_data = hrd;
+
+	return 0;
+};
+
+static int high_res_connect(struct hid_device *hdev, bool connected)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+
+	if (!connected)
+		return 0;
+
+	/* Enable HID++ wheel event output mode */
+	return hidpp_mouse_set_wheel_mode(hidpp, false, false, true);
+}
+
+/* ------------------------------------------------------------------------- */
 /* Logitech K400 devices                                                     */
 /* ------------------------------------------------------------------------- */
 
@@ -1955,6 +2130,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);
+	else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL)
+		high_res_populate_input(hidpp, input, origin_is_hid_core);
+
 }
 
 static int hidpp_input_configured(struct hid_device *hdev,
@@ -2054,6 +2232,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 		return wtp_raw_event(hdev, data, size);
 	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
 		return m560_raw_event(hdev, data, size);
+	else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL)
+		return high_res_raw_event(hdev, data, size);
 
 	return 0;
 }
@@ -2141,6 +2321,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 		ret = k400_connect(hdev, connected);
 		if (ret)
 			return;
+	} else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL) {
+		ret = high_res_connect(hdev, connected);
+		if (ret)
+			return;
 	}
 
 	if (!connected || hidpp->delayed_input)
@@ -2229,6 +2413,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		ret = k400_allocate(hdev);
 		if (ret)
 			goto allocate_fail;
+	} else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL) {
+		ret = high_res_allocate(hdev);
+		if (ret)
+			goto allocate_fail;
 	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
@@ -2354,6 +2542,14 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x402d),
 	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
+	{ /* Logitech MX Master with high resolution scroll */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x4041),
+	  .driver_data = HIDPP_QUIRK_HIRES_SCROLL },
+	{ /* Logitech MX Anywhere 2r with high resolution scroll */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x404a),
+	  .driver_data = HIDPP_QUIRK_HIRES_SCROLL },
 	{ /* Keyboard logitech K400 */
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x4024),

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Sat, 25 Mar 2017 09:36:18 -0300
Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:

> Em Fri, 24 Mar 2017 06:57:00 -0300
> Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> 
> > Em Fri, 24 Mar 2017 15:22:20 +1000
> > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> >   
> > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:    
> > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:      
> > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?        
> > > > > 
> > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > enabling the feature, we need to forward this info to userspace.
> > > > > 
> > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > a state, and we should be able to forward this state with such a new
> > > > > event.
> > > > > 
> > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > can switch to high-res or not, so a static hwdb entry won't help.      
> > > > 
> > > > Yes. In the case of high-res, there are actually two ways for those
> > > > events to arrive:
> > > > 
> > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > isn't any events reporting if the mode changed, nor the event with
> > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > 
> > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > is not direct.
> > > > 
> > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > axes changes. So, it should be possible to define a different event
> > > > for high-res wheel.
> > > > 
> > > > E. g. if the event reports high-res, it would be generating:
> > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > it will keep generating EV_REL / REL_WHEEL.
> > > > 
> > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > requiring any DB for the devices that allow switching the mode.      
> > > 
> > > sort-of. The main problem with relative axes is that we don't have a
> > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > converts kernel data to physical dimensions so that callers can use the data
> > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > around a bit.
> > > 
> > > Using ABS events simply gives us the resolution in the inital description.
> > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > first time it has come up, it would be interesting to add something like
> > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > resolution. But I also suspect that all but this use-case would have the
> > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.    
> > 
> > I see. Well, at least in the case of the feature supported by this
> > mouse, there are just two possible resolutions: low-res and high-res.
> > The high-res resolution is fixed[1].
> > 
> > As the multiplier has a fixed value per device, a hwdb could still
> > work, provided that high-res wheel events would produce a different
> > event code than low-res.
> > 
> > [1] there's a USB message that can be used to query the multiplier,
> > with is always equal to 8 for MX Anywhere 2. No idea if other
> > devices with this feature use the same multiplier.  
> 
> What I'm proposing is basically something like what's in the patch
> below (for now, just compile-tested).
> 
> So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
> switch to the HID++ report mode at device connect and handle the Wheel
> events. If the wheel event is low res, will generate
> REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.

Hmm... "H" in HWHEEL is not for "hardware" or "high-res", but,
instead, for "horizontal".

So, if we'd go for the proposal of using a different event for high-res
vertical wheel, instead of adding a new ioctl (EVIOCGREL as equivalent to
EVIOCGABS), we'll need an extra event.

Anyway, the patch below works fine with my mouse. It is against
Kernel 4.10.4.

I didn't implement yet support for the ratchet switch. Will add it on
a separate patch.

Thanks,
Mauro

[PATCH] hid-logitech-hidpp: add support for high res wheel

Some Logitech mouses (MX Anyware 2 and MX Master) have support
for a high-resolution wheel.

This wheel can work in backward-compatible mode, generating
wheel events via HID normal events, or it can use new
HID++ events that report not only the wheel movement, but also
the resolution.

Add support for it.

Tested with a MX Anywere 2 mouse:

     Codename     : Anywhere MX 2
     Kind         : mouse
     Wireless PID : 404A
     Protocol     : HID++ 4.5
     Polling rate : 8 ms (125Hz)
        Bootloader: BOT 23.00.B0007
          Firmware: MPM 02.00.B0007
          Firmware: MPM 02.00.B0007

Tested with both low-res and high-res wheel events, using ir-keytable,
from v4l-utils (upstream).

In Low res mode:

# ir-keytable -t -d /dev/input/by-id/usb-Logitech_USB_Receiver-if02-event-mouse|grep REL_WHEEL

1490457275.635450: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
1490457275.653380: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-2
1490457275.667460: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-3
1490457275.683345: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-3
1490457275.693389: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-4
1490457275.701382: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-2

In High res mode:

# ir-keytable -t -d /dev/input/by-id/usb-Logitech_USB_Receiver-if02-event-mouse|grep 0x000a

1490457340.494215: event type EV_REL(0x02):  (0x000a) value=2
1490457340.502223: event type EV_REL(0x02):  (0x000a) value=1
1490457340.510215: event type EV_REL(0x02):  (0x000a) value=1
1490457340.518221: event type EV_REL(0x02):  (0x000a) value=1
1490457340.580217: event type EV_REL(0x02):  (0x000a) value=-2
1490457340.588219: event type EV_REL(0x02):  (0x000a) value=-2
1490457340.596216: event type EV_REL(0x02):  (0x000a) value=-6
1490457340.604204: event type EV_REL(0x02):  (0x000a) value=-6
1490457340.612216: event type EV_REL(0x02):  (0x000a) value=-7
1490457340.620217: event type EV_REL(0x02):  (0x000a) value=-7
1490457340.628216: event type EV_REL(0x02):  (0x000a) value=-6

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 2e2515a4c070..5468a8f81ef6 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -62,6 +62,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
 #define HIDPP_QUIRK_NO_HIDINPUT			BIT(23)
 #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS	BIT(24)
+#define HIDPP_QUIRK_HIRES_SCROLL		BIT(25)
 
 #define HIDPP_QUIRK_DELAYED_INIT		(HIDPP_QUIRK_NO_HIDINPUT | \
 						 HIDPP_QUIRK_CONNECT_EVENTS)
@@ -1361,6 +1362,65 @@ static int hidpp_ff_deinit(struct hid_device *hid)
 	return 0;
 }
 
+/* -------------------------------------------------------------------------- */
+/* 0x2121: High Resolution Wheel                                              */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_HIGH_RES_WHEEL		0x2121
+
+#define CMD_MOUSE_SET_WHEEL_MODE	0x20
+
+struct high_res_wheel_data {
+	u8 feature_index;
+	struct input_dev *input;
+};
+
+/**
+ * hidpp_mouse_set_wheel_mode - Sets high resolution wheel mode
+ *
+ * @invert:	if true, inverts wheel movement
+ * @high_res:	if true, wheel is in high-resolution mode. Otherwise, low res
+ * @hidpp:	if true, report wheel events via HID++ notification. If false,
+ *		use standard HID events
+ */
+static int hidpp_mouse_set_wheel_mode(struct hidpp_device *hidpp,
+				      bool invert,
+				      bool high_res,
+				      bool hidpp_mode)
+{
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+	u8 feature_type;
+	struct hidpp_report response;
+	int ret;
+	u8 params[16] = { 0 };
+
+	params[0] = invert     ? 0x4 : 0  |
+		    high_res   ? 0x2 : 0  |
+		    hidpp_mode ? 0x1 : 0;
+
+	if (!hrd->feature_index) {
+		ret = hidpp_root_get_feature(hidpp,
+					    HIDPP_HIGH_RES_WHEEL,
+					    &hrd->feature_index,
+					    &feature_type);
+		if (ret)
+			/* means that the device is not powered up */
+			return ret;
+	}
+
+	ret = hidpp_send_fap_command_sync(hidpp, hrd->feature_index,
+					  CMD_MOUSE_SET_WHEEL_MODE,
+					  params, 16, &response);
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
+	if (ret)
+		return ret;
+
+	return 0;
+}
 
 /* ************************************************************************** */
 /*                                                                            */
@@ -1816,6 +1876,121 @@ static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 }
 
 /* ------------------------------------------------------------------------- */
+/* Logitech mouse devices with high resolution wheel                         */
+/* ------------------------------------------------------------------------- */
+
+static int high_res_raw_event(struct hid_device *hdev, u8 *data, int size)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+
+	/* sanity check */
+	if (!hrd || !hrd->feature_index) {
+		hid_err(hdev, "error in parameter\n");
+		return -EINVAL;
+	}
+
+	if (data[0] != REPORT_ID_HIDPP_LONG ||
+	    data[2] != hrd->feature_index)
+		return 1;
+
+	if (size < 8) {
+		hid_err(hdev, "error in report: size = %d: %*ph\n", size,
+			size, data);
+		return 0;
+	}
+
+	/*
+	 * high res wheel mouse events
+	 *
+	 * Wheel movement events are like:
+	 *
+	 * 11 03 0b 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
+	 *
+	 * data[0] = 0x11
+	 * data[1] = device-id
+	 * data[2] = feature index (0b)
+	 * data[3] = event type: 0x00 - wheel movement
+	 * data[4] = bitmask:
+	 *		bits 0-3: number of sampling periods combined
+	 *		bit 4:
+	 *			0 = low resolution
+	 *			1 = high resolution
+	 * data[5] - deltaV MSB
+	 * data[6] = deltaV LSB
+	 * Remaining payload is reserved
+	 *
+	 * Ratchet events are like:
+	 * 11 03 0b 10 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+	 *
+	 * data[0] = 0x11
+	 * data[1] = device-id
+	 * data[2] = feature index
+	 * data[3] = event type: 0x10 - ratchet state
+	 * data[4] = bit 0:
+	 *		1 = ratchet
+	 *		0 = free wheel
+	 * Remaining payload is reserved
+	 */
+
+	if (data[3] == 0) {
+		s16 delta = data[6] | data[5] << 8;
+		bool res = data[4] & 0x10;
+
+		/*
+		 * Report high-resolution events as REL_HWHEEL and
+		 * low-resolution events as REL_WHEEL.
+		 */
+		if (res)
+			input_report_rel(hrd->input, REL_HIRES_WHEEL, delta);
+		else
+			input_report_rel(hrd->input, REL_WHEEL, delta);
+	}
+
+	/* FIXME: also report ratchet events to userspace */
+
+	return 1;
+}
+
+static void high_res_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input_dev, bool origin_is_hid_core)
+{
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+
+	hrd->input = input_dev;
+
+	__set_bit(REL_WHEEL, hrd->input->relbit);
+	__set_bit(REL_HIRES_WHEEL, hrd->input->relbit);
+}
+
+
+static int high_res_allocate(struct hid_device *hdev)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct high_res_wheel_data *hrd;
+
+	hrd = devm_kzalloc(&hdev->dev, sizeof(struct high_res_wheel_data),
+			GFP_KERNEL);
+	if (!hrd)
+		return -ENOMEM;
+
+	hidpp->private_data = hrd;
+
+	return 0;
+};
+
+static int high_res_connect(struct hid_device *hdev, bool connected)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+
+	if (!connected)
+		return 0;
+
+	/* Enable HID++ wheel event output mode */
+	return hidpp_mouse_set_wheel_mode(hidpp, false, false, true);
+}
+
+/* ------------------------------------------------------------------------- */
 /* Logitech K400 devices                                                     */
 /* ------------------------------------------------------------------------- */
 
@@ -1955,6 +2130,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);
+	else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL)
+		high_res_populate_input(hidpp, input, origin_is_hid_core);
+
 }
 
 static int hidpp_input_configured(struct hid_device *hdev,
@@ -2054,6 +2232,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 		return wtp_raw_event(hdev, data, size);
 	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
 		return m560_raw_event(hdev, data, size);
+	else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL)
+		return high_res_raw_event(hdev, data, size);
 
 	return 0;
 }
@@ -2141,6 +2321,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 		ret = k400_connect(hdev, connected);
 		if (ret)
 			return;
+	} else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL) {
+		ret = high_res_connect(hdev, connected);
+		if (ret)
+			return;
 	}
 
 	if (!connected || hidpp->delayed_input)
@@ -2229,6 +2413,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		ret = k400_allocate(hdev);
 		if (ret)
 			goto allocate_fail;
+	} else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL) {
+		ret = high_res_allocate(hdev);
+		if (ret)
+			goto allocate_fail;
 	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
@@ -2354,6 +2542,14 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x402d),
 	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
+	{ /* Logitech MX Master with high resolution scroll */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x4041),
+	  .driver_data = HIDPP_QUIRK_CONNECT_EVENTS | HIDPP_QUIRK_HIRES_SCROLL },
+	{ /* Logitech MX Anywhere 2r with high resolution scroll */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x404a),
+	  .driver_data = HIDPP_QUIRK_CONNECT_EVENTS | HIDPP_QUIRK_HIRES_SCROLL },
 	{ /* Keyboard logitech K400 */
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x4024),
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 3af60ee69053..23b2d377af59 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -703,6 +703,7 @@
 #define REL_DIAL		0x07
 #define REL_WHEEL		0x08
 #define REL_MISC		0x09
+#define REL_HIRES_WHEEL		0x0a
 #define REL_MAX			0x0f
 #define REL_CNT			(REL_MAX+1)
 

Re: Support for Logitech MX Anywhere 2

[Peter Hutterer]

On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:
> Em Sat, 25 Mar 2017 09:36:18 -0300
> Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> 
> > Em Fri, 24 Mar 2017 06:57:00 -0300
> > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > 
> > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > >   
> > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:    
> > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:      
> > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?        
> > > > > > 
> > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > 
> > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > a state, and we should be able to forward this state with such a new
> > > > > > event.
> > > > > > 
> > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > can switch to high-res or not, so a static hwdb entry won't help.      
> > > > > 
> > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > events to arrive:
> > > > > 
> > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > 
> > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > is not direct.
> > > > > 
> > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > axes changes. So, it should be possible to define a different event
> > > > > for high-res wheel.
> > > > > 
> > > > > E. g. if the event reports high-res, it would be generating:
> > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > 
> > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > requiring any DB for the devices that allow switching the mode.      
> > > > 
> > > > sort-of. The main problem with relative axes is that we don't have a
> > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > converts kernel data to physical dimensions so that callers can use the data
> > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > around a bit.
> > > > 
> > > > Using ABS events simply gives us the resolution in the inital description.
> > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > first time it has come up, it would be interesting to add something like
> > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > resolution. But I also suspect that all but this use-case would have the
> > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.    
> > > 
> > > I see. Well, at least in the case of the feature supported by this
> > > mouse, there are just two possible resolutions: low-res and high-res.
> > > The high-res resolution is fixed[1].
> > > 
> > > As the multiplier has a fixed value per device, a hwdb could still
> > > work, provided that high-res wheel events would produce a different
> > > event code than low-res.
> > > 
> > > [1] there's a USB message that can be used to query the multiplier,
> > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > devices with this feature use the same multiplier.  

let's not assume that and plan ahead, because sooner or later this will be
configurable in some device. Probably before we get the first kernel out
with this patchset in. :)

a hwdb could still work in that case, but it gets quite tricky when it
becomes user-configurable. Now you need the user (or supporting software) to
drop hwdb in entries which is less than ideal. If there is a value that can
be queried from the device, we should figure out how to use it.

> > What I'm proposing is basically something like what's in the patch
> > below (for now, just compile-tested).
> > 
> > So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
> > switch to the HID++ report mode at device connect and handle the Wheel
> > events. If the wheel event is low res, will generate
> > REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.
> 
> Hmm... "H" in HWHEEL is not for "hardware" or "high-res", but,
> instead, for "horizontal".

yep :)

> So, if we'd go for the proposal of using a different event for high-res
> vertical wheel, instead of adding a new ioctl (EVIOCGREL as equivalent to
> EVIOCGABS), we'll need an extra event.
> 
> Anyway, the patch below works fine with my mouse. It is against
> Kernel 4.10.4.

quick skim of the patch looks ok, but the big issue with this isn't the
technical bit but the policy bit. So the last hunk adding the event code
should accompanied by a Documentation/input/event-codes.txt hunk, explaining
the use of the code and how it interacts with the other event codes.
You'll probably find that as you write that documentation, more
questions will come up.

Cheers,
   Peter

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Mon, 27 Mar 2017 11:38:45 +1000
Peter Hutterer <peter.hutterer@who-t.net> escreveu:

> On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:
> > Em Sat, 25 Mar 2017 09:36:18 -0300
> > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> >   
> > > Em Fri, 24 Mar 2017 06:57:00 -0300
> > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > >   
> > > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > >     
> > > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:      
> > > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:        
> > > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?          
> > > > > > > 
> > > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > > 
> > > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > > a state, and we should be able to forward this state with such a new
> > > > > > > event.
> > > > > > > 
> > > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > > can switch to high-res or not, so a static hwdb entry won't help.        
> > > > > > 
> > > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > > events to arrive:
> > > > > > 
> > > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > > 
> > > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > > is not direct.
> > > > > > 
> > > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > > axes changes. So, it should be possible to define a different event
> > > > > > for high-res wheel.
> > > > > > 
> > > > > > E. g. if the event reports high-res, it would be generating:
> > > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > > 
> > > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > > requiring any DB for the devices that allow switching the mode.        
> > > > > 
> > > > > sort-of. The main problem with relative axes is that we don't have a
> > > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > > converts kernel data to physical dimensions so that callers can use the data
> > > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > > around a bit.
> > > > > 
> > > > > Using ABS events simply gives us the resolution in the inital description.
> > > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > > first time it has come up, it would be interesting to add something like
> > > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > > resolution. But I also suspect that all but this use-case would have the
> > > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.      
> > > > 
> > > > I see. Well, at least in the case of the feature supported by this
> > > > mouse, there are just two possible resolutions: low-res and high-res.
> > > > The high-res resolution is fixed[1].
> > > > 
> > > > As the multiplier has a fixed value per device, a hwdb could still
> > > > work, provided that high-res wheel events would produce a different
> > > > event code than low-res.
> > > > 
> > > > [1] there's a USB message that can be used to query the multiplier,
> > > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > > devices with this feature use the same multiplier.    
> 
> let's not assume that and plan ahead, because sooner or later this will be
> configurable in some device. Probably before we get the first kernel out
> with this patchset in. :)

Yeah, it sounds likely that newer devices may allow to set it.

But the actual question here is: how userspace would handle it?

When the device is in ratchet mode (e. g. in "discrete" mode), the number
of events received for a single ratchet position movement should be multiple
of the high-res multiplier.

For example, MX Anywhere 2 has a fixed resolution (HID++ feature
reports multiplier == 8).

On this device, moving the wheel down just one ratchet position,
in low-res mode it produces just one event:

URBs:
>>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00

events:
1490616222.091664: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
1490616222.091664: event type EV_SYN(0x00).

in high-resolution mode, the same movement produces 8 events:

URBs:
>>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00

events:
1490616255.382047: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
1490616255.382047: event type EV_SYN(0x00).
1490616255.434046: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
1490616255.434046: event type EV_SYN(0x00).
1490616255.462060: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
1490616255.462060: event type EV_SYN(0x00).
1490616255.477994: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
1490616255.477994: event type EV_SYN(0x00).
1490616255.502022: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
1490616255.502022: event type EV_SYN(0x00).
1490616255.510016: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
1490616255.510016: event type EV_SYN(0x00).
1490616255.542061: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
1490616255.542061: event type EV_SYN(0x00).
1490616255.584051: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
1490616255.584051: event type EV_SYN(0x00).

Assuming that the user is, for example, navigating on a browser,
he would likely be expecting that one ratchet position will scroll
just one line of text, no matter in what resolution. E. g. the
scroll delta should be calculated by:

	float delta = evdev.value / (multiplier * 1.0)

---

Assuming that, we'll have, on some future, a mouse with a adjustable
wheel resolution with a multiplier between 1 and 8, when the wheel
is in free wheel mode, the delta logic could, instead, assume a value
in the middle of the multiplier range, e. g.:

	float delta = evdev.value / 4.0

This way, changing the wheel resolution would cause the text
to scroll faster of slower, with would likely be what the
user wants when changing the wheel resolution and placing the
wheel in free wheel mode.

> a hwdb could still work in that case, but it gets quite tricky when it
> becomes user-configurable. Now you need the user (or supporting software) to
> drop hwdb in entries which is less than ideal. If there is a value that can
> be queried from the device, we should figure out how to use it.

Querying the value is easy, but we'll need to report it somehow to
userspace.

We could report it either via some ioctl that would be enum/query/set
the wheel resolution (similar to EVIOCGABS/EVIOCSABS) or via some
event that would be report via read() telling about wheel
resolution changes.

On the latter, we could, for example, create an EV_SW for resolution
change that would be generated if the resolution of the new event is
different than the previously reported one. On this device, detecting
the resolution is trivial, as every events report it, but we have no
means to know how some other device would implement it.

> > > What I'm proposing is basically something like what's in the patch
> > > below (for now, just compile-tested).
> > > 
> > > So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
> > > switch to the HID++ report mode at device connect and handle the Wheel
> > > events. If the wheel event is low res, will generate
> > > REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.  
> > 
> > Hmm... "H" in HWHEEL is not for "hardware" or "high-res", but,
> > instead, for "horizontal".  
> 
> yep :)
> 
> > So, if we'd go for the proposal of using a different event for high-res
> > vertical wheel, instead of adding a new ioctl (EVIOCGREL as equivalent to
> > EVIOCGABS), we'll need an extra event.
> > 
> > Anyway, the patch below works fine with my mouse. It is against
> > Kernel 4.10.4.  
> 
> quick skim of the patch looks ok, but the big issue with this isn't the
> technical bit but the policy bit. 

Yes.

> So the last hunk adding the event code
> should accompanied by a Documentation/input/event-codes.txt hunk, explaining
> the use of the code and how it interacts with the other event codes.
> You'll probably find that as you write that documentation, more
> questions will come up.

Sure I will document it, but we should first define how we'll report it,
e. g. as a different event (like proposed on this RFC), via new ioctls or
via a resolution change event.

Thanks,
Mauro

Re: Support for Logitech MX Anywhere 2

[Benjamin Tissoires]

On Mar 27 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> Em Mon, 27 Mar 2017 11:38:45 +1000
> Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> 
> > On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:
> > > Em Sat, 25 Mar 2017 09:36:18 -0300
> > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > >   
> > > > Em Fri, 24 Mar 2017 06:57:00 -0300
> > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > >   
> > > > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > >     
> > > > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:      
> > > > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:        
> > > > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?          
> > > > > > > > 
> > > > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > > > 
> > > > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > > > a state, and we should be able to forward this state with such a new
> > > > > > > > event.
> > > > > > > > 
> > > > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > > > can switch to high-res or not, so a static hwdb entry won't help.        
> > > > > > > 
> > > > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > > > events to arrive:
> > > > > > > 
> > > > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > > > 
> > > > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > > > is not direct.
> > > > > > > 
> > > > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > > > axes changes. So, it should be possible to define a different event
> > > > > > > for high-res wheel.
> > > > > > > 
> > > > > > > E. g. if the event reports high-res, it would be generating:
> > > > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > > > 
> > > > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > > > requiring any DB for the devices that allow switching the mode.        
> > > > > > 
> > > > > > sort-of. The main problem with relative axes is that we don't have a
> > > > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > > > converts kernel data to physical dimensions so that callers can use the data
> > > > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > > > around a bit.
> > > > > > 
> > > > > > Using ABS events simply gives us the resolution in the inital description.
> > > > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > > > first time it has come up, it would be interesting to add something like
> > > > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > > > resolution. But I also suspect that all but this use-case would have the
> > > > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.      
> > > > > 
> > > > > I see. Well, at least in the case of the feature supported by this
> > > > > mouse, there are just two possible resolutions: low-res and high-res.
> > > > > The high-res resolution is fixed[1].
> > > > > 
> > > > > As the multiplier has a fixed value per device, a hwdb could still
> > > > > work, provided that high-res wheel events would produce a different
> > > > > event code than low-res.
> > > > > 
> > > > > [1] there's a USB message that can be used to query the multiplier,
> > > > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > > > devices with this feature use the same multiplier.    
> > 
> > let's not assume that and plan ahead, because sooner or later this will be
> > configurable in some device. Probably before we get the first kernel out
> > with this patchset in. :)
> 
> Yeah, it sounds likely that newer devices may allow to set it.
> 
> But the actual question here is: how userspace would handle it?
> 
> When the device is in ratchet mode (e. g. in "discrete" mode), the number
> of events received for a single ratchet position movement should be multiple
> of the high-res multiplier.
> 
> For example, MX Anywhere 2 has a fixed resolution (HID++ feature
> reports multiplier == 8).
> 
> On this device, moving the wheel down just one ratchet position,
> in low-res mode it produces just one event:
> 
> URBs:
> >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> 
> events:
> 1490616222.091664: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
> 1490616222.091664: event type EV_SYN(0x00).
> 
> in high-resolution mode, the same movement produces 8 events:
> 
> URBs:
> >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00

I wonder if in that case, the driver shouldn't convert those into a
single REL_WHEEL. The driver knows the state of the ratchet and can
detect such situation (and also match if the multiplier is user
configurable).

OTOH, if the highres wheel has the correct settings in the hwdb, there
is no reasons for libinput to not handle the 8 highres events equal one
line given that it already converts the incoming events into physical
dimensions. 

> 
> events:
> 1490616255.382047: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> 1490616255.382047: event type EV_SYN(0x00).
> 1490616255.434046: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> 1490616255.434046: event type EV_SYN(0x00).
> 1490616255.462060: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> 1490616255.462060: event type EV_SYN(0x00).
> 1490616255.477994: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> 1490616255.477994: event type EV_SYN(0x00).
> 1490616255.502022: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> 1490616255.502022: event type EV_SYN(0x00).
> 1490616255.510016: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> 1490616255.510016: event type EV_SYN(0x00).
> 1490616255.542061: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> 1490616255.542061: event type EV_SYN(0x00).
> 1490616255.584051: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> 1490616255.584051: event type EV_SYN(0x00).
> 
> Assuming that the user is, for example, navigating on a browser,
> he would likely be expecting that one ratchet position will scroll
> just one line of text, no matter in what resolution. E. g. the
> scroll delta should be calculated by:
> 
> 	float delta = evdev.value / (multiplier * 1.0)
> 
> ---
> 
> Assuming that, we'll have, on some future, a mouse with a adjustable
> wheel resolution with a multiplier between 1 and 8, when the wheel
> is in free wheel mode, the delta logic could, instead, assume a value
> in the middle of the multiplier range, e. g.:
> 
> 	float delta = evdev.value / 4.0
> 
> This way, changing the wheel resolution would cause the text
> to scroll faster of slower, with would likely be what the
> user wants when changing the wheel resolution and placing the
> wheel in free wheel mode.
> 
> > a hwdb could still work in that case, but it gets quite tricky when it
> > becomes user-configurable. Now you need the user (or supporting software) to
> > drop hwdb in entries which is less than ideal. If there is a value that can
> > be queried from the device, we should figure out how to use it.
> 
> Querying the value is easy, but we'll need to report it somehow to
> userspace.
> 
> We could report it either via some ioctl that would be enum/query/set
> the wheel resolution (similar to EVIOCGABS/EVIOCSABS) or via some
> event that would be report via read() telling about wheel
> resolution changes.

I know we have user configurable mouse resolutions, but do you really
believe we will have user configurable wheel resolution? That seems a
little bit unlikely to me and if it really happens, it would be a FW
trick to change the scrolling speed (like the user configurable
resolution is a FW trick to change the mouse speed).

So I'd say let's focus on a fixed wheel resolution (one for normal
wheel, one for highres) and really start thinking at user configurable
mouse wheel resolution when the device becomes available and that we
have a proper use case for it.

> 
> On the latter, we could, for example, create an EV_SW for resolution

Note that EV_SW is used for switches, like the physical switches that
have a state (on-off).

> change that would be generated if the resolution of the new event is
> different than the previously reported one. On this device, detecting
> the resolution is trivial, as every events report it, but we have no
> means to know how some other device would implement it.
> 
> > > > What I'm proposing is basically something like what's in the patch
> > > > below (for now, just compile-tested).
> > > > 
> > > > So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
> > > > switch to the HID++ report mode at device connect and handle the Wheel
> > > > events. If the wheel event is low res, will generate
> > > > REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.  
> > > 
> > > Hmm... "H" in HWHEEL is not for "hardware" or "high-res", but,
> > > instead, for "horizontal".  
> > 
> > yep :)
> > 
> > > So, if we'd go for the proposal of using a different event for high-res
> > > vertical wheel, instead of adding a new ioctl (EVIOCGREL as equivalent to
> > > EVIOCGABS), we'll need an extra event.
> > > 
> > > Anyway, the patch below works fine with my mouse. It is against
> > > Kernel 4.10.4.  
> > 
> > quick skim of the patch looks ok, but the big issue with this isn't the
> > technical bit but the policy bit. 
> 
> Yes.
> 
> > So the last hunk adding the event code
> > should accompanied by a Documentation/input/event-codes.txt hunk, explaining
> > the use of the code and how it interacts with the other event codes.
> > You'll probably find that as you write that documentation, more
> > questions will come up.
> 
> Sure I will document it, but we should first define how we'll report it,
> e. g. as a different event (like proposed on this RFC), via new ioctls or
> via a resolution change event.

I would be happy with the new event. Dmitry would need to validate the
change, but it seems the best choice for me. Then, I would suggest
masking the ratchet button and events from the userspace in the driver,
so that the burden of handling the highres wheel doesn't get too
complex. I think libinput handles the wheel as physical dimensions, so
it should be able to handle properly the highres wheel. xorg-input-evdev
would need some update though.

Cheers,
Benjamin

> 
> Thanks,
> Mauro

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Fri, 31 Mar 2017 12:03:08 +0200
Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:

> On Mar 27 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > Em Mon, 27 Mar 2017 11:38:45 +1000
> > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> >   
> > > On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:  
> > > > Em Sat, 25 Mar 2017 09:36:18 -0300
> > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > >     
> > > > > Em Fri, 24 Mar 2017 06:57:00 -0300
> > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > >     
> > > > > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > > >       
> > > > > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:        
> > > > > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:          
> > > > > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?            
> > > > > > > > > 
> > > > > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > > > > 
> > > > > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > > > > a state, and we should be able to forward this state with such a new
> > > > > > > > > event.
> > > > > > > > > 
> > > > > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > > > > can switch to high-res or not, so a static hwdb entry won't help.          
> > > > > > > > 
> > > > > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > > > > events to arrive:
> > > > > > > > 
> > > > > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > > > > 
> > > > > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > > > > is not direct.
> > > > > > > > 
> > > > > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > > > > axes changes. So, it should be possible to define a different event
> > > > > > > > for high-res wheel.
> > > > > > > > 
> > > > > > > > E. g. if the event reports high-res, it would be generating:
> > > > > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > > > > 
> > > > > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > > > > requiring any DB for the devices that allow switching the mode.          
> > > > > > > 
> > > > > > > sort-of. The main problem with relative axes is that we don't have a
> > > > > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > > > > converts kernel data to physical dimensions so that callers can use the data
> > > > > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > > > > around a bit.
> > > > > > > 
> > > > > > > Using ABS events simply gives us the resolution in the inital description.
> > > > > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > > > > first time it has come up, it would be interesting to add something like
> > > > > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > > > > resolution. But I also suspect that all but this use-case would have the
> > > > > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.        
> > > > > > 
> > > > > > I see. Well, at least in the case of the feature supported by this
> > > > > > mouse, there are just two possible resolutions: low-res and high-res.
> > > > > > The high-res resolution is fixed[1].
> > > > > > 
> > > > > > As the multiplier has a fixed value per device, a hwdb could still
> > > > > > work, provided that high-res wheel events would produce a different
> > > > > > event code than low-res.
> > > > > > 
> > > > > > [1] there's a USB message that can be used to query the multiplier,
> > > > > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > > > > devices with this feature use the same multiplier.      
> > > 
> > > let's not assume that and plan ahead, because sooner or later this will be
> > > configurable in some device. Probably before we get the first kernel out
> > > with this patchset in. :)  
> > 
> > Yeah, it sounds likely that newer devices may allow to set it.
> > 
> > But the actual question here is: how userspace would handle it?
> > 
> > When the device is in ratchet mode (e. g. in "discrete" mode), the number
> > of events received for a single ratchet position movement should be multiple
> > of the high-res multiplier.
> > 
> > For example, MX Anywhere 2 has a fixed resolution (HID++ feature
> > reports multiplier == 8).
> > 
> > On this device, moving the wheel down just one ratchet position,
> > in low-res mode it produces just one event:
> > 
> > URBs:  
> > >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00  
> > 
> > events:
> > 1490616222.091664: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
> > 1490616222.091664: event type EV_SYN(0x00).
> > 
> > in high-resolution mode, the same movement produces 8 events:
> > 
> > URBs:  
> > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00  
> 
> I wonder if in that case, the driver shouldn't convert those into a
> single REL_WHEEL. The driver knows the state of the ratchet and can
> detect such situation (and also match if the multiplier is user
> configurable).

IMHO, it shouldn't. While you have the finger at the wheel, you
can control the speed of the movement. You can also decide you
don't want to scroll and return to the previous position, like
on this movement (here, I moved the wheel down, slowly, then
I returned it back to the original position, on a fast move):

000033934 ms 000734 ms (783955 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000034718 ms 000784 ms (119937 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000034838 ms 000120 ms (075936 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000034914 ms 000076 ms (097951 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000035012 ms 000098 ms (071950 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000035084 ms 000072 ms (143879 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000035228 ms 000144 ms (312011 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
000035540 ms 000312 ms (2213961 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
000037754 ms 002214 ms (075957 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
000037830 ms 000076 ms (031940 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
000037862 ms 000032 ms (015955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
000037878 ms 000016 ms (023917 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
000037902 ms 000024 ms (023955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
000037926 ms 000024 ms (029959 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00

If I was scrolling a screen that would allow scrolling on less than a
line, I would expect the screen to follow the speed of my finger.

> OTOH, if the highres wheel has the correct settings in the hwdb, there
> is no reasons for libinput to not handle the 8 highres events equal one
> line given that it already converts the incoming events into physical
> dimensions. 

Yes.

> > 
> > events:
> > 1490616255.382047: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > 1490616255.382047: event type EV_SYN(0x00).
> > 1490616255.434046: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > 1490616255.434046: event type EV_SYN(0x00).
> > 1490616255.462060: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > 1490616255.462060: event type EV_SYN(0x00).
> > 1490616255.477994: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > 1490616255.477994: event type EV_SYN(0x00).
> > 1490616255.502022: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > 1490616255.502022: event type EV_SYN(0x00).
> > 1490616255.510016: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > 1490616255.510016: event type EV_SYN(0x00).
> > 1490616255.542061: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > 1490616255.542061: event type EV_SYN(0x00).
> > 1490616255.584051: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > 1490616255.584051: event type EV_SYN(0x00).
> > 
> > Assuming that the user is, for example, navigating on a browser,
> > he would likely be expecting that one ratchet position will scroll
> > just one line of text, no matter in what resolution. E. g. the
> > scroll delta should be calculated by:
> > 
> > 	float delta = evdev.value / (multiplier * 1.0)
> > 
> > ---
> > 
> > Assuming that, we'll have, on some future, a mouse with a adjustable
> > wheel resolution with a multiplier between 1 and 8, when the wheel
> > is in free wheel mode, the delta logic could, instead, assume a value
> > in the middle of the multiplier range, e. g.:
> > 
> > 	float delta = evdev.value / 4.0
> > 
> > This way, changing the wheel resolution would cause the text
> > to scroll faster of slower, with would likely be what the
> > user wants when changing the wheel resolution and placing the
> > wheel in free wheel mode.
> >   
> > > a hwdb could still work in that case, but it gets quite tricky when it
> > > becomes user-configurable. Now you need the user (or supporting software) to
> > > drop hwdb in entries which is less than ideal. If there is a value that can
> > > be queried from the device, we should figure out how to use it.  
> > 
> > Querying the value is easy, but we'll need to report it somehow to
> > userspace.
> > 
> > We could report it either via some ioctl that would be enum/query/set
> > the wheel resolution (similar to EVIOCGABS/EVIOCSABS) or via some
> > event that would be report via read() telling about wheel
> > resolution changes.  
> 
> I know we have user configurable mouse resolutions, but do you really
> believe we will have user configurable wheel resolution? That seems a
> little bit unlikely to me and if it really happens, it would be a FW
> trick to change the scrolling speed (like the user configurable
> resolution is a FW trick to change the mouse speed).
> 
> So I'd say let's focus on a fixed wheel resolution (one for normal
> wheel, one for highres) and really start thinking at user configurable
> mouse wheel resolution when the device becomes available and that we
> have a proper use case for it.

Works for me.

> > 
> > On the latter, we could, for example, create an EV_SW for resolution  
> 
> Note that EV_SW is used for switches, like the physical switches that
> have a state (on-off).

The ratchet switch is a switch and has a state. Physically, the switch
is at the wheel: if you press it, it will switch to ON (ratchet mode).
So, if you try move the wheel, you'll feel a resistance the movement.
You'll also feel "clicks" on your finger when you scroll.

If you press again, it will switch to the OFF state (free wheel), and
there will be no resistance to wheel movements anymore, nor any
"clicks" will be felt on your finger.

> > change that would be generated if the resolution of the new event is
> > different than the previously reported one. On this device, detecting
> > the resolution is trivial, as every events report it, but we have no
> > means to know how some other device would implement it.
> >   
> > > > > What I'm proposing is basically something like what's in the patch
> > > > > below (for now, just compile-tested).
> > > > > 
> > > > > So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
> > > > > switch to the HID++ report mode at device connect and handle the Wheel
> > > > > events. If the wheel event is low res, will generate
> > > > > REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.    
> > > > 
> > > > Hmm... "H" in HWHEEL is not for "hardware" or "high-res", but,
> > > > instead, for "horizontal".    
> > > 
> > > yep :)
> > >   
> > > > So, if we'd go for the proposal of using a different event for high-res
> > > > vertical wheel, instead of adding a new ioctl (EVIOCGREL as equivalent to
> > > > EVIOCGABS), we'll need an extra event.
> > > > 
> > > > Anyway, the patch below works fine with my mouse. It is against
> > > > Kernel 4.10.4.    
> > > 
> > > quick skim of the patch looks ok, but the big issue with this isn't the
> > > technical bit but the policy bit.   
> > 
> > Yes.
> >   
> > > So the last hunk adding the event code
> > > should accompanied by a Documentation/input/event-codes.txt hunk, explaining
> > > the use of the code and how it interacts with the other event codes.
> > > You'll probably find that as you write that documentation, more
> > > questions will come up.  
> > 
> > Sure I will document it, but we should first define how we'll report it,
> > e. g. as a different event (like proposed on this RFC), via new ioctls or
> > via a resolution change event.  
> 
> I would be happy with the new event. Dmitry would need to validate the
> change, but it seems the best choice for me.

Good! I'll prepare a patchset with the documentation.

> Then, I would suggest
> masking the ratchet button and events from the userspace in the driver,
> so that the burden of handling the highres wheel doesn't get too
> complex. 

Not sure what you're meaning here. Are you meaning that, instead of
enabling those events to do something like this at the driver?

	__clear_bit(REL_HIRES_WHEEL, hrd->input->relbit);
	__clear_bit(SW_RATCHET, hrd->input->swbit);

> I think libinput handles the wheel as physical dimensions, so
> it should be able to handle properly the highres wheel. xorg-input-evdev
> would need some update though.
> 
> Cheers,
> Benjamin
> 
> > 
> > Thanks,
> > Mauro  



Thanks,
Mauro

Re: Support for Logitech MX Anywhere 2

[Benjamin Tissoires]

On Mar 31 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> Em Fri, 31 Mar 2017 12:03:08 +0200
> Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:
> 
> > On Mar 27 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > > Em Mon, 27 Mar 2017 11:38:45 +1000
> > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > >   
> > > > On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:  
> > > > > Em Sat, 25 Mar 2017 09:36:18 -0300
> > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > >     
> > > > > > Em Fri, 24 Mar 2017 06:57:00 -0300
> > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > >     
> > > > > > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > > > >       
> > > > > > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:        
> > > > > > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:          
> > > > > > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?            
> > > > > > > > > > 
> > > > > > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > > > > > 
> > > > > > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > > > > > a state, and we should be able to forward this state with such a new
> > > > > > > > > > event.
> > > > > > > > > > 
> > > > > > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > > > > > can switch to high-res or not, so a static hwdb entry won't help.          
> > > > > > > > > 
> > > > > > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > > > > > events to arrive:
> > > > > > > > > 
> > > > > > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > > > > > 
> > > > > > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > > > > > is not direct.
> > > > > > > > > 
> > > > > > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > > > > > axes changes. So, it should be possible to define a different event
> > > > > > > > > for high-res wheel.
> > > > > > > > > 
> > > > > > > > > E. g. if the event reports high-res, it would be generating:
> > > > > > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > > > > > 
> > > > > > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > > > > > requiring any DB for the devices that allow switching the mode.          
> > > > > > > > 
> > > > > > > > sort-of. The main problem with relative axes is that we don't have a
> > > > > > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > > > > > converts kernel data to physical dimensions so that callers can use the data
> > > > > > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > > > > > around a bit.
> > > > > > > > 
> > > > > > > > Using ABS events simply gives us the resolution in the inital description.
> > > > > > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > > > > > first time it has come up, it would be interesting to add something like
> > > > > > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > > > > > resolution. But I also suspect that all but this use-case would have the
> > > > > > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.        
> > > > > > > 
> > > > > > > I see. Well, at least in the case of the feature supported by this
> > > > > > > mouse, there are just two possible resolutions: low-res and high-res.
> > > > > > > The high-res resolution is fixed[1].
> > > > > > > 
> > > > > > > As the multiplier has a fixed value per device, a hwdb could still
> > > > > > > work, provided that high-res wheel events would produce a different
> > > > > > > event code than low-res.
> > > > > > > 
> > > > > > > [1] there's a USB message that can be used to query the multiplier,
> > > > > > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > > > > > devices with this feature use the same multiplier.      
> > > > 
> > > > let's not assume that and plan ahead, because sooner or later this will be
> > > > configurable in some device. Probably before we get the first kernel out
> > > > with this patchset in. :)  
> > > 
> > > Yeah, it sounds likely that newer devices may allow to set it.
> > > 
> > > But the actual question here is: how userspace would handle it?
> > > 
> > > When the device is in ratchet mode (e. g. in "discrete" mode), the number
> > > of events received for a single ratchet position movement should be multiple
> > > of the high-res multiplier.
> > > 
> > > For example, MX Anywhere 2 has a fixed resolution (HID++ feature
> > > reports multiplier == 8).
> > > 
> > > On this device, moving the wheel down just one ratchet position,
> > > in low-res mode it produces just one event:
> > > 
> > > URBs:  
> > > >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00  
> > > 
> > > events:
> > > 1490616222.091664: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
> > > 1490616222.091664: event type EV_SYN(0x00).
> > > 
> > > in high-resolution mode, the same movement produces 8 events:
> > > 
> > > URBs:  
> > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00  
> > 
> > I wonder if in that case, the driver shouldn't convert those into a
> > single REL_WHEEL. The driver knows the state of the ratchet and can
> > detect such situation (and also match if the multiplier is user
> > configurable).
> 
> IMHO, it shouldn't. While you have the finger at the wheel, you
> can control the speed of the movement. You can also decide you
> don't want to scroll and return to the previous position, like
> on this movement (here, I moved the wheel down, slowly, then
> I returned it back to the original position, on a fast move):
> 
> 000033934 ms 000734 ms (783955 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000034718 ms 000784 ms (119937 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000034838 ms 000120 ms (075936 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000034914 ms 000076 ms (097951 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000035012 ms 000098 ms (071950 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000035084 ms 000072 ms (143879 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000035228 ms 000144 ms (312011 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000035540 ms 000312 ms (2213961 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000037754 ms 002214 ms (075957 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000037830 ms 000076 ms (031940 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000037862 ms 000032 ms (015955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000037878 ms 000016 ms (023917 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000037902 ms 000024 ms (023955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000037926 ms 000024 ms (029959 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> 
> If I was scrolling a screen that would allow scrolling on less than a
> line, I would expect the screen to follow the speed of my finger.

Alright.

> 
> > OTOH, if the highres wheel has the correct settings in the hwdb, there
> > is no reasons for libinput to not handle the 8 highres events equal one
> > line given that it already converts the incoming events into physical
> > dimensions. 
> 
> Yes.
> 
> > > 
> > > events:
> > > 1490616255.382047: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > 1490616255.382047: event type EV_SYN(0x00).
> > > 1490616255.434046: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > 1490616255.434046: event type EV_SYN(0x00).
> > > 1490616255.462060: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > 1490616255.462060: event type EV_SYN(0x00).
> > > 1490616255.477994: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > 1490616255.477994: event type EV_SYN(0x00).
> > > 1490616255.502022: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > 1490616255.502022: event type EV_SYN(0x00).
> > > 1490616255.510016: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > 1490616255.510016: event type EV_SYN(0x00).
> > > 1490616255.542061: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > 1490616255.542061: event type EV_SYN(0x00).
> > > 1490616255.584051: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > 1490616255.584051: event type EV_SYN(0x00).
> > > 
> > > Assuming that the user is, for example, navigating on a browser,
> > > he would likely be expecting that one ratchet position will scroll
> > > just one line of text, no matter in what resolution. E. g. the
> > > scroll delta should be calculated by:
> > > 
> > > 	float delta = evdev.value / (multiplier * 1.0)
> > > 
> > > ---
> > > 
> > > Assuming that, we'll have, on some future, a mouse with a adjustable
> > > wheel resolution with a multiplier between 1 and 8, when the wheel
> > > is in free wheel mode, the delta logic could, instead, assume a value
> > > in the middle of the multiplier range, e. g.:
> > > 
> > > 	float delta = evdev.value / 4.0
> > > 
> > > This way, changing the wheel resolution would cause the text
> > > to scroll faster of slower, with would likely be what the
> > > user wants when changing the wheel resolution and placing the
> > > wheel in free wheel mode.
> > >   
> > > > a hwdb could still work in that case, but it gets quite tricky when it
> > > > becomes user-configurable. Now you need the user (or supporting software) to
> > > > drop hwdb in entries which is less than ideal. If there is a value that can
> > > > be queried from the device, we should figure out how to use it.  
> > > 
> > > Querying the value is easy, but we'll need to report it somehow to
> > > userspace.
> > > 
> > > We could report it either via some ioctl that would be enum/query/set
> > > the wheel resolution (similar to EVIOCGABS/EVIOCSABS) or via some
> > > event that would be report via read() telling about wheel
> > > resolution changes.  
> > 
> > I know we have user configurable mouse resolutions, but do you really
> > believe we will have user configurable wheel resolution? That seems a
> > little bit unlikely to me and if it really happens, it would be a FW
> > trick to change the scrolling speed (like the user configurable
> > resolution is a FW trick to change the mouse speed).
> > 
> > So I'd say let's focus on a fixed wheel resolution (one for normal
> > wheel, one for highres) and really start thinking at user configurable
> > mouse wheel resolution when the device becomes available and that we
> > have a proper use case for it.
> 
> Works for me.
> 
> > > 
> > > On the latter, we could, for example, create an EV_SW for resolution  
> > 
> > Note that EV_SW is used for switches, like the physical switches that
> > have a state (on-off).
> 
> The ratchet switch is a switch and has a state. Physically, the switch
> is at the wheel: if you press it, it will switch to ON (ratchet mode).
> So, if you try move the wheel, you'll feel a resistance the movement.
> You'll also feel "clicks" on your finger when you scroll.

I was referring to EV_SW for resolution change (user configurable
resolution change). I am fine have an SW_RATCHET with the on/off state.
However, for resolution change, this basically changes the properties of
the device, and this is going to be a can of worms.

> 
> If you press again, it will switch to the OFF state (free wheel), and
> there will be no resistance to wheel movements anymore, nor any
> "clicks" will be felt on your finger.
> 
> > > change that would be generated if the resolution of the new event is
> > > different than the previously reported one. On this device, detecting
> > > the resolution is trivial, as every events report it, but we have no
> > > means to know how some other device would implement it.
> > >   
> > > > > > What I'm proposing is basically something like what's in the patch
> > > > > > below (for now, just compile-tested).
> > > > > > 
> > > > > > So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
> > > > > > switch to the HID++ report mode at device connect and handle the Wheel
> > > > > > events. If the wheel event is low res, will generate
> > > > > > REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.    
> > > > > 
> > > > > Hmm... "H" in HWHEEL is not for "hardware" or "high-res", but,
> > > > > instead, for "horizontal".    
> > > > 
> > > > yep :)
> > > >   
> > > > > So, if we'd go for the proposal of using a different event for high-res
> > > > > vertical wheel, instead of adding a new ioctl (EVIOCGREL as equivalent to
> > > > > EVIOCGABS), we'll need an extra event.
> > > > > 
> > > > > Anyway, the patch below works fine with my mouse. It is against
> > > > > Kernel 4.10.4.    
> > > > 
> > > > quick skim of the patch looks ok, but the big issue with this isn't the
> > > > technical bit but the policy bit.   
> > > 
> > > Yes.
> > >   
> > > > So the last hunk adding the event code
> > > > should accompanied by a Documentation/input/event-codes.txt hunk, explaining
> > > > the use of the code and how it interacts with the other event codes.
> > > > You'll probably find that as you write that documentation, more
> > > > questions will come up.  
> > > 
> > > Sure I will document it, but we should first define how we'll report it,
> > > e. g. as a different event (like proposed on this RFC), via new ioctls or
> > > via a resolution change event.  
> > 
> > I would be happy with the new event. Dmitry would need to validate the
> > change, but it seems the best choice for me.
> 
> Good! I'll prepare a patchset with the documentation.
> 
> > Then, I would suggest
> > masking the ratchet button and events from the userspace in the driver,
> > so that the burden of handling the highres wheel doesn't get too
> > complex. 
> 
> Not sure what you're meaning here. Are you meaning that, instead of
> enabling those events to do something like this at the driver?
> 
> 	__clear_bit(REL_HIRES_WHEEL, hrd->input->relbit);
> 	__clear_bit(SW_RATCHET, hrd->input->swbit);

No. You can't change the properties/axis once the device has been
registered because we don't have a PROP_ event that mentions to clients
that the device changed. I was referring here to not forwarding the
ratchet switch to user space, and convert silently in the driver the
highres wheel events into lowres wheel when ratchet is on. But as
mentioned above, libinput shouldn't care about the ratchet state as long
as both highres and lowres are mapped to the same physical dimensions.

Cheers,
Benjamin

> > I think libinput handles the wheel as physical dimensions, so
> > it should be able to handle properly the highres wheel. xorg-input-evdev
> > would need some update though.
> > 
> > Cheers,
> > Benjamin
> > 
> > > 
> > > Thanks,
> > > Mauro  
> 
> 
> 
> Thanks,
> Mauro

Re: Support for Logitech MX Anywhere 2

[Peter Hutterer]

On Fri, Mar 31, 2017 at 02:28:27PM +0200, Benjamin Tissoires wrote:
> On Mar 31 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > Em Fri, 31 Mar 2017 12:03:08 +0200
> > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:
> > 
> > > On Mar 27 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > > > Em Mon, 27 Mar 2017 11:38:45 +1000
> > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > >   
> > > > > On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:  
> > > > > > Em Sat, 25 Mar 2017 09:36:18 -0300
> > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > >     
> > > > > > > Em Fri, 24 Mar 2017 06:57:00 -0300
> > > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > > >     
> > > > > > > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > > > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > > > > >       
> > > > > > > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:        
> > > > > > > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > > > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:          
> > > > > > > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?            
> > > > > > > > > > > 
> > > > > > > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > > > > > > 
> > > > > > > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > > > > > > a state, and we should be able to forward this state with such a new
> > > > > > > > > > > event.
> > > > > > > > > > > 
> > > > > > > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > > > > > > can switch to high-res or not, so a static hwdb entry won't help.          
> > > > > > > > > > 
> > > > > > > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > > > > > > events to arrive:
> > > > > > > > > > 
> > > > > > > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > > > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > > > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > > > > > > 
> > > > > > > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > > > > > > is not direct.
> > > > > > > > > > 
> > > > > > > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > > > > > > axes changes. So, it should be possible to define a different event
> > > > > > > > > > for high-res wheel.
> > > > > > > > > > 
> > > > > > > > > > E. g. if the event reports high-res, it would be generating:
> > > > > > > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > > > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > > > > > > 
> > > > > > > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > > > > > > requiring any DB for the devices that allow switching the mode.          
> > > > > > > > > 
> > > > > > > > > sort-of. The main problem with relative axes is that we don't have a
> > > > > > > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > > > > > > converts kernel data to physical dimensions so that callers can use the data
> > > > > > > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > > > > > > around a bit.
> > > > > > > > > 
> > > > > > > > > Using ABS events simply gives us the resolution in the inital description.
> > > > > > > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > > > > > > first time it has come up, it would be interesting to add something like
> > > > > > > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > > > > > > resolution. But I also suspect that all but this use-case would have the
> > > > > > > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.        
> > > > > > > > 
> > > > > > > > I see. Well, at least in the case of the feature supported by this
> > > > > > > > mouse, there are just two possible resolutions: low-res and high-res.
> > > > > > > > The high-res resolution is fixed[1].
> > > > > > > > 
> > > > > > > > As the multiplier has a fixed value per device, a hwdb could still
> > > > > > > > work, provided that high-res wheel events would produce a different
> > > > > > > > event code than low-res.
> > > > > > > > 
> > > > > > > > [1] there's a USB message that can be used to query the multiplier,
> > > > > > > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > > > > > > devices with this feature use the same multiplier.      
> > > > > 
> > > > > let's not assume that and plan ahead, because sooner or later this will be
> > > > > configurable in some device. Probably before we get the first kernel out
> > > > > with this patchset in. :)  
> > > > 
> > > > Yeah, it sounds likely that newer devices may allow to set it.
> > > > 
> > > > But the actual question here is: how userspace would handle it?
> > > > 
> > > > When the device is in ratchet mode (e. g. in "discrete" mode), the number
> > > > of events received for a single ratchet position movement should be multiple
> > > > of the high-res multiplier.
> > > > 
> > > > For example, MX Anywhere 2 has a fixed resolution (HID++ feature
> > > > reports multiplier == 8).
> > > > 
> > > > On this device, moving the wheel down just one ratchet position,
> > > > in low-res mode it produces just one event:
> > > > 
> > > > URBs:  
> > > > >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00  
> > > > 
> > > > events:
> > > > 1490616222.091664: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
> > > > 1490616222.091664: event type EV_SYN(0x00).
> > > > 
> > > > in high-resolution mode, the same movement produces 8 events:
> > > > 
> > > > URBs:  
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00  
> > > 
> > > I wonder if in that case, the driver shouldn't convert those into a
> > > single REL_WHEEL. The driver knows the state of the ratchet and can
> > > detect such situation (and also match if the multiplier is user
> > > configurable).
> > 
> > IMHO, it shouldn't. While you have the finger at the wheel, you
> > can control the speed of the movement. You can also decide you
> > don't want to scroll and return to the previous position, like
> > on this movement (here, I moved the wheel down, slowly, then
> > I returned it back to the original position, on a fast move):
> > 
> > 000033934 ms 000734 ms (783955 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000034718 ms 000784 ms (119937 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000034838 ms 000120 ms (075936 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000034914 ms 000076 ms (097951 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000035012 ms 000098 ms (071950 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000035084 ms 000072 ms (143879 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000035228 ms 000144 ms (312011 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000035540 ms 000312 ms (2213961 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037754 ms 002214 ms (075957 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037830 ms 000076 ms (031940 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037862 ms 000032 ms (015955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037878 ms 000016 ms (023917 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037902 ms 000024 ms (023955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037926 ms 000024 ms (029959 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 
> > If I was scrolling a screen that would allow scrolling on less than a
> > line, I would expect the screen to follow the speed of my finger.
> 
> Alright.
>
> > > OTOH, if the highres wheel has the correct settings in the hwdb, there
> > > is no reasons for libinput to not handle the 8 highres events equal one
> > > line given that it already converts the incoming events into physical
> > > dimensions. 
> > 
> > Yes.

I *think* this should be possible with the current libinput, without even
exposing more API. libinput provides a scroll delta for wheels in degrees
and a 'discrete' value, it would be fairly trivial to hook up the highres to
the degrees only and use the discrete as cumulative. So you'd get a sequence
like this:
   scroll event (deg 2, discrete 0)
   scroll event (deg 2, discrete 0)
   scroll event (deg 2, discrete 0)
   scroll event (deg 2, discrete 1)

a client that supports this, and I think current clients should basically
already support this can pick between normal and hires, without extra code.
what the impact of this is, I don't quite know yet.

> > > > events:
> > > > 1490616255.382047: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.382047: event type EV_SYN(0x00).
> > > > 1490616255.434046: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.434046: event type EV_SYN(0x00).
> > > > 1490616255.462060: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.462060: event type EV_SYN(0x00).
> > > > 1490616255.477994: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.477994: event type EV_SYN(0x00).
> > > > 1490616255.502022: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.502022: event type EV_SYN(0x00).
> > > > 1490616255.510016: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.510016: event type EV_SYN(0x00).
> > > > 1490616255.542061: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.542061: event type EV_SYN(0x00).
> > > > 1490616255.584051: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.584051: event type EV_SYN(0x00).
> > > > 
> > > > Assuming that the user is, for example, navigating on a browser,
> > > > he would likely be expecting that one ratchet position will scroll
> > > > just one line of text, no matter in what resolution. E. g. the
> > > > scroll delta should be calculated by:
> > > > 
> > > > 	float delta = evdev.value / (multiplier * 1.0)
> > > > 
> > > > ---
> > > > 
> > > > Assuming that, we'll have, on some future, a mouse with a adjustable
> > > > wheel resolution with a multiplier between 1 and 8, when the wheel
> > > > is in free wheel mode, the delta logic could, instead, assume a value
> > > > in the middle of the multiplier range, e. g.:
> > > > 
> > > > 	float delta = evdev.value / 4.0
> > > > 
> > > > This way, changing the wheel resolution would cause the text
> > > > to scroll faster of slower, with would likely be what the
> > > > user wants when changing the wheel resolution and placing the
> > > > wheel in free wheel mode.
> > > >   
> > > > > a hwdb could still work in that case, but it gets quite tricky when it
> > > > > becomes user-configurable. Now you need the user (or supporting software) to
> > > > > drop hwdb in entries which is less than ideal. If there is a value that can
> > > > > be queried from the device, we should figure out how to use it.  
> > > > 
> > > > Querying the value is easy, but we'll need to report it somehow to
> > > > userspace.
> > > > 
> > > > We could report it either via some ioctl that would be enum/query/set
> > > > the wheel resolution (similar to EVIOCGABS/EVIOCSABS) or via some
> > > > event that would be report via read() telling about wheel
> > > > resolution changes.  
> > > 
> > > I know we have user configurable mouse resolutions, but do you really
> > > believe we will have user configurable wheel resolution? That seems a
> > > little bit unlikely to me and if it really happens, it would be a FW
> > > trick to change the scrolling speed (like the user configurable
> > > resolution is a FW trick to change the mouse speed).
> > > 
> > > So I'd say let's focus on a fixed wheel resolution (one for normal
> > > wheel, one for highres) and really start thinking at user configurable
> > > mouse wheel resolution when the device becomes available and that we
> > > have a proper use case for it.
> > 
> > Works for me.
> > 
> > > > 
> > > > On the latter, we could, for example, create an EV_SW for resolution  
> > > 
> > > Note that EV_SW is used for switches, like the physical switches that
> > > have a state (on-off).
> > 
> > The ratchet switch is a switch and has a state. Physically, the switch
> > is at the wheel: if you press it, it will switch to ON (ratchet mode).
> > So, if you try move the wheel, you'll feel a resistance the movement.
> > You'll also feel "clicks" on your finger when you scroll.
> 
> I was referring to EV_SW for resolution change (user configurable
> resolution change). I am fine have an SW_RATCHET with the on/off state.
> However, for resolution change, this basically changes the properties of
> the device, and this is going to be a can of worms.

the question is basically whether the switch needs to be exposed to
userspace or not. If we have separate event codes, it's not really needed,
it doesn't add anything - see also the comment below, libinput won't care
about the switch unless I have to for event interpretation.

Cheers,
   Peter

> > If you press again, it will switch to the OFF state (free wheel), and
> > there will be no resistance to wheel movements anymore, nor any
> > "clicks" will be felt on your finger.
> > 
> > > > change that would be generated if the resolution of the new event is
> > > > different than the previously reported one. On this device, detecting
> > > > the resolution is trivial, as every events report it, but we have no
> > > > means to know how some other device would implement it.
> > > >   
> > > > > > > What I'm proposing is basically something like what's in the patch
> > > > > > > below (for now, just compile-tested).
> > > > > > > 
> > > > > > > So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
> > > > > > > switch to the HID++ report mode at device connect and handle the Wheel
> > > > > > > events. If the wheel event is low res, will generate
> > > > > > > REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.    
> > > > > > 
> > > > > > Hmm... "H" in HWHEEL is not for "hardware" or "high-res", but,
> > > > > > instead, for "horizontal".    
> > > > > 
> > > > > yep :)
> > > > >   
> > > > > > So, if we'd go for the proposal of using a different event for high-res
> > > > > > vertical wheel, instead of adding a new ioctl (EVIOCGREL as equivalent to
> > > > > > EVIOCGABS), we'll need an extra event.
> > > > > > 
> > > > > > Anyway, the patch below works fine with my mouse. It is against
> > > > > > Kernel 4.10.4.    
> > > > > 
> > > > > quick skim of the patch looks ok, but the big issue with this isn't the
> > > > > technical bit but the policy bit.   
> > > > 
> > > > Yes.
> > > >   
> > > > > So the last hunk adding the event code
> > > > > should accompanied by a Documentation/input/event-codes.txt hunk, explaining
> > > > > the use of the code and how it interacts with the other event codes.
> > > > > You'll probably find that as you write that documentation, more
> > > > > questions will come up.  
> > > > 
> > > > Sure I will document it, but we should first define how we'll report it,
> > > > e. g. as a different event (like proposed on this RFC), via new ioctls or
> > > > via a resolution change event.  
> > > 
> > > I would be happy with the new event. Dmitry would need to validate the
> > > change, but it seems the best choice for me.
> > 
> > Good! I'll prepare a patchset with the documentation.
> > 
> > > Then, I would suggest
> > > masking the ratchet button and events from the userspace in the driver,
> > > so that the burden of handling the highres wheel doesn't get too
> > > complex. 
> > 
> > Not sure what you're meaning here. Are you meaning that, instead of
> > enabling those events to do something like this at the driver?
> > 
> > 	__clear_bit(REL_HIRES_WHEEL, hrd->input->relbit);
> > 	__clear_bit(SW_RATCHET, hrd->input->swbit);
> 
> No. You can't change the properties/axis once the device has been
> registered because we don't have a PROP_ event that mentions to clients
> that the device changed. I was referring here to not forwarding the
> ratchet switch to user space, and convert silently in the driver the
> highres wheel events into lowres wheel when ratchet is on. But as
> mentioned above, libinput shouldn't care about the ratchet state as long
> as both highres and lowres are mapped to the same physical dimensions.
> 
> Cheers,
> Benjamin
> 
> > > I think libinput handles the wheel as physical dimensions, so
> > > it should be able to handle properly the highres wheel. xorg-input-evdev
> > > would need some update though.
> > > 
> > > Cheers,
> > > Benjamin
> > > 
> > > > 
> > > > Thanks,
> > > > Mauro  
> > 
> > 
> > 
> > Thanks,
> > Mauro

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Mon, 3 Apr 2017 14:43:07 +1000
Peter Hutterer <peter.hutterer@who-t.net> escreveu:

> On Fri, Mar 31, 2017 at 02:28:27PM +0200, Benjamin Tissoires wrote:
> > On Mar 31 2017 or thereabouts, Mauro Carvalho Chehab wrote:  
> > > Em Fri, 31 Mar 2017 12:03:08 +0200
> > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:
> > >   
> > > > On Mar 27 2017 or thereabouts, Mauro Carvalho Chehab wrote:  
> > > > > Em Mon, 27 Mar 2017 11:38:45 +1000
> > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > >     
> > > > > > On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:    
> > > > > > > Em Sat, 25 Mar 2017 09:36:18 -0300
> > > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > > >       
> > > > > > > > Em Fri, 24 Mar 2017 06:57:00 -0300
> > > > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > > > >       
> > > > > > > > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > > > > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > > > > > >         
> > > > > > > > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:          
> > > > > > > > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > > > > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:            
> > > > > > > > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?              
> > > > > > > > > > > > 
> > > > > > > > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > > > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > > > > > > > 
> > > > > > > > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > > > > > > > a state, and we should be able to forward this state with such a new
> > > > > > > > > > > > event.
> > > > > > > > > > > > 
> > > > > > > > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > > > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > > > > > > > can switch to high-res or not, so a static hwdb entry won't help.            
> > > > > > > > > > > 
> > > > > > > > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > > > > > > > events to arrive:
> > > > > > > > > > > 
> > > > > > > > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > > > > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > > > > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > > > > > > > 
> > > > > > > > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > > > > > > > is not direct.
> > > > > > > > > > > 
> > > > > > > > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > > > > > > > axes changes. So, it should be possible to define a different event
> > > > > > > > > > > for high-res wheel.
> > > > > > > > > > > 
> > > > > > > > > > > E. g. if the event reports high-res, it would be generating:
> > > > > > > > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > > > > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > > > > > > > 
> > > > > > > > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > > > > > > > requiring any DB for the devices that allow switching the mode.            
> > > > > > > > > > 
> > > > > > > > > > sort-of. The main problem with relative axes is that we don't have a
> > > > > > > > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > > > > > > > converts kernel data to physical dimensions so that callers can use the data
> > > > > > > > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > > > > > > > around a bit.
> > > > > > > > > > 
> > > > > > > > > > Using ABS events simply gives us the resolution in the inital description.
> > > > > > > > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > > > > > > > first time it has come up, it would be interesting to add something like
> > > > > > > > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > > > > > > > resolution. But I also suspect that all but this use-case would have the
> > > > > > > > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.          
> > > > > > > > > 
> > > > > > > > > I see. Well, at least in the case of the feature supported by this
> > > > > > > > > mouse, there are just two possible resolutions: low-res and high-res.
> > > > > > > > > The high-res resolution is fixed[1].
> > > > > > > > > 
> > > > > > > > > As the multiplier has a fixed value per device, a hwdb could still
> > > > > > > > > work, provided that high-res wheel events would produce a different
> > > > > > > > > event code than low-res.
> > > > > > > > > 
> > > > > > > > > [1] there's a USB message that can be used to query the multiplier,
> > > > > > > > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > > > > > > > devices with this feature use the same multiplier.        
> > > > > > 
> > > > > > let's not assume that and plan ahead, because sooner or later this will be
> > > > > > configurable in some device. Probably before we get the first kernel out
> > > > > > with this patchset in. :)    
> > > > > 
> > > > > Yeah, it sounds likely that newer devices may allow to set it.
> > > > > 
> > > > > But the actual question here is: how userspace would handle it?
> > > > > 
> > > > > When the device is in ratchet mode (e. g. in "discrete" mode), the number
> > > > > of events received for a single ratchet position movement should be multiple
> > > > > of the high-res multiplier.
> > > > > 
> > > > > For example, MX Anywhere 2 has a fixed resolution (HID++ feature
> > > > > reports multiplier == 8).
> > > > > 
> > > > > On this device, moving the wheel down just one ratchet position,
> > > > > in low-res mode it produces just one event:
> > > > > 
> > > > > URBs:    
> > > > > >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00    
> > > > > 
> > > > > events:
> > > > > 1490616222.091664: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
> > > > > 1490616222.091664: event type EV_SYN(0x00).
> > > > > 
> > > > > in high-resolution mode, the same movement produces 8 events:
> > > > > 
> > > > > URBs:    
> > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00    
> > > > 
> > > > I wonder if in that case, the driver shouldn't convert those into a
> > > > single REL_WHEEL. The driver knows the state of the ratchet and can
> > > > detect such situation (and also match if the multiplier is user
> > > > configurable).  
> > > 
> > > IMHO, it shouldn't. While you have the finger at the wheel, you
> > > can control the speed of the movement. You can also decide you
> > > don't want to scroll and return to the previous position, like
> > > on this movement (here, I moved the wheel down, slowly, then
> > > I returned it back to the original position, on a fast move):
> > > 
> > > 000033934 ms 000734 ms (783955 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000034718 ms 000784 ms (119937 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000034838 ms 000120 ms (075936 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000034914 ms 000076 ms (097951 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000035012 ms 000098 ms (071950 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000035084 ms 000072 ms (143879 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000035228 ms 000144 ms (312011 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000035540 ms 000312 ms (2213961 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000037754 ms 002214 ms (075957 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000037830 ms 000076 ms (031940 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000037862 ms 000032 ms (015955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000037878 ms 000016 ms (023917 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000037902 ms 000024 ms (023955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 000037926 ms 000024 ms (029959 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 
> > > If I was scrolling a screen that would allow scrolling on less than a
> > > line, I would expect the screen to follow the speed of my finger.  
> > 
> > Alright.
> >  
> > > > OTOH, if the highres wheel has the correct settings in the hwdb, there
> > > > is no reasons for libinput to not handle the 8 highres events equal one
> > > > line given that it already converts the incoming events into physical
> > > > dimensions.   
> > > 
> > > Yes.  
> 
> I *think* this should be possible with the current libinput, without even
> exposing more API. libinput provides a scroll delta for wheels in degrees
> and a 'discrete' value, it would be fairly trivial to hook up the highres to
> the degrees only and use the discrete as cumulative. So you'd get a sequence
> like this:
>    scroll event (deg 2, discrete 0)
>    scroll event (deg 2, discrete 0)
>    scroll event (deg 2, discrete 0)
>    scroll event (deg 2, discrete 1)
> 
> a client that supports this, and I think current clients should basically
> already support this can pick between normal and hires, without extra code.
> what the impact of this is, I don't quite know yet.

Hmm... if I understood well, the idea would be to use something
similar to udev's MOUSE_WHEEL_CLICK_ANGLE, like[1]:

	mouse:usb:v046dp404a:name:Logitech Anywhere Mouse MX 2:
	mouse:usb:v046dpc52b:name:Logitech Unifying Device. Wireless PID:404a:
	MOUSE_WHEEL_CLICK_ANGLE=16

[1] I measured the real wheel angle here. In low-resolution, there are 18
steps at the wheel (in ratchet mode), meaning 20 degrees. It means that, 
in high-resolution, where multiplier = 8, the minimum angle to produce an
event would be 2.5 degrees.

And add a code at libinput similar to this [2]:

diff --git a/src/evdev.c b/src/evdev.c
index 2a57b25835fe..b5198ca6154d 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -1023,6 +1023,24 @@ fallback_process_relative(struct fallback_dispatch *dispatch,
 			&wheel_degrees,
 			&discrete);
 		break;
+	case REL_HIRES_WHEEL:
+		fallback_flush_pending_event(dispatch, device, time);
+		wheel_degrees.y = -1 * e->value *
+					device->scroll.wheel_click_angle.x / 8;
+		discrete.y = -1 * e->value;
+
+		source = device->scroll.is_tilt.vertical ?
+				LIBINPUT_POINTER_AXIS_SOURCE_WHEEL_TILT:
+				LIBINPUT_POINTER_AXIS_SOURCE_WHEEL;
+
+		evdev_notify_axis(
+			device,
+			time,
+			AS_MASK(LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL),
+			source,
+			&wheel_degrees,
+			&discrete);
+		break;
 	case REL_HWHEEL:
 		fallback_flush_pending_event(dispatch, device, time);
 		wheel_degrees.x = e->value *

[2] I hardcoded there a multiply of 8, e. g. I'm doing:

		wheel_degrees.y = -1 * e->value *
					device->scroll.wheel_click_angle.x / 8;

Just to as a quick test code, but, ideally, the multiplier should either
be obtained via some ioctl or come from some udev property, e. g. either
a MOUSE_WHEEL_MULTIPLIER or a MOUSE_WHEEL_HIRES_CLICK_ANGLE property.

I'll do some tests here with the above code, and see if it does what's
expected.


Thanks,
Mauro

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Mon, 3 Apr 2017 09:49:03 -0300
Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:

> Em Mon, 3 Apr 2017 14:43:07 +1000
> Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> 
> > On Fri, Mar 31, 2017 at 02:28:27PM +0200, Benjamin Tissoires wrote:
> > > On Mar 31 2017 or thereabouts, Mauro Carvalho Chehab wrote:  
> > > > Em Fri, 31 Mar 2017 12:03:08 +0200
> > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:
> > > >   
> > > > > On Mar 27 2017 or thereabouts, Mauro Carvalho Chehab wrote:  
> > > > > > Em Mon, 27 Mar 2017 11:38:45 +1000
> > > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > > >     
> > > > > > > On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:    
> > > > > > > > Em Sat, 25 Mar 2017 09:36:18 -0300
> > > > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > > > >       
> > > > > > > > > Em Fri, 24 Mar 2017 06:57:00 -0300
> > > > > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > > > > >       
> > > > > > > > > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > > > > > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > > > > > > >         
> > > > > > > > > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:          
> > > > > > > > > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > > > > > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:            
> > > > > > > > > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > > > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > > > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?              
> > > > > > > > > > > > > 
> > > > > > > > > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > > > > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > > > > > > > > a state, and we should be able to forward this state with such a new
> > > > > > > > > > > > > event.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > > > > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > > > > > > > > can switch to high-res or not, so a static hwdb entry won't help.            
> > > > > > > > > > > > 
> > > > > > > > > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > > > > > > > > events to arrive:
> > > > > > > > > > > > 
> > > > > > > > > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > > > > > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > > > > > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > > > > > > > > 
> > > > > > > > > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > > > > > > > > is not direct.
> > > > > > > > > > > > 
> > > > > > > > > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > > > > > > > > axes changes. So, it should be possible to define a different event
> > > > > > > > > > > > for high-res wheel.
> > > > > > > > > > > > 
> > > > > > > > > > > > E. g. if the event reports high-res, it would be generating:
> > > > > > > > > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > > > > > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > > > > > > > > 
> > > > > > > > > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > > > > > > > > requiring any DB for the devices that allow switching the mode.            
> > > > > > > > > > > 
> > > > > > > > > > > sort-of. The main problem with relative axes is that we don't have a
> > > > > > > > > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > > > > > > > > converts kernel data to physical dimensions so that callers can use the data
> > > > > > > > > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > > > > > > > > around a bit.
> > > > > > > > > > > 
> > > > > > > > > > > Using ABS events simply gives us the resolution in the inital description.
> > > > > > > > > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > > > > > > > > first time it has come up, it would be interesting to add something like
> > > > > > > > > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > > > > > > > > resolution. But I also suspect that all but this use-case would have the
> > > > > > > > > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.          
> > > > > > > > > > 
> > > > > > > > > > I see. Well, at least in the case of the feature supported by this
> > > > > > > > > > mouse, there are just two possible resolutions: low-res and high-res.
> > > > > > > > > > The high-res resolution is fixed[1].
> > > > > > > > > > 
> > > > > > > > > > As the multiplier has a fixed value per device, a hwdb could still
> > > > > > > > > > work, provided that high-res wheel events would produce a different
> > > > > > > > > > event code than low-res.
> > > > > > > > > > 
> > > > > > > > > > [1] there's a USB message that can be used to query the multiplier,
> > > > > > > > > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > > > > > > > > devices with this feature use the same multiplier.        
> > > > > > > 
> > > > > > > let's not assume that and plan ahead, because sooner or later this will be
> > > > > > > configurable in some device. Probably before we get the first kernel out
> > > > > > > with this patchset in. :)    
> > > > > > 
> > > > > > Yeah, it sounds likely that newer devices may allow to set it.
> > > > > > 
> > > > > > But the actual question here is: how userspace would handle it?
> > > > > > 
> > > > > > When the device is in ratchet mode (e. g. in "discrete" mode), the number
> > > > > > of events received for a single ratchet position movement should be multiple
> > > > > > of the high-res multiplier.
> > > > > > 
> > > > > > For example, MX Anywhere 2 has a fixed resolution (HID++ feature
> > > > > > reports multiplier == 8).
> > > > > > 
> > > > > > On this device, moving the wheel down just one ratchet position,
> > > > > > in low-res mode it produces just one event:
> > > > > > 
> > > > > > URBs:    
> > > > > > >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00    
> > > > > > 
> > > > > > events:
> > > > > > 1490616222.091664: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
> > > > > > 1490616222.091664: event type EV_SYN(0x00).
> > > > > > 
> > > > > > in high-resolution mode, the same movement produces 8 events:
> > > > > > 
> > > > > > URBs:    
> > > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00    
> > > > > 
> > > > > I wonder if in that case, the driver shouldn't convert those into a
> > > > > single REL_WHEEL. The driver knows the state of the ratchet and can
> > > > > detect such situation (and also match if the multiplier is user
> > > > > configurable).  
> > > > 
> > > > IMHO, it shouldn't. While you have the finger at the wheel, you
> > > > can control the speed of the movement. You can also decide you
> > > > don't want to scroll and return to the previous position, like
> > > > on this movement (here, I moved the wheel down, slowly, then
> > > > I returned it back to the original position, on a fast move):
> > > > 
> > > > 000033934 ms 000734 ms (783955 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000034718 ms 000784 ms (119937 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000034838 ms 000120 ms (075936 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000034914 ms 000076 ms (097951 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000035012 ms 000098 ms (071950 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000035084 ms 000072 ms (143879 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000035228 ms 000144 ms (312011 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000035540 ms 000312 ms (2213961 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000037754 ms 002214 ms (075957 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000037830 ms 000076 ms (031940 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000037862 ms 000032 ms (015955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000037878 ms 000016 ms (023917 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000037902 ms 000024 ms (023955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 000037926 ms 000024 ms (029959 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 
> > > > If I was scrolling a screen that would allow scrolling on less than a
> > > > line, I would expect the screen to follow the speed of my finger.  
> > > 
> > > Alright.
> > >  
> > > > > OTOH, if the highres wheel has the correct settings in the hwdb, there
> > > > > is no reasons for libinput to not handle the 8 highres events equal one
> > > > > line given that it already converts the incoming events into physical
> > > > > dimensions.   
> > > > 
> > > > Yes.  
> > 
> > I *think* this should be possible with the current libinput, without even
> > exposing more API. libinput provides a scroll delta for wheels in degrees
> > and a 'discrete' value, it would be fairly trivial to hook up the highres to
> > the degrees only and use the discrete as cumulative. So you'd get a sequence
> > like this:
> >    scroll event (deg 2, discrete 0)
> >    scroll event (deg 2, discrete 0)
> >    scroll event (deg 2, discrete 0)
> >    scroll event (deg 2, discrete 1)
> > 
> > a client that supports this, and I think current clients should basically
> > already support this can pick between normal and hires, without extra code.
> > what the impact of this is, I don't quite know yet.
> 
> Hmm... if I understood well, the idea would be to use something
> similar to udev's MOUSE_WHEEL_CLICK_ANGLE, like[1]:
> 
> 	mouse:usb:v046dp404a:name:Logitech Anywhere Mouse MX 2:
> 	mouse:usb:v046dpc52b:name:Logitech Unifying Device. Wireless PID:404a:
> 	MOUSE_WHEEL_CLICK_ANGLE=16
> 
> [1] I measured the real wheel angle here. In low-resolution, there are 18
> steps at the wheel (in ratchet mode), meaning 20 degrees. It means that, 
> in high-resolution, where multiplier = 8, the minimum angle to produce an
> event would be 2.5 degrees.
> 
> And add a code at libinput similar to this [2]:
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 2a57b25835fe..b5198ca6154d 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1023,6 +1023,24 @@ fallback_process_relative(struct fallback_dispatch *dispatch,
>  			&wheel_degrees,
>  			&discrete);
>  		break;
> +	case REL_HIRES_WHEEL:
> +		fallback_flush_pending_event(dispatch, device, time);
> +		wheel_degrees.y = -1 * e->value *
> +					device->scroll.wheel_click_angle.x / 8;
> +		discrete.y = -1 * e->value;
> +
> +		source = device->scroll.is_tilt.vertical ?
> +				LIBINPUT_POINTER_AXIS_SOURCE_WHEEL_TILT:
> +				LIBINPUT_POINTER_AXIS_SOURCE_WHEEL;
> +
> +		evdev_notify_axis(
> +			device,
> +			time,
> +			AS_MASK(LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL),
> +			source,
> +			&wheel_degrees,
> +			&discrete);
> +		break;
>  	case REL_HWHEEL:
>  		fallback_flush_pending_event(dispatch, device, time);
>  		wheel_degrees.x = e->value *
> 
> [2] I hardcoded there a multiply of 8, e. g. I'm doing:
> 
> 		wheel_degrees.y = -1 * e->value *
> 					device->scroll.wheel_click_angle.x / 8;
> 
> Just to as a quick test code, but, ideally, the multiplier should either
> be obtained via some ioctl or come from some udev property, e. g. either
> a MOUSE_WHEEL_MULTIPLIER or a MOUSE_WHEEL_HIRES_CLICK_ANGLE property.
> 
> I'll do some tests here with the above code, and see if it does what's
> expected.

I modified the Fedora 25 libinput.spec, adding the above patch
(actually, a modified version of the above, as Fedora uses libinput-1.6.3).

This is with low-resolution wheel events (tools/event-debug):

 event4   POINTER_AXIS      +0.75s	vert 15.00* horiz 0.00 (wheel)
 event4   POINTER_AXIS      +0.82s	vert 15.00* horiz 0.00 (wheel)
 event4   POINTER_AXIS      +0.91s	vert 15.00* horiz 0.00 (wheel)
 event4   POINTER_AXIS      +0.99s	vert 15.00* horiz 0.00 (wheel)
 event4   POINTER_AXIS      +1.08s	vert 15.00* horiz 0.00 (wheel)
 event4   POINTER_AXIS      +1.21s	vert 15.00* horiz 0.00 (wheel)
 event4   POINTER_AXIS      +1.62s	vert 15.00* horiz 0.00 (wheel)

And this is with high-resolution events:

 event4   POINTER_AXIS     +63.56s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.57s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.59s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.60s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.63s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.65s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.68s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.71s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.78s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.82s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.88s	vert 1.88* horiz 0.00 (wheel)
 event4   POINTER_AXIS     +63.95s	vert 1.88* horiz 0.00 (wheel)

I confirmed with ./tools/event-gui that the same angular movement
produced the same results at the GUI window. So, it seems
that the above code works.

Yet, scrolling random apps (tested with Firefox, claws-mail, kate)
produced a movement 8 times faster. I tested on both X11 and Wayland.

So, I suspect that applications should be modified too, in order for
them to consider the scroll angle.

Thanks,
Mauro

Re: Support for Logitech MX Anywhere 2

[Mauro Carvalho Chehab]

Em Fri, 31 Mar 2017 14:28:27 +0200
Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:

> On Mar 31 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > Em Fri, 31 Mar 2017 12:03:08 +0200
> > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:
> >   
> > > On Mar 27 2017 or thereabouts, Mauro Carvalho Chehab wrote:  
> > > > Em Mon, 27 Mar 2017 11:38:45 +1000
> > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > >     
> > > > > On Sat, Mar 25, 2017 at 01:02:10PM -0300, Mauro Carvalho Chehab wrote:    
> > > > > > Em Sat, 25 Mar 2017 09:36:18 -0300
> > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > >       
> > > > > > > Em Fri, 24 Mar 2017 06:57:00 -0300
> > > > > > > Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> > > > > > >       
> > > > > > > > Em Fri, 24 Mar 2017 15:22:20 +1000
> > > > > > > > Peter Hutterer <peter.hutterer@who-t.net> escreveu:
> > > > > > > >         
> > > > > > > > > On Thu, Mar 23, 2017 at 02:29:00PM -0300, Mauro Carvalho Chehab wrote:          
> > > > > > > > > > Em Thu, 23 Mar 2017 11:59:56 +0100
> > > > > > > > > > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:            
> > > > > > > > > > > > With regards to ratchet, it probably makes sense to query its state
> > > > > > > > > > > > when the driver starts, as IMHO, it should work on a way similar to
> > > > > > > > > > > > <CAPS LOCK>. Btw, are there any event already defined for ratchet mode?              
> > > > > > > > > > > 
> > > > > > > > > > > There is not. And that's where the problem goes a little bit beyond just
> > > > > > > > > > > enabling the feature, we need to forward this info to userspace.
> > > > > > > > > > > 
> > > > > > > > > > > There should be some EV_SWITCH SW_RATCHET created IMO. The ratchet has
> > > > > > > > > > > a state, and we should be able to forward this state with such a new
> > > > > > > > > > > event.
> > > > > > > > > > > 
> > > > > > > > > > > The thing I am more worried is how can we report the high-res wheel
> > > > > > > > > > > events. I know Peter has a DB of wheel resolution, but in that case, we
> > > > > > > > > > > can switch to high-res or not, so a static hwdb entry won't help.            
> > > > > > > > > > 
> > > > > > > > > > Yes. In the case of high-res, there are actually two ways for those
> > > > > > > > > > events to arrive:
> > > > > > > > > > 
> > > > > > > > > > In HID mode, it produces standard EV_REL / REL_WHEEL events, but there
> > > > > > > > > > isn't any events reporting if the mode changed, nor the event with
> > > > > > > > > > gets wheel axes movement tell if the mouse is in low or high res.
> > > > > > > > > > 
> > > > > > > > > > So, in HID mode, identifying if the wheel is in low or high res
> > > > > > > > > > is not direct.
> > > > > > > > > > 
> > > > > > > > > > When the device is in HID+ mode, the resolution mode comes together with
> > > > > > > > > > axes changes. So, it should be possible to define a different event
> > > > > > > > > > for high-res wheel.
> > > > > > > > > > 
> > > > > > > > > > E. g. if the event reports high-res, it would be generating:
> > > > > > > > > > EV_REL / REL_HI_RES_WHEEL. If the packet arrives with low-res,
> > > > > > > > > > it will keep generating EV_REL / REL_WHEEL.
> > > > > > > > > > 
> > > > > > > > > > This way, Peter's code (libinput, I guess?) could handle it without
> > > > > > > > > > requiring any DB for the devices that allow switching the mode.            
> > > > > > > > > 
> > > > > > > > > sort-of. The main problem with relative axes is that we don't have a
> > > > > > > > > resolution info. The reason we have a hwdb for wheels is that libinput
> > > > > > > > > converts kernel data to physical dimensions so that callers can use the data
> > > > > > > > > in a reliable manner. Switching to a hi-res-wheel just moves the problem
> > > > > > > > > around a bit.
> > > > > > > > > 
> > > > > > > > > Using ABS events simply gives us the resolution in the inital description.
> > > > > > > > > That's (I suspect) the only reason Benjamin suggested it. This isn't the
> > > > > > > > > first time it has come up, it would be interesting to add something like
> > > > > > > > > EVIOCGREL as equivalent to EVIOCGABS and start augmenting rel data with
> > > > > > > > > resolution. But I also suspect that all but this use-case would have the
> > > > > > > > > kernel return a digital shrug anyway, so I'm not sure it's worth the effort.          
> > > > > > > > 
> > > > > > > > I see. Well, at least in the case of the feature supported by this
> > > > > > > > mouse, there are just two possible resolutions: low-res and high-res.
> > > > > > > > The high-res resolution is fixed[1].
> > > > > > > > 
> > > > > > > > As the multiplier has a fixed value per device, a hwdb could still
> > > > > > > > work, provided that high-res wheel events would produce a different
> > > > > > > > event code than low-res.
> > > > > > > > 
> > > > > > > > [1] there's a USB message that can be used to query the multiplier,
> > > > > > > > with is always equal to 8 for MX Anywhere 2. No idea if other
> > > > > > > > devices with this feature use the same multiplier.        
> > > > > 
> > > > > let's not assume that and plan ahead, because sooner or later this will be
> > > > > configurable in some device. Probably before we get the first kernel out
> > > > > with this patchset in. :)    
> > > > 
> > > > Yeah, it sounds likely that newer devices may allow to set it.
> > > > 
> > > > But the actual question here is: how userspace would handle it?
> > > > 
> > > > When the device is in ratchet mode (e. g. in "discrete" mode), the number
> > > > of events received for a single ratchet position movement should be multiple
> > > > of the high-res multiplier.
> > > > 
> > > > For example, MX Anywhere 2 has a fixed resolution (HID++ feature
> > > > reports multiplier == 8).
> > > > 
> > > > On this device, moving the wheel down just one ratchet position,
> > > > in low-res mode it produces just one event:
> > > > 
> > > > URBs:    
> > > > >>> 11 03 0b 00 01 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00    
> > > > 
> > > > events:
> > > > 1490616222.091664: event type EV_REL(0x02): REL_WHEEL (0x0008) value=-1
> > > > 1490616222.091664: event type EV_SYN(0x00).
> > > > 
> > > > in high-resolution mode, the same movement produces 8 events:
> > > > 
> > > > URBs:    
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00    
> > > 
> > > I wonder if in that case, the driver shouldn't convert those into a
> > > single REL_WHEEL. The driver knows the state of the ratchet and can
> > > detect such situation (and also match if the multiplier is user
> > > configurable).  
> > 
> > IMHO, it shouldn't. While you have the finger at the wheel, you
> > can control the speed of the movement. You can also decide you
> > don't want to scroll and return to the previous position, like
> > on this movement (here, I moved the wheel down, slowly, then
> > I returned it back to the original position, on a fast move):
> > 
> > 000033934 ms 000734 ms (783955 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000034718 ms 000784 ms (119937 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000034838 ms 000120 ms (075936 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000034914 ms 000076 ms (097951 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000035012 ms 000098 ms (071950 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000035084 ms 000072 ms (143879 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000035228 ms 000144 ms (312011 us EP=83, Dev=08) >>> 11 03 0b 00 11 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000035540 ms 000312 ms (2213961 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037754 ms 002214 ms (075957 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037830 ms 000076 ms (031940 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037862 ms 000032 ms (015955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037878 ms 000016 ms (023917 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037902 ms 000024 ms (023955 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 000037926 ms 000024 ms (029959 us EP=83, Dev=08) >>> 11 03 0b 00 11 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 
> > If I was scrolling a screen that would allow scrolling on less than a
> > line, I would expect the screen to follow the speed of my finger.  
> 
> Alright.
> 
> >   
> > > OTOH, if the highres wheel has the correct settings in the hwdb, there
> > > is no reasons for libinput to not handle the 8 highres events equal one
> > > line given that it already converts the incoming events into physical
> > > dimensions.   
> > 
> > Yes.
> >   
> > > > 
> > > > events:
> > > > 1490616255.382047: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.382047: event type EV_SYN(0x00).
> > > > 1490616255.434046: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.434046: event type EV_SYN(0x00).
> > > > 1490616255.462060: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.462060: event type EV_SYN(0x00).
> > > > 1490616255.477994: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.477994: event type EV_SYN(0x00).
> > > > 1490616255.502022: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.502022: event type EV_SYN(0x00).
> > > > 1490616255.510016: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.510016: event type EV_SYN(0x00).
> > > > 1490616255.542061: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.542061: event type EV_SYN(0x00).
> > > > 1490616255.584051: event type EV_REL(0x02): REL_HIRES_WHEEL (0x000a) value=-1
> > > > 1490616255.584051: event type EV_SYN(0x00).
> > > > 
> > > > Assuming that the user is, for example, navigating on a browser,
> > > > he would likely be expecting that one ratchet position will scroll
> > > > just one line of text, no matter in what resolution. E. g. the
> > > > scroll delta should be calculated by:
> > > > 
> > > > 	float delta = evdev.value / (multiplier * 1.0)
> > > > 
> > > > ---
> > > > 
> > > > Assuming that, we'll have, on some future, a mouse with a adjustable
> > > > wheel resolution with a multiplier between 1 and 8, when the wheel
> > > > is in free wheel mode, the delta logic could, instead, assume a value
> > > > in the middle of the multiplier range, e. g.:
> > > > 
> > > > 	float delta = evdev.value / 4.0
> > > > 
> > > > This way, changing the wheel resolution would cause the text
> > > > to scroll faster of slower, with would likely be what the
> > > > user wants when changing the wheel resolution and placing the
> > > > wheel in free wheel mode.
> > > >     
> > > > > a hwdb could still work in that case, but it gets quite tricky when it
> > > > > becomes user-configurable. Now you need the user (or supporting software) to
> > > > > drop hwdb in entries which is less than ideal. If there is a value that can
> > > > > be queried from the device, we should figure out how to use it.    
> > > > 
> > > > Querying the value is easy, but we'll need to report it somehow to
> > > > userspace.
> > > > 
> > > > We could report it either via some ioctl that would be enum/query/set
> > > > the wheel resolution (similar to EVIOCGABS/EVIOCSABS) or via some
> > > > event that would be report via read() telling about wheel
> > > > resolution changes.    
> > > 
> > > I know we have user configurable mouse resolutions, but do you really
> > > believe we will have user configurable wheel resolution? That seems a
> > > little bit unlikely to me and if it really happens, it would be a FW
> > > trick to change the scrolling speed (like the user configurable
> > > resolution is a FW trick to change the mouse speed).
> > > 
> > > So I'd say let's focus on a fixed wheel resolution (one for normal
> > > wheel, one for highres) and really start thinking at user configurable
> > > mouse wheel resolution when the device becomes available and that we
> > > have a proper use case for it.  
> > 
> > Works for me.
> >   
> > > > 
> > > > On the latter, we could, for example, create an EV_SW for resolution    
> > > 
> > > Note that EV_SW is used for switches, like the physical switches that
> > > have a state (on-off).  
> > 
> > The ratchet switch is a switch and has a state. Physically, the switch
> > is at the wheel: if you press it, it will switch to ON (ratchet mode).
> > So, if you try move the wheel, you'll feel a resistance the movement.
> > You'll also feel "clicks" on your finger when you scroll.  
> 
> I was referring to EV_SW for resolution change (user configurable
> resolution change). I am fine have an SW_RATCHET with the on/off state.
> However, for resolution change, this basically changes the properties of
> the device, and this is going to be a can of worms.

IMHO, the Kernel driver should not be changing the resolution
automatically. This is something that userspace can do, if they
want.

> 
> > 
> > If you press again, it will switch to the OFF state (free wheel), and
> > there will be no resistance to wheel movements anymore, nor any
> > "clicks" will be felt on your finger.
> >   
> > > > change that would be generated if the resolution of the new event is
> > > > different than the previously reported one. On this device, detecting
> > > > the resolution is trivial, as every events report it, but we have no
> > > > means to know how some other device would implement it.
> > > >     
> > > > > > > What I'm proposing is basically something like what's in the patch
> > > > > > > below (for now, just compile-tested).
> > > > > > > 
> > > > > > > So, for MX Anywhere 2 and MX master, the hid-logitech-hidpp driver should
> > > > > > > switch to the HID++ report mode at device connect and handle the Wheel
> > > > > > > events. If the wheel event is low res, will generate
> > > > > > > REL_WHEEL events. if the wheel is in high resolution, REL_HWHEEL.      
> > > > > > 
> > > > > > Hmm... "H" in HWHEEL is not for "hardware" or "high-res", but,
> > > > > > instead, for "horizontal".      
> > > > > 
> > > > > yep :)
> > > > >     
> > > > > > So, if we'd go for the proposal of using a different event for high-res
> > > > > > vertical wheel, instead of adding a new ioctl (EVIOCGREL as equivalent to
> > > > > > EVIOCGABS), we'll need an extra event.
> > > > > > 
> > > > > > Anyway, the patch below works fine with my mouse. It is against
> > > > > > Kernel 4.10.4.      
> > > > > 
> > > > > quick skim of the patch looks ok, but the big issue with this isn't the
> > > > > technical bit but the policy bit.     
> > > > 
> > > > Yes.
> > > >     
> > > > > So the last hunk adding the event code
> > > > > should accompanied by a Documentation/input/event-codes.txt hunk, explaining
> > > > > the use of the code and how it interacts with the other event codes.
> > > > > You'll probably find that as you write that documentation, more
> > > > > questions will come up.    
> > > > 
> > > > Sure I will document it, but we should first define how we'll report it,
> > > > e. g. as a different event (like proposed on this RFC), via new ioctls or
> > > > via a resolution change event.    
> > > 
> > > I would be happy with the new event. Dmitry would need to validate the
> > > change, but it seems the best choice for me.  
> > 
> > Good! I'll prepare a patchset with the documentation.
> >   
> > > Then, I would suggest
> > > masking the ratchet button and events from the userspace in the driver,
> > > so that the burden of handling the highres wheel doesn't get too
> > > complex.   
> > 
> > Not sure what you're meaning here. Are you meaning that, instead of
> > enabling those events to do something like this at the driver?
> > 
> > 	__clear_bit(REL_HIRES_WHEEL, hrd->input->relbit);
> > 	__clear_bit(SW_RATCHET, hrd->input->swbit);  
> 
> No. You can't change the properties/axis once the device has been
> registered because we don't have a PROP_ event that mentions to clients
> that the device changed. I was referring here to not forwarding the
> ratchet switch to user space, and convert silently in the driver the
> highres wheel events into lowres wheel when ratchet is on. But as
> mentioned above, libinput shouldn't care about the ratchet state as long
> as both highres and lowres are mapped to the same physical dimensions.

IMHO, the best would be to forward the ratchet switch to userspace.
This way, some userspace app could use the switch event to do things
like changing the resolution, if it makes sense for some application.

If not, application can just ignore it.

I'm sending a new patchset. The difference from the one I sent
before is just documentation.

Regards,
Mauro