Re: [PATCH 2/2] Input: sentelic: Absolute mode and multitouch support for Cx+ hardware.

On Sat, 28 Jan 2012, Dmitry Torokhov wrote:

Hi Oskari,

On Wed, Jan 25, 2012 at 01:22:54AM +0200, Oskari Saarenmaa wrote:

quoted

- Implement absolute position mode and multitouch support for Cx+ revision
  hardware based on latest documentation provided by Sentelic.
- Supports two-finger horizontal and vertical scrolling, edge scrolling and
  clicks on the clickpad.  Hardware click-on-tap is disabled.

The patch was developed and tested on an ASUS Zenbook UX21E.

Signed-off-by: Oskari Saarenmaa <redacted>

Adding Tai-hwa Liang so he has a chance to comment. Also some comments
below...

  Sorry for the late response. Just returned from Chinese lunar New Year
holiday...

quoted

---
 drivers/input/mouse/sentelic.c |  149 +++++++++++++++++++++++++++++++++++++---
 drivers/input/mouse/sentelic.h |    8 ++
 2 files changed, 148 insertions(+), 9 deletions(-)

diff --git a/drivers/input/mouse/sentelic.c b/drivers/input/mouse/sentelic.c
index 87c910d..e851cf5 100644
--- a/drivers/input/mouse/sentelic.c
+++ b/drivers/input/mouse/sentelic.c

@@ -21,6 +21,7 @@

 #include <linux/module.h>
 #include <linux/input.h>
+#include <linux/input/mt.h>
 #include <linux/ctype.h>
 #include <linux/libps2.h>
 #include <linux/serio.h>

@@ -37,7 +38,7 @@
 #define	FSP_CMD_TIMEOUT2	30

 /** Driver version. */
-static const char fsp_drv_ver[] = "1.0.0-K";
+static const char fsp_drv_ver[] = "1.0.0-K-OS1";

 /*
  * Make sure that the value being sent to FSP will not conflict with

@@ -671,8 +672,11 @@ static psmouse_ret_t fsp_process_byte(struct psmouse *psmouse)
 	struct input_dev *dev = psmouse->dev;
 	struct fsp_data *ad = psmouse->private;
 	unsigned char *packet = psmouse->packet;
-	unsigned char button_status = 0, lscroll = 0, rscroll = 0;
+	unsigned short abs_x, abs_y, fingers = 0;
+	unsigned short vscroll = 0, hscroll = 0, lscroll = 0, rscroll = 0;
+	unsigned short l_btn, r_btn, m_btn;
 	int rel_x, rel_y;
+	static bool lifted;

 	if (psmouse->pktcnt < 4)
 		return PSMOUSE_GOOD_DATA;

@@ -684,9 +688,97 @@ static psmouse_ret_t fsp_process_byte(struct psmouse *psmouse)
 	fsp_packet_debug(psmouse, packet);

 	switch (psmouse->packet[0] >> FSP_PKT_TYPE_SHIFT) {
+	case FSP_PKT_TYPE_NOTIFY:
+		/* Notify packets are sent with Cx and newer
+		 * touchpads if register 0x90 bit 1 is set.
+		 */
+		switch (packet[2]) {
+		case 0x86:
+			vscroll =  1;
+			break;
+		case 0x82:
+			vscroll = -1;
+			break;
+		case 0x84:
+			hscroll =  1;
+			break;
+		case 0x80:

  Based on Oskari's version, the attached patch also turns these magic numbers into pre-defined constants(patch against Dmitry's master branch).
Perhaps you guys could have some time to take a look at it?

  Note that I only tested the compilation part as I don't have the real hardware due to holiday shutdown. Will need to test these after getting
back to the office.

quoted

+			hscroll = -1;
+			break;
+		}
+		input_report_rel(dev, REL_WHEEL, vscroll);
+		input_report_rel(dev, REL_HWHEEL, hscroll);
+		break;

Do we need to support the non-absolute mode if we can support absolute
mode for the device? Users expect to have true multi-touch support
nowadays, I think we should keep the driver as simple as possible and
support only absolute mode for MT devices.

  This is true for newer hardware; however, it would be better to stick
with relative mode for older hardware(ex: FSP found on OLPC) since the
relative coordination outputs are much better on them when compared with
abs. outputs.

quoted

+
 	case FSP_PKT_TYPE_ABS:
