All of lore.kernel.org
 help / color / mirror / Atom feed
* Issue with egalax_ts in linux-boundary
@ 2013-07-01 15:53 Erik Botö
  2013-07-01 16:45 ` Eric Nelson
  2013-07-01 16:56 ` Fabio Estevam
  0 siblings, 2 replies; 9+ messages in thread
From: Erik Botö @ 2013-07-01 15:53 UTC (permalink / raw)
  To: meta-freescale@yoctoproject.org

[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]

Hi,

When trying to use a nitrogen6x board with Freescale MCIMX-LVDS1
touchscreen together with Qt5 to create a QtQuick GUI that uses multitouch
gestures I discovered that the touchscreen driver (egalax_ts) does not work
as intended with Qt5. I'm not entirely sure if it's Qt that is not handling
a case that should be ok or if it's the driver that is misbehaving, but I
think it's the driver that is at fault.

The problem is that when pressing two fingers on the screen and then when
you lift one finger only the lifted finger will be reported before sending
the next SYN_REPORT, instead of also reporting the other present fingers
before SYN_REPORT.

Is there a separate mailing list for boundary-linux or is this list ok? I
thought this might be useful to get "on the record" here anyway if someone
else hits the same issue.

Cheers,
Erik Botö
Pelagicore AB

Proposed fix:

--- git/drivers/input/touchscreen/egalax_ts.c 2013-06-19 18:17:31.073736481
+0200
+++ git.new/drivers/input/touchscreen/egalax_ts.c 2013-07-01
17:04:21.054466334 +0200
@@ -173,6 +173,23 @@
  input_report_abs(input_dev, ABS_MT_TRACKING_ID, id);
  input_event(input_dev, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
  input_mt_sync(input_dev);
+ /* report other pointers as well */
+ 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
  }

