diff mbox

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

Message ID 1313169407-4358-5-git-send-email-djkurtz@chromium.org (mailing list archive)
State Superseded
Headers show

Commit Message

Daniel Kurtz Aug. 12, 2011, 5:16 p.m. UTC 
Synaptics makes (at least) two kinds of touchpad sensors:
 * Older pads use a profile sensor that could only infer the location
   of individual fingers based on the projection of their profiles
   onto row and column sensors.
 * Newer pads use an image sensor that can track true finger position
   using a two-dimensional sensor grid.

Both sensor types support an "Advanced Gesture Mode":
 When multiple fingers are detected, the touchpad sends alternating
 "Advanced Gesture Mode" (AGM) and "Simple Gesture Mode" (SGM)
 packets.
 The AGM packets have w=2, and contain reduced resolution finger data
 The SGM packets have w={0,1} and contain full resolution finger data

Profile sensors try to report the "upper" (larger y value) finger in
the SGM packet, and the lower (smaller y value) in the AGM packet.
However, due to the nature of the profile sensor, they easily get
confused when fingers cross, and can start reporting the x-coordinate
of one with the y-coordinate of the other.  Thus, for profile
sensors, "semi-mt" was created, which reports a "bounding box"
created by pairing min and max coordinates of the two pairs of
reported fingers.

Image sensors can report the actual coordinates of two of the fingers
present.  This patch detects if the touchpad is an image sensor and
reports finger data using the MT-B protocol.

NOTE: This patch only adds partial support for 2-finger gestures.
      The proper interpretation of the slot contents when more than
      two fingers are present is left to later patches.  Also,
      handling of 'number of fingers' transitions is incomplete.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c |  104 +++++++++++++++++++++++++++++++++++++-
 drivers/input/mouse/synaptics.h |    3 +
 2 files changed, 104 insertions(+), 3 deletions(-)

Comments

Henrik Rydberg Aug. 12, 2011, 9:09 p.m. UTC | #1 
Hi Daniel,

looks good, some details below.

> +static void synaptics_report_slot_empty(struct input_dev *dev, int slot)
> +{
> +	input_mt_slot(dev, slot);
> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
> +}
> +
> +static void synaptics_report_slot_sgm(struct input_dev *dev, int slot,
> +				      const struct synaptics_hw_state *sgm)
> +{
> +	input_mt_slot(dev, slot);
> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> +	input_report_abs(dev, ABS_MT_POSITION_X, sgm->x);
> +	input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(sgm->y));
> +	input_report_abs(dev, ABS_MT_PRESSURE, sgm->z);
> +	/* SGM can sometimes contain valid width */
> +	if (sgm->w >= 4)
> +		input_report_abs(dev, ABS_MT_TOUCH_MAJOR, sgm->w);
> +}

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...

