some questions about Synaptics Clickpad driver patch

some questions about Synaptics Clickpad driver patch

[Jian-feng Ding]

Hi IWai,

  I am from MeeGo team of Intel, and being to enable the clickpad in HP Mini210 netbook by applying your great patch.
  I have tried the old copy from: https://patchwork.kernel.org/patch/67335/ , it just worked fine for both
  left/mid/right clicks. And today, I noticed your patch has been integrated to dtor's input subsys git tree as the
  commit: 5f57d67da87332a9a1ba8fa7a33bf0680e1c76e7
    http://git.kernel.org/?p=linux/kernel/git/dtor/input.git;a=commit;h=5f57d67da87332a9a1ba8fa7a33bf0680e1c76e7

  It seems the latest copy, right?
  I noticed you have removed the code for left/mid/right clicks diffenrentiation, and only export left clicks events
  to user level. In the commit comment, you mentioned:

    The kernel driver morphs to the left button. The real handling of Clickpad is done rather in X driver side.

  Did you mean the left/mid/right clicks differentiation should be done in x.org input drv? If so, would you give
  me some clues for it?

  Thanks!

- jf.ding
 

Re: some questions about Synaptics Clickpad driver patch

[Takashi Iwai]

At Wed, 21 Apr 2010 16:40:58 +0800,
Jian-feng Ding wrote:
> 
> Hi IWai,
> 
>   I am from MeeGo team of Intel, and being to enable the clickpad in HP Mini210 netbook by applying your great patch.
>   I have tried the old copy from: https://patchwork.kernel.org/patch/67335/ , it just worked fine for both
>   left/mid/right clicks. And today, I noticed your patch has been integrated to dtor's input subsys git tree as the
>   commit: 5f57d67da87332a9a1ba8fa7a33bf0680e1c76e7
>     http://git.kernel.org/?p=linux/kernel/git/dtor/input.git;a=commit;h=5f57d67da87332a9a1ba8fa7a33bf0680e1c76e7
> 
>   It seems the latest copy, right?
>   I noticed you have removed the code for left/mid/right clicks diffenrentiation, and only export left clicks events
>   to user level. In the commit comment, you mentioned:
> 
>     The kernel driver morphs to the left button. The real handling of Clickpad is done rather in X driver side.
> 
>   Did you mean the left/mid/right clicks differentiation should be done in x.org input drv? If so, would you give
>   me some clues for it?

Exactly.  The button handling is done in X driver, rather in the kernel.
In this way, we can handle the events more flexibly.

FYI, the latest version is below, which I sent now to X devel list.
It's for the latest xorg git.


thanks,

Takashi

---
>From 3cef0f10914b23702d9233b3220933ab029c0a18 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Tue, 13 Apr 2010 17:25:23 +0200
Subject: [PATCH] Add Clickpad support

This patch adds the support for Synaptics Clickpad devices.
It requires the change in Linux kernel synaptics input driver, found in
	https://patchwork.kernel.org/patch/92435/
The kernel patch will be included in 2.6.34-rc6 or later.

When the kernel driver sets only the left-button bit evbit, Clickpad
mode is activated.  In this mode, the bottom touch area is used as
button emulations.  Clicking at the bottom-left, bottom-center and
bottom-right zone corresponds to a left, center and right click.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 src/eventcomm.c    |    6 ++++
 src/synaptics.c    |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/synapticsstr.h |    2 +
 3 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/src/eventcomm.c b/src/eventcomm.c
index d00d810..10d183d 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -252,6 +252,12 @@ event_query_axis_ranges(LocalDevicePtr local)
 	if ((priv->has_triple = (TEST_BIT(BTN_TOOL_TRIPLETAP, keybits) != 0)))
 	   strcat(buf, " triple");
 	xf86Msg(X_INFO, "%s: buttons:%s\n", local->name, buf);
+
+	/* clickpad device reports only the single left button mask */
+	if (priv->has_left && !priv->has_right && !priv->has_middle) {
+	    priv->is_clickpad = TRUE;
+	    xf86Msg(X_INFO, "%s: is Clickpad device\n", local->name);
+	}
     }
 }
 
diff --git a/src/synaptics.c b/src/synaptics.c
index 091dbe1..951b3fa 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -457,6 +457,18 @@ static void set_default_parameters(LocalDevicePtr local)
         vertResolution = priv->resy;
     }
 