-		dev_warn(&psmouse->ps2dev.serio->dev,
-			 "Unexpected absolute mode packet, ignored.\n");
+		/* Absolute packets are sent with version Cx and newer
+		 * touchpads if register 0x90 bit 0 is set.
+		 */
+		abs_x = (packet[1] << 2) | ((packet[3] >> 2) & 0x03);
+		abs_y = (packet[2] << 2) | (packet[3] & 0x03);
+
+		l_btn = packet[0] & BIT(0);
+		r_btn = packet[0] & BIT(1);
+		m_btn = packet[0] & BIT(2);
+
+		if (packet[1] || packet[2] || packet[3]) {
+			/* at least one finger is down */
+			fingers++;
+			lifted = false;
+		} else {
+			if (lifted == false) {
+				lifted = true;
+				return PSMOUSE_FULL_PACKET;
+			}
+		}
+
+		if ((packet[0] & (BIT(4)|BIT(5))) == 0 && l_btn) {
+			/* on pad click, let other components handle this.
+			 * NOTE: do not filter out on-pad clicks when
+			 * we're in multitouch mode, BIT(5), they are real
+			 * clickpad-clicks, not just single finger taps.
+			 */
+			l_btn = 0;

Hmm, so this is a clickpad device? We need to set clickpad property
then.

  I presume you're talking about something like SYN_CAP_CLICKPAD() in
synaptics.c?  Given that retrieving capability information would incur more register reading overhead, I'm wondering about if there's any
way to read PnP ID in psmouse.c such that sentelic.c can take advantage
of these vendor specific tags to identify the hardware capability?

quoted

+		}
+
+		if (packet[0] & BIT(5)) {
+			/* multitouch mode: two fingers down */
+			fingers++;
+			if ((packet[0] & BIT(4)) == 0 && l_btn && r_btn) {
+				/* middle-click in multitouch mode */
+				l_btn = 0;
+				r_btn = 0;
+				m_btn = 1;
+			}

The middle button emulation should be handled in userspace. Let
synaptics X driver deal with it.

  Agreed.

quoted

+			if (packet[0] & BIT(2)) {
+				/* right finger down, ignore the event */

Needs better comment.

quoted

+				return PSMOUSE_FULL_PACKET;
+			}
+		}
+
+		input_report_key(dev, BTN_TOUCH, fingers > 0);
+		input_report_abs(dev, ABS_X, abs_x);
+		input_report_abs(dev, ABS_Y, abs_y);
+
+		input_mt_slot(dev, 0);
+		input_mt_report_slot_state(dev, MT_TOOL_FINGER, fingers >= 1);
+		if (fingers >= 1) {
+			input_report_abs(dev, ABS_MT_POSITION_X, abs_x);
+			input_report_abs(dev, ABS_MT_POSITION_Y, abs_y);
+		}
+
+		input_mt_slot(dev, 1);
+		input_mt_report_slot_state(dev, MT_TOOL_FINGER, fingers >= 2);
+		if (fingers >= 2) {
+			input_report_abs(dev, ABS_MT_POSITION_X, abs_x);
+			input_report_abs(dev, ABS_MT_POSITION_Y, abs_y);
+		}
+
+		input_report_key(dev, BTN_TOOL_FINGER, fingers == 1);
+		input_report_key(dev, BTN_TOOL_DOUBLETAP, fingers == 2);
+		input_report_key(dev, BTN_LEFT, l_btn);
+		input_report_key(dev, BTN_MIDDLE, m_btn);
+		input_report_key(dev, BTN_RIGHT, r_btn);
 		break;

 	case FSP_PKT_TYPE_NORMAL_OPC:

