Re: [PATCH] HID: hid-multitouch: support fine-grain orientation reporting

From: Wei-Ning Huang
Date: Thu Sep 28 2017 - 05:25:44 EST


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@xxxxxxxxxxxx> wrote:
> Hi Wei-Ning,
>
> On Tue, Sep 26, 2017 at 8:03 PM, Wei-Ning Huang <wnhuang@xxxxxxxxxxxx> 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@xxxxxxxxxx>
>> Reviewed-by: Dmitry Torokhov <dtor@xxxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxx | Cell: +886 910-380678