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

[Hans de Goede]

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:

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