[PATCH 4/4] Use mtdev to receive all events using MT Slots protocol

[Peter Hutterer]

On Wed, Jun 23, 2010 at 10:04:40AM -0400, Chase Douglas wrote:
> mtdev translates protocol A evdev devices into protocol B devices. We
> just feed events through it to get what we want.
> 
> Hopefully someday the kernel will standardize on the MT Slots protocol
> and we can remove this.
> 
> Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
> ---
>  configure.ac    |    6 ++++++
>  src/Makefile.am |    2 +-
>  src/evdev.c     |   33 +++++++++++++++++++++++++++++++--
>  src/evdev.h     |    3 +++
>  4 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 63460b7..2b34b25 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -56,6 +56,12 @@ AC_ARG_WITH(xorg-module-dir,
>  inputdir=${moduledir}/input
>  AC_SUBST(inputdir)
>  
> +AC_ARG_WITH(mtdev,
> +            AC_HELP_STRING([--with-mtdev],
> +                           [Use mtdev library for multitouch evdev protocol translation [default=disabled]]),
> +            [PKG_CHECK_MODULES(MTDEV, mtdev) AC_DEFINE(USE_MTDEV, 1, Whether to use mtdev or not)],
> +            [AC_DEFINE(USE_MTDEV, 0, Whether to use mtdev or not)])
> +

maybe "auto" instead?

>  # X Server SDK location is required to install evdev header files
>  # This location is also relayed in the xorg-evdev.pc file
>  sdkdir=`$PKG_CONFIG --variable=sdkdir xorg-server`
> diff --git a/src/Makefile.am b/src/Makefile.am
> index a5c89ac..caa36d2 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -29,7 +29,7 @@ AM_CFLAGS = $(XORG_CFLAGS) $(CWARNFLAGS)
>  AM_CPPFLAGS =-I$(top_srcdir)/include
>  
>  @DRIVER_NAME at _drv_la_LTLIBRARIES = @DRIVER_NAME at _drv.la
> - at DRIVER_NAME@_drv_la_LDFLAGS = -module -avoid-version
> + at DRIVER_NAME@_drv_la_LDFLAGS = -module -avoid-version @MTDEV_LIBS@
>  @DRIVER_NAME at _drv_ladir = @inputdir@
>  
>  @DRIVER_NAME at _drv_la_SOURCES = @DRIVER_NAME at .c \
> diff --git a/src/evdev.c b/src/evdev.c
> index 777eebe..4564cdd 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -66,9 +66,13 @@
>  #define MAXDEVICES MAX_DEVICES
>  #endif
>  
> +#if USE_MTDEV
> +#include <mtdev.h>
> +#else
>  #ifndef ABS_MT_SLOT
>  #define ABS_MT_SLOT 0x2f
>  #endif
> +#endif /* USE_MTDEV */
>  
>  #define ArrayLength(a) (sizeof(a) / (sizeof((a)[0])))
>  
> @@ -750,7 +754,16 @@ EvdevReadInput(InputInfoPtr pInfo)
>  
>      while (len == sizeof(ev))
>      {
> -        len = read(pInfo->fd, &ev, sizeof(ev));
> +#if USE_MTDEV
> +        EvdevPtr pEvdev = pInfo->private;
> +        if (pEvdev->mtdevPtr) {

new line for the {


> +            len = mtdev_get(pEvdev->mtdevPtr, pInfo->fd, ev, NUM_EVENTS);
> +            if (len > 0)
> +                len *= sizeof(struct input_event);
> +        }
> +        else

no new line for the else

> +#endif /* USE_MTDEV */
> +            len = read(pInfo->fd, &ev, sizeof(ev));
>          if (len <= 0)
>          {
>              if (errno == ENODEV) /* May happen after resume */
> @@ -1805,6 +1818,12 @@ EvdevProc(DeviceIntPtr device, int what)
>              close(pInfo->fd);
>              pInfo->fd = -1;
>          }
> +#if USE_MTDEV
> +        if (pEvdev->mtdevPtr) {
> +            mtdev_close(pEvdev->mtdevPtr);
> +            free(pEvdev->mtdevPtr);

this should reset pEvdev->mtdevPtr to NULL.

> +        }
> +#endif /* USE_MTDEV */
>          EvdevRemoveDevice(pInfo);
>          pEvdev->min_maj = 0;
>  	break;
> @@ -2093,13 +2112,23 @@ EvdevProbe(InputInfoPtr pInfo)
>  
>          if (TestBit(ABS_MT_POSITION_X, pEvdev->abs_bitmask) &&
>              TestBit(ABS_MT_POSITION_Y, pEvdev->abs_bitmask) &&
> -            TestBit(ABS_MT_SLOT, pEvdev->abs_bitmask)) {
> +            (TestBit(ABS_MT_SLOT, pEvdev->abs_bitmask) || USE_MTDEV)) {

if we're not doing anything with the device without mtdev it'd be good to
state so in the log. "multitouch device but compiled without mt support." or
so.

that goes for the rest of the code, if mtdev is required, we might as well
ignore all MT events.

>              xf86Msg(X_INFO, "%s: Found absolute multitouch device.\n", pInfo->name);
>              pEvdev->flags |= EVDEV_MULTITOUCH;
>              if (!pEvdev->num_buttons) {
>                  pEvdev->num_buttons = 7; /* LMR + scroll wheels */
>                  pEvdev->flags |= EVDEV_BUTTON_EVENTS;
>              }
> +
> +#if USE_MTDEV
> +            pEvdev->mtdevPtr = malloc(sizeof(struct mtdev));
> +            if (!pEvdev->mtdevPtr || mtdev_open(pEvdev->mtdevPtr, pInfo->fd)) {
> +                free(pEvdev->mtdevPtr);
> +                pEvdev->mtdevPtr = NULL;
> +                xf86Msg(X_WARNING, "%s: failed to initialize mtdev.\n",
> +                        pInfo->name);
> +            }
> +#endif /* USE_MTDEV */
>          }
>          if ((TestBit(ABS_X, pEvdev->abs_bitmask) &&
>               TestBit(ABS_Y, pEvdev->abs_bitmask))) {
> diff --git a/src/evdev.h b/src/evdev.h
> index 25c63a7..b528589 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -198,6 +198,9 @@ typedef struct {
>      unsigned int mt_max_touchpoints; /* the number of simultaneous touchpoints
>                                        * the device can support */
>      unsigned int mt_current_touchpoint;
> +#if USE_MTDEV
> +    struct mtdev *mtdevPtr;

urgh, please don't name this mtdevPtr. just mtdev will do. this Ptr/Rec
naming is a disease.

> +#endif
>  } EvdevRec, *EvdevPtr;
>  
>  /* Event posting functions */
> -- 
> 1.7.0.4

The rest of the patch looks ok, though I'd prefer to move the MT stuff out
of the standard to avoid confusion with too many ifdefs.

so instead of having the block of code inside EvdevProbe, I'd like it to be
just

#if USE_MTDEV
        EvdevMTInit(...);
#endif

with the actual code in that function then. Right now, we dont need a
separate file for these but I think it would make the code more readable.
 
Cheers,
  Peter