Bug 144682

I wouldn't know how to influence that - X11 or Qt issue.

X11 surely doesn't do anything "intelligent" with the X/Y axis it simply
reports events :-) I you want you can send us a xev protocol which contains
the button events while scrolling fast/slow

By the way I cannot reproduce this

reproducable in beta3 - and with xev ;-)

I'll attach the xev log.

Created attachment 66418 [details]
xev log (scrolling up and down fast)

Short summary:
# grep button xev-mausrad-scrolling  |sort -u
    state 0x0, button 4, same_screen YES
    state 0x0, button 5, same_screen YES
    state 0x0, button 6, same_screen YES
    state 0x0, button 7, same_screen YES
    state 0x1000, button 5, same_screen YES
    state 0x800, button 4, same_screen YES

Thanks for the log. Matthias could this a bug in the patch you
made for the mouse driver ?

Thanks

Wow.

I doubt that my patch is the culprit, but it is possible.
But be aware that the mouse driver *has* *bugs*.
Lots of them.

I also remember that mouse buttons higher than 5 do not work. I guess these issues are not connected, though. I'll check this out. Have to reproduce this first.

happened again while updating to beta6

Just tried to reproduce (beta 7), but I don't get any other events other than button 4 and 5. Please state the exact model and version of your mouse, so there is a chance we can reproduce this here with the same model.

It's a Logitech Cordless Desktop Pro (Keyboard + Mouse) with an USB receiver. Unfortunately, "Cordless MouseMan Optical", "CE 0682", "[...] L203085" and "S/N LZZ21208317" are the only things that are still readable at the mouse's bottom, the rest is unreadable :-(

My laptop also has a synaptics touchpad (scrolling with it doesn't work while installation - but that's another thing I already reported - bug 104588).

# lsusb
Bus 004 Device 001: ID 0000:0000
Bus 003 Device 002: ID 046d:c505 Logitech, Inc. Cordless Mouse+Keyboard Receiver
Bus 003 Device 001: ID 0000:0000
Bus 002 Device 001: ID 0000:0000
Bus 001 Device 001: ID 0000:0000

/var/log/messages when connecting the USB receiver:
Mar 16 18:04:31 cboltz kernel: usb 3-1: new low speed USB device using uhci_hcd and address 3
Mar 16 18:04:32 cboltz kernel: input: USB HID v1.10 Keyboard [Logitech USB Receiver] on usb-0000:00:1d.1-1
Mar 16 18:04:32 cboltz kernel: input: USB HID v1.10 Mouse [Logitech USB Receiver] on usb-0000:00:1d.1-1

Created attachment 73345 [details]
hwinfo --mouse (from running 10.0 system)

happened again when updating to beta8

... and with RC1

From Zephaniah E. Hull:

Under one variation of a ps2 mouse protocol with wheel, the byte used
for indicating wheel movement is just a signed byte which can show more
then just one step of movement in either direction.

For another common variation this is somewhat different, one value for
moved up one step, another for down one step, one for moving the second
wheel one step, another for moving the second wheel the other direction
one step..

For the first wheel's single step action the protocols are identical.

Created attachment 79316 [details]
Fix for broken mouse wheel event handling

This patch fixes broken mouse wheel event handling, which resulted in the behavior you were experiencing (only with some logitech mice).

Stefan, this is against the modular build, if you have issues applying the patch, I'd gladly help. The mouse driver hasn't changed much in the meantime, though.

Date: Thu, 20 Apr 2006 20:36:59 +0200
From: Matthias Hopf <mhopf@suse.de>
To: xorg@lists.freedesktop.org
Subject: mouse driver update

I've just checked in an update for the mouse driver that should finally
work well with mouse wheels (no more button 6&7 events if moved fast).

So far interpretation of z axis events was either pretty much broken, or
waxis button events were originally used for something so completely
alien to me that I even don't understand it now, right after nuking the code.

The new code should work ok with two wheels as well, except that
mapping the second wheel on a different axis (x or y) won't work -
mapping to buttons should.

I won't fix this right now, as

a) I don't have a two-wheel mouse to test
b) Only the MouseMan+ protocol sets data about the secondary wheel
c) The linux kernel mouse emulation doesn't know anything about multi-wheel
   mice

There is a chance that this checkin breaks some dual-wheel mice that
worked before (because as Zephaniah pointed out there are two
interpretations of the protocol), but I don't have one, and the old
implementation was completely nuts and at the wrong place anyway.

So if you happen to have a mouse with a wheel that worked before and
doesn't with the new driver, send me a notice and we can debug this.

Thanks

Matthias

xorg-x11 package submitted.

I've looked at the issue a few days ago, and I found that this is basically a bug _in the kernel_, and not in x.org, (though the x.org mouse driver has its own bugs).

What I finally came up at the end, are the two patches at:
https://svn.uhulinux.hu/packages/dev/kernel/patches/60-other/exps2.patch
https://svn.uhulinux.hu/packages/dev/x.org-input-mouse/patches/exps2.patch

Sorry for being late, but I did not have time to comment here.

Tomorrow I'll be able to discuss the patches in detail, this is just a quick comment for now because I do not really agree with the above posted patch.

This kernel patch changes the protocol to the second variant Zephaniah pointed out, and it adds dual wheel support to the kernel.
The Xorg patch moves analysis of the EXPS2 protocol to the right place.

Actually, both patches can be added without harm (maybe with a bit tweaking) in addition to my patch. However, I'd like to understand how the two protocol variants can be distinguished.

Reopening for adding at least a configuration option (and some sort of autodetection).

As far as I understand the protocols:

Basically there are 3 variants:
 - basic PS/2, with 3 byte packets, without wheel info, reply to 0xF2 command is "0"
 - Intellimouse PS/2 ("ImPS/2"), with 4 byte packets, wheel info in the 4th byte, reply to the 0xF2 command is "3"
 - Intellimouse Explorer PS/2 ("ExPS/2" as X calls it, or "ImExPS/2" as the kernel calls it), with 4 byte packets, wheel info in the 4th byte, reply to the 0xF2 command is "4"

The difference between the latter to is in the 4th byte:

  bits:  7  6  5  4  3  2  1  0
