[PATCH evdev 2/2] Add SYN_DROPPED handling #59702
Benjamin Tissoires
benjamin.tissoires at gmail.com
Fri Jan 25 06:23:19 PST 2013
On Wed, Jan 23, 2013 at 3:19 AM, Chung-yih Wang <cywang at chromium.org> wrote:
> If an evdev client cannot consume evdev events in its queue fast enough, the
> evdev kernel driver will enqueue a SYN_DROPPED event and clear the queue
> once the client's queue is full. The result is that the X driver will be out
> of sync with respect to the kernel driver state. The patch tries to handle the
> SYN_DROPPED event by retrieving the kernel driver's state. Retrieving this
> state is inherently non-atomic, since it requires a sequence of ioctls. We use
> a simple before and after time stamping approach to deal with the race
> condition between partially syncing state and any potentially stale events that
> arrive during synchronization.
Hi,
just a little comment in addition to Peter's.
>
> Signed-off-by: Chung-yih Wang <cywang at chromium.org>
> ---
> src/evdev.c | 371 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> src/evdev.h | 17 +++
> 2 files changed, 381 insertions(+), 7 deletions(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index 6564cd0..fc8a08d 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -128,6 +128,8 @@ static void EvdevSetCalibration(InputInfoPtr pInfo, int num_calibration, int cal
> static int EvdevOpenDevice(InputInfoPtr pInfo);
> static void EvdevCloseDevice(InputInfoPtr pInfo);
>
> +static int EvdevInjectEvent(InputInfoPtr pInfo, uint16_t type,
> + uint16_t code, int32_t value);
> static void EvdevInitAxesLabels(EvdevPtr pEvdev, int mode, int natoms, Atom *atoms);
> static void EvdevInitOneAxisLabel(EvdevPtr pEvdev, int axis,
> const char **labels, int label_idx, Atom *atoms);
> @@ -135,6 +137,20 @@ static void EvdevInitButtonLabels(EvdevPtr pEvdev, int natoms, Atom *atoms);
> static void EvdevInitProperty(DeviceIntPtr dev);
> static int EvdevSetProperty(DeviceIntPtr dev, Atom atom,
> XIPropertyValuePtr val, BOOL checkonly);
> +static void EvdevSyncState(InputInfoPtr pInfo);
> +static void EvdevGetKernelTime(struct timeval *current_time,
> + BOOL use_monotonic);
> +static int EvdevKeyStateSync(InputInfoPtr pInfo);
> +static int EvdevAbsAxesSync(InputInfoPtr pInfo);
> +static int EvdevAbsMtSlotSync(InputInfoPtr pInfo);
> +static int EvdevInjectAbsMtAxisChangeEvent(InputInfoPtr pInfo, int slot_index,
> + uint16_t code, int32_t value);
> +static int EvdevCheckAbsMtAxesChange(InputInfoPtr pInfo, MTSlotInfoPtr slots,
> + int *count_after_synreport);
> +static int EvdevGetAllSlotVals(InputInfoPtr pInfo, MTSlotInfoPtr slots);
> +static int EvdevAbsMtStateSync(InputInfoPtr pInfo, int *count_after_synreport);
> +static int EvdevAbsStateSync(InputInfoPtr pInfo, int *count_after_synreport);
> +
> static Atom prop_product_id;
> static Atom prop_invert;
> static Atom prop_calibration;
> @@ -208,6 +224,11 @@ static inline void EvdevSetBit(unsigned long *array, int bit)
> array[bit / LONG_BITS] |= (1LL << (bit % LONG_BITS));
> }
>
> +static inline void EvdevClearBit(unsigned long *array, int bit)
> +{
> + array[bit / LONG_BITS] &= ~(1LL << (bit % LONG_BITS));
> +}
> +
> static int
> EvdevGetMajorMinor(InputInfoPtr pInfo)
> {
> @@ -649,6 +670,11 @@ EvdevProcessButtonEvent(InputInfoPtr pInfo, struct input_event *ev)
> /* Get the signed value, earlier kernels had this as unsigned */
> value = ev->value;
>
> + if (ev->value)
> + EvdevSetBit(pEvdev->key_state_bitmask, ev->code);
> + else
> + EvdevClearBit(pEvdev->key_state_bitmask, ev->code);
> +
> /* Handle drag lock */
> if (EvdevDragLockFilterEvent(pInfo, button, value))
> return;
> @@ -779,6 +805,7 @@ EvdevProcessTouchEvent(InputInfoPtr pInfo, struct input_event *ev)
> if (pEvdev->slot_state == SLOTSTATE_EMPTY)
> pEvdev->slot_state = SLOTSTATE_UPDATE;
> if (ev->code == ABS_MT_TRACKING_ID) {
> + pEvdev->cached_tid[slot_index] = ev->value;
> if (ev->value >= 0) {
> pEvdev->slot_state = SLOTSTATE_OPEN;
>
> @@ -993,7 +1020,7 @@ static void EvdevPostQueuedEvents(InputInfoPtr pInfo, int num_v, int first_v,
> * Take the synchronization input event and process it accordingly; the motion
> * notify events are sent first, then any button/key press/release events.
> */
> -static void
> +static BOOL
> EvdevProcessSyncEvent(InputInfoPtr pInfo, struct input_event *ev)
> {
> int i;
> @@ -1001,6 +1028,11 @@ EvdevProcessSyncEvent(InputInfoPtr pInfo, struct input_event *ev)
> int v[MAX_VALUATORS] = {};
> EvdevPtr pEvdev = pInfo->private;
>
> + if (ev->code == SYN_DROPPED) {
> + xf86IDrvMsg(pInfo, X_INFO, "+++ SYN_DROPPED +++\n");
> + return TRUE;
> + }
> +
> EvdevProcessProximityState(pInfo);
>
> EvdevProcessValuators(pInfo);
> @@ -1028,16 +1060,20 @@ EvdevProcessSyncEvent(InputInfoPtr pInfo, struct input_event *ev)
> pEvdev->abs_queued = 0;
> pEvdev->rel_queued = 0;
> pEvdev->prox_queued = 0;
> -
> + return FALSE;
> }
>
> /**
> * Process the events from the device; nothing is actually posted to the server
> - * until an EV_SYN event is received.
> + * until an EV_SYN event is received. As the SYN_DROPPED event indicates that the
> + * state of evdev driver will be out of sync with the event queue, additional
> + * handling is required for processing the SYN_DROPPED event. The function returns
> + * TRUE if a SYN_DROPPED event is received, FALSE otherwise.
> */
> -static void
> +static BOOL
> EvdevProcessEvent(InputInfoPtr pInfo, struct input_event *ev)
> {
> + BOOL syn_dropped = FALSE;
> switch (ev->type) {
> case EV_REL:
> EvdevProcessRelativeMotionEvent(pInfo, ev);
> @@ -1049,9 +1085,10 @@ EvdevProcessEvent(InputInfoPtr pInfo, struct input_event *ev)
> EvdevProcessKeyEvent(pInfo, ev);
> break;
> case EV_SYN:
> - EvdevProcessSyncEvent(pInfo, ev);
> + syn_dropped = EvdevProcessSyncEvent(pInfo, ev);
> break;
> }
> + return syn_dropped;
> }
>
> #undef ABS_X_VALUE
> @@ -1082,6 +1119,308 @@ EvdevFreeMasks(EvdevPtr pEvdev)
> #endif
> }
>
> +static void
> +EvdevGetKernelTime(struct timeval *current_time, BOOL use_monotonic) {
> + struct timespec now;
> + clockid_t clockid = (use_monotonic) ? CLOCK_MONOTONIC : CLOCK_REALTIME;
> +
> + clock_gettime(clockid, &now);
> + current_time->tv_sec = now.tv_sec;
> + current_time->tv_usec = now.tv_nsec / 1000;
> +}
> +
> +static int
> +EvdevInjectEvent(InputInfoPtr pInfo, uint16_t type, uint16_t code,
> + int32_t value) {
> + EvdevPtr pEvdev = pInfo->private;
> + struct input_event ev;
> +
> + ev.type = type;
> + ev.code = code;
> + ev.value = value;
> + EvdevGetKernelTime(&ev.time, pEvdev->is_monotonic);
> + /* Inject the event by processing it */
> + EvdevProcessEvent(pInfo, &ev);
> + return 1;
> +}
> +
> +static int
> +EvdevKeyStateSync(InputInfoPtr pInfo) {
> + EvdevPtr pEvdev = pInfo->private;
> + unsigned long key_state_bitmask[NLONGS(KEY_CNT)];
> + int i, ev_count = 0;
> + int len = sizeof(key_state_bitmask);
> +
> + if (ioctl(pInfo->fd, EVIOCGKEY(len), key_state_bitmask) < 0) {
> + xf86IDrvMsg(pInfo, X_ERROR,
> + "ioctl EVIOCGKEY failed: %s\n", strerror(errno));
> + return !Success;
> + }
> + for (i = 0; i < KEY_CNT; i++) {
> + int orig_value, current_value;
> + if (!EvdevBitIsSet(pEvdev->key_bitmask, i))
> + continue;
> + orig_value = EvdevBitIsSet(pEvdev->key_state_bitmask, i);
> + current_value = EvdevBitIsSet(key_state_bitmask, i);
> + if (current_value == orig_value)
> + continue;
> + ev_count += EvdevInjectEvent(pInfo, EV_KEY, i, current_value);
> + }
> + return ev_count;
> +}
> +
> +static int
> +EvdevAbsAxesSync(InputInfoPtr pInfo) {
> + EvdevPtr device = pInfo->private;
> + struct input_absinfo absinfo;
> + int i, ev_count = 0;
> +
> + /* Sync all ABS_ axes excluding ABS_MT_ axes */
> + for (i = ABS_X; i < ABS_MAX; i++) {
> + if (i >= ABS_MT_SLOT && i <= _ABS_MT_LAST)
> + continue;
> + if (!EvdevBitIsSet(device->abs_bitmask, i))
> + continue;
> + if (ioctl(pInfo->fd, EVIOCGABS(i), &absinfo) < 0) {
> + xf86IDrvMsg(pInfo, X_ERROR, "ioctl EVIOCGABS(%zu) failed: %s\n",
> + i, strerror(errno));
> + } else if (absinfo.value != device->absinfo[i].value) {
> + ev_count += EvdevInjectEvent(pInfo, EV_ABS, i, absinfo.value);
> + }
> + }
> + return ev_count;
> +}
> +
> +static int
> +EvdevAbsMtSlotSync(InputInfoPtr pInfo) {
> + EvdevPtr device = pInfo->private;
> + struct input_absinfo absinfo;
> + int ev_count = 0;
> +
> + if (ioctl(pInfo->fd, EVIOCGABS(ABS_MT_SLOT), &absinfo) < 0) {
> + xf86IDrvMsg(pInfo, X_ERROR, "ioctl EVIOCGABS(ABS_MT_SLOT) failed: %s\n",
> + strerror(errno));
> + return 0;
> + }
> + if (device->cur_slot != absinfo.value)
> + ev_count = EvdevInjectEvent(pInfo, EV_ABS, ABS_MT_SLOT, absinfo.value);
> + return ev_count;
> +}
> +
> +static int
> +EvdevInjectAbsMtAxisChangeEvent(InputInfoPtr pInfo, int slot_index,
> + uint16_t code, int32_t value) {
> + EvdevPtr device = pInfo->private;
> + int ev_count = 0;
> +
> + if (device->cur_slot != slot_index)
> + ev_count += EvdevInjectEvent(pInfo, EV_ABS, ABS_MT_SLOT, slot_index);
> + ev_count += EvdevInjectEvent(pInfo, EV_ABS, code, value);
> + return ev_count;
> +}
> +
> +static int
> +EvdevCheckAbsMtAxesChange(InputInfoPtr pInfo, MTSlotInfoPtr slots,
> + int *count_after_synreport)
> +{
> + EvdevPtr device = pInfo->private;
> + int i, j, ev_count = 0;
> + int total_ev_count = 0;
> +
> + /*
> + * There will be five conditions of a slot change after SYN_DROPPED:
> + * a. Finger leaving, i.e., tracking id changes from a non-negative
> + * number to -1.
> + * b. Finger arriving, i.e., tracking id changes from -1 to a
> + * non-negative number.
> + * c. Finger changing, i.e., original finger leaving and new finger
> + * arriving, tracking id changes from a non-negative number to
> + * another one.
> + * d. Same finger, but axes change, i.e., no tracking id changes, but some
> + * axes values have changed.
> + * e. Fingers arrive and leave: tracking ID was -1, and is still -1, but
> + * some axes values have changed.
> + * f. nothing changed
> + *
> + * To have X server seamless of SYN_DROPPED event, additional event
> + * injections will be required except for conditions e and f:
> + *
> + * Finger leaving (a): all axes of the slot should be updated first, then
> + * followed with tracking id change (-1).
> + *
> + * Finger arriving (b): new tracking id should be injected first, followed
> + * with all axes updates.
> + *
> + * Finger changing (c): first, inject finger leaving with tracking id -1,
> + * followed with new tracking id event, then update all axes data.
> + *
> + * Same finger, but axes change (d): all axes updates should be injected
> + *
> + */
> +
> + for (i = 0; i < num_slots(device); i++) {
> + int curr_tid = slots[ABS_MT_TRACKING_ID - _ABS_MT_FIRST].values[i];
> + int orig_tid = device->cached_tid[i];
> +
> + /* For conditions b and c, inject the tracking id change events first */
> + if (orig_tid != curr_tid && curr_tid != -1) {
> + /* For (c), inject the leaving event for original finger */
> + if (orig_tid != -1) {
> + ev_count += EvdevInjectAbsMtAxisChangeEvent(pInfo,
> + i,
> + ABS_MT_TRACKING_ID,
> + -1);
> + ev_count += EvdevInjectEvent(pInfo, EV_SYN, SYN_REPORT, 0);
> + /* Reset the count_after_synreport after SYN_REPORT event */
> + total_ev_count += ev_count;
> + *count_after_synreport = ev_count = 0;
> + }
> + /* For (b) and (c), set the new tid before updating axes */
> + ev_count += EvdevInjectAbsMtAxisChangeEvent(pInfo,
> + i,
> + ABS_MT_TRACKING_ID,
> + curr_tid);
> + }
> +
> +
> + for (j = _ABS_MT_FIRST; j <= _ABS_MT_LAST; j++) {
> + int axis = j - _ABS_MT_FIRST;
> + int map, orig_value, curr_value;
> + if ((j == ABS_MT_TRACKING_ID) ||
> + ((map = device->axis_map[j]) == -1))
> + continue;
> + if (!EvdevBitIsSet(device->abs_bitmask, j))
> + continue;
> +
> + orig_value = valuator_mask_get(device->last_mt_vals[i], map);
> + curr_value = slots[axis].values[i];
> +
> + if (orig_value == curr_value)
> + continue;
> +
> + /* For condition e, internal axes values should be updated */
> + if (orig_tid == -1 && curr_tid == -1) {
> + valuator_mask_set(device->last_mt_vals[i], map, curr_value);
> + continue;
> + }
> +
> + /* In addition to condition d, all axes updates will be injected */
> + ev_count += EvdevInjectAbsMtAxisChangeEvent(pInfo,
> + i,
> + j,
> + curr_value);
> + }
> +
> + /* For condition a, inject finger leaving event */
> + if (orig_tid != -1 && curr_tid == -1) {
> + ev_count += EvdevInjectAbsMtAxisChangeEvent(pInfo,
> + i,
> + ABS_MT_TRACKING_ID,
> + -1);
> + }
> + }
> + /* Update current slot index if it is different from cur_slot value */
> + ev_count += EvdevAbsMtSlotSync(pInfo);
> + *count_after_synreport += ev_count;
> +
> + return total_ev_count + ev_count;
> +}
> +
> +static int
> +EvdevGetAllSlotVals(InputInfoPtr pInfo, MTSlotInfoPtr slots)
> +{
> + EvdevPtr device = pInfo->private;
> + int i;
> +
> + /* Retrieve current ABS_MT_ axes for all slots */
> + for (i = _ABS_MT_FIRST; i <= _ABS_MT_LAST; i++) {
> + MTSlotInfoPtr req = &slots[i - _ABS_MT_FIRST];
> + if (!EvdevBitIsSet(device->abs_bitmask, i))
> + continue;
> + req->code = i;
> + if (ioctl(pInfo->fd, EVIOCGMTSLOTS((sizeof(*req))), req) < 0) {
> + xf86IDrvMsg(pInfo, X_ERROR,
> + "ioctl EVIOCGMTSLOTS(req.code=%d) failed: %s\n",
> + req->code, strerror(errno));
> + return !Success;
> + }
Given the current status of the multitouch handling in evdev, this is
not the exact reliable thing (but there is no exact reliable thing in
fact).
xf86-input-evdev uses mtdev to uniform multitouch protocol A and B,
and mtdev does not has a way to retrieve the current slot state. Which
means that devices following protocol A won't be fixed with your
patch.
It's not a very bad issue, as I'm working on a cleanup of multitouch
handling in evdev, and that devices following protocol A are
disappearing, but I just wanted to warn you this.
Anyway, as long as mtdev is not fixed with the same kind of patch,
devices following protocol A will be broken in case of a
EV_SYN_DROPPED event.
I think we should just mention this fact with a comment.
Cheers,
Benjamin
> + }
> +
> + return Success;
> +}
> +
> +static int
> +EvdevAbsMtStateSync(InputInfoPtr pInfo, int *count_after_synreport) {
> + MTSlotInfo slots[_ABS_MT_CNT];
> + int ev_count = 0;
> +
> + /* Get all current slots axes, then check if there is any update required */
> + if (EvdevGetAllSlotVals(pInfo, slots) == Success) {
> + ev_count = EvdevCheckAbsMtAxesChange(pInfo, slots,
> + count_after_synreport);
> + }
> +
> + return ev_count;
> +}
> +
> +static int
> +EvdevAbsStateSync(InputInfoPtr pInfo, int *count_after_synreport) {
> + EvdevPtr device = pInfo->private;
> + int ev_count;
> +
> + /* Sync all ABS_ axes */
> + ev_count = EvdevAbsAxesSync(pInfo);
> + *count_after_synreport += ev_count;
> +
> + /* Sync ABS_MT_ axes for all slots if exists */
> + if (device->num_mt_vals)
> + ev_count += EvdevAbsMtStateSync(pInfo, count_after_synreport);
> +
> + return ev_count;
> +}
> +
> +/**
> + * Synchronize the current state with kernel evdev driver.
> + */
> +static void
> +EvdevSyncState(InputInfoPtr pInfo)
> +{
> + int ev_count = 0;
> + int ev_count_after_synreport = 0;
> + EvdevPtr device = pInfo->private;
> +
> + EvdevGetKernelTime(&device->before_sync_time, device->is_monotonic);
> +
> + ev_count = EvdevKeyStateSync(pInfo);
> + ev_count_after_synreport += ev_count;
> +
> + /*
> + * TODO: sync all led, switch and sound states as well. We probably need
> + * to post events out actively if the new states are different from the
> + * cached ones.
> + */
> +
> + /* sync abs and abs_mt value/limits */
> + ev_count += EvdevAbsStateSync(pInfo, &ev_count_after_synreport);
> +
> + /*
> + * Push SYN_REPORT event out if there is any event injected
> + * during the state synchronization.
> + */
> + if (ev_count_after_synreport)
> + ev_count += EvdevInjectEvent(pInfo, EV_SYN, SYN_REPORT, 0);
> +
> + EvdevGetKernelTime(&device->after_sync_time, device->is_monotonic);
> +
> + xf86IDrvMsg(pInfo, X_INFO,
> + "Sync_State: before %ld.%ld after %ld.%ld injected events=%d\n",
> + device->before_sync_time.tv_sec,
> + device->before_sync_time.tv_usec,
> + device->after_sync_time.tv_sec,
> + device->after_sync_time.tv_usec,
> + ev_count);
> +}
> +
> /* just a magic number to reduce the number of reads */
> #define NUM_EVENTS 16
>
> @@ -1090,6 +1429,7 @@ EvdevReadInput(InputInfoPtr pInfo)
> {
> struct input_event ev[NUM_EVENTS];
> int i, len = sizeof(ev);
> + BOOL sync_evdev_state = FALSE;
>
> while (len == sizeof(ev))
> {
> @@ -1120,9 +1460,23 @@ EvdevReadInput(InputInfoPtr pInfo)
> break;
> }
>
> - for (i = 0; i < len/sizeof(ev[0]); i++)
> - EvdevProcessEvent(pInfo, &ev[i]);
> + for (i = 0; i < len/sizeof(ev[0]); i++) {
> + if (sync_evdev_state)
> + break;
> + if (timercmp(&ev[i].time, &pEvdev->before_sync_time, <)) {
> + /* Ignore events before last sync time */
> + continue;
> + } else if (timercmp(&ev[i].time, &pEvdev->after_sync_time, >)) {
> + /* Event_Process returns TRUE if SYN_DROPPED detected */
> + sync_evdev_state = EvdevProcessEvent(pInfo, &ev[i]);
> + } else {
> + /* If the event occurred during sync, then sync again */
> + sync_evdev_state = TRUE;
> + }
> + }
> }
> + if (sync_evdev_state)
> + EvdevSyncState(pInfo);
> }
>
> static void
> @@ -1308,6 +1662,7 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device)
> }
>
> for (i = 0; i < num_slots(pEvdev); i++) {
> + pEvdev->cached_tid[i] = -1;
> pEvdev->last_mt_vals[i] = valuator_mask_new(num_mt_axes_total);
> if (!pEvdev->last_mt_vals[i]) {
> xf86IDrvMsg(pInfo, X_ERROR,
> @@ -1833,6 +2188,8 @@ EvdevOn(DeviceIntPtr device)
> Evdev3BEmuOn(pInfo);
> pEvdev->flags |= EVDEV_INITIALIZED;
> device->public.on = TRUE;
> + pEvdev->slot_state = SLOTSTATE_EMPTY;
> + EvdevSyncState(pInfo);
>
> return Success;
> }
> diff --git a/src/evdev.h b/src/evdev.h
> index 58a3fa3..277eff5 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -101,6 +101,12 @@
> /* Number of longs needed to hold the given number of bits */
> #define NLONGS(x) (((x) + LONG_BITS - 1) / LONG_BITS)
>
> +#define _ABS_MT_FIRST ABS_MT_TOUCH_MAJOR
> +#define _ABS_MT_LAST ABS_MT_DISTANCE
> +#define _ABS_MT_CNT (_ABS_MT_LAST - _ABS_MT_FIRST + 1)
> +
> +#define MAX_SLOT_COUNT 64
> +
> /* Function key mode */
> enum fkeymode {
> FKEYMODE_UNKNOWN = 0,
> @@ -251,8 +257,19 @@ typedef struct {
> EventQueueRec queue[EVDEV_MAXQUEUE];
>
> enum fkeymode fkeymode;
> +
> + /* Sync timestamps */
> + unsigned long key_state_bitmask[NLONGS(KEY_CNT)];
> + struct timeval before_sync_time;
> + struct timeval after_sync_time;
> + int32_t cached_tid[MAX_SLOT_COUNT];
> } EvdevRec, *EvdevPtr;
>
> +typedef struct {
> + uint32_t code;
> + int32_t values[MAX_SLOT_COUNT];
> +} MTSlotInfo, *MTSlotInfoPtr;
> +
> /* Event posting functions */
> void EvdevQueueKbdEvent(InputInfoPtr pInfo, struct input_event *ev, int value);
> void EvdevQueueButtonEvent(InputInfoPtr pInfo, int button, int value);
> --
> 1.7.7.3
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
More information about the xorg-devel
mailing list