diff mbox

[v3,10/12] HID: multitouch: report MT_TOOL_PALM for non-confident touches

Message ID 20180621120908.16706-11-benjamin.tissoires@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Benjamin Tissoires June 21, 2018, 12:09 p.m. UTC
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

According to Microsoft specification [1] for Precision Touchpads (and
Touchscreens) the devices use "confidence" reports to signal accidental
touches, or contacts that are "too large to be a finger". Instead of
simply marking contact inactive in this case (which causes issues if
contact was originally proper and we lost confidence in it later, as
this results in accidental clicks, drags, etc), let's report such
contacts as MT_TOOL_PALM and let userspace decide what to do.
Additionally, let's report contact size for such touches as maximum
allowed for major/minor, which should help userspace that is not yet
aware of MT_TOOL_PALM to still perform palm rejection.

[1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[splitted and rebased]
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v2:
- dropped the delayed release in case of palm detection. This is
  a FW bug for PTP devices, and should only happen with touchscreens
- move input_set_abs_params(hi->input, ABS_MT_TOOL_TYPE,...) into
  its own test so other devices can add MT_QUIRK_CONFIDENCE to their
  quirks.

no changes in v3
---
 drivers/hid/hid-multitouch.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

Comments

Peter Hutterer June 27, 2018, 1:08 a.m. UTC | #1
On Thu, Jun 21, 2018 at 02:09:06PM +0200, Benjamin Tissoires wrote:
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> According to Microsoft specification [1] for Precision Touchpads (and
> Touchscreens) the devices use "confidence" reports to signal accidental
> touches, or contacts that are "too large to be a finger". Instead of
> simply marking contact inactive in this case (which causes issues if
> contact was originally proper and we lost confidence in it later, as
> this results in accidental clicks, drags, etc), let's report such
> contacts as MT_TOOL_PALM and let userspace decide what to do.
> Additionally, let's report contact size for such touches as maximum
> allowed for major/minor, which should help userspace that is not yet
> aware of MT_TOOL_PALM to still perform palm rejection.

tbh. I have a queasy feeling about this last bit. MT_TOOL_PALM has shown to
not be reliable on some devices (X1 Carbon 6th) and we mask it out on those.
Changing the major/minor means it's impossible to rely on the touch size for
palm detection once a device supports MT_TOOL_PALM.

Cheers,
   Peter

> 
> [1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> [splitted and rebased]
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> ---
> 
> changes in v2:
> - dropped the delayed release in case of palm detection. This is
>   a FW bug for PTP devices, and should only happen with touchscreens
> - move input_set_abs_params(hi->input, ABS_MT_TOOL_TYPE,...) into
>   its own test so other devices can add MT_QUIRK_CONFIDENCE to their
>   quirks.
> 
> no changes in v3
> ---
>  drivers/hid/hid-multitouch.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index a4a274ebfbef..36c8b67bb032 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -736,6 +736,13 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  				cls->name == MT_CLS_WIN_8_DUAL) &&
>  				field->application == HID_DG_TOUCHPAD)
>  				app->quirks |= MT_QUIRK_CONFIDENCE;
> +
> +			if (app->quirks & MT_QUIRK_CONFIDENCE)
> +				input_set_abs_params(hi->input,
> +						     ABS_MT_TOOL_TYPE,
> +						     MT_TOOL_FINGER,
> +						     MT_TOOL_PALM, 0, 0);
> +
>  			MT_STORE_FIELD(confidence_state);
>  			return 1;
>  		case HID_DG_TIPSWITCH:
> @@ -958,10 +965,12 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
>  	if (quirks & MT_QUIRK_HOVERING)
>  		inrange_state = *slot->inrange_state;
>  
> -	active = (*slot->tip_state || inrange_state) && confidence_state;
> +	active = *slot->tip_state || inrange_state;
>  
>  	if (app->application == HID_GD_SYSTEM_MULTIAXIS)
>  		tool = MT_TOOL_DIAL;
> +	else if (unlikely(!confidence_state))
> +		tool = MT_TOOL_PALM;
>  
>  	input_mt_slot(input, slotnum);
>  	input_mt_report_slot_state(input, tool, active);
> @@ -993,11 +1002,20 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
>  			orientation = -azimuth;
>  		}
>  
> -		/*
> -		 * divided by two to match visual scale of touch
> -		 * for devices with this quirk
> -		 */
> -		if (quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
> +		if (unlikely(!confidence_state)) {
> +			/*
> +			 * When reporting palm, set contact to maximum
> +			 * size to help userspace that does not
> +			 * recognize MT_TOOL_PALM to reject contacts
> +			 * that are too large.
> +			 */
> +			major = input_abs_get_max(input, ABS_MT_TOUCH_MAJOR);
> +			minor = input_abs_get_max(input, ABS_MT_TOUCH_MINOR);
> +		} else if (quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
> +			/*
> +			 * divided by two to match visual scale of touch
> +			 * for devices with this quirk
> +			 */
>  			major = major >> 1;
>  			minor = minor >> 1;
>  		}
> -- 
> 2.14.3
> 
--
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
Dmitry Torokhov July 4, 2018, 3:18 p.m. UTC | #2
On Wed, Jun 27, 2018 at 11:08:58AM +1000, Peter Hutterer wrote:
> On Thu, Jun 21, 2018 at 02:09:06PM +0200, Benjamin Tissoires wrote:
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > 
> > According to Microsoft specification [1] for Precision Touchpads (and
> > Touchscreens) the devices use "confidence" reports to signal accidental
> > touches, or contacts that are "too large to be a finger". Instead of
> > simply marking contact inactive in this case (which causes issues if
> > contact was originally proper and we lost confidence in it later, as
> > this results in accidental clicks, drags, etc), let's report such
> > contacts as MT_TOOL_PALM and let userspace decide what to do.
> > Additionally, let's report contact size for such touches as maximum
> > allowed for major/minor, which should help userspace that is not yet
> > aware of MT_TOOL_PALM to still perform palm rejection.
> 
> tbh. I have a queasy feeling about this last bit. MT_TOOL_PALM has shown to
> not be reliable on some devices (X1 Carbon 6th) and we mask it out on those.
> Changing the major/minor means it's impossible to rely on the touch size for
> palm detection once a device supports MT_TOOL_PALM.

OK, then let's drop this bit.

Thanks.
diff mbox

Patch

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a4a274ebfbef..36c8b67bb032 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -736,6 +736,13 @@  static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 				cls->name == MT_CLS_WIN_8_DUAL) &&
 				field->application == HID_DG_TOUCHPAD)
 				app->quirks |= MT_QUIRK_CONFIDENCE;