IMPS/2: z7 z6 z5 z4 z3 z2 z1 z0
EXPS/2:  0  0 b5 b4 z3 z2 z1 z0

0 means: always 0
zN means: Nth bit of wheel info
bN means: button N bit

the IMPS/2 protocol only reports 1 wheel, with a signed 8-bit int (z7..z0)
the EXPS/2 protocol reports the state of 2 wheels:
            z3 z2 z1 z0
 no motion   0  0  0  0
 wheel1 up   0  0  0  1
 wheel1 dn   1  1  1  1
 wheel2 up   0  0  1  0
 wheel2 dn   1  1  1  0


After all, the "autodetection" is already there, because x.org already know by the id to the 0xf2 command whether the other end (kernel/mouse) is speaking imps/2 or exps/2 protocol.
There are two bugs in the current kernel, which my patch addresses:
- it does not properly interpret the wheel info on ps/2 mice (but it does correctly interpret it for usb mice)
- it does not properly speak the exps/2 protocol on the /dev/input/mice mousedev (x.org asks for exps/2, the kernel switches to it, but without the patch, it will still use the imps/2 style 4th byte)


I hope this makes things a bit more clear. I'll be happy to clarify things further to fix this issue perfectly.


ps: I could not really find info about the protocols anywhere on the net. I recall seeing a spec somewhere on microsoft.com a few years ago but I cannot find it now. I've played around with mice quite a lot in the 2.4 era of the kernel, so basicly the above info are the results of my investigations.

Created attachment 79405 [details]
Improved patch

Improved multiwheel handling.

By default no secondary wheel is configured now (ZAxisMapping "4 5" instead of "4 5 6 7"). As soon as 4 buttons are configured for wheels, a different protocol is tried. If this fails (because the mouse has only one wheel and talks a different protocol thus) the driver falls back to single wheel.

Unfortunately, this can be only detected if the wheel is turned really fast. Some horizontal wheel button events may have already been created then. This is as good as we can get, therefore the default is now that only one wheel is present.

It's even worse that we cannot autodetect a secondary wheel, so for these mice the user just *has* to configure ZAxisMapping himself. Maybe Zephaniah has an idea for that I cannot see myself right now.

(In reply to comment #19)
> As far as I understand the protocols:
> 
> Basically there are 3 variants:

Currently, we're only talking about the EXPS2 protocol. Handling of the IMPS2 protocol in the mouse driver was broken, it is fixed by my patch now.

> the IMPS/2 protocol only reports 1 wheel, with a signed 8-bit int (z7..z0)
> the EXPS/2 protocol reports the state of 2 wheels:

As far as I understood Zephaniah there are two interpretations for the wheel byte on EXPS2, this singlebit multiwheel protocol, and a multibit protocol (like the kernel is using right now, with the single wheel data in the lower nibble of byte 3).

> There are two bugs in the current kernel, which my patch addresses:
> - it does not properly interpret the wheel info on ps/2 mice (but it does
> correctly interpret it for usb mice)
> - it does not properly speak the exps/2 protocol on the /dev/input/mice
> mousedev (x.org asks for exps/2, the kernel switches to it, but without the
> patch, it will still use the imps/2 style 4th byte)

Is this patch pending for going upstream? It looks good to me.
As long as the kernel is wrong with its interpretation of the EXPS2 protocol (if it is), we have to deal with it in the mouse driver, so I guess my current solution is as good as we can get it.

> I hope this makes things a bit more clear. I'll be happy to clarify things
> further to fix this issue perfectly.

Thanks :)

> ps: I could not really find info about the protocols anywhere on the net. I

Same for me...

