All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] xen/kbdif: add multi-touch support
@ 2017-01-19  9:24 Oleksandr Andrushchenko
  2017-01-19  9:24 ` [PATCH v1 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
  2017-01-19  9:24 ` [PATCH v1 2/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
  0 siblings, 2 replies; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-19  9:24 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, konrad.wilk, vlad.babchuk, andrii.anisov, olekstysh,
	andr2000, al1img, joculator

From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>

Hi, all!

This series updates existing kbdif protocol documentation
and adds multi-touch support

Thank you,
Oleksandr Andrushchenko

Oleksandr Andrushchenko (2):
  xen/kbdif: update protocol documentation
  xen/kbdif: add multi-touch support

 xen/include/public/io/kbdif.h | 464 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 437 insertions(+), 27 deletions(-)

-- 
2.7.4


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v1 1/2] xen/kbdif: update protocol documentation
  2017-01-19  9:24 [PATCH v1 0/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
@ 2017-01-19  9:24 ` Oleksandr Andrushchenko
  2017-01-19 18:56   ` Stefano Stabellini
  2017-01-19  9:24 ` [PATCH v1 2/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
  1 sibling, 1 reply; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-19  9:24 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, konrad.wilk, vlad.babchuk, andrii.anisov, olekstysh,
	andr2000, al1img, joculator

From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>

Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
---
 xen/include/public/io/kbdif.h | 248 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 221 insertions(+), 27 deletions(-)

diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
index 2d2aebdd3f28..c00faa3af5d2 100644
--- a/xen/include/public/io/kbdif.h
+++ b/xen/include/public/io/kbdif.h
@@ -26,46 +26,226 @@
 #ifndef __XEN_PUBLIC_IO_KBDIF_H__
 #define __XEN_PUBLIC_IO_KBDIF_H__
 
-/* In events (backend -> frontend) */
+/*
+ *****************************************************************************
+ *                     Feature and Parameter Negotiation
+ *****************************************************************************
+ *
+ * The two halves of a para-virtual driver utilize nodes within the
+ * XenStore to communicate capabilities and to negotiate operating parameters.
+ * This section enumerates these nodes which reside in the respective front and
+ * backend portions of XenStore, following XenBus convention.
+ *
+ * All data in XenStore is stored as strings.  Nodes specifying numeric
+ * values are encoded in decimal. Integer value ranges listed below are
+ * expressed as fixed sized integer types capable of storing the conversion
+ * of a properly formated node string, without loss of information.
+ *
+ *****************************************************************************
+ *                            Backend XenBus Nodes
+ *****************************************************************************
+ *
+ *---------------------------- Features supported ----------------------------
+ *
+ * Capable backend advertises supported features by publishing
+ * corresponding entries in XenStore and puts 1 as the value of the entry.
+ * If a feature is not supported then 0 must be set or feature entry omitted.
+ *
+ * feature-abs-pointer
+ *      Values:         <uint>
+ *
+ *      Backends, which support reporting of absolute coordinates for pointer
+ *      device should set this to 1.
+ *
+ *------------------------- Pointer Device Parameters ------------------------
+ *
+ * width
+ *      Values:         <uint>
+ *
+ *      Maximum X coordinate (width) to be used by the frontend
+ *      while reporting input events, pixels, [0; UINT32_MAX].
+ *
+ * height
+ *      Values:         <uint>
+ *
+ *      Maximum Y coordinate (height) to be used by the frontend
+ *      while reporting input events, pixels, [0; UINT32_MAX].
+ *
+ *****************************************************************************
+ *                            Frontend XenBus Nodes
+ *****************************************************************************
+ *
+ *------------------------------ Feature request -----------------------------
+ *
+ * Capable frontend requests features from backend via setting corresponding
+ * entries to 1 in XenStore. Requests for features not advertised as supported
+ * by the backend have no effect.
+ *
+ * request-abs-pointer
+ *      Values:         <uint>
+ *
+ *      Request backend to report absolute pointer coordinates
+ *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
+ *
+ *----------------------- Request Transport Parameters -----------------------
+ *
+ * event-channel
+ *      Values:         <uint>
+ *
+ *      The identifier of the Xen event channel used to signal activity
+ *      in the ring buffer.
+ *
+ * page-gref
+ *      Values:         <uint>
+ *
+ *      The Xen grant reference granting permission for the backend to map
+ *      a sole page in a single page sized event ring buffer.
+ *
+ * page-ref
+ *      Values:         <uint>
+ *
+ *      OBSOLETE, not recommended for use.
+ *      PFN of the shared page.
+ */
 
 /*
- * Frontends should ignore unknown in events.
+ * EVENT CODES.
  */
 
-/* Pointer movement event */
-#define XENKBD_TYPE_MOTION  1
-/* Event type 2 currently not used */
-/* Key event (includes pointer buttons) */
-#define XENKBD_TYPE_KEY     3
+#define XENKBD_TYPE_MOTION             1
+#define XENKBD_TYPE_RESERVED           2
+#define XENKBD_TYPE_KEY                3
+#define XENKBD_TYPE_POS                4
+
 /*
- * Pointer position event
- * Capable backend sets feature-abs-pointer in xenstore.
- * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting
- * request-abs-update in xenstore.
+ * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS.
+ */
+
+#define XENKBD_DRIVER_NAME             "vkbd"
+
+#define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
+#define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
+#define XENKBD_FIELD_RING_GREF         "page-gref"
+#define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
+#define XENKBD_FIELD_WIDTH             "width"
+#define XENKBD_FIELD_HEIGHT            "height"
+
+/* OBSOLETE, not recommended for use */
+#define XENKBD_FIELD_RING_REF          "page-ref"
+
+/*
+ *****************************************************************************
+ * Description of the protocol between frontend and backend driver.
+ *****************************************************************************
+ *
+ * The two halves of a Para-virtual driver communicate with
+ * each other using a shared page and an event channel.
+ * Shared page contains a ring with event structures.
+ *
+ * All reserved fields in the structures below must be 0.
+ *
+ *****************************************************************************
+ *                           Backend to frontend events
+ *****************************************************************************
+ *
+ * Frontends should ignore unknown in events.
+ * All event packets have the same length (40 octets)
+ * All event packets have common header:
+ *
+ *          0         octet
+ * +-----------------+
+ * |       type      |
+ * +-----------------+
+ * type - uint8_t, event code, XENKBD_TYPE_???
+ *
+ *
+ * Pointer relative movement event
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |  _TYPE_MOTION  |                     reserved                     | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                               rel_x                               | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                               rel_y                               | 12
+ * +----------------+----------------+----------------+----------------+
+ * |                               rel_z                               | 16
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 20
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 40
+ * +----------------+----------------+----------------+----------------+
+ *
+ * rel_x - int32_t, relative X motion
+ * rel_y - int32_t, relative Y motion
+ * rel_z - int32_t, relative Z motion (wheel)
  */
-#define XENKBD_TYPE_POS     4
 
 struct xenkbd_motion
 {
-    uint8_t type;        /* XENKBD_TYPE_MOTION */
-    int32_t rel_x;       /* relative X motion */
-    int32_t rel_y;       /* relative Y motion */
-    int32_t rel_z;       /* relative Z motion (wheel) */
+    uint8_t type;
+    int32_t rel_x;
+    int32_t rel_y;
+    int32_t rel_z;
 };
 
+/*
+ * Key event (includes pointer buttons)
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |  _TYPE_KEY     |     pressed    |            reserved             | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                              keycode                              | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 12
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 40
+ * +----------------+----------------+----------------+----------------+
+ *
+ * pressed - uint8_t, 1 if pressed; 0 otherwise
+ * keycode - uint32_t, KEY_* from linux/input.h
+ */
+
 struct xenkbd_key
 {
-    uint8_t type;         /* XENKBD_TYPE_KEY */
-    uint8_t pressed;      /* 1 if pressed; 0 otherwise */
-    uint32_t keycode;     /* KEY_* from linux/input.h */
+    uint8_t type;
+    uint8_t pressed;
+    uint32_t keycode;
 };
 
+/*
+ * Pointer absolute position event
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |  _TYPE_POS     |                     reserved                     | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                               abs_x                               | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                               abs_y                               | 12
+ * +----------------+----------------+----------------+----------------+
+ * |                               rel_z                               | 16
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 20
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 40
+ * +----------------+----------------+----------------+----------------+
+ *
+ * abs_x - int32_t, absolute X position (in FB pixels)
+ * abs_y - int32_t, absolute Y position (in FB pixels)
+ * rel_z - int32_t, relative Z motion (wheel)
+ */
+
 struct xenkbd_position
 {
-    uint8_t type;        /* XENKBD_TYPE_POS */
-    int32_t abs_x;       /* absolute X position (in FB pixels) */
-    int32_t abs_y;       /* absolute Y position (in FB pixels) */
-    int32_t rel_z;       /* relative Z motion (wheel) */
+    uint8_t type;
+    int32_t abs_x;
+    int32_t abs_y;
+    int32_t rel_z;
 };
 
 #define XENKBD_IN_EVENT_SIZE 40
@@ -79,12 +259,22 @@ union xenkbd_in_event
     char pad[XENKBD_IN_EVENT_SIZE];
 };
 
-/* Out events (frontend -> backend) */
-
 /*
+ *****************************************************************************
+ *                            Frontend to backend events
+ *****************************************************************************
+ *
  * Out events may be sent only when requested by backend, and receipt
  * of an unknown out event is an error.
  * No out events currently defined.
+
+ * All event packets have the same length (40 octets)
+ * All event packets have common header:
+ *          0         octet
+ * +-----------------+
+ * |       type      |
+ * +-----------------+
+ * type - uint8_t, event code
  */
 
 #define XENKBD_OUT_EVENT_SIZE 40
@@ -95,7 +285,11 @@ union xenkbd_out_event
     char pad[XENKBD_OUT_EVENT_SIZE];
 };
 
-/* shared page */
+/*
+ *****************************************************************************
+ *                            Shared page
+ *****************************************************************************
+ */
 
 #define XENKBD_IN_RING_SIZE 2048
 #define XENKBD_IN_RING_LEN (XENKBD_IN_RING_SIZE / XENKBD_IN_EVENT_SIZE)
@@ -119,7 +313,7 @@ struct xenkbd_page
     uint32_t out_cons, out_prod;
 };
 
-#endif
+#endif /* __XEN_PUBLIC_IO_KBDIF_H__ */
 
 /*
  * Local variables:
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-19  9:24 [PATCH v1 0/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
  2017-01-19  9:24 ` [PATCH v1 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
@ 2017-01-19  9:24 ` Oleksandr Andrushchenko
  2017-01-19 22:22   ` Stefano Stabellini
  1 sibling, 1 reply; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-19  9:24 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, konrad.wilk, vlad.babchuk, andrii.anisov, olekstysh,
	andr2000, al1img, joculator

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. 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
+ *
+ * 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/2] xen/kbdif: update protocol documentation
  2017-01-19  9:24 ` [PATCH v1 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
@ 2017-01-19 18:56   ` Stefano Stabellini
  2017-01-20  6:58     ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2017-01-19 18:56 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: sstabellini, konrad.wilk, vlad.babchuk, andrii.anisov, olekstysh,
	al1img, xen-devel, joculator

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 | 248 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 221 insertions(+), 27 deletions(-)
> 
> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> index 2d2aebdd3f28..c00faa3af5d2 100644
> --- a/xen/include/public/io/kbdif.h
> +++ b/xen/include/public/io/kbdif.h
> @@ -26,46 +26,226 @@
>  #ifndef __XEN_PUBLIC_IO_KBDIF_H__
>  #define __XEN_PUBLIC_IO_KBDIF_H__
>  
> -/* In events (backend -> frontend) */
> +/*
> + *****************************************************************************
> + *                     Feature and Parameter Negotiation
> + *****************************************************************************
> + *
> + * The two halves of a para-virtual driver utilize nodes within the

within "XenStore", drop "the"

Aside from this minor this:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> + * XenStore to communicate capabilities and to negotiate operating parameters.
> + * This section enumerates these nodes which reside in the respective front and
> + * backend portions of XenStore, following XenBus convention.
> + *
> + * All data in XenStore is stored as strings.  Nodes specifying numeric
> + * values are encoded in decimal. Integer value ranges listed below are
> + * expressed as fixed sized integer types capable of storing the conversion
> + * of a properly formated node string, without loss of information.
> + *
> + *****************************************************************************
> + *                            Backend XenBus Nodes
> + *****************************************************************************
> + *
> + *---------------------------- Features supported ----------------------------
> + *
> + * Capable backend advertises supported features by publishing
> + * corresponding entries in XenStore and puts 1 as the value of the entry.
> + * If a feature is not supported then 0 must be set or feature entry omitted.
> + *
> + * feature-abs-pointer
> + *      Values:         <uint>
> + *
> + *      Backends, which support reporting of absolute coordinates for pointer
> + *      device should set this to 1.
> + *
> + *------------------------- Pointer Device Parameters ------------------------
> + *
> + * width
> + *      Values:         <uint>
> + *
> + *      Maximum X coordinate (width) to be used by the frontend
> + *      while reporting input events, pixels, [0; UINT32_MAX].
> + *
> + * height
> + *      Values:         <uint>
> + *
> + *      Maximum Y coordinate (height) to be used by the frontend
> + *      while reporting input events, pixels, [0; UINT32_MAX].
> + *
> + *****************************************************************************
> + *                            Frontend XenBus Nodes
> + *****************************************************************************
> + *
> + *------------------------------ Feature request -----------------------------
> + *
> + * Capable frontend requests features from backend via setting corresponding
> + * entries to 1 in XenStore. Requests for features not advertised as supported
> + * by the backend have no effect.
> + *
> + * request-abs-pointer
> + *      Values:         <uint>
> + *
> + *      Request backend to report absolute pointer coordinates
> + *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
> + *
> + *----------------------- Request Transport Parameters -----------------------
> + *
> + * event-channel
> + *      Values:         <uint>
> + *
> + *      The identifier of the Xen event channel used to signal activity
> + *      in the ring buffer.
> + *
> + * page-gref
> + *      Values:         <uint>
> + *
> + *      The Xen grant reference granting permission for the backend to map
> + *      a sole page in a single page sized event ring buffer.
> + *
> + * page-ref
> + *      Values:         <uint>
> + *
> + *      OBSOLETE, not recommended for use.
> + *      PFN of the shared page.
> + */
>  
>  /*
> - * Frontends should ignore unknown in events.
> + * EVENT CODES.
>   */
>  
> -/* Pointer movement event */
> -#define XENKBD_TYPE_MOTION  1
> -/* Event type 2 currently not used */
> -/* Key event (includes pointer buttons) */
> -#define XENKBD_TYPE_KEY     3
> +#define XENKBD_TYPE_MOTION             1
> +#define XENKBD_TYPE_RESERVED           2
> +#define XENKBD_TYPE_KEY                3
> +#define XENKBD_TYPE_POS                4
> +
>  /*
> - * Pointer position event
> - * Capable backend sets feature-abs-pointer in xenstore.
> - * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting
> - * request-abs-update in xenstore.
> + * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS.
> + */
> +
> +#define XENKBD_DRIVER_NAME             "vkbd"
> +
> +#define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
> +#define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
> +#define XENKBD_FIELD_RING_GREF         "page-gref"
> +#define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
> +#define XENKBD_FIELD_WIDTH             "width"
> +#define XENKBD_FIELD_HEIGHT            "height"
> +
> +/* OBSOLETE, not recommended for use */
> +#define XENKBD_FIELD_RING_REF          "page-ref"
> +
> +/*
> + *****************************************************************************
> + * Description of the protocol between frontend and backend driver.
> + *****************************************************************************
> + *
> + * The two halves of a Para-virtual driver communicate with
> + * each other using a shared page and an event channel.
> + * Shared page contains a ring with event structures.
> + *
> + * All reserved fields in the structures below must be 0.
> + *
> + *****************************************************************************
> + *                           Backend to frontend events
> + *****************************************************************************
> + *
> + * Frontends should ignore unknown in events.
> + * All event packets have the same length (40 octets)
> + * All event packets have common header:
> + *
> + *          0         octet
> + * +-----------------+
> + * |       type      |
> + * +-----------------+
> + * type - uint8_t, event code, XENKBD_TYPE_???
> + *
> + *
> + * Pointer relative movement event
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |  _TYPE_MOTION  |                     reserved                     | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                               rel_x                               | 8
> + * +----------------+----------------+----------------+----------------+
> + * |                               rel_y                               | 12
> + * +----------------+----------------+----------------+----------------+
> + * |                               rel_z                               | 16
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 20
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 40
> + * +----------------+----------------+----------------+----------------+
> + *
> + * rel_x - int32_t, relative X motion
> + * rel_y - int32_t, relative Y motion
> + * rel_z - int32_t, relative Z motion (wheel)
>   */
> -#define XENKBD_TYPE_POS     4
>  
>  struct xenkbd_motion
>  {
> -    uint8_t type;        /* XENKBD_TYPE_MOTION */
> -    int32_t rel_x;       /* relative X motion */
> -    int32_t rel_y;       /* relative Y motion */
> -    int32_t rel_z;       /* relative Z motion (wheel) */
> +    uint8_t type;
> +    int32_t rel_x;
> +    int32_t rel_y;
> +    int32_t rel_z;
>  };
>  
> +/*
> + * Key event (includes pointer buttons)
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |  _TYPE_KEY     |     pressed    |            reserved             | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                              keycode                              | 8
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 12
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 40
> + * +----------------+----------------+----------------+----------------+
> + *
> + * pressed - uint8_t, 1 if pressed; 0 otherwise
> + * keycode - uint32_t, KEY_* from linux/input.h
> + */
> +
>  struct xenkbd_key
>  {
> -    uint8_t type;         /* XENKBD_TYPE_KEY */
> -    uint8_t pressed;      /* 1 if pressed; 0 otherwise */
> -    uint32_t keycode;     /* KEY_* from linux/input.h */
> +    uint8_t type;
> +    uint8_t pressed;
> +    uint32_t keycode;
>  };
>  
> +/*
> + * Pointer absolute position event
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |  _TYPE_POS     |                     reserved                     | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                               abs_x                               | 8
> + * +----------------+----------------+----------------+----------------+
> + * |                               abs_y                               | 12
> + * +----------------+----------------+----------------+----------------+
> + * |                               rel_z                               | 16
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 20
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 40
> + * +----------------+----------------+----------------+----------------+
> + *
> + * abs_x - int32_t, absolute X position (in FB pixels)
> + * abs_y - int32_t, absolute Y position (in FB pixels)
> + * rel_z - int32_t, relative Z motion (wheel)
> + */
> +
>  struct xenkbd_position
>  {
> -    uint8_t type;        /* XENKBD_TYPE_POS */
> -    int32_t abs_x;       /* absolute X position (in FB pixels) */
> -    int32_t abs_y;       /* absolute Y position (in FB pixels) */
> -    int32_t rel_z;       /* relative Z motion (wheel) */
> +    uint8_t type;
> +    int32_t abs_x;
> +    int32_t abs_y;
> +    int32_t rel_z;
>  };
>  
>  #define XENKBD_IN_EVENT_SIZE 40
> @@ -79,12 +259,22 @@ union xenkbd_in_event
>      char pad[XENKBD_IN_EVENT_SIZE];
>  };
>  
> -/* Out events (frontend -> backend) */
> -
>  /*
> + *****************************************************************************
> + *                            Frontend to backend events
> + *****************************************************************************
> + *
>   * Out events may be sent only when requested by backend, and receipt
>   * of an unknown out event is an error.
>   * No out events currently defined.
> +
> + * All event packets have the same length (40 octets)
> + * All event packets have common header:
> + *          0         octet
> + * +-----------------+
> + * |       type      |
> + * +-----------------+
> + * type - uint8_t, event code
>   */
>  
>  #define XENKBD_OUT_EVENT_SIZE 40
> @@ -95,7 +285,11 @@ union xenkbd_out_event
>      char pad[XENKBD_OUT_EVENT_SIZE];
>  };
>  
> -/* shared page */
> +/*
> + *****************************************************************************
> + *                            Shared page
> + *****************************************************************************
> + */
>  
>  #define XENKBD_IN_RING_SIZE 2048
>  #define XENKBD_IN_RING_LEN (XENKBD_IN_RING_SIZE / XENKBD_IN_EVENT_SIZE)
> @@ -119,7 +313,7 @@ struct xenkbd_page
>      uint32_t out_cons, out_prod;
>  };
>  
> -#endif
> +#endif /* __XEN_PUBLIC_IO_KBDIF_H__ */
>  
>  /*
>   * Local variables:
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-19  9:24 ` [PATCH v1 2/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
@ 2017-01-19 22:22   ` Stefano Stabellini
  2017-01-20  7:27     ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2017-01-19 22:22 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: sstabellini, konrad.wilk, vlad.babchuk, andrii.anisov, olekstysh,
	al1img, xen-devel, joculator

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: it looks
like multi-touch devices use the same ring and evtchn used by the
pointer and keyboard.

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.

If only one device is supported, do we need a XENKBD_PATH_MTOUCH folder?



> 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?


> + * 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/2] xen/kbdif: update protocol documentation
  2017-01-19 18:56   ` Stefano Stabellini
@ 2017-01-20  6:58     ` Oleksandr Andrushchenko
  2017-01-20 17:43       ` Stefano Stabellini
  0 siblings, 1 reply; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-20  6:58 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: konrad.wilk, vlad.babchuk, andrii.anisov, olekstysh, al1img,
	xen-devel, joculator

On 01/19/2017 08:56 PM, 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 | 248 +++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 221 insertions(+), 27 deletions(-)
>>
>> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
>> index 2d2aebdd3f28..c00faa3af5d2 100644
>> --- a/xen/include/public/io/kbdif.h
>> +++ b/xen/include/public/io/kbdif.h
>> @@ -26,46 +26,226 @@
>>   #ifndef __XEN_PUBLIC_IO_KBDIF_H__
>>   #define __XEN_PUBLIC_IO_KBDIF_H__
>>   
>> -/* In events (backend -> frontend) */
>> +/*
>> + *****************************************************************************
>> + *                     Feature and Parameter Negotiation
>> + *****************************************************************************
>> + *
>> + * The two halves of a para-virtual driver utilize nodes within the
> within "XenStore", drop "the"
>
> Aside from this minor this:
>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Thanks,
with this change done, can I put your Reviewed-by
into the next version of this patch?
>
>> + * XenStore to communicate capabilities and to negotiate operating parameters.
>> + * This section enumerates these nodes which reside in the respective front and
>> + * backend portions of XenStore, following XenBus convention.
>> + *
>> + * All data in XenStore is stored as strings.  Nodes specifying numeric
>> + * values are encoded in decimal. Integer value ranges listed below are
>> + * expressed as fixed sized integer types capable of storing the conversion
>> + * of a properly formated node string, without loss of information.
>> + *
>> + *****************************************************************************
>> + *                            Backend XenBus Nodes
>> + *****************************************************************************
>> + *
>> + *---------------------------- Features supported ----------------------------
>> + *
>> + * Capable backend advertises supported features by publishing
>> + * corresponding entries in XenStore and puts 1 as the value of the entry.
>> + * If a feature is not supported then 0 must be set or feature entry omitted.
>> + *
>> + * feature-abs-pointer
>> + *      Values:         <uint>
>> + *
>> + *      Backends, which support reporting of absolute coordinates for pointer
>> + *      device should set this to 1.
>> + *
>> + *------------------------- Pointer Device Parameters ------------------------
>> + *
>> + * width
>> + *      Values:         <uint>
>> + *
>> + *      Maximum X coordinate (width) to be used by the frontend
>> + *      while reporting input events, pixels, [0; UINT32_MAX].
>> + *
>> + * height
>> + *      Values:         <uint>
>> + *
>> + *      Maximum Y coordinate (height) to be used by the frontend
>> + *      while reporting input events, pixels, [0; UINT32_MAX].
>> + *
>> + *****************************************************************************
>> + *                            Frontend XenBus Nodes
>> + *****************************************************************************
>> + *
>> + *------------------------------ Feature request -----------------------------
>> + *
>> + * Capable frontend requests features from backend via setting corresponding
>> + * entries to 1 in XenStore. Requests for features not advertised as supported
>> + * by the backend have no effect.
>> + *
>> + * request-abs-pointer
>> + *      Values:         <uint>
>> + *
>> + *      Request backend to report absolute pointer coordinates
>> + *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
>> + *
>> + *----------------------- Request Transport Parameters -----------------------
>> + *
>> + * event-channel
>> + *      Values:         <uint>
>> + *
>> + *      The identifier of the Xen event channel used to signal activity
>> + *      in the ring buffer.
>> + *
>> + * page-gref
>> + *      Values:         <uint>
>> + *
>> + *      The Xen grant reference granting permission for the backend to map
>> + *      a sole page in a single page sized event ring buffer.
>> + *
>> + * page-ref
>> + *      Values:         <uint>
>> + *
>> + *      OBSOLETE, not recommended for use.
>> + *      PFN of the shared page.
>> + */
>>   
>>   /*
>> - * Frontends should ignore unknown in events.
>> + * EVENT CODES.
>>    */
>>   
>> -/* Pointer movement event */
>> -#define XENKBD_TYPE_MOTION  1
>> -/* Event type 2 currently not used */
>> -/* Key event (includes pointer buttons) */
>> -#define XENKBD_TYPE_KEY     3
>> +#define XENKBD_TYPE_MOTION             1
>> +#define XENKBD_TYPE_RESERVED           2
>> +#define XENKBD_TYPE_KEY                3
>> +#define XENKBD_TYPE_POS                4
>> +
>>   /*
>> - * Pointer position event
>> - * Capable backend sets feature-abs-pointer in xenstore.
>> - * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting
>> - * request-abs-update in xenstore.
>> + * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS.
>> + */
>> +
>> +#define XENKBD_DRIVER_NAME             "vkbd"
>> +
>> +#define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
>> +#define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
>> +#define XENKBD_FIELD_RING_GREF         "page-gref"
>> +#define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
>> +#define XENKBD_FIELD_WIDTH             "width"
>> +#define XENKBD_FIELD_HEIGHT            "height"
>> +
>> +/* OBSOLETE, not recommended for use */
>> +#define XENKBD_FIELD_RING_REF          "page-ref"
>> +
>> +/*
>> + *****************************************************************************
>> + * Description of the protocol between frontend and backend driver.
>> + *****************************************************************************
>> + *
>> + * The two halves of a Para-virtual driver communicate with
>> + * each other using a shared page and an event channel.
>> + * Shared page contains a ring with event structures.
>> + *
>> + * All reserved fields in the structures below must be 0.
>> + *
>> + *****************************************************************************
>> + *                           Backend to frontend events
>> + *****************************************************************************
>> + *
>> + * Frontends should ignore unknown in events.
>> + * All event packets have the same length (40 octets)
>> + * All event packets have common header:
>> + *
>> + *          0         octet
>> + * +-----------------+
>> + * |       type      |
>> + * +-----------------+
>> + * type - uint8_t, event code, XENKBD_TYPE_???
>> + *
>> + *
>> + * Pointer relative movement event
>> + *         0                1                 2               3        octet
>> + * +----------------+----------------+----------------+----------------+
>> + * |  _TYPE_MOTION  |                     reserved                     | 4
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               rel_x                               | 8
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               rel_y                               | 12
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               rel_z                               | 16
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 20
>> + * +----------------+----------------+----------------+----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 40
>> + * +----------------+----------------+----------------+----------------+
>> + *
>> + * rel_x - int32_t, relative X motion
>> + * rel_y - int32_t, relative Y motion
>> + * rel_z - int32_t, relative Z motion (wheel)
>>    */
>> -#define XENKBD_TYPE_POS     4
>>   
>>   struct xenkbd_motion
>>   {
>> -    uint8_t type;        /* XENKBD_TYPE_MOTION */
>> -    int32_t rel_x;       /* relative X motion */
>> -    int32_t rel_y;       /* relative Y motion */
>> -    int32_t rel_z;       /* relative Z motion (wheel) */
>> +    uint8_t type;
>> +    int32_t rel_x;
>> +    int32_t rel_y;
>> +    int32_t rel_z;
>>   };
>>   
>> +/*
>> + * Key event (includes pointer buttons)
>> + *         0                1                 2               3        octet
>> + * +----------------+----------------+----------------+----------------+
>> + * |  _TYPE_KEY     |     pressed    |            reserved             | 4
>> + * +----------------+----------------+----------------+----------------+
>> + * |                              keycode                              | 8
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 12
>> + * +----------------+----------------+----------------+----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 40
>> + * +----------------+----------------+----------------+----------------+
>> + *
>> + * pressed - uint8_t, 1 if pressed; 0 otherwise
>> + * keycode - uint32_t, KEY_* from linux/input.h
>> + */
>> +
>>   struct xenkbd_key
>>   {
>> -    uint8_t type;         /* XENKBD_TYPE_KEY */
>> -    uint8_t pressed;      /* 1 if pressed; 0 otherwise */
>> -    uint32_t keycode;     /* KEY_* from linux/input.h */
>> +    uint8_t type;
>> +    uint8_t pressed;
>> +    uint32_t keycode;
>>   };
>>   
>> +/*
>> + * Pointer absolute position event
>> + *         0                1                 2               3        octet
>> + * +----------------+----------------+----------------+----------------+
>> + * |  _TYPE_POS     |                     reserved                     | 4
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               abs_x                               | 8
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               abs_y                               | 12
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               rel_z                               | 16
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 20
>> + * +----------------+----------------+----------------+----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 40
>> + * +----------------+----------------+----------------+----------------+
>> + *
>> + * abs_x - int32_t, absolute X position (in FB pixels)
>> + * abs_y - int32_t, absolute Y position (in FB pixels)
>> + * rel_z - int32_t, relative Z motion (wheel)
>> + */
>> +
>>   struct xenkbd_position
>>   {
>> -    uint8_t type;        /* XENKBD_TYPE_POS */
>> -    int32_t abs_x;       /* absolute X position (in FB pixels) */
>> -    int32_t abs_y;       /* absolute Y position (in FB pixels) */
>> -    int32_t rel_z;       /* relative Z motion (wheel) */
>> +    uint8_t type;
>> +    int32_t abs_x;
>> +    int32_t abs_y;
>> +    int32_t rel_z;
>>   };
>>   
>>   #define XENKBD_IN_EVENT_SIZE 40
>> @@ -79,12 +259,22 @@ union xenkbd_in_event
>>       char pad[XENKBD_IN_EVENT_SIZE];
>>   };
>>   
>> -/* Out events (frontend -> backend) */
>> -
>>   /*
>> + *****************************************************************************
>> + *                            Frontend to backend events
>> + *****************************************************************************
>> + *
>>    * Out events may be sent only when requested by backend, and receipt
>>    * of an unknown out event is an error.
>>    * No out events currently defined.
>> +
>> + * All event packets have the same length (40 octets)
>> + * All event packets have common header:
>> + *          0         octet
>> + * +-----------------+
>> + * |       type      |
>> + * +-----------------+
>> + * type - uint8_t, event code
>>    */
>>   
>>   #define XENKBD_OUT_EVENT_SIZE 40
>> @@ -95,7 +285,11 @@ union xenkbd_out_event
>>       char pad[XENKBD_OUT_EVENT_SIZE];
>>   };
>>   
>> -/* shared page */
>> +/*
>> + *****************************************************************************
>> + *                            Shared page
>> + *****************************************************************************
>> + */
>>   
>>   #define XENKBD_IN_RING_SIZE 2048
>>   #define XENKBD_IN_RING_LEN (XENKBD_IN_RING_SIZE / XENKBD_IN_EVENT_SIZE)
>> @@ -119,7 +313,7 @@ struct xenkbd_page
>>       uint32_t out_cons, out_prod;
>>   };
>>   
>> -#endif
>> +#endif /* __XEN_PUBLIC_IO_KBDIF_H__ */
>>   
>>   /*
>>    * Local variables:
>> -- 
>> 2.7.4
>>


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-19 22:22   ` Stefano Stabellini
@ 2017-01-20  7:27     ` Oleksandr Andrushchenko
  2017-01-20 17:52       ` Stefano Stabellini
  0 siblings, 1 reply; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-20  7:27 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: konrad.wilk, vlad.babchuk, andrii.anisov, olekstysh, al1img,
	xen-devel, joculator

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.
>
>> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/2] xen/kbdif: update protocol documentation
  2017-01-20  6:58     ` Oleksandr Andrushchenko
@ 2017-01-20 17:43       ` Stefano Stabellini
  0 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2017-01-20 17:43 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: Stefano Stabellini, konrad.wilk, vlad.babchuk, andrii.anisov,
	olekstysh, al1img, xen-devel, joculator

On Fri, 20 Jan 2017, Oleksandr Andrushchenko wrote:
> On 01/19/2017 08:56 PM, 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 | 248
> > > +++++++++++++++++++++++++++++++++++++-----
> > >   1 file changed, 221 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> > > index 2d2aebdd3f28..c00faa3af5d2 100644
> > > --- a/xen/include/public/io/kbdif.h
> > > +++ b/xen/include/public/io/kbdif.h
> > > @@ -26,46 +26,226 @@
> > >   #ifndef __XEN_PUBLIC_IO_KBDIF_H__
> > >   #define __XEN_PUBLIC_IO_KBDIF_H__
> > >   -/* In events (backend -> frontend) */
> > > +/*
> > > +
> > > *****************************************************************************
> > > + *                     Feature and Parameter Negotiation
> > > +
> > > *****************************************************************************
> > > + *
> > > + * The two halves of a para-virtual driver utilize nodes within the
> > within "XenStore", drop "the"
> > 
> > Aside from this minor this:
> > 
> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> Thanks,
> with this change done, can I put your Reviewed-by
> into the next version of this patch?

Of course. Thanks,

Stefano

> > > + * XenStore to communicate capabilities and to negotiate operating
> > > parameters.
> > > + * This section enumerates these nodes which reside in the respective
> > > front and
> > > + * backend portions of XenStore, following XenBus convention.
> > > + *
> > > + * All data in XenStore is stored as strings.  Nodes specifying numeric
> > > + * values are encoded in decimal. Integer value ranges listed below are
> > > + * expressed as fixed sized integer types capable of storing the
> > > conversion
> > > + * of a properly formated node string, without loss of information.
> > > + *
> > > +
> > > *****************************************************************************
> > > + *                            Backend XenBus Nodes
> > > +
> > > *****************************************************************************
> > > + *
> > > + *---------------------------- Features supported
> > > ----------------------------
> > > + *
> > > + * Capable backend advertises supported features by publishing
> > > + * corresponding entries in XenStore and puts 1 as the value of the
> > > entry.
> > > + * If a feature is not supported then 0 must be set or feature entry
> > > omitted.
> > > + *
> > > + * feature-abs-pointer
> > > + *      Values:         <uint>
> > > + *
> > > + *      Backends, which support reporting of absolute coordinates for
> > > pointer
> > > + *      device should set this to 1.
> > > + *
> > > + *------------------------- Pointer Device Parameters
> > > ------------------------
> > > + *
> > > + * width
> > > + *      Values:         <uint>
> > > + *
> > > + *      Maximum X coordinate (width) to be used by the frontend
> > > + *      while reporting input events, pixels, [0; UINT32_MAX].
> > > + *
> > > + * height
> > > + *      Values:         <uint>
> > > + *
> > > + *      Maximum Y coordinate (height) to be used by the frontend
> > > + *      while reporting input events, pixels, [0; UINT32_MAX].
> > > + *
> > > +
> > > *****************************************************************************
> > > + *                            Frontend XenBus Nodes
> > > +
> > > *****************************************************************************
> > > + *
> > > + *------------------------------ Feature request
> > > -----------------------------
> > > + *
> > > + * Capable frontend requests features from backend via setting
> > > corresponding
> > > + * entries to 1 in XenStore. Requests for features not advertised as
> > > supported
> > > + * by the backend have no effect.
> > > + *
> > > + * request-abs-pointer
> > > + *      Values:         <uint>
> > > + *
> > > + *      Request backend to report absolute pointer coordinates
> > > + *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
> > > + *
> > > + *----------------------- Request Transport Parameters
> > > -----------------------
> > > + *
> > > + * event-channel
> > > + *      Values:         <uint>
> > > + *
> > > + *      The identifier of the Xen event channel used to signal activity
> > > + *      in the ring buffer.
> > > + *
> > > + * page-gref
> > > + *      Values:         <uint>
> > > + *
> > > + *      The Xen grant reference granting permission for the backend to
> > > map
> > > + *      a sole page in a single page sized event ring buffer.
> > > + *
> > > + * page-ref
> > > + *      Values:         <uint>
> > > + *
> > > + *      OBSOLETE, not recommended for use.
> > > + *      PFN of the shared page.
> > > + */
> > >     /*
> > > - * Frontends should ignore unknown in events.
> > > + * EVENT CODES.
> > >    */
> > >   -/* Pointer movement event */
> > > -#define XENKBD_TYPE_MOTION  1
> > > -/* Event type 2 currently not used */
> > > -/* Key event (includes pointer buttons) */
> > > -#define XENKBD_TYPE_KEY     3
> > > +#define XENKBD_TYPE_MOTION             1
> > > +#define XENKBD_TYPE_RESERVED           2
> > > +#define XENKBD_TYPE_KEY                3
> > > +#define XENKBD_TYPE_POS                4
> > > +
> > >   /*
> > > - * Pointer position event
> > > - * Capable backend sets feature-abs-pointer in xenstore.
> > > - * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting
> > > - * request-abs-update in xenstore.
> > > + * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS.
> > > + */
> > > +
> > > +#define XENKBD_DRIVER_NAME             "vkbd"
> > > +
> > > +#define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
> > > +#define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
> > > +#define XENKBD_FIELD_RING_GREF         "page-gref"
> > > +#define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
> > > +#define XENKBD_FIELD_WIDTH             "width"
> > > +#define XENKBD_FIELD_HEIGHT            "height"
> > > +
> > > +/* OBSOLETE, not recommended for use */
> > > +#define XENKBD_FIELD_RING_REF          "page-ref"
> > > +
> > > +/*
> > > +
> > > *****************************************************************************
> > > + * Description of the protocol between frontend and backend driver.
> > > +
> > > *****************************************************************************
> > > + *
> > > + * The two halves of a Para-virtual driver communicate with
> > > + * each other using a shared page and an event channel.
> > > + * Shared page contains a ring with event structures.
> > > + *
> > > + * All reserved fields in the structures below must be 0.
> > > + *
> > > +
> > > *****************************************************************************
> > > + *                           Backend to frontend events
> > > +
> > > *****************************************************************************
> > > + *
> > > + * Frontends should ignore unknown in events.
> > > + * All event packets have the same length (40 octets)
> > > + * All event packets have common header:
> > > + *
> > > + *          0         octet
> > > + * +-----------------+
> > > + * |       type      |
> > > + * +-----------------+
> > > + * type - uint8_t, event code, XENKBD_TYPE_???
> > > + *
> > > + *
> > > + * Pointer relative movement event
> > > + *         0                1                 2               3
> > > octet
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |  _TYPE_MOTION  |                     reserved                     |
> > > 4
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               rel_x                               |
> > > 8
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               rel_y                               |
> > > 12
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               rel_z                               |
> > > 16
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 20
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 40
> > > + * +----------------+----------------+----------------+----------------+
> > > + *
> > > + * rel_x - int32_t, relative X motion
> > > + * rel_y - int32_t, relative Y motion
> > > + * rel_z - int32_t, relative Z motion (wheel)
> > >    */
> > > -#define XENKBD_TYPE_POS     4
> > >     struct xenkbd_motion
> > >   {
> > > -    uint8_t type;        /* XENKBD_TYPE_MOTION */
> > > -    int32_t rel_x;       /* relative X motion */
> > > -    int32_t rel_y;       /* relative Y motion */
> > > -    int32_t rel_z;       /* relative Z motion (wheel) */
> > > +    uint8_t type;
> > > +    int32_t rel_x;
> > > +    int32_t rel_y;
> > > +    int32_t rel_z;
> > >   };
> > >   +/*
> > > + * Key event (includes pointer buttons)
> > > + *         0                1                 2               3
> > > octet
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |  _TYPE_KEY     |     pressed    |            reserved             |
> > > 4
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                              keycode                              |
> > > 8
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 12
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 40
> > > + * +----------------+----------------+----------------+----------------+
> > > + *
> > > + * pressed - uint8_t, 1 if pressed; 0 otherwise
> > > + * keycode - uint32_t, KEY_* from linux/input.h
> > > + */
> > > +
> > >   struct xenkbd_key
> > >   {
> > > -    uint8_t type;         /* XENKBD_TYPE_KEY */
> > > -    uint8_t pressed;      /* 1 if pressed; 0 otherwise */
> > > -    uint32_t keycode;     /* KEY_* from linux/input.h */
> > > +    uint8_t type;
> > > +    uint8_t pressed;
> > > +    uint32_t keycode;
> > >   };
> > >   +/*
> > > + * Pointer absolute position event
> > > + *         0                1                 2               3
> > > octet
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |  _TYPE_POS     |                     reserved                     |
> > > 4
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               abs_x                               |
> > > 8
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               abs_y                               |
> > > 12
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               rel_z                               |
> > > 16
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 20
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 40
> > > + * +----------------+----------------+----------------+----------------+
> > > + *
> > > + * abs_x - int32_t, absolute X position (in FB pixels)
> > > + * abs_y - int32_t, absolute Y position (in FB pixels)
> > > + * rel_z - int32_t, relative Z motion (wheel)
> > > + */
> > > +
> > >   struct xenkbd_position
> > >   {
> > > -    uint8_t type;        /* XENKBD_TYPE_POS */
> > > -    int32_t abs_x;       /* absolute X position (in FB pixels) */
> > > -    int32_t abs_y;       /* absolute Y position (in FB pixels) */
> > > -    int32_t rel_z;       /* relative Z motion (wheel) */
> > > +    uint8_t type;
> > > +    int32_t abs_x;
> > > +    int32_t abs_y;
> > > +    int32_t rel_z;
> > >   };
> > >     #define XENKBD_IN_EVENT_SIZE 40
> > > @@ -79,12 +259,22 @@ union xenkbd_in_event
> > >       char pad[XENKBD_IN_EVENT_SIZE];
> > >   };
> > >   -/* Out events (frontend -> backend) */
> > > -
> > >   /*
> > > +
> > > *****************************************************************************
> > > + *                            Frontend to backend events
> > > +
> > > *****************************************************************************
> > > + *
> > >    * Out events may be sent only when requested by backend, and receipt
> > >    * of an unknown out event is an error.
> > >    * No out events currently defined.
> > > +
> > > + * All event packets have the same length (40 octets)
> > > + * All event packets have common header:
> > > + *          0         octet
> > > + * +-----------------+
> > > + * |       type      |
> > > + * +-----------------+
> > > + * type - uint8_t, event code
> > >    */
> > >     #define XENKBD_OUT_EVENT_SIZE 40
> > > @@ -95,7 +285,11 @@ union xenkbd_out_event
> > >       char pad[XENKBD_OUT_EVENT_SIZE];
> > >   };
> > >   -/* shared page */
> > > +/*
> > > +
> > > *****************************************************************************
> > > + *                            Shared page
> > > +
> > > *****************************************************************************
> > > + */
> > >     #define XENKBD_IN_RING_SIZE 2048
> > >   #define XENKBD_IN_RING_LEN (XENKBD_IN_RING_SIZE / XENKBD_IN_EVENT_SIZE)
> > > @@ -119,7 +313,7 @@ struct xenkbd_page
> > >       uint32_t out_cons, out_prod;
> > >   };
> > >   -#endif
> > > +#endif /* __XEN_PUBLIC_IO_KBDIF_H__ */
> > >     /*
> > >    * Local variables:
> > > -- 
> > > 2.7.4
> > > 
> 

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-20  7:27     ` Oleksandr Andrushchenko
@ 2017-01-20 17:52       ` Stefano Stabellini
  2017-01-20 18:46         ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2017-01-20 17:52 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: Stefano Stabellini, konrad.wilk, vlad.babchuk, andrii.anisov,
	olekstysh, al1img, xen-devel, joculator

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.

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?

This patch is OK as is.

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> > > 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-20 17:52       ` Stefano Stabellini
@ 2017-01-20 18:46         ` Oleksandr Andrushchenko
  2017-01-20 23:01           ` Stefano Stabellini
  0 siblings, 1 reply; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-20 18:46 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: konrad.wilk, vlad.babchuk, andrii.anisov, olekstysh, al1img,
	xen-devel, joculator

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-20 18:46         ` Oleksandr Andrushchenko
@ 2017-01-20 23:01           ` Stefano Stabellini
  2017-01-21 14:34             ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2017-01-20 23:01 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: Stefano Stabellini, konrad.wilk, vlad.babchuk, andrii.anisov,
	olekstysh, al1img, xen-devel, joculator

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@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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-20 23:01           ` Stefano Stabellini
@ 2017-01-21 14:34             ` Oleksandr Andrushchenko
  2017-01-23 19:49               ` Stefano Stabellini
  0 siblings, 1 reply; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-21 14:34 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: vlad.babchuk, andrii.anisov, olekstysh, al1img, xen-devel, joculator


[-- Attachment #1.1: Type: text/plain, Size: 20781 bytes --]

On 01/21/2017 01:01 AM, Stefano Stabellini wrote:
> 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@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
> Sorry, now I am confused again :-S
In the mail-thread you mentioned above there is a picture of
the xenstore entries and conclusion:

1. No change to the existing kbd+ptr:
1.1. They still share a dedicated (single) connection
channel as they did before multi-touch: page-ref + event-channel:

/local/domain/2/device/vkbd/0/page-ref = "1248025"
/local/domain/2/device/vkbd/0/page-gref = "336"
/local/domain/2/device/vkbd/0/event-channel = "43"

1.2. No multi-touch events via that connection
1.3. Old backends/frontends will see no difference
after multi-touch

2. Each multi-touch device has its own:
2.1. Configuration (width x height, num-contacts)
2.2. Own connection channel (page-ref, event-channel)

/local/domain/2/device/vkbd/0/mtouch/0/width = "3200"
/local/domain/2/device/vkbd/0/mtouch/0/height = "3200"
/local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5"
/local/domain/2/device/vkbd/0/mtouch/0/page-ref = "1252386"
/local/domain/2/device/vkbd/0/mtouch/0/page-gref = "335"
/local/domain/2/device/vkbd/0/mtouch/0/event-channel = "44"

/local/domain/2/device/vkbd/0/mtouch/1/width = "6400"
/local/domain/2/device/vkbd/0/mtouch/1/height = "6400"
/local/domain/2/device/vkbd/0/mtouch/1/num-contacts = "10"
/local/domain/2/device/vkbd/0/mtouch/1/page-ref = "2376038"
/local/domain/2/device/vkbd/0/mtouch/1/page-gref = "337"
/local/domain/2/device/vkbd/0/mtouch/1/event-channel = "45"
So, for the example above: 1 kbd + 1 ptr + 2 mt devices
within a single driver, *3 connections*

> There are no ring-ref and event-channel under mtouch, are there?
there are ring-refs and event-channels under mtouch *per multi-touch*
device
> So, why
> would XENKBD_PATH_MTOUCH help distinguish them?
it groups multiple mtouch devices
Hope this clarifies things a bit
>>> 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?
> 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
>>>>>>


[-- Attachment #1.2: Type: text/html, Size: 22093 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-21 14:34             ` Oleksandr Andrushchenko
@ 2017-01-23 19:49               ` Stefano Stabellini
  2017-01-24  7:31                 ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2017-01-23 19:49 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: Stefano Stabellini, vlad.babchuk, andrii.anisov, olekstysh,
	al1img, xen-devel, joculator

On Sat, 21 Jan 2017, Oleksandr Andrushchenko wrote:
> In the mail-thread you mentioned above there is a picture of
> the xenstore entries and conclusion:
> 
> 1. No change to the existing kbd+ptr:
> 1.1. They still share a dedicated (single) connection
> channel as they did before multi-touch: page-ref + event-channel:
> 
> /local/domain/2/device/vkbd/0/page-ref = "1248025"
> /local/domain/2/device/vkbd/0/page-gref = "336"
> /local/domain/2/device/vkbd/0/event-channel = "43"
> 
> 1.2. No multi-touch events via that connection
> 1.3. Old backends/frontends will see no difference
> after multi-touch
> 
> 2. Each multi-touch device has its own:
> 2.1. Configuration (width x height, num-contacts)
> 2.2. Own connection channel (page-ref, event-channel)
> 
> /local/domain/2/device/vkbd/0/mtouch/0/width = "3200"
> /local/domain/2/device/vkbd/0/mtouch/0/height = "3200"
> /local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5"
> /local/domain/2/device/vkbd/0/mtouch/0/page-ref = "1252386"
> /local/domain/2/device/vkbd/0/mtouch/0/page-gref = "335"
> /local/domain/2/device/vkbd/0/mtouch/0/event-channel = "44"
> 
> /local/domain/2/device/vkbd/0/mtouch/1/width = "6400"
> /local/domain/2/device/vkbd/0/mtouch/1/height = "6400"
> /local/domain/2/device/vkbd/0/mtouch/1/num-contacts = "10"
> /local/domain/2/device/vkbd/0/mtouch/1/page-ref = "2376038"
> /local/domain/2/device/vkbd/0/mtouch/1/page-gref = "337"
> /local/domain/2/device/vkbd/0/mtouch/1/event-channel = "45"
> So, for the example above: 1 kbd + 1 ptr + 2 mt devices
> within a single driver, *3 connections*
> 
>> There are no ring-ref and event-channel under mtouch, are there? 
> 
> there are ring-refs and event-channels under mtouch *per multi-touch*
> device

Now, it is clear. Sorry it took so many back and forth.

page-ref and event-channel should definitely be described under
"Multi-touch Device Parameters". When I asked you to remove them, I
meant not just from the description but also from the protocol. This is
where the confusion originated: in the current patch the two parameters
are not described, hence I assumed they didn't exist and you were
reusing the existing ring (/local/domain/2/device/vkbd/0/page-ref).


Aside from new message structs, which look fine to me, we have two
important choices to make:

a) number of multitouch devices per PV frontend/backend connection
(By frontend/backend connection, I mean by xenstore device,
such as /local/domain/2/device/vkbd/0.)

b) number of multitouch devices sharing the same ring
(By ring I mean page-ref + event channel.)

Both these choices need to be motivated. As I wrote in the past, I would
go for 1 multitouch device per frontend/backend connection, reusing the
existing ring (/local/domain/2/device/vkbd/0/page-ref and
/local/domain/2/device/vkbd/0/event-channel), because I assume that
there won't be many multitouch devices, less than 5, so we can afford to
create new PV devices for each of them. I am also assuming that it is
fine for them to share ring with the keyboard or pointer devices without
effects on performance because they should be all low frequency devices.

The current proposal supports multiple multitouch devices per
frontend/backnend connection and allocates a new dedicated ring for each
multitouch device. Your proposal might very well be the best solution,
but why? The architectural choices need to be motivated. How many
multitouch devices is reasonable to expect to be present on a platform?
What is the multitouch events frequency we expect from them? The answer
to the first question motivates choice a), the answer to the second
question motivates choice b). Please add them both to the protocol
description.  It is OK to add the explanation to kbdif.h. You also
mentioned something about multiple PV devices causing troubles to Linux.
If that is an issue somehow, please add info about that too.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-23 19:49               ` Stefano Stabellini
@ 2017-01-24  7:31                 ` Oleksandr Andrushchenko
  2017-01-24 23:05                   ` Stefano Stabellini
  0 siblings, 1 reply; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-24  7:31 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: vlad.babchuk, andrii.anisov, olekstysh, al1img, xen-devel, joculator

On 01/23/2017 09:49 PM, Stefano Stabellini wrote:
> On Sat, 21 Jan 2017, Oleksandr Andrushchenko wrote:
>> In the mail-thread you mentioned above there is a picture of
>> the xenstore entries and conclusion:
>>
>> 1. No change to the existing kbd+ptr:
>> 1.1. They still share a dedicated (single) connection
>> channel as they did before multi-touch: page-ref + event-channel:
>>
>> /local/domain/2/device/vkbd/0/page-ref = "1248025"
>> /local/domain/2/device/vkbd/0/page-gref = "336"
>> /local/domain/2/device/vkbd/0/event-channel = "43"
>>
>> 1.2. No multi-touch events via that connection
>> 1.3. Old backends/frontends will see no difference
>> after multi-touch
>>
>> 2. Each multi-touch device has its own:
>> 2.1. Configuration (width x height, num-contacts)
>> 2.2. Own connection channel (page-ref, event-channel)
>>
>> /local/domain/2/device/vkbd/0/mtouch/0/width = "3200"
>> /local/domain/2/device/vkbd/0/mtouch/0/height = "3200"
>> /local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5"
>> /local/domain/2/device/vkbd/0/mtouch/0/page-ref = "1252386"
>> /local/domain/2/device/vkbd/0/mtouch/0/page-gref = "335"
>> /local/domain/2/device/vkbd/0/mtouch/0/event-channel = "44"
>>
>> /local/domain/2/device/vkbd/0/mtouch/1/width = "6400"
>> /local/domain/2/device/vkbd/0/mtouch/1/height = "6400"
>> /local/domain/2/device/vkbd/0/mtouch/1/num-contacts = "10"
>> /local/domain/2/device/vkbd/0/mtouch/1/page-ref = "2376038"
>> /local/domain/2/device/vkbd/0/mtouch/1/page-gref = "337"
>> /local/domain/2/device/vkbd/0/mtouch/1/event-channel = "45"
>> So, for the example above: 1 kbd + 1 ptr + 2 mt devices
>> within a single driver, *3 connections*
>>
>>> There are no ring-ref and event-channel under mtouch, are there?
>> there are ring-refs and event-channels under mtouch *per multi-touch*
>> device
> Now, it is clear. Sorry it took so many back and forth.
np
> page-ref and event-channel should definitely be described under
> "Multi-touch Device Parameters". When I asked you to remove them, I
> meant not just from the description but also from the protocol. This is
> where the confusion originated: in the current patch the two parameters
> are not described, hence I assumed they didn't exist and you were
> reusing the existing ring (/local/domain/2/device/vkbd/0/page-ref).
understood
>
> Aside from new message structs, which look fine to me, we have two
> important choices to make:
>
> a) number of multitouch devices per PV frontend/backend connection
> (By frontend/backend connection, I mean by xenstore device,
> such as /local/domain/2/device/vkbd/0.)
>
> b) number of multitouch devices sharing the same ring
> (By ring I mean page-ref + event channel.)
>
> Both these choices need to be motivated. As I wrote in the past, I would
> go for 1 multitouch device per frontend/backend connection, reusing the
> existing ring (/local/domain/2/device/vkbd/0/page-ref and
> /local/domain/2/device/vkbd/0/event-channel), because I assume that
> there won't be many multitouch devices, less than 5,
yes, I have 2 mt devices in my use-case
> so we can afford to
> create new PV devices for each of them.
see at the bottom
>   I am also assuming that it is
> fine for them to share ring with the keyboard or pointer devices without
> effects on performance because they should be all low frequency devices.
sounds reasonable, but see below
> The current proposal supports multiple multitouch devices per
> frontend/backnend connection and allocates a new dedicated ring for each
> multitouch device. Your proposal might very well be the best solution,
> but why? The architectural choices need to be motivated. How many
> multitouch devices is reasonable to expect to be present on a platform?
> What is the multitouch events frequency we expect from them? The answer
> to the first question motivates choice a), the answer to the second
> question motivates choice b). Please add them both to the protocol
> description.  It is OK to add the explanation to kbdif.h. You also
> mentioned something about multiple PV devices causing troubles to Linux.
> If that is an issue somehow, please add info about that too.
Well, all the above sounds reasonable, some comments:
1. Event frequency: here we are talking about 60-120(?)Hz
scan frequency of a touch screen generating at least 2
events for each contact: frame (syn) and position.
So, we can estimate for 10 fingers the value to be
60 * 2 * 10 = 1200 events a second per multi-touch device.
Of course, this is rough estimate which I believe would
be smaller in real life.

2. "create new PV devices for each of them"
2.1. Driver
This is something which you are seeing in xenstore as
   /local/domain/2/device/vkbd/0
0 - being index of the *driver*.
And the challenge here is that if you want multiple *devices*
to be allocated in this way:
   /local/domain/2/device/vkbd/0
   /local/domain/2/device/vkbd/1
   /local/domain/2/device/vkbd/2
then you have to register multiple *DRIVERS*.
What we normally do in the PV front driver is we call
*xenbus_register_frontend*,passing a structure
*struct xenbus_driver* with *.ids* set to a single entry "vkbd".

I am a little bit confused here about front implementation:
how to dynamically register multiple "vkbd" *drivers* depending on
configuration, so we have the layout you mention.

2.2. Input device
You are virtually not limited on number of *DEVICES within a DRIVER*,
but they all are under "/local/domain/2/device/vkbd/0" entry.
That being said, under the same xenstore entry you can allocate
ptr+kbd+N*mt input devices.

Bottom line:
================
a) From (1), probably it would be fine to send these events
over the existing ring (kbd+ptr), but mt event structures
need an index to indicate which mt device is the owner of
the event
b) We may still want to have a dedicated ring either for all
mt devices (depending on event frequency we consider reasonable)
or per mt-device.
c) I see no simple way to support multiple drivers comparing to
multiple devices (*DISCLAIMER* which is natural to Linux).

All,
what do you think on the above? What is the approach we are going to
take?

Thank you,
Oleksandr

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-24  7:31                 ` Oleksandr Andrushchenko
@ 2017-01-24 23:05                   ` Stefano Stabellini
  2017-01-25  8:39                     ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2017-01-24 23:05 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: Stefano Stabellini, vlad.babchuk, andrii.anisov, olekstysh,
	al1img, xen-devel, joculator

On Tue, 24 Jan 2017, Oleksandr Andrushchenko wrote:
> On 01/23/2017 09:49 PM, Stefano Stabellini wrote:
> > On Sat, 21 Jan 2017, Oleksandr Andrushchenko wrote:
> > > In the mail-thread you mentioned above there is a picture of
> > > the xenstore entries and conclusion:
> > > 
> > > 1. No change to the existing kbd+ptr:
> > > 1.1. They still share a dedicated (single) connection
> > > channel as they did before multi-touch: page-ref + event-channel:
> > > 
> > > /local/domain/2/device/vkbd/0/page-ref = "1248025"
> > > /local/domain/2/device/vkbd/0/page-gref = "336"
> > > /local/domain/2/device/vkbd/0/event-channel = "43"
> > > 
> > > 1.2. No multi-touch events via that connection
> > > 1.3. Old backends/frontends will see no difference
> > > after multi-touch
> > > 
> > > 2. Each multi-touch device has its own:
> > > 2.1. Configuration (width x height, num-contacts)
> > > 2.2. Own connection channel (page-ref, event-channel)
> > > 
> > > /local/domain/2/device/vkbd/0/mtouch/0/width = "3200"
> > > /local/domain/2/device/vkbd/0/mtouch/0/height = "3200"
> > > /local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5"
> > > /local/domain/2/device/vkbd/0/mtouch/0/page-ref = "1252386"
> > > /local/domain/2/device/vkbd/0/mtouch/0/page-gref = "335"
> > > /local/domain/2/device/vkbd/0/mtouch/0/event-channel = "44"
> > > 
> > > /local/domain/2/device/vkbd/0/mtouch/1/width = "6400"
> > > /local/domain/2/device/vkbd/0/mtouch/1/height = "6400"
> > > /local/domain/2/device/vkbd/0/mtouch/1/num-contacts = "10"
> > > /local/domain/2/device/vkbd/0/mtouch/1/page-ref = "2376038"
> > > /local/domain/2/device/vkbd/0/mtouch/1/page-gref = "337"
> > > /local/domain/2/device/vkbd/0/mtouch/1/event-channel = "45"
> > > So, for the example above: 1 kbd + 1 ptr + 2 mt devices
> > > within a single driver, *3 connections*
> > > 
> > > > There are no ring-ref and event-channel under mtouch, are there?
> > > there are ring-refs and event-channels under mtouch *per multi-touch*
> > > device
> > Now, it is clear. Sorry it took so many back and forth.
> np
> > page-ref and event-channel should definitely be described under
> > "Multi-touch Device Parameters". When I asked you to remove them, I
> > meant not just from the description but also from the protocol. This is
> > where the confusion originated: in the current patch the two parameters
> > are not described, hence I assumed they didn't exist and you were
> > reusing the existing ring (/local/domain/2/device/vkbd/0/page-ref).
> understood
> > 
> > Aside from new message structs, which look fine to me, we have two
> > important choices to make:
> > 
> > a) number of multitouch devices per PV frontend/backend connection
> > (By frontend/backend connection, I mean by xenstore device,
> > such as /local/domain/2/device/vkbd/0.)
> > 
> > b) number of multitouch devices sharing the same ring
> > (By ring I mean page-ref + event channel.)
> > 
> > Both these choices need to be motivated. As I wrote in the past, I would
> > go for 1 multitouch device per frontend/backend connection, reusing the
> > existing ring (/local/domain/2/device/vkbd/0/page-ref and
> > /local/domain/2/device/vkbd/0/event-channel), because I assume that
> > there won't be many multitouch devices, less than 5,
> yes, I have 2 mt devices in my use-case
> > so we can afford to
> > create new PV devices for each of them.
> see at the bottom
> >   I am also assuming that it is
> > fine for them to share ring with the keyboard or pointer devices without
> > effects on performance because they should be all low frequency devices.
> sounds reasonable, but see below
> > The current proposal supports multiple multitouch devices per
> > frontend/backnend connection and allocates a new dedicated ring for each
> > multitouch device. Your proposal might very well be the best solution,
> > but why? The architectural choices need to be motivated. How many
> > multitouch devices is reasonable to expect to be present on a platform?
> > What is the multitouch events frequency we expect from them? The answer
> > to the first question motivates choice a), the answer to the second
> > question motivates choice b). Please add them both to the protocol
> > description.  It is OK to add the explanation to kbdif.h. You also
> > mentioned something about multiple PV devices causing troubles to Linux.
> > If that is an issue somehow, please add info about that too.
> Well, all the above sounds reasonable, some comments:
> 1. Event frequency: here we are talking about 60-120(?)Hz
> scan frequency of a touch screen generating at least 2
> events for each contact: frame (syn) and position.
> So, we can estimate for 10 fingers the value to be
> 60 * 2 * 10 = 1200 events a second per multi-touch device.
> Of course, this is rough estimate which I believe would
> be smaller in real life.
> 
> 2. "create new PV devices for each of them"
> 2.1. Driver
> This is something which you are seeing in xenstore as
>   /local/domain/2/device/vkbd/0
> 0 - being index of the *driver*.
> And the challenge here is that if you want multiple *devices*
> to be allocated in this way:
>   /local/domain/2/device/vkbd/0
>   /local/domain/2/device/vkbd/1
>   /local/domain/2/device/vkbd/2
> then you have to register multiple *DRIVERS*.
> What we normally do in the PV front driver is we call
> *xenbus_register_frontend*,passing a structure
> *struct xenbus_driver* with *.ids* set to a single entry "vkbd".
> 
> I am a little bit confused here about front implementation:
> how to dynamically register multiple "vkbd" *drivers* depending on
> configuration, so we have the layout you mention.

You shouldn't have to do anything in Linux, it should happen naturally.
The toolstack (libxl, xl) should configure two or more vkbd devices on
xenstore, for example:

   /local/domain/2/device/vkbd/0
   /local/domain/2/device/vkbd/1

then the Linux xenbus frontend should automatically allocate a driver
for each of them. xenkbd_init should be called once, but you would get
two xenkbd_probe calls, with different struct xenbus_device* arguments,
one for each vkbd device.

I don't think that the toolstack is able to create multiple vkbd devices
today (that's where you would need to make your modifications), but it
certainly can create multiple vifs:

vif=[ 'bridge=xenbr0', 'bridge=xenbr1' ]

You can experiment with that to see how the xenbus driver initialization
works. You should see multiple drivers/net/xen-netfront.c:netfront_probe
calls. Then, you could hack libxl to create two vkbd devices and see
what happens. Linux should pick them up automatically.

Does that answer your question?


> 2.2. Input device
> You are virtually not limited on number of *DEVICES within a DRIVER*,
> but they all are under "/local/domain/2/device/vkbd/0" entry.
> That being said, under the same xenstore entry you can allocate
> ptr+kbd+N*mt input devices.
> 
> Bottom line:
> ================
> a) From (1), probably it would be fine to send these events
> over the existing ring (kbd+ptr), but mt event structures
> need an index to indicate which mt device is the owner of
> the event
> b) We may still want to have a dedicated ring either for all
> mt devices (depending on event frequency we consider reasonable)
> or per mt-device.

The events frequency you describe is high enough that it's better not to
share a ring across multiple mt devices. However, it might still be OK
for a single mt device to share a ring with kbd and ptr as ptr and kbd
are lower frequency.


> c) I see no simple way to support multiple drivers comparing to
> multiple devices (*DISCLAIMER* which is natural to Linux).
> 
> All,
> what do you think on the above? What is the approach we are going to
> take?

Thanks for the info, this is very helpful!

I would appreciate feedback from others as well, but I think that we
need one ring per mt device, maybe shared with ptr and kbd. Having one
xenstore device per mt device would be more in line with the usual way
of doing things, and would be my preference, but we need to figure out
if there is a problem with that approach first. Let me know if you find
any issues with that.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/2] xen/kbdif: add multi-touch support
  2017-01-24 23:05                   ` Stefano Stabellini
@ 2017-01-25  8:39                     ` Oleksandr Andrushchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-25  8:39 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: vlad.babchuk, andrii.anisov, olekstysh, al1img, xen-devel, joculator

On 01/25/2017 01:05 AM, Stefano Stabellini wrote:
> On Tue, 24 Jan 2017, Oleksandr Andrushchenko wrote:
>> On 01/23/2017 09:49 PM, Stefano Stabellini wrote:
>>> On Sat, 21 Jan 2017, Oleksandr Andrushchenko wrote:
>>>> In the mail-thread you mentioned above there is a picture of
>>>> the xenstore entries and conclusion:
>>>>
>>>> 1. No change to the existing kbd+ptr:
>>>> 1.1. They still share a dedicated (single) connection
>>>> channel as they did before multi-touch: page-ref + event-channel:
>>>>
>>>> /local/domain/2/device/vkbd/0/page-ref = "1248025"
>>>> /local/domain/2/device/vkbd/0/page-gref = "336"
>>>> /local/domain/2/device/vkbd/0/event-channel = "43"
>>>>
>>>> 1.2. No multi-touch events via that connection
>>>> 1.3. Old backends/frontends will see no difference
>>>> after multi-touch
>>>>
>>>> 2. Each multi-touch device has its own:
>>>> 2.1. Configuration (width x height, num-contacts)
>>>> 2.2. Own connection channel (page-ref, event-channel)
>>>>
>>>> /local/domain/2/device/vkbd/0/mtouch/0/width = "3200"
>>>> /local/domain/2/device/vkbd/0/mtouch/0/height = "3200"
>>>> /local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5"
>>>> /local/domain/2/device/vkbd/0/mtouch/0/page-ref = "1252386"
>>>> /local/domain/2/device/vkbd/0/mtouch/0/page-gref = "335"
>>>> /local/domain/2/device/vkbd/0/mtouch/0/event-channel = "44"
>>>>
>>>> /local/domain/2/device/vkbd/0/mtouch/1/width = "6400"
>>>> /local/domain/2/device/vkbd/0/mtouch/1/height = "6400"
>>>> /local/domain/2/device/vkbd/0/mtouch/1/num-contacts = "10"
>>>> /local/domain/2/device/vkbd/0/mtouch/1/page-ref = "2376038"
>>>> /local/domain/2/device/vkbd/0/mtouch/1/page-gref = "337"
>>>> /local/domain/2/device/vkbd/0/mtouch/1/event-channel = "45"
>>>> So, for the example above: 1 kbd + 1 ptr + 2 mt devices
>>>> within a single driver, *3 connections*
>>>>
>>>>> There are no ring-ref and event-channel under mtouch, are there?
>>>> there are ring-refs and event-channels under mtouch *per multi-touch*
>>>> device
>>> Now, it is clear. Sorry it took so many back and forth.
>> np
>>> page-ref and event-channel should definitely be described under
>>> "Multi-touch Device Parameters". When I asked you to remove them, I
>>> meant not just from the description but also from the protocol. This is
>>> where the confusion originated: in the current patch the two parameters
>>> are not described, hence I assumed they didn't exist and you were
>>> reusing the existing ring (/local/domain/2/device/vkbd/0/page-ref).
>> understood
>>> Aside from new message structs, which look fine to me, we have two
>>> important choices to make:
>>>
>>> a) number of multitouch devices per PV frontend/backend connection
>>> (By frontend/backend connection, I mean by xenstore device,
>>> such as /local/domain/2/device/vkbd/0.)
>>>
>>> b) number of multitouch devices sharing the same ring
>>> (By ring I mean page-ref + event channel.)
>>>
>>> Both these choices need to be motivated. As I wrote in the past, I would
>>> go for 1 multitouch device per frontend/backend connection, reusing the
>>> existing ring (/local/domain/2/device/vkbd/0/page-ref and
>>> /local/domain/2/device/vkbd/0/event-channel), because I assume that
>>> there won't be many multitouch devices, less than 5,
>> yes, I have 2 mt devices in my use-case
>>> so we can afford to
>>> create new PV devices for each of them.
>> see at the bottom
>>>    I am also assuming that it is
>>> fine for them to share ring with the keyboard or pointer devices without
>>> effects on performance because they should be all low frequency devices.
>> sounds reasonable, but see below
>>> The current proposal supports multiple multitouch devices per
>>> frontend/backnend connection and allocates a new dedicated ring for each
>>> multitouch device. Your proposal might very well be the best solution,
>>> but why? The architectural choices need to be motivated. How many
>>> multitouch devices is reasonable to expect to be present on a platform?
>>> What is the multitouch events frequency we expect from them? The answer
>>> to the first question motivates choice a), the answer to the second
>>> question motivates choice b). Please add them both to the protocol
>>> description.  It is OK to add the explanation to kbdif.h. You also
>>> mentioned something about multiple PV devices causing troubles to Linux.
>>> If that is an issue somehow, please add info about that too.
>> Well, all the above sounds reasonable, some comments:
>> 1. Event frequency: here we are talking about 60-120(?)Hz
>> scan frequency of a touch screen generating at least 2
>> events for each contact: frame (syn) and position.
>> So, we can estimate for 10 fingers the value to be
>> 60 * 2 * 10 = 1200 events a second per multi-touch device.
>> Of course, this is rough estimate which I believe would
>> be smaller in real life.
>>
>> 2. "create new PV devices for each of them"
>> 2.1. Driver
>> This is something which you are seeing in xenstore as
>>    /local/domain/2/device/vkbd/0
>> 0 - being index of the *driver*.
>> And the challenge here is that if you want multiple *devices*
>> to be allocated in this way:
>>    /local/domain/2/device/vkbd/0
>>    /local/domain/2/device/vkbd/1
>>    /local/domain/2/device/vkbd/2
>> then you have to register multiple *DRIVERS*.
>> What we normally do in the PV front driver is we call
>> *xenbus_register_frontend*,passing a structure
>> *struct xenbus_driver* with *.ids* set to a single entry "vkbd".
>>
>> I am a little bit confused here about front implementation:
>> how to dynamically register multiple "vkbd" *drivers* depending on
>> configuration, so we have the layout you mention.
> You shouldn't have to do anything in Linux, it should happen naturally.
> The toolstack (libxl, xl) should configure two or more vkbd devices on
> xenstore, for example:
>
>     /local/domain/2/device/vkbd/0
>     /local/domain/2/device/vkbd/1
>
> then the Linux xenbus frontend should automatically allocate a driver
> for each of them. xenkbd_init should be called once, but you would get
> two xenkbd_probe calls, with different struct xenbus_device* arguments,
> one for each vkbd device.
>
> I don't think that the toolstack is able to create multiple vkbd devices
> today (that's where you would need to make your modifications), but it
> certainly can create multiple vifs:
>
> vif=[ 'bridge=xenbr0', 'bridge=xenbr1' ]
>
> You can experiment with that to see how the xenbus driver initialization
> works. You should see multiple drivers/net/xen-netfront.c:netfront_probe
> calls. Then, you could hack libxl to create two vkbd devices and see
> what happens. Linux should pick them up automatically.
>
> Does that answer your question?
It definitely does, thank you!
I didn't know about this possibility so now it makes
much more sense to me wrt what you were saying
>
>> 2.2. Input device
>> You are virtually not limited on number of *DEVICES within a DRIVER*,
>> but they all are under "/local/domain/2/device/vkbd/0" entry.
>> That being said, under the same xenstore entry you can allocate
>> ptr+kbd+N*mt input devices.
>>
>> Bottom line:
>> ================
>> a) From (1), probably it would be fine to send these events
>> over the existing ring (kbd+ptr), but mt event structures
>> need an index to indicate which mt device is the owner of
>> the event
>> b) We may still want to have a dedicated ring either for all
>> mt devices (depending on event frequency we consider reasonable)
>> or per mt-device.
> The events frequency you describe is high enough that it's better not to
> share a ring across multiple mt devices. However, it might still be OK
> for a single mt device to share a ring with kbd and ptr as ptr and kbd
> are lower frequency.
>
I think we can share the same ring
>> c) I see no simple way to support multiple drivers comparing to
>> multiple devices (*DISCLAIMER* which is natural to Linux).
>>
>> All,
>> what do you think on the above? What is the approach we are going to
>> take?
> Thanks for the info, this is very helpful!
>
> I would appreciate feedback from others as well, but I think that we
> need one ring per mt device, maybe shared with ptr and kbd. Having one
> xenstore device per mt device would be more in line with the usual way
> of doing things, and would be my preference, but we need to figure out
> if there is a problem with that approach first. Let me know if you find
> any issues with that.
I see no issues at the moment (tested my current
front implementation a bit), so I will update "mt
support" to reflect the latest findings

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-01-25  8:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-19  9:24 [PATCH v1 0/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
2017-01-19  9:24 ` [PATCH v1 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
2017-01-19 18:56   ` Stefano Stabellini
2017-01-20  6:58     ` Oleksandr Andrushchenko
2017-01-20 17:43       ` Stefano Stabellini
2017-01-19  9:24 ` [PATCH v1 2/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
2017-01-19 22:22   ` Stefano Stabellini
2017-01-20  7:27     ` Oleksandr Andrushchenko
2017-01-20 17:52       ` Stefano Stabellini
2017-01-20 18:46         ` Oleksandr Andrushchenko
2017-01-20 23:01           ` Stefano Stabellini
2017-01-21 14:34             ` Oleksandr Andrushchenko
2017-01-23 19:49               ` Stefano Stabellini
2017-01-24  7:31                 ` Oleksandr Andrushchenko
2017-01-24 23:05                   ` Stefano Stabellini
2017-01-25  8:39                     ` Oleksandr Andrushchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.