* [PATCH 0/3] Input: synaptics - multitouch and multifinger support @ 2010-10-08 14:57 Chase Douglas 2010-10-08 14:57 ` [PATCH 1/3] Input: synaptics - add multitouch support Chase Douglas 2010-10-08 16:37 ` [PATCH 0/3] Input: synaptics - multitouch and multifinger support Takashi Iwai 0 siblings, 2 replies; 36+ messages in thread From: Chase Douglas @ 2010-10-08 14:57 UTC (permalink / raw) To: linux-input, xorg-devel Cc: Dmitry Torokhov, Takashi Iwai, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor Tobyn Bertram reverse engineered the multitouch protocol for Synaptics devices. I've been able to take his work and produce a series of commits to enable MT and multifinger (MF) support. Unfortunately, there's a tricky issue with some Synaptics touchpads that have integrated buttons. For example, the left and right buttons on the touchpad of my Dell Mini 1012 consist of the lower ~20% of the touchpad surface. The touchpad physically clicks under these areas. The X synaptics input module now has a parameter to disable touches occuring over the button area, but this solution still doesn't work perfectly. If you click a button and drag with another finger near the clicking finger, the touchpad gets confused. Now that we have full MT support, we can try to handle this scenario better. What I've found to work best is to make touches vanish if they occur over the button area of the trackpad while any button is held. This works in conjunction with the X synaptics driver to disable single touch control over the button area. With full MT support, the touchpad doesn't seem to get confused when a click and drag occurs with two fingers close to each other, and it enables MT gestures and MF support across the entire trackpad when no buttons are held. The first question is whether this seems appropriate to others, or if some other method would work better. Secondarily, should the solution occur in the kernel, like I have in the third patch of this series, or should it occur in the X input module? Although we don't have this information today, we may be able to query the touchpad in the future to know the area of the integrated buttons. If that were possible, would the recommended location for the hack change? Thanks, -- Chase ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/3] Input: synaptics - add multitouch support 2010-10-08 14:57 [PATCH 0/3] Input: synaptics - multitouch and multifinger support Chase Douglas @ 2010-10-08 14:57 ` Chase Douglas 2010-10-08 14:57 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chase Douglas ` (2 more replies) 2010-10-08 16:37 ` [PATCH 0/3] Input: synaptics - multitouch and multifinger support Takashi Iwai 1 sibling, 3 replies; 36+ messages in thread From: Chase Douglas @ 2010-10-08 14:57 UTC (permalink / raw) To: linux-input, xorg-devel Cc: Dmitry Torokhov, Takashi Iwai, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor Newer Synaptics devices support multitouch. It appears there is no touch tracking, so the non-slotted protocol is used. Multitouch mode is disabled by default because it makes click-and-drag on touchpads with integrated buttons even more difficult than it already is. Maybe if/when the X synaptics input module works around this issue we can enable it by default. Credit goes to Tobyn Bertram for reverse engineering the protocol. Reported-by: Tobyn Bertram Signed-off-by: Chase Douglas <chase.douglas@canonical.com> --- drivers/input/mouse/synaptics.c | 78 +++++++++++++++++++++++++++++++++++---- drivers/input/mouse/synaptics.h | 3 + 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 96b70a4..990598f 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -44,6 +44,10 @@ #define YMIN_NOMINAL 1408 #define YMAX_NOMINAL 4448 +static bool synaptics_multitouch; +module_param(synaptics_multitouch, bool, 0644); +MODULE_PARM_DESC(synaptics_multitouch, + "Enable multitouch mode on Synaptics devices"); /***************************************************************************** * Stuff we need even when we do not want native Synaptics support @@ -279,6 +283,22 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate) synaptics_mode_cmd(psmouse, priv->mode); } +static void synaptics_set_multitouch_mode(struct psmouse *psmouse) +{ + static unsigned char param = 0xc8; + struct synaptics_data *priv = psmouse->private; + + if (!SYN_CAP_MULTITOUCH(priv->ext_cap_0c) || !synaptics_multitouch) + return; + if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL)) + return; + if (ps2_command(&psmouse->ps2dev, ¶m, PSMOUSE_CMD_SETRATE)) + return; + + priv->multitouch = 1; + printk(KERN_INFO "Synaptics: Multitouch mode enabled\n"); +} + /***************************************************************************** * Synaptics pass-through PS/2 port support ****************************************************************************/ @@ -362,18 +382,30 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data memset(hw, 0, sizeof(struct synaptics_hw_state)); if (SYN_MODEL_NEWABS(priv->model_id)) { - hw->x = (((buf[3] & 0x10) << 8) | - ((buf[1] & 0x0f) << 8) | - buf[4]); - hw->y = (((buf[3] & 0x20) << 7) | - ((buf[1] & 0xf0) << 4) | - buf[5]); - - hw->z = buf[2]; hw->w = (((buf[0] & 0x30) >> 2) | ((buf[0] & 0x04) >> 1) | ((buf[3] & 0x04) >> 2)); + if (SYN_MULTITOUCH(priv, hw)) { + /* Multitouch data is half normal resolution */ + hw->x = (((buf[4] & 0x0f) << 8) | + buf[1]) << 1; + hw->y = (((buf[4] & 0xf0) << 4) | + buf[2]) << 1; + + hw->z = ((buf[3] & 0x30) | + (buf[5] & 0x0f)) << 1; + } else { + hw->x = (((buf[3] & 0x10) << 8) | + ((buf[1] & 0x0f) << 8) | + buf[4]); + hw->y = (((buf[3] & 0x20) << 7) | + ((buf[1] & 0xf0) << 4) | + buf[5]); + + hw->z = buf[2]; + } + hw->left = (buf[0] & 0x01) ? 1 : 0; hw->right = (buf[0] & 0x02) ? 1 : 0; @@ -445,6 +477,18 @@ static void synaptics_process_packet(struct psmouse *psmouse) synaptics_parse_hw_state(psmouse->packet, priv, &hw); + if (SYN_MULTITOUCH(priv, &hw)) { + if (hw.z > 0) { + input_report_abs(dev, ABS_MT_POSITION_X, hw.x); + input_report_abs(dev, ABS_MT_POSITION_Y, + YMAX_NOMINAL + YMIN_NOMINAL - hw.y); + input_report_abs(dev, ABS_MT_PRESSURE, hw.z); + } + + input_mt_sync(dev); + return; + } + if (hw.scroll) { priv->scroll += hw.scroll; @@ -499,6 +543,12 @@ static void synaptics_process_packet(struct psmouse *psmouse) if (hw.z > 0) { input_report_abs(dev, ABS_X, hw.x); input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y); + if (priv->multitouch) { + input_report_abs(dev, ABS_MT_POSITION_X, hw.x); + input_report_abs(dev, ABS_MT_POSITION_Y, + YMAX_NOMINAL + YMIN_NOMINAL - hw.y); + input_report_abs(dev, ABS_MT_PRESSURE, hw.z); + } } input_report_abs(dev, ABS_PRESSURE, hw.z); @@ -525,6 +575,8 @@ static void synaptics_process_packet(struct psmouse *psmouse) for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) input_report_key(dev, BTN_0 + i, hw.ext_buttons & (1 << i)); + if (priv->multitouch) + input_mt_sync(dev); input_sync(dev); } @@ -605,6 +657,14 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv) YMIN_NOMINAL, priv->y_max ?: YMAX_NOMINAL, 0, 0); input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0); + if (priv->multitouch) { + input_set_abs_params(dev, ABS_MT_POSITION_X, XMIN_NOMINAL, + priv->x_max ?: XMAX_NOMINAL, 0, 0); + input_set_abs_params(dev, ABS_MT_POSITION_Y, YMIN_NOMINAL, + priv->y_max ?: YMAX_NOMINAL, 0, 0); + input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0); + } + if (SYN_CAP_PALMDETECT(priv->capabilities)) input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0); @@ -745,6 +805,8 @@ int synaptics_init(struct psmouse *psmouse) goto init_fail; } + synaptics_set_multitouch_mode(psmouse); + priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS; printk(KERN_INFO "Synaptics Touchpad, model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n", diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h index b6aa7d2..5126c9c 100644 --- a/drivers/input/mouse/synaptics.h +++ b/drivers/input/mouse/synaptics.h @@ -53,6 +53,7 @@ #define SYN_CAP_PRODUCT_ID(ec) (((ec) & 0xff0000) >> 16) #define SYN_CAP_CLICKPAD(ex0c) ((ex0c) & 0x100100) #define SYN_CAP_MAX_DIMENSIONS(ex0c) ((ex0c) & 0x020000) +#define SYN_CAP_MULTITOUCH(ex0c) ((ex0c) & 0x080000) /* synaptics modes query bits */ #define SYN_MODE_ABSOLUTE(m) ((m) & (1 << 7)) @@ -78,6 +79,7 @@ #define SYN_NEWABS_STRICT 1 #define SYN_NEWABS_RELAXED 2 #define SYN_OLDABS 3 +#define SYN_MULTITOUCH(priv, hw) ((priv)->multitouch && (hw)->w == 2) /* * A structure to describe the state of the touchpad hardware (buttons and pad) @@ -110,6 +112,7 @@ struct synaptics_data { unsigned char pkt_type; /* packet type - old, new, etc */ unsigned char mode; /* current mode byte */ int scroll; + int multitouch; /* Whether device provides MT */ }; void synaptics_module_init(void); -- 1.7.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 2/3] Input: synaptics - add multitouch multifinger support 2010-10-08 14:57 ` [PATCH 1/3] Input: synaptics - add multitouch support Chase Douglas @ 2010-10-08 14:57 ` Chase Douglas 2010-10-08 14:58 ` [PATCH 3/3] Input: synaptics - remove touches over button click area Chase Douglas 2010-10-10 15:44 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chris Bagwell 2010-10-10 15:37 ` [PATCH 1/3] Input: synaptics - add multitouch support Chris Bagwell 2010-10-10 15:41 ` Chris Bagwell 2 siblings, 2 replies; 36+ messages in thread From: Chase Douglas @ 2010-10-08 14:57 UTC (permalink / raw) To: linux-input, xorg-devel Cc: Dmitry Torokhov, Takashi Iwai, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor Newer multitouch Synaptics trackpads do not advertise multifinger support. Now that we have multitouch support, we can use the number of touches to report multifinger functionality. In conjunction with the X synaptics input module, this enables functionality such as two finger scrolling. Signed-off-by: Chase Douglas <chase.douglas@canonical.com> --- drivers/input/mouse/synaptics.c | 24 +++++++++++++----------- drivers/input/mouse/synaptics.h | 1 + 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 990598f..7289d88 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -471,7 +471,6 @@ static void synaptics_process_packet(struct psmouse *psmouse) struct input_dev *dev = psmouse->dev; struct synaptics_data *priv = psmouse->private; struct synaptics_hw_state hw; - int num_fingers; int finger_width; int i; @@ -483,6 +482,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) input_report_abs(dev, ABS_MT_POSITION_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y); input_report_abs(dev, ABS_MT_PRESSURE, hw.z); + priv->num_fingers++; } input_mt_sync(dev); @@ -510,13 +510,13 @@ static void synaptics_process_packet(struct psmouse *psmouse) } if (hw.z > 0) { - num_fingers = 1; + priv->num_fingers++; finger_width = 5; if (SYN_CAP_EXTENDED(priv->capabilities)) { switch (hw.w) { case 0 ... 1: if (SYN_CAP_MULTIFINGER(priv->capabilities)) - num_fingers = hw.w + 2; + priv->num_fingers = hw.w + 2; break; case 2: if (SYN_MODEL_PEN(priv->model_id)) @@ -528,10 +528,8 @@ static void synaptics_process_packet(struct psmouse *psmouse) break; } } - } else { - num_fingers = 0; + } else finger_width = 0; - } /* Post events * BTN_TOUCH has to be first as mousedev relies on it when doing @@ -555,15 +553,19 @@ static void synaptics_process_packet(struct psmouse *psmouse) if (SYN_CAP_PALMDETECT(priv->capabilities)) input_report_abs(dev, ABS_TOOL_WIDTH, finger_width); - input_report_key(dev, BTN_TOOL_FINGER, num_fingers == 1); + input_report_key(dev, BTN_TOOL_FINGER, priv->num_fingers == 1); input_report_key(dev, BTN_LEFT, hw.left); input_report_key(dev, BTN_RIGHT, hw.right); - if (SYN_CAP_MULTIFINGER(priv->capabilities)) { - input_report_key(dev, BTN_TOOL_DOUBLETAP, num_fingers == 2); - input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3); + if (SYN_CAP_MULTIFINGER(priv->capabilities) || priv->multitouch) { + input_report_key(dev, BTN_TOOL_DOUBLETAP, + priv->num_fingers == 2); + input_report_key(dev, BTN_TOOL_TRIPLETAP, + priv->num_fingers == 3); } + priv->num_fingers = 0; + if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) input_report_key(dev, BTN_MIDDLE, hw.middle); @@ -674,7 +676,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv) __set_bit(BTN_LEFT, dev->keybit); __set_bit(BTN_RIGHT, dev->keybit); - if (SYN_CAP_MULTIFINGER(priv->capabilities)) { + if (SYN_CAP_MULTIFINGER(priv->capabilities) || priv->multitouch) { __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit); __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit); } diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h index 5126c9c..0989b8d 100644 --- a/drivers/input/mouse/synaptics.h +++ b/drivers/input/mouse/synaptics.h @@ -113,6 +113,7 @@ struct synaptics_data { unsigned char mode; /* current mode byte */ int scroll; int multitouch; /* Whether device provides MT */ + unsigned int num_fingers; /* Number of fingers touching */ }; void synaptics_module_init(void); -- 1.7.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-08 14:57 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chase Douglas @ 2010-10-08 14:58 ` Chase Douglas 2010-10-10 15:58 ` Chris Bagwell 2010-10-11 16:24 ` Chris Bagwell 2010-10-10 15:44 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chris Bagwell 1 sibling, 2 replies; 36+ messages in thread From: Chase Douglas @ 2010-10-08 14:58 UTC (permalink / raw) To: linux-input, xorg-devel Cc: Dmitry Torokhov, Takashi Iwai, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor Now that we have proper multitouch support, we can handle integrated buttons better. If we know the top of the buttons on the touchpad, we can ignore any touches that occur within the touchpad area while a button is clicked. It may be possible to get the button area by querying the device, but for now allow the user to manually set it. A note on why this works: the Synaptics touchpads have pseudo touch tracking. When two touches are on the touchpad, an MT touch packet with just the X, Y, and pressure values is sent before a normal Synaptics touch packet. When one touch is obviously in motion and the other is stationary, the touchpad controller sends the touch in motion in the normal packet and the stationary touch in the MT packet. Single touch emulation is provided by the normal packet, so an action like clicking a button and dragging with another finger still works as expected. Tested on a Dell Mini 1012 with synaptics_multitouch=1 and synaptics_button_thresh=4100. Signed-off-by: Chase Douglas <chase.douglas@canonical.com> --- drivers/input/mouse/synaptics.c | 16 +++++++++++++++- 1 files changed, 15 insertions(+), 1 deletions(-) diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 7289d88..e67778d 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -49,6 +49,12 @@ module_param(synaptics_multitouch, bool, 0644); MODULE_PARM_DESC(synaptics_multitouch, "Enable multitouch mode on Synaptics devices"); +static int synaptics_button_thresh = YMIN_NOMINAL + YMAX_NOMINAL; +module_param(synaptics_button_thresh, int, 0644); +MODULE_PARM_DESC(synaptics_button_thres, + "Y coordinate threshold of integrated buttons on Synaptics " + "devices"); + /***************************************************************************** * Stuff we need even when we do not want native Synaptics support ****************************************************************************/ @@ -463,6 +469,10 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data } } +#define TOUCH_OVER_BUTTON(hw) (((hw).left || (hw).middle || (hw).right) && \ + (YMAX_NOMINAL + YMIN_NOMINAL - (hw).y > \ + synaptics_button_thresh)) + /* * called for each full received packet from the touchpad */ @@ -477,7 +487,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) synaptics_parse_hw_state(psmouse->packet, priv, &hw); if (SYN_MULTITOUCH(priv, &hw)) { - if (hw.z > 0) { + if (hw.z > 0 && !TOUCH_OVER_BUTTON(hw)) { input_report_abs(dev, ABS_MT_POSITION_X, hw.x); input_report_abs(dev, ABS_MT_POSITION_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y); @@ -509,6 +519,10 @@ static void synaptics_process_packet(struct psmouse *psmouse) return; } + /* If touch occurs over depressed button, ignore it */ + if (TOUCH_OVER_BUTTON(hw)) + hw.z = 0; + if (hw.z > 0) { priv->num_fingers++; finger_width = 5; -- 1.7.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-08 14:58 ` [PATCH 3/3] Input: synaptics - remove touches over button click area Chase Douglas @ 2010-10-10 15:58 ` Chris Bagwell 2010-10-11 16:24 ` Chris Bagwell 1 sibling, 0 replies; 36+ messages in thread From: Chris Bagwell @ 2010-10-10 15:58 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Takashi Iwai, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On 10/08/2010 09:58 AM, Chase Douglas wrote: > Now that we have proper multitouch support, we can handle integrated > buttons better. If we know the top of the buttons on the touchpad, we > can ignore any touches that occur within the touchpad area while a > button is clicked. It may be possible to get the button area by querying > the device, but for now allow the user to manually set it. > > A note on why this works: the Synaptics touchpads have pseudo touch > tracking. When two touches are on the touchpad, an MT touch packet with > just the X, Y, and pressure values is sent before a normal Synaptics > touch packet. When one touch is obviously in motion and the other is > stationary, the touchpad controller sends the touch in motion in the > normal packet and the stationary touch in the MT packet. Single touch > emulation is provided by the normal packet, so an action like clicking > a button and dragging with another finger still works as expected. > > Tested on a Dell Mini 1012 with synaptics_multitouch=1 and > synaptics_button_thresh=4100. > Even if we did not submit the MT logic, I'd go a totally different direction and move clickpad button press support fully to xf86-input-synaptics and I'd remove the logic from kernel side that maps HW's middle button to left button. It seems just limping a long with single button support anyways. I haven't had time to review Takashi's xf86-input-synaptics patches just sent yet but seems along this line of thinking as well. Chris ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-08 14:58 ` [PATCH 3/3] Input: synaptics - remove touches over button click area Chase Douglas 2010-10-10 15:58 ` Chris Bagwell @ 2010-10-11 16:24 ` Chris Bagwell 2010-10-11 17:10 ` Takashi Iwai 1 sibling, 1 reply; 36+ messages in thread From: Chris Bagwell @ 2010-10-11 16:24 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Takashi Iwai, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On Fri, Oct 8, 2010 at 9:58 AM, Chase Douglas <chase.douglas@canonical.com> wrote: > Now that we have proper multitouch support, we can handle integrated > buttons better. If we know the top of the buttons on the touchpad, we > can ignore any touches that occur within the touchpad area while a > button is clicked. It may be possible to get the button area by querying > the device, but for now allow the user to manually set it. > > A note on why this works: the Synaptics touchpads have pseudo touch > tracking. When two touches are on the touchpad, an MT touch packet with > just the X, Y, and pressure values is sent before a normal Synaptics > touch packet. When one touch is obviously in motion and the other is > stationary, the touchpad controller sends the touch in motion in the > normal packet and the stationary touch in the MT packet. Single touch > emulation is provided by the normal packet, so an action like clicking > a button and dragging with another finger still works as expected. > > Tested on a Dell Mini 1012 with synaptics_multitouch=1 and > synaptics_button_thresh=4100. > > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > --- > drivers/input/mouse/synaptics.c | 16 +++++++++++++++- > 1 files changed, 15 insertions(+), 1 deletions(-) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index 7289d88..e67778d 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -49,6 +49,12 @@ module_param(synaptics_multitouch, bool, 0644); > MODULE_PARM_DESC(synaptics_multitouch, > "Enable multitouch mode on Synaptics devices"); > > +static int synaptics_button_thresh = YMIN_NOMINAL + YMAX_NOMINAL; > +module_param(synaptics_button_thresh, int, 0644); > +MODULE_PARM_DESC(synaptics_button_thres, > + "Y coordinate threshold of integrated buttons on Synaptics " > + "devices"); > + > /***************************************************************************** > * Stuff we need even when we do not want native Synaptics support > ****************************************************************************/ > @@ -463,6 +469,10 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data > } > } > > +#define TOUCH_OVER_BUTTON(hw) (((hw).left || (hw).middle || (hw).right) && \ > + (YMAX_NOMINAL + YMIN_NOMINAL - (hw).y > \ > + synaptics_button_thresh)) > + > /* > * called for each full received packet from the touchpad > */ > @@ -477,7 +487,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) > synaptics_parse_hw_state(psmouse->packet, priv, &hw); > > if (SYN_MULTITOUCH(priv, &hw)) { > - if (hw.z > 0) { > + if (hw.z > 0 && !TOUCH_OVER_BUTTON(hw)) { > input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > input_report_abs(dev, ABS_MT_POSITION_Y, > YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > @@ -509,6 +519,10 @@ static void synaptics_process_packet(struct psmouse *psmouse) > return; > } > > + /* If touch occurs over depressed button, ignore it */ > + if (TOUCH_OVER_BUTTON(hw)) > + hw.z = 0; > + > if (hw.z > 0) { > priv->num_fingers++; > finger_width = 5; > -- > 1.7.1 > > I'm convinced now that clickpad style touchpads can't work without multi-touch and something like logic in xf86-input-multitouch. So now I'd like to just consider how the MT-enabled touchpad interface can best work with non-multitouch aware applications since thats what users will need to deal with on fresh installs for a while. I believe the above approach of setting hw.z to zero would cause havoc on non-multi-touch aware applications. I see three main choices: 1) Do not report any button presses when in click area and report ABS_X/ABS_Y based on first finger touch always. Something like xf86-input-synaptics RBCornerButton feature would be responsible for button presses and can support left/middle/right concepts easily. The downside is a mis-configured box will not be able to use GUI since no button presses will work. Also, there is no clear way to auto-enable RBCornerButton-like features in user land in the same way is being done in some patches that consider single button touchpads as clickpads. 2.1) Send BTN_LEFT when in click area and ABS_X/ABS_Y tracks 1st finger during 1 touch and 2nd finger during multi-touch. xf86-input-synaptics needs change to detect left/middle/right based on ABS_X/ABS_Y values right at report of BTN_LEFT for clickpads. Touching drag finger before click finger breaks click-and-drag. 2.2) Send BTN_LEFT when in click area and ABS_X/ABS_Y tracks 1st finger during 1 touch and middle point of 2 fingers during multi-touch. Touching drag finger before click finger breaks click-and-drag and left/middle/right detection. 2.3) Send BTN_LEFT when in click area and ABS_X/ABS_Y indicates first finger always. I'm sure this is behavior of synaptics touchpad before we enable MT-packets. Touching drag finger first will break left/middle/right detection. Touching click finger first breaks click-and-drag. 3) A version of finger tracking in #2.1 could be done by detecting which touch of two touches is in click area (as patch does) and preferring non-click area data when it exists. This has same issues as #2.1 and has down side that we need to query hw or hard code click areas. Nothing ideal. I'd probably pick #2.1 (or #3 if we can auto-detect click area) since it at least allows consistent detecting of left/middle/right. I believe #2.1 is approach that bcm 5974 takes but not sure from only quick review. Chris -- 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-11 16:24 ` Chris Bagwell @ 2010-10-11 17:10 ` Takashi Iwai 2010-10-11 17:30 ` Dmitry Torokhov 2010-10-11 17:46 ` Chris Bagwell 0 siblings, 2 replies; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 17:10 UTC (permalink / raw) To: Chris Bagwell Cc: Chase Douglas, linux-input, xorg-devel, Dmitry Torokhov, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Mon, 11 Oct 2010 11:24:04 -0500, Chris Bagwell wrote: > > On Fri, Oct 8, 2010 at 9:58 AM, Chase Douglas > <chase.douglas@canonical.com> wrote: > > Now that we have proper multitouch support, we can handle integrated > > buttons better. If we know the top of the buttons on the touchpad, we > > can ignore any touches that occur within the touchpad area while a > > button is clicked. It may be possible to get the button area by querying > > the device, but for now allow the user to manually set it. > > > > A note on why this works: the Synaptics touchpads have pseudo touch > > tracking. When two touches are on the touchpad, an MT touch packet with > > just the X, Y, and pressure values is sent before a normal Synaptics > > touch packet. When one touch is obviously in motion and the other is > > stationary, the touchpad controller sends the touch in motion in the > > normal packet and the stationary touch in the MT packet. Single touch > > emulation is provided by the normal packet, so an action like clicking > > a button and dragging with another finger still works as expected. > > > > Tested on a Dell Mini 1012 with synaptics_multitouch=1 and > > synaptics_button_thresh=4100. > > > > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > > --- > > drivers/input/mouse/synaptics.c | 16 +++++++++++++++- > > 1 files changed, 15 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > > index 7289d88..e67778d 100644 > > --- a/drivers/input/mouse/synaptics.c > > +++ b/drivers/input/mouse/synaptics.c > > @@ -49,6 +49,12 @@ module_param(synaptics_multitouch, bool, 0644); > > MODULE_PARM_DESC(synaptics_multitouch, > > "Enable multitouch mode on Synaptics devices"); > > > > +static int synaptics_button_thresh = YMIN_NOMINAL + YMAX_NOMINAL; > > +module_param(synaptics_button_thresh, int, 0644); > > +MODULE_PARM_DESC(synaptics_button_thres, > > + "Y coordinate threshold of integrated buttons on Synaptics " > > + "devices"); > > + > > /***************************************************************************** > > * Stuff we need even when we do not want native Synaptics support > > ****************************************************************************/ > > @@ -463,6 +469,10 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data > > } > > } > > > > +#define TOUCH_OVER_BUTTON(hw) (((hw).left || (hw).middle || (hw).right) && \ > > + (YMAX_NOMINAL + YMIN_NOMINAL - (hw).y > \ > > + synaptics_button_thresh)) > > + > > /* > > * called for each full received packet from the touchpad > > */ > > @@ -477,7 +487,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) > > synaptics_parse_hw_state(psmouse->packet, priv, &hw); > > > > if (SYN_MULTITOUCH(priv, &hw)) { > > - if (hw.z > 0) { > > + if (hw.z > 0 && !TOUCH_OVER_BUTTON(hw)) { > > input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > > input_report_abs(dev, ABS_MT_POSITION_Y, > > YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > > @@ -509,6 +519,10 @@ static void synaptics_process_packet(struct psmouse *psmouse) > > return; > > } > > > > + /* If touch occurs over depressed button, ignore it */ > > + if (TOUCH_OVER_BUTTON(hw)) > > + hw.z = 0; > > + > > if (hw.z > 0) { > > priv->num_fingers++; > > finger_width = 5; > > -- > > 1.7.1 > > > > > > I'm convinced now that clickpad style touchpads can't work without > multi-touch and something like logic in xf86-input-multitouch. Actually Clickpad works without multi-touch patch. With my patches to synaptics, it worked in some level. There are many restrictions (e.g. pushing the button first then drag), though. Takashi -- 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-11 17:10 ` Takashi Iwai @ 2010-10-11 17:30 ` Dmitry Torokhov 2010-10-11 17:40 ` Takashi Iwai 2010-10-11 17:46 ` Chris Bagwell 1 sibling, 1 reply; 36+ messages in thread From: Dmitry Torokhov @ 2010-10-11 17:30 UTC (permalink / raw) To: Takashi Iwai Cc: Chris Bagwell, Chase Douglas, linux-input, xorg-devel, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On Mon, Oct 11, 2010 at 07:10:33PM +0200, Takashi Iwai wrote: > At Mon, 11 Oct 2010 11:24:04 -0500, > Chris Bagwell wrote: > > > > On Fri, Oct 8, 2010 at 9:58 AM, Chase Douglas > > <chase.douglas@canonical.com> wrote: > > > Now that we have proper multitouch support, we can handle integrated > > > buttons better. If we know the top of the buttons on the touchpad, we > > > can ignore any touches that occur within the touchpad area while a > > > button is clicked. It may be possible to get the button area by querying > > > the device, but for now allow the user to manually set it. > > > > > > A note on why this works: the Synaptics touchpads have pseudo touch > > > tracking. When two touches are on the touchpad, an MT touch packet with > > > just the X, Y, and pressure values is sent before a normal Synaptics > > > touch packet. When one touch is obviously in motion and the other is > > > stationary, the touchpad controller sends the touch in motion in the > > > normal packet and the stationary touch in the MT packet. Single touch > > > emulation is provided by the normal packet, so an action like clicking > > > a button and dragging with another finger still works as expected. > > > > > > Tested on a Dell Mini 1012 with synaptics_multitouch=1 and > > > synaptics_button_thresh=4100. > > > > > > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > > > --- > > > drivers/input/mouse/synaptics.c | 16 +++++++++++++++- > > > 1 files changed, 15 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > > > index 7289d88..e67778d 100644 > > > --- a/drivers/input/mouse/synaptics.c > > > +++ b/drivers/input/mouse/synaptics.c > > > @@ -49,6 +49,12 @@ module_param(synaptics_multitouch, bool, 0644); > > > MODULE_PARM_DESC(synaptics_multitouch, > > > "Enable multitouch mode on Synaptics devices"); > > > > > > +static int synaptics_button_thresh = YMIN_NOMINAL + YMAX_NOMINAL; > > > +module_param(synaptics_button_thresh, int, 0644); > > > +MODULE_PARM_DESC(synaptics_button_thres, > > > + "Y coordinate threshold of integrated buttons on Synaptics " > > > + "devices"); > > > + > > > /***************************************************************************** > > > * Stuff we need even when we do not want native Synaptics support > > > ****************************************************************************/ > > > @@ -463,6 +469,10 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data > > > } > > > } > > > > > > +#define TOUCH_OVER_BUTTON(hw) (((hw).left || (hw).middle || (hw).right) && \ > > > + (YMAX_NOMINAL + YMIN_NOMINAL - (hw).y > \ > > > + synaptics_button_thresh)) > > > + > > > /* > > > * called for each full received packet from the touchpad > > > */ > > > @@ -477,7 +487,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) > > > synaptics_parse_hw_state(psmouse->packet, priv, &hw); > > > > > > if (SYN_MULTITOUCH(priv, &hw)) { > > > - if (hw.z > 0) { > > > + if (hw.z > 0 && !TOUCH_OVER_BUTTON(hw)) { > > > input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > > > input_report_abs(dev, ABS_MT_POSITION_Y, > > > YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > > > @@ -509,6 +519,10 @@ static void synaptics_process_packet(struct psmouse *psmouse) > > > return; > > > } > > > > > > + /* If touch occurs over depressed button, ignore it */ > > > + if (TOUCH_OVER_BUTTON(hw)) > > > + hw.z = 0; > > > + > > > if (hw.z > 0) { > > > priv->num_fingers++; > > > finger_width = 5; > > > -- > > > 1.7.1 > > > > > > > > > > I'm convinced now that clickpad style touchpads can't work without > > multi-touch and something like logic in xf86-input-multitouch. > > Actually Clickpad works without multi-touch patch. With my patches to > synaptics, it worked in some level. There are many restrictions (e.g. > pushing the button first then drag), though. > I am OK with devices not working perfectly with default/existing drivers, but we should allow enough functionality (movement, primary - left - click and so on) so that user can go through install and/or upgrade of userpsace component. This also mean that we need to have userspace component available before changing the behavior that will cause adverse effect for older setups. Of course if we could avoid degrading older setups that would be even better. -- 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-11 17:30 ` Dmitry Torokhov @ 2010-10-11 17:40 ` Takashi Iwai 0 siblings, 0 replies; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 17:40 UTC (permalink / raw) To: Dmitry Torokhov Cc: Chris Bagwell, Chase Douglas, linux-input, xorg-devel, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Mon, 11 Oct 2010 10:30:16 -0700, Dmitry Torokhov wrote: > > On Mon, Oct 11, 2010 at 07:10:33PM +0200, Takashi Iwai wrote: > > At Mon, 11 Oct 2010 11:24:04 -0500, > > Chris Bagwell wrote: > > > > > > On Fri, Oct 8, 2010 at 9:58 AM, Chase Douglas > > > <chase.douglas@canonical.com> wrote: > > > > Now that we have proper multitouch support, we can handle integrated > > > > buttons better. If we know the top of the buttons on the touchpad, we > > > > can ignore any touches that occur within the touchpad area while a > > > > button is clicked. It may be possible to get the button area by querying > > > > the device, but for now allow the user to manually set it. > > > > > > > > A note on why this works: the Synaptics touchpads have pseudo touch > > > > tracking. When two touches are on the touchpad, an MT touch packet with > > > > just the X, Y, and pressure values is sent before a normal Synaptics > > > > touch packet. When one touch is obviously in motion and the other is > > > > stationary, the touchpad controller sends the touch in motion in the > > > > normal packet and the stationary touch in the MT packet. Single touch > > > > emulation is provided by the normal packet, so an action like clicking > > > > a button and dragging with another finger still works as expected. > > > > > > > > Tested on a Dell Mini 1012 with synaptics_multitouch=1 and > > > > synaptics_button_thresh=4100. > > > > > > > > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > > > > --- > > > > drivers/input/mouse/synaptics.c | 16 +++++++++++++++- > > > > 1 files changed, 15 insertions(+), 1 deletions(-) > > > > > > > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > > > > index 7289d88..e67778d 100644 > > > > --- a/drivers/input/mouse/synaptics.c > > > > +++ b/drivers/input/mouse/synaptics.c > > > > @@ -49,6 +49,12 @@ module_param(synaptics_multitouch, bool, 0644); > > > > MODULE_PARM_DESC(synaptics_multitouch, > > > > "Enable multitouch mode on Synaptics devices"); > > > > > > > > +static int synaptics_button_thresh = YMIN_NOMINAL + YMAX_NOMINAL; > > > > +module_param(synaptics_button_thresh, int, 0644); > > > > +MODULE_PARM_DESC(synaptics_button_thres, > > > > + "Y coordinate threshold of integrated buttons on Synaptics " > > > > + "devices"); > > > > + > > > > /***************************************************************************** > > > > * Stuff we need even when we do not want native Synaptics support > > > > ****************************************************************************/ > > > > @@ -463,6 +469,10 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data > > > > } > > > > } > > > > > > > > +#define TOUCH_OVER_BUTTON(hw) (((hw).left || (hw).middle || (hw).right) && \ > > > > + (YMAX_NOMINAL + YMIN_NOMINAL - (hw).y > \ > > > > + synaptics_button_thresh)) > > > > + > > > > /* > > > > * called for each full received packet from the touchpad > > > > */ > > > > @@ -477,7 +487,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) > > > > synaptics_parse_hw_state(psmouse->packet, priv, &hw); > > > > > > > > if (SYN_MULTITOUCH(priv, &hw)) { > > > > - if (hw.z > 0) { > > > > + if (hw.z > 0 && !TOUCH_OVER_BUTTON(hw)) { > > > > input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > > > > input_report_abs(dev, ABS_MT_POSITION_Y, > > > > YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > > > > @@ -509,6 +519,10 @@ static void synaptics_process_packet(struct psmouse *psmouse) > > > > return; > > > > } > > > > > > > > + /* If touch occurs over depressed button, ignore it */ > > > > + if (TOUCH_OVER_BUTTON(hw)) > > > > + hw.z = 0; > > > > + > > > > if (hw.z > 0) { > > > > priv->num_fingers++; > > > > finger_width = 5; > > > > -- > > > > 1.7.1 > > > > > > > > > > > > > > I'm convinced now that clickpad style touchpads can't work without > > > multi-touch and something like logic in xf86-input-multitouch. > > > > Actually Clickpad works without multi-touch patch. With my patches to > > synaptics, it worked in some level. There are many restrictions (e.g. > > pushing the button first then drag), though. > > > > I am OK with devices not working perfectly with default/existing > drivers, but we should allow enough functionality (movement, primary > - left - click and so on) so that user can go through install and/or > upgrade of userpsace component. > > This also mean that we need to have userspace component available before > changing the behavior that will cause adverse effect for older setups. > Of course if we could avoid degrading older setups that would be even > better. The xorg synaptics patches work with 2.6.34 or later kernel that contains SYN_CAP_CLICKPAD() in kernel driver. The question from now on is about the multi-touch patch, I'd say. One concern is the compatibility. The multi-touch is definitely benefit for non-clickpad devices. I guess integrating mtdev into xorg synaptics would be a good way to go, while picking up the minimal support for MT (like my patches) might be a quicker at this moment. In anyway, I see no merit for implementing clickpad support in the kernel driver for the time being, especially since the multi-touch can be better supported in the user-space. Takashi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-11 17:10 ` Takashi Iwai 2010-10-11 17:30 ` Dmitry Torokhov @ 2010-10-11 17:46 ` Chris Bagwell 2010-10-11 17:54 ` Henrik Rydberg 2010-10-11 18:29 ` Takashi Iwai 1 sibling, 2 replies; 36+ messages in thread From: Chris Bagwell @ 2010-10-11 17:46 UTC (permalink / raw) To: Takashi Iwai Cc: Chase Douglas, linux-input, xorg-devel, Dmitry Torokhov, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On Mon, Oct 11, 2010 at 12:10 PM, Takashi Iwai <tiwai@suse.de> wrote: > At Mon, 11 Oct 2010 11:24:04 -0500, > Chris Bagwell wrote: >> >> On Fri, Oct 8, 2010 at 9:58 AM, Chase Douglas >> <chase.douglas@canonical.com> wrote: >> > Now that we have proper multitouch support, we can handle integrated >> > buttons better. If we know the top of the buttons on the touchpad, we >> > can ignore any touches that occur within the touchpad area while a >> > button is clicked. It may be possible to get the button area by querying >> > the device, but for now allow the user to manually set it. >> > >> > A note on why this works: the Synaptics touchpads have pseudo touch >> > tracking. When two touches are on the touchpad, an MT touch packet with >> > just the X, Y, and pressure values is sent before a normal Synaptics >> > touch packet. When one touch is obviously in motion and the other is >> > stationary, the touchpad controller sends the touch in motion in the >> > normal packet and the stationary touch in the MT packet. Single touch >> > emulation is provided by the normal packet, so an action like clicking >> > a button and dragging with another finger still works as expected. >> > >> > Tested on a Dell Mini 1012 with synaptics_multitouch=1 and >> > synaptics_button_thresh=4100. >> > >> > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> >> > --- >> > drivers/input/mouse/synaptics.c | 16 +++++++++++++++- >> > 1 files changed, 15 insertions(+), 1 deletions(-) >> > >> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c >> > index 7289d88..e67778d 100644 >> > --- a/drivers/input/mouse/synaptics.c >> > +++ b/drivers/input/mouse/synaptics.c >> > @@ -49,6 +49,12 @@ module_param(synaptics_multitouch, bool, 0644); >> > MODULE_PARM_DESC(synaptics_multitouch, >> > "Enable multitouch mode on Synaptics devices"); >> > >> > +static int synaptics_button_thresh = YMIN_NOMINAL + YMAX_NOMINAL; >> > +module_param(synaptics_button_thresh, int, 0644); >> > +MODULE_PARM_DESC(synaptics_button_thres, >> > + "Y coordinate threshold of integrated buttons on Synaptics " >> > + "devices"); >> > + >> > /***************************************************************************** >> > * Stuff we need even when we do not want native Synaptics support >> > ****************************************************************************/ >> > @@ -463,6 +469,10 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data >> > } >> > } >> > >> > +#define TOUCH_OVER_BUTTON(hw) (((hw).left || (hw).middle || (hw).right) && \ >> > + (YMAX_NOMINAL + YMIN_NOMINAL - (hw).y > \ >> > + synaptics_button_thresh)) >> > + >> > /* >> > * called for each full received packet from the touchpad >> > */ >> > @@ -477,7 +487,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) >> > synaptics_parse_hw_state(psmouse->packet, priv, &hw); >> > >> > if (SYN_MULTITOUCH(priv, &hw)) { >> > - if (hw.z > 0) { >> > + if (hw.z > 0 && !TOUCH_OVER_BUTTON(hw)) { >> > input_report_abs(dev, ABS_MT_POSITION_X, hw.x); >> > input_report_abs(dev, ABS_MT_POSITION_Y, >> > YMAX_NOMINAL + YMIN_NOMINAL - hw.y); >> > @@ -509,6 +519,10 @@ static void synaptics_process_packet(struct psmouse *psmouse) >> > return; >> > } >> > >> > + /* If touch occurs over depressed button, ignore it */ >> > + if (TOUCH_OVER_BUTTON(hw)) >> > + hw.z = 0; >> > + >> > if (hw.z > 0) { >> > priv->num_fingers++; >> > finger_width = 5; >> > -- >> > 1.7.1 >> > >> > >> >> I'm convinced now that clickpad style touchpads can't work without >> multi-touch and something like logic in xf86-input-multitouch. > > Actually Clickpad works without multi-touch patch. With my patches to > synaptics, it worked in some level. There are many restrictions (e.g. > pushing the button first then drag), though. > True, but if I understand synaptic hardware MT behavior (sends actively moving finger in higher resolution packet regardless of original finger touch) then your patch will result in jumpy cursor on X side and that side would need patches to attempt to guess invalid data and discard. I've worked on a few similar patches to various xf86-input-* and generally they've failed to detect difference between invalid packets vs. fast user movements. The main point of my 3 options was to address jumpy cursor in xf86-input-* that are not MT aware. I think ABS_X/ABS_Y should only allow its meaning to change at detectable time periods so user can account for it and, specifically, that time period is best at transition of BTN_TOOL_DOUBLETAP. Assuming its easy enough to support exact rules for ABS_X/ABS_Y changing meanings on kernel side (which I think it probably is pretty easy), I think we should do it so that applications don't have to become MT-aware as the official solution for jumpy cursors. Chris -- 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-11 17:46 ` Chris Bagwell @ 2010-10-11 17:54 ` Henrik Rydberg 2010-10-11 18:29 ` Takashi Iwai 1 sibling, 0 replies; 36+ messages in thread From: Henrik Rydberg @ 2010-10-11 17:54 UTC (permalink / raw) To: Chris Bagwell Cc: Takashi Iwai, Chase Douglas, linux-input, xorg-devel, Dmitry Torokhov, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor On 10/11/2010 07:46 PM, Chris Bagwell wrote: [...] >>> I'm convinced now that clickpad style touchpads can't work without >>> multi-touch and something like logic in xf86-input-multitouch. >> >> Actually Clickpad works without multi-touch patch. With my patches to >> synaptics, it worked in some level. There are many restrictions (e.g. >> pushing the button first then drag), though. >> > > True, but if I understand synaptic hardware MT behavior (sends > actively moving finger in higher resolution packet regardless of > original finger touch) then your patch will result in jumpy cursor on > X side and that side would need patches to attempt to guess invalid > data and discard. I've worked on a few similar patches to various > xf86-input-* and generally they've failed to detect difference between > invalid packets vs. fast user movements. > > The main point of my 3 options was to address jumpy cursor in > xf86-input-* that are not MT aware. I think ABS_X/ABS_Y should only > allow its meaning to change at detectable time periods so user can > account for it and, specifically, that time period is best at > transition of BTN_TOOL_DOUBLETAP. > > Assuming its easy enough to support exact rules for ABS_X/ABS_Y > changing meanings on kernel side (which I think it probably is pretty > easy), I think we should do it so that applications don't have to > become MT-aware as the official solution for jumpy cursors. A solution like the one you describe was actually proposed and agreed upon in the beginning of this thread, so I think the only thing left to do is actually write the patch. :-) I can only assume that Takashi or Chase will be back in a bit with an update. Cheers, Henrik ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/3] Input: synaptics - remove touches over button click area 2010-10-11 17:46 ` Chris Bagwell 2010-10-11 17:54 ` Henrik Rydberg @ 2010-10-11 18:29 ` Takashi Iwai 1 sibling, 0 replies; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 18:29 UTC (permalink / raw) To: Chris Bagwell Cc: Chase Douglas, linux-input, xorg-devel, Dmitry Torokhov, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Mon, 11 Oct 2010 12:46:40 -0500, Chris Bagwell wrote: > > On Mon, Oct 11, 2010 at 12:10 PM, Takashi Iwai <tiwai@suse.de> wrote: > > At Mon, 11 Oct 2010 11:24:04 -0500, > > Chris Bagwell wrote: > >> > >> On Fri, Oct 8, 2010 at 9:58 AM, Chase Douglas > >> <chase.douglas@canonical.com> wrote: > >> > Now that we have proper multitouch support, we can handle integrated > >> > buttons better. If we know the top of the buttons on the touchpad, we > >> > can ignore any touches that occur within the touchpad area while a > >> > button is clicked. It may be possible to get the button area by querying > >> > the device, but for now allow the user to manually set it. > >> > > >> > A note on why this works: the Synaptics touchpads have pseudo touch > >> > tracking. When two touches are on the touchpad, an MT touch packet with > >> > just the X, Y, and pressure values is sent before a normal Synaptics > >> > touch packet. When one touch is obviously in motion and the other is > >> > stationary, the touchpad controller sends the touch in motion in the > >> > normal packet and the stationary touch in the MT packet. Single touch > >> > emulation is provided by the normal packet, so an action like clicking > >> > a button and dragging with another finger still works as expected. > >> > > >> > Tested on a Dell Mini 1012 with synaptics_multitouch=1 and > >> > synaptics_button_thresh=4100. > >> > > >> > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > >> > --- > >> > drivers/input/mouse/synaptics.c | 16 +++++++++++++++- > >> > 1 files changed, 15 insertions(+), 1 deletions(-) > >> > > >> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > >> > index 7289d88..e67778d 100644 > >> > --- a/drivers/input/mouse/synaptics.c > >> > +++ b/drivers/input/mouse/synaptics.c > >> > @@ -49,6 +49,12 @@ module_param(synaptics_multitouch, bool, 0644); > >> > MODULE_PARM_DESC(synaptics_multitouch, > >> > "Enable multitouch mode on Synaptics devices"); > >> > > >> > +static int synaptics_button_thresh = YMIN_NOMINAL + YMAX_NOMINAL; > >> > +module_param(synaptics_button_thresh, int, 0644); > >> > +MODULE_PARM_DESC(synaptics_button_thres, > >> > + "Y coordinate threshold of integrated buttons on Synaptics " > >> > + "devices"); > >> > + > >> > /***************************************************************************** > >> > * Stuff we need even when we do not want native Synaptics support > >> > ****************************************************************************/ > >> > @@ -463,6 +469,10 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data > >> > } > >> > } > >> > > >> > +#define TOUCH_OVER_BUTTON(hw) (((hw).left || (hw).middle || (hw).right) && \ > >> > + (YMAX_NOMINAL + YMIN_NOMINAL - (hw).y > \ > >> > + synaptics_button_thresh)) > >> > + > >> > /* > >> > * called for each full received packet from the touchpad > >> > */ > >> > @@ -477,7 +487,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) > >> > synaptics_parse_hw_state(psmouse->packet, priv, &hw); > >> > > >> > if (SYN_MULTITOUCH(priv, &hw)) { > >> > - if (hw.z > 0) { > >> > + if (hw.z > 0 && !TOUCH_OVER_BUTTON(hw)) { > >> > input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > >> > input_report_abs(dev, ABS_MT_POSITION_Y, > >> > YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > >> > @@ -509,6 +519,10 @@ static void synaptics_process_packet(struct psmouse *psmouse) > >> > return; > >> > } > >> > > >> > + /* If touch occurs over depressed button, ignore it */ > >> > + if (TOUCH_OVER_BUTTON(hw)) > >> > + hw.z = 0; > >> > + > >> > if (hw.z > 0) { > >> > priv->num_fingers++; > >> > finger_width = 5; > >> > -- > >> > 1.7.1 > >> > > >> > > >> > >> I'm convinced now that clickpad style touchpads can't work without > >> multi-touch and something like logic in xf86-input-multitouch. > > > > Actually Clickpad works without multi-touch patch. With my patches to > > synaptics, it worked in some level. There are many restrictions (e.g. > > pushing the button first then drag), though. > > > > True, but if I understand synaptic hardware MT behavior (sends > actively moving finger in higher resolution packet regardless of > original finger touch) then your patch will result in jumpy cursor on > X side and that side would need patches to attempt to guess invalid > data and discard. I've worked on a few similar patches to various > xf86-input-* and generally they've failed to detect difference between > invalid packets vs. fast user movements. Right. I also tackled to this once. > The main point of my 3 options was to address jumpy cursor in > xf86-input-* that are not MT aware. I think ABS_X/ABS_Y should only > allow its meaning to change at detectable time periods so user can > account for it and, specifically, that time period is best at > transition of BTN_TOOL_DOUBLETAP. > > Assuming its easy enough to support exact rules for ABS_X/ABS_Y > changing meanings on kernel side (which I think it probably is pretty > easy), I think we should do it so that applications don't have to > become MT-aware as the official solution for jumpy cursors. Hm, so are you suggesting that we need to transform MT events to non-MT events in the kernel side with some clever filtering? I don't know whether this is really needed... If the patch is minimal amount, it might be worth to get in, though. However, I feel that we should forget about clickpad support in kernel; as you mentioned, Clickpad works properly only with the multi-touch, after all... Rather I'm concerned how to decide the kernel driver's behavior, whether behaving in MT or non-MT mode. As Henrik suggested, in MT mode, all events should be ABS_MT_* type. But non-MT-aware user-space apps won't understand all these events. So, we'll need some fallback mechanism for non-MT apps, anyway. I've implemented multi_touch module option for that purpose, but it's hacikish. Dmitry, do you have any suggestion? thanks, Takashi -- 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] Input: synaptics - add multitouch multifinger support 2010-10-08 14:57 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chase Douglas 2010-10-08 14:58 ` [PATCH 3/3] Input: synaptics - remove touches over button click area Chase Douglas @ 2010-10-10 15:44 ` Chris Bagwell 1 sibling, 0 replies; 36+ messages in thread From: Chris Bagwell @ 2010-10-10 15:44 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Takashi Iwai, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On 10/08/2010 09:57 AM, Chase Douglas wrote: > Newer multitouch Synaptics trackpads do not advertise multifinger > support. Now that we have multitouch support, we can use the number of > touches to report multifinger functionality. > > In conjunction with the X synaptics input module, this enables > functionality such as two finger scrolling. > > Signed-off-by: Chase Douglas<chase.douglas@canonical.com> > --- > drivers/input/mouse/synaptics.c | 24 +++++++++++++----------- > drivers/input/mouse/synaptics.h | 1 + > 2 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index 990598f..7289d88 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -471,7 +471,6 @@ static void synaptics_process_packet(struct psmouse *psmouse) > struct input_dev *dev = psmouse->dev; > struct synaptics_data *priv = psmouse->private; > struct synaptics_hw_state hw; > - int num_fingers; > int finger_width; > int i; > > @@ -483,6 +482,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) > input_report_abs(dev, ABS_MT_POSITION_Y, > YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > input_report_abs(dev, ABS_MT_PRESSURE, hw.z); > + priv->num_fingers++; > } > > input_mt_sync(dev); > @@ -510,13 +510,13 @@ static void synaptics_process_packet(struct psmouse *psmouse) > } > > if (hw.z> 0) { > - num_fingers = 1; > + priv->num_fingers++; In this area of code, its not as obvious your relying on MT packets to always come before standard packets. I think its worth a comment here or below on why your initialising priv->num_fingers at bottom of processing instead of at top of processing. It will also help explain to reader why mt_sync events work out as expected. > finger_width = 5; > if (SYN_CAP_EXTENDED(priv->capabilities)) { > switch (hw.w) { > case 0 ... 1: > if (SYN_CAP_MULTIFINGER(priv->capabilities)) > - num_fingers = hw.w + 2; > + priv->num_fingers = hw.w + 2; > break; > case 2: > if (SYN_MODEL_PEN(priv->model_id)) > @@ -528,10 +528,8 @@ static void synaptics_process_packet(struct psmouse *psmouse) > break; > } > } > - } else { > - num_fingers = 0; > + } else > finger_width = 0; > - } > > /* Post events > * BTN_TOUCH has to be first as mousedev relies on it when doing > @@ -555,15 +553,19 @@ static void synaptics_process_packet(struct psmouse *psmouse) > if (SYN_CAP_PALMDETECT(priv->capabilities)) > input_report_abs(dev, ABS_TOOL_WIDTH, finger_width); > > - input_report_key(dev, BTN_TOOL_FINGER, num_fingers == 1); > + input_report_key(dev, BTN_TOOL_FINGER, priv->num_fingers == 1); > input_report_key(dev, BTN_LEFT, hw.left); > input_report_key(dev, BTN_RIGHT, hw.right); > > - if (SYN_CAP_MULTIFINGER(priv->capabilities)) { > - input_report_key(dev, BTN_TOOL_DOUBLETAP, num_fingers == 2); > - input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3); > + if (SYN_CAP_MULTIFINGER(priv->capabilities) || priv->multitouch) { > + input_report_key(dev, BTN_TOOL_DOUBLETAP, > + priv->num_fingers == 2); > + input_report_key(dev, BTN_TOOL_TRIPLETAP, > + priv->num_fingers == 3); > } > > + priv->num_fingers = 0; > + > if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) > input_report_key(dev, BTN_MIDDLE, hw.middle); > > @@ -674,7 +676,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv) > __set_bit(BTN_LEFT, dev->keybit); > __set_bit(BTN_RIGHT, dev->keybit); > > - if (SYN_CAP_MULTIFINGER(priv->capabilities)) { > + if (SYN_CAP_MULTIFINGER(priv->capabilities) || priv->multitouch) { > __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit); > __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit); > } > diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h > index 5126c9c..0989b8d 100644 > --- a/drivers/input/mouse/synaptics.h > +++ b/drivers/input/mouse/synaptics.h > @@ -113,6 +113,7 @@ struct synaptics_data { > unsigned char mode; /* current mode byte */ > int scroll; > int multitouch; /* Whether device provides MT */ > + unsigned int num_fingers; /* Number of fingers touching */ > }; > > void synaptics_module_init(void); ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/3] Input: synaptics - add multitouch support 2010-10-08 14:57 ` [PATCH 1/3] Input: synaptics - add multitouch support Chase Douglas 2010-10-08 14:57 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chase Douglas @ 2010-10-10 15:37 ` Chris Bagwell 2010-10-10 15:41 ` Chris Bagwell 2 siblings, 0 replies; 36+ messages in thread From: Chris Bagwell @ 2010-10-10 15:37 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Takashi Iwai, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On 10/08/2010 09:57 AM, Chase Douglas wrote: > Newer Synaptics devices support multitouch. It appears there is no touch > tracking, so the non-slotted protocol is used. > > Multitouch mode is disabled by default because it makes click-and-drag > on touchpads with integrated buttons even more difficult than it already > is. Maybe if/when the X synaptics input module works around this issue > we can enable it by default. I don't have access to a clickpad and I'm trying to understand its unique issues better. Can you give a little more information on how X synaptics driver behaves differently with MT enabled compared to how it behaves with MT disabled? On non-clickpad's, the X/Y will always track close to first finger touch. If clickpad's continue this behaviour in non-MT mode then I'd assume click-and-drag will only work if you touch the drag finger before the click finger. If you click first then at best I'd expect extremely slow movement since it tracks close but not exactly to first finger. Does MT mode change behaviour? Your patch #3 description sounds like the non-MT packet tracks moving finger always and so it constantly swapping its finger meaning. Off hand, I'd think that helps click-and-drag issue although it creates others. As example of what issues it creates, I'd expect xf86-input-synaptics to go crazy with cursor jumps when its 2 finger gestures are turned off and you randomly touch an extra finger to touchpad since the meaning of ABS_X/ABS_Y is changing without warning to it (and it doesn't understand MT right now). I agree with the other comments that we want to avoid options as much as possible. > > Credit goes to Tobyn Bertram for reverse engineering the protocol. > > Reported-by: Tobyn Bertram > Signed-off-by: Chase Douglas<chase.douglas@canonical.com> > --- > drivers/input/mouse/synaptics.c | 78 +++++++++++++++++++++++++++++++++++---- > drivers/input/mouse/synaptics.h | 3 + > 2 files changed, 73 insertions(+), 8 deletions(-) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index 96b70a4..990598f 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -44,6 +44,10 @@ > #define YMIN_NOMINAL 1408 > #define YMAX_NOMINAL 4448 > > +static bool synaptics_multitouch; > +module_param(synaptics_multitouch, bool, 0644); > +MODULE_PARM_DESC(synaptics_multitouch, > + "Enable multitouch mode on Synaptics devices"); > > /***************************************************************************** > * Stuff we need even when we do not want native Synaptics support > @@ -279,6 +283,22 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate) > synaptics_mode_cmd(psmouse, priv->mode); > } > > +static void synaptics_set_multitouch_mode(struct psmouse *psmouse) > +{ > + static unsigned char param = 0xc8; > + struct synaptics_data *priv = psmouse->private; > + > + if (!SYN_CAP_MULTITOUCH(priv->ext_cap_0c) || !synaptics_multitouch) > + return; > + if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL)) > + return; > + if (ps2_command(&psmouse->ps2dev,¶m, PSMOUSE_CMD_SETRATE)) > + return; > + > + priv->multitouch = 1; > + printk(KERN_INFO "Synaptics: Multitouch mode enabled\n"); > +} > + > /***************************************************************************** > * Synaptics pass-through PS/2 port support > ****************************************************************************/ > @@ -362,18 +382,30 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data > memset(hw, 0, sizeof(struct synaptics_hw_state)); > > if (SYN_MODEL_NEWABS(priv->model_id)) { > - hw->x = (((buf[3]& 0x10)<< 8) | > - ((buf[1]& 0x0f)<< 8) | > - buf[4]); > - hw->y = (((buf[3]& 0x20)<< 7) | > - ((buf[1]& 0xf0)<< 4) | > - buf[5]); > - > - hw->z = buf[2]; > hw->w = (((buf[0]& 0x30)>> 2) | > ((buf[0]& 0x04)>> 1) | > ((buf[3]& 0x04)>> 2)); > > + if (SYN_MULTITOUCH(priv, hw)) { > + /* Multitouch data is half normal resolution */ > + hw->x = (((buf[4]& 0x0f)<< 8) | > + buf[1])<< 1; > + hw->y = (((buf[4]& 0xf0)<< 4) | > + buf[2])<< 1; > + > + hw->z = ((buf[3]& 0x30) | > + (buf[5]& 0x0f))<< 1; > + } else { > + hw->x = (((buf[3]& 0x10)<< 8) | > + ((buf[1]& 0x0f)<< 8) | > + buf[4]); > + hw->y = (((buf[3]& 0x20)<< 7) | > + ((buf[1]& 0xf0)<< 4) | > + buf[5]); > + > + hw->z = buf[2]; > + } > + > hw->left = (buf[0]& 0x01) ? 1 : 0; > hw->right = (buf[0]& 0x02) ? 1 : 0; > > @@ -445,6 +477,18 @@ static void synaptics_process_packet(struct psmouse *psmouse) > > synaptics_parse_hw_state(psmouse->packet, priv,&hw); > > + if (SYN_MULTITOUCH(priv,&hw)) { > + if (hw.z> 0) { > + input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > + input_report_abs(dev, ABS_MT_POSITION_Y, > + YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > + input_report_abs(dev, ABS_MT_PRESSURE, hw.z); > + } > + > + input_mt_sync(dev); > + return; > + } > + > if (hw.scroll) { > priv->scroll += hw.scroll; > > @@ -499,6 +543,12 @@ static void synaptics_process_packet(struct psmouse *psmouse) > if (hw.z> 0) { > input_report_abs(dev, ABS_X, hw.x); > input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > + if (priv->multitouch) { > + input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > + input_report_abs(dev, ABS_MT_POSITION_Y, > + YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > + input_report_abs(dev, ABS_MT_PRESSURE, hw.z); > + } > } > input_report_abs(dev, ABS_PRESSURE, hw.z); > > @@ -525,6 +575,8 @@ static void synaptics_process_packet(struct psmouse *psmouse) > for (i = 0; i< SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) > input_report_key(dev, BTN_0 + i, hw.ext_buttons& (1<< i)); > > + if (priv->multitouch) > + input_mt_sync(dev); This mt_sync would seem more nature to be sent after ABS_MT_PRESSURE to match MT packet processing. Chris > input_sync(dev); > } > > @@ -605,6 +657,14 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv) > YMIN_NOMINAL, priv->y_max ?: YMAX_NOMINAL, 0, 0); > input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0); > > + if (priv->multitouch) { > + input_set_abs_params(dev, ABS_MT_POSITION_X, XMIN_NOMINAL, > + priv->x_max ?: XMAX_NOMINAL, 0, 0); > + input_set_abs_params(dev, ABS_MT_POSITION_Y, YMIN_NOMINAL, > + priv->y_max ?: YMAX_NOMINAL, 0, 0); > + input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0); > + } > + > if (SYN_CAP_PALMDETECT(priv->capabilities)) > input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0); > > @@ -745,6 +805,8 @@ int synaptics_init(struct psmouse *psmouse) > goto init_fail; > } > > + synaptics_set_multitouch_mode(psmouse); > + > priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS; > > printk(KERN_INFO "Synaptics Touchpad, model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n", > diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h > index b6aa7d2..5126c9c 100644 > --- a/drivers/input/mouse/synaptics.h > +++ b/drivers/input/mouse/synaptics.h > @@ -53,6 +53,7 @@ > #define SYN_CAP_PRODUCT_ID(ec) (((ec)& 0xff0000)>> 16) > #define SYN_CAP_CLICKPAD(ex0c) ((ex0c)& 0x100100) > #define SYN_CAP_MAX_DIMENSIONS(ex0c) ((ex0c)& 0x020000) > +#define SYN_CAP_MULTITOUCH(ex0c) ((ex0c)& 0x080000) > > /* synaptics modes query bits */ > #define SYN_MODE_ABSOLUTE(m) ((m)& (1<< 7)) > @@ -78,6 +79,7 @@ > #define SYN_NEWABS_STRICT 1 > #define SYN_NEWABS_RELAXED 2 > #define SYN_OLDABS 3 > +#define SYN_MULTITOUCH(priv, hw) ((priv)->multitouch&& (hw)->w == 2) > > /* > * A structure to describe the state of the touchpad hardware (buttons and pad) > @@ -110,6 +112,7 @@ struct synaptics_data { > unsigned char pkt_type; /* packet type - old, new, etc */ > unsigned char mode; /* current mode byte */ > int scroll; > + int multitouch; /* Whether device provides MT */ > }; > > void synaptics_module_init(void); ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/3] Input: synaptics - add multitouch support 2010-10-08 14:57 ` [PATCH 1/3] Input: synaptics - add multitouch support Chase Douglas 2010-10-08 14:57 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chase Douglas 2010-10-10 15:37 ` [PATCH 1/3] Input: synaptics - add multitouch support Chris Bagwell @ 2010-10-10 15:41 ` Chris Bagwell 2 siblings, 0 replies; 36+ messages in thread From: Chris Bagwell @ 2010-10-10 15:41 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Takashi Iwai, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On 10/08/2010 09:57 AM, Chase Douglas wrote: > Newer Synaptics devices support multitouch. It appears there is no touch > tracking, so the non-slotted protocol is used. > > Multitouch mode is disabled by default because it makes click-and-drag > on touchpads with integrated buttons even more difficult than it already > is. Maybe if/when the X synaptics input module works around this issue > we can enable it by default. I don't have access to a clickpad and I'm trying to understand its unique issues better. Can you give a little more information on how X synaptics driver behaves differently with MT enabled compared to how it behaves with MT disabled? On non-clickpad's, the X/Y will always track close to first finger touch. If clickpad's continue this behaviour in non-MT mode then I'd assume click-and-drag will only work if you touch the drag finger before the click finger. If you click first then at best I'd expect extremely slow movement since it tracks close but not exactly to first finger. Does MT mode change behaviour? Your patch #3 description sounds like the non-MT packet tracks moving finger always and so it constantly swapping its finger meaning. Off hand, I'd think that helps click-and-drag issue although it creates others. As example of what issues it creates, I'd expect xf86-input-synaptics to go crazy with cursor jumps when its 2 finger gestures are turned off and you randomly touch an extra finger to touchpad since the meaning of ABS_X/ABS_Y is changing without warning to it (and it doesn't understand MT right now). I agree with the other comments that we want to avoid options as much as possible. > > Credit goes to Tobyn Bertram for reverse engineering the protocol. > > Reported-by: Tobyn Bertram > Signed-off-by: Chase Douglas<chase.douglas@canonical.com> > --- > drivers/input/mouse/synaptics.c | 78 +++++++++++++++++++++++++++++++++++---- > drivers/input/mouse/synaptics.h | 3 + > 2 files changed, 73 insertions(+), 8 deletions(-) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index 96b70a4..990598f 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -44,6 +44,10 @@ > #define YMIN_NOMINAL 1408 > #define YMAX_NOMINAL 4448 > > +static bool synaptics_multitouch; > +module_param(synaptics_multitouch, bool, 0644); > +MODULE_PARM_DESC(synaptics_multitouch, > + "Enable multitouch mode on Synaptics devices"); > > /***************************************************************************** > * Stuff we need even when we do not want native Synaptics support > @@ -279,6 +283,22 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate) > synaptics_mode_cmd(psmouse, priv->mode); > } > > +static void synaptics_set_multitouch_mode(struct psmouse *psmouse) > +{ > + static unsigned char param = 0xc8; > + struct synaptics_data *priv = psmouse->private; > + > + if (!SYN_CAP_MULTITOUCH(priv->ext_cap_0c) || !synaptics_multitouch) > + return; > + if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL)) > + return; > + if (ps2_command(&psmouse->ps2dev,¶m, PSMOUSE_CMD_SETRATE)) > + return; > + > + priv->multitouch = 1; > + printk(KERN_INFO "Synaptics: Multitouch mode enabled\n"); > +} > + > /***************************************************************************** > * Synaptics pass-through PS/2 port support > ****************************************************************************/ > @@ -362,18 +382,30 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data > memset(hw, 0, sizeof(struct synaptics_hw_state)); > > if (SYN_MODEL_NEWABS(priv->model_id)) { > - hw->x = (((buf[3]& 0x10)<< 8) | > - ((buf[1]& 0x0f)<< 8) | > - buf[4]); > - hw->y = (((buf[3]& 0x20)<< 7) | > - ((buf[1]& 0xf0)<< 4) | > - buf[5]); > - > - hw->z = buf[2]; > hw->w = (((buf[0]& 0x30)>> 2) | > ((buf[0]& 0x04)>> 1) | > ((buf[3]& 0x04)>> 2)); > > + if (SYN_MULTITOUCH(priv, hw)) { > + /* Multitouch data is half normal resolution */ > + hw->x = (((buf[4]& 0x0f)<< 8) | > + buf[1])<< 1; > + hw->y = (((buf[4]& 0xf0)<< 4) | > + buf[2])<< 1; > + > + hw->z = ((buf[3]& 0x30) | > + (buf[5]& 0x0f))<< 1; > + } else { > + hw->x = (((buf[3]& 0x10)<< 8) | > + ((buf[1]& 0x0f)<< 8) | > + buf[4]); > + hw->y = (((buf[3]& 0x20)<< 7) | > + ((buf[1]& 0xf0)<< 4) | > + buf[5]); > + > + hw->z = buf[2]; > + } > + > hw->left = (buf[0]& 0x01) ? 1 : 0; > hw->right = (buf[0]& 0x02) ? 1 : 0; > > @@ -445,6 +477,18 @@ static void synaptics_process_packet(struct psmouse *psmouse) > > synaptics_parse_hw_state(psmouse->packet, priv,&hw); > > + if (SYN_MULTITOUCH(priv,&hw)) { > + if (hw.z> 0) { > + input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > + input_report_abs(dev, ABS_MT_POSITION_Y, > + YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > + input_report_abs(dev, ABS_MT_PRESSURE, hw.z); > + } > + > + input_mt_sync(dev); > + return; > + } > + > if (hw.scroll) { > priv->scroll += hw.scroll; > > @@ -499,6 +543,12 @@ static void synaptics_process_packet(struct psmouse *psmouse) > if (hw.z> 0) { > input_report_abs(dev, ABS_X, hw.x); > input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > + if (priv->multitouch) { > + input_report_abs(dev, ABS_MT_POSITION_X, hw.x); > + input_report_abs(dev, ABS_MT_POSITION_Y, > + YMAX_NOMINAL + YMIN_NOMINAL - hw.y); > + input_report_abs(dev, ABS_MT_PRESSURE, hw.z); > + } > } > input_report_abs(dev, ABS_PRESSURE, hw.z); > > @@ -525,6 +575,8 @@ static void synaptics_process_packet(struct psmouse *psmouse) > for (i = 0; i< SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) > input_report_key(dev, BTN_0 + i, hw.ext_buttons& (1<< i)); > > + if (priv->multitouch) > + input_mt_sync(dev); This mt_sync would seem more nature to be sent after ABS_MT_PRESSURE to match MT packet processing. Chris > input_sync(dev); > } > > @@ -605,6 +657,14 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv) > YMIN_NOMINAL, priv->y_max ?: YMAX_NOMINAL, 0, 0); > input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0); > > + if (priv->multitouch) { > + input_set_abs_params(dev, ABS_MT_POSITION_X, XMIN_NOMINAL, > + priv->x_max ?: XMAX_NOMINAL, 0, 0); > + input_set_abs_params(dev, ABS_MT_POSITION_Y, YMIN_NOMINAL, > + priv->y_max ?: YMAX_NOMINAL, 0, 0); > + input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0); > + } > + > if (SYN_CAP_PALMDETECT(priv->capabilities)) > input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0); > > @@ -745,6 +805,8 @@ int synaptics_init(struct psmouse *psmouse) > goto init_fail; > } > > + synaptics_set_multitouch_mode(psmouse); > + > priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS; > > printk(KERN_INFO "Synaptics Touchpad, model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n", > diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h > index b6aa7d2..5126c9c 100644 > --- a/drivers/input/mouse/synaptics.h > +++ b/drivers/input/mouse/synaptics.h > @@ -53,6 +53,7 @@ > #define SYN_CAP_PRODUCT_ID(ec) (((ec)& 0xff0000)>> 16) > #define SYN_CAP_CLICKPAD(ex0c) ((ex0c)& 0x100100) > #define SYN_CAP_MAX_DIMENSIONS(ex0c) ((ex0c)& 0x020000) > +#define SYN_CAP_MULTITOUCH(ex0c) ((ex0c)& 0x080000) > > /* synaptics modes query bits */ > #define SYN_MODE_ABSOLUTE(m) ((m)& (1<< 7)) > @@ -78,6 +79,7 @@ > #define SYN_NEWABS_STRICT 1 > #define SYN_NEWABS_RELAXED 2 > #define SYN_OLDABS 3 > +#define SYN_MULTITOUCH(priv, hw) ((priv)->multitouch&& (hw)->w == 2) > > /* > * A structure to describe the state of the touchpad hardware (buttons and pad) > @@ -110,6 +112,7 @@ struct synaptics_data { > unsigned char pkt_type; /* packet type - old, new, etc */ > unsigned char mode; /* current mode byte */ > int scroll; > + int multitouch; /* Whether device provides MT */ > }; > > void synaptics_module_init(void); ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 14:57 [PATCH 0/3] Input: synaptics - multitouch and multifinger support Chase Douglas 2010-10-08 14:57 ` [PATCH 1/3] Input: synaptics - add multitouch support Chase Douglas @ 2010-10-08 16:37 ` Takashi Iwai 2010-10-08 16:38 ` Takashi Iwai ` (2 more replies) 1 sibling, 3 replies; 36+ messages in thread From: Takashi Iwai @ 2010-10-08 16:37 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Fri, 8 Oct 2010 10:57:57 -0400, Chase Douglas wrote: > > Tobyn Bertram reverse engineered the multitouch protocol for Synaptics devices. > I've been able to take his work and produce a series of commits to enable MT > and multifinger (MF) support. > > Unfortunately, there's a tricky issue with some Synaptics touchpads that have > integrated buttons. For example, the left and right buttons on the touchpad of > my Dell Mini 1012 consist of the lower ~20% of the touchpad surface. The > touchpad physically clicks under these areas. > > The X synaptics input module now has a parameter to disable touches occuring > over the button area, but this solution still doesn't work perfectly. If you > click a button and drag with another finger near the clicking finger, the > touchpad gets confused. > > Now that we have full MT support, we can try to handle this scenario better. > What I've found to work best is to make touches vanish if they occur over the > button area of the trackpad while any button is held. This works in conjunction > with the X synaptics driver to disable single touch control over the button > area. With full MT support, the touchpad doesn't seem to get confused when a > click and drag occurs with two fingers close to each other, and it enables MT > gestures and MF support across the entire trackpad when no buttons are held. > > The first question is whether this seems appropriate to others, or if some > other method would work better. Secondarily, should the solution occur in the > kernel, like I have in the third patch of this series, or should it occur in > the X input module? Although we don't have this information today, we may be > able to query the touchpad in the future to know the area of the integrated > buttons. If that were possible, would the recommended location for the hack > change? Great! Finally someone found it out! I found this and made a series of patches in 4 months ago. Since then, Novell legal prohibited me to send the patches to the upstream due to "possible patent infringing". Now you cracked out. Yay. FWIW, my corresponding patch is below. It really looks similar in the end ;) I added a kconfig just to be safer. Regarding the "clickpad" support: in my case, I implemented almost everything about it in xorg driver. I'm going to submit xorg patches. thanks, Takashi --- >From 4c88fb69bdfb20a6ad030c7a19eed1e76beb0442 Mon Sep 17 00:00:00 2001 From: Takashi Iwai <tiwai@suse.de> Date: Mon, 14 Jun 2010 12:46:48 +0200 Subject: [PATCH] input: Add multi-touch protocol support to Synaptics This patch adds the experimental support of the multi-touch protocol for recent Synaptics devices. Note that the protocol was analyzed just by a pure guess work by watching device outputs, so it might be incorrect. Also, the check of the multi-touch capability based on the 0x0c caps might be also wrong. In the multi-touch mode, the driver gives MT_POSITION_X, MT_POSITION_Y and MT_PRESSURE abs events to follow the type A in Documentation/input/multi-touch-protocol.txt. The device supports up to two finger points, as far as I've tested. As the driver now gives the MT_* events, this extension might result in the incompatible event outputs. Thus make sure that the user-space side (e.g. X11 synaptics driver) is also updated to support MT_* events. Signed-off-by: Takashi Iwai <tiwai@suse.de> --- drivers/input/mouse/Kconfig | 11 ++++ drivers/input/mouse/synaptics.c | 113 +++++++++++++++++++++++++++++++++----- drivers/input/mouse/synaptics.h | 1 + 3 files changed, 110 insertions(+), 15 deletions(-) diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig index 91d3517..6dd0d29 100644 --- a/drivers/input/mouse/Kconfig +++ b/drivers/input/mouse/Kconfig @@ -76,6 +76,17 @@ config MOUSE_PS2_SYNAPTICS_LED Say Y here if you have a Synaptics device with an embedded LED. This will enable LED class driver to control the LED device. +config MOUSE_PS2_SYNAPTICS_MULTI_TOUCH + bool "Support multi-touch protocol of Synaptics devices" + depends on MOUSE_PS2_SYNAPTICS + help + Say Y here for enabling the multi-touch protocol of recent + Syanptics devices. This may result in incompatible input + events, thus make sure that you update X11 synaptics driver + beforehand with the multi-protocol touch. + + If unsure, say N. + config MOUSE_PS2_LIFEBOOK bool "Fujitsu Lifebook PS/2 mouse protocol extension" if EMBEDDED default y diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 00799bc..79d9463 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -181,6 +181,12 @@ static int synaptics_capability(struct psmouse *psmouse) } } +#ifdef CONFIG_MOUSE_PS2_SYNAPTICS_MULTI_TOUCH + /* FIXME: is this the right guess? */ + if (priv->ext_cap_0c & (0x08 << 16)) + priv->can_multi_touch = 1; +#endif + return 0; } @@ -458,6 +464,32 @@ static void synaptics_free_led(struct psmouse *psmouse) #define synaptics_sync_led(ps) do {} while (0) #endif +/* change to the multi-touch mode; + * this is done by sending SYN_QUE_MODEL cmd but setting a parameter + * by SETRATE instead of querying via GETINFO. + * 0xc8 seems to be the multi-touch mode. + */ +static int synaptics_init_multi_touch(struct psmouse *psmouse) +{ + unsigned char param[1]; + + if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL)) + return -1; + param[0] = 0xc8; + if (ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_SETRATE)) + return -1; + return 0; +} + +#ifdef CONFIG_MOUSE_PS2_SYNAPTICS_MULTI_TOUCH +#define is_multi_touch(priv) (priv)->can_multi_touch +#else +#define is_multi_touch(priv) 0 +#endif +/* the multi-touch packet contains w=2 (like pen) */ +#define is_multi_touch_packet(priv, hw) \ + (is_multi_touch(priv) && (hw)->w == 2) + /***************************************************************************** * Functions to interpret the absolute mode packets ****************************************************************************/ @@ -467,17 +499,27 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data memset(hw, 0, sizeof(struct synaptics_hw_state)); if (SYN_MODEL_NEWABS(priv->model_id)) { - hw->x = (((buf[3] & 0x10) << 8) | - ((buf[1] & 0x0f) << 8) | - buf[4]); - hw->y = (((buf[3] & 0x20) << 7) | - ((buf[1] & 0xf0) << 4) | - buf[5]); - - hw->z = buf[2]; hw->w = (((buf[0] & 0x30) >> 2) | ((buf[0] & 0x04) >> 1) | ((buf[3] & 0x04) >> 2)); + if (is_multi_touch_packet(priv, hw)) { + /* a multi-touch packet is encoded differently; + * it appears to have half-resolutions. I might + * have missed the lowest bits, but it's hard + * to recognize. + */ + hw->x = ((buf[4] & 0x0f) << 8 | buf[1]) << 1; + hw->y = ((buf[4] & 0xf0) << 4 | buf[2]) << 1; + hw->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1; + } else { + hw->x = (((buf[3] & 0x10) << 8) | + ((buf[1] & 0x0f) << 8) | + buf[4]); + hw->y = (((buf[3] & 0x20) << 7) | + ((buf[1] & 0xf0) << 4) | + buf[5]); + hw->z = buf[2]; + } hw->left = (buf[0] & 0x01) ? 1 : 0; hw->right = (buf[0] & 0x02) ? 1 : 0; @@ -492,7 +534,7 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data } else if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) { hw->middle = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0; - if (hw->w == 2) + if (!is_multi_touch(priv) && hw->w == 2) hw->scroll = (signed char)(buf[1]); } @@ -550,6 +592,18 @@ static void synaptics_process_packet(struct psmouse *psmouse) synaptics_parse_hw_state(psmouse->packet, priv, &hw); + if (is_multi_touch_packet(priv, &hw)) { + /* multi-touching */ + if (hw.z > 0) { + input_report_abs(dev, ABS_MT_POSITION_X, hw.x); + input_report_abs(dev, ABS_MT_POSITION_Y, + YMAX_NOMINAL + YMIN_NOMINAL - hw.y); + } + input_report_abs(dev, ABS_MT_PRESSURE, hw.z); + input_mt_sync(dev); + return; + } + if (hw.scroll) { priv->scroll += hw.scroll; @@ -576,7 +630,8 @@ static void synaptics_process_packet(struct psmouse *psmouse) if (SYN_CAP_EXTENDED(priv->capabilities)) { switch (hw.w) { case 0 ... 1: - if (SYN_CAP_MULTIFINGER(priv->capabilities)) + if (SYN_CAP_MULTIFINGER(priv->capabilities) || + is_multi_touch(priv)) num_fingers = hw.w + 2; break; case 2: @@ -602,10 +657,15 @@ static void synaptics_process_packet(struct psmouse *psmouse) if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0); if (hw.z > 0) { - input_report_abs(dev, ABS_X, hw.x); - input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y); + int key; + key = is_multi_touch(priv) ? ABS_MT_POSITION_X : ABS_X; + input_report_abs(dev, key, hw.x); + key = is_multi_touch(priv) ? ABS_MT_POSITION_Y : ABS_Y; + input_report_abs(dev, key, YMAX_NOMINAL + YMIN_NOMINAL - hw.y); } - input_report_abs(dev, ABS_PRESSURE, hw.z); + input_report_abs(dev, + is_multi_touch(priv) ? ABS_MT_PRESSURE : ABS_PRESSURE, + hw.z); if (SYN_CAP_PALMDETECT(priv->capabilities)) input_report_abs(dev, ABS_TOOL_WIDTH, finger_width); @@ -614,7 +674,7 @@ static void synaptics_process_packet(struct psmouse *psmouse) input_report_key(dev, BTN_LEFT, hw.left); input_report_key(dev, BTN_RIGHT, hw.right); - if (SYN_CAP_MULTIFINGER(priv->capabilities)) { + if (SYN_CAP_MULTIFINGER(priv->capabilities) || is_multi_touch(priv)) { input_report_key(dev, BTN_TOOL_DOUBLETAP, num_fingers == 2); input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3); } @@ -630,6 +690,8 @@ static void synaptics_process_packet(struct psmouse *psmouse) for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) input_report_key(dev, BTN_0 + i, hw.ext_buttons & (1 << i)); + if (is_multi_touch(priv)) + input_mt_sync(dev); input_sync(dev); } @@ -719,7 +781,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv) __set_bit(BTN_LEFT, dev->keybit); __set_bit(BTN_RIGHT, dev->keybit); - if (SYN_CAP_MULTIFINGER(priv->capabilities)) { + if (SYN_CAP_MULTIFINGER(priv->capabilities) || is_multi_touch(priv)) { __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit); __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit); } @@ -748,6 +810,16 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv) __clear_bit(BTN_RIGHT, dev->keybit); __clear_bit(BTN_MIDDLE, dev->keybit); } + + if (is_multi_touch(priv)) { + input_set_abs_params(dev, ABS_MT_POSITION_X, + XMIN_NOMINAL, XMAX_NOMINAL, 0, 0); + input_set_abs_params(dev, ABS_MT_POSITION_Y, + YMIN_NOMINAL, YMAX_NOMINAL, 0, 0); + input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0); + input_abs_set_res(dev, ABS_MT_POSITION_X, priv->x_res); + input_abs_set_res(dev, ABS_MT_POSITION_Y, priv->y_res); + } } static void synaptics_disconnect(struct psmouse *psmouse) @@ -785,6 +857,8 @@ static int synaptics_reconnect(struct psmouse *psmouse) } synaptics_sync_led(psmouse); + if (is_multi_touch(priv)) + synaptics_init_multi_touch(psmouse); return 0; } @@ -863,6 +937,15 @@ int synaptics_init(struct psmouse *psmouse) if (synaptics_init_led(psmouse) < 0) goto init_fail; + if (priv->can_multi_touch) { + if (synaptics_init_multi_touch(psmouse)) { + printk(KERN_WARNING "Synaptics: " + "unable to initialize multi-touch\n"); + priv->can_multi_touch = 0; + } else + printk(KERN_INFO "Synaptics: multi-touch enabled\n"); + } + set_input_params(psmouse->dev, priv); /* diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h index e1a9033..b586087 100644 --- a/drivers/input/mouse/synaptics.h +++ b/drivers/input/mouse/synaptics.h @@ -111,6 +111,7 @@ struct synaptics_data { unsigned char pkt_type; /* packet type - old, new, etc */ unsigned char mode; /* current mode byte */ + unsigned char can_multi_touch; /* multi-touch support */ int scroll; struct synaptics_led *led; }; -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 16:37 ` [PATCH 0/3] Input: synaptics - multitouch and multifinger support Takashi Iwai @ 2010-10-08 16:38 ` Takashi Iwai 2010-10-08 17:48 ` Takashi Iwai 2010-10-08 17:15 ` Chase Douglas 2010-10-10 7:49 ` Henrik Rydberg 2 siblings, 1 reply; 36+ messages in thread From: Takashi Iwai @ 2010-10-08 16:38 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Fri, 08 Oct 2010 18:37:22 +0200, Takashi Iwai wrote: > > At Fri, 8 Oct 2010 10:57:57 -0400, > Chase Douglas wrote: > > > > Tobyn Bertram reverse engineered the multitouch protocol for Synaptics devices. > > I've been able to take his work and produce a series of commits to enable MT > > and multifinger (MF) support. > > > > Unfortunately, there's a tricky issue with some Synaptics touchpads that have > > integrated buttons. For example, the left and right buttons on the touchpad of > > my Dell Mini 1012 consist of the lower ~20% of the touchpad surface. The > > touchpad physically clicks under these areas. > > > > The X synaptics input module now has a parameter to disable touches occuring > > over the button area, but this solution still doesn't work perfectly. If you > > click a button and drag with another finger near the clicking finger, the > > touchpad gets confused. > > > > Now that we have full MT support, we can try to handle this scenario better. > > What I've found to work best is to make touches vanish if they occur over the > > button area of the trackpad while any button is held. This works in conjunction > > with the X synaptics driver to disable single touch control over the button > > area. With full MT support, the touchpad doesn't seem to get confused when a > > click and drag occurs with two fingers close to each other, and it enables MT > > gestures and MF support across the entire trackpad when no buttons are held. > > > > The first question is whether this seems appropriate to others, or if some > > other method would work better. Secondarily, should the solution occur in the > > kernel, like I have in the third patch of this series, or should it occur in > > the X input module? Although we don't have this information today, we may be > > able to query the touchpad in the future to know the area of the integrated > > buttons. If that were possible, would the recommended location for the hack > > change? > > Great! Finally someone found it out! > I found this and made a series of patches in 4 months ago. Since > then, Novell legal prohibited me to send the patches to the upstream > due to "possible patent infringing". Now you cracked out. Yay. > > FWIW, my corresponding patch is below. It really looks similar in the > end ;) I added a kconfig just to be safer. > > Regarding the "clickpad" support: in my case, I implemented almost > everything about it in xorg driver. I'm going to submit xorg > patches. BTW, yet another kernel patch is missing; the support of embedded LED. I've posted this once, but it seems forgotten since then. Reposted below. Takashi --- From: Takashi Iwai <tiwai@suse.de> Subject: [PATCH 2/2] input: Add LED support to Synaptics device The new Synaptics devices have an LED on the top-left corner. This patch adds a new LED class device to control it. It's created dynamically upon synaptics device probing. The LED is controlled via the command 0x0a with parameters 0x88 or 0x10. This seems only on/off control although other value might be accepted. The detection of the LED isn't clear yet. It should have been the new capability bits that indicate the presence, but on real machines, it doesn't fit. So, for the time being, the driver checks the product id in the ext capability bits and assumes that LED exists on the known devices. Signed-off-by: Takashi Iwai <tiwai@suse.de> --- drivers/input/mouse/Kconfig | 9 +++ drivers/input/mouse/synaptics.c | 111 ++++++++++++++++++++++++++++++++++++++++ drivers/input/mouse/synaptics.h | 3 + 3 files changed, 123 insertions(+) --- a/drivers/input/mouse/Kconfig +++ b/drivers/input/mouse/Kconfig @@ -19,6 +19,7 @@ config MOUSE_PS2 select SERIO_LIBPS2 select SERIO_I8042 if X86 select SERIO_GSCPS2 if GSC + select LEDS_CLASS if MOUSE_PS2_SYNAPICS_LED help Say Y here if you have a PS/2 mouse connected to your system. This includes the standard 2 or 3-button PS/2 mouse, as well as PS/2 @@ -67,6 +68,14 @@ config MOUSE_PS2_SYNAPTICS If unsure, say Y. +config MOUSE_PS2_SYNAPTICS_LED + bool "Support embedded LED on Synaptics devices" + depends on MOUSE_PS2_SYNAPTICS + select NEW_LEDS + help + Say Y here if you have a Synaptics device with an embedded LED. + This will enable LED class driver to control the LED device. + config MOUSE_PS2_LIFEBOOK bool "Fujitsu Lifebook PS/2 mouse protocol extension" if EMBEDDED default y --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -28,6 +28,7 @@ #include <linux/input.h> #include <linux/serio.h> #include <linux/libps2.h> +#include <linux/leds.h> #include <linux/slab.h> #include "psmouse.h" #include "synaptics.h" @@ -353,6 +354,110 @@ static void synaptics_pt_create(struct p serio_register_port(serio); } +#ifdef CONFIG_MOUSE_PS2_SYNAPTICS_LED +/* + * LED handling: + * Some Synaptics devices have an embeded LED at the top-left corner. + */ + +struct synaptics_led { + struct psmouse *psmouse; + struct work_struct work; + struct led_classdev cdev; +}; + +static void synaptics_set_led(struct psmouse *psmouse, int on) +{ + int i; + unsigned char cmd = on ? 0x88 : 0x10; + + ps2_begin_command(&psmouse->ps2dev); + if (__ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11)) + goto out; + for (i = 6; i >= 0; i -= 2) { + unsigned char d = (cmd >> i) & 3; + if (__ps2_command(&psmouse->ps2dev, &d, PSMOUSE_CMD_SETRES)) + goto out; + } + cmd = 0x0a; + __ps2_command(&psmouse->ps2dev, &cmd, PSMOUSE_CMD_SETRATE); + out: + ps2_end_command(&psmouse->ps2dev); +} + +static void synaptics_led_work(struct work_struct *work) +{ + struct synaptics_led *led; + + led = container_of(work, struct synaptics_led, work); + synaptics_set_led(led->psmouse, led->cdev.brightness); +} + +static void synaptics_led_cdev_brightness_set(struct led_classdev *cdev, + enum led_brightness value) +{ + struct synaptics_led *led; + + led = container_of(cdev, struct synaptics_led, cdev); + schedule_work(&led->work); +} + +static void synaptics_sync_led(struct psmouse *psmouse) +{ + struct synaptics_data *priv = psmouse->private; + + if (priv->led) + synaptics_set_led(psmouse, priv->led->cdev.brightness); +} + +static int synaptics_init_led(struct psmouse *psmouse) +{ + struct synaptics_data *priv = psmouse->private; + struct synaptics_led *led; + int err; + + /* FIXME: LED is supposedly detectable in cap0c[1] 0x20, but it seems + * not working on real machines. + * So we check the product id to be sure. + */ + if (!priv->ext_cap_0c || SYN_CAP_PRODUCT_ID(priv->ext_cap) != 0xe4) + return 0; + + printk(KERN_INFO "synaptics: support LED control\n"); + led = kzalloc(sizeof(struct synaptics_led), GFP_KERNEL); + if (!led) + return -ENOMEM; + led->psmouse = psmouse; + INIT_WORK(&led->work, synaptics_led_work); + led->cdev.name = "psmouse::synaptics"; + led->cdev.brightness_set = synaptics_led_cdev_brightness_set; + led->cdev.flags = LED_CORE_SUSPENDRESUME; + err = led_classdev_register(NULL, &led->cdev); + if (err < 0) { + kfree(led); + return err; + } + priv->led = led; + return 0; +} + +static void synaptics_free_led(struct psmouse *psmouse) +{ + struct synaptics_data *priv = psmouse->private; + + if (!priv->led) + return; + cancel_work_sync(&priv->led->work); + synaptics_set_led(psmouse, 0); + led_classdev_unregister(&priv->led->cdev); + kfree(priv->led); +} +#else +#define synaptics_init_led(ps) 0 +#define synaptics_free_led(ps) do {} while (0) +#define synaptics_sync_led(ps) do {} while (0) +#endif + /***************************************************************************** * Functions to interpret the absolute mode packets ****************************************************************************/ @@ -647,6 +752,7 @@ static void set_input_params(struct inpu static void synaptics_disconnect(struct psmouse *psmouse) { + synaptics_free_led(psmouse); synaptics_reset(psmouse); kfree(psmouse->private); psmouse->private = NULL; @@ -678,6 +784,8 @@ static int synaptics_reconnect(struct ps return -1; } + synaptics_sync_led(psmouse); + return 0; } @@ -752,6 +860,9 @@ int synaptics_init(struct psmouse *psmou SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity), priv->model_id, priv->capabilities, priv->ext_cap, priv->ext_cap_0c); + if (synaptics_init_led(psmouse) < 0) + goto init_fail; + set_input_params(psmouse->dev, priv); /* --- a/drivers/input/mouse/synaptics.h +++ b/drivers/input/mouse/synaptics.h @@ -97,6 +97,8 @@ struct synaptics_hw_state { signed char scroll; }; +struct synaptics_led; + struct synaptics_data { /* Data read from the touchpad */ unsigned long int model_id; /* Model-ID */ @@ -110,6 +112,7 @@ struct synaptics_data { unsigned char pkt_type; /* packet type - old, new, etc */ unsigned char mode; /* current mode byte */ int scroll; + struct synaptics_led *led; }; void synaptics_module_init(void); ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 16:38 ` Takashi Iwai @ 2010-10-08 17:48 ` Takashi Iwai 0 siblings, 0 replies; 36+ messages in thread From: Takashi Iwai @ 2010-10-08 17:48 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Fri, 08 Oct 2010 18:38:38 +0200, Takashi Iwai wrote: > > At Fri, 08 Oct 2010 18:37:22 +0200, > Takashi Iwai wrote: > > > > At Fri, 8 Oct 2010 10:57:57 -0400, > > Chase Douglas wrote: > > > > > > Tobyn Bertram reverse engineered the multitouch protocol for Synaptics devices. > > > I've been able to take his work and produce a series of commits to enable MT > > > and multifinger (MF) support. > > > > > > Unfortunately, there's a tricky issue with some Synaptics touchpads that have > > > integrated buttons. For example, the left and right buttons on the touchpad of > > > my Dell Mini 1012 consist of the lower ~20% of the touchpad surface. The > > > touchpad physically clicks under these areas. > > > > > > The X synaptics input module now has a parameter to disable touches occuring > > > over the button area, but this solution still doesn't work perfectly. If you > > > click a button and drag with another finger near the clicking finger, the > > > touchpad gets confused. > > > > > > Now that we have full MT support, we can try to handle this scenario better. > > > What I've found to work best is to make touches vanish if they occur over the > > > button area of the trackpad while any button is held. This works in conjunction > > > with the X synaptics driver to disable single touch control over the button > > > area. With full MT support, the touchpad doesn't seem to get confused when a > > > click and drag occurs with two fingers close to each other, and it enables MT > > > gestures and MF support across the entire trackpad when no buttons are held. > > > > > > The first question is whether this seems appropriate to others, or if some > > > other method would work better. Secondarily, should the solution occur in the > > > kernel, like I have in the third patch of this series, or should it occur in > > > the X input module? Although we don't have this information today, we may be > > > able to query the touchpad in the future to know the area of the integrated > > > buttons. If that were possible, would the recommended location for the hack > > > change? > > > > Great! Finally someone found it out! > > I found this and made a series of patches in 4 months ago. Since > > then, Novell legal prohibited me to send the patches to the upstream > > due to "possible patent infringing". Now you cracked out. Yay. > > > > FWIW, my corresponding patch is below. It really looks similar in the > > end ;) I added a kconfig just to be safer. > > > > Regarding the "clickpad" support: in my case, I implemented almost > > everything about it in xorg driver. I'm going to submit xorg > > patches. > > BTW, yet another kernel patch is missing; the support of embedded LED. > I've posted this once, but it seems forgotten since then. Reposted > below. Oh, any yet another patch, which enables multi-touch mode forcibly. I see a similar option in your patch, so this might be useless. But, I found that some old laptops have a little MT-support although they have no such capability bit. They can detect multi-fingers but can't track the positions, it seems. We'd need to add some whitelist for such devices. Takashi --- From: Takashi Iwai <tiwai@suse.de> Subject: Add multi_touch parameter to psmouse driver The multi-touch feature of Synaptics device is disabled unless this option is set. Setting to 2 forces the multi-touch mode no matter whether the feature is detected or not. Signed-off-by: Takashi Iwai <tiwai@suse.de> --- drivers/input/mouse/synaptics.c | 106 +++++++++++++++++++++++++++++++--------- 1 file changed, 83 insertions(+), 23 deletions(-) --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -482,10 +482,88 @@ } #ifdef CONFIG_MOUSE_PS2_SYNAPTICS_MULTI_TOUCH -#define is_multi_touch(priv) (priv)->can_multi_touch +static int multi_touch_flag; + +static void setup_multi_touch(struct psmouse *psmouse, int verbose); + +static struct psmouse *_psmouse; + +static int param_set_multi_touch(const char *val, const struct kernel_param *kp) +{ + int mode, mode_changed; + + if (!val) + return -EINVAL; + mode = simple_strtol(val, NULL, 0); + if (mode < 0 || mode > 2) + return -EINVAL; + mode_changed = mode != multi_touch_flag; + multi_touch_flag = mode; + if (mode_changed) + setup_multi_touch(_psmouse, 1); + return 0; +} + +#define param_check_multi_touch(name, p) __param_check(name, p, int) + +static struct kernel_param_ops param_ops_multi_touch = { + .set = param_set_multi_touch, + .get = param_get_int, +}; + +module_param_named(multi_touch, multi_touch_flag, multi_touch, 0644); + +static inline int is_multi_touch(struct synaptics_data *priv) +{ + return (multi_touch_flag == 2 || + (priv->can_multi_touch && multi_touch_flag)); +} + +static void setup_multi_touch(struct psmouse *psmouse, int verbose) +{ + struct input_dev *dev; + struct synaptics_data *priv; + + _psmouse = psmouse; + if (!psmouse) + return; + dev = psmouse->dev; + priv = psmouse->private; + if (!dev || !priv) + return; + if (is_multi_touch(priv) && + !synaptics_init_multi_touch(psmouse)) { + if (verbose) + printk(KERN_INFO "Synaptics: enabling multi-touch\n"); + if (!SYN_CAP_MULTIFINGER(priv->capabilities)) { + __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit); + __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit); + } + input_set_abs_params(dev, ABS_MT_POSITION_X, + XMIN_NOMINAL, XMAX_NOMINAL, 0, 0); + input_set_abs_params(dev, ABS_MT_POSITION_Y, + YMIN_NOMINAL, YMAX_NOMINAL, 0, 0); + input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0); + input_abs_set_res(dev, ABS_MT_POSITION_X, priv->x_res); + input_abs_set_res(dev, ABS_MT_POSITION_Y, priv->y_res); + } else { + if (verbose) + printk(KERN_INFO "Synaptics: disabling multi-touch\n"); + if (!SYN_CAP_MULTIFINGER(priv->capabilities)) { + __clear_bit(BTN_TOOL_DOUBLETAP, dev->keybit); + __clear_bit(BTN_TOOL_TRIPLETAP, dev->keybit); + } + __clear_bit(ABS_MT_POSITION_X, dev->absbit); + __clear_bit(ABS_MT_POSITION_Y, dev->absbit); + __clear_bit(ABS_MT_PRESSURE, dev->absbit); + } +} + #else #define is_multi_touch(priv) 0 +#define setup_multi_touch(ps, v) do { } while (0) #endif + /* the multi-touch packet contains w=2 (like pen) */ #define is_multi_touch_packet(priv, hw) \ (is_multi_touch(priv) && (hw)->w == 2) @@ -781,7 +859,7 @@ __set_bit(BTN_LEFT, dev->keybit); __set_bit(BTN_RIGHT, dev->keybit); - if (SYN_CAP_MULTIFINGER(priv->capabilities) || is_multi_touch(priv)) { + if (SYN_CAP_MULTIFINGER(priv->capabilities)) { __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit); __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit); } @@ -810,20 +888,11 @@ __clear_bit(BTN_RIGHT, dev->keybit); __clear_bit(BTN_MIDDLE, dev->keybit); } - - if (is_multi_touch(priv)) { - input_set_abs_params(dev, ABS_MT_POSITION_X, - XMIN_NOMINAL, XMAX_NOMINAL, 0, 0); - input_set_abs_params(dev, ABS_MT_POSITION_Y, - YMIN_NOMINAL, YMAX_NOMINAL, 0, 0); - input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0); - input_abs_set_res(dev, ABS_MT_POSITION_X, priv->x_res); - input_abs_set_res(dev, ABS_MT_POSITION_Y, priv->y_res); - } } static void synaptics_disconnect(struct psmouse *psmouse) { + setup_multi_touch(NULL, 0); synaptics_free_led(psmouse); synaptics_reset(psmouse); kfree(psmouse->private); @@ -857,8 +926,7 @@ } synaptics_sync_led(psmouse); - if (is_multi_touch(priv)) - synaptics_init_multi_touch(psmouse); + setup_multi_touch(psmouse, 0); return 0; } @@ -937,16 +1005,8 @@ if (synaptics_init_led(psmouse) < 0) goto init_fail; - if (priv->can_multi_touch) { - if (synaptics_init_multi_touch(psmouse)) { - printk(KERN_WARNING "Synaptics: " - "unable to initialize multi-touch\n"); - priv->can_multi_touch = 0; - } else - printk(KERN_INFO "Synaptics: multi-touch enabled\n"); - } - set_input_params(psmouse->dev, priv); + setup_multi_touch(psmouse, 0); /* * Encode touchpad model so that it can be used to set ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 16:37 ` [PATCH 0/3] Input: synaptics - multitouch and multifinger support Takashi Iwai 2010-10-08 16:38 ` Takashi Iwai @ 2010-10-08 17:15 ` Chase Douglas 2010-10-08 17:46 ` Takashi Iwai 2010-10-08 18:04 ` Dmitry Torokhov 2010-10-10 7:49 ` Henrik Rydberg 2 siblings, 2 replies; 36+ messages in thread From: Chase Douglas @ 2010-10-08 17:15 UTC (permalink / raw) To: Takashi Iwai Cc: linux-input, xorg-devel, Dmitry Torokhov, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On Fri, 2010-10-08 at 18:37 +0200, Takashi Iwai wrote: > At Fri, 8 Oct 2010 10:57:57 -0400, > Chase Douglas wrote: > > > > Tobyn Bertram reverse engineered the multitouch protocol for Synaptics devices. > > I've been able to take his work and produce a series of commits to enable MT > > and multifinger (MF) support. > > > > Unfortunately, there's a tricky issue with some Synaptics touchpads that have > > integrated buttons. For example, the left and right buttons on the touchpad of > > my Dell Mini 1012 consist of the lower ~20% of the touchpad surface. The > > touchpad physically clicks under these areas. > > > > The X synaptics input module now has a parameter to disable touches occuring > > over the button area, but this solution still doesn't work perfectly. If you > > click a button and drag with another finger near the clicking finger, the > > touchpad gets confused. > > > > Now that we have full MT support, we can try to handle this scenario better. > > What I've found to work best is to make touches vanish if they occur over the > > button area of the trackpad while any button is held. This works in conjunction > > with the X synaptics driver to disable single touch control over the button > > area. With full MT support, the touchpad doesn't seem to get confused when a > > click and drag occurs with two fingers close to each other, and it enables MT > > gestures and MF support across the entire trackpad when no buttons are held. > > > > The first question is whether this seems appropriate to others, or if some > > other method would work better. Secondarily, should the solution occur in the > > kernel, like I have in the third patch of this series, or should it occur in > > the X input module? Although we don't have this information today, we may be > > able to query the touchpad in the future to know the area of the integrated > > buttons. If that were possible, would the recommended location for the hack > > change? > > Great! Finally someone found it out! > I found this and made a series of patches in 4 months ago. Since > then, Novell legal prohibited me to send the patches to the upstream > due to "possible patent infringing". Now you cracked out. Yay. > > FWIW, my corresponding patch is below. It really looks similar in the > end ;) I added a kconfig just to be safer. > > Regarding the "clickpad" support: in my case, I implemented almost > everything about it in xorg driver. I'm going to submit xorg > patches. So I'm confused. I was working off of source code posted to: https://bugs.launchpad.net/utouch/+bug/633225 I was under the impression that someone else had reverse engineered the protocol and written patches. But the code is exactly the same as what you've posted here. If you're the originator of the work, and my patch is accepted, I think we'll need your SOB on it. Of course, if your patch is accepted then the point is moot :). My patch essentially is a rework of yours, incorporating changes that allow for the driver to work in single touch and multitouch mode simultaneously. As is done in other MT drivers, one touch is picked to act as the single touch emulation. However, as I sit here using the touchpad and thinking some more, I'm not sure how to best handle single touch emulation for synaptics. Single touch emulation only really works when touches are tracked. The touches from the synaptics driver are swapped whenever one touch moves and the other stays stationary. I think I'm noticing some issues with the following sequence of events: 1. Two touches begin, triggering a DOUBLETAP key event 2. X synaptics treats DOUBLETAP as scroll events 3. One touch moves up, it's sent as ABS_{X,Y}, scroll performed 4. The touch in motion stops 5. Other touch moves up, it's now sent as ABS_{X,Y} 6. X synaptics scroll behaves poorly because this new touch's X,Y are in a different place from the first touch's X,Y. Scrolling seems to snap back to original location Certainly it's not hard to perform touch tracking when only two touches are active. However, Henrik, as the MT input guru, has resisted doing in kernel tracking, at least in a hackish, per-driver manner. It's why he wrote mtdev in user space, for example. However, unless mtdev is extended to support single touch emulation, we're kind of stuck without in kernel tracking. -- Chase ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 17:15 ` Chase Douglas @ 2010-10-08 17:46 ` Takashi Iwai 2010-10-08 18:04 ` Dmitry Torokhov 1 sibling, 0 replies; 36+ messages in thread From: Takashi Iwai @ 2010-10-08 17:46 UTC (permalink / raw) To: Chase Douglas Cc: linux-input, xorg-devel, Dmitry Torokhov, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Fri, 08 Oct 2010 13:15:35 -0400, Chase Douglas wrote: > > On Fri, 2010-10-08 at 18:37 +0200, Takashi Iwai wrote: > > At Fri, 8 Oct 2010 10:57:57 -0400, > > Chase Douglas wrote: > > > > > > Tobyn Bertram reverse engineered the multitouch protocol for Synaptics devices. > > > I've been able to take his work and produce a series of commits to enable MT > > > and multifinger (MF) support. > > > > > > Unfortunately, there's a tricky issue with some Synaptics touchpads that have > > > integrated buttons. For example, the left and right buttons on the touchpad of > > > my Dell Mini 1012 consist of the lower ~20% of the touchpad surface. The > > > touchpad physically clicks under these areas. > > > > > > The X synaptics input module now has a parameter to disable touches occuring > > > over the button area, but this solution still doesn't work perfectly. If you > > > click a button and drag with another finger near the clicking finger, the > > > touchpad gets confused. > > > > > > Now that we have full MT support, we can try to handle this scenario better. > > > What I've found to work best is to make touches vanish if they occur over the > > > button area of the trackpad while any button is held. This works in conjunction > > > with the X synaptics driver to disable single touch control over the button > > > area. With full MT support, the touchpad doesn't seem to get confused when a > > > click and drag occurs with two fingers close to each other, and it enables MT > > > gestures and MF support across the entire trackpad when no buttons are held. > > > > > > The first question is whether this seems appropriate to others, or if some > > > other method would work better. Secondarily, should the solution occur in the > > > kernel, like I have in the third patch of this series, or should it occur in > > > the X input module? Although we don't have this information today, we may be > > > able to query the touchpad in the future to know the area of the integrated > > > buttons. If that were possible, would the recommended location for the hack > > > change? > > > > Great! Finally someone found it out! > > I found this and made a series of patches in 4 months ago. Since > > then, Novell legal prohibited me to send the patches to the upstream > > due to "possible patent infringing". Now you cracked out. Yay. > > > > FWIW, my corresponding patch is below. It really looks similar in the > > end ;) I added a kconfig just to be safer. > > > > Regarding the "clickpad" support: in my case, I implemented almost > > everything about it in xorg driver. I'm going to submit xorg > > patches. > > So I'm confused. I was working off of source code posted to: > > https://bugs.launchpad.net/utouch/+bug/633225 > > I was under the impression that someone else had reverse engineered the > protocol and written patches. But the code is exactly the same as what > you've posted here. If you're the originator of the work, and my patch > is accepted, I think we'll need your SOB on it. Of course, if your patch > is accepted then the point is moot :). Hm, then this was a leak, likely taken from Novell bugzilla or build service. The patch was written and published once before I was prohibited to send to upstream, so basically it was fine to go outside by itself ;) > My patch essentially is a rework of yours, incorporating changes that > allow for the driver to work in single touch and multitouch mode > simultaneously. As is done in other MT drivers, one touch is picked to > act as the single touch emulation. > > However, as I sit here using the touchpad and thinking some more, I'm > not sure how to best handle single touch emulation for synaptics. Single > touch emulation only really works when touches are tracked. The touches > from the synaptics driver are swapped whenever one touch moves and the > other stays stationary. I think I'm noticing some issues with the > following sequence of events: > > 1. Two touches begin, triggering a DOUBLETAP key event > 2. X synaptics treats DOUBLETAP as scroll events > 3. One touch moves up, it's sent as ABS_{X,Y}, scroll performed > 4. The touch in motion stops > 5. Other touch moves up, it's now sent as ABS_{X,Y} > 6. X synaptics scroll behaves poorly because this new touch's X,Y are in > a different place from the first touch's X,Y. Scrolling seems to snap > back to original location > > Certainly it's not hard to perform touch tracking when only two touches > are active. However, Henrik, as the MT input guru, has resisted doing in > kernel tracking, at least in a hackish, per-driver manner. It's why he > wrote mtdev in user space, for example. However, unless mtdev is > extended to support single touch emulation, we're kind of stuck without > in kernel tracking. Yeah, there are lots of rooms regarding the handling of multi-touch touchpad. I feel that handling the touchpad in the same way as touch-screen isn't always wise even though they are both multi-touch, since they are completely different devices; the movement of the former is relative while the latter takes always the absolute coordinate. Anyway, I'd love to help improving things once when I'm back to work. thanks, Takashi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 17:15 ` Chase Douglas 2010-10-08 17:46 ` Takashi Iwai @ 2010-10-08 18:04 ` Dmitry Torokhov 2010-10-08 19:31 ` Takashi Iwai 1 sibling, 1 reply; 36+ messages in thread From: Dmitry Torokhov @ 2010-10-08 18:04 UTC (permalink / raw) To: Chase Douglas Cc: Takashi Iwai, linux-input, xorg-devel, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On Friday, October 08, 2010 10:15:35 am Chase Douglas wrote: > On Fri, 2010-10-08 at 18:37 +0200, Takashi Iwai wrote: > > At Fri, 8 Oct 2010 10:57:57 -0400, > > > > Chase Douglas wrote: > > > > > > > > > Tobyn Bertram reverse engineered the multitouch protocol for Synaptics > > > devices. I've been able to take his work and produce a series of > > > commits to enable MT and multifinger (MF) support. > > > > > > > > > > > > Unfortunately, there's a tricky issue with some Synaptics touchpads > > > that have integrated buttons. For example, the left and right buttons > > > on the touchpad of my Dell Mini 1012 consist of the lower ~20% of the > > > touchpad surface. The touchpad physically clicks under these areas. > > > > > > > > > > > > The X synaptics input module now has a parameter to disable touches > > > occuring over the button area, but this solution still doesn't work > > > perfectly. If you click a button and drag with another finger near the > > > clicking finger, the touchpad gets confused. > > > > > > > > > > > > Now that we have full MT support, we can try to handle this scenario > > > better. What I've found to work best is to make touches vanish if they > > > occur over the button area of the trackpad while any button is held. > > > This works in conjunction with the X synaptics driver to disable > > > single touch control over the button area. With full MT support, the > > > touchpad doesn't seem to get confused when a click and drag occurs > > > with two fingers close to each other, and it enables MT gestures and > > > MF support across the entire trackpad when no buttons are held. > > > > > > > > > > > > The first question is whether this seems appropriate to others, or if > > > some other method would work better. Secondarily, should the solution > > > occur in the kernel, like I have in the third patch of this series, or > > > should it occur in the X input module? Although we don't have this > > > information today, we may be able to query the touchpad in the future > > > to know the area of the integrated buttons. If that were possible, > > > would the recommended location for the hack change? > > > > > > > > Great! Finally someone found it out! > > I found this and made a series of patches in 4 months ago. Since > > then, Novell legal prohibited me to send the patches to the upstream > > due to "possible patent infringing". Now you cracked out. Yay. > > > > > > > > FWIW, my corresponding patch is below. It really looks similar in the > > end ;) I added a kconfig just to be safer. > > > > > > > > Regarding the "clickpad" support: in my case, I implemented almost > > everything about it in xorg driver. I'm going to submit xorg > > patches. > > So I'm confused. I was working off of source code posted to: > > https://bugs.launchpad.net/utouch/+bug/633225 > > I was under the impression that someone else had reverse engineered the > protocol and written patches. But the code is exactly the same as what > you've posted here. If you're the originator of the work, and my patch > is accepted, I think we'll need your SOB on it. Comment #6 is quite clear on this matter: > Takashi Iwai from OpenSuse has done quite a bit of work for the Synaptics > Clickpad including some experimental multitouch support, his repo is here: > http://download.opensuse.org/repositories/home:/tiwai:/clickpad:/openSUSE_ > 11.3/openSUSE_11.3/src/ > > I have played around with the synaptics.c code in the kernel to add > multitouch events (ABS_MT_POSITION_X, ABS_MT_POSITION_Y, ABS_MT_PRESSURE) > using Takashi's work as a model. So I do believe we need to have Takashi's SOB at the very least and maybe credit him as the author of the patches. -- Dmitry ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 18:04 ` Dmitry Torokhov @ 2010-10-08 19:31 ` Takashi Iwai 2010-10-10 21:04 ` Dmitry Torokhov 0 siblings, 1 reply; 36+ messages in thread From: Takashi Iwai @ 2010-10-08 19:31 UTC (permalink / raw) To: Dmitry Torokhov Cc: Chase Douglas, linux-input, xorg-devel, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Fri, 8 Oct 2010 11:04:01 -0700, Dmitry Torokhov wrote: > > On Friday, October 08, 2010 10:15:35 am Chase Douglas wrote: > > On Fri, 2010-10-08 at 18:37 +0200, Takashi Iwai wrote: > > > At Fri, 8 Oct 2010 10:57:57 -0400, > > > > > > Chase Douglas wrote: > > > > > > > > > > > > Tobyn Bertram reverse engineered the multitouch protocol for Synaptics > > > > devices. I've been able to take his work and produce a series of > > > > commits to enable MT and multifinger (MF) support. > > > > > > > > > > > > > > > > Unfortunately, there's a tricky issue with some Synaptics touchpads > > > > that have integrated buttons. For example, the left and right buttons > > > > on the touchpad of my Dell Mini 1012 consist of the lower ~20% of the > > > > touchpad surface. The touchpad physically clicks under these areas. > > > > > > > > > > > > > > > > The X synaptics input module now has a parameter to disable touches > > > > occuring over the button area, but this solution still doesn't work > > > > perfectly. If you click a button and drag with another finger near the > > > > clicking finger, the touchpad gets confused. > > > > > > > > > > > > > > > > Now that we have full MT support, we can try to handle this scenario > > > > better. What I've found to work best is to make touches vanish if they > > > > occur over the button area of the trackpad while any button is held. > > > > This works in conjunction with the X synaptics driver to disable > > > > single touch control over the button area. With full MT support, the > > > > touchpad doesn't seem to get confused when a click and drag occurs > > > > with two fingers close to each other, and it enables MT gestures and > > > > MF support across the entire trackpad when no buttons are held. > > > > > > > > > > > > > > > > The first question is whether this seems appropriate to others, or if > > > > some other method would work better. Secondarily, should the solution > > > > occur in the kernel, like I have in the third patch of this series, or > > > > should it occur in the X input module? Although we don't have this > > > > information today, we may be able to query the touchpad in the future > > > > to know the area of the integrated buttons. If that were possible, > > > > would the recommended location for the hack change? > > > > > > > > > > > > Great! Finally someone found it out! > > > I found this and made a series of patches in 4 months ago. Since > > > then, Novell legal prohibited me to send the patches to the upstream > > > due to "possible patent infringing". Now you cracked out. Yay. > > > > > > > > > > > > FWIW, my corresponding patch is below. It really looks similar in the > > > end ;) I added a kconfig just to be safer. > > > > > > > > > > > > Regarding the "clickpad" support: in my case, I implemented almost > > > everything about it in xorg driver. I'm going to submit xorg > > > patches. > > > > So I'm confused. I was working off of source code posted to: > > > > https://bugs.launchpad.net/utouch/+bug/633225 > > > > I was under the impression that someone else had reverse engineered the > > protocol and written patches. But the code is exactly the same as what > > you've posted here. If you're the originator of the work, and my patch > > is accepted, I think we'll need your SOB on it. > > Comment #6 is quite clear on this matter: > > > Takashi Iwai from OpenSuse has done quite a bit of work for the Synaptics > > Clickpad including some experimental multitouch support, his repo is here: > > http://download.opensuse.org/repositories/home:/tiwai:/clickpad:/openSUSE_ > > 11.3/openSUSE_11.3/src/ > > > > I have played around with the synaptics.c code in the kernel to add > > multitouch events (ABS_MT_POSITION_X, ABS_MT_POSITION_Y, ABS_MT_PRESSURE) > > using Takashi's work as a model. > > So I do believe we need to have Takashi's SOB at the very least and maybe > credit him as the author of the patches. I sent my original one, so this should suffice, right? thanks, Takashi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 19:31 ` Takashi Iwai @ 2010-10-10 21:04 ` Dmitry Torokhov 2010-10-11 7:35 ` Takashi Iwai 0 siblings, 1 reply; 36+ messages in thread From: Dmitry Torokhov @ 2010-10-10 21:04 UTC (permalink / raw) To: Takashi Iwai Cc: Chase Douglas, linux-input, xorg-devel, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor On Fri, Oct 08, 2010 at 09:31:30PM +0200, Takashi Iwai wrote: > At Fri, 8 Oct 2010 11:04:01 -0700, > Dmitry Torokhov wrote: > > > > So I do believe we need to have Takashi's SOB at the very least and maybe > > credit him as the author of the patches. > > I sent my original one, so this should suffice, right? > Takashi, Generally I think people should not add or remove any SOB lines except for their own as the work on the patches, so original patches should retain their original SOBs... In absence of that I'd like to have explicit confirmations that I can add SOBs statements; otherwise it just dilutes the value of SOBs significantly. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-10 21:04 ` Dmitry Torokhov @ 2010-10-11 7:35 ` Takashi Iwai 2010-10-11 7:48 ` Henrik Rydberg 0 siblings, 1 reply; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 7:35 UTC (permalink / raw) To: Dmitry Torokhov Cc: Chase Douglas, linux-input, xorg-devel, Chris Bagwell, Andy Whitcroft, Henrik Rydberg, linux-kernel, Peter Hutterer, Duncan McGreggor At Sun, 10 Oct 2010 14:04:34 -0700, Dmitry Torokhov wrote: > > On Fri, Oct 08, 2010 at 09:31:30PM +0200, Takashi Iwai wrote: > > At Fri, 8 Oct 2010 11:04:01 -0700, > > Dmitry Torokhov wrote: > > > > > > So I do believe we need to have Takashi's SOB at the very least and maybe > > > credit him as the author of the patches. > > > > I sent my original one, so this should suffice, right? > > > > Takashi, > > Generally I think people should not add or remove any SOB lines except > for their own as the work on the patches, so original patches should > retain their original SOBs... In absence of that I'd like to have > explicit confirmations that I can add SOBs statements; otherwise it just > dilutes the value of SOBs significantly. Yeah, in general, sign-offs should be retained. In this particular case, it's also my (well more exactly my employer's) fault, that blocked the submission of the original patch. In anyway, feel free to add my sign-off there since I already posted my own one as a reference. But, I have an open issue with Chase's patch. Maybe it's rather a question to Henrik: Shouldn't all points be reported as ABS_MT_* events? So far, only the second touch-point is reported via ABS_MT_* while the first point is still reported as ABX_[X|Y]. I corrected this in my patch I posted, but I wasn't sure, too. thanks, Takashi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 7:35 ` Takashi Iwai @ 2010-10-11 7:48 ` Henrik Rydberg 2010-10-11 7:59 ` Takashi Iwai 2010-10-11 13:41 ` Chris Bagwell 0 siblings, 2 replies; 36+ messages in thread From: Henrik Rydberg @ 2010-10-11 7:48 UTC (permalink / raw) To: Takashi Iwai Cc: Dmitry Torokhov, Chase Douglas, linux-input, xorg-devel, Chris Bagwell, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor On 10/11/2010 09:35 AM, Takashi Iwai wrote: [...] > In anyway, feel free to add my sign-off there since I already posted > my own one as a reference. > > But, I have an open issue with Chase's patch. Maybe it's rather a > question to Henrik: > > Shouldn't all points be reported as ABS_MT_* events? So far, only the > second touch-point is reported via ABS_MT_* while the first point is > still reported as ABX_[X|Y]. > > I corrected this in my patch I posted, but I wasn't sure, too. I have issues with all submitted patches, but did not give explicit reasons since there were overlapping submissions. Perhaps Chase and yourself can work out how you want to submit the new patches? And yes, all points should be reported as ABS_MT events. Thanks, Henrik ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 7:48 ` Henrik Rydberg @ 2010-10-11 7:59 ` Takashi Iwai 2010-10-11 13:41 ` Chris Bagwell 1 sibling, 0 replies; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 7:59 UTC (permalink / raw) To: Henrik Rydberg Cc: Dmitry Torokhov, Chase Douglas, linux-input, xorg-devel, Chris Bagwell, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor At Mon, 11 Oct 2010 09:48:36 +0200, Henrik Rydberg wrote: > > On 10/11/2010 09:35 AM, Takashi Iwai wrote: > [...] > > > In anyway, feel free to add my sign-off there since I already posted > > my own one as a reference. > > > > But, I have an open issue with Chase's patch. Maybe it's rather a > > question to Henrik: > > > > Shouldn't all points be reported as ABS_MT_* events? So far, only the > > second touch-point is reported via ABS_MT_* while the first point is > > still reported as ABX_[X|Y]. > > > > I corrected this in my patch I posted, but I wasn't sure, too. > > > I have issues with all submitted patches, but did not give explicit reasons > since there were overlapping submissions. Perhaps Chase and yourself can work > out how you want to submit the new patches? Yeah, we definitely work on patches and sort out issues. > And yes, all points should be reported as ABS_MT events. OK, thanks for clarification! Takashi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 7:48 ` Henrik Rydberg 2010-10-11 7:59 ` Takashi Iwai @ 2010-10-11 13:41 ` Chris Bagwell 2010-10-11 14:01 ` Takashi Iwai 1 sibling, 1 reply; 36+ messages in thread From: Chris Bagwell @ 2010-10-11 13:41 UTC (permalink / raw) To: Henrik Rydberg Cc: Takashi Iwai, Dmitry Torokhov, Chase Douglas, linux-input, xorg-devel, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor On Mon, Oct 11, 2010 at 2:48 AM, Henrik Rydberg <rydberg@euromail.se> wrote: > On 10/11/2010 09:35 AM, Takashi Iwai wrote: > [...] > >> In anyway, feel free to add my sign-off there since I already posted >> my own one as a reference. >> >> But, I have an open issue with Chase's patch. Maybe it's rather a >> question to Henrik: >> >> Shouldn't all points be reported as ABS_MT_* events? So far, only the >> second touch-point is reported via ABS_MT_* while the first point is >> still reported as ABX_[X|Y]. >> >> I corrected this in my patch I posted, but I wasn't sure, too. > > > I have issues with all submitted patches, but did not give explicit reasons > since there were overlapping submissions. Perhaps Chase and yourself can work > out how you want to submit the new patches? And yes, all points should be > reported as ABS_MT events. > > Thanks, > Henrik > And is it also safe to say that we need to continue to report ABS_X/ABS_Y *and* those values need to always track 1st finger touch for backwards compatibility? It was brought up in thread but not stated as strong requirement. BTW, there are patches in last couple months to x86-input-synaptics that will allow it to ignore jumps in values of ABS_X/ABS_Y when transition of multi-touch occur (both adding or removing fingers via BTN_TOOL_*TAP). So one new-ish option is for ABS_X/ABS_Y to not track 1st finger but become average of 2 fingers. Chris -- 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 13:41 ` Chris Bagwell @ 2010-10-11 14:01 ` Takashi Iwai 2010-10-11 14:24 ` Henrik Rydberg 0 siblings, 1 reply; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 14:01 UTC (permalink / raw) To: Chris Bagwell Cc: Henrik Rydberg, Dmitry Torokhov, Chase Douglas, linux-input, xorg-devel, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor At Mon, 11 Oct 2010 08:41:44 -0500, Chris Bagwell wrote: > > On Mon, Oct 11, 2010 at 2:48 AM, Henrik Rydberg <rydberg@euromail.se> wrote: > > On 10/11/2010 09:35 AM, Takashi Iwai wrote: > > [...] > > > >> In anyway, feel free to add my sign-off there since I already posted > >> my own one as a reference. > >> > >> But, I have an open issue with Chase's patch. Maybe it's rather a > >> question to Henrik: > >> > >> Shouldn't all points be reported as ABS_MT_* events? So far, only the > >> second touch-point is reported via ABS_MT_* while the first point is > >> still reported as ABX_[X|Y]. > >> > >> I corrected this in my patch I posted, but I wasn't sure, too. > > > > > > I have issues with all submitted patches, but did not give explicit reasons > > since there were overlapping submissions. Perhaps Chase and yourself can work > > out how you want to submit the new patches? And yes, all points should be > > reported as ABS_MT events. > > > > Thanks, > > Henrik > > > > And is it also safe to say that we need to continue to report > ABS_X/ABS_Y *and* those values need to always track 1st finger touch > for backwards compatibility? Indeed this was an implicit question of my previous inquiry. I suppose mtdev tracks only ABS_MT_*? > It was brought up in thread but not stated as strong requirement. > > BTW, there are patches in last couple months to x86-input-synaptics > that will allow it to ignore jumps in values of ABS_X/ABS_Y when > transition of multi-touch occur (both adding or removing fingers via > BTN_TOOL_*TAP). So one new-ish option is for ABS_X/ABS_Y to not track > 1st finger but become average of 2 fingers. The tracking of multi-touch is inevitably needed for clickpad devices. But, I'm reluctant for merging the clickpad support code into mtdev. Its behavior is too messy, and it's only for synaptics touchpads, not for touch-screens. For mtdev maintenance POV, I guess, it'd be cleaner to keep this away from it. As an example of mess of Clickpad: if you keep your finger on a button area and another finger on the normal area, you shouldn't trigger the multi-touch mode, no matter whether it's clicked or not. People tend to keep the finger on the button before actually dragging. But, if you put both fingers in the button area and sliding together, it should be handled as two-finger scrolling. Also, if you move one finger on a button area, it should be tracked as a normal pointer movement. Another mess is that, as the default setup, the pointer movement is too sensitive, and when user pushes down the touchpad for clicking, the pointer moves a few or more pixels. This eventually misses the target. So, some trick to drag the pointer is necessary for click action. One of my patches does it by introducing some "move threshold" value. Takashi -- 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 14:01 ` Takashi Iwai @ 2010-10-11 14:24 ` Henrik Rydberg 2010-10-11 14:49 ` Takashi Iwai 0 siblings, 1 reply; 36+ messages in thread From: Henrik Rydberg @ 2010-10-11 14:24 UTC (permalink / raw) To: Takashi Iwai Cc: Chris Bagwell, Dmitry Torokhov, Chase Douglas, linux-input, xorg-devel, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor On 10/11/2010 04:01 PM, Takashi Iwai wrote: [...] > As an example of mess of Clickpad: if you keep your finger on a button > area and another finger on the normal area, you shouldn't trigger the > multi-touch mode, no matter whether it's clicked or not. People tend > to keep the finger on the button before actually dragging. > > But, if you put both fingers in the button area and sliding together, > it should be handled as two-finger scrolling. Also, if you move one > finger on a button area, it should be tracked as a normal pointer > movement. Are you aware of http://bitmath.org/code/multitouch/? The driver was developed to address very similar issues on the macbook trackpads. > Another mess is that, as the default setup, the pointer movement is > too sensitive, and when user pushes down the touchpad for clicking, > the pointer moves a few or more pixels. This eventually misses the > target. So, some trick to drag the pointer is necessary for click > action. One of my patches does it by introducing some "move > threshold" value. Also addressed in that project. What I am hoping to see, eventually, is code similar to the core of the multitouch project in something like ginn. This would exercise the whole utouch stack in an extensible way, and finally make multitouch trackpads work the way they are supposed to. Henrik ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 14:24 ` Henrik Rydberg @ 2010-10-11 14:49 ` Takashi Iwai 2010-10-11 15:31 ` Henrik Rydberg 0 siblings, 1 reply; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 14:49 UTC (permalink / raw) To: Henrik Rydberg Cc: Chris Bagwell, Dmitry Torokhov, Chase Douglas, linux-input, xorg-devel, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor At Mon, 11 Oct 2010 16:24:41 +0200, Henrik Rydberg wrote: > > On 10/11/2010 04:01 PM, Takashi Iwai wrote: > [...] > > > As an example of mess of Clickpad: if you keep your finger on a button > > area and another finger on the normal area, you shouldn't trigger the > > multi-touch mode, no matter whether it's clicked or not. People tend > > to keep the finger on the button before actually dragging. > > > > But, if you put both fingers in the button area and sliding together, > > it should be handled as two-finger scrolling. Also, if you move one > > finger on a button area, it should be tracked as a normal pointer > > movement. > > > Are you aware of http://bitmath.org/code/multitouch/? The driver was developed > to address very similar issues on the macbook trackpads. Well, I thought Macs don't use the certain areas as virtual buttons but determines the button by the number of fingers. Or was it changed? If yes, could you point out which source file does it? I'm looking through the git tree http://bitmath.org/git/multitouch.git, but couldn't find out yet... > > Another mess is that, as the default setup, the pointer movement is > > too sensitive, and when user pushes down the touchpad for clicking, > > the pointer moves a few or more pixels. This eventually misses the > > target. So, some trick to drag the pointer is necessary for click > > action. One of my patches does it by introducing some "move > > threshold" value. > > > Also addressed in that project. What I am hoping to see, eventually, is code > similar to the core of the multitouch project in something like ginn. This would > exercise the whole utouch stack in an extensible way, and finally make > multitouch trackpads work the way they are supposed to. If the messy behavior of clickpad suits with the multi-touch library, I have nothing against it, sure. It was just my concern that the clickpad-style handling might pollute the code. thanks, Takashi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 14:49 ` Takashi Iwai @ 2010-10-11 15:31 ` Henrik Rydberg 2010-10-11 15:58 ` Takashi Iwai 0 siblings, 1 reply; 36+ messages in thread From: Henrik Rydberg @ 2010-10-11 15:31 UTC (permalink / raw) To: Takashi Iwai Cc: Chris Bagwell, Dmitry Torokhov, Chase Douglas, linux-input, xorg-devel, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor On 10/11/2010 04:49 PM, Takashi Iwai wrote: [...] >>> As an example of mess of Clickpad: if you keep your finger on a button >>> area and another finger on the normal area, you shouldn't trigger the >>> multi-touch mode, no matter whether it's clicked or not. People tend >>> to keep the finger on the button before actually dragging. >>> >>> But, if you put both fingers in the button area and sliding together, >>> it should be handled as two-finger scrolling. Also, if you move one >>> finger on a button area, it should be tracked as a normal pointer >>> movement. >> >> >> Are you aware of http://bitmath.org/code/multitouch/? The driver was developed >> to address very similar issues on the macbook trackpads. > > Well, I thought Macs don't use the certain areas as virtual buttons > but determines the button by the number of fingers. Or was it > changed? If yes, could you point out which source file does it? The trackpads with integrated button are like clickpads with one button. It would certainly be interesting to use the clickpad pattern on those as well. For both types of pads, the bottom area is special. Two-finger scroll works as normal, while a clicking finger may move around without affecting anything, nor the number of fingers on the pad. > I'm looking through the git tree > http://bitmath.org/git/multitouch.git, but couldn't find out yet... src/memory.c might be what you are looking for. There are also two unpublished branches around - one which uses the mtdev library, and one which uses utouch-grail directly (but with limited functionality). >>> Another mess is that, as the default setup, the pointer movement is >>> too sensitive, and when user pushes down the touchpad for clicking, >>> the pointer moves a few or more pixels. This eventually misses the >>> target. So, some trick to drag the pointer is necessary for click >>> action. One of my patches does it by introducing some "move >>> threshold" value. >> >> >> Also addressed in that project. What I am hoping to see, eventually, is code >> similar to the core of the multitouch project in something like ginn. This would >> exercise the whole utouch stack in an extensible way, and finally make >> multitouch trackpads work the way they are supposed to. > > If the messy behavior of clickpad suits with the multi-touch library, > I have nothing against it, sure. It was just my concern that the > clickpad-style handling might pollute the code. I am not sure it has to be messy at all, and it has to reside *somewhere*. I think something like ginn, or parallel to ginn, is the best long-run place for it. Submitting clickpad functionality to multitouch for starters would definitely work in my book. Cheers, Henrik ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 15:31 ` Henrik Rydberg @ 2010-10-11 15:58 ` Takashi Iwai 0 siblings, 0 replies; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 15:58 UTC (permalink / raw) To: Henrik Rydberg Cc: Chris Bagwell, Dmitry Torokhov, Chase Douglas, linux-input, xorg-devel, Andy Whitcroft, linux-kernel, Peter Hutterer, Duncan McGreggor At Mon, 11 Oct 2010 17:31:22 +0200, Henrik Rydberg wrote: > > On 10/11/2010 04:49 PM, Takashi Iwai wrote: > [...] > >>> As an example of mess of Clickpad: if you keep your finger on a button > > >>> area and another finger on the normal area, you shouldn't trigger the > >>> multi-touch mode, no matter whether it's clicked or not. People tend > >>> to keep the finger on the button before actually dragging. > >>> > >>> But, if you put both fingers in the button area and sliding together, > >>> it should be handled as two-finger scrolling. Also, if you move one > >>> finger on a button area, it should be tracked as a normal pointer > >>> movement. > >> > >> > >> Are you aware of http://bitmath.org/code/multitouch/? The driver was developed > >> to address very similar issues on the macbook trackpads. > > > > Well, I thought Macs don't use the certain areas as virtual buttons > > but determines the button by the number of fingers. Or was it > > changed? If yes, could you point out which source file does it? > > > The trackpads with integrated button are like clickpads with one button. It > would certainly be interesting to use the clickpad pattern on those as well. For > both types of pads, the bottom area is special. Two-finger scroll works as > normal, while a clicking finger may move around without affecting anything, nor > the number of fingers on the pad. > > > I'm looking through the git tree > > http://bitmath.org/git/multitouch.git, but couldn't find out yet... > > > src/memory.c might be what you are looking for. There are also two unpublished > branches around - one which uses the mtdev library, and one which uses > utouch-grail directly (but with limited functionality). Thanks, I found what it does now. Indeed, the clickpad behavior would be to add left/right areas (and the middle area if wanted) instead of a single button area. > >>> Another mess is that, as the default setup, the pointer movement is > >>> too sensitive, and when user pushes down the touchpad for clicking, > >>> the pointer moves a few or more pixels. This eventually misses the > >>> target. So, some trick to drag the pointer is necessary for click > >>> action. One of my patches does it by introducing some "move > >>> threshold" value. > >> > >> > >> Also addressed in that project. What I am hoping to see, eventually, is code > >> similar to the core of the multitouch project in something like ginn. This would > >> exercise the whole utouch stack in an extensible way, and finally make > >> multitouch trackpads work the way they are supposed to. > > > > If the messy behavior of clickpad suits with the multi-touch library, > > I have nothing against it, sure. It was just my concern that the > > clickpad-style handling might pollute the code. > > > I am not sure it has to be messy at all, and it has to reside *somewhere*. I > think something like ginn, or parallel to ginn, is the best long-run place for > it. Submitting clickpad functionality to multitouch for starters would > definitely work in my book. OK, that sounds good. What I was concerned is that the choice of sensitivity and acceleration functions for pointer movement aren't easy for such devices. In the button area, especially insensitive movement is required because of pressing. The default function of synaptics driver was too sensitive, and thus a special handling was needed. Takashi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-08 16:37 ` [PATCH 0/3] Input: synaptics - multitouch and multifinger support Takashi Iwai 2010-10-08 16:38 ` Takashi Iwai 2010-10-08 17:15 ` Chase Douglas @ 2010-10-10 7:49 ` Henrik Rydberg 2010-10-10 20:59 ` Dmitry Torokhov 2 siblings, 1 reply; 36+ messages in thread From: Henrik Rydberg @ 2010-10-10 7:49 UTC (permalink / raw) To: Takashi Iwai, Chase Douglas; +Cc: linux-input, Dmitry Torokhov, linux-kernel Takashi, Chase, talk about Heinz effect. :-) Rather than taking any of the patches, I was wondering if we could get a single patch including all the later findings and considerations on what devices should have the multitouch mode. Also, unless there is a really really good reason for it, without kernel parameters. After all, these patches should only add new functionality without regressions. Regarding the clickpad functionality, it is similar to the macbook pads with integrated buttons, which has been implemented in userspace. Regarding the tracking aspect of pointer emulation, Chase is completely right, and this is a generic problem for several drivers. Tracking a single point by picking the closest contact is a linear problem and could easily be performed in interrupt context. I suggest we add it directly to the synaptics driver and then revisit the question of adding generic tracking support to the kernel. Dmitry, how does that sound to you? Cheers, Henrik ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-10 7:49 ` Henrik Rydberg @ 2010-10-10 20:59 ` Dmitry Torokhov 2010-10-11 7:28 ` Takashi Iwai 0 siblings, 1 reply; 36+ messages in thread From: Dmitry Torokhov @ 2010-10-10 20:59 UTC (permalink / raw) To: Henrik Rydberg; +Cc: Takashi Iwai, Chase Douglas, linux-input, linux-kernel On Sun, Oct 10, 2010 at 09:49:45AM +0200, Henrik Rydberg wrote: > Takashi, Chase, > > talk about Heinz effect. :-) > > Rather than taking any of the patches, I was wondering if we could get a single > patch including all the later findings and considerations on what devices should > have the multitouch mode. Also, unless there is a really really good reason for > it, without kernel parameters. After all, these patches should only add new > functionality without regressions. Yes, I think we should limit number of module options (and even more so Kconfig options). I believe that either MT support is ready for prime-time and then it should be enabled unconditionally, or it is not ready and should be kept as patches. his also means that we should not introduce regressions in ST (even when emulated in MT mode). > > Regarding the clickpad functionality, it is similar to the macbook pads with > integrated buttons, which has been implemented in userspace. Right now we map the "click" to BTN_LEFT leaving it to the synaptics X driver to work our the "click zone" support. > > Regarding the tracking aspect of pointer emulation, Chase is completely right, > and this is a generic problem for several drivers. Tracking a single point by > picking the closest contact is a linear problem and could easily be performed in > interrupt context. I suggest we add it directly to the synaptics driver and then > revisit the question of adding generic tracking support to the kernel. Dmitry, > how does that sound to you? Sounds good to me. Hopefully it could be made it a library[ish] function that several MT drivers could use to produce ST-compliant events. -- Dmitry ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-10 20:59 ` Dmitry Torokhov @ 2010-10-11 7:28 ` Takashi Iwai 2010-10-11 7:40 ` Henrik Rydberg 0 siblings, 1 reply; 36+ messages in thread From: Takashi Iwai @ 2010-10-11 7:28 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Henrik Rydberg, Chase Douglas, linux-input, linux-kernel At Sun, 10 Oct 2010 13:59:32 -0700, Dmitry Torokhov wrote: > > On Sun, Oct 10, 2010 at 09:49:45AM +0200, Henrik Rydberg wrote: > > Takashi, Chase, > > > > talk about Heinz effect. :-) > > > > Rather than taking any of the patches, I was wondering if we could get a single > > patch including all the later findings and considerations on what devices should > > have the multitouch mode. Also, unless there is a really really good reason for > > it, without kernel parameters. After all, these patches should only add new > > functionality without regressions. > > Yes, I think we should limit number of module options (and even more so > Kconfig options). I believe that either MT support is ready for prime-time > and then it should be enabled unconditionally, or it is not ready and > should be kept as patches. his also means that we should not introduce > regressions in ST (even when emulated in MT mode). Right, but it's pretty hard to know whether it works really for all machines, simply because the current MT-detection is nothing but a pure guess work. As mentioned in another mail, there are devices that have no this bit but reports multi-finger detections without tracking. So, there are definitely other things to be checked. Thus, I'm inclined to add a module option, at least, for *disabling* MT for non-working machines. Also, another question is whether it's safe to enable MT even for user-space that don't understand MT. Takashi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support 2010-10-11 7:28 ` Takashi Iwai @ 2010-10-11 7:40 ` Henrik Rydberg 0 siblings, 0 replies; 36+ messages in thread From: Henrik Rydberg @ 2010-10-11 7:40 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dmitry Torokhov, Chase Douglas, linux-input, linux-kernel On 10/11/2010 09:28 AM, Takashi Iwai wrote: [...] > Right, but it's pretty hard to know whether it works really for all > machines, simply because the current MT-detection is nothing but a > pure guess work. As mentioned in another mail, there are devices that > have no this bit but reports multi-finger detections without > tracking. So, there are definitely other things to be checked. > > Thus, I'm inclined to add a module option, at least, for *disabling* > MT for non-working machines. Also, another question is whether it's > safe to enable MT even for user-space that don't understand MT. Yes, you can add MT support on top of the ST protocol of an existing driver. Henrik ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2010-10-11 18:29 UTC | newest] Thread overview: 36+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-08 14:57 [PATCH 0/3] Input: synaptics - multitouch and multifinger support Chase Douglas 2010-10-08 14:57 ` [PATCH 1/3] Input: synaptics - add multitouch support Chase Douglas 2010-10-08 14:57 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chase Douglas 2010-10-08 14:58 ` [PATCH 3/3] Input: synaptics - remove touches over button click area Chase Douglas 2010-10-10 15:58 ` Chris Bagwell 2010-10-11 16:24 ` Chris Bagwell 2010-10-11 17:10 ` Takashi Iwai 2010-10-11 17:30 ` Dmitry Torokhov 2010-10-11 17:40 ` Takashi Iwai 2010-10-11 17:46 ` Chris Bagwell 2010-10-11 17:54 ` Henrik Rydberg 2010-10-11 18:29 ` Takashi Iwai 2010-10-10 15:44 ` [PATCH 2/3] Input: synaptics - add multitouch multifinger support Chris Bagwell 2010-10-10 15:37 ` [PATCH 1/3] Input: synaptics - add multitouch support Chris Bagwell 2010-10-10 15:41 ` Chris Bagwell 2010-10-08 16:37 ` [PATCH 0/3] Input: synaptics - multitouch and multifinger support Takashi Iwai 2010-10-08 16:38 ` Takashi Iwai 2010-10-08 17:48 ` Takashi Iwai 2010-10-08 17:15 ` Chase Douglas 2010-10-08 17:46 ` Takashi Iwai 2010-10-08 18:04 ` Dmitry Torokhov 2010-10-08 19:31 ` Takashi Iwai 2010-10-10 21:04 ` Dmitry Torokhov 2010-10-11 7:35 ` Takashi Iwai 2010-10-11 7:48 ` Henrik Rydberg 2010-10-11 7:59 ` Takashi Iwai 2010-10-11 13:41 ` Chris Bagwell 2010-10-11 14:01 ` Takashi Iwai 2010-10-11 14:24 ` Henrik Rydberg 2010-10-11 14:49 ` Takashi Iwai 2010-10-11 15:31 ` Henrik Rydberg 2010-10-11 15:58 ` Takashi Iwai 2010-10-10 7:49 ` Henrik Rydberg 2010-10-10 20:59 ` Dmitry Torokhov 2010-10-11 7:28 ` Takashi Iwai 2010-10-11 7:40 ` Henrik Rydberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).