(In reply to comment #21)
> (In reply to comment #19)
> > As far as I understand the protocols:
> > 
> > Basically there are 3 variants:
> 
> Currently, we're only talking about the EXPS2 protocol. Handling of the IMPS2
> protocol in the mouse driver was broken, it is fixed by my patch now.

I know, I just wanted to make things as clear as possible.

> > the IMPS/2 protocol only reports 1 wheel, with a signed 8-bit int (z7..z0)
> > the EXPS/2 protocol reports the state of 2 wheels:
> 
> As far as I understood Zephaniah there are two interpretations for the wheel
> byte on EXPS2, this singlebit multiwheel protocol, and a multibit protocol
> (like the kernel is using right now, with the single wheel data in the lower
> nibble of byte 3).

As far as I understand, the two "interpretations" are exactly the imps/2 and exps/2 protocol. In other words, the exps/2 is perfectly clear and exact on its own, the problem (ie bug) comes only if you try to interpret az exps/2 packet as if it were an imps/2 or the other way around.
Currently (without any patches mentioned here), x.org parses all packets (be it imps/2 or exps/2) as if it were an imps/2 packet. My patch fixes that, and I think it solves what should be solved in x.org. Though, it still needs the fix in the kernel.

> > There are two bugs in the current kernel, which my patch addresses:
> > - it does not properly interpret the wheel info on ps/2 mice (but it does
> > correctly interpret it for usb mice)
> > - it does not properly speak the exps/2 protocol on the /dev/input/mice
> > mousedev (x.org asks for exps/2, the kernel switches to it, but without the
> > patch, it will still use the imps/2 style 4th byte)
> 
> Is this patch pending for going upstream? It looks good to me.

Unfortunately I have not yet submitted it. I wrote it two days ago, its on its way...

> As long as the kernel is wrong with its interpretation of the EXPS2 protocol
> (if it is), we have to deal with it in the mouse driver, so I guess my current
> solution is as good as we can get it.

Please, please, do not put workarounds in x.org for a buggy kernel. Fix the kernel.

(In reply to comment #22)
> > Currently, we're only talking about the EXPS2 protocol. Handling of the IMPS2
> > protocol in the mouse driver was broken, it is fixed by my patch now.
> I know, I just wanted to make things as clear as possible.

:)

> > As far as I understood Zephaniah there are two interpretations for the wheel
> > byte on EXPS2, this singlebit multiwheel protocol, and a multibit protocol
> > (like the kernel is using right now, with the single wheel data in the lower
> > nibble of byte 3).
> 
> As far as I understand, the two "interpretations" are exactly the imps/2 and
> exps/2 protocol. In other words, the exps/2 is perfectly clear and exact on its
> own, the problem (ie bug) comes only if you try to interpret az exps/2 packet
> as if it were an imps/2 or the other way around.

Not exactly. IMPS2 always uses the full byte, this variant of EXPS2 uses only the lower nibble. Because this is different, it could well be that there are mice which talk this variant.

> Currently (without any patches mentioned here), x.org parses all packets (be it
> imps/2 or exps/2) as if it were an imps/2 packet. My patch fixes that, and I

Actually, no, they were interpreted (somehow) correctly and differently, but this information was misinterpreted later (completely broken). Actually, your patch didn't work completely right with IMPS2 (we need multiple events for ABS(dz)>1), but now everything should be fixed.

> > > - it does not properly speak the exps/2 protocol on the /dev/input/mice
> > > mousedev (x.org asks for exps/2, the kernel switches to it, but without the
> > > patch, it will still use the imps/2 style 4th byte)
> > 
> > Is this patch pending for going upstream? It looks good to me.
> 
> Unfortunately I have not yet submitted it. I wrote it two days ago, its on its
> way...

Good to know!

> > As long as the kernel is wrong with its interpretation of the EXPS2 protocol
> > (if it is), we have to deal with it in the mouse driver, so I guess my current
> > solution is as good as we can get it.
> 
> Please, please, do not put workarounds in x.org for a buggy kernel. Fix the
> kernel.

We can't. Given the long time the broken kernel implementation was in place (and still is, as kernel patches tend to be discussed rather long), we cannot ignore this huge customer base.
In this very case, the workaround is trivial, and as soon as the corrected kernel interface is widespread we can just change the default ZAxisMapping to "4 5 6 7" and everything will work. With only very minor glitches to those who still have the old kernel interface.

CCing Vojtech so he can comment on the kernel patch.

(In reply to comment #23)
> > > As far as I understood Zephaniah there are two interpretations for the wheel
> > > byte on EXPS2, this singlebit multiwheel protocol, and a multibit protocol
> > > (like the kernel is using right now, with the single wheel data in the lower
> > > nibble of byte 3).
> > 
> > As far as I understand, the two "interpretations" are exactly the imps/2 and
> > exps/2 protocol. In other words, the exps/2 is perfectly clear and exact on its
> > own, the problem (ie bug) comes only if you try to interpret az exps/2 packet
> > as if it were an imps/2 or the other way around.
> 
> Not exactly. IMPS2 always uses the full byte, this variant of EXPS2 uses only
> the lower nibble. Because this is different, it could well be that there are
> mice which talk this variant.

As far as I can tell (based on infos around the net and my own investigations on various mice):
 - all mice which support the exps/2 protocol (that is, report id "4" after setting res to 200, 200, 80), also support the imps/2 protocol (that is, report id "3" after setting res to 200, 100, 80).
 - if a mouse is switched into exps/2 (setting res to 200, 200, 80), it _wont_ report +-2 for the (fast) motion of the vertical wheel, only for the motion of the horizontal (second) wheel (if it has it).
 - if a mouse is switched into imps/2 (setting res to 200, 100, 80), it wont report motions of the horizontal (second) wheel, only for the vertical (first) wheel, but in this case it can be as large as +-127.

(In reply to comment #25)
> As far as I can tell (based on infos around the net and my own investigations
> on various mice):
>  - all mice which support the exps/2 protocol (that is, report id "4" after
> setting res to 200, 200, 80), also support the imps/2 protocol (that is, report
> id "3" after setting res to 200, 100, 80).

Right, but that doesn't help.

>  - if a mouse is switched into exps/2 (setting res to 200, 200, 80), it _wont_
> report +-2 for the (fast) motion of the vertical wheel, only for the motion of
> the horizontal (second) wheel (if it has it).

The current linux kernel mouse emulation does, and that counts enough so we cannot neglegt it.

>  - if a mouse is switched into imps/2 (setting res to 200, 100, 80), it wont
> report motions of the horizontal (second) wheel, only for the vertical (first)
> wheel, but in this case it can be as large as +-127.

Still, this doesn't help :(

Also, there still seems to be a 'bug' (you may call it feature) lingering in the Xorg mouse driver, because it assumes only values -7 to +7 for IMPS2. If values outside this range are received, the driver switches to EXPS2 protocol. I assume that this detection works reasonably well (given there are no bug reports) and was necessary for some broken mice, so I won't change it.

xorg-x11 package with new patch submitted for STABLE. I let this one open for furter discussion.

I believe a better solution would be to enhance the kernel interface to support
horizontal scrolling with the EXPS2 protocol. I can make a patch for that,
but I could use the exact layout of the 4th byte for that.

(In reply to comment #28)
> I believe a better solution would be to enhance the kernel interface to support
> horizontal scrolling with the EXPS2 protocol. I can make a patch for that,
> but I could use the exact layout of the 4th byte for that.

Vojtech: I've already done that, see https://svn.uhulinux.hu/packages/dev/kernel/patches/60-other/exps2.patch

See comment #19 for the explanation.

To me the part about mousedev.c in the kernel patch of comment #17 seems to be exactly what you're looking for.

It is independent of the fix in psmouse-base.c, which seems reasonable, but I don't have enough experience here (and it definitively has to be tested against various mice).

The fix will work with the updated driver in Xorg's CVS (and upcoming SL10.1rc3) out-of-the-box. Dual wheel has to be configured (ZAxisMapping "4 5 6 7"), but the default can be changed to this as soon as the updated kernel driver is widely used.

Thanks for pointing me to the patches. I see some problems there:

The use of "%2" and "/2" looks strange. I have a mouse (Microsoft IntelliMouse Explorer v3.0) that DOES report wheel motion at least up to +-6 in ExPS/2 mode.
The updated statement will cause very strange behavior with values larger than
2. And even for mice only sending +-1 for vertical and +-2 for horizontal scrolling it doesn't seem to be the most sensible way to program that bit.

I believe we're missing a significant part of the protocol here.

Another problem is the addition of "| BIT(REL_HWHEEL)" as it is done to the probe table doesn't make sense and will keep keyboards with scrollwheels from working. 

Also the way of encoding the (h)wheel data into a packet in mousedev.c looks
like it could be expressed in somewhat less code.

(In reply to comment #31)
> The use of "%2" and "/2" looks strange.

That was just a shorthand, for the table below.

 I have a mouse (Microsoft IntelliMouse
> Explorer v3.0) that DOES report wheel motion at least up to +-6 in ExPS/2 mode.
> The updated statement will cause very strange behavior with values larger than
> 2. And even for mice only sending +-1 for vertical and +-2 for horizontal
> scrolling it doesn't seem to be the most sensible way to program that bit.

As far as I knew, in exps/2 the lower 4 bits of the 4th byte in the packet can be:
 b3 b2 b1 b0
  0  0  0  0  no motion
  0  0  0  1  wheel1 up
  0  0  1  0  wheel2 up
  1  1  1  1  wheel1 down
  1  1  1  0  wheel2 down

Is this true for your mouse?


> Another problem is the addition of "| BIT(REL_HWHEEL)" as it is done to the
> probe table doesn't make sense and will keep keyboards with scrollwheels from
> working. 

Maybe I did not fully understood the code there. Lets make things clear about the protocol first :)

> Also the way of encoding the (h)wheel data into a packet in mousedev.c looks
> like it could be expressed in somewhat less code.

I just wanted to make it clear and not look ugly like the "%2" "/2" part :)

Lets focus on tje protocol first:

No, it is not true for my mouse. It has only one wheel, and still uses the
ExplorerPS/2 protocol (being an Explorer v3.0 mouse). And it uses the whole
4th byte for the wheel axis, like an IMPS/2 mouse would. It has and reports
5 buttons.

I have also tested this with Microsoft Wireless Intellimouse Explorer v2.0,
a rather modern Explorer mouse. If initalized as ExplorerPS/2, it behaves
exactly the same way as my other Explorer mouse, and doesn't even send any
data for the horizontal scrollwheel. It will report wheel data in the 4th
byte with values up to +-5 (I didn't manage to scroll it faster, but setting
a lower report rate would allow me to go even higher).

So there must be some other magic sequence to probe and enable the wheel.

So are you basically saying that imps/2 (id=3) and exps/2 (id=4) are both reporting only 1 wheel, and the difference is only that the latter reports button4/5?

I'll retest the mice I get my hands on, but I wont be able to do it today.

Exactly, that's what I'm saying. At least with the initialization sequence
in the 2.6 kernel.

I have one mouse, from A4Tech, with two wheels that would work with your patch,
always reporting 1 for horizontal and 2 for vertical wheel, but that's the
only one that behaves like this. with the id=4 sequence.

I have checked a few mice, and found the following:

- you were right Vojtech, mice in exps/2 (id=4) are happily reporting even +-7 of wheel motion
- a4tech mice seem to behave as I explained: +-1 for wheel1 and +-2 for wheel2, no matter how fast you are scrolling. I have tested an A4Tech WOP-35 model, which has 2 _vertical_ wheels, but is think other a4tech mice speak the same protocol. (*)


So basically the problem is, that if the kernel/X receives 0xe or 0x2 as the wheel data, it does not know if it means "wheel1 twice" or "wheel2".

Currently, the kernel interprets 0xe/0x2 as "wheel1 twice", while x.org interprets it as "wheel2".

Do you have any idea how to solve this? (How could this be autodetected?)


(*)
Links that support this:
- the commit log of xfree86 mouse driver: http://cvsweb.xfree86.org/cvsweb/xc/programs/Xserver/hw/xfree86/input/mouse/mouse.c.diff?r1=1.24&r2=1.25
- the thread at: http://lists.debian.org/debian-x/2002/10/msg00445.html
- a very old patch for gpm: http://linux.schottelius.org/cgi-bin/gitweb.cgi?p=gpm;a=blob;h=fbc263f816b400661a3909866547420ea5e2cc23;hb=02ca148a4a301063ecfe588276a4610c4407ee19;f=patches/todo/imwheel/mouse_wheel.patch

Since A4tech mice are the exception and not the norm (they're far from being
the most common brand of mice), I think we should ignore them for the kernel
<-> XFree part. 

I've analyzed A4tech dual-wheel mice before - the USB variety is supported by the kernel using quirks (A4tech didn't use the regular hwheel HID HUT usage). The PS/2 version is a problem, because it can't be probed for. It behaves exactly as a standard ImPS/2 (or ExPS/2) mouse, except for the wheel anomaly. The only way to really support these both in the kernel and X is to add an option to enable this kind of dual-wheel support.

Now A4tech aren't the only dual-wheel PS/2 mice on the planet. There is Logitech supported by the PS2++ driver in the kernel, and there is Microsoft mice with their tilt wheel, currently unsupported in the kernel. 

If you have any info about the PS/2 protocol that the MS tilt wheel mice use, that would be greatly appreciated.

And that would also be the protocol of choice for mousedev.c, should we want to
teach it about the horizontal wheel.

So what I would propose:

    1) Add an option (posibly psmouse.proto=a4tech) to psmouse, to enable
       their (h)wheel decoding.
    2) Remove support for h-wheel in X, as it currently stands, putting it
       behind a similar option that would be disabled by default.
    3) Add support to both the kernel and X to autodetect MS tilt wheel mice.
    4) Possibly add tilt wheel emulation to mousedev.c
    5) Use the evdev protocol in X anyway, since that one doesn't have
       problems with h/v wheels.

