Skip to contentSkip to conversation

xen/input: add multi-touch support

PATCH In review Oleksandr Andrushchenko 2 participants 11 messages Jun 23, 2017 – Jun 30, 2017 Input drivers

lore
Share & tools
lore

Oleksandr Andrushchenko

author
#0 lore raw

PATCH 1/1 xen/input: add multi-touch support

From: Oleksandr Andrushchenko <[email protected]>

Extend xen_kbdfront to provide multi-touch support to unprivileged domains.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
Changes since initial:
- use input_set_capability instead of setting flags directly
- input_mt_init_slots: let userspace better chance of figuring how to handle the device: use INPUT_MT_DIRECT, drop INPUT_MT_DROP_UNUSED
- add error handling for input_mt_init_slots
- remove module paramters
- remove odd unlikely

---
 drivers/input/misc/xen-kbdfront.c | 135 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 2 deletions(-)
drivers/input/misc/xen-kbdfront.c +133 -2
index eb770613a9bd..9fa005038773 100644
@@ -17,6 +17,7 @@
 #include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/input.h>
+#include <linux/input/mt.h>
 #include <linux/slab.h>
 
 #include <asm/xen/hypervisor.h>

@@ -34,11 +35,14 @@
 struct xenkbd_info {
 	struct input_dev *kbd;
 	struct input_dev *ptr;
+	struct input_dev *mtouch;
 	struct xenkbd_page *page;
 	int gref;
 	int irq;
 	struct xenbus_device *xbdev;
 	char phys[32];
+	/* current MT slot/contact ID we are injecting events in */
+	int mtouch_cur_contact_id;
 };
 
 enum { KPARAM_X, KPARAM_Y, KPARAM_CNT };

@@ -100,6 +104,60 @@ static irqreturn_t input_handler(int rq, void *dev_id)
 				input_report_rel(dev, REL_WHEEL,
 						 -event->pos.rel_z);
 			break;
+		case XENKBD_TYPE_MTOUCH:
+			dev = info->mtouch;
+			if (unlikely(!dev))
+				break;
+			if (event->mtouch.contact_id !=
+					info->mtouch_cur_contact_id) {
+				info->mtouch_cur_contact_id =
+					event->mtouch.contact_id;
+				input_mt_slot(dev, event->mtouch.contact_id);
+			}
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);
+				break;
+			case XENKBD_MT_EV_UP:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   false);
+				break;
+			case XENKBD_MT_EV_MOTION:
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);
+				break;
+			case XENKBD_MT_EV_SYN:
+				input_mt_sync_frame(dev);
+				break;
+			case XENKBD_MT_EV_SHAPE:
+				input_event(dev, EV_ABS, ABS_MT_TOUCH_MAJOR,
+					    event->mtouch.u.shape.major);
+				input_event(dev, EV_ABS, ABS_MT_TOUCH_MINOR,
+					    event->mtouch.u.shape.minor);
+				break;
+			case XENKBD_MT_EV_ORIENT:
+				input_event(dev, EV_ABS, ABS_MT_ORIENTATION,
+					    event->mtouch.u.orientation);
+				break;
+			}
+			/* only report syn when requested */
+			if (event->mtouch.event_type != XENKBD_MT_EV_SYN)
+				dev = NULL;
 		}
 		if (dev)
 			input_sync(dev);

@@ -115,9 +173,9 @@ static int xenkbd_probe(struct xenbus_device *dev,
 				  const struct xenbus_device_id *id)
 {
 	int ret, i;
-	unsigned int abs;
+	unsigned int abs, touch;
 	struct xenkbd_info *info;
-	struct input_dev *kbd, *ptr;
+	struct input_dev *kbd, *ptr, *mtouch;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (!info) {

@@ -152,6 +210,17 @@ static int xenkbd_probe(struct xenbus_device *dev,
 		}
 	}
 
+	touch = xenbus_read_unsigned(dev->nodename,
+				     XENKBD_FIELD_FEAT_MTOUCH, 0);
+	if (touch) {
+		ret = xenbus_write(XBT_NIL, dev->nodename,
+				   XENKBD_FIELD_REQ_MTOUCH, "1");
+		if (ret) {
+			pr_warning("xenkbd: can't request multi-touch");
+			touch = 0;
+		}
+	}
+
 	/* keyboard */
 	kbd = input_allocate_device();
 	if (!kbd)

@@ -208,6 +277,66 @@ static int xenkbd_probe(struct xenbus_device *dev,
 	}
 	info->ptr = ptr;
 
+	/* multi-touch device */
+	if (touch) {
+		int num_cont, width, height;
+
+		mtouch = input_allocate_device();
+		if (!mtouch)
+			goto error_nomem;
+
+		num_cont = xenbus_read_unsigned(info->xbdev->nodename,
+						XENKBD_FIELD_MT_NUM_CONTACTS,
+						1);
+		width = xenbus_read_unsigned(info->xbdev->nodename,
+					     XENKBD_FIELD_MT_WIDTH,
+					     XENFB_WIDTH);
+		height = xenbus_read_unsigned(info->xbdev->nodename,
+					      XENKBD_FIELD_MT_HEIGHT,
+					      XENFB_HEIGHT);
+
+		mtouch->name = "Xen Virtual Multi-touch";
+		mtouch->phys = info->phys;
+		mtouch->id.bustype = BUS_PCI;
+		mtouch->id.vendor = 0x5853;
+		mtouch->id.product = 0xfffd;
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);
+
+		input_set_abs_params(mtouch, ABS_MT_TOUCH_MAJOR,
+				     0, 255, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_POSITION_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_POSITION_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_PRESSURE,
+				     0, 255, 0, 0);
+
+		ret = input_mt_init_slots(mtouch, num_cont, INPUT_MT_DIRECT);
+		if (ret) {
+			input_free_device(mtouch);
+			xenbus_dev_fatal(info->xbdev, ret,
+					 "input_mt_init_slots");
+			goto error;
+		}
+
+		ret = input_register_device(mtouch);
+		if (ret) {
+			input_free_device(mtouch);
+			xenbus_dev_fatal(info->xbdev, ret,
+					 "input_register_device(mtouch)");
+			goto error;
+		}
+		info->mtouch_cur_contact_id = -1;
+		info->mtouch = mtouch;
+	}
+
 	ret = xenkbd_connect_backend(dev, info);
 	if (ret < 0)
 		goto error;

@@ -240,6 +369,8 @@ static int xenkbd_remove(struct xenbus_device *dev)
 		input_unregister_device(info->kbd);
 	if (info->ptr)
 		input_unregister_device(info->ptr);
+	if (info->mtouch)
+		input_unregister_device(info->mtouch);
 	free_page((unsigned long)info->page);
 	kfree(info);
 	return 0;
