Re: [PATCH 4/8 v3] Input: synaptics - add image sensor support

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Thu, Aug 18, 2011 at 12:43:39AM +0800, Daniel Kurtz wrote:
> On Thu, Aug 18, 2011 at 12:32 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com
> > wrote:
> 
> > On Wed, Aug 17, 2011 at 10:05:48PM +0800, Daniel Kurtz wrote:
> > > On Mon, Aug 15, 2011 at 3:17 PM, Daniel Kurtz <djkurtz@chromium.org>
> > wrote:
> > > >
> > > > Hi Henrik,
> > > >
> > > > Thanks for the great feedback.
> > > >
> > > > On Sat, Aug 13, 2011 at 5:09 AM, Henrik Rydberg <rydberg@euromail.se>
> > wrote:
> > > > > Hi Daniel,
> > > > >
> > > > > looks good, some details below.
> > > > >
> > > > > The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain
> > > > > compatible with user space handling of type A devices. Also, the
> > scale
> > > > > should match the screen/pad size, such that the actual size of the
> > > > > touch area can be deduced. In addition, based on the current sensor
> > > > > technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally
> > > > > mutually exclusive.
> > > > >
> > > > > All in all, I would prefer to only report width via (the
> > single-finger
> > > > > axis) ABS_TOOL_WIDTH, and only for compatibility reasons.
> > > > >
> > > > > +
> > > > > +static void synaptics_report_slot_agm(struct input_dev *dev, int
> > slot,
> > > > > +                                   const struct synaptics_hw_state
> > *agm)
> > > > > +{
> > > > > +     input_mt_slot(dev, slot);
> > > > > +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> > > > > +     input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
> > > > > +     input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
> > > > > +     input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
> > > > > +}
> > > >
> > > > With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide...
> > > > ....
> > > > >
> > > > > Looks good otherwise.
> > > > >
> > > > > Thanks,
> > > > > Henrik
> > > > >
> > > >
> > > > Ahh....  ok, this makes sense.  I missed this detail previously.
> > > > Basically:
> > > >  * Only report synaptics "w" as ABS_TOOL_WIDTH (for backwards
> > compatibility)
> > > >  * Report synaptics "z" as ABS_MT_TOUCH_MAJOR (assuming it is really
> > > > 'width of finger in surface units'), and ABS_PRESSURE (for backwards
> > > > compatibility).
> > > >  * Don't use ABS_MT_PRESSURE.
> > > >  * This will simplify agm/sgm reporting by making them both reoprt
> > > > the same axes.
> > >
> > > Hi Henrik,
> > >
> > > I think I was wrong in my previous comment.
> > >
> > > I agree we should drop reporting synaptics 'w' (4-15, arbitrary units)
> > > in ABS_MT_TOUCH_MAJOR.
> > > Instead, to support legacy, report 'w' in ABS_TOOL_WIDTH, and do not
> > > report ABS_MT_TOUCH_MAJOR at all.
> > >
> > > However, I believe we should still report synaptics 'z' (0-255, in
> > > arbitrary units) as ABS_MT_PRESSURE.
> > > This will, in turn, be used by input_mt_report_pointer_emulation() to
> > > determine the correct ABS_PRESSURE to report for legacy userspace
> > > uses.
> >
> > Alternatively, you can use input_mt_set_value() to save Z value to be
> > used by input_mt_report_pointer_emulation() later, but not report it as
> > part of MT packet.
> >
> 
> But I want userspace to get per-contact pressure, since it is used to
> determine whether a given contact is a palm or finger, etc. The trackpad
> does not report a useful per-contact "width" that can be used to generate
> "TOUCH_MAJOR".

If the data reported is indeed pressure and it is indeed per-contact then yes,
it makes sense to report it as such. If the data reported is more like contact
size then report it as ABS_MT_TOUCH_MAJOR and use input_mt_set_value()
to "stash" it for legacy emulation.

-- 
Dmitry