Vojtech

No need for an additional option in the X mouse driver. The new driver uses the dual wheel protocol, if it is configured (ZAxisMapping "b1 b2 b3 b4"), and the single wheel protocol if not. It also falls back to single wheel protocol, if it receives pakets that are illegal in the dual wheel protocol.

Using the evdev mouse driver is something to be evaluated for further releases. The driver already exists.

Well, the real MS Explorer protocol now has support for the horizontal wheel
on MS mice and it's implemented differently from A4tech. I believe X should
support the MS one by default with "ExplorerPS/2" as the protocol, in the case
"ZAxisMapping" is set. Then you need an option for A4tech mice.

So what's the difference for these two mice? So far I'm only aware of a single protocol for two wheel mice.

I prefer having a configuration by default for which one wheel always works to one where two wheel mice work and one wheel mice don't do perfectly. Again, as soon as the kernel patch is upstream and widely spread (next SL?), we can change the default and be all happy. If there turns out that there are even more protocol variants, we need another Xorg option, though.

Voytech, I'm assigning this to you now as the main thing to be debated is the kernel's mice emulation.

Is the problem seen with 10.2 as well?

Egbert, JFYI. Since Matthias or me is in Cc of this bugreport or the reported itself, it might be interesting for you as well.

This is bogus. MouseReadInput() is for low level event reading.
Mapping details don't belong there but should all be handled in the
PostEvent handling.
I would like to separeate all these post event handlings to 
make it available to other mice drivers. Therefore it needs to
be kept away from the low level event reading code. 

