diff mbox

[10/12] Input: synaptics - decode AGM packet types

Message ID 1309324042-22943-11-git-send-email-djkurtz@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Kurtz June 29, 2011, 5:07 a.m. UTC
From: Daniel Kurtz <djkurtz@chromium.org>

A Synaptics image sensor tracks 5 fingers, but can only report 2.
This behavior is called "T5R2" = Track 5 Report 2

Algorithm for choosing which 2 fingers to report in which packet:
  Touchpad maintains 5 slots, numbered 0 to 4.
  Initially all slots are empty.
  As new fingers are detected, they are assigned the lowest available
  slot.
  Touchpad always reports:
    SGM: lowest numbered non-empty slot
    AGM: highest numbered non-empty slot, if there is one.

In addition, T5R2 touchpads have a special AGM packet type which reports
the number of fingers currently being tracked, and which finger is in
each of the two slots.  Unfortunately, these "TYPE=2" packets are only used
when more than 3 fingers are being tracked.  When less than 4 fingers
are present, the 'w' value must be used to track how many fingers are
present, and knowing which fingers are being reported is much more

Comments

Chase Douglas June 29, 2011, 10:02 a.m. UTC | #1
On 06/29/2011 06:07 AM, djkurtz@chromium.org wrote:
> From: Daniel Kurtz <djkurtz@chromium.org>
> 
> A Synaptics image sensor tracks 5 fingers, but can only report 2.
> This behavior is called "T5R2" = Track 5 Report 2
> 
> Algorithm for choosing which 2 fingers to report in which packet:
>   Touchpad maintains 5 slots, numbered 0 to 4.
>   Initially all slots are empty.
>   As new fingers are detected, they are assigned the lowest available
>   slot.
>   Touchpad always reports:
>     SGM: lowest numbered non-empty slot
>     AGM: highest numbered non-empty slot, if there is one.

Hi Daniel,

Thanks for the new patches! I will review them soon in detail, but I
would like to first confirm some details about the above.

In the older "profile" devices, we got essentially the bounding box of
all touches. This allows us to detect pinch gestures between the two
most extreme touches when there are three touches on the touchpad.

The above description makes it sound like we will no longer have a
bounding box of touches. According to the description, if I put four
fingers down in a square formation and then touch a fifth finger in the
middle of the square, I will only see the locations of one corner and
the middle of the square. This would make meaningful gesture detection
beyond two touches nearly impossible. Is this really how the touchpad
works, or is the above description not quite right?

Thanks!

-- Chase
--
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 Stone June 29, 2011, 10:07 a.m. UTC | #2
Hi Chase,

On Wed, Jun 29, 2011 at 11:02:53AM +0100, Chase Douglas wrote:
> In the older "profile" devices, we got essentially the bounding box of
> all touches. This allows us to detect pinch gestures between the two
> most extreme touches when there are three touches on the touchpad.

This isn't necessarily true for all pads.  If you look at Derek's
patchset from a couple of weeks ago, it was for a profile sensor which
had the same reporting behaviour.  Internally, the touchpad could track
many fingers, but would always report the first (in SGM packet) and most
recent (in AGM packet) fingers.  You could fake a bounding-box
calculation, but it would be the bounding box of fingers 0 and n, rather
than the bounding box of all fingers.

> The above description makes it sound like we will no longer have a
> bounding box of touches. According to the description, if I put four
> fingers down in a square formation and then touch a fifth finger in the
> middle of the square, I will only see the locations of one corner and
> the middle of the square. This would make meaningful gesture detection
> beyond two touches nearly impossible. Is this really how the touchpad
> works, or is the above description not quite right?

Yes, I believe that's how the touchpad works.  It would be nice to get
all fingers, but unfortunately PS/2 doesn't give us enough bandwidth to
do that.

