From: alexandre.belloni@free-electrons•com (Alexandre Belloni)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCHv6] staging/iio/adc: change the MXS touchscreen driver implementation
Date: Thu, 09 Jan 2014 14:31:22 +0100 [thread overview]
Message-ID: <52CEA4AA.8050503@free-electrons.com> (raw)
In-Reply-To: <1379946998-23041-1-git-send-email-jbe@pengutronix.de>
Hi,
Sorry to chime in only now but it seems that this series is breaking the
touchscreen calibration on 3.13 (and -rc7 is out so it might be too
late).
At first, I though I became a terrible clicker ;) but I found some
evidences:
xinput_calibrator is complaining about misclicks, debug output:
DEBUG: Adding click 0 (X=105, Y=59)
DEBUG: Not adding click 1 (X=100, Y=59): within 7 pixels of previous click
DEBUG: Adding click 1 (X=696, Y=58)
DEBUG: Not adding click 2 (X=700, Y=55): within 7 pixels of previous click
DEBUG: Adding click 2 (X=103, Y=422)
DEBUG: Mis-click detected, click 3 (X=438, Y=415) not aligned with click 1 (X=696, Y=58) or click 2 (X=103, Y=422) (threshold=15)
DEBUG: Adding click 0 (X=424, Y=411)
Do you see the confusion ? At some point one of the coordinates is not
updated. Successful calibration with 3.12.6 gives:
DEBUG: Adding click 0 (X=126, Y=405)
DEBUG: Adding click 1 (X=684, Y=399)
DEBUG: Adding click 2 (X=128, Y=77)
DEBUG: Adding click 3 (X=683, Y=77)
With ts_calibrate:
xres = 800, yres = 480
Took 1 samples...
Top left : X = 435 Y = 3572
Took 2 samples...
Top right : X = 2094 Y = 3553
Took 2 samples...
Bot right : X = 2939 Y = 2077
Took 2 samples...
Bot left : X = 2060 Y = 644
Took 2 samples...
Center : X = 1279 Y = 1346
We experience the same thing, when changing position on an axis, we get
an average between the previous and the new position (probably because
we got 2 samples):
- Top Left is ok
- Top right is almost ok: X should be around 3500/3700
- Bot right is starting to get really wrong: X is getting better (it
should still be around 3500/3700) but Y should be quite lower
- Bot left: Y is ok but X should be lower
- Center is completely wrong, both values should be around 2000
Last test with evtest which is always difficult because it outputs a lot
of debug. Two separate touchs, bottom left and then top right (note that
this is with fsl,ave-ctrl = <8>;):
Event: time 1389210862.451000, type 3 (Absolute), code 0 (X), value 2183
Event: time 1389210862.451000, type 3 (Absolute), code 1 (Y), value 720
Event: time 1389210862.451000, type 3 (Absolute), code 24 (Pressure), value 6
Event: time 1389210862.451000, type 1 (Key), code 330 (Touch), value 1
Event: time 1389210862.451000, -------------- Report Sync ------------
Event: time 1389210862.477721, type 3 (Absolute), code 0 (X), value 448
Event: time 1389210862.477721, type 3 (Absolute), code 1 (Y), value 667
Event: time 1389210862.477721, type 3 (Absolute), code 24 (Pressure), value 595
Event: time 1389210862.477721, -------------- Report Sync ------------
Event: time 1389210862.504718, type 3 (Absolute), code 0 (X), value 434
Event: time 1389210862.504718, type 3 (Absolute), code 1 (Y), value 704
Event: time 1389210862.504718, type 3 (Absolute), code 24 (Pressure), value 580
Event: time 1389210862.504718, -------------- Report Sync ------------
Event: time 1389210862.536688, type 1 (Key), code 330 (Touch), value 0
Event: time 1389210862.536688, -------------- Report Sync ------------
Event: time 1389210863.359697, type 3 (Absolute), code 0 (X), value 432
Event: time 1389210863.359697, type 3 (Absolute), code 1 (Y), value 726
Event: time 1389210863.359697, type 3 (Absolute), code 24 (Pressure), value 210
Event: time 1389210863.359697, type 1 (Key), code 330 (Touch), value 1
Event: time 1389210863.359697, -------------- Report Sync ------------
Event: time 1389210863.391687, type 1 (Key), code 330 (Touch), value 0
Event: time 1389210863.391687, -------------- Report Sync ------------
We get 3 reports for bottom left, then pen up, then one report for top
right that is almost exactly the same a bottom left !
Output from 3.12:
Event: time 1389210022.146809, type 3 (Absolute), code 0 (X), value 286
Event: time 1389210022.146809, type 3 (Absolute), code 1 (Y), value 504
Event: time 1389210022.146809, type 3 (Absolute), code 24 (Pressure), value 3045
Event: time 1389210022.146809, type 1 (Key), code 330 (Touch), value 1
Event: time 1389210022.146809, -------------- Report Sync ------------
Event: time 1389210022.166830, type 3 (Absolute), code 0 (X), value 256
Event: time 1389210022.166830, type 3 (Absolute), code 1 (Y), value 489
Event: time 1389210022.166830, type 3 (Absolute), code 24 (Pressure), value 3033
Event: time 1389210022.166830, -------------- Report Sync ------------
Event: time 1389210022.186812, type 3 (Absolute), code 24 (Pressure), value 0
Event: time 1389210022.186812, type 1 (Key), code 330 (Touch), value 0
Event: time 1389210022.186812, -------------- Report Sync ------------
Event: time 1389210025.196808, type 3 (Absolute), code 0 (X), value 3812
Event: time 1389210025.196808, type 3 (Absolute), code 1 (Y), value 3637
Event: time 1389210025.196808, type 3 (Absolute), code 24 (Pressure), value 4032
Event: time 1389210025.196808, type 1 (Key), code 330 (Touch), value 1
Event: time 1389210025.196808, -------------- Report Sync ------------
Event: time 1389210025.216819, type 3 (Absolute), code 24 (Pressure), value 0
Event: time 1389210025.216819, type 1 (Key), code 330 (Touch), value 0
Event: time 1389210025.216819, -------------- Report Sync ------------
While this is not necessarily an issue with ts_calibrate (it is possible
to click longer so it collects more samples), I don't see that working
with xinput_calibrator.
While I don't have much experience with the TS part of the code but I
can investigate if you don't have any idea.
On 23/09/2013 16:36, Juergen Beisert wrote:
> The following series replaces the current busy loop touchscreen implementation
> for i.MX28/i.MX23 SoCs by a fully interrupt driven implementation.
>
> Since i.MX23 and i.MX28 silicon differs, the existing implementation can
> be used for the i.MX28 SoC only.
>
> The first patch adds proper clock handling. Various platforms seems to disable
> the internal 2 kHz clock which is used by the LRADC delay units.
>
> The next two patches of this series move the i.MX28 specific definitions
> out of the way. The forth patch simplifies the register access to make it easier
> to add the i.MX23 support. Then the i.MX23 specific definitions are added, also
> the code to distinguish both SoCs at run-time.
> Up to here the existing touchscreen driver will now run on an i.MX23 Soc as well.
>
> When these i.MX SoCs are running from battery it seems not to be a good idea to
> run a busy loop to detect touches and their location. The 6th patch adds a
> fully interrupt driven implementation which makes use of the built-in delay
> and multiple sample features of the touchscreen controller. This will reduce
> the interrupt load to a minimum.
>
> The remaining patches in this series just removes the existing busy loop
> implementation, add a proposal for devicetree binding and a reminder what has
> still to be done with the LRADC driver.
>
> Changes since v5:
>
> - add missing clock handling which prevents the delay units from work (this
> should make it work on the MX28EVK and M28EVK as well)
>
> Changes since v4:
>
> - honor Jonathan's comments about function names
> - honor Dmitry's comments about workqueue canceling and interrupts
> - adding devicetree bindings proposal
>
> Changes since v3:
>
> - split adding register access functions and i.MX23 support into two patches
>
> Changes since v2:
>
> - useless debug output removed
>
> Changes since v1:
>
> - adding register access functions to make the existing code more readable
> - adding some functions to distinguish the SoCs at run-time to avoid if-else
> contructs whenever differences in the register layout between i.MX23 and
> i.MX28 must be handled
>
> Comments are welcome.
>
> Juergen
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
next prev parent reply other threads:[~2014-01-09 13:31 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-23 14:36 [PATCHv6] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
2013-09-23 14:36 ` [PATCH 1/9] Staging/iio/adc/touchscreen/MXS: add proper clock handling Juergen Beisert
2013-09-23 15:13 ` Fabio Estevam
2013-09-24 7:50 ` Jürgen Beisert
2013-09-24 13:50 ` Fabio Estevam
2013-10-01 10:57 ` Jonathan Cameron
2013-09-23 15:30 ` Lothar Waßmann
2013-09-24 7:39 ` Jürgen Beisert
2013-09-23 14:36 ` [PATCH 2/9] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC Juergen Beisert
2013-10-01 10:58 ` Jonathan Cameron
2013-09-23 14:36 ` [PATCH 3/9] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits Juergen Beisert
2013-10-01 11:00 ` Jonathan Cameron
2013-09-23 14:36 ` [PATCH 4/9] Staging/iio/adc/touchscreen/MXS: simplify register access Juergen Beisert
2013-10-01 11:01 ` Jonathan Cameron
2013-09-23 14:36 ` [PATCH 5/9] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver Juergen Beisert
2013-10-01 11:02 ` Jonathan Cameron
2013-09-23 14:36 ` [PATCH 6/9] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection Juergen Beisert
2013-10-01 9:28 ` Jürgen Beisert
2013-10-01 10:51 ` Jonathan Cameron
2013-10-01 11:06 ` Jonathan Cameron
2013-09-23 14:36 ` [PATCH 7/9] Staging/iio/adc/touchscreen/MXS: remove old touchscreen detection implementation Juergen Beisert
2013-10-01 11:13 ` Jonathan Cameron
2013-09-23 14:36 ` [PATCH 8/9] Staging/iio/adc/touchscreen/MXS: provide devicetree adaption Juergen Beisert
2013-10-01 11:14 ` Jonathan Cameron
2013-09-23 14:36 ` [PATCH 9/9] Staging/iio: add TODO reminder Juergen Beisert
2013-10-01 11:14 ` Jonathan Cameron
2013-10-01 11:21 ` Jonathan Cameron
2013-10-01 11:22 ` Jonathan Cameron
2013-09-23 15:25 ` [PATCHv6] staging/iio/adc: change the MXS touchscreen driver implementation Marek Vasut
2013-10-01 9:25 ` Jonathan Cameron
2013-10-01 10:49 ` Marek Vasut
2014-01-09 13:31 ` Alexandre Belloni [this message]
2014-01-10 8:55 ` Jürgen Beisert
2014-02-04 23:45 ` Marek Vasut
2014-02-24 13:16 ` Juergen Beisert
2014-02-24 13:33 ` Dan Carpenter
2014-02-24 14:38 ` Juergen Beisert
2014-02-24 14:47 ` Dan Carpenter
2014-02-24 14:39 ` [PATCH] staging/iio/adc/MXS/LRADC: fix touchscreen statemachine Juergen Beisert
2014-02-24 16:48 ` Alexandre Belloni
2014-02-24 21:14 ` Jonathan Cameron
2014-02-24 22:04 ` Jonathan Cameron
2014-02-24 14:26 ` [PATCHv6] staging/iio/adc: change the MXS touchscreen driver implementation Alexandre Belloni
2014-02-24 17:14 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52CEA4AA.8050503@free-electrons.com \
--to=alexandre.belloni@free-electrons$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox