Re: touchpad slight regression (snap: 20141121-20150217)
[moved to tech@]
On 27/02/15(Fri) 11:40, patrick keshishian wrote:
Hi,
On 2/26/15, Ulf Brosziewski ulf.brosziew...@t-online.de wrote:
On 02/27/2015 03:31 AM, Ulf Brosziewski wrote:
...
It might be that the following patch to wsmouse.c solves the problem
with the new version of wsconscomm. Tests would be welcome (I could
only verify that the patch does no harm to other touchpad types, i.e.,
Elantech-v4 and Alps Glidepoint).
[...]
Sorry, the change was in the wrong place and would only do a half of
the work. It should look like:
Index: wsmouse.c
===
RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v
retrieving revision 1.26
diff -u -p -r1.26 wsmouse.c
--- wsmouse.c 27 Oct 2014 13:55:05 - 1.26
+++ wsmouse.c 27 Feb 2015 02:50:06 -
@@ -433,6 +433,9 @@ wsmouse_input(struct device *wsmousedev,
}
}
+ if (sc-sc_z == 0)
+ sc-sc_w = INVALID_W;
+
mb = sc-sc_mb;
while ((d = mb ^ ub) != 0) {
/*
I can confirm this change alone causes no adverse, observable
change on my x120e's touchpad.
I the long term, I think that having a similar logic in wsmouse_input()
makes sense. As I already told Ulf, it would be really nice to improve
the wsmouse(4)/wscons(4) layer to support modern touchpad features and
get rid of wsconscomm.
But right now this chunk is really intrusive. Plus I believe it should
only be applied on touchpad ``native'' mode. Sure, we could be able to
check for WSMOUSE_INPUT_ABSOLUTE_W, but can it be a bad idea?
What about putting a similar chunk in pms_proc_synpatics() instead?
However, I would appreciate it if someone could enlighten me
as to what the Z and W axis refer.
I'm glad you asked, because it really depends on which hardware you're
using :) Plus right now our code use magic values which are most of
the time not documented. Diffs are more than welcome to improve the
situation.
Martin
Re: USBD_NO_COPY problems
On 19/02/15(Thu) 21:49, David Higgs wrote: On Feb 13, 2015, at 7:29 AM, David Higgs hig...@gmail.com wrote: On Friday, February 13, 2015, Martin Pieuchot mpieuc...@nolizard.org wrote: On 13/02/15(Fri) 00:28, David Higgs wrote: I guess nobody else has tried calling uhidev_get_report_async() yet. :) First I was getting a NULL pointer deref in the uhidev async callback. Then I realized that due to USBD_NO_COPY, xfer-buffer was always NULL. Next, I tried to use the DMA buffer, but I ended up in DDB in a very cryptic way. I believe this is because the DMA buffer isn't available when the callback is invoked. For the async callback to get a valid dmabuf, it needs to be invoked prior to usb_freemem() in usbd_transfer_complete(). The xfer-status determination would need to move up too. I'd do this myself but I don't understand the logic and ordering of pipe-repeat stuff, and am concerned about unintentionally breaking other devices. This is partially my fault, because I tested the original diff that added the USBD_NO_COPY semantics to verify that it didn't break my synchronous code paths, but hadn't yet written anything for upd(4) to check the async ones. Does the diff below help? Partially but not enough. I had already figured out that I needed that to solve the NULL pointer dereference. See my 2nd paragraph above. OK, I figured out my issue - the crazy DDB backtrace is produced when you execute a NULL callback. It still doesn’t seem legal for the callback to access DMA buffer contents after they are “freed”. I assume this won’t work in all cases (host controllers / architectures / cache behaviors), but I don’t experience any problems in my i386 VM. I tried reordering parts of usbd_transfer_complete(), but DIAGNOSTIC code became very unhappy with the results. Fortunately, the diff below doesn’t touch that code path and just fixes the uhidev layer. My async upd(4) changes will be forthcoming in a different thread. Committed since nothing uses it at the moment. Thanks! Martin
Re: touchpad slight regression (snap: 20141121-20150217)
On 02/28/2015 09:38 AM, Martin Pieuchot wrote:
[moved to tech@]
On 27/02/15(Fri) 11:40, patrick keshishian wrote:
Hi,
On 2/26/15, Ulf Brosziewskiulf.brosziew...@t-online.de wrote:
On 02/27/2015 03:31 AM, Ulf Brosziewski wrote:
...
It might be that the following patch to wsmouse.c solves the problem
with the new version of wsconscomm. Tests would be welcome (I could
only verify that the patch does no harm to other touchpad types, i.e.,
Elantech-v4 and Alps Glidepoint).
[...]
Sorry, the change was in the wrong place and would only do a half of
the work. It should look like:
Index: wsmouse.c
===
RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v
retrieving revision 1.26
diff -u -p -r1.26 wsmouse.c
--- wsmouse.c 27 Oct 2014 13:55:05 - 1.26
+++ wsmouse.c 27 Feb 2015 02:50:06 -
@@ -433,6 +433,9 @@ wsmouse_input(struct device *wsmousedev,
}
}
+ if (sc-sc_z == 0)
+ sc-sc_w = INVALID_W;
+
mb = sc-sc_mb;
while ((d = mb ^ ub) != 0) {
/*
I can confirm this change alone causes no adverse, observable
change on my x120e's touchpad.
I the long term, I think that having a similar logic in wsmouse_input()
makes sense. As I already told Ulf, it would be really nice to improve
the wsmouse(4)/wscons(4) layer to support modern touchpad features and
get rid of wsconscomm.
But right now this chunk is really intrusive. Plus I believe it should
only be applied on touchpad ``native'' mode. Sure, we could be able to
check for WSMOUSE_INPUT_ABSOLUTE_W, but can it be a bad idea?
What about putting a similar chunk in pms_proc_synpatics() instead?
However, I would appreciate it if someone could enlighten me
as to what the Z and W axis refer.
I'm glad you asked, because it really depends on which hardware you're
using :) Plus right now our code use magic values which are most of
the time not documented. Diffs are more than welcome to improve the
situation.
Martin
I was a bit hasty with that patch. I reasoned that sc_z is initialized
with INT_MAX, and it is only used in native mode. Later I thought that
change to wsconscomm might be a better (short-term) solution, something
like the code below. But it doesn't look less odd than the other patch,
or does it?
It is not clear to me how a purely local change to pms_proc_input()
could help here. As long as we don't add or redefine event types, or
introduce a special convention for W values, the only way to circumvent
the problem might be to produce an additional event at the start of
a two-finger touch (i.e., to change W from 0 to 1 and back again).
diff --git a/wsconscomm.c b/wsconscomm.c
index f6b9d88..78f47ab 100644
--- a/wsconscomm.c
+++ b/wsconscomm.c
@@ -217,6 +217,14 @@ WSConsReadHwState(InputInfoPtr pInfo,
if (hw-z == 0) {
hw-fingerWidth = 0;
hw-numFingers = 0;
+} else if (hw-numFingers == 0) {
+/*
+ * Because W may be 0 already, a two-finger touch on a
+ * Synaptics touchpad doesn't necessarily produce an update
+ * event for W.
+ */
+hw-fingerWidth = 5;
+hw-numFingers = 2;
}
hw-millis = 1000 * event.time.tv_sec + event.time.tv_nsec /
100;
SynapticsCopyHwState(hwRet, hw);