Signature · 1 line
2.7.4
author
#1 lore raw

ping

Quoted 206 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>

Extend xen_kbdfront to provide multi-touch support to unprivileged domains.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
Changes since initial:
- use input_set_capability instead of setting flags directly
- input_mt_init_slots: let userspace better chance of figuring how to handle the device: use INPUT_MT_DIRECT, drop INPUT_MT_DROP_UNUSED
- add error handling for input_mt_init_slots
- remove module paramters
- remove odd unlikely

---
  drivers/input/misc/xen-kbdfront.c | 135 +++++++++++++++++++++++++++++++++++++-
  1 file changed, 133 insertions(+), 2 deletions(-)
drivers/input/misc/xen-kbdfront.c +131 -2
index eb770613a9bd..9fa005038773 100644
@@ -17,6 +17,7 @@
  #include <linux/errno.h>
  #include <linux/module.h>
  #include <linux/input.h>
+#include <linux/input/mt.h>
  #include <linux/slab.h>
 
  #include <asm/xen/hypervisor.h>

@@ -34,11 +35,14 @@
  struct xenkbd_info {
  	struct input_dev *kbd;
  	struct input_dev *ptr;
+	struct input_dev *mtouch;
  	struct xenkbd_page *page;
  	int gref;
  	int irq;
  	struct xenbus_device *xbdev;
  	char phys[32];
+	/* current MT slot/contact ID we are injecting events in */
+	int mtouch_cur_contact_id;
  };
 
  enum { KPARAM_X, KPARAM_Y, KPARAM_CNT };

@@ -100,6 +104,60 @@ static irqreturn_t input_handler(int rq, void *dev_id)
  				input_report_rel(dev, REL_WHEEL,
  						 -event->pos.rel_z);
  			break;
+		case XENKBD_TYPE_MTOUCH:
+			dev = info->mtouch;
+			if (unlikely(!dev))
+				break;
+			if (event->mtouch.contact_id !=
+					info->mtouch_cur_contact_id) {
+				info->mtouch_cur_contact_id =
+					event->mtouch.contact_id;
+				input_mt_slot(dev, event->mtouch.contact_id);
+			}
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);
+				break;
+			case XENKBD_MT_EV_UP:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   false);
+				break;
+			case XENKBD_MT_EV_MOTION:
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);
+				break;
+			case XENKBD_MT_EV_SYN:
+				input_mt_sync_frame(dev);
+				break;
+			case XENKBD_MT_EV_SHAPE:
+				input_event(dev, EV_ABS, ABS_MT_TOUCH_MAJOR,
+					    event->mtouch.u.shape.major);
+				input_event(dev, EV_ABS, ABS_MT_TOUCH_MINOR,
+					    event->mtouch.u.shape.minor);
+				break;
+			case XENKBD_MT_EV_ORIENT:
+				input_event(dev, EV_ABS, ABS_MT_ORIENTATION,
+					    event->mtouch.u.orientation);
+				break;
+			}
+			/* only report syn when requested */
+			if (event->mtouch.event_type != XENKBD_MT_EV_SYN)
+				dev = NULL;
  		}
  		if (dev)
  			input_sync(dev);

@@ -115,9 +173,9 @@ static int xenkbd_probe(struct xenbus_device *dev,
  				  const struct xenbus_device_id *id)
  {
  	int ret, i;
-	unsigned int abs;
+	unsigned int abs, touch;
  	struct xenkbd_info *info;
-	struct input_dev *kbd, *ptr;
+	struct input_dev *kbd, *ptr, *mtouch;
 
  	info = kzalloc(sizeof(*info), GFP_KERNEL);
  	if (!info) {

@@ -152,6 +210,17 @@ static int xenkbd_probe(struct xenbus_device *dev,
  		}
  	}
 
+	touch = xenbus_read_unsigned(dev->nodename,
+				     XENKBD_FIELD_FEAT_MTOUCH, 0);
+	if (touch) {
+		ret = xenbus_write(XBT_NIL, dev->nodename,
+				   XENKBD_FIELD_REQ_MTOUCH, "1");
+		if (ret) {
+			pr_warning("xenkbd: can't request multi-touch");
+			touch = 0;
+		}
+	}
+
  	/* keyboard */
  	kbd = input_allocate_device();
  	if (!kbd)

@@ -208,6 +277,66 @@ static int xenkbd_probe(struct xenbus_device *dev,
  	}
  	info->ptr = ptr;
 
+	/* multi-touch device */
+	if (touch) {
+		int num_cont, width, height;
+
+		mtouch = input_allocate_device();
+		if (!mtouch)
+			goto error_nomem;
+
+		num_cont = xenbus_read_unsigned(info->xbdev->nodename,
+						XENKBD_FIELD_MT_NUM_CONTACTS,
+						1);
+		width = xenbus_read_unsigned(info->xbdev->nodename,
+					     XENKBD_FIELD_MT_WIDTH,
+					     XENFB_WIDTH);
+		height = xenbus_read_unsigned(info->xbdev->nodename,
+					      XENKBD_FIELD_MT_HEIGHT,
+					      XENFB_HEIGHT);
+
+		mtouch->name = "Xen Virtual Multi-touch";
+		mtouch->phys = info->phys;
+		mtouch->id.bustype = BUS_PCI;
+		mtouch->id.vendor = 0x5853;
+		mtouch->id.product = 0xfffd;
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);
+
+		input_set_abs_params(mtouch, ABS_MT_TOUCH_MAJOR,
+				     0, 255, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_POSITION_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_POSITION_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_PRESSURE,
+				     0, 255, 0, 0);
+
+		ret = input_mt_init_slots(mtouch, num_cont, INPUT_MT_DIRECT);
+		if (ret) {
+			input_free_device(mtouch);
+			xenbus_dev_fatal(info->xbdev, ret,
+					 "input_mt_init_slots");
+			goto error;
+		}
+
+		ret = input_register_device(mtouch);
+		if (ret) {
+			input_free_device(mtouch);
+			xenbus_dev_fatal(info->xbdev, ret,
+					 "input_register_device(mtouch)");
+			goto error;
+		}
+		info->mtouch_cur_contact_id = -1;
+		info->mtouch = mtouch;
+	}
+
  	ret = xenkbd_connect_backend(dev, info);
  	if (ret < 0)
  		goto error;

drivers/input/misc/xen-kbdfront.c static int xenkbd_remove(struct xenbus_device *dev) · line 376

Quoted 8 lines
drivers/input/misc/xen-kbdfront.c +2 -0
@@ -240,6 +369,8 @@ static int xenkbd_remove(struct xenbus_device *dev)
  		input_unregister_device(info->kbd);
  	if (info->ptr)
  		input_unregister_device(info->ptr);
+	if (info->mtouch)
+		input_unregister_device(info->mtouch);
  	free_page((unsigned long)info->page);
  	kfree(info);
  	return 0;

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

author
#2 lore raw
In reply to Oleksandr Andrushchenko

ping

Quoted 214 lines · Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <[email protected]>

Extend xen_kbdfront to provide multi-touch support to unprivileged domains.

Signed-off-by Oleksandr Andrushchenko <[email protected]>

---
Changes since initial:
- use input_set_capability instead of setting flags directly
- input_mt_init_slots: let userspace better chance of figuring how to handle the device: use INPUT_MT_DIRECT, drop INPUT_MT_DROP_UNUSED
- add error handling for input_mt_init_slots
- remove module paramters
- remove odd unlikely

---
  drivers/input/misc/xen-kbdfront.c | 135 +++++++++++++++++++++++++++++++++++++-
  1 file changed, 133 insertions(+), 2 deletions(-)
drivers/input/misc/xen-kbdfront.c +133 -2
index eb770613a9bd..9fa005038773 100644
@@ -17,6 +17,7 @@
  #include <linux/errno.h>
  #include <linux/module.h>
  #include <linux/input.h>
+#include <linux/input/mt.h>
  #include <linux/slab.h>
 
  #include <asm/xen/hypervisor.h>

@@ -34,11 +35,14 @@
  struct xenkbd_info {
  	struct input_dev *kbd;
  	struct input_dev *ptr;
+	struct input_dev *mtouch;
  	struct xenkbd_page *page;
  	int gref;
  	int irq;
  	struct xenbus_device *xbdev;
  	char phys[32];
+	/* current MT slot/contact ID we are injecting events in */
+	int mtouch_cur_contact_id;
  };
 
  enum { KPARAM_X, KPARAM_Y, KPARAM_CNT };

@@ -100,6 +104,60 @@ static irqreturn_t input_handler(int rq, void *dev_id)
  				input_report_rel(dev, REL_WHEEL,
  						 -event->pos.rel_z);
  			break;
+		case XENKBD_TYPE_MTOUCH:
+			dev = info->mtouch;
+			if (unlikely(!dev))
+				break;
+			if (event->mtouch.contact_id !=
+					info->mtouch_cur_contact_id) {
+				info->mtouch_cur_contact_id =
+					event->mtouch.contact_id;
+				input_mt_slot(dev, event->mtouch.contact_id);
+			}
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);
+				break;
+			case XENKBD_MT_EV_UP:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   false);
+				break;
+			case XENKBD_MT_EV_MOTION:
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);
+				break;
+			case XENKBD_MT_EV_SYN:
+				input_mt_sync_frame(dev);
+				break;
+			case XENKBD_MT_EV_SHAPE:
+				input_event(dev, EV_ABS, ABS_MT_TOUCH_MAJOR,
+					    event->mtouch.u.shape.major);
+				input_event(dev, EV_ABS, ABS_MT_TOUCH_MINOR,
+					    event->mtouch.u.shape.minor);
+				break;
+			case XENKBD_MT_EV_ORIENT:
+				input_event(dev, EV_ABS, ABS_MT_ORIENTATION,
+					    event->mtouch.u.orientation);
+				break;
+			}
+			/* only report syn when requested */
+			if (event->mtouch.event_type != XENKBD_MT_EV_SYN)
+				dev = NULL;
  		}
  		if (dev)
  			input_sync(dev);

@@ -115,9 +173,9 @@ static int xenkbd_probe(struct xenbus_device *dev,
  				  const struct xenbus_device_id *id)
  {
  	int ret, i;
-	unsigned int abs;
+	unsigned int abs, touch;
  	struct xenkbd_info *info;
-	struct input_dev *kbd, *ptr;
+	struct input_dev *kbd, *ptr, *mtouch;
 
  	info = kzalloc(sizeof(*info), GFP_KERNEL);
  	if (!info) {

@@ -152,6 +210,17 @@ static int xenkbd_probe(struct xenbus_device *dev,
  		}
  	}
 
+	touch = xenbus_read_unsigned(dev->nodename,
+				     XENKBD_FIELD_FEAT_MTOUCH, 0);
+	if (touch) {
+		ret = xenbus_write(XBT_NIL, dev->nodename,
+				   XENKBD_FIELD_REQ_MTOUCH, "1");
+		if (ret) {
+			pr_warning("xenkbd: can't request multi-touch");
+			touch = 0;
+		}
+	}
+
  	/* keyboard */
  	kbd = input_allocate_device();
  	if (!kbd)

@@ -208,6 +277,66 @@ static int xenkbd_probe(struct xenbus_device *dev,
  	}
  	info->ptr = ptr;
 
+	/* multi-touch device */
+	if (touch) {
+		int num_cont, width, height;
+
+		mtouch = input_allocate_device();
+		if (!mtouch)
+			goto error_nomem;
+
+		num_cont = xenbus_read_unsigned(info->xbdev->nodename,
+						XENKBD_FIELD_MT_NUM_CONTACTS,
+						1);
+		width = xenbus_read_unsigned(info->xbdev->nodename,
+					     XENKBD_FIELD_MT_WIDTH,
+					     XENFB_WIDTH);
+		height = xenbus_read_unsigned(info->xbdev->nodename,
+					      XENKBD_FIELD_MT_HEIGHT,
+					      XENFB_HEIGHT);
+
+		mtouch->name = "Xen Virtual Multi-touch";
+		mtouch->phys = info->phys;
+		mtouch->id.bustype = BUS_PCI;
+		mtouch->id.vendor = 0x5853;
+		mtouch->id.product = 0xfffd;
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);
+
+		input_set_abs_params(mtouch, ABS_MT_TOUCH_MAJOR,
+				     0, 255, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_POSITION_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_POSITION_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_MT_PRESSURE,
+				     0, 255, 0, 0);
+
+		ret = input_mt_init_slots(mtouch, num_cont, INPUT_MT_DIRECT);
+		if (ret) {
+			input_free_device(mtouch);
+			xenbus_dev_fatal(info->xbdev, ret,
+					 "input_mt_init_slots");
+			goto error;
+		}
+
+		ret = input_register_device(mtouch);
+		if (ret) {
+			input_free_device(mtouch);
+			xenbus_dev_fatal(info->xbdev, ret,
+					 "input_register_device(mtouch)");
+			goto error;
+		}
+		info->mtouch_cur_contact_id = -1;
+		info->mtouch = mtouch;
+	}
+
  	ret = xenkbd_connect_backend(dev, info);
  	if (ret < 0)
  		goto error;

