Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1583736 > unrolled thread

Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

Started byDmitry Torokhov <dmitry.torokhov@gmail.com>
First post2017-02-17 21:50 +0100
Last post2017-02-21 12:10 +0100
Articles 20 on this page of 60 — 8 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-17 21:50 +0100
    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Sebastian Reichel <sre@kernel.org> - 2017-02-18 04:30 +0100
      Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-18 12:40 +0100
        Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Sebastian Reichel <sre@kernel.org> - 2017-02-19 00:50 +0100
          Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-19 13:10 +0100
            Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Sebastian Reichel <sre@kernel.org> - 2017-02-19 21:20 +0100
              Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 18:00 +0100
    Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new  common binding names, pre-calibration, flipping and rotation Andreas Kemnade <andreas@kemnade.info> - 2017-02-18 09:20 +0100
    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-18 10:20 +0100
      Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-18 12:40 +0100
        Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-18 19:10 +0100
          Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-18 20:30 +0100
            Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-19 00:00 +0100
              Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-19 13:10 +0100
                Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-19 15:20 +0100
                  Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-19 18:10 +0100
                    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-19 18:20 +0100
                      Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-19 19:00 +0100
                        Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-19 20:10 +0100
                          Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-19 20:40 +0100
                            Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-19 22:00 +0100
                              Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-19 23:10 +0100
                                Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-19 23:30 +0100
                                  Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 18:00 +0100
                                    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-20 20:30 +0100
                                      Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 21:30 +0100
                                    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Petr Cvek <petr.cvek@tul.cz> - 2017-02-20 23:30 +0100
                                      Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-21 09:40 +0100
                              Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new  common binding names, pre-calibration, flipping and rotation Andreas Kemnade <andreas@kemnade.info> - 2017-02-19 23:40 +0100
                                Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new  common binding names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-19 23:50 +0100
                                  Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 18:00 +0100
                                    Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new  common binding names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-20 20:40 +0100
                                      Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 21:30 +0100
                                        Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 22:00 +0100
    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-18 12:40 +0100
      Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-20 02:20 +0100
        Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 18:00 +0100
          Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-20 20:50 +0100
            Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 21:40 +0100
              Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-20 22:10 +0100
                Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 22:30 +0100
                  Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-20 23:00 +0100
                    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-21 07:50 +0100
                      Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-21 10:00 +0100
            Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-20 22:10 +0100
              Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-20 22:30 +0100
                Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-20 23:00 +0100
                  Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-20 23:30 +0100
                    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-21 08:00 +0100
                  Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Petr Cvek <petr.cvek@tul.cz> - 2017-02-20 23:40 +0100
                    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-20 23:50 +0100
                  Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-21 07:40 +0100
                    Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-21 10:10 +0100
                      Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new  common binding names, pre-calibration, flipping and rotation Andreas Kemnade <andreas@kemnade.info> - 2017-02-21 18:20 +0100
                Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-20 23:10 +0100
                  Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-21 08:00 +0100
            Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-02-21 08:20 +0100
              Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pali Rohár <pali.rohar@gmail.com> - 2017-02-21 09:50 +0100
                Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new  common binding names, pre-calibration, flipping and rotation Christ van Willegen <cvwillegen@gmail.com> - 2017-02-21 10:00 +0100
          Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding  names, pre-calibration, flipping and rotation Pavel Machek <pavel@ucw.cz> - 2017-02-21 12:10 +0100

Page 1 of 3  [1] 2 3  Next page →

#1583736 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2017-02-17 21:50 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tbXDB-DC-33@gated-at.bofh.it>
Hi Nikolaus,

On Sat, Jan 28, 2017 at 10:44:35PM +0100, H. Nikolaus Schaller wrote:
> Hi Dmitry,
> 
> > Am 28.01.2017 um 20:33 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > 
> > Hi Nikolaus,
> > 
> > On Wed, Dec 28, 2016 at 03:53:16PM +0100, H. Nikolaus Schaller wrote:
> >> commit b98abe52fa8e ("Input: add common DT binding for touchscreens")
> >> introduced common DT bindings for touchscreens [1] and a helper function to
> >> parse the DT.
> >> 
> >> commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes")
> >> added another helper for parsing axis inversion and swapping
> >> and applying them to x and y coordinates.
> >> 
> >> Both helpers have been integrated to accommodate any orientation of the
> >> touch panel in relation to the LCD.
> >> 
> >> A new feature is to introduce scaling the min/max ADC values to the screen
> >> size.
> >> 
> >> This makes it possible to pre-calibrate the touch so that is (almost)
> >> exactly matches the LCD pixel coordinates it is glued onto. This allows to
> >> well enough operate the touch before a user space calibration step can
> >> improve the precision.
> > 
> > I question whether doing scaling in kernel is really right solution.
> 
> Since lower left corner does not exactly report [0 0] and upper right corner
> does not report [4095 4095] from the ADC we need offset and steepness correction
> of the ADC values.
> 
> This steepness is the scaling that must happen in kernel and I don't understand
> why you want to propagate this ADC errors to user-space by avoiding scaling.
> 
> Let me iterate what we want to achieve:
> * use new common bindings
> * offset and steepness calibration of the ADC (called pre-calibration).
>   This makes a real device much more reliable to operate with factory installed
>   scaling factors.
> * flipping and rotation
> 
> (note that touch pixel to LCD pixel scaling is not explicitly on this list!)

That was explicitly called out in the patch:

"A new feature is to introduce scaling the min/max ADC values to the
screen size."

> 
> Now to achieve the ADC pre-calibration we must calculate
> 
> 	x' = (x - ti,min-x) / (ti,max-x-ti,min-x) giving a rante from 0.0 ... 1.0
> 
> This is scaled up to what is defined by touchscreen-size-x, i.e.
> 
> 	x' = (touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x)
> 
> How do you want to avoid this scaling to take place? It happens automatically.
> It is not even an additional line of code. And is necessary for compensating ADC
> offsets and steepness.
> 
> So the only way to avoid the scaling option is to eliminate the precalibration/ADC
> compensation which is essential for a device which has no means to properly
> calibrate before operating the device through touch.
> 
> The other option would be to avoid common bindings and set
> 
> 	touchscreen-size-x = (ti,max-x-ti,min-x)
> 
> This is heavily dependent on specific ADC offsets forwarded to user-space.
> IMHO the worst we can do (and the current tsc2007 driver does it that way!).
> 
> > 
> > Why do you want this?
> 
> It seems that you assume that we want to enforce 1:1 scaling between touch pixels
> and LCD pixels and have designed code to achieve exactly that.
> 
> This is not the case. It is just a byproduct that you can do it.
> 
> And since it is easier to understand we have made the examples this way.
> 
> > If your touch resolution is lower than your screen
> > then it might be useful, but if it is lower then you are losing data
> > that can be very helpful for gesture recognition, and I hope you design
> > your userspace so it can handle not only "bad" hardware, but "good" as
> > well. And even with "bad" there are a lot of tricks that can be done to
> > get "better" touch position in userspace.
> 
> You can just *choose* DT parameters touchscreen-size-x to match the LCD size.
> This of course reduces the touch precision to full LCD pixels. For finger-
> touch operated devices, subpixel precision is rarely needed.
> 
> Also, some user-spaces (e.g. older Replicant for GTA04) assume that there is
> such an 1:1 mapping and they will be perfectly happy about this. 
> 
> But, if you can modify your user-space easily, you can also choose a different
> factor.
> 
> You can for example define touchscreen-size-x=<4096> and then you get almost
> the highest precision of the ADC and won't loose any bits. Or even define a
> bigger range and get steps >1 bit.
> 
> LCD driver (e.g. X11 calibration matrix) can scale it down again to LCD pixels.
> 
> So effectively we get for LCD pixels when the touch sets a mouse pointer:
> 
> 	x'' = user-space-scale * <<input-event<< ((touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x))
> 
> The most simple setup would then be but others are possible:
> 
> 	user-space-scale = 1
> 	touchscreen-size-x = LCD-size-x
> 
> Let me cite myself:
> 
> >> This makes it possible to pre-calibrate the touch so that is (almost)

Yes, it allows you to pre-calibrate, I get it. But you still do properly
calibrate later, right? So you are doing double work here, once in
kernel, and second time in userspace.

I'd be more open to allowing setting the "min-axis" values to allow
reporting typical range for given device, and let userspace scale as it
sees fit.