Cheers,
Daniel
--
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
Chase Douglas June 29, 2011, 10:32 a.m. UTC | #3
On 06/29/2011 11:07 AM, Daniel Stone wrote:
> Hi Chase,
> 
> On Wed, Jun 29, 2011 at 11:02:53AM +0100, Chase Douglas wrote:
>> In the older "profile" devices, we got essentially the bounding box of
>> all touches. This allows us to detect pinch gestures between the two
>> most extreme touches when there are three touches on the touchpad.
> 
> This isn't necessarily true for all pads.  If you look at Derek's
> patchset from a couple of weeks ago, it was for a profile sensor which
> had the same reporting behaviour.  Internally, the touchpad could track
> many fingers, but would always report the first (in SGM packet) and most
> recent (in AGM packet) fingers.  You could fake a bounding-box
> calculation, but it would be the bounding box of fingers 0 and n, rather
> than the bounding box of all fingers.

Are we really sure that Derek's trackpad was a profile device? I mean
*really* sure. Because if Synaptics is putting out two different devices
that are identical in protocol but behave differently, then that's a
*big* problem for us gesture developers :).

I would be surprised if a profile device were providing the first and
third touch locations as opposed to the bounding box because then
there's no difference between it and this new "image sensor" device type.

>> The above description makes it sound like we will no longer have a
>> bounding box of touches. According to the description, if I put four
>> fingers down in a square formation and then touch a fifth finger in the
>> middle of the square, I will only see the locations of one corner and
>> the middle of the square. This would make meaningful gesture detection
>> beyond two touches nearly impossible. Is this really how the touchpad
>> works, or is the above description not quite right?
> 
> Yes, I believe that's how the touchpad works.  It would be nice to get
> all fingers, but unfortunately PS/2 doesn't give us enough bandwidth to
> do that.

If this is true for these new image devices, then we'll need to set a
different property bit other than SEMI_MT (maybe FIRST_LAST_MT? I'm not
good with names :). I'm thinking we should disable gesture support for
these trackpads in uTouch when there are more than two touches because
we can't be sure what is going on in any meaningful way. The bounding
box is more useful than this, which is really sad.

-- Chase
--
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 June 29, 2011, 11:04 a.m. UTC | #4
Hi Chase,

Thanks for looking at the patches!

On Wed, Jun 29, 2011 at 6:02 PM, Chase Douglas
<chase.douglas@canonical.com> wrote:
>
> On 06/29/2011 06:07 AM, djkurtz@chromium.org wrote:
> > From: Daniel Kurtz <djkurtz@chromium.org>
> >
> > A Synaptics image sensor tracks 5 fingers, but can only report 2.
> > This behavior is called "T5R2" = Track 5 Report 2
> >
> > Algorithm for choosing which 2 fingers to report in which packet:
> >   Touchpad maintains 5 slots, numbered 0 to 4.
> >   Initially all slots are empty.
> >   As new fingers are detected, they are assigned the lowest available
> >   slot.
> >   Touchpad always reports:
> >     SGM: lowest numbered non-empty slot
> >     AGM: highest numbered non-empty slot, if there is one.
>
> Hi Daniel,
>
> Thanks for the new patches! I will review them soon in detail, but I
> would like to first confirm some details about the above.
>
> In the older "profile" devices, we got essentially the bounding box of
> all touches. This allows us to detect pinch gestures between the two
> most extreme touches when there are three touches on the touchpad.
>
> The above description makes it sound like we will no longer have a
> bounding box of touches. According to the description, if I put four
> fingers down in a square formation and then touch a fifth finger in the
> middle of the square, I will only see the locations of one corner and
> the middle of the square. This would make meaningful gesture detection
> beyond two touches nearly impossible. Is this really how the touchpad
> works, or is the above description not quite right?

This is how the image sensor works, from my observations.

As for profile sensors the kernel driver creates a "best guess
bounding box" by picking:
   top_left = (min(x1,x2), min(y1,y2))
   bottom_right = (max(x1,x2), max(y1,y2))

