| Message ID | 20171010041631.22093-1-wnhuang@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Wei-Ning, > The current hid-multitouch driver only allow the report of two > orientations, vertical and horizontal. We use the Azimuth orientation > usage 0x3F under the Digitizer usage page to report orientation if the > device supports it. > > Changelog: > v1 -> v2: > - Fix commit message. > - Remove resolution reporting for ABS_MT_ORIENTATION. > v2 -> v3: > - Fix commit message. > v3 -> v4: > - Fix ABS_MT_ORIENTATION ABS param range. > - Don't set ABS_MT_ORIENTATION in ABS_DG_HEIGHT when it is already > set by ABS_DG_AZIMUTH. > > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> > Reviewed-by: Dmitry Torokhov <dtor@chromium.org> > --- > drivers/hid/hid-multitouch.c | 52 ++++++++++++++++++++++++++++++++++++++++---- > include/linux/hid.h | 1 + > 2 files changed, 49 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > index 440b999304a5..3317dae64ef7 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 { > @@ -571,8 +572,15 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi, > if (!(cls->quirks & MT_QUIRK_NO_AREA)) { > set_abs(hi->input, ABS_MT_TOUCH_MINOR, field, > cls->sn_height); > - input_set_abs_params(hi->input, > - ABS_MT_ORIENTATION, 0, 1, 0, 0); > + > + /* > + * Only set ABS_MT_ORIENTATION if it is not > + * already set by the HID_DG_AZIMUTH usage. > + */ > + if (!test_bit(ABS_MT_ORIENTATION, > + hi->input->absbit)) > + input_set_abs_params(hi->input, > + ABS_MT_ORIENTATION, 0, 1, 0, 0); > } > mt_store_field(usage, td, hi); > return 1; > @@ -591,6 +599,21 @@ 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, > + -field->logical_maximum / 4, > + field->logical_maximum / 4, > + cls->sn_move ? > + field->logical_maximum / cls->sn_move : 0, 0); > + 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 +706,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 +726,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 +817,22 @@ 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]. > + * > + * Note that ABS_MT_ORIENTATION require us to report > + * the limit of [-MAX/4, MAX/4], but the value can go > + * out of range to [-MAX/2, MAX/2] to report an upside > + * down ellipsis. > + */ > + 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 > Acked-by: Henrik Rydberg <rydberg@bitmath.org> This version looks good - thank you Wei-Ning, thank you Benjamin. Henrik -- 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
On Oct 10 2017 or thereabouts, Wei-Ning Huang wrote: > From: Wei-Ning Huang <wnhuang@chromium.org> > > The current hid-multitouch driver only allow the report of two > orientations, vertical and horizontal. We use the Azimuth orientation > usage 0x3F under the Digitizer usage page to report orientation if the > device supports it. > > Changelog: > v1 -> v2: > - Fix commit message. > - Remove resolution reporting for ABS_MT_ORIENTATION. > v2 -> v3: > - Fix commit message. > v3 -> v4: > - Fix ABS_MT_ORIENTATION ABS param range. > - Don't set ABS_MT_ORIENTATION in ABS_DG_HEIGHT when it is already > set by ABS_DG_AZIMUTH. > > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> > Reviewed-by: Dmitry Torokhov <dtor@chromium.org> Looks good now: Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Cheers, Benjamin > --- > drivers/hid/hid-multitouch.c | 52 ++++++++++++++++++++++++++++++++++++++++---- > include/linux/hid.h | 1 + > 2 files changed, 49 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > index 440b999304a5..3317dae64ef7 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 { > @@ -571,8 +572,15 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi, > if (!(cls->quirks & MT_QUIRK_NO_AREA)) { > set_abs(hi->input, ABS_MT_TOUCH_MINOR, field, > cls->sn_height); > - input_set_abs_params(hi->input, > - ABS_MT_ORIENTATION, 0, 1, 0, 0); > + > + /* > + * Only set ABS_MT_ORIENTATION if it is not > + * already set by the HID_DG_AZIMUTH usage. > + */ > + if (!test_bit(ABS_MT_ORIENTATION, > + hi->input->absbit)) > + input_set_abs_params(hi->input, > + ABS_MT_ORIENTATION, 0, 1, 0, 0); > } > mt_store_field(usage, td, hi); > return 1; > @@ -591,6 +599,21 @@ 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, > + -field->logical_maximum / 4, > + field->logical_maximum / 4, > + cls->sn_move ? > + field->logical_maximum / cls->sn_move : 0, 0); > + 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 +706,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 +726,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 +817,22 @@ 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]. > + * > + * Note that ABS_MT_ORIENTATION require us to report > + * the limit of [-MAX/4, MAX/4], but the value can go > + * out of range to [-MAX/2, MAX/2] to report an upside > + * down ellipsis. > + */ > + 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 > -- 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
On Mon, Oct 9, 2017 at 11:57 PM, Henrik Rydberg <rydberg@bitmath.org> wrote: > Hi Wei-Ning, > >> The current hid-multitouch driver only allow the report of two >> orientations, vertical and horizontal. We use the Azimuth orientation >> usage 0x3F under the Digitizer usage page to report orientation if the >> device supports it. >> >> Changelog: >> v1 -> v2: >> - Fix commit message. >> - Remove resolution reporting for ABS_MT_ORIENTATION. >> v2 -> v3: >> - Fix commit message. >> v3 -> v4: >> - Fix ABS_MT_ORIENTATION ABS param range. >> - Don't set ABS_MT_ORIENTATION in ABS_DG_HEIGHT when it is already >> set by ABS_DG_AZIMUTH. >> >> Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> >> Reviewed-by: Dmitry Torokhov <dtor@chromium.org> >> --- >> drivers/hid/hid-multitouch.c | 52 ++++++++++++++++++++++++++++++++++++++++---- >> include/linux/hid.h | 1 + >> 2 files changed, 49 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c >> index 440b999304a5..3317dae64ef7 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 { >> @@ -571,8 +572,15 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi, >> if (!(cls->quirks & MT_QUIRK_NO_AREA)) { >> set_abs(hi->input, ABS_MT_TOUCH_MINOR, field, >> cls->sn_height); >> - input_set_abs_params(hi->input, >> - ABS_MT_ORIENTATION, 0, 1, 0, 0); >> + >> + /* >> + * Only set ABS_MT_ORIENTATION if it is not >> + * already set by the HID_DG_AZIMUTH usage. >> + */ >> + if (!test_bit(ABS_MT_ORIENTATION, >> + hi->input->absbit)) >> + input_set_abs_params(hi->input, >> + ABS_MT_ORIENTATION, 0, 1, 0, 0); >> } >> mt_store_field(usage, td, hi); >> return 1; >> @@ -591,6 +599,21 @@ 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, >> + -field->logical_maximum / 4, >> + field->logical_maximum / 4, So do we expect userspace to have special handling for the range when "min" is negative? I.e. when range is [0,1] it is understood that we are reporting the first quadrant only, with 0 reported when finger is roughly vertical and 1 is when it is horizontal. Similarly, the range [0-90] would describe the 1st quadrant as well. This matches the in-kernel documentation. However here we have [-90, 90] that describes 2 quadrants. Do we want to keep it as is and have userspace adapt (we probably will need a patch to multi-touch-protocol.txt), or should this be: input_set_abs_params(hi->input, ABS_MT_ORIENTATION, 0, field->logical_maximum / 4, ...); and userspace should be able to handle reported negative events and have them being understood as going counter-clockwise into the 4th and then 3rd quadrant? Thanks, Dmitry -- 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
On Oct 10 2017 or thereabouts, Dmitry Torokhov wrote: > On Mon, Oct 9, 2017 at 11:57 PM, Henrik Rydberg <rydberg@bitmath.org> wrote: > > Hi Wei-Ning, > > > >> The current hid-multitouch driver only allow the report of two > >> orientations, vertical and horizontal. We use the Azimuth orientation > >> usage 0x3F under the Digitizer usage page to report orientation if the > >> device supports it. > >> > >> Changelog: > >> v1 -> v2: > >> - Fix commit message. > >> - Remove resolution reporting for ABS_MT_ORIENTATION. > >> v2 -> v3: > >> - Fix commit message. > >> v3 -> v4: > >> - Fix ABS_MT_ORIENTATION ABS param range. > >> - Don't set ABS_MT_ORIENTATION in ABS_DG_HEIGHT when it is already > >> set by ABS_DG_AZIMUTH. > >> > >> Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> > >> Reviewed-by: Dmitry Torokhov <dtor@chromium.org> > >> --- > >> drivers/hid/hid-multitouch.c | 52 ++++++++++++++++++++++++++++++++++++++++---- > >> include/linux/hid.h | 1 + > >> 2 files changed, 49 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > >> index 440b999304a5..3317dae64ef7 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 { > >> @@ -571,8 +572,15 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi, > >> if (!(cls->quirks & MT_QUIRK_NO_AREA)) { > >> set_abs(hi->input, ABS_MT_TOUCH_MINOR, field, > >> cls->sn_height); > >> - input_set_abs_params(hi->input, > >> - ABS_MT_ORIENTATION, 0, 1, 0, 0); > >> + > >> + /* > >> + * Only set ABS_MT_ORIENTATION if it is not > >> + * already set by the HID_DG_AZIMUTH usage. > >> + */ > >> + if (!test_bit(ABS_MT_ORIENTATION, > >> + hi->input->absbit)) > >> + input_set_abs_params(hi->input, > >> + ABS_MT_ORIENTATION, 0, 1, 0, 0); > >> } > >> mt_store_field(usage, td, hi); > >> return 1; > >> @@ -591,6 +599,21 @@ 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, > >> + -field->logical_maximum / 4, > >> + field->logical_maximum / 4, > > > So do we expect userspace to have special handling for the range when > "min" is negative? I.e. when range is [0,1] it is understood that we > are reporting the first quadrant only, with 0 reported when finger is > roughly vertical and 1 is when it is horizontal. Similarly, the range > [0-90] would describe the 1st quadrant as well. This matches the > in-kernel documentation. However here we have [-90, 90] that describes > 2 quadrants. Do we want to keep it as is and have userspace adapt (we > probably will need a patch to multi-touch-protocol.txt), or should Actually, I requested the [-90, 90] change because the only other user in the kernel I could find that support ABS_MT_ORIENTATION with a different range than [0,1] was hid-magicmouse and it was not following the documentation to the letter: https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git/tree/drivers/hid/hid-magicmouse.c?h=for-next#n411 After a deeper search, we have (reporting ABS_MT_ORIENTATION different from [0,1]): drivers/hid/hid-magicmouse.c -> [-32,32] drivers/input/touchscreen/stmfts.c -> [0,255] drivers/input/touchscreen/atmel_mxt_ts.c -> [0,255] drivers/input/mouse/bcm5974.c -> [-16384, 16384] drivers/input/mouse/cyapa.c -> [-127, 127] drivers/input/misc/xen-kbdfront.c -> ???? (set_abs_params is not even called) So we clearly already have messed up everywhere. I suspect the [0,255] ranges to be the min/max already reported by the touchscreen. I am not sure if libinput even uses ABS_MT_ORIENTATION, but I'd go for fixing the documentation. And re-reading it, it's not clear that the doc tells us to have [0,90]. It mentions negative values and out of ranges too, so we might just as well simply clarify that we rather have [-90,90], with 0 being "north". Cheers, Benjamin > this be: > > input_set_abs_params(hi->input, ABS_MT_ORIENTATION, 0, > field->logical_maximum / 4, ...); > > and userspace should be able to handle reported negative events and > have them being understood as going counter-clockwise into the 4th and > then 3rd quadrant? > > Thanks, > Dmitry -- 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
On Wed, 11 Oct 2017, Benjamin Tissoires wrote: > I am not sure if libinput even uses ABS_MT_ORIENTATION I don't think it does, so that should be okay. However ... > , but I'd go for fixing the documentation. And re-reading it, it's not > clear that the doc tells us to have [0,90]. It mentions negative values > and out of ranges too, so we might just as well simply clarify that we > rather have [-90,90], with 0 being "north". ... I'd like the documentation fix to go in together in one go with this patch if possible. Thanks,
On Oct 11 2017 or thereabouts, Jiri Kosina wrote: > On Wed, 11 Oct 2017, Benjamin Tissoires wrote: > > > I am not sure if libinput even uses ABS_MT_ORIENTATION > > I don't think it does, so that should be okay. However ... I had a meeting this Peter at noon today. The summary is that libinput doesn't uses ABS_MT_ORIENTATION, and that the documentation requires actually 3 things: - 0 is Y-aligned, up ("north") - maximum should be aligned with X, pointing toward the right ("east") - negative and out of range values are allowed From this, we can conclude that the minimum doesn't matter, as long as it is 0 or -max, it is the same from the user space point of view. One thing that the documentation suggests is that if we report [0, max], this would indicate that out of ranges values won't be triggered, and [-max, max] would seem to indicate that the data might be negative and so out of range values would be acceptable. Anyway, no changes in any cases from userspace. > > > , but I'd go for fixing the documentation. And re-reading it, it's not > > clear that the doc tells us to have [0,90]. It mentions negative values > > and out of ranges too, so we might just as well simply clarify that we > > rather have [-90,90], with 0 being "north". > > ... I'd like the documentation fix to go in together in one go with this > patch if possible. > Sounds like a plan. Cheers, Benjamin > Thanks, > > -- > Jiri Kosina > SUSE Labs > -- 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
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 440b999304a5..3317dae64ef7 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 { @@ -571,8 +572,15 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi, if (!(cls->quirks & MT_QUIRK_NO_AREA)) { set_abs(hi->input, ABS_MT_TOUCH_MINOR, field, cls->sn_height); - input_set_abs_params(hi->input, - ABS_MT_ORIENTATION, 0, 1, 0, 0); + + /* + * Only set ABS_MT_ORIENTATION if it is not + * already set by the HID_DG_AZIMUTH usage. + */ + if (!test_bit(ABS_MT_ORIENTATION, + hi->input->absbit)) + input_set_abs_params(hi->input, + ABS_MT_ORIENTATION, 0, 1, 0, 0); } mt_store_field(usage, td, hi); return 1; @@ -591,6 +599,21 @@ 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, + -field->logical_maximum / 4, + field->logical_maximum / 4, + cls->sn_move ? + field->logical_maximum / cls->sn_move : 0, 0); + 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 +706,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 +726,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 +817,22 @@ 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]. + * + * Note that ABS_MT_ORIENTATION require us to report + * the limit of [-MAX/4, MAX/4], but the value can go + * out of range to [-MAX/2, MAX/2] to report an upside + * down ellipsis. + */ + 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