> +
> +static void synaptics_report_mt(struct psmouse *psmouse,
> +				int count,
> +				const struct synaptics_hw_state *sgm)
> +{
> +	struct input_dev *dev = psmouse->dev;
> +	struct synaptics_data *priv = psmouse->private;
> +	struct synaptics_hw_state *agm = &priv->agm;
> +
> +	switch (count) {
> +	case 0:
> +		synaptics_report_slot_empty(dev, 0);
> +		synaptics_report_slot_empty(dev, 1);
> +		break;
> +	case 1:
> +		synaptics_report_slot_sgm(dev, 0, sgm);
> +		synaptics_report_slot_empty(dev, 1);
> +		break;
> +	case 2:
> +	case 3: /* Fall-through case */
> +		synaptics_report_slot_sgm(dev, 0, sgm);
> +		synaptics_report_slot_agm(dev, 1, agm);
> +		break;
> +	}
> +
> +	/* Don't use active slot count to generate BTN_TOOL events. */
> +	input_mt_report_pointer_emulation(dev, false);
> +
> +	/* Send the number of fingers reported by touchpad itself. */
> +	input_mt_report_finger_count(dev, count);
> +
> +	input_report_key(dev, BTN_LEFT, sgm->left);
> +	input_sync(dev);
> +}
> +
> +static void synaptics_image_sensor_process(struct psmouse *psmouse,
> +					   struct synaptics_hw_state *sgm)
> +{
> +	int count;
> +
> +	if (sgm->z == 0)
> +		count = 0;
> +	else if (sgm->w >= 4)
> +		count = 1;
> +	else if (sgm->w == 0)
> +		count = 2;
> +	else
> +		count = 3;
> +
> +	/* Send resulting input events to user space */
> +	synaptics_report_mt(psmouse, count, sgm);
> +}
> +
>  /*
>   *  called for each full received packet from the touchpad
>   */
> @@ -558,6 +642,11 @@ static void synaptics_process_packet(struct psmouse *psmouse)
>  	if (synaptics_parse_hw_state(psmouse->packet, priv, &hw))
>  		return;
>  
> +	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
> +		synaptics_image_sensor_process(psmouse, &hw);
> +		return;
> +	}
> +
>  	if (hw.scroll) {
>  		priv->scroll += hw.scroll;
>  
> @@ -739,7 +828,16 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>  	set_abs_position_params(dev, priv, ABS_X, ABS_Y);
>  	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
>  
> -	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
> +	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
> +		input_mt_init_slots(dev, 2);
> +		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
> +					ABS_MT_POSITION_Y);
> +		/* Image sensors can report per-contact pressure */
> +		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +		/* Image sensors can sometimes report per-contact width */
> +		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> +	} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
> +		/* Non-image sensors with AGM use semi-mt */
>  		__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
>  		input_mt_init_slots(dev, 2);
>  		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index a9efbf3..0ea7616 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -74,6 +74,8 @@
>   * 2	0x04	reduced filtering	firmware does less filtering on
>   *					position data, driver should watch
>   *					for noise.
> + * 2	0x08	image sensor		image sensor tracks 5 fingers, but only
> + *					reports 2.
>   * 2	0x20	report min		query 0x0f gives min coord reported
>   */
>  #define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100000) /* 1-button ClickPad */
> @@ -82,6 +84,7 @@
>  #define SYN_CAP_MIN_DIMENSIONS(ex0c)	((ex0c) & 0x002000)
>  #define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
>  #define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & 0x000400)
> +#define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & 0x000800)
>  
>  /* synaptics modes query bits */
>  #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
> -- 
> 1.7.3.1
> 

Looks good otherwise.

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Kurtz Aug. 15, 2011, 7:17 a.m. UTC | #2 
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.
>
>> +static void synaptics_report_slot_empty(struct input_dev *dev, int slot)
>> +{
>> +     input_mt_slot(dev, slot);
>> +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
>> +}
>> +
>> +static void synaptics_report_slot_sgm(struct input_dev *dev, int slot,
>> +                                   const struct synaptics_hw_state *sgm)
>> +{
>> +     input_mt_slot(dev, slot);
>> +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
>> +     input_report_abs(dev, ABS_MT_POSITION_X, sgm->x);
>> +     input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(sgm->y));
>> +     input_report_abs(dev, ABS_MT_PRESSURE, sgm->z);
>> +     /* SGM can sometimes contain valid width */
>> +     if (sgm->w >= 4)
>> +             input_report_abs(dev, ABS_MT_TOUCH_MAJOR, sgm->w);
>> +}
>
> 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.

I will rework these patches to do this.

Thanks,
-Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Kurtz Aug. 17, 2011, 2:05 p.m. UTC | #3 
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.

Do you agree with this?

Thanks,
-Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Aug. 17, 2011, 4:32 p.m. UTC | #4 
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.
Daniel Kurtz Aug. 17, 2011, 4:47 p.m. UTC | #5 
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.
>
> --
> Dmitry

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".
Does it therefore sound reasonable to report ABS_MT_PRESSURE?

-Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Aug. 17, 2011, 5:34 p.m. UTC | #6 
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.
diff mbox

Patch

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 30d05fd..a9960d2 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -304,7 +304,8 @@  static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
 	static unsigned char param = 0xc8;
 	struct synaptics_data *priv = psmouse->private;
 
-	if (!SYN_CAP_ADV_GESTURE(priv->ext_cap_0c))
+	if (!(SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) ||
+			SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)))
 		return 0;
 
 	if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL))
@@ -463,7 +464,9 @@  static int synaptics_parse_hw_state(const unsigned char buf[],
 			hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0;
 		}
 