However, at least from my experiments with one profile sensor, there
is no guarantee that this rectangle actually contains all touches on
the pad.
In fact, I think it is trying really hard to return the position of
the first two touches that it is tracking.
When there are two fingers, it returns the finger with the larger
y-value (and what the trackpad believes to be its x-), and the SGM
packet, and the other finger in the AGM.
When there are three or more fingers, it still reports the first two,
irregardless of where the new fingers are added.

It would be very curious if different profile sensors behaved differently.

In any case, this patch should not change the behavior of profile
sensors, since it only applies to image sensors.

Thanks,
-Daniel
--
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 June 29, 2011, 11:26 a.m. UTC | #5
Hi Daniel, Chase,

On Wed, Jun 29, 2011 at 6:32 PM, Chase Douglas
<chase.douglas@canonical.com> wrote:
> On 06/29/2011 11:07 AM, Daniel Stone wrote:
>> Hi Chase,
>>
>> On Wed, Jun 29, 2011 at 11:02:53AM +0100, Chase Douglas wrote:
>>> In the older "profile" devices, we got essentially the bounding box of
>>> all touches. This allows us to detect pinch gestures between the two
>>> most extreme touches when there are three touches on the touchpad.
>>
>> This isn't necessarily true for all pads.  If you look at Derek's
>> patchset from a couple of weeks ago, it was for a profile sensor which
>> had the same reporting behaviour.  Internally, the touchpad could track
>> many fingers, but would always report the first (in SGM packet) and most
>> recent (in AGM packet) fingers.  You could fake a bounding-box
>> calculation, but it would be the bounding box of fingers 0 and n, rather
>> than the bounding box of all fingers.
>
> Are we really sure that Derek's trackpad was a profile device? I mean
> *really* sure. Because if Synaptics is putting out two different devices
> that are identical in protocol but behave differently, then that's a
> *big* problem for us gesture developers :).
>
> I would be surprised if a profile device were providing the first and
> third touch locations as opposed to the bounding box because then
> there's no difference between it and this new "image sensor" device type.

I agree with Chase, the 0:n box behavior sounds more like image sensor
than profile sensor.
However, as I just mentioned in the previous email, I have looked at a
profile sensor that tries to report first and second finger positions,
not a bounding box.
In any case, the result is:
You end up computing (usually) a 0:1 "bounding box", with n >= 2
fingers on the pad.
I say usually because it is pretty easy to confuse the finger tracking
of a profile sensor.

>>> The above description makes it sound like we will no longer have a
>>> bounding box of touches. According to the description, if I put four
>>> fingers down in a square formation and then touch a fifth finger in the
>>> middle of the square, I will only see the locations of one corner and
>>> the middle of the square. This would make meaningful gesture detection
>>> beyond two touches nearly impossible. Is this really how the touchpad
>>> works, or is the above description not quite right?
>>
>> Yes, I believe that's how the touchpad works.  It would be nice to get
>> all fingers, but unfortunately PS/2 doesn't give us enough bandwidth to
>> do that.
>
> If this is true for these new image devices, then we'll need to set a
> different property bit other than SEMI_MT (maybe FIRST_LAST_MT? I'm not
> good with names :). I'm thinking we should disable gesture support for
> these trackpads in uTouch when there are more than two touches because
> we can't be sure what is going on in any meaningful way. The bounding
> box is more useful than this, which is really sad.
>
> -- Chase

This is exactly what this patch set does by defining the T5R2 property.

It is very challenging to support any 3+ finger gestures with a
touchpad that only reports two fingers... whether it is semi-mt or
T5R2.
You must always make an assumption about where those hidden fingers are.
But, that's a problem for userspace... :)