I took pains to separate those things once.

Please fix.

@@ -1506,7 +1503,21 @@ MouseReadInput(InputInfoPtr pInfo)
 		      (pBuf[3] & 0x20) >> 1;        /* button 5 */
 	    dx = (pBuf[0] & 0x10) ?    pBuf[1]-256  :  pBuf[1];
 	    dy = (pBuf[0] & 0x20) ?  -(pBuf[2]-256) : -pBuf[2];
-	    dz = (pBuf[3] & 0x08) ? (pBuf[3] & 0x0f) - 16 : (pBuf[3] & 0x0f);
+	    if (pMse->negativeW != MSE_NOAXISMAP) {
+		switch (pBuf[3] & 0x0f) {
+		case 0x00:          break;
+		case 0x01: dz =  1; break;
+		case 0x02: dw =  1; break;
+		case 0x0e: dw = -1; break;
+		case 0x0f: dz = -1; break;
+		default:
+		    xf86Msg(X_INFO,
+			    "Mouse autoprobe: Disabling secondary wheel\n");
+		    pMse->negativeW = pMse->positiveW = MSE_NOAXISMAP;
+		}
+	    }
+	    if (pMse->negativeW == MSE_NOAXISMAP)
+	        dz = (pBuf[3]&0x08) ? (pBuf[3]&0x0f) - 16 : (pBuf[3]&0x0f);

Originally the driver read data from the PS/2 interface directly. Here we should have had two different protocols to distinguish. Now since the kernel deals with the different mice protocols internally it should also normalize this zaxis/button behavior.

> Currently, the kernel interprets 0xe/0x2 as "wheel1 twice", while x.org
> interprets it as "wheel2".
> 
I don't see this in the EXPPS2 protocol handling.
The patch however contains:
     else if (dw > 0 || dz > 1)
  	             zbutton = pMse->positiveW;
The dz > 1 seems to try to take the A4Tech protocol into account (in the wrong place, though).
 Get rid of the insane
 if (dw < 0 || dz < -1)
               ^^^^^^^
and   else if (dw > 0 || dz > 1)
                        ^^^^^^^^
seems to have fixed the original bug report in the first place.

I wonder if the new ExplorerPS/2 mice use the two upper bits to signal the second wheel. If so it'd be easy to implement - hoping that all other mice un the planet using this protocol are sane enough to always set these bits to 0 instead of letting them float.

Do we really want to  map fast mouse wheel movements to a series of up/down events unconditionally? If a user does want to use the wheel for button events this could trigger unexpected behavior. This should at least be configurable.
Mapping wheel events to buttons which later on (in xkb) get remapped to key events: welcome to KludgeWorld!

We are moving away from this mouse driver and to evdev anyhow.
I'd suggest to do the following:
1. We ignore the strange behavior of A4Tech PS/2 mice in the EXPS2 protocol. The worst thing that can happen is that when the user scrolls the second wheel, the movements are treated as faster wheel 1 movements.
2. Just for the sake of completeness add a new protocol or option for the A4Tech mice to the X driver to account for the A4Tech quirk. This will be irrelevant for Linux users unless they still use 2.4 or lower kernels. A heuristic can be put in place to detect that someone has erronously set this option - similar to Matthias' fallback. 
3. Add support for the second wheel of the newer MS ExplorerPS/2 mice once we know the details of their protocol. If the kernel interface that speaks EXPPS2 to X supports this also we can savely use this driver to support 2 wheel mice.
4. Only allow for multiple button events when wheels are mapped to buttons 4-7 - the ones that are used by xkb to send the scroll key events. Also add an option to turn this off.

Assigning this to myself.

> I'd suggest to do the following:
> 1. We ignore the strange behavior of A4Tech PS/2 mice in the EXPS2 protocol.
> The worst thing that can happen is that when the user scrolls the second wheel,
> the movements are treated as faster wheel 1 movements.

Which is exactly what happens. I just confirmed that as I'm the 'proud' owner of such a device. 

(In reply to comment #48)
> Do we really want to  map fast mouse wheel movements to a series of up/down
> events unconditionally? If a user does want to use the wheel for button
> events this could trigger unexpected behavior.

Huh? Who would expect that turning the mouse wheel fast would do something else than turning the wheel slowly? The average user just expects different scrolling speed (obviously), nothing else.


> This should at least be configurable.

If you really want this, you should call it _K_onfigurable ;-))
(opposed to non-confi_G_urable) *SCNR*

Egbert, 

the Linux Kernel already has support for th H-Wheel on the IntelliMouse explorer,
just look at the psmouse-base.c source file. It doesn't have A4Tech mouse support for PS/2 mice (only for USB, which A4Tech also screwed awfully).

If there is support for receiving ImEXPS/2 H-Wheel data in X, mousedev.c in the kernel might make sense to be extended to support that.

Anyway, I'll take a look at adding support for A4Tech PS/2 mice via an extra parameter to the kernel in psmouse-base.c.

Vojtech

