All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] xen/kbdif: add multi-touch support
@ 2017-01-06  9:32 Oleksandr Andrushchenko
  2017-01-06  9:32 ` [PATCH 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-06  9:32 UTC (permalink / raw)
  To: xen-devel
  Cc: lars.kurth, sstabellini, vlad.babchuk, dario.faggioli,
	julien.grall, andrii.anisov, olekstysh, andr2000, al1img,
	JBeulich, 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 | 477 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 450 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] 17+ messages in thread

* [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-06  9:32 [PATCH 0/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
@ 2017-01-06  9:32 ` Oleksandr Andrushchenko
  2017-01-06 22:20   ` Stefano Stabellini
  2017-01-06  9:32 ` [PATCH 2/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
  2017-01-11  8:01 ` [PATCH 0/2] " Oleksandr Andrushchenko
  2 siblings, 1 reply; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-06  9:32 UTC (permalink / raw)
  To: xen-devel
  Cc: lars.kurth, sstabellini, vlad.babchuk, dario.faggioli,
	julien.grall, andrii.anisov, olekstysh, andr2000, al1img,
	JBeulich, joculator

From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>

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

diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
index 2d2aebd..0e19a40 100644
--- a/xen/include/public/io/kbdif.h
+++ b/xen/include/public/io/kbdif.h
@@ -26,46 +26,227 @@
 #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 the XenStore, following the XenBus convention.
+ *
+ * All data in the 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 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 from 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.
+ *      A unique (within the guest domain given) value identifying event
+ *      channel and page reference pair.
+ */
 
 /*
- * 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 packets.
+ *
+ * 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                      |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 rel_x                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 rel_y                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 rel_z                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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             |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                keycode                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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                       |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 abs_x                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 abs_y                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 rel_z                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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 +260,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 +286,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 +314,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] 17+ messages in thread

* [PATCH 2/2] xen/kbdif: add multi-touch support
  2017-01-06  9:32 [PATCH 0/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
  2017-01-06  9:32 ` [PATCH 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
@ 2017-01-06  9:32 ` Oleksandr Andrushchenko
  2017-01-06 22:37   ` Stefano Stabellini
  2017-01-11  8:01 ` [PATCH 0/2] " Oleksandr Andrushchenko
  2 siblings, 1 reply; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-06  9:32 UTC (permalink / raw)
  To: xen-devel
  Cc: lars.kurth, sstabellini, vlad.babchuk, dario.faggioli,
	julien.grall, andrii.anisov, olekstysh, andr2000, al1img,
	JBeulich, joculator

From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>

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

diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
index 0e19a40..1b446f9 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 from backend to report absolute pointer coordinates
  *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
  *
+ * request-multi-touch
+ *      Values:         <uint>
+ *
+ *      Request from backend to report multi-touch events.
+ *
  *----------------------- Request Transport Parameters -----------------------
  *
  * event-channel
@@ -107,6 +118,43 @@
  *      OBSOLETE, not recommended for use.
  *      A unique (within the guest domain given) value identifying event
  *      channel and page reference pair.
+ *
+ *----------------------- Multi-touch Device Parameters -----------------------
+ *
+ * Every multi-touch input device uses a dedicated event ring and is
+ * 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].
+ *
+ *----------------------- Multi-touch 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.
  */
 
 /*
@@ -117,6 +165,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.
@@ -125,11 +183,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"
@@ -249,6 +313,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    |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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    |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 abs_x                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 abs_y                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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    |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * Multi-touch motion event
+ *          0                 1                  2                3        octet
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |  _TYPE_MTOUCH   |  _MT_EV_MOTION  |    contact_id   |     reserved    |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 abs_x                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 abs_y                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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    |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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    |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 major                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                                 minor                                 |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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    |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |            orientation            |             reserved              |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |                               reserved                                |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
+ * 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
-- 
2.7.4


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

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

* Re: [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-06  9:32 ` [PATCH 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
@ 2017-01-06 22:20   ` Stefano Stabellini
  2017-01-10  7:21     ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Stefano Stabellini @ 2017-01-06 22:20 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: lars.kurth, sstabellini, vlad.babchuk, dario.faggioli,
	julien.grall, andrii.anisov, olekstysh, al1img, JBeulich,
	xen-devel, joculator

On Fri, 6 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 | 249 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 222 insertions(+), 27 deletions(-)
> 
> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> index 2d2aebd..0e19a40 100644
> --- a/xen/include/public/io/kbdif.h
> +++ b/xen/include/public/io/kbdif.h
> @@ -26,46 +26,227 @@
>  #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

It's just "XenStore", not "the XenStore", all across the doc.


> + * XenStore to communicate capabilities and to negotiate operating parameters.
> + * This section enumerates these nodes which reside in the respective front and
> + * backend portions of the XenStore, following the XenBus convention.
> + *
> + * All data in the 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 feature is not supported then 0 must be set or feature entry omitted.
        ^ a feature

> + *
> + * 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 from 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.
> + *      A unique (within the guest domain given) value identifying event
> + *      channel and page reference pair.

Actually page-ref is just the pfn of the page, it doesn't have any event channel info.


> + */
>  
>  /*
> - * 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 packets.
                                       ^ I would call them "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                      |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 rel_x                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 rel_y                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 rel_z                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

I guess this means that we are skipping some bytes because they are all
reserved, right?  If so, it would be useful to write the byte count at this
point. What's the total size of the event struct?


> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * 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             |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                keycode                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

same here



> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * 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                       |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 abs_x                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 abs_y                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 rel_z                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

same here


> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * 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 +260,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 +286,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 +314,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] 17+ messages in thread

* Re: [PATCH 2/2] xen/kbdif: add multi-touch support
  2017-01-06  9:32 ` [PATCH 2/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
@ 2017-01-06 22:37   ` Stefano Stabellini
  2017-01-10  7:53     ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Stefano Stabellini @ 2017-01-06 22:37 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: lars.kurth, sstabellini, vlad.babchuk, dario.faggioli,
	julien.grall, andrii.anisov, olekstysh, al1img, JBeulich,
	xen-devel, joculator

On Fri, 6 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 | 228 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 228 insertions(+)
> 
> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> index 0e19a40..1b446f9 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 from backend to report absolute pointer coordinates
>   *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
>   *
> + * request-multi-touch
> + *      Values:         <uint>
> + *
> + *      Request from backend to report multi-touch events.

I think in English should be "request backend to report multi-touch
events". Same above.


> + *
>   *----------------------- Request Transport Parameters -----------------------
>   *
>   * event-channel
> @@ -107,6 +118,43 @@
>   *      OBSOLETE, not recommended for use.
>   *      A unique (within the guest domain given) value identifying event
>   *      channel and page reference pair.
> + *
> + *----------------------- Multi-touch Device Parameters -----------------------
> + *
> + * 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.


> + * configured via XenStore properties under XENKBD_PATH_MTOUCH folder.

I don't understand why we need a new xenstore folder. Why do we need
XENKBD_PATH_MTOUCH?


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

This is OK.


> + *----------------------- Multi-touch 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.
>   */

No need for this. If request-multi-touch, the usual xenkbd ring will be
used for multi touch events. We can use the event-channel and page-gref
already specified under "Request Transport Parameters".



>  
>  /*
> @@ -117,6 +165,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.
> @@ -125,11 +183,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"
> @@ -249,6 +313,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    |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+

Given that the structs are all of the same size, shouldn't there be more
reserved bytes here?


> + * 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    |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 abs_x                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 abs_y                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

Please write the byte count to specify how many bytes have been skipped.


> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * 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    |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

Please write the byte count to specify how many bytes have been skipped.


> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * Multi-touch motion event
> + *          0                 1                  2                3        octet
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |  _TYPE_MTOUCH   |  _MT_EV_MOTION  |    contact_id   |     reserved    |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 abs_x                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 abs_y                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

Please write the byte count to specify how many bytes have been skipped.


> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * 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    |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

Please write the byte count to specify how many bytes have been skipped.


> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * 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    |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 major                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                                 minor                                 |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

Please write the byte count to specify how many bytes have been skipped.


> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * 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    |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |            orientation            |             reserved              |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |                               reserved                                |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +-----------------+-----------------+-----------------+-----------------+

Please write the byte count to specify how many bytes have been skipped.


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

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

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

* Re: [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-06 22:20   ` Stefano Stabellini
@ 2017-01-10  7:21     ` Oleksandr Andrushchenko
  2017-01-11 17:35       ` Dario Faggioli
  0 siblings, 1 reply; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-10  7:21 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: lars.kurth, vlad.babchuk, dario.faggioli, julien.grall,
	andrii.anisov, olekstysh, al1img, JBeulich, xen-devel, joculator


On 01/07/2017 12:20 AM, Stefano Stabellini wrote:
> On Fri, 6 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 | 249 +++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 222 insertions(+), 27 deletions(-)
>>
>> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
>> index 2d2aebd..0e19a40 100644
>> --- a/xen/include/public/io/kbdif.h
>> +++ b/xen/include/public/io/kbdif.h
>> @@ -26,46 +26,227 @@
>>   #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
> It's just "XenStore", not "the XenStore", all across the doc.
Fixed
>
>
>> + * XenStore to communicate capabilities and to negotiate operating parameters.
>> + * This section enumerates these nodes which reside in the respective front and
>> + * backend portions of the XenStore, following the XenBus convention.
>> + *
>> + * All data in the 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 feature is not supported then 0 must be set or feature entry omitted.
>          ^ a feature
Fixed
>> + *
>> + * 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 from 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.
>> + *      A unique (within the guest domain given) value identifying event
>> + *      channel and page reference pair.
> Actually page-ref is just the pfn of the page, it doesn't have any event channel info.
Yes, I see this in the front driver which is concrete front's
implementation(?). For that reason I put something about
unique value. But I'll change that to "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 packets.
>                                         ^ I would call them "event structures"
Done
>
>> + * 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                      |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 rel_x                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 rel_y                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 rel_z                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> I guess this means that we are skipping some bytes because they are all
> reserved, right?  If so, it would be useful to write the byte count at this
> point. What's the total size of the event struct?
>
IMO, we shouldn't put any sizes here because:
1. Above we say "All event packets have the same
    length (40 octets)"
2. All the event structures are part of the
union xenkbd_in_event, which has
char pad[XENKBD_IN_EVENT_SIZE];
which effectively regulates the size of the event.

So, I think that the event structures on their own
shouldn't have the total size in their description.
The same applies to the below comments of the same kind.
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * 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             |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                keycode                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> same here
>
>
>
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * 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                       |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 abs_x                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 abs_y                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 rel_z                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> same here
>
>
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * 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 +260,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 +286,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 +314,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] 17+ messages in thread

* Re: [PATCH 2/2] xen/kbdif: add multi-touch support
  2017-01-06 22:37   ` Stefano Stabellini
@ 2017-01-10  7:53     ` Oleksandr Andrushchenko
  2017-01-11  0:29       ` Stefano Stabellini
  0 siblings, 1 reply; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-10  7:53 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: lars.kurth, vlad.babchuk, dario.faggioli, julien.grall,
	andrii.anisov, olekstysh, al1img, JBeulich, xen-devel, joculator


On 01/07/2017 12:37 AM, Stefano Stabellini wrote:
> On Fri, 6 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 | 228 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 228 insertions(+)
>>
>> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
>> index 0e19a40..1b446f9 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 from backend to report absolute pointer coordinates
>>    *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
>>    *
>> + * request-multi-touch
>> + *      Values:         <uint>
>> + *
>> + *      Request from backend to report multi-touch events.
> I think in English should be "request backend to report multi-touch
> events". Same above.
Done
>
>> + *
>>    *----------------------- Request Transport Parameters -----------------------
>>    *
>>    * event-channel
>> @@ -107,6 +118,43 @@
>>    *      OBSOLETE, not recommended for use.
>>    *      A unique (within the guest domain given) value identifying event
>>    *      channel and page reference pair.
>> + *
>> + *----------------------- Multi-touch Device Parameters -----------------------
>> + *
>> + * 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.
>
Done
>> + * configured via XenStore properties under XENKBD_PATH_MTOUCH folder.
> I don't understand why we need a new xenstore folder. Why do we need
> XENKBD_PATH_MTOUCH?
This is just another (IMO, cleaner) approach to define
properties in XenStore for multiple instances.
Let's see what vif does on my PC:
/local/domain/11/device/vif/0/queue-0/tx-ring-ref = "2304"
...
/local/domain/11/device/vif/0/queue-1/tx-ring-ref = "2306"
...
/local/domain/11/device/vif/0/queue-2/tx-ring-ref = "2308"
...
/local/domain/11/device/vif/0/queue-3/tx-ring-ref = "2310"
...
/local/domain/11/device/vif/0/request-rx-copy = "1"

We have folders "queue-%d" per queue which in my case are

under XENKBD_PATH_MTOUCH.

/local/domain/11/device/vkbd/0/mtouch/0/width = "3200"
/local/domain/11/device/vkbd/0/mtouch/0/height = "3200"
/local/domain/11/device/vkbd/0/mtouch/0/num-contacts = "5"
/local/domain/11/device/vkbd/0/mtouch/1/width = "6400"
/local/domain/11/device/vkbd/0/mtouch/1/height = "6400"
/local/domain/11/device/vkbd/0/mtouch/1/num-contacts = "10"

Namely, instead of "mtouch-%d" I use "mtouch/%d/.
This is just like using "/local/domain/%d" but not
"/local/domain-%d", so "mtouch/%d" seem to be more
consistent.
>
>> + * 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].
> This is OK.
>
>
>> + *----------------------- Multi-touch 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.
>>    */
> No need for this. If request-multi-touch, the usual xenkbd ring will be
> used for multi touch events. We can use the event-channel and page-gref
> already specified under "Request Transport Parameters".
Ok, will remove
>
>
>>   
>>   /*
>> @@ -117,6 +165,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.
>> @@ -125,11 +183,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"
>> @@ -249,6 +313,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    |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
> Given that the structs are all of the same size, shouldn't there be more
> reserved bytes here?
why? The common part is:
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 {
>
>> + * 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    |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 abs_x                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 abs_y                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> Please write the byte count to specify how many bytes have been skipped.
>
As I already commented in [PATCH 1/2] I would keep it as as.
1. "All event packets have the same length (40 octets)"
2.
union xenkbd_in_event
{
     uint8_t type;
...
     char pad[XENKBD_IN_EVENT_SIZE];
};
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * 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    |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> Please write the byte count to specify how many bytes have been skipped.
>
>
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * Multi-touch motion event
>> + *          0                 1                  2                3        octet
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |  _TYPE_MTOUCH   |  _MT_EV_MOTION  |    contact_id   |     reserved    |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 abs_x                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 abs_y                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> Please write the byte count to specify how many bytes have been skipped.
>
>
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * 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    |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> Please write the byte count to specify how many bytes have been skipped.
>
>
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * 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    |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 major                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                                 minor                                 |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> Please write the byte count to specify how many bytes have been skipped.
>
>
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * 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    |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |            orientation            |             reserved              |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +-----------------+-----------------+-----------------+-----------------+
> Please write the byte count to specify how many bytes have been skipped.
>
>
>> + * |                               reserved                                |
>> + * +-----------------+-----------------+-----------------+-----------------+
>> + *
>> + * 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
>> -- 
>> 2.7.4
>>


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

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

* Re: [PATCH 2/2] xen/kbdif: add multi-touch support
  2017-01-10  7:53     ` Oleksandr Andrushchenko
@ 2017-01-11  0:29       ` Stefano Stabellini
  2017-01-11  9:32         ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Stefano Stabellini @ 2017-01-11  0:29 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: lars.kurth, Stefano Stabellini, vlad.babchuk, dario.faggioli,
	julien.grall, andrii.anisov, olekstysh, al1img, JBeulich,
	xen-devel, joculator

On Tue, 10 Jan 2017, Oleksandr Andrushchenko wrote:
> On 01/07/2017 12:37 AM, Stefano Stabellini wrote:
> > On Fri, 6 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 | 228
> > > ++++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 228 insertions(+)
> > > 
> > > diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> > > index 0e19a40..1b446f9 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 from backend to report absolute pointer coordinates
> > >    *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
> > >    *
> > > + * request-multi-touch
> > > + *      Values:         <uint>
> > > + *
> > > + *      Request from backend to report multi-touch events.
> > I think in English should be "request backend to report multi-touch
> > events". Same above.
> Done
> > 
> > > + *
> > >    *----------------------- Request Transport Parameters
> > > -----------------------
> > >    *
> > >    * event-channel
> > > @@ -107,6 +118,43 @@
> > >    *      OBSOLETE, not recommended for use.
> > >    *      A unique (within the guest domain given) value identifying event
> > >    *      channel and page reference pair.
> > > + *
> > > + *----------------------- Multi-touch Device Parameters
> > > -----------------------
> > > + *
> > > + * 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.
> > 
> Done
> > > + * configured via XenStore properties under XENKBD_PATH_MTOUCH folder.
> > I don't understand why we need a new xenstore folder. Why do we need
> > XENKBD_PATH_MTOUCH?
> This is just another (IMO, cleaner) approach to define
> properties in XenStore for multiple instances.
> Let's see what vif does on my PC:
> /local/domain/11/device/vif/0/queue-0/tx-ring-ref = "2304"
> ...
> /local/domain/11/device/vif/0/queue-1/tx-ring-ref = "2306"
> ...
> /local/domain/11/device/vif/0/queue-2/tx-ring-ref = "2308"
> ...
> /local/domain/11/device/vif/0/queue-3/tx-ring-ref = "2310"
> ...
> /local/domain/11/device/vif/0/request-rx-copy = "1"
> 
> We have folders "queue-%d" per queue which in my case are
> 
> under XENKBD_PATH_MTOUCH.
> 
> /local/domain/11/device/vkbd/0/mtouch/0/width = "3200"
> /local/domain/11/device/vkbd/0/mtouch/0/height = "3200"
> /local/domain/11/device/vkbd/0/mtouch/0/num-contacts = "5"
> /local/domain/11/device/vkbd/0/mtouch/1/width = "6400"
> /local/domain/11/device/vkbd/0/mtouch/1/height = "6400"
> /local/domain/11/device/vkbd/0/mtouch/1/num-contacts = "10"
> 
> Namely, instead of "mtouch-%d" I use "mtouch/%d/.
> This is just like using "/local/domain/%d" but not
> "/local/domain-%d", so "mtouch/%d" seem to be more
> consistent.

I agree that mtouch/%d is better than mtouch-%d, but it's only necessary
if we have more than one mtouch per vkbd. I would configure it so that
one can only have one mtouch per vkbd:

  /local/domain/11/device/vkbd/0/mtouch/width = "3200"
  /local/domain/11/device/vkbd/0/mtouch/height = "3200"
  /local/domain/11/device/vkbd/1/mtouch/width = "6400"
  /local/domain/11/device/vkbd/1/mtouch/height = "6400"

This is also what I was referring to when I wrote:

> It is useful to distinguish mouse events from keyboard events, from
> multitouch events more easily, but I don't think we should support
> multiple keyboards or multiple mice on the same xenkbd ring. If we need
> two mice, we can setup two xenkdb rings.

The same applies to multitouch devices, I think.

But maybe you have good reasons for supporting multiple multitouch
devices on a single vkbd connection? How many do you expect to have on a
single VM?

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

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

* Re: [PATCH 0/2] xen/kbdif: add multi-touch support
  2017-01-06  9:32 [PATCH 0/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
  2017-01-06  9:32 ` [PATCH 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
  2017-01-06  9:32 ` [PATCH 2/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
@ 2017-01-11  8:01 ` Oleksandr Andrushchenko
  2 siblings, 0 replies; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-11  8:01 UTC (permalink / raw)
  To: xen-devel
  Cc: lars.kurth, sstabellini, vlad.babchuk, dario.faggioli,
	julien.grall, andrii.anisov, olekstysh, al1img, JBeulich,
	joculator


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

As agreed on PV call PFA pahole results


On 01/06/2017 11:32 AM, Oleksandr Andrushchenko wrote:
> 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 | 477 +++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 450 insertions(+), 27 deletions(-)
>


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: kbdif.diff --]
[-- Type: text/x-patch; name="kbdif.diff", Size: 0 bytes --]



[-- Attachment #3: kbdif-x32.txt --]
[-- Type: text/plain, Size: 2552 bytes --]

struct xenkbd_motion {
	uint8_t                    type;                 /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	int32_t                    rel_x;                /*     4     4 */
	int32_t                    rel_y;                /*     8     4 */
	int32_t                    rel_z;                /*    12     4 */

	/* size: 16, cachelines: 1, members: 4 */
	/* sum members: 13, holes: 1, sum holes: 3 */
	/* last cacheline: 16 bytes */
};
struct xenkbd_key {
	uint8_t                    type;                 /*     0     1 */
	uint8_t                    pressed;              /*     1     1 */

	/* XXX 2 bytes hole, try to pack */

	uint32_t                   keycode;              /*     4     4 */

	/* size: 8, cachelines: 1, members: 3 */
	/* sum members: 6, holes: 1, sum holes: 2 */
	/* last cacheline: 8 bytes */
};
struct xenkbd_position {
	uint8_t                    type;                 /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	int32_t                    abs_x;                /*     4     4 */
	int32_t                    abs_y;                /*     8     4 */
	int32_t                    rel_z;                /*    12     4 */

	/* size: 16, cachelines: 1, members: 4 */
	/* sum members: 13, holes: 1, sum holes: 3 */
	/* last cacheline: 16 bytes */
};
struct xenkbd_mtouch {
	uint8_t                    type;                 /*     0     1 */
	uint8_t                    event_type;           /*     1     1 */
	uint8_t                    contact_id;           /*     2     1 */
	uint8_t                    reserved[5];          /*     3     5 */
	union {
		struct {
			int32_t    abs_x;                /*     8     4 */
			int32_t    abs_y;                /*    12     4 */
		} pos;                                   /*           8 */
		struct {
			uint32_t   major;                /*     8     4 */
			uint32_t   minor;                /*    12     4 */
		} shape;                                 /*           8 */
		int16_t            orientation;          /*           2 */
	} u;                                             /*     8     8 */

	/* size: 16, cachelines: 1, members: 5 */
	/* last cacheline: 16 bytes */
};
struct xenkbd_page {
	uint32_t                   in_cons;              /*     0     4 */
	uint32_t                   in_prod;              /*     4     4 */
	uint32_t                   out_cons;             /*     8     4 */
	uint32_t                   out_prod;             /*    12     4 */

	/* size: 16, cachelines: 1, members: 4 */
	/* last cacheline: 16 bytes */
};

[-- Attachment #4: kbdif-x64.txt --]
[-- Type: text/plain, Size: 2552 bytes --]

struct xenkbd_motion {
	uint8_t                    type;                 /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	int32_t                    rel_x;                /*     4     4 */
	int32_t                    rel_y;                /*     8     4 */
	int32_t                    rel_z;                /*    12     4 */

	/* size: 16, cachelines: 1, members: 4 */
	/* sum members: 13, holes: 1, sum holes: 3 */
	/* last cacheline: 16 bytes */
};
struct xenkbd_key {
	uint8_t                    type;                 /*     0     1 */
	uint8_t                    pressed;              /*     1     1 */

	/* XXX 2 bytes hole, try to pack */

	uint32_t                   keycode;              /*     4     4 */

	/* size: 8, cachelines: 1, members: 3 */
	/* sum members: 6, holes: 1, sum holes: 2 */
	/* last cacheline: 8 bytes */
};
struct xenkbd_position {
	uint8_t                    type;                 /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	int32_t                    abs_x;                /*     4     4 */
	int32_t                    abs_y;                /*     8     4 */
	int32_t                    rel_z;                /*    12     4 */

	/* size: 16, cachelines: 1, members: 4 */
	/* sum members: 13, holes: 1, sum holes: 3 */
	/* last cacheline: 16 bytes */
};
struct xenkbd_mtouch {
	uint8_t                    type;                 /*     0     1 */
	uint8_t                    event_type;           /*     1     1 */
	uint8_t                    contact_id;           /*     2     1 */
	uint8_t                    reserved[5];          /*     3     5 */
	union {
		struct {
			int32_t    abs_x;                /*     8     4 */
			int32_t    abs_y;                /*    12     4 */
		} pos;                                   /*           8 */
		struct {
			uint32_t   major;                /*     8     4 */
			uint32_t   minor;                /*    12     4 */
		} shape;                                 /*           8 */
		int16_t            orientation;          /*           2 */
	} u;                                             /*     8     8 */

	/* size: 16, cachelines: 1, members: 5 */
	/* last cacheline: 16 bytes */
};
struct xenkbd_page {
	uint32_t                   in_cons;              /*     0     4 */
	uint32_t                   in_prod;              /*     4     4 */
	uint32_t                   out_cons;             /*     8     4 */
	uint32_t                   out_prod;             /*    12     4 */

	/* size: 16, cachelines: 1, members: 4 */
	/* last cacheline: 16 bytes */
};

[-- Attachment #5: 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] 17+ messages in thread

* Re: [PATCH 2/2] xen/kbdif: add multi-touch support
  2017-01-11  0:29       ` Stefano Stabellini
@ 2017-01-11  9:32         ` Oleksandr Andrushchenko
  2017-01-19 22:10           ` Stefano Stabellini
  0 siblings, 1 reply; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-11  9:32 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: lars.kurth, vlad.babchuk, dario.faggioli, julien.grall,
	andrii.anisov, olekstysh, al1img, JBeulich, xen-devel, joculator

On 01/11/2017 02:29 AM, Stefano Stabellini wrote:
> On Tue, 10 Jan 2017, Oleksandr Andrushchenko wrote:
>> On 01/07/2017 12:37 AM, Stefano Stabellini wrote:
>>> On Fri, 6 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 | 228
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 228 insertions(+)
>>>>
>>>> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
>>>> index 0e19a40..1b446f9 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 from backend to report absolute pointer coordinates
>>>>     *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
>>>>     *
>>>> + * request-multi-touch
>>>> + *      Values:         <uint>
>>>> + *
>>>> + *      Request from backend to report multi-touch events.
>>> I think in English should be "request backend to report multi-touch
>>> events". Same above.
>> Done
>>>> + *
>>>>     *----------------------- Request Transport Parameters
>>>> -----------------------
>>>>     *
>>>>     * event-channel
>>>> @@ -107,6 +118,43 @@
>>>>     *      OBSOLETE, not recommended for use.
>>>>     *      A unique (within the guest domain given) value identifying event
>>>>     *      channel and page reference pair.
>>>> + *
>>>> + *----------------------- Multi-touch Device Parameters
>>>> -----------------------
>>>> + *
>>>> + * 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.
>>>
>> Done
>>>> + * configured via XenStore properties under XENKBD_PATH_MTOUCH folder.
>>> I don't understand why we need a new xenstore folder. Why do we need
>>> XENKBD_PATH_MTOUCH?
>> This is just another (IMO, cleaner) approach to define
>> properties in XenStore for multiple instances.
>> Let's see what vif does on my PC:
>> /local/domain/11/device/vif/0/queue-0/tx-ring-ref = "2304"
>> ...
>> /local/domain/11/device/vif/0/queue-1/tx-ring-ref = "2306"
>> ...
>> /local/domain/11/device/vif/0/queue-2/tx-ring-ref = "2308"
>> ...
>> /local/domain/11/device/vif/0/queue-3/tx-ring-ref = "2310"
>> ...
>> /local/domain/11/device/vif/0/request-rx-copy = "1"
>>
>> We have folders "queue-%d" per queue which in my case are
>>
>> under XENKBD_PATH_MTOUCH.
>>
>> /local/domain/11/device/vkbd/0/mtouch/0/width = "3200"
>> /local/domain/11/device/vkbd/0/mtouch/0/height = "3200"
>> /local/domain/11/device/vkbd/0/mtouch/0/num-contacts = "5"
>> /local/domain/11/device/vkbd/0/mtouch/1/width = "6400"
>> /local/domain/11/device/vkbd/0/mtouch/1/height = "6400"
>> /local/domain/11/device/vkbd/0/mtouch/1/num-contacts = "10"
>>
>> Namely, instead of "mtouch-%d" I use "mtouch/%d/.
>> This is just like using "/local/domain/%d" but not
>> "/local/domain-%d", so "mtouch/%d" seem to be more
>> consistent.
> I agree that mtouch/%d is better than mtouch-%d, but it's only necessary
> if we have more than one mtouch per vkbd. I would configure it so that
> one can only have one mtouch per vkbd:
>
>    /local/domain/11/device/vkbd/0/mtouch/width = "3200"
>    /local/domain/11/device/vkbd/0/mtouch/height = "3200"
>    /local/domain/11/device/vkbd/1/mtouch/width = "6400"
>    /local/domain/11/device/vkbd/1/mtouch/height = "6400"
correct me if I'm wrong, but this notation implies
multiple drivers support, not a single driver with
multiple devices.
If so, *DISCLAIMER* *Linux* I see no simple solution to
do that in the front driver.
Could you please clarify?
> This is also what I was referring to when I wrote:
>
>> It is useful to distinguish mouse events from keyboard events, from
>> multitouch events more easily, but I don't think we should support
>> multiple keyboards or multiple mice on the same xenkbd ring. If we need
>> two mice, we can setup two xenkdb rings.
> The same applies to multitouch devices, I think.
>
> But maybe you have good reasons for supporting multiple multitouch
> devices on a single vkbd connection? How many do you expect to have on a
> single VM?
well, let me put the whole tree from xenstore:

/local/domain/0/backend/vkbd/2/0/frontend-id = "2"
/local/domain/0/backend/vkbd/2/0/frontend = "/local/domain/2/device/vkbd/0"
/local/domain/0/backend/vkbd/2/0/dev_id = "0"
/local/domain/0/backend/vkbd/2/0/state = "6"
/local/domain/0/backend/vkbd/2/0/feature-abs-pointer = "1"
/local/domain/0/backend/vkbd/2/0/width = "2048"
/local/domain/0/backend/vkbd/2/0/height = "2048"
/local/domain/0/backend/vkbd/2/0/feature-multi-touch = "1"

/local/domain/2/device/vkbd/0/backend-id = "0"
/local/domain/2/device/vkbd/0/backend = "/local/domain/0/backend/vkbd/2/0"
/local/domain/2/device/vkbd/0/state = "6"
/local/domain/2/device/vkbd/0/request-abs-pointer = "1"
/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"

/local/domain/2/device/vkbd/0/request-multi-touch = "1"

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

 From the above:
1. No change to the existing kbd+ptr:
1.1. They still share a dedicated (single) connection
channel as they did before multi-touch.
1.2. No multi-touch events via that connection
1.3. Old backends/frontends will see no difference
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)

So, for the example above: 1 kbd + 1 ptr + 2 mt devices
within a single driver, 3 connections

Note: this doesn't allow multiple kbd and ptr devices.

Hope this answers your questions,
Oleksandr

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

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

* Re: [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-10  7:21     ` Oleksandr Andrushchenko
@ 2017-01-11 17:35       ` Dario Faggioli
  2017-01-11 18:40         ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Dario Faggioli @ 2017-01-11 17:35 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, Stefano Stabellini
  Cc: lars.kurth, vlad.babchuk, julien.grall, andrii.anisov, olekstysh,
	al1img, JBeulich, xen-devel, joculator


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

On Tue, 2017-01-10 at 09:21 +0200, Oleksandr Andrushchenko wrote:
> On 01/07/2017 12:20 AM, Stefano Stabellini wrote:
> > On Fri, 6 Jan 2017, Oleksandr Andrushchenko wrote:
> > > |                               reserved                         
> > >        |
> > > + * +-----------------+-----------------+-----------------+----
> > > -------------+
> > > + *
> > > |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
> > > /\/\/\/|
> > > + * +-----------------+-----------------+-----------------+----
> > > -------------+
> > I guess this means that we are skipping some bytes because they are
> > all
> > reserved, right?  If so, it would be useful to write the byte count
> > at this
> > point. What's the total size of the event struct?
> > 
> IMO, we shouldn't put any sizes here because:
> 1. Above we say "All event packets have the same
>     length (40 octets)"
> 2. All the event structures are part of the
> union xenkbd_in_event, which has
> char pad[XENKBD_IN_EVENT_SIZE];
> which effectively regulates the size of the event.
> 
In which case, you can use either 40 or XENKBD_IN_EVENT_SIZE (probably
the latter).

It's indeed a repetition, but a good one, IMO: it helps the reader, as
she won't have to go back to figure out how big the struct was, how the
macro was call and to what value it was defined).

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 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] 17+ messages in thread

* Re: [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-11 17:35       ` Dario Faggioli
@ 2017-01-11 18:40         ` Oleksandr Andrushchenko
  2017-01-11 22:50           ` Dario Faggioli
  0 siblings, 1 reply; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-11 18:40 UTC (permalink / raw)
  To: Dario Faggioli, Stefano Stabellini
  Cc: lars.kurth, vlad.babchuk, julien.grall, andrii.anisov, olekstysh,
	al1img, JBeulich, xen-devel, joculator

On 01/11/2017 07:35 PM, Dario Faggioli wrote:
> On Tue, 2017-01-10 at 09:21 +0200, Oleksandr Andrushchenko wrote:
>> On 01/07/2017 12:20 AM, Stefano Stabellini wrote:
>>> On Fri, 6 Jan 2017, Oleksandr Andrushchenko wrote:
>>>> |                               reserved
>>>>         |
>>>> + * +-----------------+-----------------+-----------------+----
>>>> -------------+
>>>> + *
>>>> |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
>>>> /\/\/\/|
>>>> + * +-----------------+-----------------+-----------------+----
>>>> -------------+
>>> I guess this means that we are skipping some bytes because they are
>>> all
>>> reserved, right?  If so, it would be useful to write the byte count
>>> at this
>>> point. What's the total size of the event struct?
>>>
>> IMO, we shouldn't put any sizes here because:
>> 1. Above we say "All event packets have the same
>>      length (40 octets)"
>> 2. All the event structures are part of the
>> union xenkbd_in_event, which has
>> char pad[XENKBD_IN_EVENT_SIZE];
>> which effectively regulates the size of the event.
>>
> In which case, you can use either 40 or XENKBD_IN_EVENT_SIZE (probably
> the latter).
>
> It's indeed a repetition, but a good one, IMO: it helps the reader, as
> she won't have to go back to figure out how big the struct was, how the
> macro was call and to what value it was defined).
I am still not convinced that we should put it.
Probably we can go the way other RFCs do, like TCP [1], 802.11 [2] etc:
those do not define any reserved fields at the bottom of structures,
(which are effectively padding?) but are limited to only those fields
which are defined. This means that, for example, instead of

  *          0                 1                  2 3        octet
  * 
+-----------------+-----------------+-----------------+-----------------+
  * |   _TYPE_MOTION  | reserved                      |
  * 
+-----------------+-----------------+-----------------+-----------------+
  * | rel_x                                 |
  * 
+-----------------+-----------------+-----------------+-----------------+
  * | rel_y                                 |
  * 
+-----------------+-----------------+-----------------+-----------------+
  * | rel_z                                 |
  * 
+-----------------+-----------------+-----------------+-----------------+
  * | reserved                                |
  * 
+-----------------+-----------------+-----------------+-----------------+
  * 
|/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
  * 
+-----------------+-----------------+-----------------+-----------------+
  * | reserved                                |
  * 
+-----------------+-----------------+-----------------+-----------------+

simply put

  *          0                 1                  2 3        octet
  * 
+-----------------+-----------------+-----------------+-----------------+
  * |   _TYPE_MOTION  | reserved                      |
  * 
+-----------------+-----------------+-----------------+-----------------+
  * | rel_x                                 |
  * 
+-----------------+-----------------+-----------------+-----------------+
  * | rel_y                                 |
  * 
+-----------------+-----------------+-----------------+-----------------+
  * | rel_z                                 |
  * 
+-----------------+-----------------+-----------------+-----------------+

What do you think?


[1] https://www.ietf.org/rfc/rfc793.txt
[2] https://tools.ietf.org/html/rfc5416
> Regards,
> Dario
Thank you,
Oleksandr

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

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

* Re: [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-11 18:40         ` Oleksandr Andrushchenko
@ 2017-01-11 22:50           ` Dario Faggioli
  2017-01-12  6:36             ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Dario Faggioli @ 2017-01-11 22:50 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, Stefano Stabellini
  Cc: lars.kurth, vlad.babchuk, julien.grall, andrii.anisov, olekstysh,
	al1img, JBeulich, xen-devel, joculator


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

On Wed, 2017-01-11 at 20:40 +0200, Oleksandr Andrushchenko wrote:
> On 01/11/2017 07:35 PM, Dario Faggioli wrote:
> > 
> > It's indeed a repetition, but a good one, IMO: it helps the reader,
> > as
> > she won't have to go back to figure out how big the struct was, how
> > the
> > macro was call and to what value it was defined).
> I am still not convinced that we should put it.
> Probably we can go the way other RFCs do, like TCP [1], 802.11 [2]
> etc:
> those do not define any reserved fields at the bottom of structures,
> (which are effectively padding?) but are limited to only those fields
> which are defined.
>
In principle, I like the idea of following the example of those RFCs.
However, I'd say that what we should value most is consistency within
our own source tree.

But, TBH, there aren't many binary diagram already committed in
include/public/io, so it's hard to tell.

FWIW, I still think that providing a clue to the reader about the size
--even if already specified somewhere else-- would be beneficial, but
it's a rather minor thing, and I certainly can leave with whatever you
and the maintainer(s) agree upon.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 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] 17+ messages in thread

* Re: [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-11 22:50           ` Dario Faggioli
@ 2017-01-12  6:36             ` Oleksandr Andrushchenko
  2017-01-18 19:41               ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-12  6:36 UTC (permalink / raw)
  To: konrad.wilk
  Cc: lars.kurth, Stefano Stabellini, vlad.babchuk, Dario Faggioli,
	julien.grall, andrii.anisov, olekstysh, al1img, JBeulich,
	xen-devel, joculator

On 01/12/2017 12:50 AM, Dario Faggioli wrote:
> On Wed, 2017-01-11 at 20:40 +0200, Oleksandr Andrushchenko wrote:
>> On 01/11/2017 07:35 PM, Dario Faggioli wrote:
>>>   
>>> It's indeed a repetition, but a good one, IMO: it helps the reader,
>>> as
>>> she won't have to go back to figure out how big the struct was, how
>>> the
>>> macro was call and to what value it was defined).
>> I am still not convinced that we should put it.
>> Probably we can go the way other RFCs do, like TCP [1], 802.11 [2]
>> etc:
>> those do not define any reserved fields at the bottom of structures,
>> (which are effectively padding?) but are limited to only those fields
>> which are defined.
>>
> In principle, I like the idea of following the example of those RFCs.
> However, I'd say that what we should value most is consistency within
> our own source tree.
>
> But, TBH, there aren't many binary diagram already committed in
> include/public/io, so it's hard to tell.
>
> FWIW, I still think that providing a clue to the reader about the size
> --even if already specified somewhere else-- would be beneficial, but
> it's a rather minor thing, and I certainly can leave with whatever you
> and the maintainer(s) agree upon.
fair enough
> Regards,
> Dario
Konrad, could you please define what that ASCII box
notation should look like?

Thank you,
Oleksandr

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

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

* Re: [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-12  6:36             ` Oleksandr Andrushchenko
@ 2017-01-18 19:41               ` Oleksandr Andrushchenko
  2017-01-18 20:28                 ` Stefano Stabellini
  0 siblings, 1 reply; 17+ messages in thread
From: Oleksandr Andrushchenko @ 2017-01-18 19:41 UTC (permalink / raw)
  To: konrad.wilk, Stefano Stabellini
  Cc: lars.kurth, vlad.babchuk, Dario Faggioli, julien.grall,
	andrii.anisov, olekstysh, al1img, JBeulich, xen-devel, joculator

On 01/12/2017 08:36 AM, Oleksandr Andrushchenko wrote:
> On 01/12/2017 12:50 AM, Dario Faggioli wrote:
>> On Wed, 2017-01-11 at 20:40 +0200, Oleksandr Andrushchenko wrote:
>>> On 01/11/2017 07:35 PM, Dario Faggioli wrote:
>>>>   It's indeed a repetition, but a good one, IMO: it helps the reader,
>>>> as
>>>> she won't have to go back to figure out how big the struct was, how
>>>> the
>>>> macro was call and to what value it was defined).
>>> I am still not convinced that we should put it.
>>> Probably we can go the way other RFCs do, like TCP [1], 802.11 [2]
>>> etc:
>>> those do not define any reserved fields at the bottom of structures,
>>> (which are effectively padding?) but are limited to only those fields
>>> which are defined.
>>>
>> In principle, I like the idea of following the example of those RFCs.
>> However, I'd say that what we should value most is consistency within
>> our own source tree.
>>
>> But, TBH, there aren't many binary diagram already committed in
>> include/public/io, so it's hard to tell.
>>
>> FWIW, I still think that providing a clue to the reader about the size
>> --even if already specified somewhere else-- would be beneficial, but
>> it's a rather minor thing, and I certainly can leave with whatever you
>> and the maintainer(s) agree upon.
> fair enough
>> Regards,
>> Dario
> Konrad, could you please define what that ASCII box
> notation should look like?
>
Stefano, Konrad
As per my understanding this is the only thing blocking multi-touch
and updated kbdif protocol from being upgraded/extended )
Could you please make some decision on this any time soon?
> Thank you,
> Oleksandr


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

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

* Re: [PATCH 1/2] xen/kbdif: update protocol documentation
  2017-01-18 19:41               ` Oleksandr Andrushchenko
@ 2017-01-18 20:28                 ` Stefano Stabellini
  0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2017-01-18 20:28 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: lars.kurth, Stefano Stabellini, konrad.wilk, vlad.babchuk,
	Dario Faggioli, julien.grall, andrii.anisov, olekstysh, al1img,
	JBeulich, xen-devel, joculator

On Wed, 18 Jan 2017, Oleksandr Andrushchenko wrote:
> On 01/12/2017 08:36 AM, Oleksandr Andrushchenko wrote:
> > On 01/12/2017 12:50 AM, Dario Faggioli wrote:
> > > On Wed, 2017-01-11 at 20:40 +0200, Oleksandr Andrushchenko wrote:
> > > > On 01/11/2017 07:35 PM, Dario Faggioli wrote:
> > > > >   It's indeed a repetition, but a good one, IMO: it helps the reader,
> > > > > as
> > > > > she won't have to go back to figure out how big the struct was, how
> > > > > the
> > > > > macro was call and to what value it was defined).
> > > > I am still not convinced that we should put it.
> > > > Probably we can go the way other RFCs do, like TCP [1], 802.11 [2]
> > > > etc:
> > > > those do not define any reserved fields at the bottom of structures,
> > > > (which are effectively padding?) but are limited to only those fields
> > > > which are defined.
> > > > 
> > > In principle, I like the idea of following the example of those RFCs.
> > > However, I'd say that what we should value most is consistency within
> > > our own source tree.
> > > 
> > > But, TBH, there aren't many binary diagram already committed in
> > > include/public/io, so it's hard to tell.
> > > 
> > > FWIW, I still think that providing a clue to the reader about the size
> > > --even if already specified somewhere else-- would be beneficial, but
> > > it's a rather minor thing, and I certainly can leave with whatever you
> > > and the maintainer(s) agree upon.
> > fair enough
> > > Regards,
> > > Dario
> > Konrad, could you please define what that ASCII box
> > notation should look like?
> > 
> Stefano, Konrad
> As per my understanding this is the only thing blocking multi-touch
> and updated kbdif protocol from being upgraded/extended )
> Could you please make some decision on this any time soon?

I would use something like:
    
            0                 1                 2                 3
  * +-----------------+-----------------+-----------------+-----------------+ 
  * |   _TYPE_MOTION  |                       reserved                      |4
  * +-----------------+-----------------+-----------------+-----------------+
  * |                                 rel_x                                 |8
  * +-----------------+-----------------+-----------------+-----------------+
  * |                                 rel_y                                 |12
  * +-----------------+-----------------+-----------------+-----------------+
  * |                                 rel_z                                 |16
  * +-----------------+-----------------+-----------------+-----------------+
  * |                               reserved                                |20
  * +-----------------+-----------------+-----------------+-----------------+
  * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|40
  * +-----------------+-----------------+-----------------+-----------------+

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

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

* Re: [PATCH 2/2] xen/kbdif: add multi-touch support
  2017-01-11  9:32         ` Oleksandr Andrushchenko
@ 2017-01-19 22:10           ` Stefano Stabellini
  0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2017-01-19 22:10 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: lars.kurth, Stefano Stabellini, konrad.wilk, vlad.babchuk,
	dario.faggioli, julien.grall, andrii.anisov, olekstysh, al1img,
	JBeulich, xen-devel, joculator

On Wed, 11 Jan 2017, Oleksandr Andrushchenko wrote:
> On 01/11/2017 02:29 AM, Stefano Stabellini wrote:
> > On Tue, 10 Jan 2017, Oleksandr Andrushchenko wrote:
> > > On 01/07/2017 12:37 AM, Stefano Stabellini wrote:
> > > > On Fri, 6 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 | 228
> > > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > >    1 file changed, 228 insertions(+)
> > > > > 
> > > > > diff --git a/xen/include/public/io/kbdif.h
> > > > > b/xen/include/public/io/kbdif.h
> > > > > index 0e19a40..1b446f9 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 from backend to report absolute pointer coordinates
> > > > >     *      (XENKBD_TYPE_POS) instead of relative ones
> > > > > (XENKBD_TYPE_MOTION).
> > > > >     *
> > > > > + * request-multi-touch
> > > > > + *      Values:         <uint>
> > > > > + *
> > > > > + *      Request from backend to report multi-touch events.
> > > > I think in English should be "request backend to report multi-touch
> > > > events". Same above.
> > > Done
> > > > > + *
> > > > >     *----------------------- Request Transport Parameters
> > > > > -----------------------
> > > > >     *
> > > > >     * event-channel
> > > > > @@ -107,6 +118,43 @@
> > > > >     *      OBSOLETE, not recommended for use.
> > > > >     *      A unique (within the guest domain given) value identifying
> > > > > event
> > > > >     *      channel and page reference pair.
> > > > > + *
> > > > > + *----------------------- Multi-touch Device Parameters
> > > > > -----------------------
> > > > > + *
> > > > > + * 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.
> > > > 
> > > Done
> > > > > + * configured via XenStore properties under XENKBD_PATH_MTOUCH
> > > > > folder.
> > > > I don't understand why we need a new xenstore folder. Why do we need
> > > > XENKBD_PATH_MTOUCH?
> > > This is just another (IMO, cleaner) approach to define
> > > properties in XenStore for multiple instances.
> > > Let's see what vif does on my PC:
> > > /local/domain/11/device/vif/0/queue-0/tx-ring-ref = "2304"
> > > ...
> > > /local/domain/11/device/vif/0/queue-1/tx-ring-ref = "2306"
> > > ...
> > > /local/domain/11/device/vif/0/queue-2/tx-ring-ref = "2308"
> > > ...
> > > /local/domain/11/device/vif/0/queue-3/tx-ring-ref = "2310"
> > > ...
> > > /local/domain/11/device/vif/0/request-rx-copy = "1"
> > > 
> > > We have folders "queue-%d" per queue which in my case are
> > > 
> > > under XENKBD_PATH_MTOUCH.
> > > 
> > > /local/domain/11/device/vkbd/0/mtouch/0/width = "3200"
> > > /local/domain/11/device/vkbd/0/mtouch/0/height = "3200"
> > > /local/domain/11/device/vkbd/0/mtouch/0/num-contacts = "5"
> > > /local/domain/11/device/vkbd/0/mtouch/1/width = "6400"
> > > /local/domain/11/device/vkbd/0/mtouch/1/height = "6400"
> > > /local/domain/11/device/vkbd/0/mtouch/1/num-contacts = "10"
> > > 
> > > Namely, instead of "mtouch-%d" I use "mtouch/%d/.
> > > This is just like using "/local/domain/%d" but not
> > > "/local/domain-%d", so "mtouch/%d" seem to be more
> > > consistent.
> > I agree that mtouch/%d is better than mtouch-%d, but it's only necessary
> > if we have more than one mtouch per vkbd. I would configure it so that
> > one can only have one mtouch per vkbd:
> > 
> >    /local/domain/11/device/vkbd/0/mtouch/width = "3200"
> >    /local/domain/11/device/vkbd/0/mtouch/height = "3200"
> >    /local/domain/11/device/vkbd/1/mtouch/width = "6400"
> >    /local/domain/11/device/vkbd/1/mtouch/height = "6400"
> correct me if I'm wrong, but this notation implies
> multiple drivers support, not a single driver with
> multiple devices.
> If so, *DISCLAIMER* *Linux* I see no simple solution to
> do that in the front driver.
> Could you please clarify?

I am not familiar with the multitouch driver infrastructure in Linux.
Why is it easier to advertise multiple multitouch devices under a single
xenstore connection? I would have thought it would be more difficult to
handle from Linux point of view.


> > This is also what I was referring to when I wrote:
> > 
> > > It is useful to distinguish mouse events from keyboard events, from
> > > multitouch events more easily, but I don't think we should support
> > > multiple keyboards or multiple mice on the same xenkbd ring. If we need
> > > two mice, we can setup two xenkdb rings.
> > The same applies to multitouch devices, I think.
> > 
> > But maybe you have good reasons for supporting multiple multitouch
> > devices on a single vkbd connection? How many do you expect to have on a
> > single VM?
> well, let me put the whole tree from xenstore:
> 
> /local/domain/0/backend/vkbd/2/0/frontend-id = "2"
> /local/domain/0/backend/vkbd/2/0/frontend = "/local/domain/2/device/vkbd/0"
> /local/domain/0/backend/vkbd/2/0/dev_id = "0"
> /local/domain/0/backend/vkbd/2/0/state = "6"
> /local/domain/0/backend/vkbd/2/0/feature-abs-pointer = "1"
> /local/domain/0/backend/vkbd/2/0/width = "2048"
> /local/domain/0/backend/vkbd/2/0/height = "2048"
> /local/domain/0/backend/vkbd/2/0/feature-multi-touch = "1"
> 
> /local/domain/2/device/vkbd/0/backend-id = "0"
> /local/domain/2/device/vkbd/0/backend = "/local/domain/0/backend/vkbd/2/0"
> /local/domain/2/device/vkbd/0/state = "6"
> /local/domain/2/device/vkbd/0/request-abs-pointer = "1"
> /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"
> 
> /local/domain/2/device/vkbd/0/request-multi-touch = "1"
> 
> /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"
> 
> From the above:
> 1. No change to the existing kbd+ptr:
> 1.1. They still share a dedicated (single) connection
> channel as they did before multi-touch.
> 1.2. No multi-touch events via that connection
> 1.3. Old backends/frontends will see no difference
> 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)
> 
> So, for the example above: 1 kbd + 1 ptr + 2 mt devices
> within a single driver, 3 connections
> 
> Note: this doesn't allow multiple kbd and ptr devices.
> 
> Hope this answers your questions,
> Oleksandr
> 

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

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

end of thread, other threads:[~2017-01-19 22:10 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-06  9:32 [PATCH 0/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
2017-01-06  9:32 ` [PATCH 1/2] xen/kbdif: update protocol documentation Oleksandr Andrushchenko
2017-01-06 22:20   ` Stefano Stabellini
2017-01-10  7:21     ` Oleksandr Andrushchenko
2017-01-11 17:35       ` Dario Faggioli
2017-01-11 18:40         ` Oleksandr Andrushchenko
2017-01-11 22:50           ` Dario Faggioli
2017-01-12  6:36             ` Oleksandr Andrushchenko
2017-01-18 19:41               ` Oleksandr Andrushchenko
2017-01-18 20:28                 ` Stefano Stabellini
2017-01-06  9:32 ` [PATCH 2/2] xen/kbdif: add multi-touch support Oleksandr Andrushchenko
2017-01-06 22:37   ` Stefano Stabellini
2017-01-10  7:53     ` Oleksandr Andrushchenko
2017-01-11  0:29       ` Stefano Stabellini
2017-01-11  9:32         ` Oleksandr Andrushchenko
2017-01-19 22:10           ` Stefano Stabellini
2017-01-11  8:01 ` [PATCH 0/2] " 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.