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

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

[Allen Hung]

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 <allen_hung@dell.com>
---
 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

[PATCH 2/2] HID: multitouch: enable palm rejection for Windows Precision Touchpad

[Allen Hung]

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 <allen_hung@dell.com>
---
 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

Re: [PATCH 2/2] HID: multitouch: enable palm rejection for Windows Precision Touchpad

[Benjamin Tissoires]

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 <allen_hung@dell.com>

Thanks Allen for taking care of that.

For the series:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

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

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

[Andy Lutomirski]

On 06/23/2016 01:31 AM, Allen Hung wrote:
> 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 <allen_hung@dell.com>
> ---
>  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.)

--Andy

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

[Mario Limonciello]

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,

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

[Andy Lutomirski]

On Mon, Jun 27, 2016 at 10:55 AM, Andy Lutomirski <luto@kernel.org> wrote:
> On 06/23/2016 01:31 AM, Allen Hung wrote:
>>
>> 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 <allen_hung@dell.com>
>> ---
>>  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

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

[Jiri Kosina]

Hi,

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

Thanks everybody,

-- 
Jiri Kosina
SUSE Labs

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

[Andy Lutomirski]

On Mon, Jun 27, 2016 at 3:25 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Mon, Jun 27, 2016 at 10:55 AM, Andy Lutomirski <luto@kernel.org> 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.)
>>
>
> 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

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

[Mario Limonciello]

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

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

[Jiri Kosina]

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

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

[Mario_Limonciello]

> -----Original Message-----
> From: Jiri Kosina [mailto:jikos@kernel.org]
> Sent: Thursday, June 30, 2016 2:40 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Hung, Allen <Allen_Hung@Dell.com>; Benjamin Tissoires
> <benjamin.tissoires@redhat.com>; linux-input@vger.kernel.org; LKML
> <linux-kernel@vger.kernel.org>
> Subject: Re: [1/2] Revert "HID: multitouch: enable palm rejection if device
> implements confidence usage"
> 
> 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

Sorry I missed that, thanks.