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

[Oleksandr Andrushchenko <andr2000@gmail.com>]

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@epam.com>
>>>>
>>>> Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
>>>> ---
>>>>    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
> This patch is OK as is.
>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>
Thank you,
can I also put this "Reviewed-by" into the patch?

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@lists.xen.org
https://lists.xen.org/xen-devel