[PATCH 1/2] Revert "HID: multitouch: enable palm rejection if device implements confidence usage"

This reverts commit 25a84db15b3f ("HID: multitouch: enable palm rejection
if device implements confidence usage")

The commit enables palm rejection for Win8 Precision Touchpad devices but
the quirk MT_QUIRK_VALID_IS_CONFIDENCE it is using is not working very
properly. This quirk is originally designed for some WIn7 touchscreens. Use
of this for a Win8 Precision Touchpad will cause unexpected pointer jumping
problem.

Signed-off-by: Allen Hung <redacted>
---
 drivers/hid/hid-multitouch.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 95b7d61..4ef7006 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -502,11 +502,6 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			mt_store_field(usage, td, hi);
 			return 1;
 		case HID_DG_CONFIDENCE:
-			if (cls->name == MT_CLS_WIN_8 &&
-				field->application == HID_DG_TOUCHPAD) {
-				cls->quirks &= ~MT_QUIRK_ALWAYS_VALID;
-				cls->quirks |= MT_QUIRK_VALID_IS_CONFIDENCE;
-			}
 			mt_store_field(usage, td, hi);
 			return 1;
 		case HID_DG_TIPSWITCH:

-- 
2.7.4

The usage Confidence is mandary to Windows Precision Touchpad devices. If
it is examined in input_mapping on a WIndows Precision Touchpad, a new add
quirk MT_QUIRK_CONFIDENCE desgned for such devices will be applied to the
device. A touch with the confidence bit is not set is determined as
invalid.

Tested on Dell XPS13 9343

Signed-off-by: Allen Hung <redacted>
---
 drivers/hid/hid-multitouch.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 4ef7006..fb6f1f4 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -61,6 +61,7 @@ MODULE_LICENSE("GPL");
 #define MT_QUIRK_ALWAYS_VALID		(1 << 4)
 #define MT_QUIRK_VALID_IS_INRANGE	(1 << 5)
 #define MT_QUIRK_VALID_IS_CONFIDENCE	(1 << 6)
+#define MT_QUIRK_CONFIDENCE		(1 << 7)
 #define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE	(1 << 8)
 #define MT_QUIRK_NO_AREA		(1 << 9)
 #define MT_QUIRK_IGNORE_DUPLICATES	(1 << 10)

@@ -78,6 +79,7 @@ struct mt_slot {
 	__s32 contactid;	/* the device ContactID assigned to this slot */
 	bool touch_state;	/* is the touch valid? */
 	bool inrange_state;	/* is the finger in proximity of the sensor? */
+	bool confidence_state;  /* is the touch made by a finger? */
 };
 
 struct mt_class {

@@ -502,6 +504,9 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			mt_store_field(usage, td, hi);
 			return 1;
 		case HID_DG_CONFIDENCE:
+			if (cls->name == MT_CLS_WIN_8 &&
+				field->application == HID_DG_TOUCHPAD)
+				cls->quirks |= MT_QUIRK_CONFIDENCE;
 			mt_store_field(usage, td, hi);
 			return 1;
 		case HID_DG_TIPSWITCH:

@@ -614,6 +619,7 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
 		return;
 
 	if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {
+		int active;
 		int slotnum = mt_compute_slot(td, input);
 		struct mt_slot *s = &td->curdata;
 		struct input_mt *mt = input->mt;

@@ -628,10 +634,14 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
 				return;
 		}
 
+		if (!(td->mtclass.quirks & MT_QUIRK_CONFIDENCE))
+			s->confidence_state = 1;
+		active = (s->touch_state || s->inrange_state) &&
+							s->confidence_state;
+
 		input_mt_slot(input, slotnum);
-		input_mt_report_slot_state(input, MT_TOOL_FINGER,
-			s->touch_state || s->inrange_state);
-		if (s->touch_state || s->inrange_state) {
+		input_mt_report_slot_state(input, MT_TOOL_FINGER, active);
+		if (active) {
 			/* this finger is in proximity of the sensor */
 			int wide = (s->w > s->h);
 			/* divided by two to match visual scale of touch */

@@ -696,6 +706,8 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 			td->curdata.touch_state = value;
 			break;
 		case HID_DG_CONFIDENCE:
+			if (quirks & MT_QUIRK_CONFIDENCE)
+				td->curdata.confidence_state = value;
 			if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
 				td->curvalid = value;
 			break;

-- 
2.7.4

On Jun 23 2016 or thereabouts, Allen Hung wrote:

The usage Confidence is mandary to Windows Precision Touchpad devices. If
it is examined in input_mapping on a WIndows Precision Touchpad, a new add
quirk MT_QUIRK_CONFIDENCE desgned for such devices will be applied to the
device. A touch with the confidence bit is not set is determined as
invalid.

Tested on Dell XPS13 9343

Signed-off-by: Allen Hung <redacted>

Thanks Allen for taking care of that.

For the series:
Reviewed-by: Benjamin Tissoires <redacted>

Cheers,
Benjamin

On 06/23/2016 01:31 AM, Allen Hung wrote:

This and patch 2 are:

Tested-by: Andy Lutomirski <luto@kernel.org> # XPS 13 9350, BIOS 1.4.3

(And I'm curious why 1.4.3 seems to have been pulled.  Might it have been this issue?  For better or for worse, I upgraded in the narrow window in which it was available on Dell's website.)

--Andy

Andy,

(And I'm curious why 1.4.3 seems to have been pulled.  Might it have
been this issue?  For better or for worse, I upgraded in the narrow
window in which it was available on Dell's website.)

It wasn't pulled in relation to this issue.  It was something else. 
Nothing I'm aware of in the 1.4.3 BIOS changes should have made this any
worse than previously.  Maybe something else in the stack moved that
exaggerated the problem or you started palming the touchpad more?

Your Tested-by: is that meaning that it's fixed entirely for you?  Not
getting stuck at all now?

Thanks,

On Mon, Jun 27, 2016 at 10:55 AM, Andy Lutomirski [off-list ref] wrote:

On 06/23/2016 01:31 AM, Allen Hung wrote:

quoted

This reverts commit 25a84db15b3f ("HID: multitouch: enable palm rejection
if device implements confidence usage")

The commit enables palm rejection for Win8 Precision Touchpad devices but
the quirk MT_QUIRK_VALID_IS_CONFIDENCE it is using is not working very
properly. This quirk is originally designed for some WIn7 touchscreens.
Use
of this for a Win8 Precision Touchpad will cause unexpected pointer
jumping
problem.

Signed-off-by: Allen Hung <redacted>
---
 drivers/hid/hid-multitouch.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 95b7d61..4ef7006 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -502,11 +502,6 @@ static int mt_touch_input_mapping(struct hid_device

*hdev, struct hid_input *hi,
                        mt_store_field(usage, td, hi);
                        return 1;
                case HID_DG_CONFIDENCE:
-                       if (cls->name == MT_CLS_WIN_8 &&
-                               field->application == HID_DG_TOUCHPAD) {
-                               cls->quirks &= ~MT_QUIRK_ALWAYS_VALID;
-                               cls->quirks |=
MT_QUIRK_VALID_IS_CONFIDENCE;
-                       }
                        mt_store_field(usage, td, hi);
                        return 1;
                case HID_DG_TIPSWITCH:

This and patch 2 are:

Tested-by: Andy Lutomirski <luto@kernel.org> # XPS 13 9350, BIOS 1.4.3

(And I'm curious why 1.4.3 seems to have been pulled.  Might it have been
this issue?  For better or for worse, I upgraded in the narrow window in
which it was available on Dell's website.)

I should amend that Tested-by slightly: these patches seem to make the
problem merely about as bad as it was before.  It's still not perfect
-- I still occasionally get stuck touches.

--Andy

Hi,

I've applied the series to hid.git#for-4.7/upstream-fixes

Thanks everybody,

-- 
Jiri Kosina
SUSE Labs

On Mon, Jun 27, 2016 at 3:25 PM, Andy Lutomirski [off-list ref] wrote:

On Mon, Jun 27, 2016 at 10:55 AM, Andy Lutomirski [off-list ref] wrote:

quoted

This and patch 2 are:

Tested-by: Andy Lutomirski <luto@kernel.org> # XPS 13 9350, BIOS 1.4.3

(And I'm curious why 1.4.3 seems to have been pulled.  Might it have been
this issue?  For better or for worse, I upgraded in the narrow window in
which it was available on Dell's website.)

I should amend that Tested-by slightly: these patches seem to make the
problem merely about as bad as it was before.  It's still not perfect
-- I still occasionally get stuck touches.

On further investigation, I'm not getting stuck touches AFAICT.  I'm
experiencing an upgraded version of libinput being clever and
incorrectly deciding that I middle-clicked.  Sorry for the noise.

https://bugs.freedesktop.org/show_bug.cgi?id=96710

--Andy

I've applied the series to hid.git#for-4.7/upstream-fixes

Hi Jiri,

Would you also submit this to -stable? 
I think it should be generally applicable at least a few releases back
since Allen's original submit.

Thanks,

On Wed, 29 Jun 2016, Mario Limonciello wrote:

Would you also submit this to -stable? 
I think it should be generally applicable at least a few releases back
since Allen's original submit.

Both commits (already in Linus' tree as of today) contain the -stable 
kernel annotation.

-- 
Jiri Kosina
SUSE Labs

-----Original Message-----
From: Jiri Kosina [mailto:jikos@kernel.org]
Sent: Thursday, June 30, 2016 2:40 AM
To: Limonciello, Mario <redacted>
Cc: Hung, Allen <redacted>; Benjamin Tissoires
[off-list ref]; linux-input@vger.kernel.org; LKML
[off-list ref]
Subject: Re: [1/2] Revert "HID: multitouch: enable palm rejection if device
implements confidence usage"

On Wed, 29 Jun 2016, Mario Limonciello wrote:

quoted

Would you also submit this to -stable?
I think it should be generally applicable at least a few releases back
since Allen's original submit.

Both commits (already in Linus' tree as of today) contain the -stable
kernel annotation.

--
Jiri Kosina
SUSE Labs

Sorry I missed that, thanks.