| Message ID | 20241223190416.52871-1-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | Input: atmel_mxt_ts - support KoD knob | expand |
On 1/13/25 8:43 PM, Dmitry Torokhov wrote: > On Mon, Dec 23, 2024 at 08:03:24PM +0100, Marek Vasut wrote: >> Add support for T152 KoD knob events [1]. The KoD touch controller >> family supports up to two knobs attached to the glass. Each knob can >> be turned in either direction and the touch controller processes the >> event and reports the knob position for each knob. Each knob is also >> pressure sensitive, the pressure is reported as well. Each knob also >> supports center press and additional buttons, which are reported as >> BTN_0/BTN_1 for the center press for each knob, and BTN_A/BTN_B for >> the additional buttons on the knob. >> >> The knob is similar to Dell Canvas 27 knob already supported by >> hid-multitouch, except it is non-removable and there can be up to >> two such knobs . >> >> This implementation is extracted and heavily reworked from Atmel >> downstream patchset work by Michael <mksgong@gmail.com> from [2] >> branch master as of commit 9c77fbf32982 ("Merge pull request #35 >> from atmel-maxtouch/20240103_HA_protocol_fixes"). >> >> [1] https://www.microchip.com/en-us/products/touch-and-gesture/maxtouch-touchscreen-controllers/kod-family >> [2] https://github.com/atmel-maxtouch/maXTouch_linux >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> --- >> Note: I am not sure whether reporting the second wheel as GAS/REL_HWHEEL >> is the right thing to do, I don't think it is. Maybe there is some >> better way to handle multiple knobs ? > > How about creating separate input devices for these? This is what I had originally, but ... why ? This is a single input device, touchscreen with up to two knobs , so why would it be multiple input devices ?
On Thu, 16 Jan 2025 at 03:03, Marek Vasut <marex@denx.de> wrote: > > On 1/16/25 2:01 AM, Dmitry Torokhov wrote: > > On Mon, Jan 13, 2025 at 09:08:47PM +0100, Marek Vasut wrote: > >> On 1/13/25 8:43 PM, Dmitry Torokhov wrote: > >>> How about creating separate input devices for these? > >> This is what I had originally, but ... why ? > >> > >> This is a single input device, touchscreen with up to two knobs , so why > >> would it be multiple input devices ? > > > > So as you can see it is hard to express the knobs purpose within a > > single input device. Additionally (as far as I understand) knobs are > > not connected to the touchscreen function but rather rotary encoders > > just happened to be mounted on the touchscreen. They are not considered > > contacts. > > From the touch controller perspective, they are contacts. > > > Therefore I think it makes sense to report them as 2 separate input > > devices (maybe modeling after how drivers/input/misc/rotary-encoder.c > > does things). > Not really, the knobs also act as buttons, so the user might navigate a > finger on the touch surface to point to an object, and turn or press the > knob to trigger some action. This is similar to the Dell Canvas 27 knob > already mentioned above, except that one was not glued to the glass, it > was movable. As an observation: atmel_mxt_ts already supports the T15 key array. This turns an area of the matrix into a series of keys, which the driver registers and will report in mxt_proc_t15_messages(). I think we see these as being similar to scrollwheels or side buttons on a mouse - they are very associated with the pointing device itself. Nick
Resending: Plain text mode: Some clarifications:the two knobs on the display are configured to be predefined widget areas/shapes. In this case, two pushable knob dials with a predefined center area which acts as a push button. Current knobs report dents or an absolute position, relative data and graduations (although graduations are not well defined yet). Idea was to create two separate input devices (i.e. to associate all reports to these input devices) as the knobs are separately reporting wheel type data, although are part of the same touchscreen. Not clear what ABS_GAS event type is and assumed that REL_HWHEEL was more of a mouse related event code. > > On Thu, Jan 16, 2025 at 1:59 AM Nick Dyer <nick@shmanahar.org> wrote: >> >> On Thu, 16 Jan 2025 at 03:03, Marek Vasut <marex@denx.de> wrote: >> > >> > On 1/16/25 2:01 AM, Dmitry Torokhov wrote: >> > > On Mon, Jan 13, 2025 at 09:08:47PM +0100, Marek Vasut wrote: >> > >> On 1/13/25 8:43 PM, Dmitry Torokhov wrote: >> > >>> How about creating separate input devices for these? >> > >> This is what I had originally, but ... why ? >> > >> >> > >> This is a single input device, touchscreen with up to two knobs , so why >> > >> would it be multiple input devices ? >> > > >> > > So as you can see it is hard to express the knobs purpose within a >> > > single input device. Additionally (as far as I understand) knobs are >> > > not connected to the touchscreen function but rather rotary encoders >> > > just happened to be mounted on the touchscreen. They are not considered >> > > contacts. >> > >> > From the touch controller perspective, they are contacts. >> > >> > > Therefore I think it makes sense to report them as 2 separate input >> > > devices (maybe modeling after how drivers/input/misc/rotary-encoder.c >> > > does things). >> > Not really, the knobs also act as buttons, so the user might navigate a >> > finger on the touch surface to point to an object, and turn or press the >> > knob to trigger some action. This is similar to the Dell Canvas 27 knob >> > already mentioned above, except that one was not glued to the glass, it >> > was movable. >> >> As an observation: atmel_mxt_ts already supports the T15 key array. >> This turns an area of the matrix into a series of keys, which the >> driver registers and will report in mxt_proc_t15_messages(). >> >> I think we see these as being similar to scrollwheels or side buttons >> on a mouse - they are very associated with the pointing device itself. >> >> Nick
On 1/20/25 7:53 AM, Michael Gong wrote: > Resending: Plain text mode: Hi, both emails made it through. > Some clarifications:the two knobs on the display are configured to be > predefined widget areas/shapes. > In this case, two pushable knob dials with a predefined center area > which acts as a push button. > Current knobs report dents or an absolute position, relative data and > graduations (although graduations are not well defined yet). > > Idea was to create two separate input devices (i.e. to associate all > reports to these input devices) as the knobs are separately reporting > wheel type data, although are part of the same touchscreen. Indeed, I saw the downstream driver. > Not clear what ABS_GAS event type is and assumed that REL_HWHEEL was > more of a mouse related event code. GAS is a gas pedal , like in a car , what would really be needed is wheel2 code I think.
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index 6cd1c87eb1877..04b979e1501d2 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -75,6 +75,7 @@ #define MXT_SPT_CTECONFIG_T46 46 #define MXT_SPT_DYNAMICCONFIGURATIONCONTAINER_T71 71 #define MXT_TOUCH_MULTITOUCHSCREEN_T100 100 +#define MXT_SPT_WIDGET_T152 152 /* MXT_GEN_MESSAGE_T5 object */ #define MXT_RPTID_NOMSG 0xff @@ -186,6 +187,22 @@ enum t100_type { MXT_T100_TYPE_LARGE_TOUCH = 6, }; +/* T152 Message Types */ +#define MXT_BUTTON_KNOB 0x02 +#define MXT_POSITION_KNOB 0x03 + +/* T152 Widget Object */ +#define MXT_T152_MAXDETENT 0x0a + +/* T152 Knob masks */ +#define MXT_KNOB_TYPE_MASK 0x07 +#define MXT_KNOB_EVENT_MASK 0x07 + +/* T152 Position Events */ +#define MXT_POS_PUSH_MOVE 0x03 +#define MXT_POS_PUSH 0x04 +#define MXT_POS_UNPUSH 0x06 + #define MXT_DISTANCE_ACTIVE_TOUCH 0 #define MXT_DISTANCE_HOVERING 1 @@ -336,6 +353,10 @@ struct mxt_data { u8 T97_reportid_max; u8 T100_reportid_min; u8 T100_reportid_max; + u16 T152_address; + u8 T152_reportid_min; + u8 T152_reportid_max; + u8 T152_obj_size; /* for fw update in bootloader */ struct completion bl_completion; @@ -1026,6 +1047,41 @@ static void mxt_proc_t100_message(struct mxt_data *data, u8 *message) data->update_input = true; } +static void mxt_proc_t152_messages(struct mxt_data *data, u8 *message) +{ + struct input_dev *input_dev = data->input_dev; + int id = message[0] - data->T152_reportid_min; + u8 msg_type = message[1] & MXT_KNOB_TYPE_MASK; + u16 state; + + if (msg_type == MXT_BUTTON_KNOB) { + state = get_unaligned_le16(&message[5]); + input_report_key(data->input_dev, BTN_0, !!(state & BIT(0))); + input_report_key(data->input_dev, BTN_1, !!(state & BIT(1))); + input_sync(data->input_dev); + } else if (msg_type == MXT_POSITION_KNOB) { + input_mt_report_slot_state(input_dev, MT_TOOL_DIAL, 1); + + input_report_abs(input_dev, id ? ABS_GAS : ABS_WHEEL, + get_unaligned_le16(&message[5])); + input_report_abs(input_dev, ABS_PRESSURE, message[8]); + + /* Knob turn CCW is 0xffff or -1 , CW is 1 */ + input_report_rel(input_dev, id ? REL_HWHEEL : REL_WHEEL, + (s16)get_unaligned_le16(&message[3])); + + state = message[2] & MXT_KNOB_EVENT_MASK; + if (state == MXT_POS_PUSH || state == MXT_POS_PUSH_MOVE) + input_report_key(input_dev, id ? BTN_B : BTN_A, 1); + else if (state == MXT_POS_UNPUSH) + input_report_key(input_dev, id ? BTN_B : BTN_A, 0); + + input_mt_report_slot_inactive(input_dev); + + input_sync(input_dev); + } +} + static int mxt_proc_message(struct mxt_data *data, u8 *message) { u8 report_id = message[0]; @@ -1053,6 +1109,9 @@ static int mxt_proc_message(struct mxt_data *data, u8 *message) } else if (report_id >= data->T100_reportid_min && report_id <= data->T100_reportid_max) { mxt_proc_t100_message(data, message); + } else if (report_id >= data->T152_reportid_min && + report_id <= data->T152_reportid_max) { + mxt_proc_t152_messages(data, message); } else if (report_id == data->T19_reportid) { mxt_input_button(data, message); data->update_input = true; @@ -1747,6 +1806,8 @@ static void mxt_free_object_table(struct mxt_data *data) data->T97_reportid_max = 0; data->T100_reportid_min = 0; data->T100_reportid_max = 0; + data->T152_reportid_min = 0; + data->T152_reportid_max = 0; data->max_reportid = 0; } @@ -1841,6 +1902,12 @@ static int mxt_parse_object_table(struct mxt_data *data, /* first two report IDs reserved */ data->num_touchids = object->num_report_ids - 2; break; + case MXT_SPT_WIDGET_T152: + data->T152_address = object->start_address; + data->T152_reportid_min = min_id; + data->T152_reportid_max = max_id; + data->T152_obj_size = mxt_obj_size(object); + break; } end_address = object->start_address @@ -2235,6 +2302,50 @@ static int mxt_initialize_input_device(struct mxt_data *data) EV_KEY, data->t15_keymap[i]); } + /* For T152 knobs */ + if (data->T152_address) { + u8 detent_ofs = data->T152_address + MXT_T152_MAXDETENT; + u8 bufp, bufs; + + error = __mxt_read_reg(data->client, detent_ofs, 1, &bufp); + if (error) + goto err_free_mem; + + error = __mxt_read_reg(data->client, detent_ofs + data->T152_obj_size, + 1, &bufs); + if (error) + goto err_free_mem; + + if (bufp || bufs) { + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, + 0, MT_TOOL_MAX, 0, 0); + + /* Pressure reporting on both knobs */ + input_set_capability(input_dev, EV_ABS, ABS_PRESSURE); + input_set_abs_params(input_dev, ABS_PRESSURE, 0, 255, 0, 0); + } + + /* First knob */ + if (bufp) { + input_set_capability(input_dev, EV_KEY, BTN_0); /* Center */ + input_set_capability(input_dev, EV_KEY, BTN_A); /* Push */ + input_set_capability(input_dev, EV_REL, REL_WHEEL); + input_set_capability(input_dev, EV_ABS, ABS_WHEEL); + input_set_abs_params(input_dev, ABS_WHEEL, 0, bufp - 1, 0, 0); + input_dev->relbit[0] |= BIT_MASK(REL_WHEEL); + } + + /* Second knob */ + if (bufs) { + input_set_capability(input_dev, EV_KEY, BTN_1); /* Center */ + input_set_capability(input_dev, EV_KEY, BTN_B); /* Push */ + input_set_capability(input_dev, EV_REL, REL_HWHEEL); + input_set_capability(input_dev, EV_ABS, ABS_GAS); + input_set_abs_params(input_dev, ABS_GAS, 0, bufs - 1, 0, 0); + input_dev->relbit[0] |= BIT_MASK(REL_HWHEEL); + } + } + input_set_drvdata(input_dev, data); error = input_register_device(input_dev);
Add support for T152 KoD knob events [1]. The KoD touch controller family supports up to two knobs attached to the glass. Each knob can be turned in either direction and the touch controller processes the event and reports the knob position for each knob. Each knob is also pressure sensitive, the pressure is reported as well. Each knob also supports center press and additional buttons, which are reported as BTN_0/BTN_1 for the center press for each knob, and BTN_A/BTN_B for the additional buttons on the knob. The knob is similar to Dell Canvas 27 knob already supported by hid-multitouch, except it is non-removable and there can be up to two such knobs . This implementation is extracted and heavily reworked from Atmel downstream patchset work by Michael <mksgong@gmail.com> from [2] branch master as of commit 9c77fbf32982 ("Merge pull request #35 from atmel-maxtouch/20240103_HA_protocol_fixes"). [1] https://www.microchip.com/en-us/products/touch-and-gesture/maxtouch-touchscreen-controllers/kod-family [2] https://github.com/atmel-maxtouch/maXTouch_linux Signed-off-by: Marek Vasut <marex@denx.de> --- Note: I am not sure whether reporting the second wheel as GAS/REL_HWHEEL is the right thing to do, I don't think it is. Maybe there is some better way to handle multiple knobs ? --- Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Michael <mksgong@gmail.com> Cc: Nick Dyer <nick@shmanahar.org> Cc: linux-input@vger.kernel.org --- drivers/input/touchscreen/atmel_mxt_ts.c | 111 +++++++++++++++++++++++ 1 file changed, 111 insertions(+)