[Raring][PULL] Cypress PS/2 Trackpad driver

[Kamal Mostafa]

On Thu, 2013-01-17 at 14:38 -0600, Seth Forshee wrote:
> On Thu, Jan 17, 2013 at 11:04:22AM -0800, Kamal Mostafa wrote:
> > The Cypress PS/2 Trackpad driver [...]
> > plus my "simulated multitouch" Cypress
> > driver enhancement (denied by upstream, but required for Unity
> > multitouch functionality; is included in our P and Q version).


> I'm not really so keen on this simulated multitouch patch.
> 

Hi Seth-

Thanks for reviewing this, and for all your help with this driver.  I'm
certainly open to discussion about the validity of the Cypress simulated
multitouch patch...


> First, I think you overstate the case for the simulated multitouch
> functionality.

The main case for it is that the shipping Precise and Quantal SAUCE
versions of this driver do include the simulated multitouch enhancement.
The original driver as supplied by Cypress included a variant of it. 

So for the Cypress trackpad, Unity multi-touch gestures (>2 fingers)
work now in P and Q, but if we omit my simul-mt patch then they won't
work anymore in Raring: a regression.


>  Without that patch the driver still has semi-mt support,
> which should be adequate to support most two-finger gestures: drag,
> pinch, spread, and two-finger clicks at minimum. It looks like there's
> enough information coming out of the driver to support 3, 4, and 5
> finger clicks as well.

Correct.  The Cypress driver (with or without the simul-mt patch) does
already emit the newer fingers-count API events along with the semi-mt
bounding box via a call to input_mt_report_finger_count(), but ...


>  My understanding is that our userspace doesn't
> handle those events the same as with full mt devices, but I don't see
> that as being a driver problem.

Correct.  Our userspace (Unity via x-x-input-synaptics) does not support
that newer fingers-count API at all for semi-mt devices.  If my reading
of x-x-i-s is correct, even the latest upstream version of x-x-i-s does
not.  I spent several frustrating hours trying to make x-x-i-s to
support it, but couldn't figure out how without a major overhaul.

Upstream input subsystem maintainers agree with you that this is "not a
driver problem", but userspace doesn't seem to want to take
responsibility for it either.

From the perspective of a user of the machine, I do not care whether the
code is in the driver or in userspace...  If the hardware is capable of
>2 finger support and we have working code to support it, then I would
like it to work on my Ubuntu system.


> 
> As for gestures involving 3+ fingers, the code [Kamal's simul-mt patch]
> just generates fake touch
> points offset horizontally from the first one. That means every gesture
> looks like a drag.

A drag or a tap.  As coded, it works very nicely for Unity's 3-finger
drag and 4-finger tap gestures.


> 
> In fact, with knowing the number of contacts and the bounding box I'd
> think that userspace should even be able to support a 3+ finger drag
> gesture. Probably better than the simulated multitouch even, because if
> the bounding box isn't staying approximately the same size the gesture
> likely isn't a drag.

Agreed.  That's exactly what userspace *should* be doing for semi-mt
devices.  ;-)


> 
> What's wrong with limiting it to semi-mt, in line with the actual
> capabilities of the hardware?

My feeling is that the addition of the simul-mt patch is useful and
harmless, so what's wrong with including it?


> One more comment, regarding the implementation. The patch lacks
> protection for report_data->contact_cnt > CTYP_MAX_MT_SLOTS whereas the
> rest of the driver seems to protect against it. I'm assuming that
> because you've increased that max number of slots from 2 to 5 there's no
> longer a concern of having more contacts than slots? Because if that
> happens you're going to overrun report_data->contacts.

Your assumption is correct.  contact_cnt cannot ever become > 5, and the
checks for contact_cnt > CTYP_MAX_MT_SLOTS in the rest of the driver are
actually necessary *only* to support the semi-mt case.  Those checks
aren't for overrun protection, they are what implements the regular
semi-mt "maximum-of-two-coordinate-pairs" behavior.

 -Kamal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20130117/b56883fb/attachment.sig>