Patchwork [linux-boundary] egalax_ts: Always report all touch points before input_sync()

login
register
mail settings
Submitter Erik Botö
Date July 3, 2013, 6:28 a.m.
Message ID <1372832922-13477-1-git-send-email-erik.boto@pelagicore.com>
Download mbox | patch
Permalink /patch/52865/
State Changes Requested
Delegated to: Otavio Salvador
Headers show

Comments

Erik Botö - July 3, 2013, 6:28 a.m.
---
 drivers/input/touchscreen/egalax_ts.c | 36 +++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)
Erik Botö - July 3, 2013, 7:20 a.m.
Hi,

Since the motivation for this patch was only discussed in a separate
mail thread I'll summarize here:

The previous behavior of the driver did not work properly with Qt5
QtQuick multi touch-point gestures, due to how touch-points are
reported when removing a touch-point. My interpretation of the
available documentation [1] was that the driver should report all
touch-points between SYN_REPORTs, but it is not explicitly stated so.
I've found another mail-thread [2] where the creator of the protocol
states:

"The protocol defines a generic way of sending a variable amount of
contacts. The
contact count is obtained by counting the number of non-empty finger packets
between SYN_REPORT events." - Henrik Rydberg

I think this verifies my assumption that all touch-points should be
reported between SYN_REPORTs, otherwise it can not be used to obtain
the count.

[1] - https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt
[2] - http://lists.x.org/archives/xorg-devel/2010-March/006466.html

Cheers,
Erik Botö

On Wed, Jul 3, 2013 at 8:28 AM, Erik Botö <erik.boto@pelagicore.com> wrote:
> ---
>  drivers/input/touchscreen/egalax_ts.c | 36 +++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
> index 0b6cde7..271f820 100644
> --- a/drivers/input/touchscreen/egalax_ts.c
> +++ b/drivers/input/touchscreen/egalax_ts.c
> @@ -133,7 +133,6 @@ retry:
>         }
>
>         if (down) {
> -               /* should also report old pointers */
>                 events[id].valid = valid;
>                 events[id].status = down;
>                 events[id].x = x;
> @@ -144,23 +143,6 @@ retry:
>                 input_report_abs(input_dev, ABS_Y, y);
>                 input_event(data->input_dev, EV_KEY, BTN_TOUCH, 1);
>                 input_report_abs(input_dev, ABS_PRESSURE, 1);
> -#else
> -               for (i = 0; i < MAX_SUPPORT_POINTS; i++) {
> -                       if (!events[i].valid)
> -                               continue;
> -                       dev_dbg(&client->dev, "report id:%d valid:%d x:%d y:%d",
> -                               i, valid, x, y);
> -
> -                       input_report_abs(input_dev,
> -                                        ABS_MT_TRACKING_ID, i);
> -                       input_report_abs(input_dev,
> -                                        ABS_MT_TOUCH_MAJOR, 1);
> -                       input_report_abs(input_dev,
> -                                        ABS_MT_POSITION_X, events[i].x);
> -                       input_report_abs(input_dev,
> -                                        ABS_MT_POSITION_Y, events[i].y);
> -                       input_mt_sync(input_dev);
> -               }
>  #endif
>         } else {
>                 dev_dbg(&client->dev, "release id:%d\n", id);
> @@ -176,6 +158,24 @@ retry:
>  #endif
>         }
>
> +#ifndef CONFIG_TOUCHSCREEN_EGALAX_SINGLE_TOUCH
> +       /* report all pointers */
> +       for (i = 0; i < MAX_SUPPORT_POINTS; i++) {
> +               if (!events[i].valid)
> +                       continue;
> +               dev_dbg(&client->dev, "report id:%d valid:%d x:%d y:%d",
> +                       i, valid, x, y);
> +                       input_report_abs(input_dev,
> +                                ABS_MT_TRACKING_ID, i);
> +               input_report_abs(input_dev,
> +                                ABS_MT_TOUCH_MAJOR, 1);
> +               input_report_abs(input_dev,
> +                                ABS_MT_POSITION_X, events[i].x);
> +               input_report_abs(input_dev,
> +                                ABS_MT_POSITION_Y, events[i].y);
> +               input_mt_sync(input_dev);
> +       }
> +#endif
>         input_sync(input_dev);
>         return IRQ_HANDLED;
>  }
> --
> 1.8.1.2
>
Otavio Salvador - July 3, 2013, 12:54 p.m.
On Wed, Jul 3, 2013 at 4:20 AM, Erik Botö <erik.boto@pelagicore.com> wrote:
> Hi,
>
> Since the motivation for this patch was only discussed in a separate
> mail thread I'll summarize here:
>
> The previous behavior of the driver did not work properly with Qt5
> QtQuick multi touch-point gestures, due to how touch-points are
> reported when removing a touch-point. My interpretation of the
> available documentation [1] was that the driver should report all
> touch-points between SYN_REPORTs, but it is not explicitly stated so.
> I've found another mail-thread [2] where the creator of the protocol
> states:
>
> "The protocol defines a generic way of sending a variable amount of
> contacts. The
> contact count is obtained by counting the number of non-empty finger packets
> between SYN_REPORT events." - Henrik Rydberg
>
> I think this verifies my assumption that all touch-points should be
> reported between SYN_REPORTs, otherwise it can not be used to obtain
> the count.
>
> [1] - https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt
> [2] - http://lists.x.org/archives/xorg-devel/2010-March/006466.html

