Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] HID: hid-multitouch: support fine-grain orientation reporting
@ 2017-09-27  3:03 Wei-Ning Huang
  2017-09-27  5:55 ` Dmitry Torokhov
  0 siblings, 1 reply; 3+ messages in thread
From: Wei-Ning Huang @ 2017-09-27  3:03 UTC (permalink / raw)
  To: LKML, Linux Input; +Cc: Dmitry Torokhov, Jiri Kosina, Wei-Ning Huang

The current hid-multitouch driver only allow the report of two
orientations, vertical and horizontal. We use the Azimuth orientation
usage 0x5b under the Digitizer usage page to report orientation directly
from the hid device. A new quirk MT_QUIRK_REPORT_ORIENTATION is added so
user can enable this only if their device supports it.

Signed-off-by: Wei-Ning Huang <wnhuang@google.com>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
---
 drivers/hid/hid-multitouch.c | 38 ++++++++++++++++++++++++++++++++++++--
 include/linux/hid.h          |  1 +
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 440b999304a5..4663bf4b2892 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
 #define MT_IO_FLAGS_PENDING_SLOTS	2
 
 struct mt_slot {
-	__s32 x, y, cx, cy, p, w, h;
+	__s32 x, y, cx, cy, p, w, h, a;
 	__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? */
+	bool has_azimuth;       /* the contact reports azimuth */
 };
 
 struct mt_class {
@@ -591,6 +592,23 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			td->cc_index = field->index;
 			td->cc_value_index = usage->usage_index;
 			return 1;
+		case HID_DG_AZIMUTH:
+			hid_map_usage(hi, usage, bit, max,
+				EV_ABS, ABS_MT_ORIENTATION);
+			/*
+			 * Azimuth has the range of [0, MAX) representing a full
+			 * revolution. Set ABS_MT_ORIENTATION to a quarter of
+			 * MAX according the definition of ABS_MT_ORIENTATION
+			 */
+			input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
+				0, field->logical_maximum / 4,
+				cls->sn_move ?
+				field->logical_maximum / cls->sn_move : 0, 0);
+			input_abs_set_res(hi->input, ABS_MT_ORIENTATION,
+				hidinput_calc_abs_res(field,
+					ABS_MT_ORIENTATION));
+			mt_store_field(usage, td, hi);
+			return 1;
 		case HID_DG_CONTACTMAX:
 			/* we don't set td->last_slot_field as contactcount and
 			 * contact max are global to the report */
@@ -683,6 +701,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
 			int wide = (s->w > s->h);
 			int major = max(s->w, s->h);
 			int minor = min(s->w, s->h);
+			int orientation = wide;
+
+			if (s->has_azimuth)
+				orientation = s->a;
 
 			/*
 			 * divided by two to match visual scale of touch
@@ -699,7 +721,8 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
 			input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);
 			input_event(input, EV_ABS, ABS_MT_DISTANCE,
 				!s->touch_state);
-			input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
+			input_event(input, EV_ABS, ABS_MT_ORIENTATION,
+				orientation);
 			input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
 			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
 			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
@@ -789,6 +812,17 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 			break;
 		case HID_DG_CONTACTCOUNT:
 			break;
+		case HID_DG_AZIMUTH:
+			/*
+			 * Azimuth is counter-clockwise and ranges from [0, MAX)
+			 * (a full revolution). Convert it to clockwise ranging
+			 * [-MAX/2, MAX/2].
+			 */
+			if (value > field->logical_maximum / 2)
+				value -= field->logical_maximum;
+			td->curdata.a = -value;
+			td->curdata.has_azimuth = true;
+			break;
 		case HID_DG_TOUCH:
 			/* do nothing */
 			break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index ab05a86269dc..d81b9b6fd83a 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -281,6 +281,7 @@ struct hid_item {
 
 #define HID_DG_DEVICECONFIG	0x000d000e
 #define HID_DG_DEVICESETTINGS	0x000d0023
+#define HID_DG_AZIMUTH		0x000d003f
 #define HID_DG_CONFIDENCE	0x000d0047
 #define HID_DG_WIDTH		0x000d0048
 #define HID_DG_HEIGHT		0x000d0049
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: hid-multitouch: support fine-grain orientation reporting
  2017-09-27  3:03 [PATCH] HID: hid-multitouch: support fine-grain orientation reporting Wei-Ning Huang
@ 2017-09-27  5:55 ` Dmitry Torokhov
  2017-09-28  9:25   ` Wei-Ning Huang
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Torokhov @ 2017-09-27  5:55 UTC (permalink / raw)
  To: Wei-Ning Huang
  Cc: LKML, Linux Input, Dmitry Torokhov, Jiri Kosina, Wei-Ning Huang,
	Benjamin Tissoires, rydberg

Hi Wei-Ning,

On Tue, Sep 26, 2017 at 8:03 PM, Wei-Ning Huang <wnhuang@chromium.org> wrote:
>
> The current hid-multitouch driver only allow the report of two
> orientations, vertical and horizontal. We use the Azimuth orientation
> usage 0x5b under the Digitizer usage page to report orientation directly
> from the hid device. A new quirk MT_QUIRK_REPORT_ORIENTATION is added so
> user can enable this only if their device supports it.

The patch description is stale, there is no quirk anymore, and the
usage is 0x3f.

>
> Signed-off-by: Wei-Ning Huang <wnhuang@google.com>
> Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
> ---
>  drivers/hid/hid-multitouch.c | 38 ++++++++++++++++++++++++++++++++++++--
>  include/linux/hid.h          |  1 +
>  2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 440b999304a5..4663bf4b2892 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
>  #define MT_IO_FLAGS_PENDING_SLOTS      2
>
>  struct mt_slot {
> -       __s32 x, y, cx, cy, p, w, h;
> +       __s32 x, y, cx, cy, p, w, h, a;
>         __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? */
> +       bool has_azimuth;       /* the contact reports azimuth */
>  };
>
>  struct mt_class {
> @@ -591,6 +592,23 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>                         td->cc_index = field->index;
>                         td->cc_value_index = usage->usage_index;
>                         return 1;
> +               case HID_DG_AZIMUTH:
> +                       hid_map_usage(hi, usage, bit, max,
> +                               EV_ABS, ABS_MT_ORIENTATION);
> +                       /*
> +                        * Azimuth has the range of [0, MAX) representing a full
> +                        * revolution. Set ABS_MT_ORIENTATION to a quarter of
> +                        * MAX according the definition of ABS_MT_ORIENTATION
> +                        */
> +                       input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
> +                               0, field->logical_maximum / 4,
> +                               cls->sn_move ?
> +                               field->logical_maximum / cls->sn_move : 0, 0);
> +                       input_abs_set_res(hi->input, ABS_MT_ORIENTATION,
> +                               hidinput_calc_abs_res(field,
> +                                       ABS_MT_ORIENTATION));

I do not think resolution makes sense for ABS_MT_ORIENTATION. The MAX
always corresponds to 90 degrees.

> +                       mt_store_field(usage, td, hi);
> +                       return 1;
>                 case HID_DG_CONTACTMAX:
>                         /* we don't set td->last_slot_field as contactcount and
>                          * contact max are global to the report */
> @@ -683,6 +701,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>                         int wide = (s->w > s->h);
>                         int major = max(s->w, s->h);
>                         int minor = min(s->w, s->h);
> +                       int orientation = wide;
> +
> +                       if (s->has_azimuth)
> +                               orientation = s->a;
>
>                         /*
>                          * divided by two to match visual scale of touch
> @@ -699,7 +721,8 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>                         input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);
>                         input_event(input, EV_ABS, ABS_MT_DISTANCE,
>                                 !s->touch_state);
> -                       input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
> +                       input_event(input, EV_ABS, ABS_MT_ORIENTATION,
> +                               orientation);
>                         input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
>                         input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>                         input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> @@ -789,6 +812,17 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
>                         break;
>                 case HID_DG_CONTACTCOUNT:
>                         break;
> +               case HID_DG_AZIMUTH:
> +                       /*
> +                        * Azimuth is counter-clockwise and ranges from [0, MAX)
> +                        * (a full revolution). Convert it to clockwise ranging
> +                        * [-MAX/2, MAX/2].

Benjamin, Henrik, have you worked with devices reporting azimuth? I
assume MS HID spec uses 0 to represent due North, same as we have in
Linux:

https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/supporting-usages-in-digitizer-report-descriptors

> +                        */
> +                       if (value > field->logical_maximum / 2)
> +                               value -= field->logical_maximum;
> +                       td->curdata.a = -value;
> +                       td->curdata.has_azimuth = true;
> +                       break;
>                 case HID_DG_TOUCH:
>                         /* do nothing */
>                         break;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index ab05a86269dc..d81b9b6fd83a 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -281,6 +281,7 @@ struct hid_item {
>
>  #define HID_DG_DEVICECONFIG    0x000d000e
>  #define HID_DG_DEVICESETTINGS  0x000d0023
> +#define HID_DG_AZIMUTH         0x000d003f
>  #define HID_DG_CONFIDENCE      0x000d0047
>  #define HID_DG_WIDTH           0x000d0048
>  #define HID_DG_HEIGHT          0x000d0049
> --
> 2.12.2
>

Thanks,
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: hid-multitouch: support fine-grain orientation reporting
  2017-09-27  5:55 ` Dmitry Torokhov
