diff mbox

Fix sentelic multi-touch driver

Message ID 1206291029033.19930@cnaqrzbavhz.fragryvp.pbz (mailing list archive)
State New, archived
Headers show

Commit Message

Tai-hwa Liang June 29, 2012, 3:03 a.m. UTC
On Thu, 28 Jun 2012, Olivier Goffart wrote:
[...]
> - Touch the touchpad with one finger and move it: this moves the cursor
> - Place a second finger: this allows you to scoll
> - Release a finger: you should now be able to move the cursor again.  This is
> the part that does not work in one of the patch

   Sorry, I didn't realise that one of my patch submitted a few months ago
(namely, http://www.spinics.net/lists/linux-input/msg19443.html) isn't
in the master tree, which is different from my local working copy.

   With the correct flag(FSP_BIT_SWC1_EN_GID) being set, the hardware
clears MFMC bit during 2 -> 1 finger transition.  The attached patch is
an aggregated version against Dmitry's master branch, which supposes to fix 
the testing case listed above.

Thanks,

Tai-hwa Liang

Comments

Olivier Goffart June 29, 2012, 7:52 a.m. UTC | #1
On Friday 29 June 2012 11:03:12 Tai-hwa Liang wrote:
> On Thu, 28 Jun 2012, Olivier Goffart wrote:
> [...]
> 
> > - Touch the touchpad with one finger and move it: this moves the cursor
> > - Place a second finger: this allows you to scoll
> > - Release a finger: you should now be able to move the cursor again.  This
> > is the part that does not work in one of the patch
> 
>    Sorry, I didn't realise that one of my patch submitted a few months ago
> (namely, http://www.spinics.net/lists/linux-input/msg19443.html) isn't
> in the master tree, which is different from my local working copy.
> 
>    With the correct flag(FSP_BIT_SWC1_EN_GID) being set, the hardware
> clears MFMC bit during 2 -> 1 finger transition.  The attached patch is
> an aggregated version against Dmitry's master branch, which supposes to fix
> the testing case listed above.
> 
> Thanks,
> 
> Tai-hwa Liang

I am testing against the stable 3.4.y branch.
With that patch, scrolling is very slow, and it emits middle click sometimes 
while I want to scroll fast. 

Looking at the debug output, i see a lot of NOTIFY packets, for the same  
number of packets/seconds (77). So then there is 5 times less multi touch 
events. Maybe that's why.

Regards
diff mbox

Patch

diff --git a/drivers/input/mouse/sentelic.c b/drivers/input/mouse/sentelic.c
index 3f5649f..c8a20c0 100644
--- a/drivers/input/mouse/sentelic.c
+++ b/drivers/input/mouse/sentelic.c
@@ -706,8 +706,9 @@  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, fgrs = 0;
+	unsigned char button_status = 0;
+	unsigned short abs_x, abs_y, fgrs;
+	unsigned short vscroll = 0, hscroll = 0, lscroll = 0, rscroll = 0;
 	int rel_x, rel_y;
 
 	if (psmouse->pktcnt < 4)
@@ -720,65 +721,99 @@  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:
+		if (packet[1] != FSP_NOTIFY_MSG_GID)
+			break;	/* unsupported message types */
+
+		switch (packet[2]) {
+		case FSP_GID_SP_UP:
+			vscroll =  1;
+			break;
+		case FSP_GID_SP_DOWN:
+			vscroll = -1;
+			break;
+		case FSP_GID_SP_LEFT:
+			hscroll =  1;
+			break;
+		case FSP_GID_SP_RIGHT:
+			hscroll = -1;
+			break;
+		default:
+			dev_dbg(&psmouse->ps2dev.serio->dev,
+				"GID 0x%x\n", packet[2]);
+			break;
+		}
+		input_report_rel(dev, REL_WHEEL, vscroll);
+		input_report_rel(dev, REL_HWHEEL, hscroll);
+		input_event(dev, EV_MSC, MSC_GESTURE, packet[2]);
+		break;
+
 	case FSP_PKT_TYPE_ABS:
+		if ((packet[0] == 0x48 || packet[0] == 0x49) &&
+		    packet[1] == 0 && packet[2] == 0) {
+			/*
+			 * filtering out erratic movement which will cause
+			 * unexpected cursor jumping to the top-left corner
+			 */
+			packet[3] &= 0xf0;
+		}
+		if (!IS_MFMC(packet[0]) && (packet[0] & (FSP_PB0_LBTN |
+		    FSP_PB0_PHY_BTN)) == FSP_PB0_LBTN) {
+			/*
+			 * On-pad click in SFAC mode should be handled
+			 * by userspace.  On-pad clicks in MFMC mode
+			 * are real clickpad clicks, and not ignored.
+			 */
+			packet[0] &= ~FSP_PB0_LBTN;
+		}
 		abs_x = GET_ABS_X(packet);
 		abs_y = GET_ABS_Y(packet);
 
-		if (packet[0] & FSP_PB0_MFMC) {
+		if (abs_x == 0 || abs_y == 0) {
 			/*
-			 * MFMC packet: assume that there are two fingers on
-			 * pad
+			 * workaround for buggy firmware which sends zero
+			 * coordinates even if there is finger on pad
 			 */
-			fgrs = 2;
-
-			/* MFMC packet */
-			if (packet[0] & FSP_PB0_MFMC_FGR2) {
-				/* 2nd finger */
-				if (ad->last_mt_fgr == 2) {
-					/*
-					 * workaround for buggy firmware
-					 * which doesn't clear MFMC bit if
-					 * the 1st finger is up
-					 */
-					fgrs = 1;
-					fsp_set_slot(dev, 0, false, 0, 0);
-				}
-				ad->last_mt_fgr = 2;
-
-				fsp_set_slot(dev, 1, fgrs == 2, abs_x, abs_y);
+			if (!IS_MFMC(packet[0])) {
+				/* don't report bad movement */
+				fgrs = 0;
 			} else {
-				/* 1st finger */
-				if (ad->last_mt_fgr == 1) {
-					/*
-					 * workaround for buggy firmware
-					 * which doesn't clear MFMC bit if
-					 * the 2nd finger is up
-					 */
-					fgrs = 1;
-					fsp_set_slot(dev, 1, false, 0, 0);
-				}
-				ad->last_mt_fgr = 1;
-				fsp_set_slot(dev, 0, fgrs != 0, abs_x, abs_y);
-			}
-		} else {
-			/* SFAC packet */
-			if ((packet[0] & (FSP_PB0_LBTN|FSP_PB0_PHY_BTN)) ==
-				FSP_PB0_LBTN) {
-				/* On-pad click in SFAC mode should be handled
-				 * by userspace.  On-pad clicks in MFMC mode
-				 * are real clickpad clicks, and not ignored.
+				/*
+				 * for zero 2nd MFMC packet, use saved
+				 * coordinate instead
 				 */
-				packet[0] &= ~FSP_PB0_LBTN;
+				abs_x = ad->mfmc_x1;
+				abs_y = ad->mfmc_y1;
+				fgrs = 2;
 			}
-
-			/* no multi-finger information */
-			ad->last_mt_fgr = 0;
-
-			if (abs_x != 0 && abs_y != 0)
+		} else {
+			/* finger(s) down */
+			if (!IS_MFMC(packet[0])) {
+				/* SFAC, MFMC CPB packet */
 				fgrs = 1;
 
-			fsp_set_slot(dev, 0, fgrs > 0, abs_x, abs_y);
-			fsp_set_slot(dev, 1, false, 0, 0);
+				fsp_set_slot(dev, 0, true, abs_x, abs_y);
+				fsp_set_slot(dev, 1, false, 0, 0);
+			} else if (IS_MFMC_FGR1(packet[0])) {
+				/* MFMC 1st finger */
+				fgrs = 2;
+
+				/*
+				 * accumulate the coordaintes and proceed to
+				 * the next run
+				 */
+				ad->mfmc_x1 = abs_x;
+				ad->mfmc_y1 = abs_y;
+			} else {
+				/* MFMC 2nd finger */
+				fgrs = 2;
+
+				/* report both fingers' coordinate */
+				fsp_set_slot(dev, 1, true, abs_x, abs_y);
+				abs_x = ad->mfmc_x1;
+				abs_y = ad->mfmc_y1;
+				fsp_set_slot(dev, 0, true, abs_x, abs_y);
+			}
 		}
 		if (fgrs > 0) {
 			input_report_abs(dev, ABS_X, abs_x);
@@ -916,7 +951,10 @@  static int fsp_activate_protocol(struct psmouse *psmouse)
 				  FSP_BIT_SWC1_EN_ABS_1F |
 				  FSP_BIT_SWC1_EN_ABS_2F |
 				  FSP_BIT_SWC1_EN_FUP_OUT |
-				  FSP_BIT_SWC1_EN_ABS_CON)) {
+				  FSP_BIT_SWC1_EN_ABS_CON |
+				  FSP_BIT_SWC1_EN_GID |
+				  FSP_BIT_SWC1_GST_GRP0 |
+				  FSP_BIT_SWC1_GST_GRP1)) {
 			psmouse_err(psmouse,
 				    "Unable to enable absolute coordinates output.\n");
 			return -EIO;
@@ -963,6 +1001,9 @@  static int fsp_set_input_params(struct psmouse *psmouse)
 		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);
+
+		/* device generated gesture ID events */
+		input_set_capability(dev, EV_MSC, MSC_GESTURE);
 	}
 
 	return 0;
diff --git a/drivers/input/mouse/sentelic.h b/drivers/input/mouse/sentelic.h
index aa697ec..8a5cc59 100644
--- a/drivers/input/mouse/sentelic.h
+++ b/drivers/input/mouse/sentelic.h
@@ -90,6 +90,112 @@ 
 #define	FSP_PB0_MUST_SET	BIT(3)
 #define	FSP_PB0_PHY_BTN		BIT(4)
 #define	FSP_PB0_MFMC		BIT(5)
+/* packet type. See FSP_PKT_TYPE_XXX */
+#define	FSP_PB0_TYPE		(BIT(7) | BIT(6))
+#define	FSP_PB0_TYPE_REL	(0)
+#define	FSP_PB0_TYPE_ABS	BIT(6)
+#define	FSP_PB0_TYPE_NOTIFY	BIT(7)
+#define	FSP_PB0_TYPE_CUSTOM	(BIT(7) | BIT(6))
+
+/*
+ * physical clickpad button = MFMC packet without physical button.
+ */
+#define	IS_PHY_CPB(_pb0_)	\
+	(((_pb0_) & (FSP_PB0_TYPE | FSP_PB0_MFMC | FSP_PB0_PHY_BTN)) == \
+	(FSP_PB0_TYPE_ABS | FSP_PB0_MFMC))
+/*
+ * single-finger absolute coordinate
+ */
+#define	IS_SFAC(_pb0_)		\
+	(((_pb0_) & (FSP_PB0_TYPE | FSP_PB0_MFMC)) == FSP_PB0_TYPE_ABS)
+/*
+ * multi-finger multi-coordinate (physical clickpad button is excluded)
+ */
+#define	IS_MFMC(_pb0_)		\
+	(((_pb0_) & (FSP_PB0_TYPE | FSP_PB0_MFMC | FSP_PB0_PHY_BTN)) == \
+	(FSP_PB0_TYPE_ABS | FSP_PB0_MFMC | FSP_PB0_PHY_BTN))
+#define	IS_MFMC_FGR1(_pb0_)	\
+	(((_pb0_) & (FSP_PB0_TYPE | FSP_PB0_MFMC | FSP_PB0_PHY_BTN | \
+		FSP_PB0_MFMC_FGR2)) == (FSP_PB0_TYPE_ABS | \
+		FSP_PB0_MFMC | FSP_PB0_PHY_BTN))
+#define	IS_MFMC_FGR2(_pb0_)	\
+	(((_pb0_) & (FSP_PB0_TYPE | FSP_PB0_MFMC | FSP_PB0_PHY_BTN | \
+		FSP_PB0_MFMC_FGR2)) == (FSP_PB0_TYPE_ABS | \
+		FSP_PB0_MFMC | FSP_PB0_PHY_BTN | FSP_PB0_MFMC_FGR2))
+
+
+/* notification message types */
+#define	FSP_NOTIFY_MSG_GID	(0xba)
+#define	FSP_NOTIFY_MSG_HX_GST	(0xc0)
+
+/* gesture IDs */
+/** GID for 2F Straight Up             */
+#define	FSP_GID_SP1		(0x86)
+#define	FSP_GID_SP_UP		FSP_GID_SP1
+
+/** GID for 2F Straight Down           */
+#define	FSP_GID_SP5		(0x82)
+#define	FSP_GID_SP_DOWN		FSP_GID_SP5
+
+/** GID for 2F Straight Right          */
+#define	FSP_GID_SP2		(0x80)
+#define	FSP_GID_SP_RIGHT	FSP_GID_SP2
+
+/** GID for 2F Straight Left           */
+#define	FSP_GID_SP6		(0x84)
+#define	FSP_GID_SP_LEFT		FSP_GID_SP6
+
+/** GID for 2F Zoom In                 */
+#define	FSP_GID_SC6		(0x8F)
+#define	FSP_GID_SC_ZOOM_IN	FSP_GID_SC6
+
+/** GID for 2F Zoom Out                */
+#define	FSP_GID_SC3		(0x8B)
+#define	FSP_GID_SC_ZOOM_OUT	FSP_GID_SC3
+
+/** GID for 2F Curve CW                */
+#define	FSP_GID_DC1		(0xC4)
+#define	FSP_GID_DC_ROT_CW	FSP_GID_DC1
+
+/** GID for 2F Curve CCW               */
+#define	FSP_GID_DC2		(0xC0)
+#define	FSP_GID_DC_ROT_CCW	FSP_GID_DC2
+
+/** GID for 3F Straight Up             */
+#define	FSP_GID_TS4		(0x2E)
+#define	FSP_GID_TS_UP		FSP_GID_TS4
+
+/** GID for 3F Straight Down           */
+#define	FSP_GID_TS2		(0x2A)
+#define	FSP_GID_TS_DOWN		FSP_GID_TS2
+
+/** GID for 3F Straight Right          */
+#define	FSP_GID_TS1		(0x28)
+#define	FSP_GID_TS_RIGHT	FSP_GID_TS1
+
+/** GID for 3F Straight Left           */
+#define	FSP_GID_TS3		(0x2C)
+#define	FSP_GID_TS_LEFT		FSP_GID_TS3
+
+/** GID for 2F Click                   */
+#define	FSP_GID_DF1		(0x11)
+#define	FSP_GID_DF_CLICK	FSP_GID_DF1
+
+/** GID for 2F Double Click            */
+#define	FSP_GID_DF2		(0x12)
+#define	FSP_GID_DF_DBLCLICK	FSP_GID_DF2
+
+/** GID for 3F Click                   */
+#define	FSP_GID_TF1		(0x19)
+#define	FSP_GID_TF_CLICK	FSP_GID_TF1
+
+/** GID for 3F Double Click            */
+#define	FSP_GID_TF2		(0x1A)
+#define	FSP_GID_TF_DBLCLICK	FSP_GID_TF2
+
+/** GID for Palm                       */
+#define	FSP_GID_PM1		(0x38)
+#define	FSP_GID_PALM		FSP_GID_PM1
 
 /* hardware revisions */
 #define	FSP_VER_STL3888_A4	(0xC1)
@@ -116,7 +222,8 @@  struct fsp_data {
 
 	unsigned char	last_reg;	/* Last register we requested read from */
 	unsigned char	last_val;
-	unsigned int	last_mt_fgr;	/* Last seen finger(multitouch) */
+	unsigned short	mfmc_x1;	/* X coordinate for the 1st finger */
+	unsigned short	mfmc_y1;	/* Y coordinate for the 1st finger */
 };
 
 #ifdef CONFIG_MOUSE_PS2_SENTELIC