+    /* Clickpad mode -- bottom area is used as buttons */
+    if (priv->is_clickpad) {
+	int button_bottom;
+	/* Clickpad devices usually the button area at the bottom, and
+	 * its size seems ca. 20% of the touchpad height no matter how
+	 * large the pad is.
+	 */
+	button_bottom = priv->maxy - (abs(priv->maxy - priv->miny) * 20) / 100;
+	if (button_bottom < b && button_bottom >= t)
+	    b = button_bottom;
+    }
+
     /* set the parameters */
     pars->left_edge = xf86SetIntOption(opts, "LeftEdge", l);
     pars->right_edge = xf86SetIntOption(opts, "RightEdge", r);
@@ -2052,6 +2064,59 @@ HandleClickWithFingers(SynapticsParameters *para, struct SynapticsHwState *hw)
     }
 }
 
+/* clickpad event handling */
+static void
+HandleClickpad(LocalDevicePtr local, struct SynapticsHwState *hw, edge_type edge)
+{
+    SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
+    SynapticsParameters *para = &priv->synpara;
+
+    if (edge & BOTTOM_EDGE) {
+	/* button area */
+	int width = priv->maxx - priv->minx;
+	int left_button_x, right_button_x;
+
+	/* left and right clickpad button ranges;
+	 * the gap between them is interpreted as a middle-button click
+	 */
+	left_button_x = width * 2/ 5 + priv->minx;
+	right_button_x = width * 3 / 5 + priv->minx;
+
+	/* clickpad reports only one button, and we need
+	 * to fake left/right buttons depending on the touch position
+	 */
+	if (hw->left) { /* clicked? */
+	    hw->left = 0;
+	    if (hw->x < left_button_x)
+		hw->left = 1;
+	    else if (hw->x > right_button_x)
+		hw->right = 1;
+	    else
+		hw->middle = 1;
+	}
+
+	/* Don't move pointer position in the button area during clicked,
+	 * except for horiz/vert scrolling is enabled.
+	 *
+	 * The synaptics driver tends to be pretty sensitive.  This hack
+	 * is to avoid that the pointer moves slightly and misses the
+	 * poistion you aimed to click.
+	 *
+	 * Also, when the pointer movement is reported, the dragging
+	 * (with a sort of multi-touching) doesn't work well, too.
+	 */
+	if (hw->left || !(para->scroll_edge_horiz ||
+			  ((edge & RIGHT_EDGE) && para->scroll_edge_vert)))
+	    hw->z = 0; /* don't move pointer */
+
+    } else if (hw->left) {
+	/* dragging */
+	hw->left = priv->prev_hw.left;
+	hw->right = priv->prev_hw.right;
+	hw->middle = priv->prev_hw.middle;
+    }
+    priv->prev_hw = *hw;
+}
 
 /*
  * React on changes in the hardware state. This function is called every time
@@ -2102,6 +2167,12 @@ HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
     if (para->touchpad_off == 1)
 	return delay;
 
+    edge = edge_detection(priv, hw->x, hw->y);
+
+    /* Clickpad handling for button area */
+    if (priv->is_clickpad)
+	HandleClickpad(local, hw, edge);
+
     /* Treat the first two multi buttons as up/down for now. */
     hw->up |= hw->multi[0];
     hw->down |= hw->multi[1];
@@ -2152,7 +2223,6 @@ HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
 	hw->multi[2] = hw->multi[3] = FALSE;
     }
 
-    edge = edge_detection(priv, hw->x, hw->y);
     inside_active_area = is_inside_active_area(priv, hw->x, hw->y);
 
     finger = SynapticsDetectFinger(priv, hw);
diff --git a/src/synapticsstr.h b/src/synapticsstr.h
index bd19c79..05e43d3 100644
--- a/src/synapticsstr.h
+++ b/src/synapticsstr.h
@@ -232,6 +232,8 @@ typedef struct _SynapticsPrivateRec
     Bool has_double;			/* double click detected for this device */
     Bool has_triple;			/* triple click detected for this device */
     Bool has_pressure;			/* device reports pressure */
+    Bool is_clickpad;			/* is Clickpad device (one-button) */
+    struct SynapticsHwState prev_hw;	/* previous h/w state (for clickpad) */
 
     enum TouchpadModel model;          /* The detected model */
 } SynapticsPrivate;