-		if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) && hw->w == 2) {
+		if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)
+				|| SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
+				&& hw->w == 2) {
 			synaptics_parse_agm(buf, priv);
 			return 1;
 		}
@@ -543,6 +546,87 @@  static void synaptics_report_semi_mt_data(struct input_dev *dev,
 	}
 }
 
+static void synaptics_report_slot_empty(struct input_dev *dev, int slot)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
+}
+
+static void synaptics_report_slot_sgm(struct input_dev *dev, int slot,
+				      const struct synaptics_hw_state *sgm)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
+	input_report_abs(dev, ABS_MT_POSITION_X, sgm->x);
+	input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(sgm->y));
+	input_report_abs(dev, ABS_MT_PRESSURE, sgm->z);
+	/* SGM can sometimes contain valid width */
+	if (sgm->w >= 4)
+		input_report_abs(dev, ABS_MT_TOUCH_MAJOR, sgm->w);
+}
+
+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);
+}
+
+static void synaptics_report_mt(struct psmouse *psmouse,
+				int count,
+				const struct synaptics_hw_state *sgm)
+{
+	struct input_dev *dev = psmouse->dev;
+	struct synaptics_data *priv = psmouse->private;
+	struct synaptics_hw_state *agm = &priv->agm;
+
+	switch (count) {
+	case 0:
+		synaptics_report_slot_empty(dev, 0);
+		synaptics_report_slot_empty(dev, 1);
+		break;
+	case 1:
+		synaptics_report_slot_sgm(dev, 0, sgm);
+		synaptics_report_slot_empty(dev, 1);
+		break;
+	case 2:
+	case 3: /* Fall-through case */
+		synaptics_report_slot_sgm(dev, 0, sgm);
+		synaptics_report_slot_agm(dev, 1, agm);
+		break;
+	}
+
+	/* Don't use active slot count to generate BTN_TOOL events. */
+	input_mt_report_pointer_emulation(dev, false);
+
+	/* Send the number of fingers reported by touchpad itself. */
+	input_mt_report_finger_count(dev, count);
+
+	input_report_key(dev, BTN_LEFT, sgm->left);
+	input_sync(dev);
+}
+
+static void synaptics_image_sensor_process(struct psmouse *psmouse,
+					   struct synaptics_hw_state *sgm)
+{
+	int count;
+
+	if (sgm->z == 0)
+		count = 0;
+	else if (sgm->w >= 4)
+		count = 1;
+	else if (sgm->w == 0)
+		count = 2;
+	else
+		count = 3;
+
+	/* Send resulting input events to user space */
+	synaptics_report_mt(psmouse, count, sgm);
+}
+
 /*
  *  called for each full received packet from the touchpad
  */
@@ -558,6 +642,11 @@  static void synaptics_process_packet(struct psmouse *psmouse)
 	if (synaptics_parse_hw_state(psmouse->packet, priv, &hw))
 		return;
 
+	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
+		synaptics_image_sensor_process(psmouse, &hw);
+		return;
+	}
+
 	if (hw.scroll) {
 		priv->scroll += hw.scroll;
 
@@ -739,7 +828,16 @@  static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	set_abs_position_params(dev, priv, ABS_X, ABS_Y);
 	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
 
-	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
+	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
+		input_mt_init_slots(dev, 2);
+		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
+					ABS_MT_POSITION_Y);
+		/* Image sensors can report per-contact pressure */
+		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
+		/* Image sensors can sometimes report per-contact width */
+		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
+	} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
+		/* Non-image sensors with AGM use semi-mt */
 		__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
 		input_mt_init_slots(dev, 2);
 		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index a9efbf3..0ea7616 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -74,6 +74,8 @@ 
  * 2	0x04	reduced filtering	firmware does less filtering on
  *					position data, driver should watch
  *					for noise.
+ * 2	0x08	image sensor		image sensor tracks 5 fingers, but only
+ *					reports 2.
  * 2	0x20	report min		query 0x0f gives min coord reported
  */
 #define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100000) /* 1-button ClickPad */
@@ -82,6 +84,7 @@ 
 #define SYN_CAP_MIN_DIMENSIONS(ex0c)	((ex0c) & 0x002000)
 #define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
 #define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & 0x000400)
+#define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & 0x000800)
 
 /* synaptics modes query bits */
 #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))