Re: [Xen-devel] [PATCH v1 2/2] xen/kbdif: add multi-touch support

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 20 Jan 2017, Oleksandr Andrushchenko wrote:
> On 01/20/2017 07:52 PM, Stefano Stabellini wrote:
> > On Fri, 20 Jan 2017, Oleksandr Andrushchenko wrote:
> > > On 01/20/2017 12:22 AM, Stefano Stabellini wrote:
> > > > On Thu, 19 Jan 2017, Oleksandr Andrushchenko wrote:
> > > > > From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
> > > > > 
> > > > > Signed-off-by: Oleksandr Andrushchenko
> > > > > <Oleksandr_Andrushchenko@xxxxxxxx>
> > > > > ---
> > > > >    xen/include/public/io/kbdif.h | 216
> > > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > >    1 file changed, 216 insertions(+)
> > > > > 
> > > > > diff --git a/xen/include/public/io/kbdif.h
> > > > > b/xen/include/public/io/kbdif.h
> > > > > index c00faa3af5d2..8d8ba9af1cb1 100644
> > > > > --- a/xen/include/public/io/kbdif.h
> > > > > +++ b/xen/include/public/io/kbdif.h
> > > > > @@ -57,6 +57,12 @@
> > > > >     *      Backends, which support reporting of absolute coordinates
> > > > > for
> > > > > pointer
> > > > >     *      device should set this to 1.
> > > > >     *
> > > > > + * feature-multi-touch
> > > > > + *      Values:         <uint>
> > > > > + *
> > > > > + *      Backends, which support reporting of multi-touch events
> > > > > + *      should set this to 1.
> > > > > + *
> > > > >     *------------------------- Pointer Device Parameters
> > > > > ------------------------
> > > > >     *
> > > > >     * width
> > > > > @@ -87,6 +93,11 @@
> > > > >     *      Request backend to report absolute pointer coordinates
> > > > >     *      (XENKBD_TYPE_POS) instead of relative ones
> > > > > (XENKBD_TYPE_MOTION).
> > > > >     *
> > > > > + * request-multi-touch
> > > > > + *      Values:         <uint>
> > > > > + *
> > > > > + *      Request backend to report multi-touch events.
> > > > > + *
> > > > >     *----------------------- Request Transport Parameters
> > > > > -----------------------
> > > > >     *
> > > > >     * event-channel
> > > > > @@ -106,6 +117,30 @@
> > > > >     *
> > > > >     *      OBSOLETE, not recommended for use.
> > > > >     *      PFN of the shared page.
> > > > > + *
> > > > > + *----------------------- Multi-touch Device Parameters
> > > > > -----------------------
> > > > > + *
> > > > > + * Every multi-touch input device uses a dedicated event
> > > > > frontend/backend
> > > > > + * connection configured via XenStore properties under
> > > > > + * XENKBD_PATH_MTOUCH folder.
> > > > This sentence doesn't seem to match the rest of the patch:
> > > lets break it into smaller parts
> > > > it looks
> > > > like multi-touch devices use the same ring and evtchn used by the
> > > > pointer and keyboard.
> > > *Every* multi-touch input device uses a *dedicated* event frontend/backend
> > > *connection*
> > > So, it is clearly stated that there is a dedicated/separate connection
> > > (which consists of a ring and corresponding event channel: here we have
> > > already discussed this part:
> > > http://marc.info/?l=xen-devel&m=148412731428686):
> > > 
> > > > > > > + * Every multi-touch input device uses a dedicated event ring and
> > > > > > > is
> > > > > > For clarity I would say "Every multi-touch input device uses a
> > > > > > dedicated
> > > > > > frontend/backend connection". That includes a ring, an event
> > > > > > channel,
> > > > > > and xenstore entries.
> > > 
> > > > Also, it is not clear whether multiple multitouch devices are supported
> > > > with a single frontend/backend connection (as you described in
> > > > http://marc.info/?l=xen-devel&m=148412731428686) or not. Please clarify.
> > > Same as above:
> > > 
> > > *Every* multi-touch input device uses a *dedicated* event frontend/backend
> > > *connection*
> > > 
> > > > If only one device is supported, do we need a XENKBD_PATH_MTOUCH folder?
> > > > 
> > > For consistency and simplicity: at
> > > http://marc.info/?l=xen-devel&m=148412731428686 I have an example of how
> > > XenStore entries look like. If you define multiple mtouch devices then
> > > you'll
> > > iterate over folder contents, e.g.
> > > 
> > > /local/domain/11/device/vkbd/0/mtouch/0/width = "3200"
> > > /local/domain/11/device/vkbd/0/mtouch/1/width = "6400"
> > > 
> > > So, you'll have something like for (i = 0; i < num_mtouch_devices; i++) {
> > > process(i); } If you have a single mtouch device nothing changes. Thus, I
> > > see
> > > no reason in making any difference for single or multiple-devices.
> > All right. I got confused because I read first the other email where you
> > still suggested to use the other approach. Sorry. The wording is clear
> > enough.
> No problem, glad we are on the same page now
> > Aside from clarity, XENKBD_PATH_MTOUCH is important to distinguish the
> > width and height parameters of the mtouch device from the ones of the
> > pointers device, is that right?
> Exactly. Also for ring-ref and event-channel

Sorry, now I am confused again :-S
There are no ring-ref and event-channel under mtouch, are there? So, why
would XENKBD_PATH_MTOUCH help distinguish them?


> > This patch is OK as is.
> > 
> > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > 
> Thank you,
> can I also put this "Reviewed-by" into the patch?

Sure, unless there was a misunderstanding.


> Anyways, I have to wait for some other comments if any
> and publish another version of the series with
> comments addressed ("the" in the previous patch)
> > > > > The values below are per emulated multi-touch
> > > > > + * input device:
> > > > > + *
> > > > > + * num-contacts
> > > > > + *      Values:         <uint>
> > > > > + *
> > > > > + *      Number of simultaneous touches reported.
> > > > > + *
> > > > > + * width
> > > > > + *      Values:         <uint>
> > > > > + *
> > > > > + *      Width of the touch area to be used by the frontend
> > > > > + *      while reporting input events, pixels, [0; UINT32_MAX].
> > > > > + *
> > > > > + * height
> > > > > + *      Values:         <uint>
> > > > > + *
> > > > > + *      Height of the touch area to be used by the frontend
> > > > > + *      while reporting input events, pixels, [0; UINT32_MAX].
> > > > >     */
> > > > >      /*
> > > > > @@ -116,6 +151,16 @@
> > > > >    #define XENKBD_TYPE_RESERVED           2
> > > > >    #define XENKBD_TYPE_KEY                3
> > > > >    #define XENKBD_TYPE_POS                4
> > > > > +#define XENKBD_TYPE_MTOUCH             5
> > > > > +
> > > > > +/* Multi-touch event sub-codes */
> > > > > +
> > > > > +#define XENKBD_MT_EV_DOWN              0
> > > > > +#define XENKBD_MT_EV_UP                1
> > > > > +#define XENKBD_MT_EV_MOTION            2
> > > > > +#define XENKBD_MT_EV_SYN               3
> > > > > +#define XENKBD_MT_EV_SHAPE             4
> > > > > +#define XENKBD_MT_EV_ORIENT            5
> > > > >      /*
> > > > >     * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS.
> > > > > @@ -124,11 +169,17 @@
> > > > >    #define XENKBD_DRIVER_NAME             "vkbd"
> > > > >      #define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
> > > > > +#define XENKBD_FIELD_FEAT_MTOUCH       "feature-multi-touch"
> > > > >    #define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
> > > > > +#define XENKBD_FIELD_REQ_MTOUCH        "request-multi-touch"
> > > > >    #define XENKBD_FIELD_RING_GREF         "page-gref"
> > > > >    #define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
> > > > >    #define XENKBD_FIELD_WIDTH             "width"
> > > > >    #define XENKBD_FIELD_HEIGHT            "height"
> > > > > +#define XENKBD_FIELD_NUM_CONTACTS      "num-contacts"
> > > > > +
> > > > > +/* Path entries */
> > > > > +#define XENKBD_PATH_MTOUCH             "mtouch"
> > > > >      /* OBSOLETE, not recommended for use */
> > > > >    #define XENKBD_FIELD_RING_REF          "page-ref"
> > > > > @@ -248,6 +299,170 @@ struct xenkbd_position
> > > > >        int32_t rel_z;
> > > > >    };
> > > > >    +/*
> > > > > + * Multi-touch event and its sub-types
> > > > > + *
> > > > > + * All multi-touch event packets have common header:
> > > > > + *
> > > > > + *         0                1                 2               3
> > > > > octet
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |  _TYPE_MTOUCH  |   event_type   |   contact_id   |    reserved
> > > > > |
> > > > > 4
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 8
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > + * event_type - unt8_t, multi-touch event sub-type, XENKBD_MT_EV_???
> > > > > + * contact_id - unt8_t, ID of the contact
> > > > If multiple devices are supported, don't we need a per multitouch device
> > > > ID to identify the source?
> > > > 
> > > Again, *Every* multi-touch input device uses a *dedicated* event
> > > frontend/backend *connection* Thus, events are passed w/o indices via
> > > dedicated channels
> > > > > + * Touch interactions can consist of one or more contacts.
> > > > > + * For each contact, a series of events is generated, starting
> > > > > + * with a down event, followed by zero or more motion events,
> > > > > + * and ending with an up event. Events relating to the same
> > > > > + * contact point can be identified by the ID of the sequence: contact
> > > > > ID.
> > > > > + * Contact ID may be reused after XENKBD_MT_EV_UP event and
> > > > > + * is in the [0; XENKBD_FIELD_NUM_CONTACTS - 1] range.
> > > > > + *
> > > > > + * For further information please refer to documentation on Wayland
> > > > > [1],
> > > > > + * Linux [2] and Windows [3] multi-touch support.
> > > > > + *
> > > > > + * [1]
> > > > > https://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml
> > > > > + * [2]
> > > > > https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt
> > > > > + * [3]
> > > > > https://msdn.microsoft.com/en-us/library/jj151564(v=vs.85).aspx
> > > > > + *
> > > > > + *
> > > > > + * Multi-touch down event - sent when a new touch is made: touch is
> > > > > assigned
> > > > > + * a unique contact ID, sent with this and consequent events related
> > > > > + * to this touch.
> > > > > + *         0                1                 2               3
> > > > > octet
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |  _TYPE_MTOUCH  |   _MT_EV_DOWN  |   contact_id   |    reserved
> > > > > |
> > > > > 4
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 8
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                               abs_x
> > > > > |
> > > > > 12
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                               abs_y
> > > > > |
> > > > > 16
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 20
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 40
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > + * abs_x - int32_t, absolute X position, in pixels
> > > > > + * abs_y - int32_t, absolute Y position, in pixels
> > > > > + *
> > > > > + * Multi-touch contact release event
> > > > > + *         0                1                 2               3
> > > > > octet
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |  _TYPE_MTOUCH  |  _MT_EV_UP     |   contact_id   |    reserved
> > > > > |
> > > > > 4
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 8
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 40
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > + * Multi-touch motion event
> > > > > + *         0                1                 2               3
> > > > > octet
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |  _TYPE_MTOUCH  |  _MT_EV_MOTION |   contact_id   |    reserved
> > > > > |
> > > > > 4
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 8
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                               abs_x
> > > > > |
> > > > > 12
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                               abs_y
> > > > > |
> > > > > 16
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 20
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 40
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > + * abs_x - int32_t, absolute X position, in pixels,
> > > > > + * abs_y - int32_t, absolute Y position, in pixels,
> > > > > + *
> > > > > + * Multi-touch input synchronization event - shows end of a set of
> > > > > events
> > > > > + * which logically belong together.
> > > > > + *         0                1                 2               3
> > > > > octet
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |  _TYPE_MTOUCH  |  _MT_EV_SYN    |   contact_id   |    reserved
> > > > > |
> > > > > 4
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 8
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 40
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > + * Multi-touch shape event - touch point's shape has changed its
> > > > > shape.
> > > > > + * Shape is approximated by an ellipse through the major and minor
> > > > > axis
> > > > > + * lengths: major is the longer diameter of the ellipse and minor is
> > > > > the
> > > > > + * shorter one. Center of the ellipse is reported via
> > > > > + * XENKBD_MT_EV_DOWN/XENKBD_MT_EV_MOTION events.
> > > > > + *         0                1                 2               3
> > > > > octet
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |  _TYPE_MTOUCH  |  _MT_EV_SHAPE  |   contact_id   |    reserved
> > > > > |
> > > > > 4
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 8
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                               major
> > > > > |
> > > > > 12
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                               minor
> > > > > |
> > > > > 16
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 20
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 40
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > + * major - unt32_t, length of the major axis, pixels
> > > > > + * minor - unt32_t, length of the minor axis, pixels
> > > > > + *
> > > > > + * Multi-touch orientation event - touch point's shape has changed
> > > > > + * its orientation: calculated as a clockwise angle between the major
> > > > > axis
> > > > > + * of the ellipse and positive Y axis in degrees, [-180; +180].
> > > > > + *         0                1                 2               3
> > > > > octet
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |  _TYPE_MTOUCH  |  _MT_EV_ORIENT |   contact_id   |    reserved
> > > > > |
> > > > > 4
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 8
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |           orientation           |            reserved
> > > > > |
> > > > > 12
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 16
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + * |                             reserved
> > > > > |
> > > > > 40
> > > > > + *
> > > > > +----------------+----------------+----------------+----------------+
> > > > > + *
> > > > > + * orientation - int16_t, clockwise angle of the major axis
> > > > > + */
> > > > > +
> > > > > +struct xenkbd_mtouch {
> > > > > +    uint8_t type;             /* XENKBD_TYPE_MTOUCH */
> > > > > +    uint8_t event_type;       /* XENKBD_MT_EV_??? */
> > > > > +    uint8_t contact_id;
> > > > > +    uint8_t reserved[5];      /* reserved for the future use */
> > > > > +    union {
> > > > > +        struct {
> > > > > +            int32_t abs_x;    /* absolute X position, pixels */
> > > > > +            int32_t abs_y;    /* absolute Y position, pixels */
> > > > > +        } pos;
> > > > > +        struct {
> > > > > +            uint32_t major;   /* length of the major axis, pixels */
> > > > > +            uint32_t minor;   /* length of the minor axis, pixels */
> > > > > +        } shape;
> > > > > +        int16_t orientation;  /* clockwise angle of the major axis */
> > > > > +    } u;
> > > > > +};
> > > > > +
> > > > >    #define XENKBD_IN_EVENT_SIZE 40
> > > > >      union xenkbd_in_event
> > > > > @@ -256,6 +471,7 @@ union xenkbd_in_event
> > > > >        struct xenkbd_motion motion;
> > > > >        struct xenkbd_key key;
> > > > >        struct xenkbd_position pos;
> > > > > +    struct xenkbd_mtouch mtouch;
> > > > >        char pad[XENKBD_IN_EVENT_SIZE];
> > > > >    };
> > > > >    
> > > > > -- 
> > > > > 2.7.4
> > > > > 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel