From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752601AbdF2TYu (ORCPT ); Thu, 29 Jun 2017 15:24:50 -0400 Received: from mail-pg0-f65.google.com ([74.125.83.65]:33476 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751591AbdF2TYp (ORCPT ); Thu, 29 Jun 2017 15:24:45 -0400 Date: Thu, 29 Jun 2017 12:24:42 -0700 User-Agent: K-9 Mail for Android In-Reply-To: References: <1498198195-12293-1-git-send-email-andr2000@gmail.com> <20170629081719.GA21557@dtor-ws> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Subject: Re: [PATCH v1] xen/input: add multi-touch support To: Oleksandr Andrushchenko CC: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, joculator@gmail.com, al1img@gmail.com, vlad.babchuk@gmail.com, andrii.anisov@gmail.com, olekstysh@gmail.com, Oleksandr Andrushchenko From: Dmitry Torokhov Message-ID: <75F63113-9756-4A21-930B-CDE24871BCA3@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v5TJQRgK010526 On June 29, 2017 11:40:30 AM PDT, Oleksandr Andrushchenko 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. 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. -- Dmitry