@@ -699,6 +791,7 @@ static psmouse_ret_t fsp_process_byte(struct psmouse *psmouse)
 		/* normal packet */
 		/* special packet data translation from on-pad packets */
 		if (packet[3] != 0) {
+			unsigned char button_status = 0;
 			if (packet[3] & BIT(0))
 				button_status |= 0x01;	/* wheel down */
 			if (packet[3] & BIT(1))

@@ -776,6 +869,7 @@ static int fsp_activate_protocol(struct psmouse *psmouse)
 	val &= ~(FSP_BIT_EN_MSID7 | FSP_BIT_EN_MSID8 | FSP_BIT_EN_AUTO_MSID8);
 	/* Ensure we are not in absolute mode */
 	val &= ~FSP_BIT_EN_PKT_G0;
+
 	if (pad->buttons == 0x06) {
 		/* Left/Middle/Right & Scroll Up/Down/Right/Left */
 		val |= FSP_BIT_EN_MSID6;

@@ -799,6 +893,19 @@ static int fsp_activate_protocol(struct psmouse *psmouse)
 	fsp_onpad_vscr(psmouse, true);
 	fsp_onpad_hscr(psmouse, true);

+	/* Enable absolute positioning, two finger mode and continuous output
+	 * on Cx and newer pads (version ID 0xE0+)
+	 */
+	if (pad->ver >= 0xE0) {
+		val = FSP_CX_ABSOLUTE_MODE |
+			FSP_CX_2FINGERS_OUTPUT |
+			FSP_CX_CONTINUOUS_MODE;
+		if (fsp_reg_write(psmouse, FSP_REG_SWREG1, val)) {
+			dev_warn(&psmouse->ps2dev.serio->dev,
+				 "Failed to enable multitouch settings.\n");
+		}
+	}
+
 	return 0;
 }

@@ -854,6 +961,7 @@ static int fsp_reconnect(struct psmouse *psmouse)

 int fsp_init(struct psmouse *psmouse)
 {
+	struct input_dev *dev = psmouse->dev;
 	struct fsp_data *priv;
 	int ver, rev, buttons;
 	int error;

@@ -880,11 +988,34 @@ int fsp_init(struct psmouse *psmouse)
 	priv->flags |= FSPDRV_FLAG_EN_OPC;

 	/* Set up various supported input event bits */
-	__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
-	__set_bit(BTN_BACK, psmouse->dev->keybit);
-	__set_bit(BTN_FORWARD, psmouse->dev->keybit);
-	__set_bit(REL_WHEEL, psmouse->dev->relbit);
-	__set_bit(REL_HWHEEL, psmouse->dev->relbit);
+	__set_bit(BTN_MIDDLE, dev->keybit);
+	__set_bit(BTN_BACK, dev->keybit);
+	__set_bit(BTN_FORWARD, dev->keybit);
+	__set_bit(REL_WHEEL, dev->relbit);
+	__set_bit(REL_HWHEEL, dev->relbit);
+
+	/* Set up multitouch mode on Cx+ version hardware (reg value 0xE0+)
+	 * for example ASUS Zenbook UX21E has Sentelic touchpad version 0xE3
+	 */
+	if (ver >= 0xE0) {
+		/* NOTE: maximum values are not documented anywhere and are
+		 * not available in any register, these are the maximum
+		 * values reported by a Zenbook.
+		 */
+		int abs_x = 965, abs_y = 705;
+
+		__set_bit(EV_ABS, dev->evbit);
+		__clear_bit(EV_REL, dev->evbit);
+		__set_bit(BTN_TOUCH, dev->keybit);
+		__set_bit(BTN_TOOL_FINGER, dev->keybit);
+		__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+
+		input_set_abs_params(dev, ABS_X, 0, abs_x, 0, 0);
+		input_set_abs_params(dev, ABS_Y, 0, abs_y, 0, 0);
+		input_mt_init_slots(dev, 2);
+		input_set_abs_params(dev, ABS_MT_POSITION_X, 0, abs_x, 0, 0);
+		input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, abs_y, 0, 0);
+	}

 	psmouse->protocol_handler = fsp_process_byte;
 	psmouse->disconnect = fsp_disconnect;

diff --git a/drivers/input/mouse/sentelic.h b/drivers/input/mouse/sentelic.h
index 2e4af24..c72ca5e 100644
--- a/drivers/input/mouse/sentelic.h
+++ b/drivers/input/mouse/sentelic.h

@@ -41,6 +41,7 @@
 #define	FSP_REG_OPTZ_YLO	0x36
 #define	FSP_REG_OPTZ_YHI	0x37
 #define	FSP_REG_SYSCTL5		0x40
+#define	FSP_REG_SWREG1		0x90
 #define	FSP_BIT_90_DEGREE	BIT(0)
 #define	FSP_BIT_EN_MSID6	BIT(1)
 #define	FSP_BIT_EN_MSID7	BIT(2)

@@ -64,6 +65,13 @@
 #define	FSP_PKT_TYPE_NORMAL_OPC	(0x03)
 #define	FSP_PKT_TYPE_SHIFT	(6)

+/* swreg1 values, supported in Cx hardware */
+#define FSP_CX_ABSOLUTE_MODE	BIT(0)
+#define FSP_CX_GESTURE_OUTPUT	BIT(1)
+#define FSP_CX_2FINGERS_OUTPUT	BIT(2)
+#define FSP_CX_FINGER_UP_OUTPUT	BIT(3)
+#define FSP_CX_CONTINUOUS_MODE	BIT(4)
+
 #ifdef __KERNEL__

 struct fsp_data {
--

1.7.7.6

Thanks.

-- 
Sincerely yours,

Tai-hwa Liang