Hi Joshua,
this patch breaks ubcmtp on the MacBook Pro 8.2. There is no problem
with the padding, but the pressure value is always 0, and wsmouse
doesn't register any touches. I believe only type4 models fill the
'pressure' field. At least for now, it's better to use the width
value (touch_major) as substitute for pressure (hidmt already does
this). In case we add to wsmouse or wstpad some mechanism for palm/
thumb detection based on thresholds, this would work as well.
So I would prefer:
+ sc->frame[contacts].pressure =
+ (int16_t)letoh16(finger->touch_major);
Moreover, it shouldn't be necessary to set the PRESSURE_LO/PRESSURE_HI
parameters in ubcmtp. Only touchpads that are too sensitive need
them (if they are set, wsmouse ignores touches with lower pressure).
ok bru@ for the padding part
On 08/16/2018 04:51 AM, joshua stein wrote:
> Type 4 devices like the MacBookPro12,1 have extra padding in between
> each chunk of finger data that needs to be skipped, otherwise the
> offset will be wrong and each additional finger will read as static
> coordinates. This must have been overlooked when it was added to
> the driver in 2015 since the first finger of data always read
> properly.
>
> Also, we can now pass in the actual pressure data instead of having
> to use a constant value just to make the synaptics driver happy
> since we now have wstpad. The per-type min/max pressure values can
> also be passed in when calling wsmouse_configure.
>
> Tested on the 2015 MacBook Pro. Tests on other Apple devices would
> be appreciated.
>
>
> Index: sys/dev/usb/ubcmtp.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ubcmtp.c,v
> retrieving revision 1.18
> diff -u -p -u -p -r1.18 ubcmtp.c
> --- sys/dev/usb/ubcmtp.c 30 Jul 2018 15:56:30 -0000 1.18
> +++ sys/dev/usb/ubcmtp.c 16 Aug 2018 02:42:11 -0000
> @@ -118,6 +118,7 @@ struct ubcmtp_finger {
> #define UBCMTP_TYPE4_TPLEN UBCMTP_TYPE4_TPOFF + UBCMTP_ALL_FINGER_SIZE
> #define UBCMTP_TYPE4_TPIFACE 2
> #define UBCMTP_TYPE4_BTOFF 31
> +#define UBCMTP_TYPE4_FINGERPAD (1 * sizeof(uint16_t))
>
> #define UBCMTP_FINGER_ORIENT 16384
> #define UBCMTP_SN_PRESSURE 45
> @@ -128,9 +129,6 @@ struct ubcmtp_finger {
> /* Identify clickpads in ubcmtp_configure. */
> #define IS_CLICKPAD(ubcmtp_type) (ubcmtp_type != UBCMTP_TYPE1)
>
> -/* Use a constant, synaptics-compatible pressure value for now. */
> -#define DEFAULT_PRESSURE 40
> -
> struct ubcmtp_limit {
> int limit;
> int min;
> @@ -337,6 +335,7 @@ struct ubcmtp_softc {
> int sc_tp_epaddr; /* endpoint addr */
> int tp_maxlen; /* max size of tp data */
> int tp_offset; /* finger offset into data */
> + int tp_fingerpad; /* padding between finger data
> */
> uint8_t *tp_pkt;
>
> int bt_ifacenum; /* button interface number */
> @@ -425,6 +424,7 @@ ubcmtp_attach(struct device *parent, str
>
> sc->sc_udev = uaa->device;
> sc->sc_status = 0;
> + sc->tp_fingerpad = 0;
>
> if ((udd = usbd_get_device_descriptor(dev)) == NULL) {
> printf("ubcmtp: failed getting device descriptor\n");
> @@ -478,6 +478,7 @@ ubcmtp_attach(struct device *parent, str
> sc->tp_maxlen = UBCMTP_TYPE4_TPLEN;
> sc->tp_offset = UBCMTP_TYPE4_TPOFF;
> sc->tp_ifacenum = UBCMTP_TYPE4_TPIFACE;
> + sc->tp_fingerpad = UBCMTP_TYPE4_FINGERPAD;
> break;
> }
>
> @@ -520,6 +521,10 @@ int
> ubcmtp_configure(struct ubcmtp_softc *sc)
> {
> struct wsmousehw *hw = wsmouse_get_hw(sc->sc_wsmousedev);
> + struct wsmouse_param params[] = {
> + { WSMOUSECFG_PRESSURE_LO, sc->dev_type->l_pressure.min },
> + { WSMOUSECFG_PRESSURE_HI, sc->dev_type->l_pressure.max },
> + };
>
> hw->type = WSMOUSE_TYPE_TOUCHPAD;
> hw->hw_type = (IS_CLICKPAD(sc->dev_type->type)
> @@ -531,7 +536,7 @@ ubcmtp_configure(struct ubcmtp_softc *sc
> hw->mt_slots = UBCMTP_MAX_FINGERS;
> hw->flags = WSMOUSEHW_MT_TRACKING;
>
> - return wsmouse_configure(sc->sc_wsmousedev, NULL, 0);
> + return wsmouse_configure(sc->sc_wsmousedev, params, 0);
> }
>
> int
> @@ -793,9 +798,9 @@ void
> ubcmtp_tp_intr(struct usbd_xfer *xfer, void *priv, usbd_status status)
> {
> struct ubcmtp_softc *sc = priv;
> - struct ubcmtp_finger *pkt;
> + struct ubcmtp_finger *finger;
> u_int32_t pktlen;
> - int i, s, btn, contacts, fingers;
> + int off, s, btn, contacts = 0;
>
> if (usbd_is_dying(sc->sc_udev) || !(sc->sc_status & UBCMTP_ENABLED))
> return;
> @@ -816,16 +821,19 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v
> if (sc->tp_pkt == NULL || pktlen < sc->tp_offset)
> return;
>
> - pkt = (struct ubcmtp_finger *)(sc->tp_pkt + sc->tp_offset);
> - fingers = (pktlen - sc->tp_offset) / sizeof(struct ubcmtp_finger);
> -
> contacts = 0;
> - for (i = 0; i < fingers; i++) {
> - if ((int16_t)letoh16(pkt[i].touch_major) == 0)
> + for (off = sc->tp_offset; off < pktlen;
> + off += (sizeof(struct ubcmtp_finger) + sc->tp_fingerpad)) {
> + finger = (struct ubcmtp_finger *)(sc->tp_pkt + off);
> +
> + if ((int16_t)letoh16(finger->touch_major) == 0)
> continue; /* finger lifted */
> - sc->frame[contacts].x = (int16_t)letoh16(pkt[i].abs_x);
> - sc->frame[contacts].y = (int16_t)letoh16(pkt[i].abs_y);
> - sc->frame[contacts].pressure = DEFAULT_PRESSURE;
> +
> + sc->frame[contacts].x = (int16_t)letoh16(finger->abs_x);
> + sc->frame[contacts].y = (int16_t)letoh16(finger->abs_y);
> + sc->frame[contacts].pressure =
> + (int16_t)letoh16(finger->pressure);
> +
> contacts++;
> }
>
>
>
