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

From: Oleksandr Andrushchenko
Date: Fri Jun 30 2017 - 03:41:22 EST


Hi, Dmitry!

On 06/29/2017 10:24 PM, Dmitry Torokhov wrote:
On June 29, 2017 11:40:30 AM PDT, Oleksandr Andrushchenko <andr2000@xxxxxxxxx> wrote:
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?
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?
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