> 
> > 
> >> 
> >> Please note that the old ti,fuzz properties have been removed since they
> >> are replaced by the common bindings touchscreen-fuzz-x/y/z.
> >> 
> >> Finally, calculate_pressure has been renamed to calculate_resistance
> >> because that is what it is doing.
> > 
> > That is not what your patch does though. In the presence of
> > "ti,report-resistance" parameter you start reporting resistance through
> > ABS_PRESSURE
> 
> Well, there is some historic confusion wether this driver reports resistance
> or pressure.
> 
> The unpatched tsc2007 driver does it wrong (please test!) and we fix it on
> the fly (because a separate patch is much more complex than doing it right
> immediately).
> 
> This ti,report-resistance property is a means to get the old (wrong) meaning back
> in case someone urgently needs it and can't fix the user-space workaround which
> he must be using.
>
> 
> AFAIK there is no mainline board using the DT except ours (and the upcoming
> OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this
> compatibility property. We don't need it for our devices.
>

You seem to be treating DT data as something very fluid, which is wrong.
You need to treat it as a firmware, unlikely to change once device is
shipped. Unlike legacy platform data, the fact that DTS files are not
present in mainline does not mean that we can ignore such users and
change behavior at will.

That said, if driver behavior is out of line from the subsystem
expectations, we need to fix it.

 
> That the function name is wrong is a second issue and this double negation might
> confuse a litte.
> 
> Please test on a real device if the patched driver reports pressure now (unless
> ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware.
So all my observations are from code and data sheets.

That said, what is the values emitted as ABS_PRESSURE when finger is not
touching the device, barely touching the device, or pressing firmly?
It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have
confusion as to what is being reported.

I am adding a few more folks to the CC so we can try and soft this out.
Sebastian, Pali, Pavel, any input here?

Thanks.

-- 
Dmitry

[toc] | [next] | [standalone]

#1583839

FromSebastian Reichel <sre@kernel.org>
Date2017-02-18 04:30 +0100
Message-ID<tc3SF-4Tu-7@gated-at.bofh.it>
In reply to#1583736

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Fri, Feb 17, 2017 at 12:40:41PM -0800, Dmitry Torokhov wrote:
> > AFAIK there is no mainline board using the DT except ours (and the upcoming
> > OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this
> > compatibility property. We don't need it for our devices.

$ cd linux.git/arch
$ git grep -l tsc2004
arm/boot/dts/imx6qdl-nit6xlite.dtsi
arm/boot/dts/imx7d-nitrogen7.dts
arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
arm/boot/dts/omap4-var-som-om44.dtsi
$ git grep -l tsc2005
arm/boot/dts/omap3-n900.dts
$ git grep -l tsc2007
arm/boot/dts/imx28-tx28.dts
arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
arm/boot/dts/imx53-tx53-x03x.dts
arm/boot/dts/imx6qdl-tx6.dtsi
arm/boot/dts/imx6ul-tx6ul.dtsi
arm/boot/dts/omap3-gta04.dtsi
sh/boards/mach-ecovec24/setup.c

> You seem to be treating DT data as something very fluid, which is wrong.
> You need to treat it as a firmware, unlikely to change once device is
> shipped. Unlike legacy platform data, the fact that DTS files are not
> present in mainline does not mean that we can ignore such users and
> change behavior at will.
> 
> That said, if driver behavior is out of line from the subsystem
> expectations, we need to fix it.
> 
>  
> > That the function name is wrong is a second issue and this double negation might
> > confuse a litte.
> > 
> > Please test on a real device if the patched driver reports pressure now (unless
> > ti,report-resistance is specified).
> 
> I unfortunately can not test this driver as I do not have the hardware.
> So all my observations are from code and data sheets.
> 
> That said, what is the values emitted as ABS_PRESSURE when finger is not
> touching the device, barely touching the device, or pressing firmly?
> It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have
> confusion as to what is being reported.

As far as I can see all calculate Rtouch and ADS7846 reports
(Rmax - Rtouch), which looks sensible.

> I am adding a few more folks to the CC so we can try and soft this out.
> Sebastian, Pali, Pavel, any input here?

I think tsc200x works, since usually userspace is Xorg and I think
it only cares for x/y coordinates + boolean pressure. Since
no-pressure is correctly reported as 0, everything works as
expected. I currently don't have X running on my N900 due some
omapdrm bug, so I can't test this, sorry.

I suggest to put the resistance vs pressure thing in its own patch,
that also fixes tsc200x-core and merge it to linux-next after the
merge window.

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1583886 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-18 12:40 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tcbwR-1i3-9@gated-at.bofh.it>
In reply to#1583839

[Multipart message — attachments visible in raw view] — view raw

Hi Sebastian,

> Am 18.02.2017 um 04:22 schrieb Sebastian Reichel <sre@kernel.org>:
> 
> Hi,
> 
> On Fri, Feb 17, 2017 at 12:40:41PM -0800, Dmitry Torokhov wrote:
>>> AFAIK there is no mainline board using the DT except ours (and the upcoming
>>> OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this
>>> compatibility property. We don't need it for our devices.
> 
> $ cd linux.git/arch
> $ git grep -l tsc2004
> arm/boot/dts/imx6qdl-nit6xlite.dtsi
> arm/boot/dts/imx7d-nitrogen7.dts
> arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
> arm/boot/dts/omap4-var-som-om44.dtsi
> $ git grep -l tsc2005
> arm/boot/dts/omap3-n900.dts

Those are not relevant since tsc2004/5 and tsc2007 are independent drivers and don't
share code. Hence the N900 is not influenced by this patch series.

If it has a similar issue, it should be fixed of course.

> $ git grep -l tsc2007
> arm/boot/dts/imx28-tx28.dts
> arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
> arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
> arm/boot/dts/imx53-tx53-x03x.dts
> arm/boot/dts/imx6qdl-tx6.dtsi
> arm/boot/dts/imx6ul-tx6ul.dtsi
> arm/boot/dts/omap3-gta04.dtsi
> sh/boards/mach-ecovec24/setup.c

Sorry, I was a little imprecise here, because I looked for the min/max properties.
Of course, the imx devices use the tsc2007 as well.

Maybe we should edit all these DTS and set the "ti,report-resistance" property
by default. Then, no user should notice a difference.

Is any user/maintainer of these devices following this discussion and can comment?

> 
>> You seem to be treating DT data as something very fluid, which is wrong.
>> You need to treat it as a firmware, unlikely to change once device is
>> shipped. Unlike legacy platform data, the fact that DTS files are not
>> present in mainline does not mean that we can ignore such users and
>> change behavior at will.
>> 
>> That said, if driver behavior is out of line from the subsystem
>> expectations, we need to fix it.
>> 
>> 
>>> That the function name is wrong is a second issue and this double negation might
>>> confuse a litte.
>>> 
>>> Please test on a real device if the patched driver reports pressure now (unless
>>> ti,report-resistance is specified).
>> 
>> I unfortunately can not test this driver as I do not have the hardware.
>> So all my observations are from code and data sheets.
>> 
>> That said, what is the values emitted as ABS_PRESSURE when finger is not
>> touching the device, barely touching the device, or pressing firmly?
>> It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have
>> confusion as to what is being reported.
> 
> As far as I can see all calculate Rtouch and ADS7846 reports
> (Rmax - Rtouch), which looks sensible.

I don't see where this subtraction from Rmax takes place for the tsc2007:

http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2007.c#L131

> 
>> I am adding a few more folks to the CC so we can try and soft this out.
>> Sebastian, Pali, Pavel, any input here?
> 
> I think tsc200x works, since usually userspace is Xorg and I think
> it only cares for x/y coordinates + boolean pressure. Since
> no-pressure is correctly reported as 0, everything works as
> expected.

No pressure is usually treated as a special case in these drivers,
so reduction to "boolean" in user-space works well by accident and
might still hide a bug.

> I currently don't have X running on my N900 due some
> omapdrm bug, so I can't test this, sorry.

I usually look with evtest if ABS_PRESSURE is monotonic.

> 
> I suggest to put the resistance vs pressure thing in its own patch,
> that also fixes tsc200x-core and merge it to linux-next after the
> merge window.
> 
> -- Sebastian

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1584040

FromSebastian Reichel <sre@kernel.org>
Date2017-02-19 00:50 +0100
Message-ID<tcmVk-8mB-11@gated-at.bofh.it>
In reply to#1583886

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Sat, Feb 18, 2017 at 12:33:34PM +0100, H. Nikolaus Schaller wrote:
> Hi Sebastian,
> 
> > Am 18.02.2017 um 04:22 schrieb Sebastian Reichel <sre@kernel.org>:
> > 
> > Hi,
> > 
> > On Fri, Feb 17, 2017 at 12:40:41PM -0800, Dmitry Torokhov wrote:
> >>> AFAIK there is no mainline board using the DT except ours (and the upcoming
> >>> OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this
> >>> compatibility property. We don't need it for our devices.
> > 
> > $ cd linux.git/arch
> > $ git grep -l tsc2004
> > arm/boot/dts/imx6qdl-nit6xlite.dtsi
> > arm/boot/dts/imx7d-nitrogen7.dts
> > arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
> > arm/boot/dts/omap4-var-som-om44.dtsi
> > $ git grep -l tsc2005
> > arm/boot/dts/omap3-n900.dts
> 
> Those are not relevant since tsc2004/5 and tsc2007 are independent drivers and don't
> share code.

Yes, I'm aware.

> Hence the N900 is not influenced by this patch series.
> If it has a similar issue, it should be fixed of course.

Right. I added them to see every board affect by the patch suggested
by me in my last paragraph.

> > $ git grep -l tsc2007
> > arm/boot/dts/imx28-tx28.dts
> > arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
> > arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
> > arm/boot/dts/imx53-tx53-x03x.dts
> > arm/boot/dts/imx6qdl-tx6.dtsi
> > arm/boot/dts/imx6ul-tx6ul.dtsi
> > arm/boot/dts/omap3-gta04.dtsi
> > sh/boards/mach-ecovec24/setup.c
> 
> Sorry, I was a little imprecise here, because I looked for the min/max properties.
> Of course, the imx devices use the tsc2007 as well.
>
> Maybe we should edit all these DTS and set the "ti,report-resistance" property
> by default. Then, no user should notice a difference.

I suggest to create a patch without the report-reistance stuff and
add it early after the merge window and see what happens. If no
users notices anything the change is not an ABI break from kernel's
PoV.

> Is any user/maintainer of these devices following this discussion and can comment?
>
> > 
> >> You seem to be treating DT data as something very fluid, which is wrong.
> >> You need to treat it as a firmware, unlikely to change once device is
> >> shipped. Unlike legacy platform data, the fact that DTS files are not
> >> present in mainline does not mean that we can ignore such users and
> >> change behavior at will.
> >> 
> >> That said, if driver behavior is out of line from the subsystem
> >> expectations, we need to fix it.
> >> 
> >> 
> >>> That the function name is wrong is a second issue and this double negation might
> >>> confuse a litte.
> >>> 
> >>> Please test on a real device if the patched driver reports pressure now (unless
> >>> ti,report-resistance is specified).
> >> 
> >> I unfortunately can not test this driver as I do not have the hardware.
> >> So all my observations are from code and data sheets.
> >> 
> >> That said, what is the values emitted as ABS_PRESSURE when finger is not
> >> touching the device, barely touching the device, or pressing firmly?
> >> It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have
> >> confusion as to what is being reported.
> > 
> > As far as I can see all calculate Rtouch and ADS7846 reports
> > (Rmax - Rtouch), which looks sensible.
> 
> I don't see where this subtraction from Rmax takes place for the tsc2007:
> 
> http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2007.c#L131

sorry if I wrote this ambiguous, let me split my sentence

1. tsc200x & ads7846 calculate Rtouch
2. ads7846 reports Rmax - Rtouch
(3. tsc200x does not, it reports Rtouch instead)
4. ads7846 behaviour looks sensible to me

> >> I am adding a few more folks to the CC so we can try and soft this out.
> >> Sebastian, Pali, Pavel, any input here?
> > 
> > I think tsc200x works, since usually userspace is Xorg and I think
> > it only cares for x/y coordinates + boolean pressure. Since
> > no-pressure is correctly reported as 0, everything works as
> > expected.
> 
> No pressure is usually treated as a special case in these drivers,
> so reduction to "boolean" in user-space works well by accident and
> might still hide a bug.

That's what I assumed.

Btw. how did you notice that tsc2007 sends "inverted" pressure values?
Just in evtest or in some non-development application? (Just asking because
the behavour obviously changes at least for that usecase)

> > I currently don't have X running on my N900 due some
> > omapdrm bug, so I can't test this, sorry.
> 
> I usually look with evtest if ABS_PRESSURE is monotonic.

That would not have helped to check if X handles the touchscreen in
a boolean way. I can provide some N900 evtest data, though (tomorrow,
I don't have my dev N900 with me at the moment).

> > I suggest to put the resistance vs pressure thing in its own patch,
> > that also fixes tsc200x-core and merge it to linux-next after the
> > merge window.

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1584119 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-19 13:10 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tcyts-7e0-13@gated-at.bofh.it>
In reply to#1584040

[Multipart message — attachments visible in raw view] — view raw

Hi Sebastian,

> Am 19.02.2017 um 00:44 schrieb Sebastian Reichel <sre@kernel.org>:
> 
> Hi,
> 
> On Sat, Feb 18, 2017 at 12:33:34PM +0100, H. Nikolaus Schaller wrote:
>> Hi Sebastian,
>> 
>>> Am 18.02.2017 um 04:22 schrieb Sebastian Reichel <sre@kernel.org>:
>>> 
>>> Hi,
>>> 
>>> On Fri, Feb 17, 2017 at 12:40:41PM -0800, Dmitry Torokhov wrote:
>>>>> AFAIK there is no mainline board using the DT except ours (and the upcoming
>>>>> OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this
>>>>> compatibility property. We don't need it for our devices.
>>> 
>>> $ cd linux.git/arch
>>> $ git grep -l tsc2004
>>> arm/boot/dts/imx6qdl-nit6xlite.dtsi
>>> arm/boot/dts/imx7d-nitrogen7.dts
>>> arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
>>> arm/boot/dts/omap4-var-som-om44.dtsi
>>> $ git grep -l tsc2005
>>> arm/boot/dts/omap3-n900.dts
>> 
>> Those are not relevant since tsc2004/5 and tsc2007 are independent drivers and don't
>> share code.
> 
> Yes, I'm aware.
> 
>> Hence the N900 is not influenced by this patch series.
>> If it has a similar issue, it should be fixed of course.
> 
> Right. I added them to see every board affect by the patch suggested
> by me in my last paragraph.

Ok!

> 
>>> $ git grep -l tsc2007
>>> arm/boot/dts/imx28-tx28.dts
>>> arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
>>> arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
>>> arm/boot/dts/imx53-tx53-x03x.dts
>>> arm/boot/dts/imx6qdl-tx6.dtsi
>>> arm/boot/dts/imx6ul-tx6ul.dtsi
>>> arm/boot/dts/omap3-gta04.dtsi
>>> sh/boards/mach-ecovec24/setup.c
>> 
>> Sorry, I was a little imprecise here, because I looked for the min/max properties.
>> Of course, the imx devices use the tsc2007 as well.
>> 
>> Maybe we should edit all these DTS and set the "ti,report-resistance" property
>> by default. Then, no user should notice a difference.
> 
> I suggest to create a patch without the report-reistance stuff and
> add it early after the merge window and see what happens. If no
> users notices anything the change is not an ABI break from kernel's
> PoV.

That looks like a good strategy.

> 
>> Is any user/maintainer of these devices following this discussion and can comment?
>> 
>>> 
>>>> You seem to be treating DT data as something very fluid, which is wrong.
>>>> You need to treat it as a firmware, unlikely to change once device is
>>>> shipped. Unlike legacy platform data, the fact that DTS files are not
>>>> present in mainline does not mean that we can ignore such users and
>>>> change behavior at will.
>>>> 
>>>> That said, if driver behavior is out of line from the subsystem
>>>> expectations, we need to fix it.
>>>> 
>>>> 
>>>>> That the function name is wrong is a second issue and this double negation might
>>>>> confuse a litte.
>>>>> 
>>>>> Please test on a real device if the patched driver reports pressure now (unless
>>>>> ti,report-resistance is specified).
>>>> 
>>>> I unfortunately can not test this driver as I do not have the hardware.
>>>> So all my observations are from code and data sheets.
>>>> 
>>>> That said, what is the values emitted as ABS_PRESSURE when finger is not
>>>> touching the device, barely touching the device, or pressing firmly?
>>>> It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have
>>>> confusion as to what is being reported.
>>> 
>>> As far as I can see all calculate Rtouch and ADS7846 reports
>>> (Rmax - Rtouch), which looks sensible.
>> 
>> I don't see where this subtraction from Rmax takes place for the tsc2007:
>> 
>> http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2007.c#L131
> 
> sorry if I wrote this ambiguous, let me split my sentence
> 
> 1. tsc200x & ads7846 calculate Rtouch
> 2. ads7846 reports Rmax - Rtouch
> (3. tsc200x does not, it reports Rtouch instead)
> 4. ads7846 behaviour looks sensible to me

agreed.

> 
>>>> I am adding a few more folks to the CC so we can try and soft this out.
>>>> Sebastian, Pali, Pavel, any input here?
>>> 
>>> I think tsc200x works, since usually userspace is Xorg and I think
>>> it only cares for x/y coordinates + boolean pressure. Since
>>> no-pressure is correctly reported as 0, everything works as
>>> expected.
>> 
>> No pressure is usually treated as a special case in these drivers,
>> so reduction to "boolean" in user-space works well by accident and
>> might still hide a bug.
> 
> That's what I assumed.
> 
> Btw. how did you notice that tsc2007 sends "inverted" pressure values?
> Just in evtest or in some non-development application? (Just asking because
> the behavour obviously changes at least for that usecase)

I don't really remember when we noticed it first. Maybe it was back in tslib times
some years ago where setting the sensitivity threshold made problems. We then
carried along our patch for a long time in our local repo (and modified it
several times) and only started upstreaming some months ago. So it was included
but we never thought about it being something as important as the pre-calibration
which is really a benefit for setup and immediate useability of the whole system.
Hence we buried it a little in this pre-calibration, flipping and rotation patch.

> 
>>> I currently don't have X running on my N900 due some
>>> omapdrm bug, so I can't test this, sorry.
>> 
>> I usually look with evtest if ABS_PRESSURE is monotonic.
> 
> That would not have helped to check if X handles the touchscreen in
> a boolean way. I can provide some N900 evtest data, though (tomorrow,
> I don't have my dev N900 with me at the moment).

AFAIK, GIMP and for example https://sourceforge.net/projects/xournal/ appear
to be able to handle X pressure, but I haven't running and tested either one
on our devices. Pressure is used in such drawing tools to simulate that some
physical pens make wider strokes on higher pressure.

This seems to indicate that X can handle pressure in a non-boolean way, but rarely does.
Especially I think the usual menu, click, drag, scroll gestures are only based
on BTN_TOUCH status and not on ABS_PRESSURE. So it is rarely noticed to make
a difference.

> 
>>> I suggest to put the resistance vs pressure thing in its own patch,
>>> that also fixes tsc200x-core and merge it to linux-next after the
>>> merge window.

Ok. I will propose a patch.

BR,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1584249

FromSebastian Reichel <sre@kernel.org>
Date2017-02-19 21:20 +0100
Message-ID<tcG7D-3r3-5@gated-at.bofh.it>
In reply to#1584119

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Sun, Feb 19, 2017 at 01:07:26PM +0100, H. Nikolaus Schaller wrote:
> I don't really remember when we noticed it first. Maybe it was
> back in tslib times some years ago where setting the sensitivity
> threshold made problems. We then carried along our patch for a
> long time in our local repo (and modified it several times) and
> only started upstreaming some months ago. [...]
> 
> AFAIK, GIMP and for example https://sourceforge.net/projects/xournal/ appear
> to be able to handle X pressure, but I haven't running and tested either one
> on our devices. Pressure is used in such drawing tools to simulate that some
> physical pens make wider strokes on higher pressure.
> 
> This seems to indicate that X can handle pressure in a non-boolean way, but rarely does.
> Especially I think the usual menu, click, drag, scroll gestures are only based
> on BTN_TOUCH status and not on ABS_PRESSURE. So it is rarely noticed to make
> a difference.

ok.

> >>> I suggest to put the resistance vs pressure thing in its own patch,
> >>> that also fixes tsc200x-core and merge it to linux-next after the
> >>> merge window.
> 
> Ok. I will propose a patch.

Thanks. I suggest to add this in the patch description:

While this patch changes the values reported to userspace,
ABS_PRESSURE is used rarely by userspace. Most software only
relies on BTN_TOUCH (boolean), which is not affected by this
patch. Some graphics software makes use of the interface and
does not work correctly with the currently used inverted
behaviour.

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1584756 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-20 18:00 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tcZtF-7aS-57@gated-at.bofh.it>
In reply to#1584249

[Multipart message — attachments visible in raw view] — view raw

Hi Sebastian,

> Am 19.02.2017 um 21:15 schrieb Sebastian Reichel <sre@kernel.org>:
> 
> Hi,
> 
> On Sun, Feb 19, 2017 at 01:07:26PM +0100, H. Nikolaus Schaller wrote:
>> I don't really remember when we noticed it first. Maybe it was
>> back in tslib times some years ago where setting the sensitivity
>> threshold made problems. We then carried along our patch for a
>> long time in our local repo (and modified it several times) and
>> only started upstreaming some months ago. [...]
>> 
>> AFAIK, GIMP and for example https://sourceforge.net/projects/xournal/ appear
>> to be able to handle X pressure, but I haven't running and tested either one
>> on our devices. Pressure is used in such drawing tools to simulate that some
>> physical pens make wider strokes on higher pressure.
>> 
>> This seems to indicate that X can handle pressure in a non-boolean way, but rarely does.
>> Especially I think the usual menu, click, drag, scroll gestures are only based
>> on BTN_TOUCH status and not on ABS_PRESSURE. So it is rarely noticed to make
>> a difference.
> 
> ok.
> 
>>>>> I suggest to put the resistance vs pressure thing in its own patch,
>>>>> that also fixes tsc200x-core and merge it to linux-next after the
>>>>> merge window.
>> 
>> Ok. I will propose a patch.
> 
> Thanks. I suggest to add this in the patch description:
> 
> While this patch changes the values reported to userspace,
> ABS_PRESSURE is used rarely by userspace. Most software only
> relies on BTN_TOUCH (boolean), which is not affected by this
> patch. Some graphics software makes use of the interface and
> does not work correctly with the currently used inverted
> behaviour.

Added. Patch set will come in some minutes (have to run checkpatch
first).

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1583857 — Re: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

FromAndreas Kemnade <andreas@kemnade.info>
Date2017-02-18 09:20 +0100
SubjectRe: [Letux-kernel] [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tc8pj-7Q0-5@gated-at.bofh.it>
In reply to#1583736

[Multipart message — attachments visible in raw view] — view raw

Hi Dmitry,

On Fri, 17 Feb 2017 12:40:41 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

[...] 
> > Let me cite myself:
> > 
> > >> This makes it possible to pre-calibrate the touch so that is
> > >> (almost)
> 
> Yes, it allows you to pre-calibrate, I get it. But you still do
> properly calibrate later, right? So you are doing double work here,
> once in kernel, and second time in userspace.
> 
I as a daily user of that tsc2007 patch series say that I had never the
wish to calibrate it later in a better way. I am doing console work
with a virtual keyboard on it. So it is rarely double work here.

Regards,
Andreas

[toc] | [prev] | [next] | [standalone]

#1583865

FromPavel Machek <pavel@ucw.cz>
Date2017-02-18 10:20 +0100
Message-ID<tc9lo-8qG-9@gated-at.bofh.it>
In reply to#1583736

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > Well, there is some historic confusion wether this driver reports resistance
> > or pressure.
> > 
> > The unpatched tsc2007 driver does it wrong (please test!) and we fix it on
> > the fly (because a separate patch is much more complex than doing it right
> > immediately).
> > 
> > This ti,report-resistance property is a means to get the old (wrong) meaning back
> > in case someone urgently needs it and can't fix the user-space workaround which
> > he must be using.
> >
> > 
> > AFAIK there is no mainline board using the DT except ours (and the upcoming
> > OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this
> > compatibility property. We don't need it for our devices.

N900 is mainline and uses DT.

> > That the function name is wrong is a second issue and this double negation might
> > confuse a litte.
> > 
> > Please test on a real device if the patched driver reports pressure now (unless
> > ti,report-resistance is specified).
> 
> I unfortunately can not test this driver as I do not have the hardware.
> So all my observations are from code and data sheets.
> 
> That said, what is the values emitted as ABS_PRESSURE when finger is not
> touching the device, barely touching the device, or pressing firmly?
> It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have
> confusion as to what is being reported.
> 
> I am adding a few more folks to the CC so we can try and soft this out.
> Sebastian, Pali, Pavel, any input here?

X work ok on N900. Nikolaus wrote rather long email, but I'm not what
the meaning is and what is supposed to be broken there.

I do this on X startup:

xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix"  1.10 0.00 -0.05  0.00 1.18 -0.10  0.00 0.00 1.00
xinput --set-prop --type=int "TSC200X touchscreen" "Evdev Axis Inversion" 0 1
xinput --set-prop --type=float "TSC2005 touchscreen" "Coordinate Transformation Matrix"  1\
.10 0.00 -0.05  0.00 1.18 -0.10  0.00 0.00 1.00
xinput --set-prop --type=int "TSC2005 touchscreen" "Evdev Axis Inversion" 0 1

And I agree that kernel should _not_ attempt rescaling itself, as it
would lose precision. Providing default calibration info is ok.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1583888 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-18 12:40 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tcbwS-1i3-15@gated-at.bofh.it>
In reply to#1583865

[Multipart message — attachments visible in raw view] — view raw

Hi Pavel,

> Am 18.02.2017 um 10:15 schrieb Pavel Machek <pavel@ucw.cz>:
> 
> Hi!
> 
>>> Well, there is some historic confusion wether this driver reports resistance
>>> or pressure.
>>> 
>>> The unpatched tsc2007 driver does it wrong (please test!) and we fix it on
>>> the fly (because a separate patch is much more complex than doing it right
>>> immediately).
>>> 
>>> This ti,report-resistance property is a means to get the old (wrong) meaning back
>>> in case someone urgently needs it and can't fix the user-space workaround which
>>> he must be using.
>>> 
>>> 
>>> AFAIK there is no mainline board using the DT except ours (and the upcoming
>>> OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this
>>> compatibility property. We don't need it for our devices.
> 
> N900 is mainline and uses DT.

Yes, but it does not use the tsc2007 and will not be influenced at all by this patch.

> 
>>> That the function name is wrong is a second issue and this double negation might
>>> confuse a litte.
>>> 
>>> Please test on a real device if the patched driver reports pressure now (unless
>>> ti,report-resistance is specified).
>> 
>> I unfortunately can not test this driver as I do not have the hardware.
>> So all my observations are from code and data sheets.
>> 
>> That said, what is the values emitted as ABS_PRESSURE when finger is not
>> touching the device, barely touching the device, or pressing firmly?
>> It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have
>> confusion as to what is being reported.
>> 
>> I am adding a few more folks to the CC so we can try and soft this out.
>> Sebastian, Pali, Pavel, any input here?
> 
> X work ok on N900. Nikolaus wrote rather long email, but I'm not what
> the meaning is and what is supposed to be broken there.

IMHO it is broken that you have to do a subtle calibration step in user-space
and repeat it for different GUI toolkits.

> 
> I do this on X startup:
> 
> xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix"  1.10 0.00 -0.05  0.00 1.18 -0.10  0.00 0.00 1.00
> xinput --set-prop --type=int "TSC200X touchscreen" "Evdev Axis Inversion" 0 1
> xinput --set-prop --type=float "TSC2005 touchscreen" "Coordinate Transformation Matrix"  1\
> .10 0.00 -0.05  0.00 1.18 -0.10  0.00 0.00 1.00
> xinput --set-prop --type=int "TSC2005 touchscreen" "Evdev Axis Inversion" 0 1

Wouldn't  it be nice to get rid of this completely, because the DT/kernel knows these
factors? Especially since they are well defined by the hardware?

> 
> And I agree that kernel should _not_ attempt rescaling itself, as it
> would lose precision.

With an almost 1:1 mapping you won't loose precision.

> Providing default calibration info is ok.

IMHO it is missing one step of automation.

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1583942

FromPavel Machek <pavel@ucw.cz>
Date2017-02-18 19:10 +0100
Message-ID<tchCi-59d-9@gated-at.bofh.it>
In reply to#1583888

[Multipart message — attachments visible in raw view] — view raw

> > And I agree that kernel should _not_ attempt rescaling itself, as it
> > would lose precision.
> 
> With an almost 1:1 mapping you won't loose precision.

How do you propose to do that?
									Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1583960 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-18 20:30 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tciRH-5QK-5@gated-at.bofh.it>
In reply to#1583942

[Multipart message — attachments visible in raw view] — view raw

> Am 18.02.2017 um 19:08 schrieb Pavel Machek <pavel@ucw.cz>:
> 
>>> And I agree that kernel should _not_ attempt rescaling itself, as it
>>> would lose precision.
>> 
>> With an almost 1:1 mapping you won't loose precision.
> 
> How do you propose to do that?

something like

xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix"  1.00 0.00 0.00  0.00 1.00 0.00  0.00 0.00 1.00

but I think it is the default of X11 if you use no coordinate transformation at all.
And having the kernel to properly scale from ADC values to screen coordinates.

BR,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1584025

FromPavel Machek <pavel@ucw.cz>
Date2017-02-19 00:00 +0100
Message-ID<tcm8W-7QL-5@gated-at.bofh.it>
In reply to#1583960

[Multipart message — attachments visible in raw view] — view raw

On Sat 2017-02-18 20:17:09, H. Nikolaus Schaller wrote:
> 
> > Am 18.02.2017 um 19:08 schrieb Pavel Machek <pavel@ucw.cz>:
> > 
> >>> And I agree that kernel should _not_ attempt rescaling itself, as it
> >>> would lose precision.
> >> 
> >> With an almost 1:1 mapping you won't loose precision.
> > 
> > How do you propose to do that?
> 
> something like
> 
> xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix"  1.00 0.00 0.00  0.00 1.00 0.00  0.00 0.00 1.00
> 
> but I think it is the default of X11 if you use no coordinate transformation at all.
> And having the kernel to properly scale from ADC values to screen coordinates.
> 

No. How do you propose doing rescaling in the kernel without loosing
precision?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1584118 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-19 13:10 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tcytr-7e0-3@gated-at.bofh.it>
In reply to#1584025

[Multipart message — attachments visible in raw view] — view raw

Hi Pavel,

> Am 18.02.2017 um 23:54 schrieb Pavel Machek <pavel@ucw.cz>:
> 
> On Sat 2017-02-18 20:17:09, H. Nikolaus Schaller wrote:
>> 
>>> Am 18.02.2017 um 19:08 schrieb Pavel Machek <pavel@ucw.cz>:
>>> 
>>>>> And I agree that kernel should _not_ attempt rescaling itself, as it
>>>>> would lose precision.
>>>> 
>>>> With an almost 1:1 mapping you won't loose precision.
>>> 
>>> How do you propose to do that?
>> 
>> something like
>> 
>> xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix"  1.00 0.00 0.00  0.00 1.00 0.00  0.00 0.00 1.00
>> 
>> but I think it is the default of X11 if you use no coordinate transformation at all.
>> And having the kernel to properly scale from ADC values to screen coordinates.
>> 
> 
> No. How do you propose doing rescaling in the kernel without loosing
> precision?

I wonder how it works with your setting

xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix"  1.10 0.00 -0.05  0.00 1.18 -0.10  0.00 0.00 1.00

This obviously also assumes that the input events report pixel coordinates or you would have factors not close to 0.00 and 1.00.
It just aligns the touch with the screen, i.e. calibrates. Or your example was incomplete.

About loosing precision: there is already noise (jitter) in real-world devices so that you can't achieve subpixel precision anyways (unless your panel has a very low resolution). Please see my answer to Dmitry some mails ago.

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1584151

FromPavel Machek <pavel@ucw.cz>
Date2017-02-19 15:20 +0100
Message-ID<tcAvf-8oP-5@gated-at.bofh.it>
In reply to#1584118

[Multipart message — attachments visible in raw view] — view raw

Hi!

> About loosing precision: there is already noise (jitter) in
> real-world devices so that you can't achieve subpixel precision
> anyways (unless your panel has a very low resolution). Please see my
> answer to Dmitry some mails ago.

Maybe you can achieve better precision with averaging.

Anyway "input is already noisy" does not mean "so it is okay to
degrade it more". Solve it properly. That means passing calibration
data from kernel to userland.

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1584186 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-19 18:10 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tcD9M-1CL-29@gated-at.bofh.it>
In reply to#1584151

[Multipart message — attachments visible in raw view] — view raw

HI Pavel,

> Am 19.02.2017 um 15:17 schrieb Pavel Machek <pavel@ucw.cz>:
> 
> Hi!
> 
>> About loosing precision: there is already noise (jitter) in
>> real-world devices so that you can't achieve subpixel precision
>> anyways (unless your panel has a very low resolution). Please see my
>> answer to Dmitry some mails ago.
> 
> Maybe you can achieve better precision with averaging.

Can you? What do you want to average here? Multiple sequential measurements?
This makes the touch slower and hence imprecise and unuseable in another way.
Anyways, the tsc2007 chip can already do such averaging.

> 
> Anyway "input is already noisy" does not mean "so it is okay to
> degrade it more".

You have not yet said how you think it is degraded *more* than in your
example.

> Solve it properly. That means passing calibration
> data from kernel to userland.

As written before, the really proper solution would be to provide floating
or fixed point subpixel input events. Not arbitrarily scaling up in kernel
and leaving downscaling to user space (where everybody can make it worse).

But I don't think it is worth implementing subpixel touch events for real
world devices due to the jitter I mentioned.

BR,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1584193

FromPavel Machek <pavel@ucw.cz>
Date2017-02-19 18:20 +0100
Message-ID<tcDjr-1Gc-13@gated-at.bofh.it>
In reply to#1584186

[Multipart message — attachments visible in raw view] — view raw

> > Solve it properly. That means passing calibration
> > data from kernel to userland.
> 
> As written before, the really proper solution would be to provide floating
> or fixed point subpixel input events. Not arbitrarily scaling up in kernel
> and leaving downscaling to user space (where everybody can make it
> worse).

That has no advantages, and floating point in kernel is hard. Also
you'd either have to invent new interface, or you'd break touchscreen
for people that already have their touchscreens calibrated. Just
pass calibration data to userland.

> But I don't think it is worth implementing subpixel touch events for real
> world devices due to the jitter I mentioned.

Yes, that's not really proper solution, that just overengineered. Not
worth implementing. Pass calibration data to userland.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1584215 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-19 19:00 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tcDW9-1Tw-5@gated-at.bofh.it>
In reply to#1584193

[Multipart message — attachments visible in raw view] — view raw

Hi Pavel,
I love discussions with you :)

> Am 19.02.2017 um 18:15 schrieb Pavel Machek <pavel@ucw.cz>:
> 
> 
>>> Solve it properly. That means passing calibration
>>> data from kernel to userland.
>> 
>> As written before, the really proper solution would be to provide floating
>> or fixed point subpixel input events. Not arbitrarily scaling up in kernel
>> and leaving downscaling to user space (where everybody can make it
>> worse).
> 
> That has no advantages,

It has the advantage of providing you with the full precision of raw data (but
properly scaled) so that you don't loose any bit of information. This is what
you just asked for - one or two mails before.

And it provides me (and my users) with properly scaled touch coordinates, which
is what I (and they - compare e.g. comment by Andreas Kemnade) ask for.

So it would be a solution that fulfills both requirements. And fulfilling
requirements of two groups of people is advantageous to making only one happy.
Isn't it?

> and floating point in kernel is hard. Also
> you'd either have to invent new interface, or you'd break touchscreen
> for people that already have their touchscreens calibrated.

No, I don't break calibration for people using a different chip.

And since we just upstream a solution for devices (GTA04) which already use this
mechanism for years, we also won't break their calibration, because they are happy
with the non-calibration it provides.

We break it only for those, who use the same chip *and* upgrade to a kernel which
includes this patch. And even then, only if they update their device tree to use
it (the default without DT modifications is to provide raw data as before). If
they stay with an older kernel or older DT there is no change and action required.

So there are ca. 5 pre-conditions that others will notice a change at all.

Do you think user-space recalibration is such a difficult task compared to
upgrading a kernel that it must therefore be avoided? Compared to other
kernel-userland synchronization problems it has the good side that you
even will notice it immediately and will know what to do (recalibrate once
and it is done forever).

> Just
> pass calibration data to userland.

What is this good for if kernel can already do the calibration for userland?
If the kernel does it, it don't even waste efforts to pass calibration data
to userland.

And don't forget: if you need calibration data in userland, there are much
better mechanisms. The simplest one is a file (e.g. xorg.conf) which is
something the kernel and X11 already supports.

I.e. adding a new mechanism to pass calibration data from the kernel to
user-space doesn't make any sense, IMHO.

If the kernel knows about calibration it should use it directly and process it.
Like in iio where you can get raw and processed data, although processing could
also be done completely in user-space. So there seem to be good reasons to do
it in kernel...

And, citing your argument from above: "Also you'd either have to invent
new interface, or you'd break touchscreen for people that already have their
touchscreens calibrated."

> 
>> But I don't think it is worth implementing subpixel touch events for real
>> world devices due to the jitter I mentioned.
> 
> Yes, that's not really proper solution, that just overengineered. Not
> worth implementing. Pass calibration data to userland.

You seem to repeat yourself and just say which solution you prefer,
but I am missing the arguments why your solution (Pass calibration data
to userland) is right and the best one. Which problems does it solve?
Which one does it solve better than others? How can you implement it in
a stable and portable way? How can you make sure that all user-space GUI
systems can and will make use of this calibration data?

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1584235

FromPavel Machek <pavel@ucw.cz>
Date2017-02-19 20:10 +0100
Message-ID<tcF1T-2M9-11@gated-at.bofh.it>
In reply to#1584215

[Multipart message — attachments visible in raw view] — view raw

Hi!

> Hi Pavel,
> I love discussions with you :)

Unfortunately I can't say the same.

> > Am 19.02.2017 um 18:15 schrieb Pavel Machek <pavel@ucw.cz>:
> > 
> > 
> >>> Solve it properly. That means passing calibration
> >>> data from kernel to userland.
> >> 
> >> As written before, the really proper solution would be to provide floating
> >> or fixed point subpixel input events. Not arbitrarily scaling up in kernel
> >> and leaving downscaling to user space (where everybody can make it
> >> worse).
> > 
> > That has no advantages,
> 
> It has the advantage of providing you with the full precision of raw data (but
> properly scaled) so that you don't loose any bit of information. This is what
> you just asked for - one or two mails before.

Not really, right? No matter what kind of fixed point you introduce,
you'll still loose precision.

> > and floating point in kernel is hard. Also
> > you'd either have to invent new interface, or you'd break touchscreen
> > for people that already have their touchscreens calibrated.
> 
> No, I don't break calibration for people using a different chip.

So you propose your touchscreen to behave differently from all other
touchscreens in tree? That's just no-go.

> > Yes, that's not really proper solution, that just overengineered. Not
> > worth implementing. Pass calibration data to userland.
> 
> You seem to repeat yourself and just say which solution you prefer,
> but I am missing the arguments why your solution (Pass calibration data
> to userland) is right and the best one.
> Which problems does it solve?

All you described.

> Which one does it solve better than others?

It is not terminally ugly.

> How can you implement it in
> a stable and portable way?

Easily.

> How can you make sure that all user-space GUI
> systems can and will make use of this calibration data?

You can't, and you don't need to.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1584241 — Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From"H. Nikolaus Schaller" <hns@goldelico.com>
Date2017-02-19 20:40 +0100
SubjectRe: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation
Message-ID<tcFuV-2Wo-5@gated-at.bofh.it>
In reply to#1584235

[Multipart message — attachments visible in raw view] — view raw

Hi Pavel,

> Am 19.02.2017 um 20:05 schrieb Pavel Machek <pavel@ucw.cz>:
> 
> Hi!
> 
>> Hi Pavel,
>> I love discussions with you :)
> 
> Unfortunately I can't say the same.

> 
>>> Am 19.02.2017 um 18:15 schrieb Pavel Machek <pavel@ucw.cz>:
>>> 
>>> 
>>>>> Solve it properly. That means passing calibration
>>>>> data from kernel to userland.
>>>> 
>>>> As written before, the really proper solution would be to provide floating
>>>> or fixed point subpixel input events. Not arbitrarily scaling up in kernel
>>>> and leaving downscaling to user space (where everybody can make it
>>>> worse).
>>> 
>>> That has no advantages,
>> 
>> It has the advantage of providing you with the full precision of raw data (but
>> properly scaled) so that you don't loose any bit of information. This is what
>> you just asked for - one or two mails before.
> 
> Not really, right? No matter what kind of fixed point you introduce,
> you'll still loose precision.

Can you please elaborate?

My thoughts:

If the ADC has 12 bit of precision, then let's say 32 bits (16 before and 16
after the decimal point) of fixed point precision are not loosing anything
if you scale to screen coordinates (in most cases they are between 480 and
1280 max).

Theoretically you can loose the LSB of the 16 bits right of the decimal point.
This is ca. 0.000015259 pixels of precision. I don't care about this loss of
precision... Do you? If yes, why?

But as said I don't think we need float or fixed point for practical systems
at all.

> 
>>> and floating point in kernel is hard. Also
>>> you'd either have to invent new interface, or you'd break touchscreen
>>> for people that already have their touchscreens calibrated.
>> 
>> No, I don't break calibration for people using a different chip.
> 
> So you propose your touchscreen to behave differently from all other
> touchscreens in tree?

No. I only propose that my touch screen behaves properly and in the
best way it can. If others are worse, they should also be improved at
some time.

And note that I am not making things different from others in tree, I
am making the tsc2007 right (incl. following the touchscreen bindings
which define the touchscreen size in "Pixels").

> That's just no-go.

In other words: you want to block any improvements unless your favourite
touchscreen is giving directions...

> 
>>> Yes, that's not really proper solution, that just overengineered. Not
>>> worth implementing. Pass calibration data to userland.
>> 
>> You seem to repeat yourself and just say which solution you prefer,
>> but I am missing the arguments why your solution (Pass calibration data
>> to userland) is right and the best one.
>> Which problems does it solve?
> 
> All you described.

I think you are missing one problem: providing already properly scaled touch
values to user space.

Please look how iio is doing raw and processed data.

> 
>> Which one does it solve better than others?
> 
> It is not terminally ugly.

"ugly" is not a technical or scientific criterion and depends on your personal
perception. Do you have a better argument?

> 
>> How can you implement it in
>> a stable and portable way?
> 
> Easily.

Please go ahead and show code.

> 
>> How can you make sure that all user-space GUI
>> systems can and will make use of this calibration data?
> 
> You can't,

Exactly. Hence my proposal is superior. Because it avoids asking
the user-space to use calibration data.

>  and you don't need to.

How can you know that I don't have to? Do you know my systems and
users and what they want?

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web