Thread (25 messages) 25 messages, 6 authors, 2016-10-17

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

From: Sebastian Reichel <hidden>
Date: 2016-09-30 14:16:19
Also in: linux-devicetree, linux-iio, linux-omap, lkml

Hi,

On Sat, Sep 24, 2016 at 07:55:27AM +0200, H. Nikolaus Schaller wrote:
quoted
So ti,max-[xy] is basically the same as touchscreen-size-[xy],
No it is not the same and should be kept separate.
quoted
except, that the generic bindings don't support min-[xy] != 0.
What would be the purpose of this? Every user-space I know
about (X11, Replicant) expects coordinates in some range
0..max so setting min in device tree makes no sense to me.
quoted
So maybe change the generic bindings like this:

touchscreen-min-x: minimum value reported by X axis ADC (default 0)
touchscreen-max-x: maximum value reported by Y axis ADC
touchscreen-min-y: minimum value reported by Y axis ADC (default 0)
touchscreen-max-y: maximum value reported by Y axis ADC
touchscreen-size-x: deprecated alias for touchscreen-max-x
touchscreen-size-y: deprecated alias for touchscreen-max-y
Initially I had thought about this but it does not solve the problems
with touch pre-calibration. Since it mixes raw coordinates with
system coordinates.
touchscreen-size-x was actually refering to your definition of
touchscreen-max-x and not system coordinates. For that it would
make much more sense to use a phandle to the screen IMHO.
To achieve the goal of having a roughly precalibrated touch which
should provide (0,0) at the lower left corner and
(touchscreen-size-x,touchscreen-size-y) in pixel coordinates of
the panel. Hence it roughly works without a calibration matrix in
user space (e.g. xorg.conf or Replicant).
well I did not mean to use touchscreen-size-x/y for describing the
size of screen, as visible in n900.dts (first implementation of the
common binding), which sets the value to 4096.
Why do we need pre-calibration? Because some systems might need
touch interaction before they can offer (force) the user into
a touch calibration step. We use these drivers and approach in
our production kernels for GTA04, OpenPandora and Pyra for a while
and nobody was even missing a user-space calibration tool any more.
I have nothing against the feature. OTOH I'm quite in of kernel
based TS calibration. Note, that you can only add it for hardware
without pre-existing touchscreen support, since you break peoples
systems otherwise (We have that problem for N900).
The underlaying problem is that you can have the same controller chip
in different board designs and there are different touch panel types.
Each one has certain physical properties but they can differ.
But you certainly want touchscreen-size-x/y to be a constant.

Now if we make touchscreen-max-x/y the same as touchscreen-size-x/y
and change the panel, we have to adjust user space transformation
each time we change the panel. This does not seem to be right
and can be done better by keeping them separately.

This is what this approach does: the roughly correct scaling of
raw values to pixel values.

ti,min-x   ->   0
...
x          ->   some value between 0 and touchscreen-size-x
                calculated by
			touchscreen-size-x * (x - min-x) / (max-x - min-x)
...
ti,max-x   ->   touchscreen-size-x

Hence the ti,min/max values describe the range of expected input
values from the ADC and the touchscreen-size-x describes the touch
in LCD pixels passed as input events.
so basically you use touchscreen-size-x to describe the screen and
not the touchscreen. When I added it, I did mean the max ADC value.
Actually I was under the impression, that X drivers would scale this
to screen size automatically. Since all my touchscreen HW required
calibration I did never test this, though.
Example:

ti,min-x = 64
ti,max-x = 4016
touchscreen-size-x = 480

If we change the panel type which presents a slightly different ADC range:

ti,min-x = 100
ti,max-x = 3900
touchscreen-size-x = 480

and we still get a coordinate range (0 .. 480).

Note that this feature can be effectively disabled if ti,min-x=0 and
ti,max-x=4095 and touchscreen-size-x=4095, i.e. reports the full
range of ADC values because then it multiplies by 1.

Our proposed driver does use these values if they are missing from DT
and therefore it should not break old DT files which expect raw values
to be reported.

I hope this clarifies what we need to achieve and you can
agree.
I did understand what you want, but I disagreed about
using touchscreen-size-x/y for system coordinates. I
now see, that it's too late for that, as other people
already did so.

I do agree with Rob, that the ti,min/max-x/y should become common,
though. Also I would do s/minimum value/minimum raw value/g.
Additionally touchscreen-size-x/y should mention, that it's used to
scale the raw values.

-- Sebastian

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help