input - wacom : support one finger touch the touchscreen way
PATCH 3/4 input - wacom : support one finger touch the touchscreen way
Quoted 98 lines · Henrik Rydberg
Hi Ping,
Quoted 9 lines · Ping Cheng
There are two types of 1FGT devices supported in
wacom_wac.c. Changing them to follow the existing touchscreen format, i.e., only reportBTN_TOUCHas a valid tool type.Touch data will be ignored if pen is in proximity. This requires a touch up sent if touch was down when pen comes in. The touch up event should be sent before any pen events emitted. Otherwise, two pointers would race for the cursor.
Quoted 8 lines
However, we can not send a touch up inside
wacom_tpc_pensince pen and touch are on different logical port. That is why we have to check if touch is up before sending pen events.Reviewed-by Chris Bagwell <[email protected]>Signed-off-by Ping Cheng <[email protected]>--- drivers/input/tablet/wacom_wac.c | 125+++++++++++++------------------------
Review
Quoted 4 lines
drivers/input/tablet/wacom_wac.h | 1 + 2 files changed, 45 insertions(+), 81 deletions(-)(quoted diff) +0
-0
b/drivers/input/tablet/wacom_wac.cdrivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac
Quoted 4 lines
index 15bab99..fc0669d 100644
drivers/input/tablet/wacom_wac.c +0
-0@@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac*wacom)
Quoted 33 lines
drivers/input/tablet/wacom_wac.c +4
-22return 1 ; } -static void wacom_tpc_touch_out (struct wacom_wac *wacom,int idx) -{ -struct input_dev *input = wacom->input; -int finger = idx +1 ; - -input_report_abs (input,ABS_X ,0 ); -input_report_abs (input,ABS_Y ,0 ); -input_report_abs (input,ABS_MISC ,0 ); -input_report_key (input, wacom->tool[finger],0 ); -if (!idx) -input_report_key (input,BTN_TOUCH ,0 ); -input_event (input,EV_MSC ,MSC_SERIAL , finger); -input_sync (input); -} - -static void wacom_tpc_touch_in (struct wacom_wac *wacom,size_t len) +static int wacom_tpc_single_touch (struct wacom_wac *wacom,size_t len) {char *data = wacom->data;struct input_dev *input = wacom->input; +bool prox =0 ; - wacom->tool[1 ] =BTN_TOOL_DOUBLETAP ; - wacom->id[0 ] =TOUCH_DEVICE_ID ; - -if (len !=WACOM_PKGLEN_TPC1FG ) { - -switch (data[0 ]) { +if (!wacom->shared->stylus_in_proximity) { +if (len ==WACOM_PKGLEN_TPC1FG )drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac · line 706
Quoted 10 lines
drivers/input/tablet/wacom_wac.c +7
-2+ prox = data[0 ] &0x01 ; + else /* with capacity */ + prox = data[1 ] &0x01 ; + }else if (wacom->shared->touch_down) + prox =0 ; +else +return 0 ; -case WACOM_REPORT_TPC1FG : - input_report_abs(input, ABS_X, le16_to_cpup((__le16*)&data[2]));
drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac · line 707
Quoted 1 line
drivers/input/tablet/wacom_wac.c +0
-1- input_report_abs(input, ABS_Y, le16_to_cpup((__le16*)&data[4]));
drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac · line 708
Quoted 1 line
drivers/input/tablet/wacom_wac.c +0
-1- input_report_abs(input, ABS_PRESSURE,
le16_to_cpup((__le16 *)&data[6]));drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac · line 709
Quoted 1 line
drivers/input/tablet/wacom_wac.c +0
-1- input_report_key(input, BTN_TOUCH,
le16_to_cpup((__le16 *)&data[6]));drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac · line 695
Quoted 8 lines
drivers/input/tablet/wacom_wac.c +1
-6-input_report_abs (input,ABS_MISC , wacom->id[0 ]); -input_report_key (input, wacom->tool[1 ],1 ); -input_sync (input); -break ; - } - }else { +if (len ==WACOM_PKGLEN_TPC1FG ) { input_report_abs(input, ABS_X,
get_unaligned_le16(&data[1]));drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac · line 696
Quoted 1 line
drivers/input/tablet/wacom_wac.c +0
-0input_report_abs(input, ABS_Y,
get_unaligned_le16(&data[3]));drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac · line 698
Quoted 6 lines
drivers/input/tablet/wacom_wac.c +2
-4-input_report_key (input,BTN_TOUCH ,1 ); -input_report_abs (input,ABS_MISC , wacom->id[1 ]); -input_report_key (input, wacom->tool[1 ],1 ); -input_sync (input); + }else { + input_report_abs(input, ABS_X, le16_to_cpup((__le16
Quoted 9 lines
*)&data[2]));
drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac · line 699
Quoted 1 line
drivers/input/tablet/wacom_wac.c +1
-0+ input_report_abs(input, ABS_Y, le16_to_cpup((__le16*)&data[4]));
Quoted 5 lines
drivers/input/tablet/wacom_wac.c +4
-0} +input_report_key (input,BTN_TOUCH , prox); + +/* keep prox bit to send proper out-prox event */ + wacom->shared->touch_down = prox;
touch_down represents the touch status that the driver gets/sets, not the actual touch down. Driver touch down (touch_down) is decided by both the actual touch and pen status, while it won't be changed by actual touch status when pen is in.
Quoted 2 lines
So in this function, touch-first-then-pen leads to
touch_down=touch_is_actually_down&& !pen_in_prox.
We need to think of the status in the driver's perspective, not the actual touch state.
As soon as the pen is in, i.e., pen data is processed, pen_in_prox will be true. The !pen_in_prox can only happen when pen data is not processed. So, to the driver, it is not in although it is actual in.
In this block, there are two possible cases for touch-first-then-pen:
touch in first (pen was not), so
touch_down = true;pen_in_prox = false;
touch still in and pen is in also:
touch_down = true;pen_in_prox = true;
Quoted 3 lines
Conversely,
pen-first-then-touch second leads to no state transition at all, which seems equivalent.
pen-first-then-touch (and pen stays in) will always have:
touch_down = false;pen_in_prox = true;
Quoted 5 lines
However, the code mixes the actual pen and touch
state with what gets reported. It seems cleaner to lettouch_down=touch_is_actually_down, and instead define what should be reported in that case.
No, we are not reporting the actual down. The actual touch can be down while the driver touch_down is false since pen is in-prox.
It is a logic generated by/for the driver, not from the firmware. Maybe I missed your point? Can you post a pesuo-code to show your idea?
Quoted 39 lines
Quoted 7 lines
drivers/input/tablet/wacom_wac.c +2
-0+ +return 1 ; }static int wacom_tpc_pen (struct wacom_wac *wacom)drivers/input/tablet/wacom_wac.c static int wacom_tpc_pen(struct wacom_wac *wacom) · line 720
Quoted 7 lines
drivers/input/tablet/wacom_wac.c +1
-1@@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom) prox = data[1 ] &0x20 ; -if (!wacom->shared->stylus_in_proximity) {/* first in prox */ +if (!wacom->shared->stylus_in_proximity) {/* Going into proximity select tool */ wacom->tool[0] = (data[1] & 0x0c) ? BTN_TOOL_RUBBER :
BTN_TOOL_PEN;drivers/input/tablet/wacom_wac.c static int wacom_tpc_pen(struct wacom_wac *wacom) · line 730
Quoted 9 lines
drivers/input/tablet/wacom_wac.c +4
-0if (wacom->tool[0 ] ==BTN_TOOL_PEN )@@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom) wacom->id[0 ] =ERASER_DEVICE_ID ; wacom->shared->stylus_in_proximity = true; + }else if (!wacom->shared->touch_down) { +input_report_key (input,BTN_STYLUS , data[1 ] &0x02 ); +input_report_key (input,BTN_STYLUS2 , data[1 ] &0x10 ); + input_report_abs(input, ABS_X, le16_to_cpup((__le16*)&data[2]));
drivers/input/tablet/wacom_wac.c static int wacom_tpc_pen(struct wacom_wac *wacom) · line 731
Quoted 1 line
drivers/input/tablet/wacom_wac.c +1
-0+ input_report_abs(input, ABS_Y, le16_to_cpup((__le16*)&data[4]));
drivers/input/tablet/wacom_wac.c static int wacom_tpc_pen(struct wacom_wac *wacom) · line 742
Quoted 11 lines
drivers/input/tablet/wacom_wac.c +11
-0+ pressure = ((data[7 ] &0x01 ) <<8 ) | data[6 ]; +if (pressure <0 ) + pressure = features->pressure_max + pressure +1 ; +input_report_abs (input,ABS_PRESSURE , pressure); +input_report_key (input,BTN_TOUCH , data[1 ] &0x05 ); +if (!prox) {/* out-prox */ + wacom->id[0 ] =0 ; + wacom->shared->stylus_in_proximity = false; + } +input_report_key (input, wacom->tool[0 ], prox); +return 1 ;
Quoted 3 lines
If
pen_in_proxis false on entry, and prox is false, this results inpen_in_proxbeing true.
How?
We get prox = false event from FW for pen only once when tool leaves prox. So, if prox is false, it means pen was in since without pen in, we do not get pen out event.
If pen_in_prox was true, touch_down has to be false. Otherwise, we won't post any events for pen. The main purpose here is that we will not post pen (and/or touch) events before the driver sets touch_down to false.
Firmware posts pen and touch data continuously as long as they both are in/down. It is the driver's responsibility to decide which ST to post so the events won't confuse the ST clients.
It is a very small corner case. But it eliminates an obvious jumpy cursor during the touch and pen transition.
I wish you have a Bamboo pen and touch to see what I mean.
Thank you for the comments.
Ping
Quoted 95 lines
drivers/input/tablet/wacom_wac.c void wacom_setup_input_capabilities(struct
Quoted 60 lines
drivers/input/tablet/wacom_wac.c +4
-42} -input_report_key (input,BTN_STYLUS , data[1 ] &0x02 ); -input_report_key (input,BTN_STYLUS2 , data[1 ] &0x10 ); -input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); -input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); - pressure = ((data[7 ] &0x01 ) <<8 ) | data[6 ]; -if (pressure <0 ) - pressure = features->pressure_max + pressure +1 ; -input_report_abs (input,ABS_PRESSURE , pressure); -input_report_key (input,BTN_TOUCH , data[1 ] &0x05 ); -if (!prox) {/* out-prox */ - wacom->id[0 ] =0 ; - wacom->shared->stylus_in_proximity = false; - } -input_report_key (input, wacom->tool[0 ], prox); -return 1 ; +return 0 ; }static int wacom_tpc_irq (struct wacom_wac *wacom,size_t len) {char *data = wacom->data; -int prox =0 ;dbg ("wacom_tpc_irq: received report #%d" , data[0 ]); -if (len ==WACOM_PKGLEN_TPC1FG || - data[0 ] ==WACOM_REPORT_TPC1FG ) {/* single touch */ - -if (wacom->shared->stylus_in_proximity) { -if (wacom->id[1 ] &0x01 ) -wacom_tpc_touch_out (wacom,0 ); - - wacom->id[1 ] =0 ; -return 0 ; - } - -if (len ==WACOM_PKGLEN_TPC1FG ) - prox = data[0 ] &0x01 ; -else /* with capacity */ - prox = data[1 ] &0x01 ; - -if (prox) -wacom_tpc_touch_in (wacom, len); -else { -wacom_tpc_touch_out (wacom,0 ); - - wacom->id[0 ] =0 ; - } -/* keep prox bit to send proper out-prox event */ - wacom->id[1 ] = prox; - }else if (data[0 ] ==WACOM_REPORT_PENABLED ) +if (len ==WACOM_PKGLEN_TPC1FG || data[0 ] ==WACOM_REPORT_TPC1FG ) +return wacom_tpc_single_touch (wacom, len); +else if (data[0 ] ==WACOM_REPORT_PENABLED )return wacom_tpc_pen (wacom);return 0 ;@@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct
input_dev*input_dev,drivers/input/tablet/wacom_wac.c void wacom_setup_input_capabilities(struct
Quoted 9 lines
drivers/input/tablet/wacom_wac.c +2
-0/* fall through */ case TABLETPC : +__clear_bit (ABS_MISC , input_dev->absbit); +if (features->device_type ==BTN_TOOL_DOUBLETAP || features->device_type ==BTN_TOOL_TRIPLETAP ) { input_abs_set_res(input_dev, ABS_X,@@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct
input_dev*input_dev,Review
Quoted 8 lines
drivers/input/tablet/wacom_wac.c +0
-1input_abs_set_res (input_dev,ABS_Y ,wacom_calculate_touch_res (features->y_max, features->y_phy)); -__set_bit (BTN_TOOL_DOUBLETAP , input_dev->keybit); }if (features->device_type !=BTN_TOOL_PEN )(quoted diff) +0
-0
b/drivers/input/tablet/wacom_wac.hQuoted 14 lines
index 8f747dd..835f756 100644
drivers/input/tablet/wacom_wac.h +1
-1@@ -88,6 +88,7 @@ struct wacom_features {struct wacom_shared {bool stylus_in_proximity; +bool touch_down; };struct wacom_wac { --1.7.4