@@ -240,6 +369,8 @@ static int xenkbd_remove(struct xenbus_device *dev)
  		input_unregister_device(info->kbd);
  	if (info->ptr)
  		input_unregister_device(info->ptr);
+	if (info->mtouch)
+		input_unregister_device(info->mtouch);
  	free_page((unsigned long)info->page);
  	kfree(info);
  	return 0;
maintainer
#3 lore raw
In reply to Oleksandr Andrushchenko

Hi Oleksandr,

Review

Quoted 12 lines · Oleksandr Andrushchenko
(quoted diff) +12 -0
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);

I was looking at this and realized that this breaks the single touch emulation for MT interface: for ST you are supposed to report the oldest contact, here you report data for all of them. Luckily input_mt_report_pointer_emulation() that is called as part of input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes that for you.

We should simply remove reporting ABS_X/ABS_Y here and in XENKBD_MT_EV_MOTION as well.

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);

This is done automatically by input_mt_init_slots() when called with INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be removed as well.

Does the patch below (on top of yours) work for you?

Thanks.

Signature · 11 lines
Dmitry


Input: xen-kbdfront - MT support fixups

From: Dmitry Torokhov <[email protected]>

Signed-off-by: Dmitry Torokhov <[email protected]>
---
 drivers/input/misc/xen-kbdfront.c |  210 +++++++++++++++++++------------------
 1 file changed, 109 insertions(+), 101 deletions(-)
drivers/input/misc/xen-kbdfront.c +109 -101
index 9fa005038773..fa130e7b734c 100644
@@ -60,6 +60,112 @@ static void xenkbd_disconnect_backend(struct xenkbd_info *);
  * to do that.
  */
 
+static void xenkbd_handle_motion_event(struct xenkbd_info *info,
+				       struct xenkbd_motion *motion)
+{
+	input_report_rel(info->ptr, REL_X, motion->rel_x);
+	input_report_rel(info->ptr, REL_Y, motion->rel_y);
+	if (motion->rel_z)
+		input_report_rel(info->ptr, REL_WHEEL, -motion->rel_z);
+	input_sync(info->ptr);
+}
+
+static void xenkbd_handle_position_event(struct xenkbd_info *info,
+					 struct xenkbd_position *pos)
+{
+	input_report_abs(info->ptr, ABS_X, pos->abs_x);
+	input_report_abs(info->ptr, ABS_Y, pos->abs_y);
+	if (pos->rel_z)
+		input_report_rel(info->ptr, REL_WHEEL, -pos->rel_z);
+	input_sync(info->ptr);
+}
+
+static void xenkbd_handle_key_event(struct xenkbd_info *info,
+				    struct xenkbd_key *key)
+{
+	struct input_dev *dev;
+
+	if (test_bit(key->keycode, info->ptr->keybit)) {
+		dev = info->ptr;
+	} else if (test_bit(key->keycode, info->kbd->keybit)) {
+		dev = info->kbd;
+	} else {
+		pr_warn("unhandled keycode 0x%x\n", key->keycode);
+		return;
+	}
+
+	input_report_key(dev, key->keycode, key->pressed);
+	input_sync(dev);
+}
+
+static void xenkbd_handle_mt_event(struct xenkbd_info *info,
+				   struct xenkbd_mtouch *mtouch)
+{
+	if (unlikely(!info->mtouch))
+		return;
+
+	if (mtouch->contact_id != info->mtouch_cur_contact_id) {
+		info->mtouch_cur_contact_id = mtouch->contact_id;
+		input_mt_slot(info->mtouch, mtouch->contact_id);
+	}
+
+	switch (mtouch->event_type) {
+	case XENKBD_MT_EV_DOWN:
+		input_mt_report_slot_state(info->mtouch, MT_TOOL_FINGER, true);
+		/* fall through */
+
+	case XENKBD_MT_EV_MOTION:
+		input_report_abs(info->mtouch, ABS_MT_POSITION_X,
+				 mtouch->u.pos.abs_x);
+		input_report_abs(info->mtouch, ABS_MT_POSITION_Y,
+				 mtouch->u.pos.abs_y);
+		break;
+
+	case XENKBD_MT_EV_SHAPE:
+		input_report_abs(info->mtouch, ABS_MT_TOUCH_MAJOR,
+				 mtouch->u.shape.major);
+		input_report_abs(info->mtouch, ABS_MT_TOUCH_MINOR,
+				 mtouch->u.shape.minor);
+		break;
+
+	case XENKBD_MT_EV_ORIENT:
+		input_report_abs(info->mtouch, ABS_MT_ORIENTATION,
+				 mtouch->u.orientation);
+		break;
+
+	case XENKBD_MT_EV_UP:
+		input_mt_report_slot_state(info->mtouch, MT_TOOL_FINGER, false);
+		break;
+
+	case XENKBD_MT_EV_SYN:
+		input_mt_sync_frame(info->mtouch);
+		input_sync(info->mtouch);
+		break;
+	}
+}
+
+static void xenkbd_handle_event(struct xenkbd_info *info,
+				union xenkbd_in_event *event)
+{
+	switch (event->type) {
+	case XENKBD_TYPE_MOTION:
+		xenkbd_handle_motion_event(info, &event->motion);
+		break;
+
+	case XENKBD_TYPE_KEY:
+		xenkbd_handle_key_event(info, &event->key);
+		break;
+
+	case XENKBD_TYPE_POS:
+		xenkbd_handle_position_event(info, &event->pos);
+		break;
+
+	case XENKBD_TYPE_MTOUCH:
+		xenkbd_handle_mt_event(info, &event->mtouch);
+		break;
+	}
+}
+
 static irqreturn_t input_handler(int rq, void *dev_id)
 {
 	struct xenkbd_info *info = dev_id;

@@ -70,98 +176,8 @@ static irqreturn_t input_handler(int rq, void *dev_id)
 	if (prod == page->in_cons)
 		return IRQ_HANDLED;
 	rmb();			/* ensure we see ring contents up to prod */
-	for (cons = page->in_cons; cons != prod; cons++) {
-		union xenkbd_in_event *event;
-		struct input_dev *dev;
-		event = &XENKBD_IN_RING_REF(page, cons);
-
-		dev = info->ptr;
-		switch (event->type) {
-		case XENKBD_TYPE_MOTION:
-			input_report_rel(dev, REL_X, event->motion.rel_x);
-			input_report_rel(dev, REL_Y, event->motion.rel_y);
-			if (event->motion.rel_z)
-				input_report_rel(dev, REL_WHEEL,
-						 -event->motion.rel_z);
-			break;
-		case XENKBD_TYPE_KEY:
-			dev = NULL;
-			if (test_bit(event->key.keycode, info->kbd->keybit))
-				dev = info->kbd;
-			if (test_bit(event->key.keycode, info->ptr->keybit))
-				dev = info->ptr;
-			if (dev)
-				input_report_key(dev, event->key.keycode,
-						 event->key.pressed);
-			else
-				pr_warn("unhandled keycode 0x%x\n",
-					event->key.keycode);
-			break;
-		case XENKBD_TYPE_POS:
-			input_report_abs(dev, ABS_X, event->pos.abs_x);
-			input_report_abs(dev, ABS_Y, event->pos.abs_y);
-			if (event->pos.rel_z)
-				input_report_rel(dev, REL_WHEEL,
-						 -event->pos.rel_z);
-			break;
-		case XENKBD_TYPE_MTOUCH:
-			dev = info->mtouch;
-			if (unlikely(!dev))
-				break;
-			if (event->mtouch.contact_id !=
-					info->mtouch_cur_contact_id) {
-				info->mtouch_cur_contact_id =
-					event->mtouch.contact_id;
-				input_mt_slot(dev, event->mtouch.contact_id);
-			}
-			switch (event->mtouch.event_type) {
-			case XENKBD_MT_EV_DOWN:
-				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
-							   true);
-				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
-					    event->mtouch.u.pos.abs_x);
-				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
-					    event->mtouch.u.pos.abs_y);
-				input_event(dev, EV_ABS, ABS_X,
-					    event->mtouch.u.pos.abs_x);
-				input_event(dev, EV_ABS, ABS_Y,
-					    event->mtouch.u.pos.abs_y);
-				break;
-			case XENKBD_MT_EV_UP:
-				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
-							   false);
-				break;
-			case XENKBD_MT_EV_MOTION:
-				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
-					    event->mtouch.u.pos.abs_x);
-				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
-					    event->mtouch.u.pos.abs_y);
-				input_event(dev, EV_ABS, ABS_X,
-					    event->mtouch.u.pos.abs_x);
-				input_event(dev, EV_ABS, ABS_Y,
-					    event->mtouch.u.pos.abs_y);
-				break;
-			case XENKBD_MT_EV_SYN:
-				input_mt_sync_frame(dev);
-				break;
-			case XENKBD_MT_EV_SHAPE:
-				input_event(dev, EV_ABS, ABS_MT_TOUCH_MAJOR,
-					    event->mtouch.u.shape.major);
-				input_event(dev, EV_ABS, ABS_MT_TOUCH_MINOR,
-					    event->mtouch.u.shape.minor);
-				break;
-			case XENKBD_MT_EV_ORIENT:
-				input_event(dev, EV_ABS, ABS_MT_ORIENTATION,
-					    event->mtouch.u.orientation);
-				break;
-			}
-			/* only report syn when requested */
-			if (event->mtouch.event_type != XENKBD_MT_EV_SYN)
-				dev = NULL;
-		}
-		if (dev)
-			input_sync(dev);
-	}
+	for (cons = page->in_cons; cons != prod; cons++)
+		xenkbd_handle_event(info, &XENKBD_IN_RING_REF(page, cons));
 	mb();			/* ensure we got ring contents */
 	page->in_cons = cons;
 	notify_remote_via_irq(info->irq);