Please add this to the commit log ;)

--
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://projetos.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
Erik Botö - July 3, 2013, 2:13 p.m.
Done, sent as v2-patch :)

Cheers,
Erik


On Wed, Jul 3, 2013 at 2:54 PM, Otavio Salvador <otavio@ossystems.com.br> wrote:
> On Wed, Jul 3, 2013 at 4:20 AM, Erik Botö <erik.boto@pelagicore.com> wrote:
>> Hi,
>>
>> Since the motivation for this patch was only discussed in a separate
>> mail thread I'll summarize here:
>>
>> The previous behavior of the driver did not work properly with Qt5
>> QtQuick multi touch-point gestures, due to how touch-points are
>> reported when removing a touch-point. My interpretation of the
>> available documentation [1] was that the driver should report all
>> touch-points between SYN_REPORTs, but it is not explicitly stated so.
>> I've found another mail-thread [2] where the creator of the protocol
>> states:
>>
>> "The protocol defines a generic way of sending a variable amount of
>> contacts. The
>> contact count is obtained by counting the number of non-empty finger packets
>> between SYN_REPORT events." - Henrik Rydberg
>>
>> I think this verifies my assumption that all touch-points should be
>> reported between SYN_REPORTs, otherwise it can not be used to obtain
>> the count.
>>
>> [1] - https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt
>> [2] - http://lists.x.org/archives/xorg-devel/2010-March/006466.html
>
> Please add this to the commit log ;)
>
> --
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://projetos.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750



--
=============================================
Erik Botö
Senior Software Engineer
Pelagicore AB
Ekelundsgatan 4, 6tr, SE-411 18 Gothenburg, Sweden
Mobile: +46 (0)76 881 72 03
E-Mail: erik.boto@pelagicore.com
=============================================
Mahadevan Mahesh-R9AADQ - July 17, 2013, 4:19 a.m.
This patch has been integrated into the FSL GIT and is available at the below link:
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/log/?h=imx_3.0.35_4.0.0

Best regards,
-Mahesh

-----Original Message-----
From: meta-freescale-bounces@yoctoproject.org [mailto:meta-freescale-bounces@yoctoproject.org] On Behalf Of Erik Botö
Sent: Wednesday, July 03, 2013 9:13 AM
To: Otavio Salvador
Cc: meta-freescale@yoctoproject.org
Subject: Re: [meta-freescale] [linux-boundary][PATCH] egalax_ts: Always report all touch points before input_sync()

Done, sent as v2-patch :)

Cheers,
Erik