(In reply to comment #52)
Voitech,
to avoid getting me confused: so far we were talking about 
IMPS/2 mice which can handle 3 buttons and 1 wheel
EXPS/2 mice which can handle 5(?) buttons and 1 wheel.
If I understood you correctly, now MS has extended the EXPS/2 protocol to also handle 2 wheels. 
What is now ImEXPS/2? The EXPS/2 protocol extended to 2 wheels? I would like to support this in the Xserver i just need to know the protocol. I'll check the kernel for that. If it's in there I'll find it and add it to X.

> Egbert, 
> 
> the Linux Kernel already has support for th H-Wheel on the IntelliMouse
> explorer,
> just look at the psmouse-base.c source file. It doesn't have A4Tech mouse
> support for PS/2 mice (only for USB, which A4Tech also screwed awfully).
> 
A4Tech is special anyway. I assume the extra bits there are used to signal the 2 extra buttons on this mouse. If you don't have this thing I can extend the kerrnel driver for this. (provided you let me dump my patch on you - not having to go thru lkml hell).


> If there is support for receiving ImEXPS/2 H-Wheel data in X, mousedev.c in the
> kernel might make sense to be extended to support that.
> 
> Anyway, I'll take a look at adding support for A4Tech PS/2 mice via an extra
> parameter to the kernel in psmouse-base.c.
> 
Vojtech, Let me look into this since I've got such a beast.

Egbert,

First of all, please have a look at these commits:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b0c9ad8e0ff154f8c4730b8c4383f49b846c97c4
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=90414be9523208f0b667fd58c22e26b8db0594de

ImEXPS/2 is the same protocol which x.org calls EXPS/2.

After switching the mouse to the EXPS/2 protocol, the 2-wheel mode can be enabled by sending the bytes 200, 80, 40. In this mode, special packets are sent by the mouse when the user scrolls the second wheel. (This is a backwards-compatible protocol.)

Unfortunately A4Tech mice do not support this protocol. What they do is that they support EXPS/2 mode and send +/-1 scroll up/down for scroll up/down and send +/-2 scroll up/down for scroll left/right. This cannot be autodetected to my knowledge.

(In reply to comment #44)
> This is bogus. MouseReadInput() is for low level event reading.

Yes.

> Mapping details don't belong there but should all be handled in the
> PostEvent handling.

IMHO this is not mapping, but how the protocol is specified to report two axes.

> I would like to separeate all these post event handlings to 
> make it available to other mice drivers. Therefore it needs to
> be kept away from the low level event reading code. 

It doesn't make sense to provide this "mapping" to other mice drivers which speak another protocol.


(In reply to comment #46)
> Originally the driver read data from the PS/2 interface directly. Here we
> should have had two different protocols to distinguish. Now since the kernel
> deals with the different mice protocols internally it should also normalize
> this zaxis/button behavior.

Yes, and this is about to change, or has already changed. So we will have to deal with both protocol variants for some time, even in /dev/input/mice emulation mode.

That there are two variants of this protocol hasn't been clear until recently, and it's IMHO quite broken - as they cannot be distinguished from each other, or only in one direction and only on some (few) events.

Actually, the whole concept that's currently used is broken, having the kernel to emulate a serial mouse over /dev/input/mice, and why on earth has such a broken protocol been selected? It also only allows for 5 buttons... I don't want to sound ungrateful, though, X has neglected input hotplugging for too long...


(In reply to comment #48)
> > Currently, the kernel interprets 0xe/0x2 as "wheel1 twice", while x.org
> > interprets it as "wheel2".

Which is wrong (read comment #39). Xorg only interprets as wheel2 if told to do so.

> I don't see this in the EXPPS2 protocol handling.

+               case 0x02: dw =  1; break;
+               case 0x0e: dw = -1; break;

> The patch however contains:
>      else if (dw > 0 || dz > 1)
>                      zbutton = pMse->positiveW;

No, it *removes* this. This code was IMHO dead ugly, and probably wrong.

>  Get rid of the insane
>  if (dw < 0 || dz < -1)
>                ^^^^^^^

Agreed.

> I wonder if the new ExplorerPS/2 mice use the two upper bits to signal the
> second wheel. If so it'd be easy to implement - hoping that all other mice un

Read comment #32 and comment #33.

> the planet using this protocol are sane enough to always set these bits to 0
> instead of letting them float.

No, they are not. They use it for multi wheel events.

> Do we really want to  map fast mouse wheel movements to a series of up/down
> events unconditionally? If a user does want to use the wheel for button events

? Wheel events are typically always mapped to button 4&5 events...
What else do you want to map mouse wheel movements to? Remember, with some explorer mice you even get a series of 1 wheel move events for a fast wheel movement, for some you get larger ones.

One sane idea would probably to leave these wheel events axis events, but I assume the mouse driver doesn't allow for more than two axes right now.

> We are moving away from this mouse driver and to evdev anyhow.

Right. That's why I wouldn't invest too much here.

> I'd suggest to do the following:
> 1. We ignore the strange behavior of A4Tech PS/2 mice in the EXPS2 protocol.

I only wanted to know the difference first. Given the description of Balazs, it seems it talks exactly the protocol that's implemented now in the Xorg mouse driver, but the kernel cannot detect that and thus emulates the new ExplorerPS/2 mapping with 2 wheels, but consolidates the W wheel events to 2 z wheel events. Apparently, nobody implemented the regular 2-wheel mode protocol in the Xorg mouse driver so far.


> 2. Just for the sake of completeness add a new protocol or option for the
> A4Tech mice to the X driver to account for the A4Tech quirk. This will be
> irrelevant for Linux users unless they still use 2.4 or lower kernels. A

ZAxisMapping "4 5 6 7" should do exactly that. When not using /dev/input/mice.

(In reply to comment #55)
> 
> > Mapping details don't belong there but should all be handled in the
> > PostEvent handling.
> 
> IMHO this is not mapping, but how the protocol is specified to report two axes.
I was referring to pMse->negativeW != MSE_NOAXISMAP, this is mapping.
This is a kludge for broken mice protocols.
And we try to derive if the user has this broken mouse from if he has configured a second wheel on an EXPS2.
This is ugly. Besides I don't want to use pMse->negativeW in the low level input function. 
> 
> > I would like to separeate all these post event handlings to 
> > make it available to other mice drivers. Therefore it needs to
> > be kept away from the low level event reading code. 
> 
> It doesn't make sense to provide this "mapping" to other mice drivers which
> speak another protocol.
> 
Right. Because we  are confusing mappings and protocols here.
The post mouse event handling, all the 'gestures' and whatever people have added to the mouse driver to rotate coordiantes etc. is unrelated to the protocol.

> 
> (In reply to comment #46)
> > Originally the driver read data from the PS/2 interface directly. Here we
> > should have had two different protocols to distinguish. Now since the kernel
> > deals with the different mice protocols internally it should also normalize
> > this zaxis/button behavior.
> 
> Yes, and this is about to change, or has already changed. So we will have to
> deal with both protocol variants for some time, even in /dev/input/mice
> emulation mode.
> 
> That there are two variants of this protocol hasn't been clear until recently,
> and it's IMHO quite broken - as they cannot be distinguished from each other,
> or only in one direction and only on some (few) events.

The only  broke variant seems to be the a4tech. Here the manufactuerer should have chosen a different protocol instead of reinterpreting an existing one.
Really bad choice.

> 
> Actually, the whole concept that's currently used is broken, having the kernel
> to emulate a serial mouse over /dev/input/mice, and why on earth has such a
> broken protocol been selected? It also only allows for 5 buttons... I don't
> want to sound ungrateful, though, X has neglected input hotplugging for too
> long...

Exps2 isn't really broken. Some mice are.
Other manufactueres are at least nice enough to add an extension the the
EXPS2 protocol.

> 
> 
> (In reply to comment #48)
> > > Currently, the kernel interprets 0xe/0x2 as "wheel1 twice", while x.org
> > > interprets it as "wheel2".
> 
> Which is wrong (read comment #39). Xorg only interprets as wheel2 if told to do
> so.

Right. This happens in the post processing code. I already said that this is
completely insane.
Moving it up to the low level input layer was the right choice. 

> 
> > I don't see this in the EXPPS2 protocol handling.
> 
> +               case 0x02: dw =  1; break;
> +               case 0x0e: dw = -1; break;

Right. That's what you've added.

> 
> > The patch however contains:
> >      else if (dw > 0 || dz > 1)
> >                      zbutton = pMse->positiveW;
> 
> No, it *removes* this. This code was IMHO dead ugly, and probably wrong.

Sorry, i was referring to the patch that was pointed to by the first link in comment #37 
> 
> >  Get rid of the insane
> >  if (dw < 0 || dz < -1)
> >                ^^^^^^^
> 
> Agreed.
> 
> > I wonder if the new ExplorerPS/2 mice use the two upper bits to signal the
> > second wheel. If so it'd be easy to implement - hoping that all other mice un
> 
> Read comment #32 and comment #33.

Again I was referring to the extended EXPS/2 protocol Vojtech had mentioned.
This isn't discussed in comment #32 and #33.
> 
> > the planet using this protocol are sane enough to always set these bits to 0
> > instead of letting them float.
> 
> No, they are not. They use it for multi wheel events.

Again I was referring to the upper two bits:
EXPS/2:  0  0 b5 b4 z3 z2 z1 z0
These are masked out in the current driver.

> 
> > Do we really want to  map fast mouse wheel movements to a series of up/down
> > events unconditionally? If a user does want to use the wheel for button events
> 
> ? Wheel events are typically always mapped to button 4&5 events...
> What else do you want to map mouse wheel movements to? Remember, with some
> explorer mice you even get a series of 1 wheel move events for a fast wheel
> movement, for some you get larger ones.

Right. But you may not always want to map buttons to wheel events. This
is configurable in xkb.

> 
> One sane idea would probably to leave these wheel events axis events, but I
> assume the mouse driver doesn't allow for more than two axes right now.

Right. Kludge around the core protocol limitations.

> 
> > We are moving away from this mouse driver and to evdev anyhow.
> 
> Right. That's why I wouldn't invest too much here.

I won't.

> 
> > I'd suggest to do the following:
> > 1. We ignore the strange behavior of A4Tech PS/2 mice in the EXPS2 protocol.
> 
> I only wanted to know the difference first. Given the description of Balazs, it
> seems it talks exactly the protocol that's implemented now in the Xorg mouse
> driver, but the kernel cannot detect that and thus emulates the new
> ExplorerPS/2 mapping with 2 wheels, but consolidates the W wheel events to 2 z
> wheel events. Apparently, nobody implemented the regular 2-wheel mode protocol
> in the Xorg mouse driver so far.

Right. That's what I will do.

> 
> 
> > 2. Just for the sake of completeness add a new protocol or option for the
> > A4Tech mice to the X driver to account for the A4Tech quirk. This will be
> > irrelevant for Linux users unless they still use 2.4 or lower kernels. A
> 
> ZAxisMapping "4 5 6 7" should do exactly that. When not using /dev/input/mice.
> 
So we assume standard EXPS2 && ZAxisMapping == user has a broken a4tech.
At least it seems to be the assumption that was made before. I will try to accomodate this somehow. 

(In reply to comment #54)
> Egbert,
> 
> First of all, please have a look at these commits:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b0c9ad8e0ff154f8c4730b8c4383f49b846c97c4
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=90414be9523208f0b667fd58c22e26b8db0594de
> 
> ImEXPS/2 is the same protocol which x.org calls EXPS/2.
> 
> After switching the mouse to the EXPS/2 protocol, the 2-wheel mode can be
> enabled by sending the bytes 200, 80, 40. In this mode, special packets are
> sent by the mouse when the user scrolls the second wheel. (This is a
> backwards-compatible protocol.)
> 

OK. I will try to implement this.

> Unfortunately A4Tech mice do not support this protocol. What they do is that
> they support EXPS/2 mode and send +/-1 scroll up/down for scroll up/down and
> send +/-2 scroll up/down for scroll left/right. This cannot be autodetected to
> my knowledge.
> 
Right, i understand this. This was what was implemented in X using a horrific kludge which used to mess up the handling for all other mice.

(In reply to comment #56)
> > IMHO this is not mapping, but how the protocol is specified to report two axes.
> I was referring to pMse->negativeW != MSE_NOAXISMAP, this is mapping.

Arguably.
I don't think so. It might look like mapping, but in fact it is disassembling wrong (broken) z axis information into z and w axis information. The code could look different, first checking this "flag", then depending on the outcome interpret the protocol. That would be duplicate code, though, and I wanted to change as little as possible in this mess.
In this particular part of the code, wheels are actually wheels, and not buttons yet. It's only the "flag" testing code, that is - well - dubious.

> This is a kludge for broken mice protocols.

Agreed.

> And we try to derive if the user has this broken mouse from if he has
> configured a second wheel on an EXPS2.
> This is ugly. Besides I don't want to use pMse->negativeW in the low level
> input function. 

Ok, you could implement a different mouse protocol, and call it ExplorerPS/2/With2Wheels or something like that. I don't think that would be more transparent, because this would result in duplicate code.

> The post mouse event handling, all the 'gestures' and whatever people have
> added to the mouse driver to rotate coordiantes etc. is unrelated to the
> protocol.

I 100% agree! All this wheel to mouse button handling is horribly broken in the code. I actually believe that this should belong in *any* mouse driver at all. This should be outside the driver code.

> The only  broke variant seems to be the a4tech. Here the manufactuerer should
> have chosen a different protocol instead of reinterpreting an existing one.
> Really bad choice.

Yes. I didn't know about the correct protocol until right now. If that was known to me before, I had probably never introduced this broken protocol as a possibility. But upstream apparently nobody had complete information.

> Exps2 isn't really broken. Some mice are.
> Other manufactueres are at least nice enough to add an extension the the
> EXPS2 protocol.

Ok, I meant the original ExPS2. W/o extensions.
Does the kernel mouse emulation include code for >5 button protocol extensions?

> > > The patch however contains:
> > >      else if (dw > 0 || dz > 1)
> > >                      zbutton = pMse->positiveW;
> > 
> > No, it *removes* this. This code was IMHO dead ugly, and probably wrong.
> 
> Sorry, i was referring to the patch that was pointed to by the first link in
> comment #37 

Ah! Yes, you're right.

> > Read comment #32 and comment #33.
> 
> Again I was referring to the extended EXPS/2 protocol Vojtech had mentioned.
> This isn't discussed in comment #32 and #33.

My bad. Misunderstood your comment.

> > One sane idea would probably to leave these wheel events axis events, but I
> > assume the mouse driver doesn't allow for more than two axes right now.
> 
> Right. Kludge around the core protocol limitations.

... Nothing to say :-/

> > wheel events. Apparently, nobody implemented the regular 2-wheel mode protocol
> > in the Xorg mouse driver so far.
> 
> Right. That's what I will do.

Great! Together with the default ZAxisMapping "4 5" this should work out-of-the-box for *all* mice when Xorg uses /dev/input/mice, when the kernel has implemented the correct two-wheel protocol bits. As soon as you send the escape sequence it should send you the extended protocol data.

The only exception is this brain-dead mouse, which needs manual intervention. Either configure /dev/psaus with ZAxisMapping "4 5 6 7", or use /dev/input/mice as before and tell the kernel to use the a4tech protocol.

> So we assume standard EXPS2 && ZAxisMapping == user has a broken a4tech.

Right now, yes. I assume it wouldn't help the code quality too much if you change this to another protocol or use an additional option.



(In reply to comment #57)
> Right, i understand this. This was what was implemented in X using a horrific
> kludge which used to mess up the handling for all other mice.

Well, it's not *that* bad. Standard behavior is just like before. Only if ZAxisMapping is manually set, behavior changes.

> 
> (In reply to comment #57)
> > Right, i understand this. This was what was implemented in X using a horrific
> > kludge which used to mess up the handling for all other mice.
> 
> Well, it's not *that* bad. Standard behavior is just like before. Only if
> ZAxisMapping is manually set, behavior changes.
> 

Matthias, I was not referring to your code here. I was referring to the code snippets I put into comment #48 which you graxiously removed :)
Sorry for being so unclear.

Seems I have a new hobby: misunderstanding you ;-)
Just kidding

I learned a lot of interesting things in this bug :-)

You might be interested in the fact that I couldn't reproduce the horicontal scrolling in 10.3 beta2 - not sure if it was you, the xorg.conf for installation changed or if it was randomly fixed...

Default config is now to only configure a single wheel.

Ah, Matthias, I recognise your code in your patch. It's the code that periodically breaks my mouse wheels and causes me to have to restart the X server - and therewith all running applications - to get them working again.

I fixed this by modifying the code to simply ignore bogus packets (but still report them in the log) instead of disabling the secondary wheel.

Now everything works fine. Occasionally I get reports of bogus packets in the log but there is no observable undesired behaviour of the mouse or wheels. I cannot induce horizontal scrolling by fast movement of the vertical wheel either (not that I ever noticed this behaviour on previous xorg versions that did not include your patch).

For full details and my patch see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=454546

(In reply to comment #63 from Pigeon Pigeon)
> Ah, Matthias, I recognise your code in your patch. It's the code that
> periodically breaks my mouse wheels and causes me to have to restart the X
> server - and therewith all running applications - to get them working again.

So you have a 2 wheel mouse?

> I fixed this by modifying the code to simply ignore bogus packets (but still
> report them in the log) instead of disabling the secondary wheel.

Hm. I added this code because too many people had two wheels configured, but using a 1 wheel mouse that sends gathered events of which some look like 2 wheel events.

> cannot induce horizontal scrolling by fast movement of the vertical wheel
> either (not that I ever noticed this behaviour on previous xorg versions that
> did not include your patch).

That very much depends on the mouse and the implementation of the mouse emulation code in the kernel.

Have to think a bit about that. These issues are not trivial, and the mouse driver code is ugly as hell. Thanks for your investigation so far.

Egbert, you wanted to do something here. That had been in June, though.

Pigeon, let's continue this on the Xorg bugzilla, and leave this bug for Egbert's enhancements, so we don't mix them up.

I guess this discussion has become obsolete by switching from mouse to evdev driver with openSUSE 11.2.