@@ -216,7 +232,7 @@ static int xenkbd_probe(struct xenbus_device *dev,
 		ret = xenbus_write(XBT_NIL, dev->nodename,
 				   XENKBD_FIELD_REQ_MTOUCH, "1");
 		if (ret) {
-			pr_warning("xenkbd: can't request multi-touch");
+			pr_warn("xenkbd: can't request multi-touch");
 			touch = 0;
 		}
 	}

@@ -301,14 +317,6 @@ static int xenkbd_probe(struct xenbus_device *dev,
 		mtouch->id.vendor = 0x5853;
 		mtouch->id.product = 0xfffd;
 
-		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
-		input_set_abs_params(mtouch, ABS_X,
-				     0, width, 0, 0);
-		input_set_abs_params(mtouch, ABS_Y,
-				     0, height, 0, 0);
-		input_set_abs_params(mtouch, ABS_PRESSURE,
-				     0, 255, 0, 0);
-
 		input_set_abs_params(mtouch, ABS_MT_TOUCH_MAJOR,
 				     0, 255, 0, 0);
 		input_set_abs_params(mtouch, ABS_MT_POSITION_X,

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

maintainer
#4 lore raw
In reply to Oleksandr Andrushchenko

Hi Oleksandr,

Review

Quoted 12 lines · Oleksandr Andrushchenko
(quoted diff) +12 -0
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);

I was looking at this and realized that this breaks the single touch emulation for MT interface: for ST you are supposed to report the oldest contact, here you report data for all of them. Luckily input_mt_report_pointer_emulation() that is called as part of input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes that for you.

We should simply remove reporting ABS_X/ABS_Y here and in XENKBD_MT_EV_MOTION as well.

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);

This is done automatically by input_mt_init_slots() when called with INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be removed as well.

Does the patch below (on top of yours) work for you?

Thanks.

Signature · 11 lines
Dmitry


Input: xen-kbdfront - MT support fixups

From: Dmitry Torokhov <[email protected]>

Signed-off-by: Dmitry Torokhov <[email protected]>
---
 drivers/input/misc/xen-kbdfront.c |  210 +++++++++++++++++++------------------
 1 file changed, 109 insertions(+), 101 deletions(-)
drivers/input/misc/xen-kbdfront.c +109 -101
index 9fa005038773..fa130e7b734c 100644
@@ -60,6 +60,112 @@ static void xenkbd_disconnect_backend(struct xenkbd_info *);
  * to do that.
  */
 