[-- Attachment #2: Type: text/html, Size: 3167 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Issue with egalax_ts in linux-boundary
  2013-07-01 15:53 Issue with egalax_ts in linux-boundary Erik Botö
@ 2013-07-01 16:45 ` Eric Nelson
  2013-07-01 18:49   ` Erik Botö
  2013-07-01 16:56 ` Fabio Estevam
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Nelson @ 2013-07-01 16:45 UTC (permalink / raw)
  To: Erik Botö; +Cc: meta-freescale@yoctoproject.org

Hi Erik,

On 07/01/2013 08:53 AM, Erik Botö wrote:
> Hi,
>
> When trying to use a nitrogen6x board with Freescale MCIMX-LVDS1
> touchscreen together with Qt5 to create a QtQuick GUI that uses
> multitouch gestures I discovered that the touchscreen driver (egalax_ts)
> does not work as intended with Qt5. I'm not entirely sure if it's Qt
> that is not handling a case that should be ok or if it's the driver that
> is misbehaving, but I think it's the driver that is at fault.
>
Have you compared against the docs?

I think this should be convered by this kernel document:
	https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/input/multi-touch-protocol.txt?id=refs/tags/v3.10

BTW, I'm happy to hear that Qt is supporting multi-touch. I didn't
realize that this was in the works.

> The problem is that when pressing two fingers on the screen and then
> when you lift one finger only the lifted finger will be reported before
> sending the next SYN_REPORT, instead of also reporting the other present
> fingers before SYN_REPORT.
>
I'll test this out when time permits.

> Is there a separate mailing list for boundary-linux or is this list ok?
> I thought this might be useful to get "on the record" here anyway if
> someone else hits the same issue.
>

This list is as good as anyplace to discuss this, especially since
the eGalax touch screen is supported by most Freescale boards as
well as those produced by us.

Note that 'linux-boundary' for Yocto has multi-touch support specfically
disabled through the kernel configuration because most non-Android
environments get confused by multi-touch.

Since the multi-touch drivers are mostly used under Android, we'll want
to investigate how this change affects that platform as well.

Regards,


Eric


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Issue with egalax_ts in linux-boundary
  2013-07-01 15:53 Issue with egalax_ts in linux-boundary Erik Botö
  2013-07-01 16:45 ` Eric Nelson
@ 2013-07-01 16:56 ` Fabio Estevam
  2013-07-01 17:06   ` Eric Nelson
  1 sibling, 1 reply; 9+ messages in thread
From: Fabio Estevam @ 2013-07-01 16:56 UTC (permalink / raw)
  To: Erik Botö; +Cc: meta-freescale@yoctoproject.org, Otavio Salvador

Hi Erik,

On Mon, Jul 1, 2013 at 12:53 PM, Erik Botö <erik.boto@pelagicore.com> wrote:
> Hi,
>
> When trying to use a nitrogen6x board with Freescale MCIMX-LVDS1 touchscreen
> together with Qt5 to create a QtQuick GUI that uses multitouch gestures I
> discovered that the touchscreen driver (egalax_ts) does not work as intended
> with Qt5. I'm not entirely sure if it's Qt that is not handling a case that
> should be ok or if it's the driver that is misbehaving, but I think it's the
> driver that is at fault.
>
> The problem is that when pressing two fingers on the screen and then when
> you lift one finger only the lifted finger will be reported before sending
> the next SYN_REPORT, instead of also reporting the other present fingers
> before SYN_REPORT.
>
> Is there a separate mailing list for boundary-linux or is this list ok? I
> thought this might be useful to get "on the record" here anyway if someone
> else hits the same issue.
>
> Cheers,
> Erik Botö
> Pelagicore AB
>
> Proposed fix:

Your proposed fix matches the code from FSL BSP 4.0.0 driver:

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/input/touchscreen/egalax_ts.c?h=imx_3.0.35_4.0.0

#ifdef CONFIG_TOUCHSCREEN_EGALAX_SINGLE_TOUCH
		input_report_abs(input_dev, ABS_X, x);
		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

Regards,

Fabio Estevam


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Issue with egalax_ts in linux-boundary
  2013-07-01 16:56 ` Fabio Estevam
@ 2013-07-01 17:06   ` Eric Nelson
  2013-07-01 18:59     ` Erik Botö
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Nelson @ 2013-07-01 17:06 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: meta-freescale@yoctoproject.org, Otavio Salvador

Thanks Fabio,

On 07/01/2013 09:56 AM, Fabio Estevam wrote:
> Hi Erik,
>
> On Mon, Jul 1, 2013 at 12:53 PM, Erik Botö <erik.boto@pelagicore.com> wrote:
>> Hi,
>>
>>  <snip>
>>
>> The problem is that when pressing two fingers on the screen and then when
>> you lift one finger only the lifted finger will be reported before sending
>> the next SYN_REPORT, instead of also reporting the other present fingers
>> before SYN_REPORT.
>>
>> <snip>
>
> Your proposed fix matches the code from FSL BSP 4.0.0 driver:
>
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/input/touchscreen/egalax_ts.c?h=imx_3.0.35_4.0.0
>
> #ifdef CONFIG_TOUCHSCREEN_EGALAX_SINGLE_TOUCH
> 		input_report_abs(input_dev, ABS_X, x);
> 		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

Also our kernel tree in branch 'boundary-imx_3.0.35_4.0.0':

https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_4.0.0/drivers/input/touchscreen/egalax_ts.c#L142

Regards,


Eric


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Issue with egalax_ts in linux-boundary
  2013-07-01 16:45 ` Eric Nelson
@ 2013-07-01 18:49   ` Erik Botö
  0 siblings, 0 replies; 9+ messages in thread
From: Erik Botö @ 2013-07-01 18:49 UTC (permalink / raw)
  To: Eric Nelson; +Cc: meta-freescale@yoctoproject.org

Hi,

On Mon, Jul 1, 2013 at 6:45 PM, Eric Nelson
<eric.nelson@boundarydevices.com> wrote:
> Have you compared against the docs?
>
> I think this should be convered by this kernel document:
>         https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/input/multi-touch-protocol.txt?id=refs/tags/v3.10
>

Yes that's what I have read. It's not entirely clear to me if it's ok
or not to do as the driver currently does, but it's not following the
example pattern of protocol A from the multi-touch-protocol.txt.

> BTW, I'm happy to hear that Qt is supporting multi-touch. I didn't
> realize that this was in the works.
>

Yes Qt5 has good multi-point support.

>
> This list is as good as anyplace to discuss this, especially since
> the eGalax touch screen is supported by most Freescale boards as
> well as those produced by us.
>
> Note that 'linux-boundary' for Yocto has multi-touch support specfically
> disabled through the kernel configuration because most non-Android
> environments get confused by multi-touch.
>
> Since the multi-touch drivers are mostly used under Android, we'll want
> to investigate how this change affects that platform as well.
>
> Regards,
>
>
> Eric

Cheers,
Erik


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Issue with egalax_ts in linux-boundary
  2013-07-01 17:06   ` Eric Nelson
@ 2013-07-01 18:59     ` Erik Botö
  2013-07-02  8:13       ` Erik Botö
  0 siblings, 1 reply; 9+ messages in thread
From: Erik Botö @ 2013-07-01 18:59 UTC (permalink / raw)
  To: Eric Nelson; +Cc: meta-freescale@yoctoproject.org, Otavio Salvador

Hi Fabio and Eric,

On Mon, Jul 1, 2013 at 7:06 PM, Eric Nelson
<eric.nelson@boundarydevices.com> wrote:
> Thanks Fabio,
>
>
> On 07/01/2013 09:56 AM, Fabio Estevam wrote:
>>
>> Hi Erik,
>>
>> On Mon, Jul 1, 2013 at 12:53 PM, Erik Botö <erik.boto@pelagicore.com>
>> wrote:
>>>
>>> Hi,
>>>
>>>  <snip>
>>>
>>>
>>> The problem is that when pressing two fingers on the screen and then when
>>> you lift one finger only the lifted finger will be reported before
>>> sending
>>> the next SYN_REPORT, instead of also reporting the other present fingers
>>> before SYN_REPORT.
>>>
>>> <snip>
>>
>>
>> Your proposed fix matches the code from FSL BSP 4.0.0 driver:
>>
>>
>> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/input/touchscreen/egalax_ts.c?h=imx_3.0.35_4.0.0
>>
>> #ifdef CONFIG_TOUCHSCREEN_EGALAX_SINGLE_TOUCH
>>                 input_report_abs(input_dev, ABS_X, x);
>>                 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
>
>
> Also our kernel tree in branch 'boundary-imx_3.0.35_4.0.0':
>
> https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_4.0.0/drivers/input/touchscreen/egalax_ts.c#L142
>
> Regards,
>
>
> Eric

I think you missed what my patch does, it duplicates the for-loop used
in the "if (down)"-case so it's also done in  the "else"-case. Now
that I frase it this way I realize it should probably be removed from
the "if (down) else ()"-statement and always done before input_sync()
instead. I'll post an updated patch-suggestion tomorrow.

The way I read multi-touch-protocol.txt all touch points should be
reported before input_sync (SYN_REPORT), which is not the case today
if one of multiple fingers are lifted.

Cheers,
Erik


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Issue with egalax_ts in linux-boundary
  2013-07-01 18:59     ` Erik Botö
@ 2013-07-02  8:13       ` Erik Botö
  2013-07-02 12:01         ` Otavio Salvador
  0 siblings, 1 reply; 9+ messages in thread
From: Erik Botö @ 2013-07-02  8:13 UTC (permalink / raw)
  To: Eric Nelson; +Cc: meta-freescale@yoctoproject.org, Otavio Salvador

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]

Hi,

On Mon, Jul 1, 2013 at 8:59 PM, Erik Botö <erik.boto@pelagicore.com> wrote:
> Now
> that I frase it this way I realize it should probably be removed from
> the "if (down) else ()"-statement and always done before input_sync()
> instead. I'll post an updated patch-suggestion tomorrow.

And here's the updated version. I simple moved the loop that reports all
touch points outside of the if-else statement.

Cheers,
Erik Botö

diff -uNr git.orig/drivers/input/touchscreen/egalax_ts.c
git/drivers/input/touchscreen/egalax_ts.c
--- git.orig/drivers/input/touchscreen/egalax_ts.c 2013-07-02
08:04:22.456501162 +0200
+++ git/drivers/input/touchscreen/egalax_ts.c 2013-07-02 08:05:53.684507633
+0200
@@ -133,7 +133,6 @@
  }

  if (down) {
- /* should also report old pointers */
  events[id].valid = valid;
  events[id].status = down;
  events[id].x = x;
@@ -144,23 +143,6 @@
  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 @@
 #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;
 }

[-- Attachment #2: Type: text/html, Size: 5242 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Issue with egalax_ts in linux-boundary
  2013-07-02  8:13       ` Erik Botö
@ 2013-07-02 12:01         ` Otavio Salvador
  2013-07-03  7:09           ` Erik Botö
  0 siblings, 1 reply; 9+ messages in thread
From: Otavio Salvador @ 2013-07-02 12:01 UTC (permalink / raw)
  To: Erik Botö; +Cc: meta-freescale@yoctoproject.org

On Tue, Jul 2, 2013 at 5:13 AM, Erik Botö <erik.boto@pelagicore.com> wrote:
> Hi,
>
>
> On Mon, Jul 1, 2013 at 8:59 PM, Erik Botö <erik.boto@pelagicore.com> wrote:
>> Now
>> that I frase it this way I realize it should probably be removed from
>> the "if (down) else ()"-statement and always done before input_sync()
>> instead. I'll post an updated patch-suggestion tomorrow.
>
> And here's the updated version. I simple moved the loop that reports all
> touch points outside of the if-else statement.

Can you do a proper commit in the GIT of the Linux kernel and send it
for review  here? This would allow for people to check it and apply in
the respective trees.

--
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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Issue with egalax_ts in linux-boundary
  2013-07-02 12:01         ` Otavio Salvador
@ 2013-07-03  7:09           ` Erik Botö
  0 siblings, 0 replies; 9+ messages in thread
From: Erik Botö @ 2013-07-03  7:09 UTC (permalink / raw)
  To: Otavio Salvador; +Cc: meta-freescale@yoctoproject.org

On Tue, Jul 2, 2013 at 2:01 PM, Otavio Salvador <otavio@ossystems.com.br> wrote:
> Can you do a proper commit in the GIT of the Linux kernel and send it
> for review  here? This would allow for people to check it and apply in
> the respective trees.
>

Good point! I've done this now in a separate mail.

> --
> 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
=============================================


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-07-03  7:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-01 15:53 Issue with egalax_ts in linux-boundary Erik Botö
2013-07-01 16:45 ` Eric Nelson
2013-07-01 18:49   ` Erik Botö
2013-07-01 16:56 ` Fabio Estevam
2013-07-01 17:06   ` Eric Nelson
2013-07-01 18:59     ` Erik Botö
2013-07-02  8:13       ` Erik Botö
2013-07-02 12:01         ` Otavio Salvador
2013-07-03  7:09           ` Erik Botö

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.