Thanks Jeff for the feedback.
Bumping this back up.
Maintainers - could you please have a look?
On Fri, May 27, 2022 at 3:36 PM Jeff LaBundy <jeff@xxxxxxxxxxx> wrote:
>
> Hi Siarhei,
>
> Apologies for the delayed response.
>
> On Tue, May 24, 2022 at 09:53:22AM -0700, Siarhei Vishniakou wrote:
> > Hi Jeff,
> >
> > I assume you are talking about touch IC controllers and not gamepad controllers.
> > The units for ABS_X and ABS_Y are already documented. The proposal
> > here is to expand the documentation to include the ABS_MT_TOUCH_MAJOR
> > and ABS_MT_TOUCH_MINOR axes in the definition.
>
> I meant any multitouch controller really. Good point about existing
> single-contact axes though.
>
> >
> > We can't fix existing devices, but the documentation would specify the
> > "correct" behaviour going forward. Based on my understanding of
> > hidinput_calc_abs_res function, the units for the resolution of
> > major/minor are already "units/mm".
> > This behaviour is already the default in linux, just not documented.
> > If you were to develop a HID touchscreen, you would already get this
> > behaviour today.
>
> ACK.
>
> >
> > I don't think the problem is the same as for ABS_MT_PRESSURE. In the
> > case of pressure, the value comes from arbitrary algorithms using
> > capacitive data to guess "pressure" rather than using physical
> > pressure sensors.
> > So most touch ICs simply can't report pressure. I think
> > ABS_MT_PRESSURE in general shouldn't even be reported, partially
> > because of the lack of appropriate sensors, and partially because of
> > the lack of explanation of what these values means (no resolution).
> > For pressure, the userspace doesn't know what to do with it, and has
> > to guess about what it means. As a result, on Android for example, we
> > use pressure in an "on-off" fashion - zero pressure means hover and
> > non-zero pressure means contact. Arguably, that's misusing the APIs.
>
> What I was trying to argue was whether or not it makes sense to define
> any axes at all, if some axes can't possibly be defined (i.e., all or
> nothing).
>
> However, you're absolutely correct that the problem statement doesn't
> apply to ABS_MT_PRESSURE because we do not generally use touch surfaces
> to report granular pressure. In fact, most controllers I'm aware of that
> do report a pressure value are actually just reporting area anyway.
>
> >
> > In the case of major and minor axes, it's very clear how they should
> > be used. There's a well-defined way to calibrate these values to
> > physical units. You can put an oval object on the screen, calculate
> > its dimensions using a caliper, and then look at the size of the oval
> > in the capacitive data.. So the devices *can* provide an accurate
> > value here if they wanted.
>
> Agreed.
>
> >
> > Do you mind explaning a bit more about how your proposal would work?
> > Would the user space have to scrape the linux folders in order to find
> > this new property that we would define to report the resolution,
> > rather than using ioctls to read the existing value from the fd of
> > /dev/input/eventX?
>
> Yes, that's what I had in mind, but it's admittedly overkill here.
>
> >
> > With the current approach, my expectation is that the touch driver
> > could certainly use the dts to read out some values like screen size,
> > scale factor, etc., but it would then be responsible to set the
> > resolutions accordingly and to scale these values as needed when
> > reporting to user space.
>
> Yup, looks like your idea is the best of both worlds. In fact I see we
> already have touchscreen-x-mm and touchscreen-y-mm defined in common
> touchscreen bindings, albeit not many drivers are using them yet.
>
> For what it's worth, as a fellow customer of input:
>
> Reviewed-by: Jeff LaBundy <jeff@xxxxxxxxxxx>
>
> Thanks for the great discussion!
>
> >
> >
> > On Fri, May 20, 2022 at 9:00 AM Jeff LaBundy <jeff@xxxxxxxxxxx> wrote:
> > >
> > > Hi Siarhei,
> > >
> > > On Fri, May 20, 2022 at 01:45:14AM -0700, Siarhei Vishniakou wrote:
> > > > Today, the resolution of size axes is not documented. As a result, it's
> > > > not clear what the canonical interpretation of this value should be. On
> > > > Android, there is a need to calculate the size of the touch ellipse in
> > > > physical units (millimeters).
> > > >
> > > > After reviewing linux source, it turned out that most of the existing
> > > > usages are already interpreting this value as "units/mm". This
> > > > documentation will make it explicit. This will help device
> > > > implementations with correctly following the linux specs, and will
> > > > ensure that the devices will work on Android without needing further
> > > > customized parameters for scaling of major/minor values.
> > > >
> > > > Signed-off-by: Siarhei Vishniakou <svv@xxxxxxxxxx>
> > > > Change-Id: I4a2de9e6d02e5fd707e5d312f5c3325734266a6e
> > > > ---
> > > > include/uapi/linux/input.h | 11 +++++++----
> > > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> > > > index ee3127461ee0..328cf545c029 100644
> > > > --- a/include/uapi/linux/input.h
> > > > +++ b/include/uapi/linux/input.h
> > > > @@ -78,10 +78,13 @@ struct input_id {
> > > > * Note that input core does not clamp reported values to the
> > > > * [minimum, maximum] limits, such task is left to userspace.
> > > > *
> > > > - * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z)
> > > > - * is reported in units per millimeter (units/mm), resolution
> > > > - * for rotational axes (ABS_RX, ABS_RY, ABS_RZ) is reported
> > > > - * in units per radian.
> > > > + * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z,
> > > > + * ABS_MT_POSITION_X, ABS_MT_POSITION_Y) is reported in units
> > > > + * per millimeter (units/mm), resolution for rotational axes
> > > > + * (ABS_RX, ABS_RY, ABS_RZ) is reported in units per radian.
> > > > + * The resolution for the size axes (ABS_MT_TOUCH_MAJOR,
> > > > + * ABS_MT_TOUCH_MINOR, ABS_MT_WIDTH_MAJOR, ABS_MT_WIDTH_MINOR)
> > > > + * is reported in units per millimeter (units/mm).
> > > > * When INPUT_PROP_ACCELEROMETER is set the resolution changes.
> > > > * The main axes (ABS_X, ABS_Y, ABS_Z) are then reported in
> > > > * units per g (units/g) and in units per degree per second
> > > > --
> > > > 2.36.1.124.g0e6072fb45-goog
> > > >
> > >
> > > Thanks for raising this point; it's a valid one. However, I'm not
> > > convinced this is the right approach.
> > >
> > > On all the controllers I've worked on, ABS_X and ABS_Y are mapped
> > > to arbitrary resolution values that don't necessarily map to real-
> > > world units. I don't think we can make any assumption at the input
> > > layer as to the physical size of the touch surface.
> > >
> > > It is the same problem for ABS_MT_PRESSURE; the values are typically
> > > controller-specific and we can't reasonably try to map this axis to
> > > any standard unit (e.g. Pascals).
> > >
> > > If user space needs to understand the mapping between axis range and
> > > physical size, maybe it is better to adopt the approach from the IIO
> > > subsystem wherein the input_dev offers a property that maps each axis
> > > (i.e. "raw" value) to some SI unit?
> > >
> > > In that case, dts could define the scaling factor between raw values
> > > and physical dimensions. At any rate, that is just my $.02.
> > >
> > > Kind regards,
> > > Jeff LaBundy
>
> Kind regards,
> Jeff LaBundy