-- 
1.7.0.4

Re: some questions about Synaptics Clickpad driver patch

[Jian-feng Ding]

I just applied your xorg input drv patch in my x.org building, and tried in my HP mini210 netbook.
It works like a charm, and a lot more stable and usable than the old kernel-only solution. Great work:)

After this patch being merged by x.org upstream, I will start to integrate it to MeeGo system.
Thanks!

- jf.ding

On Wed, Apr 21, 2010 at 05:02:48PM +0800, Takashi Iwai wrote:
> At Wed, 21 Apr 2010 16:40:58 +0800,
> Jian-feng Ding wrote:
> > 
> > Hi IWai,
> > 
> >   I am from MeeGo team of Intel, and being to enable the clickpad in HP Mini210 netbook by applying your great patch.
> >   I have tried the old copy from: https://patchwork.kernel.org/patch/67335/ , it just worked fine for both
> >   left/mid/right clicks. And today, I noticed your patch has been integrated to dtor's input subsys git tree as the
> >   commit: 5f57d67da87332a9a1ba8fa7a33bf0680e1c76e7
> >     http://git.kernel.org/?p=linux/kernel/git/dtor/input.git;a=commit;h=5f57d67da87332a9a1ba8fa7a33bf0680e1c76e7
> > 
> >   It seems the latest copy, right?
> >   I noticed you have removed the code for left/mid/right clicks diffenrentiation, and only export left clicks events
> >   to user level. In the commit comment, you mentioned:
> > 
> >     The kernel driver morphs to the left button. The real handling of Clickpad is done rather in X driver side.
> > 
> >   Did you mean the left/mid/right clicks differentiation should be done in x.org input drv? If so, would you give
> >   me some clues for it?
> 
> Exactly.  The button handling is done in X driver, rather in the kernel.
> In this way, we can handle the events more flexibly.
> 
> FYI, the latest version is below, which I sent now to X devel list.
> It's for the latest xorg git.
> 
> 
> thanks,
> 
> Takashi
> 
> ---
> From 3cef0f10914b23702d9233b3220933ab029c0a18 Mon Sep 17 00:00:00 2001
> From: Takashi Iwai <tiwai@suse.de>
> Date: Tue, 13 Apr 2010 17:25:23 +0200
> Subject: [PATCH] Add Clickpad support
> 
> This patch adds the support for Synaptics Clickpad devices.
> It requires the change in Linux kernel synaptics input driver, found in
> 	https://patchwork.kernel.org/patch/92435/
> The kernel patch will be included in 2.6.34-rc6 or later.
> 
> When the kernel driver sets only the left-button bit evbit, Clickpad
> mode is activated.  In this mode, the bottom touch area is used as
> button emulations.  Clicking at the bottom-left, bottom-center and
> bottom-right zone corresponds to a left, center and right click.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  src/eventcomm.c    |    6 ++++
>  src/synaptics.c    |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/synapticsstr.h |    2 +
>  3 files changed, 79 insertions(+), 1 deletions(-)
> 
> diff --git a/src/eventcomm.c b/src/eventcomm.c
> index d00d810..10d183d 100644
> --- a/src/eventcomm.c
> +++ b/src/eventcomm.c
> @@ -252,6 +252,12 @@ event_query_axis_ranges(LocalDevicePtr local)
>  	if ((priv->has_triple = (TEST_BIT(BTN_TOOL_TRIPLETAP, keybits) != 0)))
>  	   strcat(buf, " triple");
>  	xf86Msg(X_INFO, "%s: buttons:%s\n", local->name, buf);
> +
> +	/* clickpad device reports only the single left button mask */
> +	if (priv->has_left && !priv->has_right && !priv->has_middle) {
> +	    priv->is_clickpad = TRUE;
> +	    xf86Msg(X_INFO, "%s: is Clickpad device\n", local->name);
> +	}
>      }
>  }
>  
> diff --git a/src/synaptics.c b/src/synaptics.c
> index 091dbe1..951b3fa 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -457,6 +457,18 @@ static void set_default_parameters(LocalDevicePtr local)
>          vertResolution = priv->resy;
>      }
>  
> +    /* Clickpad mode -- bottom area is used as buttons */
> +    if (priv->is_clickpad) {
> +	int button_bottom;
> +	/* Clickpad devices usually the button area at the bottom, and
> +	 * its size seems ca. 20% of the touchpad height no matter how
> +	 * large the pad is.
> +	 */
> +	button_bottom = priv->maxy - (abs(priv->maxy - priv->miny) * 20) / 100;
> +	if (button_bottom < b && button_bottom >= t)
> +	    b = button_bottom;
> +    }
> +
>      /* set the parameters */
>      pars->left_edge = xf86SetIntOption(opts, "LeftEdge", l);
>      pars->right_edge = xf86SetIntOption(opts, "RightEdge", r);
> @@ -2052,6 +2064,59 @@ HandleClickWithFingers(SynapticsParameters *para, struct SynapticsHwState *hw)
>      }
>  }
>  
> +/* clickpad event handling */
> +static void
> +HandleClickpad(LocalDevicePtr local, struct SynapticsHwState *hw, edge_type edge)
> +{
> +    SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
> +    SynapticsParameters *para = &priv->synpara;
> +
> +    if (edge & BOTTOM_EDGE) {
> +	/* button area */
> +	int width = priv->maxx - priv->minx;
> +	int left_button_x, right_button_x;
> +
> +	/* left and right clickpad button ranges;
> +	 * the gap between them is interpreted as a middle-button click
> +	 */
> +	left_button_x = width * 2/ 5 + priv->minx;
> +	right_button_x = width * 3 / 5 + priv->minx;
> +
> +	/* clickpad reports only one button, and we need
> +	 * to fake left/right buttons depending on the touch position
> +	 */
> +	if (hw->left) { /* clicked? */
> +	    hw->left = 0;
> +	    if (hw->x < left_button_x)
> +		hw->left = 1;
> +	    else if (hw->x > right_button_x)
> +		hw->right = 1;
> +	    else
> +		hw->middle = 1;
> +	}
> +
> +	/* Don't move pointer position in the button area during clicked,
> +	 * except for horiz/vert scrolling is enabled.
> +	 *
> +	 * The synaptics driver tends to be pretty sensitive.  This hack
> +	 * is to avoid that the pointer moves slightly and misses the
> +	 * poistion you aimed to click.
> +	 *
> +	 * Also, when the pointer movement is reported, the dragging
> +	 * (with a sort of multi-touching) doesn't work well, too.
> +	 */
> +	if (hw->left || !(para->scroll_edge_horiz ||
> +			  ((edge & RIGHT_EDGE) && para->scroll_edge_vert)))
> +	    hw->z = 0; /* don't move pointer */
> +
> +    } else if (hw->left) {
> +	/* dragging */
> +	hw->left = priv->prev_hw.left;
> +	hw->right = priv->prev_hw.right;
> +	hw->middle = priv->prev_hw.middle;
> +    }
> +    priv->prev_hw = *hw;
> +}
>  
>  /*
>   * React on changes in the hardware state. This function is called every time
> @@ -2102,6 +2167,12 @@ HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
>      if (para->touchpad_off == 1)
>  	return delay;
>  
> +    edge = edge_detection(priv, hw->x, hw->y);
> +
> +    /* Clickpad handling for button area */
> +    if (priv->is_clickpad)
> +	HandleClickpad(local, hw, edge);
> +
>      /* Treat the first two multi buttons as up/down for now. */
>      hw->up |= hw->multi[0];
>      hw->down |= hw->multi[1];
> @@ -2152,7 +2223,6 @@ HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
>  	hw->multi[2] = hw->multi[3] = FALSE;
>      }
>  
> -    edge = edge_detection(priv, hw->x, hw->y);
>      inside_active_area = is_inside_active_area(priv, hw->x, hw->y);
>  
>      finger = SynapticsDetectFinger(priv, hw);
> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> index bd19c79..05e43d3 100644
> --- a/src/synapticsstr.h
> +++ b/src/synapticsstr.h
> @@ -232,6 +232,8 @@ typedef struct _SynapticsPrivateRec
>      Bool has_double;			/* double click detected for this device */
>      Bool has_triple;			/* triple click detected for this device */
>      Bool has_pressure;			/* device reports pressure */
> +    Bool is_clickpad;			/* is Clickpad device (one-button) */
> +    struct SynapticsHwState prev_hw;	/* previous h/w state (for clickpad) */
>  
>      enum TouchpadModel model;          /* The detected model */
>  } SynapticsPrivate;
> -- 
> 1.7.0.4
>