+static void xenkbd_handle_motion_event(struct xenkbd_info *info,
+				       struct xenkbd_motion *motion)
+{
+	input_report_rel(info->ptr, REL_X, motion->rel_x);
+	input_report_rel(info->ptr, REL_Y, motion->rel_y);
+	if (motion->rel_z)
+		input_report_rel(info->ptr, REL_WHEEL, -motion->rel_z);
+	input_sync(info->ptr);
+}
+
+static void xenkbd_handle_position_event(struct xenkbd_info *info,
+					 struct xenkbd_position *pos)
+{
+	input_report_abs(info->ptr, ABS_X, pos->abs_x);
+	input_report_abs(info->ptr, ABS_Y, pos->abs_y);
+	if (pos->rel_z)
+		input_report_rel(info->ptr, REL_WHEEL, -pos->rel_z);
+	input_sync(info->ptr);
+}
+
+static void xenkbd_handle_key_event(struct xenkbd_info *info,
+				    struct xenkbd_key *key)
+{
+	struct input_dev *dev;
+
+	if (test_bit(key->keycode, info->ptr->keybit)) {
+		dev = info->ptr;
+	} else if (test_bit(key->keycode, info->kbd->keybit)) {
+		dev = info->kbd;
+	} else {
+		pr_warn("unhandled keycode 0x%x\n", key->keycode);
+		return;
+	}
+
+	input_report_key(dev, key->keycode, key->pressed);
+	input_sync(dev);
+}
+
+static void xenkbd_handle_mt_event(struct xenkbd_info *info,
+				   struct xenkbd_mtouch *mtouch)
+{
+	if (unlikely(!info->mtouch))
+		return;
+
+	if (mtouch->contact_id != info->mtouch_cur_contact_id) {
+		info->mtouch_cur_contact_id = mtouch->contact_id;
+		input_mt_slot(info->mtouch, mtouch->contact_id);
+	}
+
+	switch (mtouch->event_type) {
+	case XENKBD_MT_EV_DOWN:
+		input_mt_report_slot_state(info->mtouch, MT_TOOL_FINGER, true);
+		/* fall through */
+
+	case XENKBD_MT_EV_MOTION:
+		input_report_abs(info->mtouch, ABS_MT_POSITION_X,
+				 mtouch->u.pos.abs_x);
+		input_report_abs(info->mtouch, ABS_MT_POSITION_Y,
+				 mtouch->u.pos.abs_y);
+		break;
+
+	case XENKBD_MT_EV_SHAPE:
+		input_report_abs(info->mtouch, ABS_MT_TOUCH_MAJOR,
+				 mtouch->u.shape.major);
+		input_report_abs(info->mtouch, ABS_MT_TOUCH_MINOR,
+				 mtouch->u.shape.minor);
+		break;
+
+	case XENKBD_MT_EV_ORIENT:
+		input_report_abs(info->mtouch, ABS_MT_ORIENTATION,
+				 mtouch->u.orientation);
+		break;
+
+	case XENKBD_MT_EV_UP:
+		input_mt_report_slot_state(info->mtouch, MT_TOOL_FINGER, false);
+		break;
+
+	case XENKBD_MT_EV_SYN:
+		input_mt_sync_frame(info->mtouch);
+		input_sync(info->mtouch);
+		break;
+	}
+}
+
+static void xenkbd_handle_event(struct xenkbd_info *info,
+				union xenkbd_in_event *event)
+{
+	switch (event->type) {
+	case XENKBD_TYPE_MOTION:
+		xenkbd_handle_motion_event(info, &event->motion);
+		break;
+
+	case XENKBD_TYPE_KEY:
+		xenkbd_handle_key_event(info, &event->key);
+		break;
+
+	case XENKBD_TYPE_POS:
+		xenkbd_handle_position_event(info, &event->pos);
+		break;
+
+	case XENKBD_TYPE_MTOUCH:
+		xenkbd_handle_mt_event(info, &event->mtouch);
+		break;
+	}
+}
+
 static irqreturn_t input_handler(int rq, void *dev_id)
 {
 	struct xenkbd_info *info = dev_id;

@@ -70,98 +176,8 @@ static irqreturn_t input_handler(int rq, void *dev_id)
 	if (prod == page->in_cons)
 		return IRQ_HANDLED;
 	rmb();			/* ensure we see ring contents up to prod */
-	for (cons = page->in_cons; cons != prod; cons++) {
-		union xenkbd_in_event *event;
-		struct input_dev *dev;
-		event = &XENKBD_IN_RING_REF(page, cons);
-
-		dev = info->ptr;
-		switch (event->type) {
-		case XENKBD_TYPE_MOTION:
-			input_report_rel(dev, REL_X, event->motion.rel_x);
-			input_report_rel(dev, REL_Y, event->motion.rel_y);
-			if (event->motion.rel_z)
-				input_report_rel(dev, REL_WHEEL,
-						 -event->motion.rel_z);
-			break;
-		case XENKBD_TYPE_KEY:
-			dev = NULL;
-			if (test_bit(event->key.keycode, info->kbd->keybit))
-				dev = info->kbd;
-			if (test_bit(event->key.keycode, info->ptr->keybit))
-				dev = info->ptr;
-			if (dev)
-				input_report_key(dev, event->key.keycode,
-						 event->key.pressed);
-			else
-				pr_warn("unhandled keycode 0x%x\n",
-					event->key.keycode);
-			break;
-		case XENKBD_TYPE_POS:
-			input_report_abs(dev, ABS_X, event->pos.abs_x);
-			input_report_abs(dev, ABS_Y, event->pos.abs_y);
-			if (event->pos.rel_z)
-				input_report_rel(dev, REL_WHEEL,
-						 -event->pos.rel_z);
-			break;
-		case XENKBD_TYPE_MTOUCH:
-			dev = info->mtouch;
-			if (unlikely(!dev))
-				break;
-			if (event->mtouch.contact_id !=
-					info->mtouch_cur_contact_id) {
-				info->mtouch_cur_contact_id =
-					event->mtouch.contact_id;
-				input_mt_slot(dev, event->mtouch.contact_id);
-			}
-			switch (event->mtouch.event_type) {
-			case XENKBD_MT_EV_DOWN:
-				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
-							   true);
-				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
-					    event->mtouch.u.pos.abs_x);
-				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
-					    event->mtouch.u.pos.abs_y);
-				input_event(dev, EV_ABS, ABS_X,
-					    event->mtouch.u.pos.abs_x);
-				input_event(dev, EV_ABS, ABS_Y,
-					    event->mtouch.u.pos.abs_y);
-				break;
-			case XENKBD_MT_EV_UP:
-				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
-							   false);
-				break;
-			case XENKBD_MT_EV_MOTION:
-				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
-					    event->mtouch.u.pos.abs_x);
-				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
-					    event->mtouch.u.pos.abs_y);
-				input_event(dev, EV_ABS, ABS_X,
-					    event->mtouch.u.pos.abs_x);
-				input_event(dev, EV_ABS, ABS_Y,
-					    event->mtouch.u.pos.abs_y);
-				break;
-			case XENKBD_MT_EV_SYN:
-				input_mt_sync_frame(dev);
-				break;
-			case XENKBD_MT_EV_SHAPE:
-				input_event(dev, EV_ABS, ABS_MT_TOUCH_MAJOR,
-					    event->mtouch.u.shape.major);
-				input_event(dev, EV_ABS, ABS_MT_TOUCH_MINOR,
-					    event->mtouch.u.shape.minor);
-				break;
-			case XENKBD_MT_EV_ORIENT:
-				input_event(dev, EV_ABS, ABS_MT_ORIENTATION,
-					    event->mtouch.u.orientation);
-				break;
-			}
-			/* only report syn when requested */
-			if (event->mtouch.event_type != XENKBD_MT_EV_SYN)
-				dev = NULL;
-		}
-		if (dev)
-			input_sync(dev);
-	}
+	for (cons = page->in_cons; cons != prod; cons++)
+		xenkbd_handle_event(info, &XENKBD_IN_RING_REF(page, cons));
 	mb();			/* ensure we got ring contents */
 	page->in_cons = cons;
 	notify_remote_via_irq(info->irq);

