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

From: Oleksandr Andrushchenko
Date: Thu Jun 29 2017 - 14:40:44 EST


Hi, Dmitry!

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

On 06/29/2017 11:17 AM, Dmitry Torokhov wrote:
Hi Oleksandr,

On Fri, Jun 23, 2017 at 09:09:55AM +0300, Oleksandr Andrushchenko wrote:
+ 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.

+
+ 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
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?
Thanks.

Thank you,
Oleksandr