On Fri, Oct 25, 2013 at 11:41:25AM +0100, Edd Barrett wrote:
> On Thu, Oct 24, 2013 at 10:33:22PM +0300, Henri Kemppainen wrote:
> > What happens when priv->swap_axes is set, and the ax && ay branch is
> > taken along with the wsWheelEmuFilterMotion() branch. Following
> > continue another event is processed and now the axes are swapped again
> > (ax and ay were not reset after use) and then what? Not very likely
> > I know.
>
> Ah, yes, there is the possibility of posting an inconsistent pointer sample
> in this case. Perhaps we should only update the old_ax/old_ay if the
> wsWheelEmuFilterMotion branch is not taken?
>
> What do you think? And yes, this is a very very unlikely case. You could
> argue it wouldn't matter even if it did happen.
>
> --
> Best Regards
> Edd Barrett
>
> http://www.theunixzoo.co.uk
>
Alternative solution without extra magic (need rebuild kernel).
Before (on example pms(4)):
* user move mouse
* pms(4) read state mouse and process it
* pms(4) send dx, dy and buttons in wscons
* wscons generate simple events
* ws(4) reads one event and process it immediately
After applying diff:
* user move mouse
* pms(4) read state mouse and process it
* pms(4) send dx, dy and buttons in wscons
* wscons generate simple events and adds SYNC event
* ws(4) reads events until it receives SYNC, and only then begins processing
Tested on mouse.
Comments ?
PS:
synaptics(4) is working on a similar basis
--
Alexandr Shadchin
Index: dev/pckbc/pms.c
===================================================================
RCS file: /cvs/src/sys/dev/pckbc/pms.c,v
retrieving revision 1.48
diff -u -p -r1.48 pms.c
--- dev/pckbc/pms.c 20 Sep 2013 14:07:30 -0000 1.48
+++ dev/pckbc/pms.c 26 Oct 2013 15:09:53 -0000
@@ -1155,8 +1155,7 @@ pms_proc_synaptics(struct pms_softc *sc)
if (syn->wsmode == WSMOUSE_NATIVE) {
wsmouse_input(sc->sc_wsmousedev, buttons, x, y, z, w,
WSMOUSE_INPUT_ABSOLUTE_X | WSMOUSE_INPUT_ABSOLUTE_Y |
- WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W |
- WSMOUSE_INPUT_SYNC);
+ WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W);
} else {
dx = dy = 0;
if (z > SYNAPTICS_PRESSURE) {
@@ -1470,8 +1469,7 @@ pms_proc_alps(struct pms_softc *sc)
wsmouse_input(sc->sc_wsmousedev, buttons, x, y, z, w,
WSMOUSE_INPUT_ABSOLUTE_X | WSMOUSE_INPUT_ABSOLUTE_Y |
- WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W |
- WSMOUSE_INPUT_SYNC);
+ WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W);
alps->old_fin = fin;
} else {
@@ -2321,8 +2319,7 @@ elantech_send_input(struct pms_softc *sc
WSMOUSE_INPUT_ABSOLUTE_X |
WSMOUSE_INPUT_ABSOLUTE_Y |
WSMOUSE_INPUT_ABSOLUTE_Z |
- WSMOUSE_INPUT_ABSOLUTE_W |
- WSMOUSE_INPUT_SYNC);
+ WSMOUSE_INPUT_ABSOLUTE_W);
} else {
dx = dy = 0;
Index: dev/wscons/wsmouse.c
===================================================================
RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v
retrieving revision 1.24
diff -u -p -r1.24 wsmouse.c
--- dev/wscons/wsmouse.c 18 Oct 2013 13:54:09 -0000 1.24
+++ dev/wscons/wsmouse.c 26 Oct 2013 15:09:55 -0000
@@ -452,13 +452,11 @@ wsmouse_input(struct device *wsmousedev,
ub ^= d;
}
- if (flags & WSMOUSE_INPUT_SYNC) {
- NEXT;
- ev->type = WSCONS_EVENT_SYNC;
- ev->value = 0;
- TIMESTAMP;
- ADVANCE;
- }
+ NEXT;
+ ev->type = WSCONS_EVENT_SYNC;
+ ev->value = 0;
+ TIMESTAMP;
+ ADVANCE;
/* XXX fake wscons_event notifying wsmoused(8) to close mouse device */
if (flags & WSMOUSE_INPUT_WSMOUSED_CLOSE) {
Index: dev/wscons/wsmousevar.h
===================================================================
RCS file: /cvs/src/sys/dev/wscons/wsmousevar.h,v
retrieving revision 1.6
diff -u -p -r1.6 wsmousevar.h
--- dev/wscons/wsmousevar.h 22 Jul 2012 18:28:36 -0000 1.6
+++ dev/wscons/wsmousevar.h 26 Oct 2013 15:09:55 -0000
@@ -74,7 +74,6 @@ int wsmousedevprint(void *, const char *
#define WSMOUSE_INPUT_WSMOUSED_CLOSE (1<<3) /* notify wsmoused(8) to close
mouse device */
#define WSMOUSE_INPUT_ABSOLUTE_W (1<<4)
-#define WSMOUSE_INPUT_SYNC (1<<5)
void wsmouse_input(struct device *kbddev, u_int btns,
int x, int y, int z, int w, u_int flags);
Index: ws.c
===================================================================
RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
retrieving revision 1.58
diff -u -p -r1.58 ws.c
--- ws.c 20 Jul 2013 13:24:50 -0000 1.58
+++ ws.c 26 Oct 2013 16:03:01 -0000
@@ -428,13 +428,6 @@ wsDeviceOn(DeviceIntPtr pWS)
}
}
}
- priv->buffer = XisbNew(pInfo->fd,
- sizeof(struct wscons_event) * NUMEVENTS);
- if (priv->buffer == NULL) {
- xf86IDrvMsg(pInfo, X_ERROR, "cannot alloc xisb buffer\n");
- wsClose(pInfo);
- return !Success;
- }
xf86AddEnabledDevice(pInfo);
wsmbEmuOn(pInfo);
pWS->public.on = TRUE;
@@ -462,170 +455,150 @@ wsDeviceOff(DeviceIntPtr pWS)
xf86RemoveEnabledDevice(pInfo);
wsClose(pInfo);
}
- if (priv->buffer) {
- XisbFree(priv->buffer);
- priv->buffer = NULL;
- }
pWS->public.on = FALSE;
}
-static void
-wsReadInput(InputInfoPtr pInfo)
+static Bool
+wsReadEvent(InputInfoPtr pInfo, struct wscons_event *event)
{
- WSDevicePtr priv = (WSDevicePtr)pInfo->private;
- static struct wscons_event eventList[NUMEVENTS];
- int n, c, dx, dy;
- struct wscons_event *event = eventList;
- unsigned char *pBuf;
-
- XisbBlockDuration(priv->buffer, -1);
- pBuf = (unsigned char *)eventList;
- n = 0;
- while (n < sizeof(eventList) && (c = XisbRead(priv->buffer)) >= 0) {
- pBuf[n++] = (unsigned char)c;
+ Bool rc = TRUE;
+ ssize_t len;
+
+ len = read(pInfo->fd, event, sizeof(struct wscons_event));
+ if (len <= 0) {
+ if (errno != EAGAIN)
+ xf86IDrvMsg(pInfo, X_ERROR, "read error %s\n",
+ strerror(errno));
+ rc = FALSE;
+ } else if (len != sizeof(struct wscons_event)) {
+ xf86IDrvMsg(pInfo, X_ERROR,
+ "read error, invalid number of bytes\n");
+ rc = FALSE;
}
- if (n == 0)
- return;
+ return rc;
+}
- dx = dy = 0;
- n /= sizeof(struct wscons_event);
- while (n--) {
- int buttons = priv->lastButtons;
- int newdx = 0, newdy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
- int zbutton = 0, wbutton = 0;
+static Bool
+wsReadHwState(InputInfoPtr pInfo, wsHwState *hw)
+{
+ WSDevicePtr priv = (WSDevicePtr)pInfo->private;
+ struct wscons_event event;
+
+ bzero(hw, sizeof(wsHwState));
+
+ hw->buttons = priv->lastButtons;
+ hw->ax = priv->old_ax;
+ hw->ay = priv->old_ay;
- switch (event->type) {
+ while (wsReadEvent(pInfo, &event)) {
+ switch (event.type) {
case WSCONS_EVENT_MOUSE_UP:
- buttons &= ~(1 << event->value);
- DBG(4, ErrorF("Button %d up %x\n", event->value,
- buttons));
+ hw->buttons &= ~(1 << event.value);
+ DBG(4, ErrorF("Button %d up %x\n", event.value,
+ hw->buttons));
break;
case WSCONS_EVENT_MOUSE_DOWN:
- buttons |= (1 << event->value);
- DBG(4, ErrorF("Button %d down %x\n", event->value,
- buttons));
+ hw->buttons |= (1 << event.value);
+ DBG(4, ErrorF("Button %d down %x\n", event.value,
+ hw->buttons));
break;
case WSCONS_EVENT_MOUSE_DELTA_X:
- if (!dx)
- dx = event->value;
- else
- newdx = event->value;
- DBG(4, ErrorF("Relative X %d\n", event->value));
+ hw->dx = event.value;
+ DBG(4, ErrorF("Relative X %d\n", event.value));
break;
case WSCONS_EVENT_MOUSE_DELTA_Y:
- if (!dy)
- dy = -event->value;
- else
- newdy = -event->value;
- DBG(4, ErrorF("Relative Y %d\n", event->value));
+ hw->dy = -event.value;
+ DBG(4, ErrorF("Relative Y %d\n", event.value));
+ break;
+ case WSCONS_EVENT_MOUSE_DELTA_Z:
+ hw->dz = event.value;
+ DBG(4, ErrorF("Relative Z %d\n", event.value));
+ break;
+ case WSCONS_EVENT_MOUSE_DELTA_W:
+ hw->dw = event.value;
+ DBG(4, ErrorF("Relative W %d\n", event.value));
break;
case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
- DBG(4, ErrorF("Absolute X %d\n", event->value));
- if (event->value == 4095)
- break;
- ax = event->value;
+ hw->ax = event.value;
if (priv->inv_x)
- ax = priv->max_x - ax + priv->min_x;
+ hw->ax = priv->max_x - hw->ax + priv->min_x;
+ DBG(4, ErrorF("Absolute X %d\n", event.value));
break;
case WSCONS_EVENT_MOUSE_ABSOLUTE_Y:
- DBG(4, ErrorF("Absolute Y %d\n", event->value));
- ay = event->value;
+ hw->ay = event.value;
if (priv->inv_y)
- ay = priv->max_y - ay + priv->min_y;
- break;
- case WSCONS_EVENT_MOUSE_DELTA_Z:
- DBG(4, ErrorF("Relative Z %d\n", event->value));
- dz = event->value;
+ hw->ay = priv->max_y - hw->ay + priv->min_y;
+ DBG(4, ErrorF("Absolute Y %d\n", event.value));
break;
case WSCONS_EVENT_MOUSE_ABSOLUTE_Z:
+ case WSCONS_EVENT_MOUSE_ABSOLUTE_W:
/* ignore those */
- ++event;
continue;
- break;
- case WSCONS_EVENT_MOUSE_DELTA_W:
- DBG(4, ErrorF("Relative W %d\n", event->value));
- dw = event->value;
- break;
+ case WSCONS_EVENT_SYNC:
+ DBG(4, ErrorF("Sync\n"));
+ return TRUE;
default:
xf86IDrvMsg(pInfo, X_WARNING,
- "bad wsmouse event type=%d\n", event->type);
- ++event;
+ "bad wsmouse event type=%d\n", event.type);
continue;
}
- ++event;
+ }
- if ((newdx || newdy) || ((dx || dy) &&
- event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
- event->type != WSCONS_EVENT_MOUSE_DELTA_Y)) {
- int tmpx = dx, tmpy = dy;
- dx = newdx;
- dy = newdy;
-
- if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
- continue;
-
- /* relative motion event */
- DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
- tmpx, tmpy));
- xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
- }
- if (dz && priv->Z.negative != WS_NOMAP
- && priv->Z.positive != WS_NOMAP) {
- zbutton = (dz < 0) ? priv->Z.negative :
- priv->Z.positive;
- DBG(4, ErrorF("Z -> button %d\n", zbutton));
- wsButtonClicks(pInfo, zbutton, abs(dz));
- }
- if (dw && priv->W.negative != WS_NOMAP
- && priv->W.positive != WS_NOMAP) {
- wbutton = (dw < 0) ? priv->W.negative :
- priv->W.positive;
- DBG(4, ErrorF("W -> button %d\n", wbutton));
- wsButtonClicks(pInfo, wbutton, abs(dw));
- }
- if (priv->lastButtons != buttons) {
- /* button event */
- wsSendButtons(pInfo, buttons);
- }
- if (priv->swap_axes) {
- int tmp;
+ return FALSE;
+}
- tmp = ax;
- ax = ay;
- ay = tmp;
- }
- if (ax) {
- int xdelta = ax - priv->old_ax;
- priv->old_ax = ax;
- if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
- continue;
-
- /* absolute position event */
- DBG(3, ErrorF("postMotionEvent X %d\n", ax));
- xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
- }
- if (ay) {
- int ydelta = ay - priv->old_ay;
- priv->old_ay = ay;
- if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
- continue;
-
- /* absolute position event */
- DBG(3, ErrorF("postMotionEvent y %d\n", ay));
- xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
- }
+static void
+wsReadInput(InputInfoPtr pInfo)
+{
+ WSDevicePtr priv = (WSDevicePtr)pInfo->private;
+ wsHwState hw;
+
+ if (!wsReadHwState(pInfo, &hw))
+ return;
+
+ if ((hw.dx || hw.dy) && !wsWheelEmuFilterMotion(pInfo, hw.dx, hw.dy)) {
+ /* relative motion event */
+ DBG(3, ErrorF("postMotionEvent dX %d dY %d\n", hw.dx, hw.dy));
+ xf86PostMotionEvent(pInfo->dev, 0, 0, 2, hw.dx, hw.dy);
}
- if (dx || dy) {
- if (wsWheelEmuFilterMotion(pInfo, dx, dy))
+ if (hw.dz && priv->Z.negative != WS_NOMAP
+ && priv->Z.positive != WS_NOMAP) {
+ int zbutton;
+ zbutton = (hw.dz < 0) ? priv->Z.negative : priv->Z.positive;
+ DBG(4, ErrorF("Z -> button %d (%d)\n", zbutton, abs(hw.dz)));
+ wsButtonClicks(pInfo, zbutton, abs(hw.dz));
+ }
+ if (hw.dw && priv->W.negative != WS_NOMAP
+ && priv->W.positive != WS_NOMAP) {
+ int wbutton;
+ wbutton = (hw.dw < 0) ? priv->W.negative : priv->W.positive;
+ DBG(4, ErrorF("W -> button %d (%d)\n", wbutton, abs(hw.dw)));
+ wsButtonClicks(pInfo, wbutton, abs(hw.dw));
+ }
+ if (priv->lastButtons != hw.buttons) {
+ /* button event */
+ wsSendButtons(pInfo, hw.buttons);
+ }
+ if (priv->swap_axes) {
+ int tmp;
+
+ tmp = hw.ax;
+ hw.ax = hw.ay;
+ hw.ay = tmp;
+ }
+ if ((hw.ax != priv->old_ax) || (hw.ay != priv->old_ay)) {
+ int xdelta = hw.ax - priv->old_ax;
+ int ydelta = hw.ay - priv->old_ay;
+ priv->old_ax = hw.ax;
+ priv->old_ay = hw.ay;
+ if (wsWheelEmuFilterMotion(pInfo, xdelta, ydelta))
return;
- /* relative motion event */
- DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
- dx, dy));
- xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
+ /* absolute position event */
+ DBG(3, ErrorF("postMotionEvent X %d Y %d\n", hw.ax, hw.ay));
+ xf86PostMotionEvent(pInfo->dev, 1, 0, 2, hw.ax, hw.ay);
}
- return;
}
static void
Index: ws.h
===================================================================
RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.h,v
retrieving revision 1.12
diff -u -p -r1.12 ws.h
--- ws.h 12 Jun 2012 17:44:56 -0000 1.12
+++ ws.h 26 Oct 2013 16:03:01 -0000
@@ -29,7 +29,6 @@ extern int ws_debug_level;
#define NAXES 2 /* X and Y axes only */
#define NBUTTONS 32 /* max theoretical buttons */
#define DFLTBUTTONS 3 /* default number of buttons */
-#define NUMEVENTS 16 /* max # of ws events to read at once */
#define WS_NOMAP 0
@@ -40,6 +39,12 @@ typedef struct {
int traveled_distance;
} WheelAxis, *WheelAxisPtr;
+typedef struct {
+ unsigned int buttons;
+ int dx, dy, dz, dw;
+ int ax, ay;
+} wsHwState;
+
typedef struct WSDevice {
char *devName; /* device name */
int type; /* ws device type */
@@ -50,7 +55,6 @@ typedef struct WSDevice {
int raw;
int inv_x, inv_y;
int screen_no;
- pointer buffer;
WheelAxis Z;
WheelAxis W;
struct wsmouse_calibcoords coords; /* mirror of the kernel values */