@ 2017-09-28  9:25   ` Wei-Ning Huang
  0 siblings, 0 replies; 3+ messages in thread
From: Wei-Ning Huang @ 2017-09-28  9:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: LKML, Linux Input, Jiri Kosina, Benjamin Tissoires, rydberg

Hi Dmitry,

I sent v2 to fix the commit message and orientation. Thanks.
https://patchwork.kernel.org/patch/9975565/

Wei-Ning

On Wed, Sep 27, 2017 at 1:55 PM, Dmitry Torokhov <dtor@chromium.org> wrote:
> Hi Wei-Ning,
>
> On Tue, Sep 26, 2017 at 8:03 PM, Wei-Ning Huang <wnhuang@chromium.org> wrote:
>>
>> The current hid-multitouch driver only allow the report of two
>> orientations, vertical and horizontal. We use the Azimuth orientation
>> usage 0x5b under the Digitizer usage page to report orientation directly
>> from the hid device. A new quirk MT_QUIRK_REPORT_ORIENTATION is added so
>> user can enable this only if their device supports it.
>
> The patch description is stale, there is no quirk anymore, and the
> usage is 0x3f.
>
>>
>> Signed-off-by: Wei-Ning Huang <wnhuang@google.com>
>> Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
>> ---
>>  drivers/hid/hid-multitouch.c | 38 ++++++++++++++++++++++++++++++++++++--
>>  include/linux/hid.h          |  1 +
>>  2 files changed, 37 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 440b999304a5..4663bf4b2892 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
>>  #define MT_IO_FLAGS_PENDING_SLOTS      2
>>
>>  struct mt_slot {
>> -       __s32 x, y, cx, cy, p, w, h;
>> +       __s32 x, y, cx, cy, p, w, h, a;
>>         __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? */
>> +       bool has_azimuth;       /* the contact reports azimuth */
>>  };
>>
>>  struct mt_class {
>> @@ -591,6 +592,23 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>>                         td->cc_index = field->index;
>>                         td->cc_value_index = usage->usage_index;
>>                         return 1;
>> +               case HID_DG_AZIMUTH:
>> +                       hid_map_usage(hi, usage, bit, max,
>> +                               EV_ABS, ABS_MT_ORIENTATION);
>> +                       /*
>> +                        * Azimuth has the range of [0, MAX) representing a full
>> +                        * revolution. Set ABS_MT_ORIENTATION to a quarter of
>> +                        * MAX according the definition of ABS_MT_ORIENTATION
>> +                        */
>> +                       input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
>> +                               0, field->logical_maximum / 4,
>> +                               cls->sn_move ?
>> +                               field->logical_maximum / cls->sn_move : 0, 0);
>> +                       input_abs_set_res(hi->input, ABS_MT_ORIENTATION,
>> +                               hidinput_calc_abs_res(field,
>> +                                       ABS_MT_ORIENTATION));
>
> I do not think resolution makes sense for ABS_MT_ORIENTATION. The MAX
> always corresponds to 90 degrees.
>
>> +                       mt_store_field(usage, td, hi);
>> +                       return 1;
>>                 case HID_DG_CONTACTMAX:
>>                         /* we don't set td->last_slot_field as contactcount and
>>                          * contact max are global to the report */
>> @@ -683,6 +701,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>>                         int wide = (s->w > s->h);
>>                         int major = max(s->w, s->h);
>>                         int minor = min(s->w, s->h);
>> +                       int orientation = wide;
>> +
>> +                       if (s->has_azimuth)
>> +                               orientation = s->a;
>>
>>                         /*
>>                          * divided by two to match visual scale of touch
>> @@ -699,7 +721,8 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>>                         input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);
>>                         input_event(input, EV_ABS, ABS_MT_DISTANCE,
>>                                 !s->touch_state);
>> -                       input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
>> +                       input_event(input, EV_ABS, ABS_MT_ORIENTATION,
>> +                               orientation);
>>                         input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
>>                         input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>>                         input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
>> @@ -789,6 +812,17 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
>>                         break;
>>                 case HID_DG_CONTACTCOUNT:
>>                         break;
>> +               case HID_DG_AZIMUTH:
>> +                       /*
>> +                        * Azimuth is counter-clockwise and ranges from [0, MAX)
>> +                        * (a full revolution). Convert it to clockwise ranging
>> +                        * [-MAX/2, MAX/2].
>
> Benjamin, Henrik, have you worked with devices reporting azimuth? I
> assume MS HID spec uses 0 to represent due North, same as we have in
> Linux:
>
> https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/supporting-usages-in-digitizer-report-descriptors
>
>> +                        */
>> +                       if (value > field->logical_maximum / 2)
>> +                               value -= field->logical_maximum;
>> +                       td->curdata.a = -value;
>> +                       td->curdata.has_azimuth = true;
>> +                       break;
>>                 case HID_DG_TOUCH:
>>                         /* do nothing */
>>                         break;
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index ab05a86269dc..d81b9b6fd83a 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -281,6 +281,7 @@ struct hid_item {
>>
>>  #define HID_DG_DEVICECONFIG    0x000d000e
>>  #define HID_DG_DEVICESETTINGS  0x000d0023
>> +#define HID_DG_AZIMUTH         0x000d003f
>>  #define HID_DG_CONFIDENCE      0x000d0047
>>  #define HID_DG_WIDTH           0x000d0048
>>  #define HID_DG_HEIGHT          0x000d0049
>> --
>> 2.12.2
>>
>
> Thanks,
> Dmitry



-- 
Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
wnhuang@google.com | Cell: +886 910-380678

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-09-28  9:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-27  3:03 [PATCH] HID: hid-multitouch: support fine-grain orientation reporting Wei-Ning Huang
2017-09-27  5:55 ` Dmitry Torokhov
2017-09-28  9:25   ` Wei-Ning Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox