On Sat, Oct 26, 2013 at 10:13:11PM +0600, Alexandr Shadchin wrote:
> 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 ?
Look good to me. However I've a concern about compatibility with
NetBSD. The kernel change should be documented in the commit message
for xf86-input-ws so that they can catch up with the kernel change
before they update xf86-input-ws...
>
> 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 */
--
Matthieu Herrb