@@ -216,7 +232,7 @@ static int xenkbd_probe(struct xenbus_device *dev,
 		ret = xenbus_write(XBT_NIL, dev->nodename,
 				   XENKBD_FIELD_REQ_MTOUCH, "1");
 		if (ret) {
-			pr_warning("xenkbd: can't request multi-touch");
+			pr_warn("xenkbd: can't request multi-touch");
 			touch = 0;
 		}
 	}

@@ -301,14 +317,6 @@ static int xenkbd_probe(struct xenbus_device *dev,
 		mtouch->id.vendor = 0x5853;
 		mtouch->id.product = 0xfffd;
 
-		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
-		input_set_abs_params(mtouch, ABS_X,
-				     0, width, 0, 0);
-		input_set_abs_params(mtouch, ABS_Y,
-				     0, height, 0, 0);
-		input_set_abs_params(mtouch, ABS_PRESSURE,
-				     0, 255, 0, 0);
-
 		input_set_abs_params(mtouch, ABS_MT_TOUCH_MAJOR,
 				     0, 255, 0, 0);
 		input_set_abs_params(mtouch, ABS_MT_POSITION_X,
author
#5 lore raw

Hi, Dmitry!

First of all thank you for both the comments and the patch

Quoted 25 lines · Dmitry Torokhov

Hi Oleksandr,

Review

Quoted 12 lines · Oleksandr Andrushchenko
(quoted diff) +12 -0
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);

I was looking at this and realized that this breaks the single touch emulation for MT interface: for ST you are supposed to report the oldest contact, here you report data for all of them. Luckily input_mt_report_pointer_emulation() that is called as part of input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes that for you.

We should simply remove reporting ABS_X/ABS_Y here and in XENKBD_MT_EV_MOTION as well.

Quoted 11 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);

This is done automatically by input_mt_init_slots() when called with INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be removed as well.

Great, I was not actually convinced that ABS is really needed to be put here while dealing with MT devices,
so the above can be removed

Quoted 1 line

Does the patch below (on top of yours) work for you?

Unfortunately I didn't have time to test the patch today, but will try to do so tomorrow.

Beside that, do you think that the removals above should go into my patch and the rest of yours (it looks like needed refactoring to me) should go into a separate one, not named "MT support fixups", but rather "Xen input driver refactoring"? Because part of the changes seems to be MT relevant and part is pure refactoring.
If so, do you want me to rework your patch with these changes and add on top of mine (I will put your signed off) or you will handle it on your own?

Quoted 2 lines

Thanks.

Thank you,
Oleksandr

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

author
#6 lore raw
In reply to Dmitry Torokhov

Hi, Dmitry!

First of all thank you for both the comments and the patch

Quoted 25 lines · Dmitry Torokhov

Hi Oleksandr,

Review

Quoted 12 lines · Oleksandr Andrushchenko
(quoted diff) +12 -0
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);

I was looking at this and realized that this breaks the single touch emulation for MT interface: for ST you are supposed to report the oldest contact, here you report data for all of them. Luckily input_mt_report_pointer_emulation() that is called as part of input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes that for you.

We should simply remove reporting ABS_X/ABS_Y here and in XENKBD_MT_EV_MOTION as well.

Quoted 11 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);

This is done automatically by input_mt_init_slots() when called with INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be removed as well.

Great, I was not actually convinced that ABS is really needed to be put here while dealing with MT devices,
so the above can be removed

Quoted 1 line

Does the patch below (on top of yours) work for you?

Unfortunately I didn't have time to test the patch today, but will try to do so tomorrow.

Beside that, do you think that the removals above should go into my patch and the rest of yours (it looks like needed refactoring to me) should go into a separate one, not named "MT support fixups", but rather "Xen input driver refactoring"? Because part of the changes seems to be MT relevant and part is pure refactoring.
If so, do you want me to rework your patch with these changes and add on top of mine (I will put your signed off) or you will handle it on your own?

Quoted 2 lines

Thanks.

Thank you,
Oleksandr

maintainer
#7 lore raw
Quoted 50 lines · Oleksandr Andrushchenko

Hi, Dmitry!

First of all thank you for both the comments and the patch

Quoted 3 lines · Dmitry Torokhov

Hi Oleksandr,

On Fri, Jun 23, 2017 at 09:09:55AM +0300, Oleksandr Andrushchenko

Quoted 6 lines · wrote:
Quoted 6 lines
(quoted diff) +6 -0
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);

I was looking at this and realized that this breaks the single touch emulation for MT interface: for ST you are supposed to report the

oldest

Quoted 8 lines

contact, here you report data for all of them. Luckily input_mt_report_pointer_emulation() that is called as part of input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes that for you.

We should simply remove reporting ABS_X/ABS_Y here and in XENKBD_MT_EV_MOTION as well.

Quoted 11 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);

This is done automatically by input_mt_init_slots() when called with INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be removed as well.

Great, I was not actually convinced that ABS is really needed to be put here while dealing with MT devices,
so the above can be removed

Quoted 1 line

Does the patch below (on top of yours) work for you?

Unfortunately I didn't have time to test the patch today, but will try to do so tomorrow.

Quoted 13 lines

Beside that, do you think that the removals above should go into my patch
and the rest of yours (it looks like needed refactoring to me) should go
into
a separate one, not named "MT support fixups", but rather "Xen input driver refactoring"? Because part of the changes seems to be MT relevant
and part is pure refactoring.
If so, do you want me to rework your patch with these changes and add on
top of mine (I will put your signed off) or you will handle it on your own?

I was planning on simply folding my changes into your patch and calling it a day, unless your testing would show there is an issue. It wasn't intended to be a separate patch in it's own right, I simply sent it out this way to show what exactly I was changing.

Thanks.

Signature · 6 lines
Dmitry

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
maintainer
#8 lore raw
In reply to Oleksandr Andrushchenko
Quoted 50 lines · Oleksandr Andrushchenko

Hi, Dmitry!

First of all thank you for both the comments and the patch

Quoted 3 lines · Dmitry Torokhov

Hi Oleksandr,

On Fri, Jun 23, 2017 at 09:09:55AM +0300, Oleksandr Andrushchenko