-Daniel
--
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
chris@cnpbagwell.com July 5, 2011, 6:55 p.m. UTC | #6
On Tue, Jul 5, 2011 at 1:17 PM, Chase Douglas
<chase.douglas@canonical.com> wrote:
> On 06/28/2011 10:07 PM, djkurtz@chromium.org wrote:
>> From: Daniel Kurtz <djkurtz@chromium.org>
>>
>> A Synaptics image sensor tracks 5 fingers, but can only report 2.
>> This behavior is called "T5R2" = Track 5 Report 2
>>
>> Algorithm for choosing which 2 fingers to report in which packet:
>>   Touchpad maintains 5 slots, numbered 0 to 4.
>>   Initially all slots are empty.
>>   As new fingers are detected, they are assigned the lowest available
>>   slot.
>>   Touchpad always reports:
>>     SGM: lowest numbered non-empty slot
>>     AGM: highest numbered non-empty slot, if there is one.
>>
>> In addition, T5R2 touchpads have a special AGM packet type which reports
>> the number of fingers currently being tracked, and which finger is in
>> each of the two slots.  Unfortunately, these "TYPE=2" packets are only used
>> when more than 3 fingers are being tracked.  When less than 4 fingers
>> are present, the 'w' value must be used to track how many fingers are
>> present, and knowing which fingers are being reported is much more
>> difficult, if not impossible.
>>
>> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>> ---
>>  drivers/input/mouse/synaptics.c |   39 ++++++++++++++++++++++++++++++++++-----
>>  drivers/input/mouse/synaptics.h |    7 ++++++-
>>  include/linux/input.h           |    1 +
>>  3 files changed, 41 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> index 2d7ac0a..19a9b7f 100644
>> --- a/drivers/input/mouse/synaptics.c
>> +++ b/drivers/input/mouse/synaptics.c
>> @@ -401,6 +401,14 @@ static void synaptics_pt_create(struct psmouse *psmouse)
>>  /*****************************************************************************
>>   *   Functions to interpret the absolute mode packets
>>   ****************************************************************************/
>> +/* Set AGM-CONTACT finger state */
>> +static void synaptics_agm_finger_update(struct synaptics_data *priv, int count,
>> +                                     int sgm, int agm)
>> +{
>> +     priv->agm.finger_count = count;
>> +     priv->agm.finger_sgm = sgm;
>> +     priv->agm.finger_agm = agm;
>> +}
>>
>>  static int synaptics_parse_hw_state(const unsigned char buf[],
>>                                   struct synaptics_data *priv,
>> @@ -438,11 +446,31 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>>               if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)
>>                               || SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
>>                               && hw->w == 2) {
>> -                     /* Gesture packet: (x, y, z) at half resolution */
>> -                     priv->agm.x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
>> -                     priv->agm.y = INVERT_Y((((buf[4] & 0xf0) << 4)
>> -                                           | buf[2]) << 1);
>> -                     priv->agm.z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
>> +                     int type; /* Packet type */
>> +
>> +                     type = (buf[5] & 0x30) >> 4;
>> +
>> +                     switch (type) {
>> +                     case 1:
>> +                             /* Gesture packet: (x, y, z) half resolution */
>> +                             priv->agm.w = hw->w;
>> +                             priv->agm.x = (((buf[4] & 0x0f) << 8)
>> +                                             | buf[1]) << 1;
>> +                             priv->agm.y = INVERT_Y((((buf[4] & 0xf0) << 4)
>> +                                                    | buf[2]) << 1);
>> +                             priv->agm.z = ((buf[3] & 0x30)
>> +                                             | (buf[5] & 0x0f)) << 1;
>> +                             break;
>> +
>> +                     case 2:
>> +                             /* Finger slot update */
>> +                             synaptics_agm_finger_update(priv, buf[1],
>> +                                                         buf[2], buf[4]);
>> +                             break;
>> +
>> +                     default:
>> +                             break;
>> +                     }
>>                       return 1;
>>               } else {
>>                       hw->x = (((buf[3] & 0x10) << 8) |
>> @@ -804,6 +832,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>>       input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
>>
>>       if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
>> +             __set_bit(INPUT_PROP_SYNAPTICS_T5R2, dev->propbit);
>>               input_mt_init_slots(dev, SYN_TRACK_SLOT_COUNT);
>>               input_set_abs_params(dev, ABS_MT_POSITION_X, XMIN_NOMINAL,
>>                                    priv->x_max ?: XMAX_NOMINAL, fuzz, 0);
>> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
>> index 1de2256..2214af6 100644
>> --- a/drivers/input/mouse/synaptics.h
>> +++ b/drivers/input/mouse/synaptics.h
>> @@ -122,7 +122,7 @@
>>  #define SYN_SLOT_AGM                 2
>>
>>  /* number of tracking slots for Image Sensor firmware */
>> -#define SYN_TRACK_SLOT_COUNT         2
>> +#define SYN_TRACK_SLOT_COUNT         5
>>
>>  /*
>>   * A structure to describe the state of the touchpad hardware (buttons and pad)
>> @@ -140,6 +140,11 @@ struct synaptics_hw_state {
>>       unsigned int down:1;
>>       unsigned char ext_buttons;
>>       signed char scroll;
>> +
>> +     /* Reported in AGM-CONTACT packets */
>> +     unsigned int finger_count;              /* num fingers being tracked */
>> +     unsigned int finger_sgm;                /* finger described by sgm */
>> +     unsigned int finger_agm;                /* finger described by agm */
>>  };
>>
>>  struct synaptics_data {
>> diff --git a/include/linux/input.h b/include/linux/input.h
>> index 771d6d8..732c14e 100644
>> --- a/include/linux/input.h
>> +++ b/include/linux/input.h
>> @@ -137,6 +137,7 @@ struct input_keymap_entry {
>>  #define INPUT_PROP_DIRECT            0x01    /* direct input devices */
>>  #define INPUT_PROP_BUTTONPAD         0x02    /* has button(s) under pad */
>>  #define INPUT_PROP_SEMI_MT           0x03    /* touch rectangle only */
>> +#define INPUT_PROP_SYNAPTICS_T5R2    0x04    /* synaptics track 5 report 2 */
>>
>>  #define INPUT_PROP_MAX                       0x1f
>>  #define INPUT_PROP_CNT                       (INPUT_PROP_MAX + 1)
>
> I'm trying to understand these later patches for T5R2. There are
> "hidden" touch slots now. How does userspace tell whether a touch slot
> is a hidden touch?
>

I'm trying to understand as well.  If I was a consumer of events and I
knew touchpad supports 2 touch tracking but 5 finger detection, I'd
assume:

* ABS_X/Y reports oldest touch and
BTN_TOOL_DOUBLETAP/TRIPLETAP/QUADTAP reports # of fingers touching
(ignoring lost of 5 finger for now).
* ABS_MT_* only contains 2 slots and without SEMI_MT declared.
BTN_TOOL_* could be used to detect greater than 2 finger touches.

I'm not sure I could guess what to do with 5 slots and how to expect
touch data to come over them.

Where you saying in the other thread that the 5 slots are to help user
detect finger transitions and re-act bounces in X/Y accordingly?  I'm
assuming they could react in same way to BTN_TOOL_* changes if we can
extend to 5 finger case?

Chris
--
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 July 6, 2011, 4:53 p.m. UTC | #7
On Wed, Jul 6, 2011 at 2:55 AM, Chris Bagwell <chris@cnpbagwell.com> wrote:
> On Tue, Jul 5, 2011 at 1:17 PM, Chase Douglas
> <chase.douglas@canonical.com> wrote:
>> On 06/28/2011 10:07 PM, djkurtz@chromium.org wrote:
>>> From: Daniel Kurtz <djkurtz@chromium.org>
>>>
>>> A Synaptics image sensor tracks 5 fingers, but can only report 2.
>>> This behavior is called "T5R2" = Track 5 Report 2
>>>
>>> Algorithm for choosing which 2 fingers to report in which packet:
>>>   Touchpad maintains 5 slots, numbered 0 to 4.
>>>   Initially all slots are empty.
>>>   As new fingers are detected, they are assigned the lowest available
>>>   slot.
>>>   Touchpad always reports:
>>>     SGM: lowest numbered non-empty slot
>>>     AGM: highest numbered non-empty slot, if there is one.
>>>
>>> In addition, T5R2 touchpads have a special AGM packet type which reports
>>> the number of fingers currently being tracked, and which finger is in
>>> each of the two slots.  Unfortunately, these "TYPE=2" packets are only used
>>> when more than 3 fingers are being tracked.  When less than 4 fingers
>>> are present, the 'w' value must be used to track how many fingers are
>>> present, and knowing which fingers are being reported is much more
>>> difficult, if not impossible.
>>>
>>> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>>> ---
>>>  drivers/input/mouse/synaptics.c |   39 ++++++++++++++++++++++++++++++++++-----
>>>  drivers/input/mouse/synaptics.h |    7 ++++++-
>>>  include/linux/input.h           |    1 +
>>>  3 files changed, 41 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>>> index 2d7ac0a..19a9b7f 100644
>>> --- a/drivers/input/mouse/synaptics.c
>>> +++ b/drivers/input/mouse/synaptics.c
>>> @@ -401,6 +401,14 @@ static void synaptics_pt_create(struct psmouse *psmouse)
>>>  /*****************************************************************************
>>>   *   Functions to interpret the absolute mode packets
>>>   ****************************************************************************/
>>> +/* Set AGM-CONTACT finger state */
>>> +static void synaptics_agm_finger_update(struct synaptics_data *priv, int count,
>>> +                                     int sgm, int agm)
>>> +{
>>> +     priv->agm.finger_count = count;
>>> +     priv->agm.finger_sgm = sgm;
>>> +     priv->agm.finger_agm = agm;
>>> +}
>>>
>>>  static int synaptics_parse_hw_state(const unsigned char buf[],
>>>                                   struct synaptics_data *priv,
>>> @@ -438,11 +446,31 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>>>               if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)
>>>                               || SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
>>>                               && hw->w == 2) {
>>> -                     /* Gesture packet: (x, y, z) at half resolution */
>>> -                     priv->agm.x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
>>> -                     priv->agm.y = INVERT_Y((((buf[4] & 0xf0) << 4)
>>> -                                           | buf[2]) << 1);
>>> -                     priv->agm.z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
>>> +                     int type; /* Packet type */
>>> +
>>> +                     type = (buf[5] & 0x30) >> 4;
>>> +
>>> +                     switch (type) {
>>> +                     case 1:
>>> +                             /* Gesture packet: (x, y, z) half resolution */
>>> +                             priv->agm.w = hw->w;
>>> +                             priv->agm.x = (((buf[4] & 0x0f) << 8)
>>> +                                             | buf[1]) << 1;
>>> +                             priv->agm.y = INVERT_Y((((buf[4] & 0xf0) << 4)
>>> +                                                    | buf[2]) << 1);
>>> +                             priv->agm.z = ((buf[3] & 0x30)
>>> +                                             | (buf[5] & 0x0f)) << 1;
>>> +                             break;
>>> +
>>> +                     case 2:
>>> +                             /* Finger slot update */
>>> +                             synaptics_agm_finger_update(priv, buf[1],
>>> +                                                         buf[2], buf[4]);
>>> +                             break;
>>> +
>>> +                     default:
>>> +                             break;
>>> +                     }
>>>                       return 1;
>>>               } else {
>>>                       hw->x = (((buf[3] & 0x10) << 8) |
>>> @@ -804,6 +832,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>>>       input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
>>>
>>>       if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
>>> +             __set_bit(INPUT_PROP_SYNAPTICS_T5R2, dev->propbit);
>>>               input_mt_init_slots(dev, SYN_TRACK_SLOT_COUNT);
>>>               input_set_abs_params(dev, ABS_MT_POSITION_X, XMIN_NOMINAL,
>>>                                    priv->x_max ?: XMAX_NOMINAL, fuzz, 0);
>>> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
>>> index 1de2256..2214af6 100644
>>> --- a/drivers/input/mouse/synaptics.h
>>> +++ b/drivers/input/mouse/synaptics.h
>>> @@ -122,7 +122,7 @@
>>>  #define SYN_SLOT_AGM                 2
>>>
>>>  /* number of tracking slots for Image Sensor firmware */
>>> -#define SYN_TRACK_SLOT_COUNT         2
>>> +#define SYN_TRACK_SLOT_COUNT         5
>>>
>>>  /*
>>>   * A structure to describe the state of the touchpad hardware (buttons and pad)
>>> @@ -140,6 +140,11 @@ struct synaptics_hw_state {
>>>       unsigned int down:1;
>>>       unsigned char ext_buttons;
>>>       signed char scroll;
>>> +
>>> +     /* Reported in AGM-CONTACT packets */
>>> +     unsigned int finger_count;              /* num fingers being tracked */
>>> +     unsigned int finger_sgm;                /* finger described by sgm */
>>> +     unsigned int finger_agm;                /* finger described by agm */
>>>  };
>>>
>>>  struct synaptics_data {
>>> diff --git a/include/linux/input.h b/include/linux/input.h
>>> index 771d6d8..732c14e 100644
>>> --- a/include/linux/input.h
>>> +++ b/include/linux/input.h
>>> @@ -137,6 +137,7 @@ struct input_keymap_entry {
>>>  #define INPUT_PROP_DIRECT            0x01    /* direct input devices */
>>>  #define INPUT_PROP_BUTTONPAD         0x02    /* has button(s) under pad */
>>>  #define INPUT_PROP_SEMI_MT           0x03    /* touch rectangle only */
>>> +#define INPUT_PROP_SYNAPTICS_T5R2    0x04    /* synaptics track 5 report 2 */
>>>
>>>  #define INPUT_PROP_MAX                       0x1f
>>>  #define INPUT_PROP_CNT                       (INPUT_PROP_MAX + 1)
>>
>> I'm trying to understand these later patches for T5R2. There are
>> "hidden" touch slots now. How does userspace tell whether a touch slot
>> is a hidden touch?
>>
>
> I'm trying to understand as well.  If I was a consumer of events and I
> knew touchpad supports 2 touch tracking but 5 finger detection, I'd
> assume:
>
> * ABS_X/Y reports oldest touch and
> BTN_TOOL_DOUBLETAP/TRIPLETAP/QUADTAP reports # of fingers touching
> (ignoring lost of 5 finger for now).
> * ABS_MT_* only contains 2 slots and without SEMI_MT declared.
> BTN_TOOL_* could be used to detect greater than 2 finger touches.
>
> I'm not sure I could guess what to do with 5 slots and how to expect
> touch data to come over them.
>
> Where you saying in the other thread that the 5 slots are to help user
> detect finger transitions and re-act bounces in X/Y accordingly?  I'm
> assuming they could react in same way to BTN_TOOL_* changes if we can
> extend to 5 finger case?

The "number of active slots" (ie slots with tracking_id != -1) can
also always be used to determine the number of touches.

Given 5 slots, each assigned a slot_id: {0, 1, 2, 3, 4}.
Then:
  (a) the active slot with lowest slot_id contains one valid position
(contact from SGM packet).
  (b) the active slot with highest slot_id contains the other valid
position (contact from AGM packet).
  (c) any active slots between these are "hidden", meaning that their
position data is stale, but they count towards the finger count, and
their tracking_ids stay valid.

The real benefit is what happens when fingers are removed.   Let's say
there are 4 touches: {0, 1, 2, 4}.  Now, if a subset of fingers was
removed {1, 4}, then the T5R2 method could actually identify that
fingers {0, 2} were still being reported.

Curiously if another finger were to get added at this point, it would
become the new hidden finger 1.  This is just how the image sensor
works.

This patchset deduces what the firmware is doing in the kernel, and
T5R2 allows us to use the MT-B protocol to express this state up to
userspace.

Thanks,
-Dan

>
> Chris
>
--
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
diff mbox

Patch

difficult, if not impossible.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c |   39 ++++++++++++++++++++++++++++++++++-----
 drivers/input/mouse/synaptics.h |    7 ++++++-
 include/linux/input.h           |    1 +
 3 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 2d7ac0a..19a9b7f 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -401,6 +401,14 @@  static void synaptics_pt_create(struct psmouse *psmouse)
 /*****************************************************************************
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
+/* Set AGM-CONTACT finger state */
+static void synaptics_agm_finger_update(struct synaptics_data *priv, int count,
+					int sgm, int agm)
+{
+	priv->agm.finger_count = count;
+	priv->agm.finger_sgm = sgm;
+	priv->agm.finger_agm = agm;
+}
 
 static int synaptics_parse_hw_state(const unsigned char buf[],
 				    struct synaptics_data *priv,
@@ -438,11 +446,31 @@  static int synaptics_parse_hw_state(const unsigned char buf[],
 		if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)
 				|| SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
 				&& hw->w == 2) {
-			/* Gesture packet: (x, y, z) at half resolution */
-			priv->agm.x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
-			priv->agm.y = INVERT_Y((((buf[4] & 0xf0) << 4)
-					      | buf[2]) << 1);
-			priv->agm.z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
+			int type; /* Packet type */
+
+			type = (buf[5] & 0x30) >> 4;
+
+			switch (type) {
+			case 1:
+				/* Gesture packet: (x, y, z) half resolution */
+				priv->agm.w = hw->w;
+				priv->agm.x = (((buf[4] & 0x0f) << 8)
+						| buf[1]) << 1;
+				priv->agm.y = INVERT_Y((((buf[4] & 0xf0) << 4)
+						       | buf[2]) << 1);
+				priv->agm.z = ((buf[3] & 0x30)
+						| (buf[5] & 0x0f)) << 1;
+				break;
+
+			case 2:
+				/* Finger slot update */
+				synaptics_agm_finger_update(priv, buf[1],
+							    buf[2], buf[4]);
+				break;
+
+			default:
+				break;
+			}
 			return 1;
 		} else {
 			hw->x = (((buf[3] & 0x10) << 8) |
@@ -804,6 +832,7 @@  static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
 
 	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
+		__set_bit(INPUT_PROP_SYNAPTICS_T5R2, dev->propbit);
 		input_mt_init_slots(dev, SYN_TRACK_SLOT_COUNT);
 		input_set_abs_params(dev, ABS_MT_POSITION_X, XMIN_NOMINAL,
 				     priv->x_max ?: XMAX_NOMINAL, fuzz, 0);
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 1de2256..2214af6 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -122,7 +122,7 @@ 
 #define SYN_SLOT_AGM			2
 
 /* number of tracking slots for Image Sensor firmware */
-#define SYN_TRACK_SLOT_COUNT		2
+#define SYN_TRACK_SLOT_COUNT		5
 
 /*
  * A structure to describe the state of the touchpad hardware (buttons and pad)
@@ -140,6 +140,11 @@  struct synaptics_hw_state {
 	unsigned int down:1;
 	unsigned char ext_buttons;
 	signed char scroll;
+
+	/* Reported in AGM-CONTACT packets */
+	unsigned int finger_count;		/* num fingers being tracked */
+	unsigned int finger_sgm;		/* finger described by sgm */
+	unsigned int finger_agm;		/* finger described by agm */
 };
 
 struct synaptics_data {
diff --git a/include/linux/input.h b/include/linux/input.h
index 771d6d8..732c14e 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -137,6 +137,7 @@  struct input_keymap_entry {
 #define INPUT_PROP_DIRECT		0x01	/* direct input devices */
 #define INPUT_PROP_BUTTONPAD		0x02	/* has button(s) under pad */
 #define INPUT_PROP_SEMI_MT		0x03	/* touch rectangle only */
+#define INPUT_PROP_SYNAPTICS_T5R2	0x04	/* synaptics track 5 report 2 */
 
 #define INPUT_PROP_MAX			0x1f
 #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)