+
+			if (app->quirks & MT_QUIRK_CONFIDENCE)
+				input_set_abs_params(hi->input,
+						     ABS_MT_TOOL_TYPE,
+						     MT_TOOL_FINGER,
+						     MT_TOOL_PALM, 0, 0);
+
 			MT_STORE_FIELD(confidence_state);
 			return 1;
 		case HID_DG_TIPSWITCH:
@@ -958,10 +965,12 @@  static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 	if (quirks & MT_QUIRK_HOVERING)
 		inrange_state = *slot->inrange_state;
 
-	active = (*slot->tip_state || inrange_state) && confidence_state;
+	active = *slot->tip_state || inrange_state;
 
 	if (app->application == HID_GD_SYSTEM_MULTIAXIS)
 		tool = MT_TOOL_DIAL;
+	else if (unlikely(!confidence_state))
+		tool = MT_TOOL_PALM;
 
 	input_mt_slot(input, slotnum);
 	input_mt_report_slot_state(input, tool, active);
@@ -993,11 +1002,20 @@  static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 			orientation = -azimuth;
 		}
 
-		/*
-		 * divided by two to match visual scale of touch
-		 * for devices with this quirk
-		 */
-		if (quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
+		if (unlikely(!confidence_state)) {
+			/*
+			 * When reporting palm, set contact to maximum
+			 * size to help userspace that does not
+			 * recognize MT_TOOL_PALM to reject contacts
+			 * that are too large.
+			 */
+			major = input_abs_get_max(input, ABS_MT_TOUCH_MAJOR);
+			minor = input_abs_get_max(input, ABS_MT_TOUCH_MINOR);
+		} else if (quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
+			/*
+			 * divided by two to match visual scale of touch
+			 * for devices with this quirk
+			 */
 			major = major >> 1;
 			minor = minor >> 1;
 		}