Quoted 6 lines · wrote:
Quoted 6 lines
(quoted diff) +6 -0
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);

I was looking at this and realized that this breaks the single touch emulation for MT interface: for ST you are supposed to report the

oldest

Quoted 8 lines

contact, here you report data for all of them. Luckily input_mt_report_pointer_emulation() that is called as part of input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes that for you.

We should simply remove reporting ABS_X/ABS_Y here and in XENKBD_MT_EV_MOTION as well.

Quoted 11 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);

This is done automatically by input_mt_init_slots() when called with INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be removed as well.

Great, I was not actually convinced that ABS is really needed to be put here while dealing with MT devices,
so the above can be removed

Quoted 1 line

Does the patch below (on top of yours) work for you?

Unfortunately I didn't have time to test the patch today, but will try to do so tomorrow.

Quoted 13 lines

Beside that, do you think that the removals above should go into my patch
and the rest of yours (it looks like needed refactoring to me) should go
into
a separate one, not named "MT support fixups", but rather "Xen input driver refactoring"? Because part of the changes seems to be MT relevant
and part is pure refactoring.
If so, do you want me to rework your patch with these changes and add on
top of mine (I will put your signed off) or you will handle it on your own?

I was planning on simply folding my changes into your patch and calling it a day, unless your testing would show there is an issue. It wasn't intended to be a separate patch in it's own right, I simply sent it out this way to show what exactly I was changing.

Thanks.

Signature · 1 line
Dmitry
author
#9 lore raw

Hi, Dmitry!

Quoted 64 lines · Dmitry Torokhov
Quoted 50 lines · Oleksandr Andrushchenko

Hi, Dmitry!

First of all thank you for both the comments and the patch

Quoted 3 lines · Dmitry Torokhov

Hi Oleksandr,

On Fri, Jun 23, 2017 at 09:09:55AM +0300, Oleksandr Andrushchenko

Quoted 6 lines · wrote:
Quoted 6 lines
(quoted diff) +6 -0
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);

I was looking at this and realized that this breaks the single touch emulation for MT interface: for ST you are supposed to report the

oldest

Quoted 8 lines

contact, here you report data for all of them. Luckily input_mt_report_pointer_emulation() that is called as part of input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes that for you.

We should simply remove reporting ABS_X/ABS_Y here and in XENKBD_MT_EV_MOTION as well.

Quoted 11 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);

This is done automatically by input_mt_init_slots() when called with INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be removed as well.

Great, I was not actually convinced that ABS is really needed to be put here while dealing with MT devices,
so the above can be removed

Quoted 1 line

Does the patch below (on top of yours) work for you?

Unfortunately I didn't have time to test the patch today, but will try to do so tomorrow.

Quoted 13 lines

Beside that, do you think that the removals above should go into my patch
and the rest of yours (it looks like needed refactoring to me) should go
into
a separate one, not named "MT support fixups", but rather "Xen input driver refactoring"? Because part of the changes seems to be MT relevant
and part is pure refactoring.
If so, do you want me to rework your patch with these changes and add on
top of mine (I will put your signed off) or you will handle it on your own?

Quoted 1 line

I was planning on simply folding my changes into your patch and calling it a day, unless your testing would show there is an issue.

I found no issue with the patches, but I have only tested that on ARM with our new kbd/ptr/mt backend [1]. I am not able to test that on x86 unfortunately, thus cannot confirm QEMU's backend is ok as well. What will be the next steps on my side to get MT in?

Quoted 5 lines

It wasn't intended to be a separate patch in it's own right, I simply sent it out this way to show what exactly I was changing.

Thanks.

Thank you,
Oleksandr

[1] https://github.com/xen-troops/displ_be

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

author
#10 lore raw
In reply to Dmitry Torokhov

Hi, Dmitry!

Quoted 64 lines · Dmitry Torokhov
Quoted 50 lines · Oleksandr Andrushchenko

Hi, Dmitry!

First of all thank you for both the comments and the patch

Quoted 3 lines · Dmitry Torokhov

Hi Oleksandr,

On Fri, Jun 23, 2017 at 09:09:55AM +0300, Oleksandr Andrushchenko

Quoted 6 lines · wrote:
Quoted 6 lines
(quoted diff) +6 -0
+			switch (event->mtouch.event_type) {
+			case XENKBD_MT_EV_DOWN:
+				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
+							   true);
+				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
+					    event->mtouch.u.pos.abs_x);
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
+					    event->mtouch.u.pos.abs_y);
+				input_event(dev, EV_ABS, ABS_X,
+					    event->mtouch.u.pos.abs_x);
+				input_event(dev, EV_ABS, ABS_Y,
+					    event->mtouch.u.pos.abs_y);

I was looking at this and realized that this breaks the single touch emulation for MT interface: for ST you are supposed to report the

oldest

Quoted 8 lines

contact, here you report data for all of them. Luckily input_mt_report_pointer_emulation() that is called as part of input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes that for you.

We should simply remove reporting ABS_X/ABS_Y here and in XENKBD_MT_EV_MOTION as well.

Quoted 11 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
+		input_set_abs_params(mtouch, ABS_X,
+				     0, width, 0, 0);
+		input_set_abs_params(mtouch, ABS_Y,
+				     0, height, 0, 0);
+		input_set_abs_params(mtouch, ABS_PRESSURE,
+				     0, 255, 0, 0);

This is done automatically by input_mt_init_slots() when called with INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be removed as well.

Great, I was not actually convinced that ABS is really needed to be put here while dealing with MT devices,
so the above can be removed

Quoted 1 line

Does the patch below (on top of yours) work for you?

Unfortunately I didn't have time to test the patch today, but will try to do so tomorrow.

Quoted 13 lines

Beside that, do you think that the removals above should go into my patch
and the rest of yours (it looks like needed refactoring to me) should go
into
a separate one, not named "MT support fixups", but rather "Xen input driver refactoring"? Because part of the changes seems to be MT relevant
and part is pure refactoring.
If so, do you want me to rework your patch with these changes and add on
top of mine (I will put your signed off) or you will handle it on your own?

Quoted 1 line

I was planning on simply folding my changes into your patch and calling it a day, unless your testing would show there is an issue.

I found no issue with the patches, but I have only tested that on ARM with our new kbd/ptr/mt backend [1]. I am not able to test that on x86 unfortunately, thus cannot confirm QEMU's backend is ok as well. What will be the next steps on my side to get MT in?

Quoted 5 lines

It wasn't intended to be a separate patch in it's own right, I simply sent it out this way to show what exactly I was changing.

Thanks.

Thank you,
Oleksandr

[1] https://github.com/xen-troops/displ_be