On Wed, Jul 3, 2013 at 2:54 PM, Otavio Salvador <otavio@ossystems.com.br> wrote:
> On Wed, Jul 3, 2013 at 4:20 AM, Erik Botö <erik.boto@pelagicore.com> wrote:
>> Hi,
>>
>> Since the motivation for this patch was only discussed in a separate
>> mail thread I'll summarize here:
>>
>> The previous behavior of the driver did not work properly with Qt5
>> QtQuick multi touch-point gestures, due to how touch-points are
>> reported when removing a touch-point. My interpretation of the
>> available documentation [1] was that the driver should report all
>> touch-points between SYN_REPORTs, but it is not explicitly stated so.
>> I've found another mail-thread [2] where the creator of the protocol
>> states:
>>
>> "The protocol defines a generic way of sending a variable amount of
>> contacts. The
>> contact count is obtained by counting the number of non-empty finger packets
>> between SYN_REPORT events." - Henrik Rydberg
>>
>> I think this verifies my assumption that all touch-points should be
>> reported between SYN_REPORTs, otherwise it can not be used to obtain
>> the count.
>>
>> [1] - https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt
>> [2] - http://lists.x.org/archives/xorg-devel/2010-March/006466.html
>
> Please add this to the commit log ;)
>
> --
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://projetos.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750



--
=============================================
Erik Botö
Senior Software Engineer
Pelagicore AB
Ekelundsgatan 4, 6tr, SE-411 18 Gothenburg, Sweden
Mobile: +46 (0)76 881 72 03
E-Mail: erik.boto@pelagicore.com
=============================================

Patch

diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
index 0b6cde7..271f820 100644
--- a/drivers/input/touchscreen/egalax_ts.c
+++ b/drivers/input/touchscreen/egalax_ts.c
@@ -133,7 +133,6 @@  retry:
 	}
 
 	if (down) {
-		/* should also report old pointers */
 		events[id].valid = valid;
 		events[id].status = down;
 		events[id].x = x;
@@ -144,23 +143,6 @@  retry:
 		input_report_abs(input_dev, ABS_Y, y);
 		input_event(data->input_dev, EV_KEY, BTN_TOUCH, 1);
 		input_report_abs(input_dev, ABS_PRESSURE, 1);
-#else
-		for (i = 0; i < MAX_SUPPORT_POINTS; i++) {
-			if (!events[i].valid)
-				continue;
-			dev_dbg(&client->dev, "report id:%d valid:%d x:%d y:%d",
-				i, valid, x, y);
-
-			input_report_abs(input_dev,
-					 ABS_MT_TRACKING_ID, i);
-			input_report_abs(input_dev,
-					 ABS_MT_TOUCH_MAJOR, 1);
-			input_report_abs(input_dev,
-					 ABS_MT_POSITION_X, events[i].x);
-			input_report_abs(input_dev,
-					 ABS_MT_POSITION_Y, events[i].y);
-			input_mt_sync(input_dev);
-		}
 #endif
 	} else {
 		dev_dbg(&client->dev, "release id:%d\n", id);
@@ -176,6 +158,24 @@  retry:
 #endif
 	}
 
+#ifndef CONFIG_TOUCHSCREEN_EGALAX_SINGLE_TOUCH
+	/* report all pointers */
+	for (i = 0; i < MAX_SUPPORT_POINTS; i++) {
+		if (!events[i].valid)
+			continue;
+		dev_dbg(&client->dev, "report id:%d valid:%d x:%d y:%d",
+			i, valid, x, y);
+			input_report_abs(input_dev,
+				 ABS_MT_TRACKING_ID, i);
+		input_report_abs(input_dev,
+				 ABS_MT_TOUCH_MAJOR, 1);
+		input_report_abs(input_dev,
+				 ABS_MT_POSITION_X, events[i].x);
+		input_report_abs(input_dev,
+				 ABS_MT_POSITION_Y, events[i].y);
+		input_mt_sync(input_dev);
+	}
+#endif
 	input_sync(input_dev);
 	return IRQ_HANDLED;
 }