[PATCH libinput] touchpad: add a quirk for the HP Pavilion dm4

[Peter Hutterer]

On Tue, Nov 29, 2016 at 10:11:06AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 29-11-16 04:48, Peter Hutterer wrote:
> > On Mon, Nov 28, 2016 at 03:33:25PM +0100, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 27-11-16 23:55, Peter Hutterer wrote:
> > > > This touchpad has cursor jumps for 2-finger scrolling that also affects the
> > > > single-finger emulation. So disable any multitouch bits on this device and
> > > > disallow the 2-finger scroll method. This still allows for 2-finger
> > > > tapping/clicking.
> > > > 
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=91135
> > > > 
> > > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > 
> > > This sounds a lot like what we're doing for semi-mt devices,
> > > but then with completely different code-paths. AFAICT we still
> > > allow BTN_TOOL_DOUBLETAP, etc. for semi-mt, so why not use
> > > the same method.
> > > 
> > > To be specific I'm talking about the "if (tp->semi_mt) { ... }"
> > > block with the large comment above it in tp_init_slots(), to me
> > > it sounds like you want to change that to:
> > > 
> > > 	if (tp->semi_mit ||
> > > 	    (tp->device->model_flags & EVDEV_MODEL_HP_PAVILION_DM4_TOUCHPAD)) {
> > > 		...
> > > 	}
> > > 
> > > Rather then come up with a second approach to only listen to the
> > > non-mt coordinates.
> > 
> > right, with the semi-mt we do a similar-ish thing but we still allow
> > two-finger scrolling based on the single position + BTN_TOOL_DOUBLETAP. In
> > this case the touchpad data is garbage as soon as two fingers are down, even
> > the single-touch emulation is useless. So it's not quite the same category
> > as semi-mts which can still be ok for 2fg scrolling. And since it's garbage
> > anyway, we might as well disable all MT axes.
> 
> I see, so that would require keeping this hunk of your original patch:
> 
> > > > diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> > > > index d72cb19..beb19cd 100644
> > > > --- a/src/evdev-mt-touchpad.c
> > > > +++ b/src/evdev-mt-touchpad.c
> > > > @@ -1909,6 +1909,12 @@ tp_scroll_get_methods(struct tp_dispatch *tp)
> > > >  {
> > > >  	uint32_t methods = LIBINPUT_CONFIG_SCROLL_EDGE;
> > > > 
> > > > +	/* Any movement with more than one finger has random cursor
> > > > +	 * jumps. Don't allow for 2fg scrolling on this device, see
> > > > +	 * fdo bug 91135 */
> > > > +	if (tp->device->model_flags & EVDEV_MODEL_HP_PAVILION_DM4_TOUCHPAD)
> > > > +		return LIBINPUT_CONFIG_SCROLL_EDGE;
> > > > +
> > > >  	if (tp->ntouches >= 2)
> > > >  		methods |= LIBINPUT_CONFIG_SCROLL_2FG;
> > > > 
> 
> But my above suggestion could still replace this hunk, which to me really
> feels like doing the same as the semi-mt handling but then in a more
> complicated way and having 2 code paths doing the same thing in a
> different way feels wrong:

true, and see my v2 for this one, thanks. Though now I'm thinking whether we
should just do the below for all semi-mt touchpads. libevdev
transparently masks the events so we wouldn't even receive the MT slot range
for that from the kernel, reducing the amount of work we do before ignoring
things anyway. Tempting, but not that important right now I guess.

Cheers,
   Peter

> 
> > > > diff --git a/src/evdev.c b/src/evdev.c
> > > > index fac8fcb..eb4c0d0 100644
> > > > --- a/src/evdev.c
> > > > +++ b/src/evdev.c
> > > > @@ -2762,6 +2763,17 @@ evdev_pre_configure_model_quirks(struct evdev_device *device)
> > > >  	if (device->model_flags & EVDEV_MODEL_HP_STREAM11_TOUCHPAD)
> > > >  		libevdev_enable_property(device->evdev,
> > > >  					 INPUT_PROP_BUTTONPAD);
> > > > +
> > > > +	/* Touchpad has random jumps in slots, including for single-finger
> > > > +	 * movement. See fdo bug 91135 */
> > > > +	if (device->model_flags & EVDEV_MODEL_HP_PAVILION_DM4_TOUCHPAD) {
> > > > +		unsigned int code;
> > > > +
> > > > +		for (code = ABS_MT_SLOT; code <= ABS_MT_TRACKING_ID; code++)
> > > > +			libevdev_disable_event_code(device->evdev,
> > > > +						    EV_ABS,
> > > > +						    code);
> > > > +	}
> > > >  }
> > > > 
> > > >  struct evdev_device *
> 
> Regards,
> 
> Hans