Re: [PATCH 1/4] modesetting: Fix 32bit breakage

2015-02-16 Thread Takashi Iwai
At Tue, 17 Feb 2015 12:15:04 +0900,
Michel Dänzer wrote:
> 
> On 17.02.2015 01:00, Takashi Iwai wrote:
> > The current modesetting driver fails with cirrus KMS and others on
> > 32bit architecture.  It aborts at mmap() call failure in dumb_bo.c,
> > and it's because the offset is passed as 32bit off_t; this truncates
> > DRM_FILE_PAGE_OFFSET bit and the KMS driver refuses the value.
> > 
> > This is an ad hoc fix just to let 64bit off_t being used for mmap() by
> > adding _FILE_OFFSET_BITS=64.
> > 
> > Signed-off-by: Takashi Iwai 
> > ---
> >  hw/xfree86/drivers/modesetting/dumb_bo.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/xfree86/drivers/modesetting/dumb_bo.c 
> > b/hw/xfree86/drivers/modesetting/dumb_bo.c
> > index 58d420e07568..29948d453a1a 100644
> > --- a/hw/xfree86/drivers/modesetting/dumb_bo.c
> > +++ b/hw/xfree86/drivers/modesetting/dumb_bo.c
> > @@ -25,6 +25,8 @@
> >   *
> >   */
> >  
> > +#define _FILE_OFFSET_BITS  64
> > +
> 
> This is what AC_SYS_LARGEFILE is for in configure.ac. Does this work
> instead?

I guess it would work, too, but this touches all over the code, so I'm
afraid of a possible regression somewhere.  That's why I took a
band-aid fix at this time.


thanks,

Takashi

> 
> 
> diff --git a/include/dix-config.h.in b/include/dix-config.h.in
> index 1aa77a5..b0eb696 100644
> --- a/include/dix-config.h.in
> +++ b/include/dix-config.h.in
> @@ -388,9 +388,15 @@
>  /* Vendor name */
>  #undef XVENDORNAME
> 
> +/* Number of bits in a file offset, on hosts where this is settable. */
> +#undef _FILE_OFFSET_BITS
> +
>  /* Enable GNU and other extensions to the C environment for GLIBC */
>  #undef _GNU_SOURCE
> 
> +/* Define for large files, on AIX-style hosts. */
> +#undef _LARGE_FILES
> +
>  /* Define to empty if `const' does not conform to ANSI C. */
>  #undef const
> 
> 
> 
> -- 
> Earthling Michel Dänzer   |   http://www.amd.com
> Libre software enthusiast | Mesa and X developer
> 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 2/4] modesetting: Fix the error check from DRM_IOCTL_MODE_CURSOR2

2015-02-16 Thread Takashi Iwai
The error value isn't always -EINVAL, e.g. the kernel drm core returns
-ENXIO when the corresponding ops doesn't exist.  Without this fix,
DRM_IOCTL_MODE_CURSOR2 would be dealt as success even if it
shouldn't.

Signed-off-by: Takashi Iwai 
---
 hw/xfree86/drivers/modesetting/drmmode_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 1ea799b3a244..25d16a233103 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -427,7 +427,7 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
 drmModeSetCursor2(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
   handle, ms->cursor_width, ms->cursor_height,
   cursor->bits->xhot, cursor->bits->yhot);
-if (ret == -EINVAL)
+if (ret)
 use_set_cursor2 = FALSE;
 else
 return;
-- 
2.3.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 1/4] modesetting: Fix 32bit breakage

2015-02-16 Thread Takashi Iwai
The current modesetting driver fails with cirrus KMS and others on
32bit architecture.  It aborts at mmap() call failure in dumb_bo.c,
and it's because the offset is passed as 32bit off_t; this truncates
DRM_FILE_PAGE_OFFSET bit and the KMS driver refuses the value.

This is an ad hoc fix just to let 64bit off_t being used for mmap() by
adding _FILE_OFFSET_BITS=64.

Signed-off-by: Takashi Iwai 
---
 hw/xfree86/drivers/modesetting/dumb_bo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/xfree86/drivers/modesetting/dumb_bo.c 
b/hw/xfree86/drivers/modesetting/dumb_bo.c
index 58d420e07568..29948d453a1a 100644
--- a/hw/xfree86/drivers/modesetting/dumb_bo.c
+++ b/hw/xfree86/drivers/modesetting/dumb_bo.c
@@ -25,6 +25,8 @@
  *
  */
 
+#define _FILE_OFFSET_BITS  64
+
 #include "dumb_bo.h"
 
 #include 
-- 
2.3.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 4/4] modesetting: Fix hw cursor check at the first call

2015-02-16 Thread Takashi Iwai
With the previous patch, the modesetting driver can now return whether
the driver supports hw cursor.  However, it alone doesn't suffice,
unfortunately. drmmode_load_cursor_argb_check() is called in the
following chain:

  xf86CursorSetCursor()
-> xf86SetCursor()
   -> xf86DriverLoadCursorARGB()
 -> xf86_load_cursor_argb()
   -> xf86_crtc_load_cursor_argb()
 -> drmmode_load_cursor_argb_check()

*but* at first with drmmode_crtc->cursor_up = FALSE.  Then the
function doesn't actually set the cursor but returns TRUE
unconditionally.  The actual call of drmmode_set_cursor() is done at
first via the show_cursor callback, and there is no check of sw cursor
fallback any longer at this place. Since it's called only once per
cursor setup, so the xserver still thinks as if the hw cursor is
supported.

This patch is an ad hoc fix to correct the behavior somehow: it does
call drmmode_set_cursor() at the very first time even if cursor_up is
FALSE, then quickly hides again.  In that way, whether the hw cursor
is supported is evaluated in the right place at the right time.

Of course, it might be more elegant if we have a more proper mechanism
to fall back to sw cursor at any call path.  But it'd need more
rework, so I leave this workaround as is for now.

Signed-off-by: Takashi Iwai 
---
 hw/xfree86/drivers/modesetting/drmmode_display.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 25e4d367de46..707c336b06a7 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -457,6 +457,7 @@ drmmode_load_cursor_argb_check(xf86CrtcPtr crtc, CARD32 
*image)
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 int i;
 uint32_t *ptr;
+static Bool first_time = TRUE;
 
 /* cursor should be mapped already */
 ptr = (uint32_t *) (drmmode_crtc->cursor_bo->ptr);
@@ -464,8 +465,13 @@ drmmode_load_cursor_argb_check(xf86CrtcPtr crtc, CARD32 
*image)
 for (i = 0; i < ms->cursor_width * ms->cursor_height; i++)
 ptr[i] = image[i];  // cpu_to_le32(image[i]);
 
-if (drmmode_crtc->cursor_up)
-return drmmode_set_cursor(crtc);
+if (drmmode_crtc->cursor_up || first_time) {
+Bool ret = drmmode_set_cursor(crtc);
+if (!drmmode_crtc->cursor_up)
+   drmmode_hide_cursor(crtc);
+first_time = FALSE;
+return ret;
+}
 return TRUE;
 }
 
-- 
2.3.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 0/4] A few modesetting driver fixes

2015-02-16 Thread Takashi Iwai
Hi,

here is a series of fix patches for modesetting driver we've found on
xserver-1.17.1 with QEMU cirrus and other KMS drivers.  Some of them
are ad hoc and might be rewritten in a better way, but it at least
works.

Takashi Iwai (4):
  modesetting: Fix 32bit breakage
  modesetting: Fix the error check from DRM_IOCTL_MODE_CURSOR2
  modesetting: Use load_cursor_argb_check for sw cursor fallback
  modesetting: Fix hw cursor check at the first call

 hw/xfree86/drivers/modesetting/drmmode_display.c | 27 +---
 hw/xfree86/drivers/modesetting/dumb_bo.c |  2 ++
 2 files changed, 21 insertions(+), 8 deletions(-)

-- 
2.3.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 3/4] modesetting: Use load_cursor_argb_check for sw cursor fallback

2015-02-16 Thread Takashi Iwai
The modesetting driver still has an everlasting bug of invisible
cursor on cirrus and other KMS drivers where no hardware cursor is
supported.  This patch is a part of an attempt to address it.

This patch particularly converts the current load_cursor_argb callback
of modesetting driver to load_cursor_argb_check so that it can return
whether the driver handles the hw cursor or falls back to the sw
cursor.

Signed-off-by: Takashi Iwai 
---
 hw/xfree86/drivers/modesetting/drmmode_display.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 25d16a233103..25e4d367de46 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -409,7 +409,7 @@ drmmode_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
 drmModeMoveCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, x, y);
 }
 
-static void
+static Bool
 drmmode_set_cursor(xf86CrtcPtr crtc)
 {
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
@@ -430,7 +430,7 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
 if (ret)
 use_set_cursor2 = FALSE;
 else
-return;
+return TRUE;
 }
 
 ret = drmModeSetCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, 
handle,
@@ -443,11 +443,15 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
 cursor_info->MaxWidth = cursor_info->MaxHeight = 0;
 drmmode_crtc->drmmode->sw_cursor = TRUE;
 /* fallback to swcursor */
+return FALSE;
 }
+return TRUE;
 }
 
-static void
-drmmode_load_cursor_argb(xf86CrtcPtr crtc, CARD32 *image)
+static void drmmode_hide_cursor(xf86CrtcPtr crtc);
+
+static Bool
+drmmode_load_cursor_argb_check(xf86CrtcPtr crtc, CARD32 *image)
 {
 modesettingPtr ms = modesettingPTR(crtc->scrn);
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
@@ -461,7 +465,8 @@ drmmode_load_cursor_argb(xf86CrtcPtr crtc, CARD32 *image)
 ptr[i] = image[i];  // cpu_to_le32(image[i]);
 
 if (drmmode_crtc->cursor_up)
-drmmode_set_cursor(crtc);
+return drmmode_set_cursor(crtc);
+return TRUE;
 }
 
 static void
@@ -665,7 +670,7 @@ static const xf86CrtcFuncsRec drmmode_crtc_funcs = {
 .set_cursor_position = drmmode_set_cursor_position,
 .show_cursor = drmmode_show_cursor,
 .hide_cursor = drmmode_hide_cursor,
-.load_cursor_argb = drmmode_load_cursor_argb,
+.load_cursor_argb_check = drmmode_load_cursor_argb_check,
 
 .gamma_set = drmmode_crtc_gamma_set,
 .destroy = NULL,/* XXX */
-- 
2.3.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] render: Don't generate invalid pixman format when using a 24bpp framebuffer with a 32bit depth visual.

2014-08-21 Thread Takashi Iwai
At Thu, 21 Aug 2014 20:13:48 -0500,
Keith Packard wrote:
> 
> Takashi Iwai  writes:
> 
> 
> > Meanwhile, fbCreatePixmap() has a more check and it creates with 24bpp
> > only when the passed depth <= 24 for avoiding such a problem.
> >
> > This oneliner patch just adds the similar check in fbCreateWindow().
> > This (hopefully) fixes the long-standing broken graphics mess of
> > cirrus KMS with 24bpp.
> 
> Merged.
>5d13327..fe5018e  master -> master

Thanks!


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] render: Don't generate invalid pixman format when using a 24bpp framebuffer with a 32bit depth visual.

2014-08-18 Thread Takashi Iwai
At Sun, 17 Aug 2014 14:41:32 -0700,
Keith Packard wrote:
> 
> Takashi Iwai  writes:
> 
> > That is, bitsPerPixel is replaced from 32 to 24 in CreateWindow().
> > Hence this results in the combination of depth=32/bpp=24.  Ouch.
> 
> Sounds like we need to hack Composite to fix how depth 32 windows are
> supported for this platform; those windows need to actually be listed as
> 32bpp instead of 24bpp.

Reading the relevant codes again, the problem appears to be the
inconsistent bpp between window and pixmap.  Then I tested the
oneliner below to make CreateWindow() behaving same as fb/pixmap.c,
and the problem is actually gone.

Is this approach more reasonable?


Takashi

-- 8< --
From: Takashi Iwai 
Subject: [PATCH] fb: Fix invalid bpp for 24bit depth window

We have a hack in fb layer for a 24bpp screen to use 32bpp images, and
fbCreateWindow() replaces its drawable.bitsPerPixel field
appropriately.  But, the problem is that it always replaces when 32bpp
is passed.  If the depth is 32, this results in bpp < depth, which is
actually invalid.

Meanwhile, fbCreatePixmap() has a more check and it creates with 24bpp
only when the passed depth <= 24 for avoiding such a problem.

This oneliner patch just adds the similar check in fbCreateWindow().
This (hopefully) fixes the long-standing broken graphics mess of
cirrus KMS with 24bpp.

Signed-off-by: Takashi Iwai 
---
 fb/fbwindow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fb/fbwindow.c b/fb/fbwindow.c
index 368c4b883b31..c90175faa078 100644
--- a/fb/fbwindow.c
+++ b/fb/fbwindow.c
@@ -33,7 +33,7 @@ fbCreateWindow(WindowPtr pWin)
 {
 dixSetPrivate(&pWin->devPrivates, fbGetWinPrivateKey(pWin),
   fbGetScreenPixmap(pWin->drawable.pScreen));
-if (pWin->drawable.bitsPerPixel == 32)
+if (pWin->drawable.bitsPerPixel == 32 && pWin->drawable.depth <= 24)
 pWin->drawable.bitsPerPixel =
 fbGetScreenPrivate(pWin->drawable.pScreen)->win32bpp;
 return TRUE;
-- 
2.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] render: Don't generate invalid pixman format when using a 24bpp framebuffer with a 32bit depth visual.

2014-08-15 Thread Takashi Iwai
At Fri, 15 Aug 2014 12:01:53 +0200,
Takashi Iwai wrote:
> 
> At Tue, 12 Aug 2014 16:03:54 -0700,
> Keith Packard wrote:
> > 
> > Robert Ancell  writes:
> > 
> > > When using the fb backend at 24bpp it allows a visual with 32 bit
> > > depth.
> > 
> > That's not valid; depth must never be larger than bits per pixel.
> 
> Can we put some assert to catch such a bug?
> 
> > Please
> > fix your driver, don't break the insides of the X server.
> 
> Which driver do you mean, X or KMS?  It seems happening on both fbdev
> and modesetting X drivers when cirrus/mgag200 KMS is used with 24bpp.

Looking further down, I believe it's rather a X server problem, but
the real cause is in a different place.

The KMS driver advertises properly depth 24 and bpp 24.  However,
fbFinishScreenInit() has the following code:

/*
 * By default, a 24bpp screen will use 32bpp images, this avoids
 * problems with many applications which just can't handle packed
 * pixels.  If you want real 24bit images, include a 24bpp
 * format in the pixmap formats
 */
if (bpp == 24) {
int f;

imagebpp = 32;
/*
 * Check to see if we're advertising a 24bpp image format,
 * in which case windows will use it in preference to a 32 bit
 * format.
 */
for (f = 0; f < screenInfo.numPixmapFormats; f++) {
if (screenInfo.formats[f].bitsPerPixel == 24) {
imagebpp = 24;
break;
}
}
}
if (imagebpp == 32) {
fbGetScreenPrivate(pScreen)->win32bpp = bpp;
fbGetScreenPrivate(pScreen)->pix32bpp = bpp;
}
else {
fbGetScreenPrivate(pScreen)->win32bpp = 32;
fbGetScreenPrivate(pScreen)->pix32bpp = 32;
}

Since bpp = 24 and there is no format providing 24 bpp (fbdev takes
the default screen pixmap formats), imagebpp above is 32, thus
*->wind32bpp is set to bpp, i.e. 24.

This value is referred in fbCreateWindow():

if (pWin->drawable.bitsPerPixel == 32)
pWin->drawable.bitsPerPixel =
fbGetScreenPrivate(pWin->drawable.pScreen)->win32bpp;

That is, bitsPerPixel is replaced from 32 to 24 in CreateWindow().
Hence this results in the combination of depth=32/bpp=24.  Ouch.

So, either we have to kill the hack in fb/* above, or accept that the
combo "depth 32 / bpp 24" can happen and fix the codes appropriately.
Or another better alternative?


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] render: Don't generate invalid pixman format when using a 24bpp framebuffer with a 32bit depth visual.

2014-08-15 Thread Takashi Iwai
At Tue, 12 Aug 2014 16:03:54 -0700,
Keith Packard wrote:
> 
> Robert Ancell  writes:
> 
> > When using the fb backend at 24bpp it allows a visual with 32 bit
> > depth.
> 
> That's not valid; depth must never be larger than bits per pixel.

Can we put some assert to catch such a bug?

> Please
> fix your driver, don't break the insides of the X server.

Which driver do you mean, X or KMS?  It seems happening on both fbdev
and modesetting X drivers when cirrus/mgag200 KMS is used with 24bpp.


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] render: Don't generate invalid pixman format when using a 24bpp framebuffer with a 32bit depth visual.

2014-08-07 Thread Takashi Iwai
At Fri, 6 Jun 2014 16:49:58 +1200,
Robert Ancell wrote:
> 
> Some background on this patch:
> 
> I've been trying to investigate why gnome-terminal doesn't render
> correctly when running Ubuntu 14.04 LTS inside qemu (on Ubuntu 14.10).
> The window is compressed in the X axis and has transparency (seems to
> be a buffer somewhere that is being written in RGB and read as RGBA).
> 
> The issue seems to be GTK+ applications using the RGBA visual when
> qemu/X/fb is providing a depth 24 root window. Playing around with
> test cases showed the assertion mentioned in the patch. I'm not sure
> if fb should be allowed to have a depth greater than the bpp and if
> the bug is actually there but this patch allows correct rendering in
> my test case now. I guess the Picture is being converted from RGBA to
> RGB when it hits the root window?
> 
> Note this patch doesn't fix the gnome-terminal rendering issue, so
> I'll continue down the rabbit hole. Any suggestions as to the causes
> of this are greatly appreciated.
> 
> Attached is a program that doesn't render on the current master and
> does with the applied patch.

Tested-by: Takashi Iwai 

Please someone review and take this, too.


thanks,

Takashi


> 
> --Robert
> 
> On Fri, Jun 6, 2014 at 4:36 PM, Robert Ancell
>  wrote:
> > When using the fb backend at 24bpp it allows a visual with 32 bit depth.
> > This would cause RENDER to try and create an invalid pixman buffer and hit 
> > the
> > following assertion when trying to render to it:
> >
> > *** BUG ***
> > In create_bits_image_internal: The expression PIXMAN_FORMAT_BPP (format) >= 
> > PIXMAN_FORMAT_DEPTH (format) was false
> > Set a breakpoint on '_pixman_log_error' to debug
> >
> > Fix is to ensure that the bpp is always at least as big as the drawable 
> > depth.
> > ---
> >  render/picture.c | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/render/picture.c b/render/picture.c
> > index 7da9310..f411521 100644
> > --- a/render/picture.c
> > +++ b/render/picture.c
> > @@ -762,6 +762,7 @@ CreatePicture(Picture pid,
> >  {
> >  PicturePtr pPicture;
> >  PictureScreenPtr ps = GetPictureScreen(pDrawable->pScreen);
> > +int bpp;
> >
> >  pPicture = dixAllocateScreenObjectWithPrivates(pDrawable->pScreen,
> > PictureRec, 
> > PRIVATE_PICTURE);
> > @@ -773,7 +774,10 @@ CreatePicture(Picture pid,
> >  pPicture->id = pid;
> >  pPicture->pDrawable = pDrawable;
> >  pPicture->pFormat = pFormat;
> > -pPicture->format = pFormat->format | (pDrawable->bitsPerPixel << 24);
> > +bpp = pDrawable->bitsPerPixel;
> > +if (bpp < pFormat->depth)
> > +bpp = BitsPerPixel (pFormat->depth);
> > +pPicture->format = pFormat->format | (bpp << 24);
> >
> >  /* security creation/labeling check */
> >  *error = XaceHook(XACE_RESOURCE_ACCESS, client, pid, PictureType, 
> > pPicture,
> > --
> > 2.0.0
> >
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] fb: Correctly implement CopyArea when using a window with depth 32 and 24bpp.

2014-08-07 Thread Takashi Iwai
At Fri, 6 Jun 2014 23:59:20 +1200,
Robert Ancell wrote:
> 
> This fixes gnome-terminal not showing in qemu [1] and can be triggered
> by the attached test case.
> 
> I haven't combined this with the patch in [2] because this patch is
> internally coherent with the fb logic; I still don't know if fb is
> doing the right thing here by allowing depth=32 bpp=24. The RENDER
> patch is not required for gnome-terminal (and I suspect any
> application using depth 32 visual and cairo/GTK+ to render) but
> follows the same logic.
> 
> --Robert
> 
> [1] http://lists.x.org/archives/xorg-devel/2014-June/042723.html
> [2] http://lists.x.org/archives/xorg-devel/2014-June/042721.html

I confirm that this patch fixes the problem on QEMU cirrus.

Tested-by: Takashi Iwai 

Can anyone review and take?  It's been a very long-standing and
annoying problem over years.


Thanks!

Takashi


> 
> On Fri, Jun 6, 2014 at 11:52 PM, Robert Ancell
>  wrote:
> > When using the fb backend at 24bpp it allows a visual with 32 bit depth.
> > When using CopyArea from a 32bpp pixmap to a window with a 32 bit depth it 
> > would
> > read the ARGB as RGB.
> >
> > Fix is to correctly ignore the alpha channel in the pixmap when copying.
> > ---
> >  fb/fbcopy.c | 10 +-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/fb/fbcopy.c b/fb/fbcopy.c
> > index 541ef71..5455947 100644
> > --- a/fb/fbcopy.c
> > +++ b/fb/fbcopy.c
> > @@ -242,8 +242,16 @@ fbCopyArea(DrawablePtr pSrcDrawable,
> > int xIn, int yIn, int widthSrc, int heightSrc, int xOut, int 
> > yOut)
> >  {
> >  miCopyProc copy;
> > +int src_bpp, dst_bpp;
> >
> > -if (pSrcDrawable->bitsPerPixel != pDstDrawable->bitsPerPixel)
> > +src_bpp = pSrcDrawable->bitsPerPixel;
> > +if (src_bpp < pSrcDrawable->depth)
> > +src_bpp = BitsPerPixel (pSrcDrawable->depth);
> > +dst_bpp = pDstDrawable->bitsPerPixel;
> > +if (dst_bpp < pDstDrawable->depth)
> > +dst_bpp = BitsPerPixel (pDstDrawable->depth);
> > +
> > +if (src_bpp != dst_bpp)
> >  copy = fb24_32CopyMtoN;
> >  else
> >  copy = fbCopyNtoN;
> > --
> > 2.0.0
> >
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] xf86/cursor: Check hw cursor after xf86SeCursor() call, too

2014-07-17 Thread Takashi Iwai
At Thu, 17 Jul 2014 11:09:19 -0700,
Keith Packard wrote:
> 
> Takashi Iwai  writes:
> 
> > When a video driver changed the availability of hw cursor inside the
> > xf86SetCursor() call, f86CursorSetCursor() still assumes the hw
> > cursor and clears SWCursor flag of the screen unconditionally.  This
> > results in the inconsistent state, typically the invisible cursor
> > pointer.  For example, the problem is seen with modesetting driver
> > with Cirrus or MGA KMS where the hw cursor isn't always available.
> >
> > This patch adds the check of availability of hw cursor again after the
> > call of xf86SetCursor(), and proceeds only when it passes the test
> > after the call.
> 
> This has been fixed with Michael Thayer's patch which has xf86SetCursor
> return whether the hw cursor was enabled successfully.

OK, thanks!  I myself forgot about this patch completely :)


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH] xf86/cursor: Check hw cursor after xf86SeCursor() call, too

2014-03-20 Thread Takashi Iwai
When a video driver changed the availability of hw cursor inside the
xf86SetCursor() call, f86CursorSetCursor() still assumes the hw
cursor and clears SWCursor flag of the screen unconditionally.  This
results in the inconsistent state, typically the invisible cursor
pointer.  For example, the problem is seen with modesetting driver
with Cirrus or MGA KMS where the hw cursor isn't always available.

This patch adds the check of availability of hw cursor again after the
call of xf86SetCursor(), and proceeds only when it passes the test
after the call.

Signed-off-by: Takashi Iwai 
---
 hw/xfree86/ramdac/xf86Cursor.c | 47 --
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c
index 860704e1c387..10c130e3edf2 100644
--- a/hw/xfree86/ramdac/xf86Cursor.c
+++ b/hw/xfree86/ramdac/xf86Cursor.c
@@ -295,6 +295,28 @@ xf86CursorUnrealizeCursor(DeviceIntPtr pDev, ScreenPtr 
pScreen, CursorPtr pCurs)
 return (*ScreenPriv->spriteFuncs->UnrealizeCursor) (pDev, pScreen, pCurs);
 }
 
+static Bool
+useHwCursor(ScreenPtr pScreen, xf86CursorScreenPtr ScreenPriv,
+xf86CursorInfoPtr infoPtr, CursorPtr cursor)
+{
+if (ScreenPriv->ForceHWCursorCount)
+return TRUE;
+
+#ifdef ARGB_CURSOR
+if (cursor->bits->argb &&
+infoPtr->UseHWCursorARGB &&
+(*infoPtr->UseHWCursorARGB)(pScreen, cursor))
+return TRUE;
+if (cursor->bits->argb != 0)
+return FALSE;
+#endif
+if ((cursor->bits->height <= infoPtr->MaxHeight) &&
+(cursor->bits->width <= infoPtr->MaxWidth) &&
+(!infoPtr->UseHWCursor || (*infoPtr->UseHWCursor) (pScreen, cursor)))
+return TRUE;
+return FALSE;
+}
+
 static void
 xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs,
 int x, int y)
@@ -336,28 +358,21 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, 
CursorPtr pCurs,
 ScreenPriv->SavedCursor = cursor;
 
 if (infoPtr->pScrn->vtSema && 
xorg_list_is_empty(&pScreen->pixmap_dirty_list) &&
-(ScreenPriv->ForceHWCursorCount ||
- ((
-#ifdef ARGB_CURSOR
-   cursor->bits->argb &&
-   infoPtr->UseHWCursorARGB &&
-   (*infoPtr->UseHWCursorARGB)(pScreen, cursor)) ||
-  (cursor->bits->argb == 0 &&
-#endif
-   (cursor->bits->height <= infoPtr->MaxHeight) &&
-   (cursor->bits->width <= infoPtr->MaxWidth) &&
-   (!infoPtr->UseHWCursor || (*infoPtr->UseHWCursor) (pScreen, 
cursor)) {
-
+useHwCursor(pScreen, ScreenPriv, infoPtr, cursor)) {
 if (ScreenPriv->SWCursor)   /* remove the SW cursor */
 (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen,
NullCursor, x, y);
 
 xf86SetCursor(pScreen, cursor, x, y);
-ScreenPriv->SWCursor = FALSE;
-ScreenPriv->isUp = TRUE;
 
-miPointerSetWaitForUpdate(pScreen, !infoPtr->pScrn->silkenMouse);
-return;
+/* Check again in case the hwcursor is disabled in callbacks */
+if (useHwCursor(pScreen, ScreenPriv, infoPtr, cursor)) {
+ScreenPriv->SWCursor = FALSE;
+ScreenPriv->isUp = TRUE;
+
+miPointerSetWaitForUpdate(pScreen, 
!infoPtr->pScrn->silkenMouse);
+return;
+}
 }
 
 miPointerSetWaitForUpdate(pScreen, TRUE);
-- 
1.9.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH V2 driver/input/synaptics] For serial devices try reconnecting if device is wedged

2013-08-01 Thread Takashi Iwai
At Thu, 1 Aug 2013 14:12:58 +0200,
Egbert Eich wrote:
> 
> On Thu, Aug 01, 2013 at 04:38:38PM +1000, Peter Hutterer wrote:
> > On Tue, Jul 30, 2013 at 05:27:06AM +0200, Egbert Eich wrote:
> > > From: Takashi Iwai 
> > > 
> > > Under some circumstances the synaptics device may be wedged when
> > > coming back from a sleep state. Add some magic which tries to
> > > detect this and reconnect.
> > > 
> > > Signed-off-by: Takashi Iwai 
> > > Signed-off-by: Egbert Eich 
> > > ---
> > > v2: Avoid '_' at beginning of function name.
> > > 
> > >  src/eventcomm.c|  3 ++-
> > >  src/synaptics.c| 56 
> > > +++---
> > >  src/synapticsstr.h |  3 +++
> > >  3 files changed, 54 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > > index 258a538..86b74cb 100644
> > > --- a/src/eventcomm.c
> > > +++ b/src/eventcomm.c
> > > @@ -638,7 +638,8 @@ EventReadHwState(InputInfoPtr pInfo,
> > >  }
> > >  
> > >  while (SynapticsReadEvent(pInfo, &ev)) {
> > > -switch (ev.type) {
> > > + priv->comm_read++;
> > > + switch (ev.type) {
> > 
> > indentation, goes for the rest of the patch.
> > 
> > comm_read is increased on every event so there's the (unlikely) chance of an
> > overrun. why not just set this to 1?
> 
> Yeah, for the purpose we use it for it shouldn't make a difference.
> 
> > 
> > >  case EV_SYN:
> > >  switch (ev.code) {
> > >  case SYN_REPORT:
> > > diff --git a/src/synaptics.c b/src/synaptics.c
> > > index f0a8269..007d0bb 100644
> > > --- a/src/synaptics.c
> > > +++ b/src/synaptics.c
> > > @@ -920,18 +920,30 @@ DeviceControl(DeviceIntPtr dev, int mode)
> > >  }
> > >  
> > >  static int
> > > -DeviceOn(DeviceIntPtr dev)
> > > +doDeviceOn(InputInfoPtr pInfo)
> > >  {
> > > -InputInfoPtr pInfo = dev->public.devicePrivate;
> > >  SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private);
> > > +int n;
> > >  
> > >  DBG(3, "Synaptics DeviceOn called\n");
> > >  
> > > -pInfo->fd = xf86OpenSerial(pInfo->options);
> > > +for (n = priv->retries; n >= 0; n--) {
> > 
> > I really want a comment here - I read over the >= 0 and was wondering how
> > we'd even enter the loop.
> 
>   I don't like comments for this purpose. So let's rewrite the code to
>   make this more obvious.
>   
> > 
> > > + pInfo->fd = xf86OpenSerial(pInfo->options);
> > > + if (pInfo->fd != -1)
> > > + break;
> > > + if (n)
> > > + xf86Msg(X_WARNING, "%s: cannot open input device - "
> > > + "retrying %d more times\n", pInfo->name, n);
> > > +}
> > >  if (pInfo->fd == -1) {
> > >  xf86IDrvMsg(pInfo, X_WARNING, "cannot open input device\n");
> > >  return !Success;
> > >  }
> > > +/* This has succeeded once, so chances are the hardware *really* is 
> > > present
> > > + * - this is not a hotplug device after all.
> > > + * Without trying really hard on some machines with some kernels the 
> > > device
> > > + * won't be found after S3/S4 again. */
> > > +priv->retries = 4;
> >   
> > I'd love to have some details here above "some machines with some kernels".
> > I remember this being a problem around the Fedora 8 cycle (or so) but it's
> > hasn't been seen for ages here (evdev doesnt have the code anymore).
> > 
> > what's the errno when this happens?
> 
> I don't know the details here as I never looked into this bug.
> Takashi - do you remember this? Do we still need to bother?

It's the time of SLE11-GA, i.e. 2.6.27 kernel, found on a few HP
laptops based on IronLake.  There is no problem since then.

> 
> > 
> > >  if (priv->proto_ops->DeviceOnHook &&
> > >  !priv->proto_ops->DeviceOnHook(pInfo, &priv->synpara))
> > > @@ -956,11 +968,21 @@ DeviceOn(DeviceIntPtr dev)
> > >  }
> > >  
> > >  xf86AddEnabledDevice(pInfo);
> > > -dev->public.on = TRUE;
> > >  
> > >  retur

Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Takashi Iwai
At Sat, 23 Jul 2011 12:49:29 +0200,
Julien Cristau wrote:
> 
> On Sat, Jul 23, 2011 at 12:35:11 +0200, Takashi Iwai wrote:
> 
> > At Sat, 23 Jul 2011 10:49:53 +0200,
> > Julien Cristau wrote:
> > > 
> > > On Sat, Jul 23, 2011 at 09:31:41 +0200, Takashi Iwai wrote:
> > > 
> > > > At Fri, 22 Jul 2011 21:49:08 +0200,
> > > > Julien Cristau wrote:
> > > > > 
> > > > > Is "ck-daemon isn't running" a good reason to prevent the user from
> > > > > logging in?  (Why?)
> > > > 
> > > > It's reached only when you set use_consolekit is set.  You say you
> > > > are using CK but it's not detected, thus it's an error.
> > > > 
> > > use_consolekit defaults to yes, so that's not really true.
> > 
> > Feel free to set it off as default.
> > 
> > But note that this is set true only when the consolekit support is
> > found and enabled via configure script, and most modern distros will
> > make it enabled as default anyway.
> > 
> There's a difference between a build-time setting and a run-time
> setting.

Yes, but there is no difference: most distros will do enable it as
default for runtime setting, too.

> > >  It doesn't
> > > seem to be flagged as an error by pam-ck-connector, in any case (it
> > > returns PAM_IGNORE), I'm not sure why it's fatal for xdm.
> > 
> > It could be handled as non-fatal when no error code is set, too.
> > 
> > But usually you'll face problems sooner or later in such a situation
> > because it means that you logged in without proper device
> > permissions.
> > 
> "I don't have access to the audio device" is a better failure mode than
> "I can't even log in", in my book.

Its your book, and others may think differently like me.  I'd like to
see the obvious error beforehand than too late.

That said, this is a kind of problem for which no 100% correct answer
exists.  And I personally don't care at all how the default behavior
would be, as this is about a handling of exceptional error case.  So
feel free to cook the code.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Takashi Iwai
At Sat, 23 Jul 2011 10:49:53 +0200,
Julien Cristau wrote:
> 
> On Sat, Jul 23, 2011 at 09:31:41 +0200, Takashi Iwai wrote:
> 
> > At Fri, 22 Jul 2011 21:49:08 +0200,
> > Julien Cristau wrote:
> > > 
> > > Is "ck-daemon isn't running" a good reason to prevent the user from
> > > logging in?  (Why?)
> > 
> > It's reached only when you set use_consolekit is set.  You say you
> > are using CK but it's not detected, thus it's an error.
> > 
> use_consolekit defaults to yes, so that's not really true.

Feel free to set it off as default.

But note that this is set true only when the consolekit support is
found and enabled via configure script, and most modern distros will
make it enabled as default anyway.

>  It doesn't
> seem to be flagged as an error by pam-ck-connector, in any case (it
> returns PAM_IGNORE), I'm not sure why it's fatal for xdm.

It could be handled as non-fatal when no error code is set, too.

But usually you'll face problems sooner or later in such a situation
because it means that you logged in without proper device
permissions.


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Takashi Iwai
At Fri, 22 Jul 2011 21:49:08 +0200,
Julien Cristau wrote:
> 
> > +   display_device = devtmp;
> > +}
> > +
> > +connector = ck_connector_new();
> > +if (!connector) {
> > +   LogOutOfMem("ck_connector");
> > +   return 0;
> > +}
> > +
> > +dbus_error_init(&error);
> > +ret = ck_connector_open_session_with_parameters(
> > +   connector, &error,
> > +   "unix-user", &verify->uid,
> > +   "x11-display", &display_name,
> > +   "x11-display-device", &display_device,
> > +   "remote-host-name", &remote_host_name,
> > +   "is-local", &is_local,
> > +   NULL);
> > +if (!ret) {
> > +   if (dbus_error_is_set(&error)) {
> > +   LogError("Dbus error: %s\n", error.message);
> > +   dbus_error_free(&error);
> > +   } else {
> > +   LogError("ConsoleKit error\n");
> > +   }
> > +   LogError("console-kit-daemon not running?\n");
> > +   ck_connector_unref(connector);
> > +   connector = NULL;
> > +   return 0;
> 
> Is "ck-daemon isn't running" a good reason to prevent the user from
> logging in?  (Why?)

It's reached only when you set use_consolekit is set.  You say you
are using CK but it's not detected, thus it's an error.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 08/18] Avoid unexpected jumps

2010-10-13 Thread Takashi Iwai
At Wed, 13 Oct 2010 08:29:24 +0200,
Henrik Rydberg wrote:
> 
> On 10/13/2010 08:01 AM, Takashi Iwai wrote:
> [...]
> 
> > The background of this patch is that the device sometimes doesn't report
> > the finger off from the pad, instead it sends z with a high value in
> > the last packet, then stops sending packets.  Thus the driver still
> > keeps tracking the position as a part of continuous pointer movement
> > when you touch again at a totally different point, which results in a
> > jump.
> 
> 
> I wonder if this is entirely correct... Making sure the device/driver produces
> correct data on the kernel level is the sane way to go. We must be able to 
> trust
> the basic assumptions we put up, or we end up with patches like this one in
> every application.

OTOH, if the judgment of the pointer jump has to be done in a way like
this, user-space is more flexible for different parameters, etc, in
comparison with the kernel.


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 03/18] Add the embedded LED support

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 15:56:38 +1000,
Peter Hutterer wrote:
> 
> On Wed, Oct 13, 2010 at 07:47:32AM +0200, Takashi Iwai wrote:
> > At Wed, 13 Oct 2010 13:29:21 +1000,
> > Peter Hutterer wrote:
> > > 
> > > On Fri, Oct 08, 2010 at 07:22:27PM +0200, Takashi Iwai wrote:
> > > > This patch adds the control of the embedded LED on the top-left corner
> > > > of new Synaptics devices.  For LED control, it requires the patch to
> > > > Linux synaptics input driver,
> > > > https://patchwork.kernel.org/patch/92434/
> > > > 
> > > > When a sysfs file /sys/class/leds/psmouse::synaptics exists, the driver
> > > > assumes it supports the embeded LED control.
> > > > 
> > > > The LED can be controlled via new properties, "Synaptics LED" and
> > > > "Synaptics LED Status".
> > > > 
> > > > Signed-off-by: Takashi Iwai 
> > > > ---
> > > >  include/synaptics-properties.h |6 ++
> > > >  man/synaptics.man  |9 +
> > > >  src/eventcomm.c|   32 +++-
> > > >  src/properties.c   |   15 +++
> > > >  src/synapticsstr.h |2 ++
> > > >  src/synproto.h |1 +
> > > >  tools/synclient.c  |1 +
> > > >  7 files changed, 65 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/include/synaptics-properties.h 
> > > > b/include/synaptics-properties.h
> > > > index 9c6a2ee..dd7e259 100644
> > > > --- a/include/synaptics-properties.h
> > > > +++ b/include/synaptics-properties.h
> > > > @@ -155,4 +155,10 @@
> > > >  /* 32 bit, 4 values, left, right, top, bottom */
> > > >  #define SYNAPTICS_PROP_AREA "Synaptics Area"
> > > >  
> > > > +/* 8 bit (BOOL, read-only), has_led */
> > > > +#define SYNAPTICS_PROP_LED "Synaptics LED"
> > > > +
> > > 
> > > this should be added to the "Synaptics Capabilities" property, there's no
> > > need for a separate property.
> > 
> > OK, sounds sensible.
> > 
> > > I guess sooner or later we'll see non-clickpad
> > > devices come out with leds too, making this a generic capability like
> > > two-finger tapping and similar.
> > 
> > There are already lots of laptops with an LED but also separate
> > buttons, indeed.
> > 
> > > > @@ -278,6 +280,9 @@ InitDeviceProperties(InputInfoPtr pInfo)
> > > >  values[2] = para->area_top_edge;
> > > >  values[3] = para->area_bottom_edge;
> > > >  prop_area = InitAtom(pInfo->dev, SYNAPTICS_PROP_AREA, 32, 4, 
> > > > values);
> > > > +
> > > > +prop_led = InitAtom(local->dev, SYNAPTICS_PROP_LED, 8, 1, 
> > > > ¶->has_led);
> > > > +prop_led_status = InitAtom(local->dev, SYNAPTICS_PROP_LED_STATUS, 
> > > > 8, 1, ¶->led_status);
> > > 
> > > the prop_led_status atom should only be initialized if the touchpad 
> > > actually
> > > has a led, please make this conditional on the has_led bit.
> > 
> > So, you mean that prop_led should be NULL when no LED exists, thus
> > the querying this property would lead to an error?
> 
> yeah, the property prop_led_status simply won't be available and return a
> BadAtom to a client that tries to modify it. this is more explanatory than
> having the property but not doing anything with it if the device doesn't
> have a led.
> 
> fwiw, note that all prop_* are of type Atom, which is just an int. And since
> 0 (None) isn't a property, we don't need any extra checks for it in the
> driver, the server filters for us.

Alright.

While we are at it.  Any reason all these prop_* aren't static?


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 00/18] Synaptics patches

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 14:29:09 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:24PM +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > [dropped linux-* ML since these are pure X patches]
> > 
> > here is a series of patches for xorg synaptics driver, mainly supporting
> > Clickpad devices and multi-touch features.  It also contains some fixes,
> > improvements of the current behavior, also the extension for sending key
> > events, etc.
> > 
> > Not all patches are in quite good form, and definitely need discussions
> > and/or fixes.  The whole bunch of patches have been pending for so
> > long time by some (irritating) reason, and I feel relieved now.
> > Everyone knows the same feeling in daily life, I guess, so, let me
> > flush the patches from my queue first :)
> > 
> > (BTW, I'm on vacation until Monday; will reply after that)
> 
> Thanks Takashi,
> 
> (sorry if this mail addresses some things you've already commented on, I
> wrote it while I was reviewing the patches)
> 
> Bit of a higher-level comment than the patch-specific ones. I just ran a
> test of a few of them and they seem to work. Note that they need fixes to
> compile against master, most notable LocalDevicePtr is gone from the server
> now and needs to be replaced with InputInfoPtr (identical typedefs though).
> So please replace any "LocalDevicePtr local" with "InputInfoPtr pInfo".

Ah, OK.  I had no time to actually do testing with master.
Will check later.

> There is a random mix of tab/space indentations in your patches. Please
> observe spaces only for newly written code, tabs where tabs are already
> there, and common sense everywhere else.

Noted.

> The order of the patches is a bit confusing. e.g. in 01 you add the clickpad
> support with a hardcoded button area, 02 is the man page, then you add the
> configurable button area in 07 (as part of another, unrelated change). Just
> split that out and add it to the first patch if you don't want the area to
> be hardcoded.
> I realise this was a flush of patches but making things easy to review
> should generally be of a high priority. especially in terms of commit
> messages, I want to know what you intended to do with a patch. 

Sure, the patches are mostly in the historical order.  (Man's activity
isn't always explained logically, no? :)

The reason why the configurable parameter for clickpad wasn't in the
first patch is that the first patch (ab-)uses the bottom_edge as the
button position.  In the later patch, the button area is handled again
as a normal touch area, thus this condition was removed, and a new
parameter was introduced instead.


> In terms of property usage:
> the interesting ones here are "Synaptics Capabilities" and "Synaptics
> Gestures". Things like is_clickpad and the has_led should go into the former
> to allow clients to change their UI based on this particular hardware.
> The "Synaptics Gesture" is for low-level gestures, currently only
> tap-and-drag, but the LED doubletap is an example for such a gesture that
> could be added there.

Noted.

> multitouch support: this is too geared on the clickpad. I'd rather see a
> generic multitouch support added, then the clickpad-specifics on top.

OK.

> As for gestures in the driver - no. wacom has it because it plays by
> slightly different rules, but that doesn't mean it's a good idea.
> the driver is not the place to do gestures.

Heh, this is an expected answer.

> you send key events from gestures, yet the driver has no way of knowing when
> the keymap is updated. so if you do so much as a group change, your ctrl+w
> close gesture will suddenly mark everything (french azerty vs. us qwerty).
> so each time you even do so much as change layouts, you'd have to reload the
> property. which means you have to have a daemon in place that monitors the
> current group, etc.

Hm, can't it be hooked and detected in the driver routine...?

I find sending key events more usable in the current situation.
Of course, we can create a new set of buttons for gesture actions, but
it'll take ages until apps accept new button events.  OTOH, keys are
already defined, and applicable as is.  That is, sending keys was the
only option to make things work at this moment.

> you have a gesture detection engine that's incompatible with every other one
> in the system. so what's pinch for the synaptics driver is different to the
> wacom driver's pinch, etc.
> 
> also, there's some inconsistency in the wording: you mix up pinch and zoom
> in the first patch. just because pinch _can_ mean zoom doesn't mean
> it always is. especially the way it's configured, y

Re: [PATCH 15/18] Add rotation support for synaptics driver

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 14:13:00 +1000,
Peter Hutterer wrote:
> 
> wacom calls it rotation, not sure about other drivers (evdev has an
> unfortunate mix of swapaxes and invert). some consistency would be nice.
> also, add man page update please with the allowed values.

I have no preference, so just call one as you like :)

> IMO the orientation property should go into the server anyway, so we finally
> have some consistency.

Is there any plan (a defined API) for it?

> there's also a demand for orientation other than in
> 90 deg increments, so perhaps parsing the actual degrees would be useful.

OK.


thanks,

Takashi



> Cheers,
>   Peter
> 
> On Fri, Oct 08, 2010 at 07:22:39PM +0200, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai 
> > ---
> >  include/synaptics-properties.h |3 +++
> >  src/properties.c   |8 
> >  src/synaptics.c|   35 +++
> >  src/synapticsstr.h |1 +
> >  tools/synclient.c  |1 +
> >  5 files changed, 48 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > index 5d440be..f6fbfac 100644
> > --- a/include/synaptics-properties.h
> > +++ b/include/synaptics-properties.h
> > @@ -179,4 +179,7 @@
> >  /* 32bit, read-only */
> >  #define SYNAPTICS_PROP_GESTURE_MODE "Current Gesture Mode"
> >  
> > +/* 32 bit */
> > +#define SYNAPTICS_PROP_ORIENTATION "Synaptics Orientation"
> > +
> >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > diff --git a/src/properties.c b/src/properties.c
> > index 7788a13..7278560 100644
> > --- a/src/properties.c
> > +++ b/src/properties.c
> > @@ -46,6 +46,7 @@ static Atom float_type;
> >  
> >  Atom prop_edges = 0;
> >  Atom prop_finger= 0;
> > +Atom prop_orientation   = 0;
> >  Atom prop_tap_time  = 0;
> >  Atom prop_tap_move  = 0;
> >  Atom prop_tap_durations = 0;
> > @@ -263,6 +264,8 @@ InitDeviceProperties(InputInfoPtr pInfo)
> >  
> >  prop_pressuremotion_factor = InitFloatAtom(pInfo->dev, 
> > SYNAPTICS_PROP_PRESSURE_MOTION_FACTOR, 2, fvalues);
> >  
> > +prop_orientation = InitAtom(local->dev, SYNAPTICS_PROP_ORIENTATION, 
> > 32, 1, ¶->orientation);
> > +
> >  prop_grab = InitAtom(pInfo->dev, SYNAPTICS_PROP_GRAB, 8, 1, 
> > ¶->grab_event_device);
> >  
> >  values[0] = para->tap_and_drag_gesture;
> > @@ -674,6 +677,11 @@ SetProperty(DeviceIntPtr dev, Atom property, 
> > XIPropertyValuePtr prop,
> >  
> >  para->press_motion_min_z = press[0];
> >  para->press_motion_max_z = press[1];
> > +} else if (property == prop_orientation) {
> > +if (prop->size != 1 || prop->format != 32 || prop->type != 
> > XA_INTEGER)
> > +return BadMatch;
> > +
> > +   para->orientation = *(INT32*)prop->data;
> >  } else if (property == prop_grab)
> >  {
> >  if (prop->size != 1 || prop->format != 8 || prop->type != 
> > XA_INTEGER)
> > diff --git a/src/synaptics.c b/src/synaptics.c
> > index 2f264f0..71a5134 100644
> > --- a/src/synaptics.c
> > +++ b/src/synaptics.c
> > @@ -600,6 +600,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
> >  pars->coasting_friction = xf86SetRealOption(opts, "CoastingFriction", 
> > 50);
> >  pars->press_motion_min_factor = xf86SetRealOption(opts, 
> > "PressureMotionMinFactor", 1.0);
> >  pars->press_motion_max_factor = xf86SetRealOption(opts, 
> > "PressureMotionMaxFactor", 1.0);
> > +pars->orientation = xf86SetIntOption(opts, "Orientation", 0);
> >  pars->grab_event_device = xf86SetBoolOption(opts, "GrabEventDevice", 
> > TRUE);
> >  pars->tap_and_drag_gesture = xf86SetBoolOption(opts, 
> > "TapAndDragGesture", TRUE);
> >  pars->resolution_horiz = xf86SetIntOption(opts, "HorizResolution", 
> > horizResolution);
> > @@ -2818,6 +2819,38 @@ repeat_scrollbuttons(const InputInfoPtr pInfo,
> >  return delay;
> >  }
> >  
> > +static void do_rotation(SynapticsPrivate *priv, int orientation, int *xp, 
> > int *yp)
> > +{
> > +int width = priv->maxx - priv->minx;
> > +int height = priv->maxy - priv->miny;
> > +int x = *xp;
> > +int y = *yp;
> > +
> >

Re: [PATCH 14/18] Add gesture-mode notification via input properties

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 14:06:48 +1000,
Peter Hutterer wrote:
> 
> explanation please. I prefer looking at a patch knowing what to expect, this
> title explains nothing, so I don't know if any of the code does what you
> intended.

It's for telling outside which gesture mode is activated currently.
I made a small program showing an icon beside the cursor pointer when
the multi-touch gesture is activated, so that user can "see" whether it's
in zoom or scroll or other modes.  This is pretty useful especially
when different gestures are available and switched with each other.

> one thing though: properties are prone to race conditions. the event only
> contains the name of the property, not the values. so whatever this patch
> does, if you're trying to tell the client what gesture is currently active
> or so that won't work. by the time the client can query the property, it may 
> have updated several times already.

As long as X is a single thread, it works, so far :)
When the state is changed in a signal handler, the notification is
delayed so that it won't crash with the current reading.


Anyway, this patch is just an experimental fun stuff.  I don't expect
it will be merged as is, too.  But I wanted to raise some need for
such a thing.


thanks,

Takashi

> 
> Cheers,
>   Peter
> 
> 
> On Fri, Oct 08, 2010 at 07:22:38PM +0200, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai 
> > ---
> >  include/synaptics-properties.h |6 +++
> >  src/properties.c   |   33 +++
> >  src/synaptics.c|   68 
> > +---
> >  src/synapticsstr.h |   11 ++
> >  tools/synclient.c  |1 +
> >  5 files changed, 107 insertions(+), 12 deletions(-)
> > 
> > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > index 7112bc0..5d440be 100644
> > --- a/include/synaptics-properties.h
> > +++ b/include/synaptics-properties.h
> > @@ -173,4 +173,10 @@
> >  /* 32 bit, 2 values, start, delta (0 disable) */
> >  #define SYNAPTICS_PROP_MULTI_TOUCH_PINCH "Synaptics Multi-Touch Pinch"
> >  
> > +/* 8 bit (BOOL) */
> > +#define SYNAPTICS_PROP_GESTURE_MODE_NOTIFY "Gesture Mode Notify"
> > +
> > +/* 32bit, read-only */
> > +#define SYNAPTICS_PROP_GESTURE_MODE "Current Gesture Mode"
> > +
> >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > diff --git a/src/properties.c b/src/properties.c
> > index 0f679dd..7788a13 100644
> > --- a/src/properties.c
> > +++ b/src/properties.c
> > @@ -88,6 +88,8 @@ Atom prop_led   = 0;
> >  Atom prop_led_status= 0;
> >  Atom prop_led_double_tap= 0;
> >  Atom prop_multi_touch_pinch = 0;
> > +Atom prop_gesture_mode_notify   = 0;
> > +Atom prop_gesture_mode  = 0;
> >  
> >  static Atom
> >  InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int 
> > *values)
> > @@ -296,6 +298,11 @@ InitDeviceProperties(InputInfoPtr pInfo)
> >  values[0] =  para->multi_touch_pinch_start;
> >  values[1] =  para->multi_touch_pinch_dist;
> >  prop_multi_touch_pinch = InitAtom(local->dev, 
> > SYNAPTICS_PROP_MULTI_TOUCH_PINCH, 32, 2, values);
> > +
> > +prop_gesture_mode_notify = InitAtom(local->dev, 
> > SYNAPTICS_PROP_GESTURE_MODE_NOTIFY, 8, 1, ¶->gesture_mode_notify);
> > +
> > +values[0] = 0;
> > +prop_gesture_mode = InitAtom(local->dev, SYNAPTICS_PROP_GESTURE_MODE, 
> > 32, 1, values);
> >  }
> >  
> >  int
> > @@ -542,7 +549,13 @@ SetProperty(DeviceIntPtr dev, Atom property, 
> > XIPropertyValuePtr prop,
> >  dist = (INT32*)prop->data;
> > para->multi_touch_pinch_start = dist[0];
> > para->multi_touch_pinch_dist = dist[1];
> > +} else if (property == prop_gesture_mode_notify)
> > +{
> > +INT32 *dist;
> > +if (prop->size != 1 || prop->format != 8 || prop->type != 
> > XA_INTEGER)
> > +return BadMatch;
> >  
> > +para->gesture_mode_notify = *(CARD8*)prop->data;
> >  } else if (property == prop_gestures)
> >  {
> >  BOOL *gestures;
> > @@ -724,3 +737,23 @@ void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool 
> > off)
> >  XIChangeDeviceProperty(dev, prop_off, XA_INTEGER, 8,
> > PropModeReplace, 1, &val, FALSE);
> >  }
> > +
> > +void SynapticsSetGestureModeProperty(DeviceIntPtr dev, 

Re: [PATCH 16/18] Add synxrrd daemon

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 14:19:49 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:40PM +0200, Takashi Iwai wrote:
> > This is a daemon program to sync Xrandr rotation with the synaptics device.
> 
> can we please integrate this with the respective DEs instead?

Sorry, I don't understand well.  What is DE meant here?


Takashi

> Cheers,
>   Peter
> 
> > Signed-off-by: Takashi Iwai 
> > ---
> >  tools/Makefile.am |5 +-
> >  tools/synxrrd.c   |  186 
> > +
> >  2 files changed, 190 insertions(+), 1 deletions(-)
> >  create mode 100644 tools/synxrrd.c
> > 
> > diff --git a/tools/Makefile.am b/tools/Makefile.am
> > index 2ad48f6..576 100644
> > --- a/tools/Makefile.am
> > +++ b/tools/Makefile.am
> > @@ -18,7 +18,7 @@
> >  #  IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> >  #  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE 
> > SOFTWARE.
> >  
> > -bin_PROGRAMS = synclient syndaemon
> > +bin_PROGRAMS = synclient syndaemon synxrrd
> >  
> >  AM_CPPFLAGS = -I$(top_srcdir)/include -I$(sdkdir)
> >  AM_CFLAGS = $(XI_CFLAGS)
> > @@ -30,3 +30,6 @@ syndaemon_SOURCES = syndaemon.c
> >  syndaemon_CFLAGS = $(AM_CFLAGS) $(XTST_CFLAGS)
> >  syndaemon_LDFLAGS = $(AM_LDFLAGS) $(XTST_LIBS)
> >  
> > +synxrrd_SOURCES = synxrrd.c
> > +synxrrd_LDFLAGS = -lXrandr $(XLIB_LIBS) $(XI_LIBS)
> > +
> > diff --git a/tools/synxrrd.c b/tools/synxrrd.c
> > new file mode 100644
> > index 000..9f00e5b
> > --- /dev/null
> > +++ b/tools/synxrrd.c
> > @@ -0,0 +1,186 @@
> > +/*
> > + * synxrrd.c
> > + * Copyright (C) 2010 Takashi Iwai 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define progname   "synxrrd"
> > +
> > +static int verbose;
> > +static const char *laptop_name;
> > +
> > +static int is_laptop(const char *name)
> > +{
> > +   if (laptop_name) {
> > +   if (!strcmp(name, laptop_name))
> > +   return 1;
> > +   } else {
> > +   if (strstr(name, "LVDS") ||
> > +   strstr(name, "Lvds") ||
> > +   strstr(name, "LVDs"))
> > +   return 1;
> > +   if (strstr(name, "LCD"))
> > +   return 1;
> > +   }
> > +   return 0;
> > +}
> > +
> > +static int current_rotation = -1; /* unknown */
> > +
> > +static int (*xerror_saved)(Display *, XErrorEvent *);
> > +static int xerror_code;
> > +
> > +static int xerror_ignore(Display *dpy, XErrorEvent *err)
> > +{
> > +   xerror_code = err->error_code;
> > +   return 0;
> > +}
> > +
> > +extern int (*_XErrorFunction)(Display *, XErrorEvent *);
> > +extern int _XDefaultError(Display *dpy, XErrorEvent *event);
> > +
> > +static void xerror_save(void)
> > +{
> > +   xerror_code = 0;
> > +   xerror_saved = _XErrorFunction;
> > +   _XErrorFunction = xerror_ignore;
> > +}
> > +
> > +static int xerror_restore(void)
> > +{
> > +   _XErrorFunction = xerror_saved;
> > +   return xerror_code;
> > +}
> > +
> > +static void notify_synaptics(XRRCrtcInfo *cres)
> > +{
> > +   int rotation;
> > +   char buf[256];
> > +
> > +   if (cres->rotation & 8)
> > +   rotation = 3;
> > +   else if (cres->rotation & 4)
> > +   rotation = 2;
> > +   else if (cres->rotation & 2)
> > +   rotation = 1;
> > +   else
> > +   rotation = 0;
> > +   if (current_rotation == rotation)
> > +   return;
> > +   current_rotation = rotation;
> > +   if (verbose)
> > +   fprintf(stderr, "%s: notified rotation %d\n",
> > +   progname, current_rotation);
> > +   sprintf(buf, "synclient Orientation=%d", current_rotation);
> > +   system(buf);
> > +}
> > +
> > +static void update_screen(Display *dpy, Window root)
> > +{
> > +   XRRScreenResources *res;
> > +   int i;
> > +
> > +   res = XRRGetScreenResources(dpy, root);
> > +   if (!res)
> > +   return;
> > +   for (i = 0; i < res->noutput; i++) {
> > +   XRROutputInfo *info;
> > +
> > +   info = XRRGetOutputInf

Re: [PATCH 11/18] Check multi_touch module parameter and enable forcibly

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 13:53:10 +1000,
Peter Hutterer wrote:
> 
> 
> explanation please. what does this do, why do we want to enable it forcibly,
> etc?

The patch was introduced since I made the kernel driver behaving in ST
mode as default, but switched to MT via the option dynamically.  This
was just for conservativeness, as the kernel driver doesn't send
ABS_{X,Y} and ABS_PRESSURE in MT mode, which makes ST-apps
non-working.

Now, I'm reconsidering that ABS_{X,Y} and ABS_PRESSURE can be sent
*in addition* to ABS_MT_*.  In MT mode, both ABS_{X,Y,PRESSURE} and
ABS_MT_* are sent for the primary point.
That is, old user-space apps would take only ABS_{X,Y,PRESSURE} and
ignore ABS_MT_*.  Meanwhile, MT-aware apps would take only ABS_MT_*
and ignore ABS_{X,Y,PRESSURE}.  So a patch like this won't be needed
at all...


Takashi


> Cheers,
>   Peter
> 
> On Fri, Oct 08, 2010 at 07:22:35PM +0200, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai 
> > ---
> >  src/eventcomm.c |   22 ++
> >  1 files changed, 22 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > index 5969448..2223bac 100644
> > --- a/src/eventcomm.c
> > +++ b/src/eventcomm.c
> > @@ -179,6 +179,27 @@ event_query_info(InputInfoPtr pInfo)
> >  }
> >  }
> >  
> > +/* enable multi-touch feature via module parameter */
> > +#define MULTI_TOUCH_SYSFS_PARM 
> > "/sys/module/psmouse/parameters/multi_touch"
> > +static void enable_multi_touch(void)
> > +{
> > +int fd;
> > +char val = '0';
> > +fd = open(MULTI_TOUCH_SYSFS_PARM, O_RDONLY);
> > +if (fd < 0)
> > +   return;
> > +read(fd, &val, 1);
> > +close(fd);
> > +if (val == '1' || val == '2')
> > +   return;
> > +fd = open(MULTI_TOUCH_SYSFS_PARM, O_WRONLY);
> > +if (fd < 0)
> > +   return;
> > +val = '1';
> > +write(fd, &val, 1);
> > +close(fd);
> > +}
> > +
> >  static void event_query_multi_touch(LocalDevicePtr local)
> >  {
> >  SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
> > @@ -535,6 +556,7 @@ EventReadDevDimensions(InputInfoPtr pInfo)
> >  SynapticsPrivate *priv = (SynapticsPrivate *)pInfo->private;
> >  BOOL *need_grab = (BOOL*)priv->proto_data;
> >  
> > +enable_multi_touch();
> >  if (event_query_is_touchpad(pInfo->fd, (need_grab) ? *need_grab : 
> > TRUE))
> > event_query_axis_ranges(pInfo);
> >  event_query_info(pInfo);
> > -- 
> > 1.7.3.1
> > 
> 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 10/18] Add multi-touch support

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 14:25:16 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:34PM +0200, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai 
> > ---
> >  src/eventcomm.c|   56 +++--
> >  src/synaptics.c|  136 
> > 
> >  src/synapticsstr.h |   12 +
> >  src/synproto.h |6 ++
> >  4 files changed, 196 insertions(+), 14 deletions(-)
> > 
> > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > index 76ff69d..5969448 100644
> > --- a/src/eventcomm.c
> > +++ b/src/eventcomm.c
> > @@ -53,6 +53,17 @@
> >  
> >  #define SYNAPTICS_LED_SYS_FILE 
> > "/sys/class/leds/psmouse::synaptics/brightness"
> >  
> > +#ifndef SYN_MT_REPORT
> > +#define SYN_MT_REPORT  2
> > +#endif
> > +#ifndef ABS_MT_POSITION_X
> > +#define ABS_MT_POSITION_X  0x35
> > +#define ABS_MT_POSITION_Y  0x36
> > +#endif
> > +#ifndef ABS_MT_PRESSURE
> > +#define ABS_MT_PRESSURE0x3a
> > +#endif
> > +
> >  
> > /*
> >   * Function Definitions
> >   
> > /
> > @@ -168,6 +179,22 @@ event_query_info(InputInfoPtr pInfo)
> >  }
> >  }
> >  
> > +static void event_query_multi_touch(LocalDevicePtr local)
> > +{
> > +SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
> > +unsigned long absbits[NBITS(ABS_MAX)] = {0};
> > +int rc;
> > +
> > +priv->can_multi_touch = FALSE;
> 
> has_multitouch would be more in line with the has_left, has_right, etc.
> 
> > +if (priv->model != MODEL_SYNAPTICS)
> > +   return;
> > +SYSCALL(rc = ioctl(local->fd, EVIOCGBIT(EV_ABS, sizeof(absbits)), 
> > absbits));
> > +if (rc >= 0 && TEST_BIT(ABS_MT_POSITION_X, absbits)) {
> > +   priv->can_multi_touch = TRUE;
> > +   xf86Msg(X_INFO, "%s: supports multi-touch finger detection\n", 
> > local->name);
> > +}
> > +}
> > +
> 
> I think we should just rely on MT_SLOTS here and wrap devices that don't do
> it with mtdev. It'll likely make the code simpler (especially in regards to
> tracking the primary) and I can blame Henrik if the tracking doesn't work ;)

Hm, synaptics devices don't track individual points (MT type A), thus
MT_SLOTS can't be used.


> >  static void
> >  event_query_clickpad(LocalDevicePtr local)
> >  {
> > @@ -175,7 +202,7 @@ event_query_clickpad(LocalDevicePtr local)
> >  
> >  /* clickpad device reports only the single left button mask */
> >  if (priv->has_left && !priv->has_right && !priv->has_middle &&
> > -   !priv->has_double &&
> > +   (!priv->has_double || priv->can_multi_touch) &&
> > priv->model == MODEL_SYNAPTICS) {
> > priv->is_clickpad = TRUE;
> > /* enable right/middle button caps; otherwise gnome-settings-daemon
> > @@ -383,21 +410,27 @@ EventReadHwState(InputInfoPtr pInfo,
> > switch (ev.type) {
> > case EV_SYN:
> > switch (ev.code) {
> > +   case SYN_MT_REPORT:
> > +   hw->multi_touch_count++;
> > +   break;
> > case SYN_REPORT:
> > if (comm->oneFinger)
> > -   hw->numFingers = 1;
> > +   hw->numFingers = hw->multi_touch_count ? 
> > hw->multi_touch_count : 1;
> > else if (comm->twoFingers)
> > hw->numFingers = 2;
> > else if (comm->threeFingers)
> > hw->numFingers = 3;
> > else
> > hw->numFingers = 0;
> > +   hw->multi_touch = hw->multi_touch_count;
> > +   hw->multi_touch_count = 0;
> > /* if the coord is out of range, we filter it out */
> > if (priv->is_clickpad && hw->z > 0 && (hw->x < minx || hw->x > 
> > maxx || hw->y < miny || hw->y > maxy))
> > return FALSE;
> > *hwRet = *hw;
> > return TRUE;
> > }
> > +   break;
> > case EV_KEY:
> > v = (ev.value ? TRUE : FALSE);
> > switch (ev.code) {
> > @@ -458,13 +491,25 @@ EventReadHwState(InputInfoPtr pInfo,
> > case EV_ABS:
> >

Re: [PATCH 07/18] Allow touching in clickpad button area

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 14:26:25 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:31PM +0200, Takashi Iwai wrote:
> > Instead of ignoring the mouse touch sense in the clickpad button area,
> > introduce a "stickiness" to the movement for blocking the unexpected
> > movement of the driver at clicking.
> 
> there are two changes in this patch - one is the configurable bottom area,
> one is the stickiness. please split them up into two patches, the first one
> should go into the general clickpad patch then.

Well, the stickiness is applied only in the button area, so this also
belongs to the clickpad patch.  As mentioned in another mail, I'll merge
this into the previous two clickpad patches as a single patch later.

> For both properties - if the touchpad is not a clickpad the properties
> should not be initialised on the device.

OK.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 09/18] Avoid bogus coord reporting with clickpad devices

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 13:24:53 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:33PM +0200, Takashi Iwai wrote:
> > When a clickpad device without multi-touch kernel support is used,
> > it gives wrong positions and leads to the annoying pointer jumps.
> > Filter the bogus events out in such a case.
> 
> if the events are really bogus, why doesn't the kernel driver filter them?
> 
> if they're not bogus but valid finger data, I need some more information on
> what exactly happens, because this is a bit... odd.
> We have a number of devices that send events outside the min/max range and
> need to handle those, so I'm not sure this approach is the best.

This is a workaround for clickpad dragging working even in ST mode,
thus it becomes obsoleted when MT is enabled.

AFAIK, these strange packets are sent only in ST mode.  It looks like
the device sends another data (maybe no bogus but differently encoded)
for multi-touch.

I kept this patch in my series just for making the driver working even
with unmodified kernel in ST mode.  We can drop it, basically.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 08/18] Avoid unexpected jumps

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 13:13:00 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:32PM +0200, Takashi Iwai wrote:
> > Limit the movement size for avoiding the unexpected pointer jumps.
> 
> what pointer jumps? this patch really needs more explaining on how these
> jumps happen, under what conditions, etc. simply limiting on a fixed set is
> not enough.

This patch is basically for ST mode.  I don't think this is inevitably
needed for MT.

> any solution like this walks the blurry line between dropping coordinates
> and unexpected jumps, and neither is really acceptable (the first one
> definitely isn't). going 1/5 of the touchpad in one movement isn't actually
> hard, my finger is about 1/4 the width of my touchpad, so just moving the
> finger a little bit easily covers more than 1/5 (and yes, that's normal
> movement).
> 
> a driver solution to this that tries to cater for touchpad movement also
> completely ignores pointer acceleration. a user may have the pointer slowed
> down to a point where even 1/2 the size of a touchpad is a normal movement.

OK, the size is arguable.  But a bigger finger size shouldn't be an
issue in this case because a big finger movement is still reported
continuously.

The background of this patch is that the device sometimes doesn't report
the finger off from the pad, instead it sends z with a high value in
the last packet, then stops sending packets.  Thus the driver still
keeps tracking the position as a part of continuous pointer movement
when you touch again at a totally different point, which results in a
jump.

If you have a better idea how to filter out such a case, it'd be great.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 04/18] Add tap-on-LED feature support

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 13:29:48 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:28PM +0200, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai 
> > ---
> >  include/synaptics-properties.h |3 +
> >  man/synaptics.man  |   17 +++
> >  src/properties.c   |   27 +++
> >  src/synaptics.c|   97 
> > +++-
> >  src/synapticsstr.h |6 +++
> >  tools/synclient.c  |1 +
> >  6 files changed, 149 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > index dd7e259..1343e6d 100644
> > --- a/include/synaptics-properties.h
> > +++ b/include/synaptics-properties.h
> > @@ -161,4 +161,7 @@
> >  /* 8 bit (BOOL), led_status (on/off) */
> >  #define SYNAPTICS_PROP_LED_STATUS "Synaptics LED Status"
> >  
> > +/* 8 bit (BOOL), double-tap action on LED corner (on/off) */
> > +#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Dobule Tap"
> > +
> 
> SYNAPTICS_PROP_GESTURES is essentially there for things like this, no need
> for a new property, just append it there.

OK.

> > +void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off)
> > +{
> > +uint8_t val;
> > +
> > +if (!prop_off)
> > +return;
> > +val = off;
> > +XIChangeDeviceProperty(dev, prop_off, XA_INTEGER, 8,
> > +   PropModeReplace, 1, &val, FALSE);
> > +}
> 
> last argument must be TRUE, we need to notify the clients about this change. 

Good catch.

> > +static int
> > +in_led_toggle_area(LocalDevicePtr local, struct SynapticsHwState *hw)
> > +{
> > +SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
> > +int click_led_x, click_led_y;
> > +
> > +click_led_x = (priv->maxx - priv->minx) * LED_TOGGLE_X_AREA + 
> > priv->minx;
> > +click_led_y = (priv->maxy - priv->miny) * LED_TOGGLE_Y_AREA + 
> > priv->miny;
> > +return (hw->x < click_led_x && hw->y < click_led_y);
> > +}
> > +
> > +/* clicpad button toggle point:
> 
> 
> typo

Maybe this word should be removed since it's no longer specific to
clickpad. 


> > +} else
> > +priv->led_tapped = TRUE;
> > +priv->led_tap_millis = hw->millis;
> > +return 0; /* already processed; ignore this finger event */
> > +}
> > +
> 
> two tab indentations in this hunk.

Sorry, this was my default setup of emacs (I'm a kernel programmer, so
tab is preferred).  Will de-tabify later.

> > +if (para->has_led && para->led_double_tap) {
> > +   if (inside_active_area)
> > +   finger = handle_toggle_led(local, hw, finger);
> > +if (para->touchpad_off == 1) {
> > +priv->finger_state = finger;
> > +return delay;
> > +}
> > +}
> > +
> 
> tab/space indentation mixed up. also, are you sure this is what you want
> here? the led only toggled if it's inside the user's active area? Given that
> this is a hardware feature more than a software feature, it should likely be
> triggerable regardless of the area defined, isn't it?

I'm not quite sure why I put it.  It wasn't in my original patch for
older xorg version.  IIRC, it was needed since finger variable is
updated only when inside_active_area is true.


> >  /* tap and drag detection. Needs to be performed even if the finger is 
> > in
> >   * the dead area to reset the state. */
> >  timeleft = HandleTapProcessing(priv, hw, edge, finger, 
> > inside_active_area);
> > diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> > index 88ca9de..05308c1 100644
> > --- a/src/synapticsstr.h
> > +++ b/src/synapticsstr.h
> > @@ -162,6 +162,7 @@ typedef struct _SynapticsParameters
> >  int area_left_edge, area_right_edge, area_top_edge, area_bottom_edge; 
> > /* area coordinates absolute */
> >  Bool has_led;   /* has an embedded LED */
> >  Bool led_status;/* Current status of LED 
> > (1=on) */
> > +Bool led_double_tap;   /* double-tap period in ms for 
> > touchpad LED control */
> 
> 
> spaces only please
> 
> meat of the patch looks good, bar the few comments above, you have my ack.

OK, thanks.


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 03/18] Add the embedded LED support

2010-10-12 Thread Takashi Iwai
At Wed, 13 Oct 2010 13:29:21 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:27PM +0200, Takashi Iwai wrote:
> > This patch adds the control of the embedded LED on the top-left corner
> > of new Synaptics devices.  For LED control, it requires the patch to
> > Linux synaptics input driver,
> > https://patchwork.kernel.org/patch/92434/
> > 
> > When a sysfs file /sys/class/leds/psmouse::synaptics exists, the driver
> > assumes it supports the embeded LED control.
> > 
> > The LED can be controlled via new properties, "Synaptics LED" and
> > "Synaptics LED Status".
> > 
> > Signed-off-by: Takashi Iwai 
> > ---
> >  include/synaptics-properties.h |6 ++
> >  man/synaptics.man  |9 +
> >  src/eventcomm.c|   32 +++-
> >  src/properties.c   |   15 +++
> >  src/synapticsstr.h |2 ++
> >  src/synproto.h |1 +
> >  tools/synclient.c  |1 +
> >  7 files changed, 65 insertions(+), 1 deletions(-)
> > 
> > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > index 9c6a2ee..dd7e259 100644
> > --- a/include/synaptics-properties.h
> > +++ b/include/synaptics-properties.h
> > @@ -155,4 +155,10 @@
> >  /* 32 bit, 4 values, left, right, top, bottom */
> >  #define SYNAPTICS_PROP_AREA "Synaptics Area"
> >  
> > +/* 8 bit (BOOL, read-only), has_led */
> > +#define SYNAPTICS_PROP_LED "Synaptics LED"
> > +
> 
> this should be added to the "Synaptics Capabilities" property, there's no
> need for a separate property.

OK, sounds sensible.

> I guess sooner or later we'll see non-clickpad
> devices come out with leds too, making this a generic capability like
> two-finger tapping and similar.

There are already lots of laptops with an LED but also separate
buttons, indeed.

> > @@ -278,6 +280,9 @@ InitDeviceProperties(InputInfoPtr pInfo)
> >  values[2] = para->area_top_edge;
> >  values[3] = para->area_bottom_edge;
> >  prop_area = InitAtom(pInfo->dev, SYNAPTICS_PROP_AREA, 32, 4, values);
> > +
> > +prop_led = InitAtom(local->dev, SYNAPTICS_PROP_LED, 8, 1, 
> > ¶->has_led);
> > +prop_led_status = InitAtom(local->dev, SYNAPTICS_PROP_LED_STATUS, 8, 
> > 1, ¶->led_status);
> 
> the prop_led_status atom should only be initialized if the touchpad actually
> has a led, please make this conditional on the has_led bit.

So, you mean that prop_led should be NULL when no LED exists, thus
the querying this property would lead to an error?


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 0/2] Synaptics absolute coordinate motion event support

2010-10-12 Thread Takashi Iwai
At Mon, 11 Oct 2010 13:51:35 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 03:20:40PM -0400, Joe Shaw wrote:
> > Attached are a pair of patches which add support for absolute
> > coordinate motion events to the synaptics driver.  The first patch
> > allows Absolute to be passed into XSetDeviceMode() and the latter posts
> > absolute motion events using the raw X and Y coordinates rather than
> > computing deltas.
> 
> I do wonder how many users actually use their touchpad in absolute mode.

It'd be suited well for graffiti or hand-written letter recognition,
for example.  The demand is likely a niche, though.


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 02/18] Add a brief description of Clickpad to man page

2010-10-11 Thread Takashi Iwai
At Tue, 12 Oct 2010 15:27:55 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:26PM +0200, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai 
> > ---
> >  man/synaptics.man |9 +
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> > 
> > diff --git a/man/synaptics.man b/man/synaptics.man
> > index 3f1ca9d..1561e19 100644
> > --- a/man/synaptics.man
> > +++ b/man/synaptics.man
> > @@ -726,6 +726,15 @@ and finger movement distance.
> >  Trackstick mode is exited when the finger pressure drops below
> >  FingerLow or when the finger is moved further than MaxTapMove away
> >  from the initial position.
> > +.
> > +When the input device reports a device with a single left button
> > +and without multi-fingers, the driver assumes it's a Synaptics
> > +Clickpad device and handles it in ClickZone mode.  In this mode,
> > +a left/right/middle button event is generated according to the
> > +position you click.  When Clickpad is enabled, the touchpad area
> > +is shrunk as default in 20% and the bottom area is used as the
> > +click-button area.  The area can be configurable via options or
> > +properties below.
> > 
> >  .SH "DEVICE PROPERTIES"
> >  Synaptics 1.0 and higher support input device properties if the driver is
> > -- 
> > 1.7.3.1
> 
> Please squash this in with the first patch so we have the change in one go.

Agreed.

I'm also thinking to merge the patch 07/18 "Allow touching in clickpad
button area" into a single patch.  With this patch, the clickpad
behavior is changed completely, and it's no big merit to keep the old
good "disable-in-button-area" code.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 05/18] Fix 64bit arch issue in synaptics eventcomm.c

2010-10-11 Thread Takashi Iwai
At Tue, 12 Oct 2010 15:38:24 +1000,
Peter Hutterer wrote:
> 
> On Fri, Oct 08, 2010 at 07:22:29PM +0200, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai 
> > ---
> >  src/eventcomm.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > index 1fc41ef..8a77788 100644
> > --- a/src/eventcomm.c
> > +++ b/src/eventcomm.c
> > @@ -49,7 +49,7 @@
> >  #define NBITS(x) (((x) + LONG_BITS - 1) / LONG_BITS)
> >  #define OFF(x)   ((x) % LONG_BITS)
> >  #define LONG(x)  ((x) / LONG_BITS)
> > -#define TEST_BIT(bit, array) (array[LONG(bit)] & (1 << OFF(bit)))
> > +#define TEST_BIT(bit, array) ((array[LONG(bit)] >> OFF(bit)) & 1)
> >  
> >  #define SYNAPTICS_LED_SYS_FILE 
> > "/sys/class/leds/psmouse::synaptics/brightness"
> >  
> > -- 
> > 1.7.3.1
> 
> I'd prefer the (1UL << OFF(bit)) notation with a != 0, but either way.

I'm fine with it; the compiler should be clever enough.

> Reviewed-by: Peter Hutterer 
> 
> Please add the explanation in your first reply to Mark to the commit message
> though.

Yes, this patch is basically independent from the rest, so safe to
apply alone.  I'll repost later with a proper changelog.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 3/3] Input: synaptics - remove touches over button click area

2010-10-11 Thread Takashi Iwai
At Mon, 11 Oct 2010 12:46:40 -0500,
Chris Bagwell wrote:
> 
> On Mon, Oct 11, 2010 at 12:10 PM, 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
> >>  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 
> >> > ---
> >> >  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++;
> 

Re: [PATCH 08/18] Avoid unexpected jumps

2010-10-11 Thread Takashi Iwai
At Mon, 11 Oct 2010 12:58:55 -0500,
Chris Bagwell wrote:
> 
> On Fri, Oct 8, 2010 at 12:22 PM, Takashi Iwai  wrote:
> > Limit the movement size for avoiding the unexpected pointer jumps.
> >
> 
> Hi Takashi,
> 
> This is type of patch I was concerned with.  Using only 1/5 distance
> of touchpad to discard invalid packets seems pretty low threshold.

True.  (Though, I never got such a large jump with normal operation,
maybe light-speed hand waving is needed :)

> If we can prevent change of ABS_X/ABS_Y only at touch transitions then
> we can make use of following commit to prevent unwanted jumps.
> 
> Basically, its "if (prevFinger != currentFinger) count_packet_finger = 0;".
> 
> http://cgit.freedesktop.org/xorg/driver/xf86-input-synaptics/commit/?id=a6ca4d2523904b7ce49edc29ba408979bdf0d45e

Right.  Similar codes are found in the multi-touch patch part.

This patch (move-threshold) was written just for non-MT cases.
Basically, all patches numbered before multi-touch patch were designed
for non-MT operations, thus some of them are obsolete with MT.


thanks,

Takashi

> 
> Chris
> 
> > Signed-off-by: Takashi Iwai 
> > ---
> >  src/synaptics.c    |   10 ++
> >  src/synapticsstr.h |    1 +
> >  2 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/synaptics.c b/src/synaptics.c
> > index 3ba918a..bd52730 100644
> > --- a/src/synaptics.c
> > +++ b/src/synaptics.c
> > @@ -467,6 +467,8 @@ static void set_default_parameters(InputInfoPtr pInfo)
> >     edgeMotionMaxSpeed = diag * .080;
> >     accelFactor = 200.0 / diag; /* trial-and-error */
> >
> > +    priv->move_ptr_threshold = width / 5;
> > +
> >     range = priv->maxp - priv->minp;
> >
> >     /* scaling based on defaults and a pressure of 256 */
> > @@ -1949,6 +1951,14 @@ ComputeDeltas(SynapticsPrivate *priv, const struct 
> > SynapticsHwState *hw,
> >            break;
> >        }
> >     }
> > +
> > +    if (moving_state && priv->count_packet_finger > 0 &&
> > +       priv->move_ptr_threshold > 0 ) {
> > +       int d = move_distance(HIST(0).x - hw->x, HIST(0).y - hw->y);
> > +       if (d > priv->move_ptr_threshold)
> > +           priv->count_packet_finger = 0; /* to avoid unexpected jumps */
> > +    }
> > +
> >     if (inside_area && moving_state && !priv->palm &&
> >        !priv->vert_scroll_edge_on && !priv->horiz_scroll_edge_on &&
> >        !priv->vert_scroll_twofinger_on && !priv->horiz_scroll_twofinger_on 
> > &&
> > diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> > index 44140f2..44925e5 100644
> > --- a/src/synapticsstr.h
> > +++ b/src/synapticsstr.h
> > @@ -245,6 +245,7 @@ typedef struct _SynapticsPrivateRec
> >     unsigned int clickpad_threshold;
> >     int clickpad_dx, clickpad_dy;
> >     struct SynapticsHwState prev_hw;   /* previous h/w state (for clickpad) 
> > */
> > +    int move_ptr_threshold;
> >     int prop_change_pending;
> >     Bool led_touch_state;
> >     Bool led_tapped;
> > --
> > 1.7.3.1
> >
> > ___
> > xorg-devel@lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: http://lists.x.org/mailman/listinfo/xorg-devel
> >
> 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 3/3] Input: synaptics - remove touches over button click area

2010-10-11 Thread Takashi Iwai
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
> > >  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 
> > > > ---
> > > >  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;
> > > > +
> > > &

Re: [PATCH 3/3] Input: synaptics - remove touches over button click area

2010-10-11 Thread Takashi Iwai
At Mon, 11 Oct 2010 11:24:04 -0500,
Chris Bagwell wrote:
> 
> On Fri, Oct 8, 2010 at 9: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.
> >
> > Signed-off-by: Chase Douglas 
> > ---
> >  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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-11 Thread Takashi Iwai
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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-11 Thread Takashi Iwai
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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-11 Thread Takashi Iwai
At Mon, 11 Oct 2010 08:41:44 -0500,
Chris Bagwell wrote:
> 
> On Mon, Oct 11, 2010 at 2:48 AM, 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? 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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 05/18] Fix 64bit arch issue in synaptics eventcomm.c

2010-10-11 Thread Takashi Iwai
At Mon, 11 Oct 2010 10:24:44 +0200 (CEST),
Mark Kettenis wrote:
> 
> > Date: Mon, 11 Oct 2010 09:47:56 +0200
> > From: Takashi Iwai 
> > 
> > At Fri, 08 Oct 2010 23:09:26 +0200,
> > Henrik Rydberg wrote:
> > > 
> > > On 10/08/2010 10:51 PM, Mark Kettenis wrote:
> > > 
> > > >> Date: Fri, 08 Oct 2010 21:36:02 +0200
> > > >> From: Takashi Iwai 
> > > >>
> > > >> At Fri, 8 Oct 2010 20:48:10 +0200 (CEST),
> > > >> Mark Kettenis wrote:
> > > >>>
> > > >>>> From: Takashi Iwai 
> > > >>>> Date: Fri,  8 Oct 2010 19:22:29 +0200
> > > >>>
> > > >>> Sorry, but it isn't obvious to me what issue this fixes.
> > > >>
> > > >> In C, "1" is an integer, not an unsigned long.
> > > >> Thus (1 << 33) doesn't give you the 33th bit shift, but it's undefined.
> > > > 
> > > > But if you'd actually use 33 (or event 32) as an offset, things
> > > > wouldn't work on a 32-bit platform would they?
> > > 
> > > 
> > > I believe this is what the patch is supposed to fix.
> > 
> > Yep :)
> > 
> > 
> > > > Anyway,
> > > > 
> > > >> If any, this must be (1UL << 32). 
> > > > 
> > > > This is the idiom that is much more common.  I probably wouldn't have
> > > > questioned things if you'd written it like that.  I recommend you to
> > > > stick to this version.
> > > 
> > > 
> > > For a counter-example, see the definition of test_bit() in the Linux 
> > > kernel.
> > > 
> > > >> Also, it'd be better if such a test macro returns 1 instead of a
> > > >> random non-zero value.  So does my patch.
> > > > 
> > > > In C this doesn't matter.
> > > 
> > > 
> > > It is still useful to know if a logic expression evaluates to (0, 1) or 
> > > (0,
> > > nonzero).
> > 
> > It does matter if the returned type is bigger.
> > Try an example code below on 32bit and 64bit machines:
> > 
> > 
> > #include 
> > 
> > #define LONG_BITS (sizeof(long) * 8)
> > #define OFF(x)   ((x) % LONG_BITS)
> > #define LONG(x)  ((x) / LONG_BITS)
> > 
> > #define TEST_BIT1(bit, array) (array[LONG(bit)] & (1 << OFF(bit)))
> > #define TEST_BIT2(bit, array) (array[LONG(bit)] & (1UL << OFF(bit)))
> > #define TEST_BIT3(bit, array) ((array[LONG(bit)] >> OFF(bit)) & 1)
> > 
> > static unsigned long test[4];
> > 
> > int main()
> > {
> > const int bit = 33;
> > int result;
> > 
> > /* set the bit */
> > test[LONG(bit)] |= 1UL << OFF(bit);
> > 
> > result = TEST_BIT1(bit, test);
> > printf("TEST1 %d = %d\n", bit, result);
> > 
> > result = TEST_BIT2(bit, test);
> > printf("TEST2 %d = %d\n", bit, result);
> > 
> > result = TEST_BIT3(bit, test);
> > printf("TEST3 %d = %d\n", bit, result);
> > 
> > return 0;
> > }
> > 
> > 
> > On a 32bit machine, it results in:
> >   TEST1 33 = 2
> >   TEST2 33 = 2
> >   TEST3 33 = 1
> > 
> > while on a 64bit machine, it results in:
> >   TEST1 33 = 0
> >   TEST2 33 = 0
> >   TEST3 33 = 1
> > 
> > See TEST2 still failed although it uses 1UL.
> 
> This example is meaningless.  On every reasonable 32-bit architecture,
> long is a 32-bit integer.  So
> 
> > /* set the bit */
> > test[LONG(bit)] |= 1UL << OFF(bit);
> 
> when bit is 33 (or even 32), is completely undefined on a 32-bit
> machine.  Therefore, you should never call TEST_BIT() with a first
> argument of 32 or larger, even on a 64-bit architecture, since the
> code wouldn't work on a 32-bit machine.

Hm?  TEST_BIT() is used for an long *array*, with the unlimited size.
Otherwise the macro should have been more simplified :)
Ditto for the above code.  It stores the value in an array member.
That's why LONG() and OFF() macros are used there.

> One can even argue that for this reason the change you're suggesting
> is dangerous, since people writing code on a 64-bit machine might not
> notice that they're writing coding that won't run on a 32-bit machine.

I wrote the patch because the detection of MT_* bits don't work on 64
bit machines actually.  So the current code _is_ just broken.


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-11 Thread Takashi Iwai
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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 01/18] Add Clickpad support

2010-10-11 Thread Takashi Iwai
At Sat, 09 Oct 2010 09:29:44 +0200,
Henrik Rydberg wrote:
> 
> Hi Takashi,
> 
> > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > index 85dfd09..fc5055b 100644
> > --- a/src/eventcomm.c
> > +++ b/src/eventcomm.c
> > @@ -269,6 +269,12 @@ event_query_axis_ranges(InputInfoPtr pInfo)
> > }
> >  
> > xf86Msg(X_PROBED, "%s: buttons:%s\n", pInfo->name, buf);
> > +
> > +   /* clickpad device reports only the single left button mask */
> > +   if (priv->has_left && !priv->has_right && !priv->has_middle && 
> > !priv->has_double) {
> > +   priv->is_clickpad = TRUE;
> > +   xf86Msg(X_PROBED, "%s: is Clickpad device\n", local->name);
> > +   }
> >  }
> >  }
> 
> 
> The variable has_double is true when the kernel reports multiple fingers via 
> the
> BTN_*TAP keys. This is the case for the macbook pads, and I would imagine it 
> to
> be true also for the clickpad soon. The clicking-on-lower-portion-of-pad is
> interesting for both devices. In other words, perhaps the logic should be
> reiterated?

Right, the logic in *this* patch is incomplete.  The patch series are
queued in the historical order, so it doesn't contain any MT things.
The clickpad detection with MT enablement appears in the later patch.

But, the clickpad detection in the later patch is still questionable,
and I personally don't like that.  It'd be really better if the
clickpad capability bit is exposed somehow from the driver
directly...


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-11 Thread Takashi Iwai
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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 18/18] Make action strings configurable via synclient

2010-10-11 Thread Takashi Iwai
At Sat, 09 Oct 2010 17:40:52 +0200,
walter harms wrote:
> 
> 
> 
> Takashi Iwai schrieb:
> > Signed-off-by: Takashi Iwai 
> > ---
> >  include/synaptics-properties.h |   10 ++
> >  src/keymap.c   |   58 ++--
> >  src/properties.c   |   63 
> > 
> >  src/synaptics.c|   60 ++---
> >  src/synapticsstr.h |   28 +-
> >  tools/synclient.c  |   49 --
> >  6 files changed, 203 insertions(+), 65 deletions(-)
> > 
> > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > index c31c53c..7267669 100644
> > --- a/include/synaptics-properties.h
> > +++ b/include/synaptics-properties.h
> > @@ -185,4 +185,14 @@
> >  /* 32 bit, 2 values */
> >  #define SYNAPTICS_PROP_THREEFINGER_DELTA "Synaptics Three-Finger Delta"
> >  
> > +/* STR */
> > +#define SYNAPTICS_PROP_ZOOM_IN_ACTION "Synaptics Zoom-In Action"
> > +#define SYNAPTICS_PROP_ZOOM_OUT_ACTION "Synaptics Zoom-Out Action"
> > +
> > +/* STR */
> > +#define SYNAPTICS_PROP_3FINGER_LEFT_ACTION "Synaptics 3-Finger Left Action"
> > +#define SYNAPTICS_PROP_3FINGER_RIGHT_ACTION "Synaptics 3-Finger Right 
> > Action"
> > +#define SYNAPTICS_PROP_3FINGER_UP_ACTION "Synaptics 3-Finger Up Action"
> > +#define SYNAPTICS_PROP_3FINGER_DOWN_ACTION "Synaptics 3-Finger Down Action"
> > +
> >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > diff --git a/src/keymap.c b/src/keymap.c
> > index b1e7cc6..0c365a2 100644
> > --- a/src/keymap.c
> > +++ b/src/keymap.c
> > @@ -142,15 +142,30 @@ static int string_to_keysym(const char *str)
> >  }
> >  
> >  int SynapticsParseActionStr(LocalDevicePtr local, const char *_str,
> > -   int *action, int max_actions)
> > +   SynapticsAction *action)
> >  {
> >  char *item;
> >  char *str, *next;
> > -int num_actions = 0;
> >  
> > -str = xstrdup(_str);
> > +if (action->str) {
> > +   if (_str && !strcmp(action->str, _str))
> > +   return TRUE;
> > +   free(action->str);
> > +   action->str = NULL;
> > +}
> > +
> > +action->num_actions = 0;
> > +if (!_str)
> > +   return TRUE;
> > +
> > +str = strdup(_str);
> >  if (!str)
> > -   return 0;
> > +   return FALSE;
> > +action->str = strdup(_str);
> > +if (!action->str) {
> > +   free(str);
> > +   return FALSE;
> > +}
> >  for (item = str; item && *item; item = next) {
> > int button, keysym, keycode;
> >  
> > @@ -160,7 +175,8 @@ int SynapticsParseActionStr(LocalDevicePtr local, const 
> > char *_str,
> > if (!*item)
> > continue;
> > if (sscanf(item, "Button%d", &button) == 1) {
> > -   action[num_actions++] = (ACTION_BUTTON << 16) | button;
> > +   action->action[action->num_actions++] =
> > +   (ACTION_BUTTON << 16) | button;
> > } else {
> > keysym = string_to_keysym(item);
> > if (keysym == NoSymbol) {
> > @@ -175,15 +191,17 @@ int SynapticsParseActionStr(LocalDevicePtr local, 
> > const char *_str,
> > continue;
> > }
> > if (get_modifier(keysym))
> > -   action[num_actions++] = (ACTION_KEYMOD << 16) | keycode;
> > +   action->action[action->num_actions++] =
> > +   (ACTION_KEYMOD << 16) | keycode;
> > else
> > -   action[num_actions++] = (ACTION_KEY << 16) | keycode;
> > +   action->action[action->num_actions++] =
> > +   (ACTION_KEY << 16) | keycode;
> > }
> > -   if (num_actions >= max_actions)
> > +   if (action->num_actions >= MAX_ACTIONS)
> > break;
> >  }
> >  free(str);
> > -return num_actions;
> > +return TRUE;
> >  }
> >  
> >  static void
> > @@ -196,13 +214,13 @@ Bool SynapticsInitKeyboard(DeviceIntPtr dev)
> >  return InitKeyboardDeviceStruct(dev, NULL, NULL, synaptics_kbdctrl);
> >  }
> >  
> > -void SynapticsSendAction(LocalDevicePtr local, int num_actions, int 
> > *action)
> > +void SynapticsSendAction(LocalDevicePtr local, Syn

Re: [PATCH 05/18] Fix 64bit arch issue in synaptics eventcomm.c

2010-10-11 Thread Takashi Iwai
At Fri, 08 Oct 2010 23:09:26 +0200,
Henrik Rydberg wrote:
> 
> On 10/08/2010 10:51 PM, Mark Kettenis wrote:
> 
> >> Date: Fri, 08 Oct 2010 21:36:02 +0200
> >> From: Takashi Iwai 
> >>
> >> At Fri, 8 Oct 2010 20:48:10 +0200 (CEST),
> >> Mark Kettenis wrote:
> >>>
> >>>> From: Takashi Iwai 
> >>>> Date: Fri,  8 Oct 2010 19:22:29 +0200
> >>>
> >>> Sorry, but it isn't obvious to me what issue this fixes.
> >>
> >> In C, "1" is an integer, not an unsigned long.
> >> Thus (1 << 33) doesn't give you the 33th bit shift, but it's undefined.
> > 
> > But if you'd actually use 33 (or event 32) as an offset, things
> > wouldn't work on a 32-bit platform would they?
> 
> 
> I believe this is what the patch is supposed to fix.

Yep :)


> > Anyway,
> > 
> >> If any, this must be (1UL << 32). 
> > 
> > This is the idiom that is much more common.  I probably wouldn't have
> > questioned things if you'd written it like that.  I recommend you to
> > stick to this version.
> 
> 
> For a counter-example, see the definition of test_bit() in the Linux kernel.
> 
> >> Also, it'd be better if such a test macro returns 1 instead of a
> >> random non-zero value.  So does my patch.
> > 
> > In C this doesn't matter.
> 
> 
> It is still useful to know if a logic expression evaluates to (0, 1) or (0,
> nonzero).

It does matter if the returned type is bigger.
Try an example code below on 32bit and 64bit machines:


#include 

#define LONG_BITS (sizeof(long) * 8)
#define OFF(x)   ((x) % LONG_BITS)
#define LONG(x)  ((x) / LONG_BITS)

#define TEST_BIT1(bit, array) (array[LONG(bit)] & (1 << OFF(bit)))
#define TEST_BIT2(bit, array) (array[LONG(bit)] & (1UL << OFF(bit)))
#define TEST_BIT3(bit, array) ((array[LONG(bit)] >> OFF(bit)) & 1)

static unsigned long test[4];

int main()
{
const int bit = 33;
int result;

/* set the bit */
test[LONG(bit)] |= 1UL << OFF(bit);

result = TEST_BIT1(bit, test);
printf("TEST1 %d = %d\n", bit, result);

result = TEST_BIT2(bit, test);
printf("TEST2 %d = %d\n", bit, result);

result = TEST_BIT3(bit, test);
printf("TEST3 %d = %d\n", bit, result);

return 0;
}


On a 32bit machine, it results in:
  TEST1 33 = 2
  TEST2 33 = 2
  TEST3 33 = 1

while on a 64bit machine, it results in:
  TEST1 33 = 0
  TEST2 33 = 0
  TEST3 33 = 1

See TEST2 still failed although it uses 1UL.


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-08 Thread Takashi Iwai
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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 05/18] Fix 64bit arch issue in synaptics eventcomm.c

2010-10-08 Thread Takashi Iwai
At Fri, 8 Oct 2010 20:48:10 +0200 (CEST),
Mark Kettenis wrote:
> 
> > From: Takashi Iwai 
> > Date: Fri,  8 Oct 2010 19:22:29 +0200
> 
> Sorry, but it isn't obvious to me what issue this fixes.

In C, "1" is an integer, not an unsigned long.
Thus (1 << 33) doesn't give you the 33th bit shift, but it's undefined.
If any, this must be (1UL << 32). 

Also, it'd be better if such a test macro returns 1 instead of a
random non-zero value.  So does my patch.

(Yeah, I know the changelogs are missing in most of patches;
 as mentioned, it's just flushing of initial patches ;)
 I'll rewrite and resubmit after reviews.)


Takashi

> >  src/eventcomm.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > index 1fc41ef..8a77788 100644
> > --- a/src/eventcomm.c
> > +++ b/src/eventcomm.c
> > @@ -49,7 +49,7 @@
> >  #define NBITS(x) (((x) + LONG_BITS - 1) / LONG_BITS)
> >  #define OFF(x)   ((x) % LONG_BITS)
> >  #define LONG(x)  ((x) / LONG_BITS)
> > -#define TEST_BIT(bit, array) (array[LONG(bit)] & (1 << OFF(bit)))
> > +#define TEST_BIT(bit, array) ((array[LONG(bit)] >> OFF(bit)) & 1)
> >  
> >  #define SYNAPTICS_LED_SYS_FILE 
> > "/sys/class/leds/psmouse::synaptics/brightness"
> >  
> 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-08 Thread Takashi Iwai
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 
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 

---
 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

Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-08 Thread Takashi Iwai
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 touc

[PATCH 12/18] Add pinch gesture support

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |3 +
 src/Makefile.am|3 +-
 src/keymap.c   |  227 
 src/properties.c   |   13 +++
 src/synaptics.c|  105 ++
 src/synapticsstr.h |   21 -
 tools/synclient.c  |2 +
 7 files changed, 372 insertions(+), 2 deletions(-)
 create mode 100644 src/keymap.c

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index 47f6bb1..7112bc0 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -170,4 +170,7 @@
 /* 8 bit (BOOL), double-tap action on LED corner (on/off) */
 #define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Dobule Tap"
 
+/* 32 bit, 2 values, start, delta (0 disable) */
+#define SYNAPTICS_PROP_MULTI_TOUCH_PINCH "Synaptics Multi-Touch Pinch"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/src/Makefile.am b/src/Makefile.am
index 980ab5e..625fea6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -35,7 +35,8 @@ AM_CFLAGS = $(XORG_CFLAGS)
alpscomm.c alpscomm.h \
ps2comm.c ps2comm.h \
synproto.h \
-   properties.c
+   properties.c \
+   keymap.c
 
 if BUILD_EVENTCOMM
 @driver_n...@_drv_la_sources += \
diff --git a/src/keymap.c b/src/keymap.c
new file mode 100644
index 000..b1e7cc6
--- /dev/null
+++ b/src/keymap.c
@@ -0,0 +1,227 @@
+/*
+ * Minimal keyboard support; mostly copied from other drivers
+ *
+ * Copyright (c) 2010 Takashi Iwai
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of Red Hat
+ * not be used in advertising or publicity pertaining to distribution
+ * of the software without specific, written prior permission.  Red
+ * Hat makes no representations about the suitability of this software
+ * for any purpose.  It is provided "as is" without express or implied
+ * warranty.
+ *
+ * THE AUTHORS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include "synaptics.h"
+#include "synapticsstr.h"
+
+#define ARRAY_SIZE(x)  (sizeof(x) / sizeof((x)[0]))
+
+static int get_keycode(LocalDevicePtr local, int keysym)
+{
+KeySymsPtr ksr = XkbGetCoreMap(local->dev);
+int c;
+
+for (c = ksr->minKeyCode; c <= ksr->maxKeyCode; c++) {
+   if (ksr->map[(c - ksr->minKeyCode) * ksr->mapWidth] == keysym)
+   break;
+}
+if (c > ksr->maxKeyCode)
+   c = -1;
+free(ksr);
+return c;
+}
+
+static int get_modifier(int keysym)
+{
+static struct { KeySym keysym; CARD8 mask; } modifiers[] = {
+{ XK_Shift_L,   ShiftMask },
+{ XK_Shift_R,   ShiftMask },
+{ XK_Control_L, ControlMask },
+{ XK_Control_R, ControlMask },
+{ XK_Caps_Lock, LockMask },
+{ XK_Alt_L, Mod1Mask },
+{ XK_Alt_R, Mod1Mask },
+{ XK_Num_Lock,  Mod2Mask },
+{ XK_Mode_switch,   Mod3Mask },
+{ XK_Scroll_Lock,   Mod5Mask },
+};
+int i;
+
+for (i = 0; i < ARRAY_SIZE(modifiers); i++) {
+   if (modifiers[i].keysym == keysym)
+   return modifiers[i].mask;
+}
+return 0;
+}
+
+static struct keysym_table {
+const char *str;
+int keysym;
+} keysym_ref[] = {
+{ "Control", XK_Control_L },
+{ "Shift", XK_Shift_L },
+{ "Alt", XK_Alt_L },
+{ "Mode_switch", XK_Mode_switch },
+{ "space", XK_space },
+{ "quotedbl", XK_quotedbl },
+{ "numbersign", XK_numbersign },
+{ "dollar", XK_dollar},
+{ "percent", XK_percent },
+{ "apostrophe", XK_apostrophe },
+{ "plus", XK_plus },
+{ "comma", XK_comma },
+{ "minus", XK_minus },
+{ "backslash", XK_backslash },
+{ "F1", XK_F1 },
+{ "F2", XK_F2 },
+{ "F3", XK_F3 },
+{ "F4",

[PATCH 07/18] Allow touching in clickpad button area

2010-10-08 Thread Takashi Iwai
Instead of ignoring the mouse touch sense in the clickpad button area,
introduce a "stickiness" to the movement for blocking the unexpected
movement of the driver at clicking.

Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |6 ++
 man/synaptics.man  |   30 +++-
 src/properties.c   |   15 
 src/synaptics.c|  156 +---
 src/synapticsstr.h |5 ++
 tools/synclient.c  |2 +
 6 files changed, 169 insertions(+), 45 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index 1343e6d..47f6bb1 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -155,6 +155,12 @@
 /* 32 bit, 4 values, left, right, top, bottom */
 #define SYNAPTICS_PROP_AREA "Synaptics Area"
 
+/* 32bit */
+#define SYNAPTICS_PROP_TOUCH_BUTTON_AREA "Synaptics Touch Button Area"
+
+/* 32bit */
+#define SYNAPTICS_PROP_TOUCH_BUTTON_STICKY "Synapyics Touch Button Sticky"
+
 /* 8 bit (BOOL, read-only), has_led */
 #define SYNAPTICS_PROP_LED "Synaptics LED"
 
diff --git a/man/synaptics.man b/man/synaptics.man
index d8afa1a..6d4a886 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -554,6 +554,21 @@ A "touch" event happens when the Z value goes above 
FingerHigh, and an
 "untouch" event happens when the Z value goes below FingerLow.
 .
 .TP
+.BI "Option \*qTouchButtonArea\*q \*q" integer \*q
+.
+Provides the size of the click-button area for ClickPad devices in
+percent. As default, it's 20.
+Property: "Synaptics Touch Button Area"
+.
+.TP
+.BI "Option \*qTouchButtonSticky\*q \*q" integer \*q
+.
+Provides the threshold to start moving in the click-button area.  The
+higher value is set, the pointer becomes sticky in the click-button
+area. The default value is 64.
+Property: "Synaptics Touch Button Sticky"
+.
+.TP
 .BI "Option \*qLEDDoubleTap\*q \*q" boolean \*q
 .
 Enables/disables the touchpad-control by double-tapping on the top-left
@@ -744,10 +759,9 @@ When the input device reports a device with a single left 
button
 and without multi-fingers, the driver assumes it's a Synaptics
 Clickpad device and handles it in ClickZone mode.  In this mode,
 a left/right/middle button event is generated according to the
-position you click.  When Clickpad is enabled, the touchpad area
-is shrunk as default in 20% and the bottom area is used as the
-click-button area.  The area can be configurable via options or
-properties below.
+position you click.  When Clickpad is enabled, the bottom area (as
+default 20%) is used as the click-button area.  The size of the area
+is configurable via options or properties below.
 
 .SH "DEVICE PROPERTIES"
 Synaptics 1.0 and higher support input device properties if the driver is
@@ -923,6 +937,14 @@ right button, two-finger detection, three-finger 
detection, pressure detection,
 32 bit unsigned, 2 values (read-only), vertical, horizontal in 
units/millimeter.
 
 .TP 7
+.BI "Synaptics Touch Button Area"
+32 bit.
+
+.TP 7
+.BI "Synaptics Touch Button Sticky"
+32 bit.
+
+.TP 7
 .BI "Synaptics LED"
 8 bit (BOOL, read-only), indicating whether the device has an embedded
 LED support or not.
diff --git a/src/properties.c b/src/properties.c
index 6862a09..4042edc 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -82,6 +82,8 @@ Atom prop_gestures  = 0;
 Atom prop_capabilities  = 0;
 Atom prop_resolution= 0;
 Atom prop_area  = 0;
+Atom prop_touch_button_area = 0;
+Atom prop_touch_button_sticky   = 0;
 Atom prop_led   = 0;
 Atom prop_led_status= 0;
 Atom prop_led_double_tap= 0;
@@ -282,6 +284,9 @@ InitDeviceProperties(InputInfoPtr pInfo)
 values[3] = para->area_bottom_edge;
 prop_area = InitAtom(pInfo->dev, SYNAPTICS_PROP_AREA, 32, 4, values);
 
+prop_touch_button_area = InitAtom(local->dev, 
SYNAPTICS_PROP_TOUCH_BUTTON_AREA, 32, 1, ¶->touch_button_area);
+prop_touch_button_sticky = InitAtom(local->dev, 
SYNAPTICS_PROP_TOUCH_BUTTON_STICKY, 32, 1, ¶->touch_button_sticky);
+
 prop_led = InitAtom(local->dev, SYNAPTICS_PROP_LED, 8, 1, ¶->has_led);
 prop_led_status = InitAtom(local->dev, SYNAPTICS_PROP_LED_STATUS, 8, 1, 
¶->led_status);
 
@@ -671,6 +676,16 @@ SetProperty(DeviceIntPtr dev, Atom property, 
XIPropertyValuePtr prop,
 para->area_right_edge  = area[1];
 para->area_top_edge= area[2];
 para->area_bottom_edge = area[3];
+} else if (property == prop_touch_button_area) {
+if (prop->size != 1 || prop->format != 32 || prop->type != XA_INTEGER)
+return BadMatch;
+
+   para->touch_button_area = *(INT32*)prop->data;
+} else if (property 

[PATCH 15/18] Add rotation support for synaptics driver

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |3 +++
 src/properties.c   |8 
 src/synaptics.c|   35 +++
 src/synapticsstr.h |1 +
 tools/synclient.c  |1 +
 5 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index 5d440be..f6fbfac 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -179,4 +179,7 @@
 /* 32bit, read-only */
 #define SYNAPTICS_PROP_GESTURE_MODE "Current Gesture Mode"
 
+/* 32 bit */
+#define SYNAPTICS_PROP_ORIENTATION "Synaptics Orientation"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/src/properties.c b/src/properties.c
index 7788a13..7278560 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -46,6 +46,7 @@ static Atom float_type;
 
 Atom prop_edges = 0;
 Atom prop_finger= 0;
+Atom prop_orientation   = 0;
 Atom prop_tap_time  = 0;
 Atom prop_tap_move  = 0;
 Atom prop_tap_durations = 0;
@@ -263,6 +264,8 @@ InitDeviceProperties(InputInfoPtr pInfo)
 
 prop_pressuremotion_factor = InitFloatAtom(pInfo->dev, 
SYNAPTICS_PROP_PRESSURE_MOTION_FACTOR, 2, fvalues);
 
+prop_orientation = InitAtom(local->dev, SYNAPTICS_PROP_ORIENTATION, 32, 1, 
¶->orientation);
+
 prop_grab = InitAtom(pInfo->dev, SYNAPTICS_PROP_GRAB, 8, 1, 
¶->grab_event_device);
 
 values[0] = para->tap_and_drag_gesture;
@@ -674,6 +677,11 @@ SetProperty(DeviceIntPtr dev, Atom property, 
XIPropertyValuePtr prop,
 
 para->press_motion_min_z = press[0];
 para->press_motion_max_z = press[1];
+} else if (property == prop_orientation) {
+if (prop->size != 1 || prop->format != 32 || prop->type != XA_INTEGER)
+return BadMatch;
+
+   para->orientation = *(INT32*)prop->data;
 } else if (property == prop_grab)
 {
 if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
diff --git a/src/synaptics.c b/src/synaptics.c
index 2f264f0..71a5134 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -600,6 +600,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
 pars->coasting_friction = xf86SetRealOption(opts, "CoastingFriction", 50);
 pars->press_motion_min_factor = xf86SetRealOption(opts, 
"PressureMotionMinFactor", 1.0);
 pars->press_motion_max_factor = xf86SetRealOption(opts, 
"PressureMotionMaxFactor", 1.0);
+pars->orientation = xf86SetIntOption(opts, "Orientation", 0);
 pars->grab_event_device = xf86SetBoolOption(opts, "GrabEventDevice", TRUE);
 pars->tap_and_drag_gesture = xf86SetBoolOption(opts, "TapAndDragGesture", 
TRUE);
 pars->resolution_horiz = xf86SetIntOption(opts, "HorizResolution", 
horizResolution);
@@ -2818,6 +2819,38 @@ repeat_scrollbuttons(const InputInfoPtr pInfo,
 return delay;
 }
 
+static void do_rotation(SynapticsPrivate *priv, int orientation, int *xp, int 
*yp)
+{
+int width = priv->maxx - priv->minx;
+int height = priv->maxy - priv->miny;
+int x = *xp;
+int y = *yp;
+
+switch (orientation) {
+case 1:
+   *xp = (priv->maxy - y) * width / height + priv->minx;
+   *yp = (x - priv->minx) * height / width + priv->miny;
+   break;
+case 2:
+   *xp = priv->maxx + priv->minx - x;
+   *yp = priv->maxy + priv->miny - y;
+   break;
+case 3:
+   *xp = (y - priv->miny) * width / height + priv->minx;
+   *yp = (priv->maxx - x) * height / width + priv->miny;
+   break;
+}
+}
+
+static void update_orientation(SynapticsPrivate *priv,
+  struct SynapticsHwState *hw)
+{
+SynapticsParameters *para = &priv->synpara;
+do_rotation(priv, para->orientation, &hw->x, &hw->y);
+if (hw->multi_touch > 1)
+   do_rotation(priv, para->orientation, &hw->multi_touch_x, 
&hw->multi_touch_y);
+}
+
 /*
  * React on changes in the hardware state. This function is called every time
  * the hardware state changes. The return value is used to specify how many
@@ -2843,6 +2876,8 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState 
*hw)
 
 prev_gesture_mode = priv->gesture_mode;
 
+update_orientation(priv, hw);
+
 update_multi_touch(priv, hw);
 
 update_shm(pInfo, hw);
diff --git a/src/synapticsstr.h b/src/synapticsstr.h
index 285c8f3..e972a42 100644
--- a/src/synapticsstr.h
+++ b/src/synapticsstr.h
@@ -101,6 +101,7 @@ typedef struct _SynapticsParameters
 {
 /* Parameter data */
 int left_edge, right_edge, top_edge, bottom_edge; /* edge coordinates 
absolute */
+int orientation;
 int finger_low, finger_high, finger_pr

[PATCH 17/18] Add three-finger gesture support

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |3 +
 src/properties.c   |   15 ++
 src/synaptics.c|   98 +++-
 src/synapticsstr.h |   17 +++-
 tools/synclient.c  |2 +
 5 files changed, 133 insertions(+), 2 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index f6fbfac..c31c53c 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -182,4 +182,7 @@
 /* 32 bit */
 #define SYNAPTICS_PROP_ORIENTATION "Synaptics Orientation"
 
+/* 32 bit, 2 values */
+#define SYNAPTICS_PROP_THREEFINGER_DELTA "Synaptics Three-Finger Delta"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/src/properties.c b/src/properties.c
index 7278560..9977b28 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -91,6 +91,7 @@ Atom prop_led_double_tap= 0;
 Atom prop_multi_touch_pinch = 0;
 Atom prop_gesture_mode_notify   = 0;
 Atom prop_gesture_mode  = 0;
+Atom prop_threefinger   = 0;
 
 static Atom
 InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
@@ -306,6 +307,10 @@ InitDeviceProperties(InputInfoPtr pInfo)
 
 values[0] = 0;
 prop_gesture_mode = InitAtom(local->dev, SYNAPTICS_PROP_GESTURE_MODE, 32, 
1, values);
+
+values[0] =  para->threefinger_delta_horiz;
+values[1] =  para->threefinger_delta_vert;
+prop_threefinger = InitAtom(local->dev, SYNAPTICS_PROP_THREEFINGER_DELTA, 
32, 2, values);
 }
 
 int
@@ -730,6 +735,15 @@ SetProperty(DeviceIntPtr dev, Atom property, 
XIPropertyValuePtr prop,
 if (priv->proto_ops && priv->proto_ops->UpdateLED)
 priv->proto_ops->UpdateLED(local);
 }
+} else if (property == prop_threefinger)
+{
+INT32 *delta;
+if (prop->size != 2 || prop->format != 32 || prop->type != XA_INTEGER)
+return BadMatch;
+
+delta = (INT32*)prop->data;
+   para->threefinger_delta_horiz = delta[0];
+   para->threefinger_delta_vert = delta[1];
 }
 
 return Success;
@@ -757,6 +771,7 @@ void SynapticsSetGestureModeProperty(DeviceIntPtr dev, int 
mode)
[GESTURE_HSCROLL] = 11,
[GESTURE_VSCROLL] = 12,
[GESTURE_CIRCULAR] = 13,
+   [GESTURE_3FINGER] = 15,
 };
 
 if (!prop_gesture_mode || !para->gesture_mode_notify)
diff --git a/src/synaptics.c b/src/synaptics.c
index 71a5134..747bdb9 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -612,6 +612,9 @@ static void set_default_parameters(InputInfoPtr pInfo)
 pars->multi_touch_pinch_dist = xf86SetIntOption(opts, 
"MultiTouchPinchDelta", pinchDelta);
 pars->gesture_mode_notify = xf86SetBoolOption(opts, "GestureModeNotify", 
TRUE);
 
+pars->threefinger_delta_horiz = xf86SetIntOption(opts, 
"HorizThreeFingerDelta", 750);
+pars->threefinger_delta_vert = xf86SetIntOption(opts, 
"VertThreeFingerDelta", 750);
+
 /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge 
parameters */
 if (pars->top_edge > pars->bottom_edge) {
int tmp = pars->top_edge;
@@ -827,6 +830,26 @@ static void setup_zoom_actions(LocalDevicePtr local)
action = "Control+Button5";
 priv->zoom_out_num_actions =
SynapticsParseActionStr(local, action, priv->zoom_out_action, 
MAX_ACTIONS);
+action = xf86FindOptionValue(local->options, "ThreeFingerLeftAction");
+if (!action)
+   action = "Alt+Left";
+priv->threefinger_left_num_actions =
+   SynapticsParseActionStr(local, action, priv->threefinger_left_action, 
MAX_ACTIONS);
+action = xf86FindOptionValue(local->options, "ThreeFingerRightAction");
+if (!action)
+   action = "Alt+Right";
+priv->threefinger_right_num_actions =
+   SynapticsParseActionStr(local, action, priv->threefinger_right_action, 
MAX_ACTIONS);
+action = xf86FindOptionValue(local->options, "ThreeFingerUpAction");
+if (!action)
+   action = "Control+Tab";
+priv->threefinger_up_num_actions =
+   SynapticsParseActionStr(local, action, priv->threefinger_up_action, 
MAX_ACTIONS);
+action = xf86FindOptionValue(local->options, "ThreeFingerDownAction");
+if (!action)
+   action = "Control+Shift+Tab";
+priv->threefinger_down_num_actions =
+   SynapticsParseActionStr(local, action, priv->threefinger_down_action, 
MAX_ACTIONS);
 }
 
 /*
@@ -2538,6 +2561,76 @@ handle_multi_touch_pinch(SynapticsPrivate *priv, struct 
SynapticsHwState *hw,
 }
 
 static void
+handle_three_finger_action(SynapticsPrivate *priv, struct SynapticsHwState *hw,
+  int *xdelta, int *ydelta)
+{
+SynapticsPar

[PATCH 16/18] Add synxrrd daemon

2010-10-08 Thread Takashi Iwai
This is a daemon program to sync Xrandr rotation with the synaptics device.

Signed-off-by: Takashi Iwai 
---
 tools/Makefile.am |5 +-
 tools/synxrrd.c   |  186 +
 2 files changed, 190 insertions(+), 1 deletions(-)
 create mode 100644 tools/synxrrd.c

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 2ad48f6..576 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -18,7 +18,7 @@
 #  IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
 #  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-bin_PROGRAMS = synclient syndaemon
+bin_PROGRAMS = synclient syndaemon synxrrd
 
 AM_CPPFLAGS = -I$(top_srcdir)/include -I$(sdkdir)
 AM_CFLAGS = $(XI_CFLAGS)
@@ -30,3 +30,6 @@ syndaemon_SOURCES = syndaemon.c
 syndaemon_CFLAGS = $(AM_CFLAGS) $(XTST_CFLAGS)
 syndaemon_LDFLAGS = $(AM_LDFLAGS) $(XTST_LIBS)
 
+synxrrd_SOURCES = synxrrd.c
+synxrrd_LDFLAGS = -lXrandr $(XLIB_LIBS) $(XI_LIBS)
+
diff --git a/tools/synxrrd.c b/tools/synxrrd.c
new file mode 100644
index 000..9f00e5b
--- /dev/null
+++ b/tools/synxrrd.c
@@ -0,0 +1,186 @@
+/*
+ * synxrrd.c
+ * Copyright (C) 2010 Takashi Iwai 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define progname   "synxrrd"
+
+static int verbose;
+static const char *laptop_name;
+
+static int is_laptop(const char *name)
+{
+   if (laptop_name) {
+   if (!strcmp(name, laptop_name))
+   return 1;
+   } else {
+   if (strstr(name, "LVDS") ||
+   strstr(name, "Lvds") ||
+   strstr(name, "LVDs"))
+   return 1;
+   if (strstr(name, "LCD"))
+   return 1;
+   }
+   return 0;
+}
+
+static int current_rotation = -1; /* unknown */
+
+static int (*xerror_saved)(Display *, XErrorEvent *);
+static int xerror_code;
+
+static int xerror_ignore(Display *dpy, XErrorEvent *err)
+{
+   xerror_code = err->error_code;
+   return 0;
+}
+
+extern int (*_XErrorFunction)(Display *, XErrorEvent *);
+extern int _XDefaultError(Display *dpy, XErrorEvent *event);
+
+static void xerror_save(void)
+{
+   xerror_code = 0;
+   xerror_saved = _XErrorFunction;
+   _XErrorFunction = xerror_ignore;
+}
+
+static int xerror_restore(void)
+{
+   _XErrorFunction = xerror_saved;
+   return xerror_code;
+}
+
+static void notify_synaptics(XRRCrtcInfo *cres)
+{
+   int rotation;
+   char buf[256];
+
+   if (cres->rotation & 8)
+   rotation = 3;
+   else if (cres->rotation & 4)
+   rotation = 2;
+   else if (cres->rotation & 2)
+   rotation = 1;
+   else
+   rotation = 0;
+   if (current_rotation == rotation)
+   return;
+   current_rotation = rotation;
+   if (verbose)
+   fprintf(stderr, "%s: notified rotation %d\n",
+   progname, current_rotation);
+   sprintf(buf, "synclient Orientation=%d", current_rotation);
+   system(buf);
+}
+
+static void update_screen(Display *dpy, Window root)
+{
+   XRRScreenResources *res;
+   int i;
+
+   res = XRRGetScreenResources(dpy, root);
+   if (!res)
+   return;
+   for (i = 0; i < res->noutput; i++) {
+   XRROutputInfo *info;
+
+   info = XRRGetOutputInfo(dpy, res, res->outputs[i]);
+   if (!info)
+   continue;
+   if (is_laptop(info->name)) {
+   RRCrtc crtc = info->crtc;
+   XRRCrtcInfo *cres;
+
+   XRRFreeOutputInfo(info);
+   cres = XRRGetCrtcInfo(dpy, res, crtc);
+   if (!cres)
+   continue;
+   notify_synaptics(cres);
+   XRRFreeCrtcInfo(cres);
+   break;
+   }
+   XRRFreeOutputInfo(info);
+   }
+   XRRFreeScreenResources(res);
+}
+
+static void synxrr_daemon(Display *dpy, Window root)
+{
+   int event_base, error_base;
+
+   XRRQueryExtension(dpy, &event_base, &error_base);
+
+   XRRSelectInput(dpy, root, RRScreenChangeNotifyMask);
+
+   for (;;) {
+   XEvent event;
+
+   XNextEvent(dpy, &event);
+   xerror_save();
+   XRRUpdateConfiguration(&event);
+   switch (event.type - event_base) {
+   case RRScreenChangeNotify:
+   update_screen(dpy, root);
+   break;
+   }
+   xerror_restore();
+   }
+}
+
+static void usage(void)
+{
+   fprintf(stderr, "usage: %s [options]\n", progname);
+   fprintf(stderr, "  opti

[PATCH 14/18] Add gesture-mode notification via input properties

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |6 +++
 src/properties.c   |   33 +++
 src/synaptics.c|   68 +---
 src/synapticsstr.h |   11 ++
 tools/synclient.c  |1 +
 5 files changed, 107 insertions(+), 12 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index 7112bc0..5d440be 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -173,4 +173,10 @@
 /* 32 bit, 2 values, start, delta (0 disable) */
 #define SYNAPTICS_PROP_MULTI_TOUCH_PINCH "Synaptics Multi-Touch Pinch"
 
+/* 8 bit (BOOL) */
+#define SYNAPTICS_PROP_GESTURE_MODE_NOTIFY "Gesture Mode Notify"
+
+/* 32bit, read-only */
+#define SYNAPTICS_PROP_GESTURE_MODE "Current Gesture Mode"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/src/properties.c b/src/properties.c
index 0f679dd..7788a13 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -88,6 +88,8 @@ Atom prop_led   = 0;
 Atom prop_led_status= 0;
 Atom prop_led_double_tap= 0;
 Atom prop_multi_touch_pinch = 0;
+Atom prop_gesture_mode_notify   = 0;
+Atom prop_gesture_mode  = 0;
 
 static Atom
 InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
@@ -296,6 +298,11 @@ InitDeviceProperties(InputInfoPtr pInfo)
 values[0] =  para->multi_touch_pinch_start;
 values[1] =  para->multi_touch_pinch_dist;
 prop_multi_touch_pinch = InitAtom(local->dev, 
SYNAPTICS_PROP_MULTI_TOUCH_PINCH, 32, 2, values);
+
+prop_gesture_mode_notify = InitAtom(local->dev, 
SYNAPTICS_PROP_GESTURE_MODE_NOTIFY, 8, 1, ¶->gesture_mode_notify);
+
+values[0] = 0;
+prop_gesture_mode = InitAtom(local->dev, SYNAPTICS_PROP_GESTURE_MODE, 32, 
1, values);
 }
 
 int
@@ -542,7 +549,13 @@ SetProperty(DeviceIntPtr dev, Atom property, 
XIPropertyValuePtr prop,
 dist = (INT32*)prop->data;
para->multi_touch_pinch_start = dist[0];
para->multi_touch_pinch_dist = dist[1];
+} else if (property == prop_gesture_mode_notify)
+{
+INT32 *dist;
+if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
+return BadMatch;
 
+para->gesture_mode_notify = *(CARD8*)prop->data;
 } else if (property == prop_gestures)
 {
 BOOL *gestures;
@@ -724,3 +737,23 @@ void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off)
 XIChangeDeviceProperty(dev, prop_off, XA_INTEGER, 8,
PropModeReplace, 1, &val, FALSE);
 }
+
+void SynapticsSetGestureModeProperty(DeviceIntPtr dev, int mode)
+{
+LocalDevicePtr local = (LocalDevicePtr) dev->public.devicePrivate;
+SynapticsPrivate *priv = (SynapticsPrivate *) local->private;
+SynapticsParameters *para = &priv->synpara;
+static int gesture_mode_vals[] = {
+   [GESTURE_NONE] = 0,
+   [GESTURE_PINCH] = 10,
+   [GESTURE_HSCROLL] = 11,
+   [GESTURE_VSCROLL] = 12,
+   [GESTURE_CIRCULAR] = 13,
+};
+
+if (!prop_gesture_mode || !para->gesture_mode_notify)
+   return;
+mode = gesture_mode_vals[mode];
+XIChangeDeviceProperty(dev, prop_gesture_mode, XA_INTEGER, 32,
+  PropModeReplace, 1, &mode, TRUE);
+}
diff --git a/src/synaptics.c b/src/synaptics.c
index 5ec853a..2f264f0 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -143,6 +143,7 @@ void InitDeviceProperties(InputInfoPtr pInfo);
 int SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
 BOOL checkonly);
 void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off);
+void SynapticsSetGestureModeProperty(DeviceIntPtr dev, int val);
 
 InputDriverRec SYNAPTICS = {
 1,
@@ -608,6 +609,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
 pars->led_double_tap = xf86SetBoolOption(opts, "LEDDoubleTap", TRUE);
 pars->multi_touch_pinch_start = xf86SetIntOption(opts, 
"MultiTouchPinchStart", pinchStart);
 pars->multi_touch_pinch_dist = xf86SetIntOption(opts, 
"MultiTouchPinchDelta", pinchDelta);
+pars->gesture_mode_notify = xf86SetBoolOption(opts, "GestureModeNotify", 
TRUE);
 
 /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge 
parameters */
 if (pars->top_edge > pars->bottom_edge) {
@@ -1191,7 +1193,7 @@ handle_toggle_led(LocalDevicePtr local, struct 
SynapticsHwState *hw, int finger)
 para->touchpad_off = !para->touchpad_off;
 if (priv->proto_ops && priv->proto_ops->UpdateLED)
 priv->proto_ops->UpdateLED(local);
-   priv->prop_change_pending = 1;
+   priv->prop_change_pending |= 1;
 priv->led_tapped = FALSE;
 }
   

[PATCH 18/18] Make action strings configurable via synclient

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |   10 ++
 src/keymap.c   |   58 ++--
 src/properties.c   |   63 
 src/synaptics.c|   60 ++---
 src/synapticsstr.h |   28 +-
 tools/synclient.c  |   49 --
 6 files changed, 203 insertions(+), 65 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index c31c53c..7267669 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -185,4 +185,14 @@
 /* 32 bit, 2 values */
 #define SYNAPTICS_PROP_THREEFINGER_DELTA "Synaptics Three-Finger Delta"
 
+/* STR */
+#define SYNAPTICS_PROP_ZOOM_IN_ACTION "Synaptics Zoom-In Action"
+#define SYNAPTICS_PROP_ZOOM_OUT_ACTION "Synaptics Zoom-Out Action"
+
+/* STR */
+#define SYNAPTICS_PROP_3FINGER_LEFT_ACTION "Synaptics 3-Finger Left Action"
+#define SYNAPTICS_PROP_3FINGER_RIGHT_ACTION "Synaptics 3-Finger Right Action"
+#define SYNAPTICS_PROP_3FINGER_UP_ACTION "Synaptics 3-Finger Up Action"
+#define SYNAPTICS_PROP_3FINGER_DOWN_ACTION "Synaptics 3-Finger Down Action"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/src/keymap.c b/src/keymap.c
index b1e7cc6..0c365a2 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -142,15 +142,30 @@ static int string_to_keysym(const char *str)
 }
 
 int SynapticsParseActionStr(LocalDevicePtr local, const char *_str,
-   int *action, int max_actions)
+   SynapticsAction *action)
 {
 char *item;
 char *str, *next;
-int num_actions = 0;
 
-str = xstrdup(_str);
+if (action->str) {
+   if (_str && !strcmp(action->str, _str))
+   return TRUE;
+   free(action->str);
+   action->str = NULL;
+}
+
+action->num_actions = 0;
+if (!_str)
+   return TRUE;
+
+str = strdup(_str);
 if (!str)
-   return 0;
+   return FALSE;
+action->str = strdup(_str);
+if (!action->str) {
+   free(str);
+   return FALSE;
+}
 for (item = str; item && *item; item = next) {
int button, keysym, keycode;
 
@@ -160,7 +175,8 @@ int SynapticsParseActionStr(LocalDevicePtr local, const 
char *_str,
if (!*item)
continue;
if (sscanf(item, "Button%d", &button) == 1) {
-   action[num_actions++] = (ACTION_BUTTON << 16) | button;
+   action->action[action->num_actions++] =
+   (ACTION_BUTTON << 16) | button;
} else {
keysym = string_to_keysym(item);
if (keysym == NoSymbol) {
@@ -175,15 +191,17 @@ int SynapticsParseActionStr(LocalDevicePtr local, const 
char *_str,
continue;
}
if (get_modifier(keysym))
-   action[num_actions++] = (ACTION_KEYMOD << 16) | keycode;
+   action->action[action->num_actions++] =
+   (ACTION_KEYMOD << 16) | keycode;
else
-   action[num_actions++] = (ACTION_KEY << 16) | keycode;
+   action->action[action->num_actions++] =
+   (ACTION_KEY << 16) | keycode;
}
-   if (num_actions >= max_actions)
+   if (action->num_actions >= MAX_ACTIONS)
break;
 }
 free(str);
-return num_actions;
+return TRUE;
 }
 
 static void
@@ -196,13 +214,13 @@ Bool SynapticsInitKeyboard(DeviceIntPtr dev)
 return InitKeyboardDeviceStruct(dev, NULL, NULL, synaptics_kbdctrl);
 }
 
-void SynapticsSendAction(LocalDevicePtr local, int num_actions, int *action)
+void SynapticsSendAction(LocalDevicePtr local, SynapticsAction *action)
 {
 int n;
 
-for (n = 0; n < num_actions; n++) {
-   int val = action[n] & 0x;
-   switch ((action[n] >> 16) & 0xf) {
+for (n = 0; n < action->num_actions; n++) {
+   int val = action->action[n] & 0x;
+   switch ((action->action[n] >> 16) & 0xf) {
case ACTION_KEYMOD:
xf86PostKeyboardEvent(local->dev, val, TRUE);
break;
@@ -216,12 +234,20 @@ void SynapticsSendAction(LocalDevicePtr local, int 
num_actions, int *action)
break;
}
 }
-for (n = num_actions - 1; n >= 0; n--) {
-   int val = action[n] & 0x;
-   switch ((action[n] >> 16) & 0xf) {
+for (n = action->num_actions - 1; n >= 0; n--) {
+   int val = action->action[n] & 0x;
+   switch ((action->action[n] >> 16) & 0xf) {
case ACTION_KEYMOD:
xf86PostKeyboardEvent(local->dev, val, FALSE);
break;
}
 }
 }
+
+void SynapticsFreeAction

[PATCH 11/18] Check multi_touch module parameter and enable forcibly

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 src/eventcomm.c |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/src/eventcomm.c b/src/eventcomm.c
index 5969448..2223bac 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -179,6 +179,27 @@ event_query_info(InputInfoPtr pInfo)
 }
 }
 
+/* enable multi-touch feature via module parameter */
+#define MULTI_TOUCH_SYSFS_PARM "/sys/module/psmouse/parameters/multi_touch"
+static void enable_multi_touch(void)
+{
+int fd;
+char val = '0';
+fd = open(MULTI_TOUCH_SYSFS_PARM, O_RDONLY);
+if (fd < 0)
+   return;
+read(fd, &val, 1);
+close(fd);
+if (val == '1' || val == '2')
+   return;
+fd = open(MULTI_TOUCH_SYSFS_PARM, O_WRONLY);
+if (fd < 0)
+   return;
+val = '1';
+write(fd, &val, 1);
+close(fd);
+}
+
 static void event_query_multi_touch(LocalDevicePtr local)
 {
 SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
@@ -535,6 +556,7 @@ EventReadDevDimensions(InputInfoPtr pInfo)
 SynapticsPrivate *priv = (SynapticsPrivate *)pInfo->private;
 BOOL *need_grab = (BOOL*)priv->proto_data;
 
+enable_multi_touch();
 if (event_query_is_touchpad(pInfo->fd, (need_grab) ? *need_grab : TRUE))
event_query_axis_ranges(pInfo);
 event_query_info(pInfo);
-- 
1.7.3.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 13/18] Enable pinch gesture as default

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 src/synaptics.c |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/synaptics.c b/src/synaptics.c
index 902db53..5ec853a 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -435,6 +435,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
 Bool vertTwoFingerScroll, horizTwoFingerScroll;
 int horizResolution = 1;
 int vertResolution = 1;
+int pinchStart, pinchDelta;
 int width, height, diag, range;
 
 /* read the parameters */
@@ -466,6 +467,13 @@ static void set_default_parameters(InputInfoPtr pInfo)
 edgeMotionMinSpeed = 1;
 edgeMotionMaxSpeed = diag * .080;
 accelFactor = 200.0 / diag; /* trial-and-error */
+if (priv->can_multi_touch) {
+pinchStart = 0;
+pinchDelta = 0;
+} else {
+   pinchStart = diag * 0.1;
+   pinchDelta = diag * 0.375;
+}
 
 priv->move_ptr_threshold = width / 5;
 
@@ -598,8 +606,8 @@ static void set_default_parameters(InputInfoPtr pInfo)
 pars->touch_button_area = xf86SetIntOption(opts, "TouchButtonArea", 20);
 pars->touch_button_sticky = xf86SetIntOption(opts, "TouchButtonSticky", 
64);
 pars->led_double_tap = xf86SetBoolOption(opts, "LEDDoubleTap", TRUE);
-pars->multi_touch_pinch_start = xf86SetIntOption(opts, 
"MultiTouchPinchStart", 0);
-pars->multi_touch_pinch_dist = xf86SetIntOption(opts, 
"MultiTouchPinchDelta", 0);
+pars->multi_touch_pinch_start = xf86SetIntOption(opts, 
"MultiTouchPinchStart", pinchStart);
+pars->multi_touch_pinch_dist = xf86SetIntOption(opts, 
"MultiTouchPinchDelta", pinchDelta);
 
 /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge 
parameters */
 if (pars->top_edge > pars->bottom_edge) {
-- 
1.7.3.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 04/18] Add tap-on-LED feature support

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |3 +
 man/synaptics.man  |   17 +++
 src/properties.c   |   27 +++
 src/synaptics.c|   97 +++-
 src/synapticsstr.h |6 +++
 tools/synclient.c  |1 +
 6 files changed, 149 insertions(+), 2 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index dd7e259..1343e6d 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -161,4 +161,7 @@
 /* 8 bit (BOOL), led_status (on/off) */
 #define SYNAPTICS_PROP_LED_STATUS "Synaptics LED Status"
 
+/* 8 bit (BOOL), double-tap action on LED corner (on/off) */
+#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Dobule Tap"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/man/synaptics.man b/man/synaptics.man
index 8a9767d..d8afa1a 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -553,6 +553,19 @@ coordinates are less than MaxTapMove units apart.
 A "touch" event happens when the Z value goes above FingerHigh, and an
 "untouch" event happens when the Z value goes below FingerLow.
 .
+.TP
+.BI "Option \*qLEDDoubleTap\*q \*q" boolean \*q
+.
+Enables/disables the touchpad-control by double-tapping on the top-left
+corner LED.
+.
+Some devices have an LED on the top-left corner to indicate the
+touchpad state.  User can double-tap on the LED to toggle the touchpad
+state.  This option controls whether this action is enabled or not.
+The double-tap size is same as specified in MaxDoubleTapTime.
+The default value is ON.
+Property: "Synaptics LED Double Tap"
+.
 .LP
 The MaxDoubleTapTime parameter has the same function as the MaxTapTime
 parameter, but for the second, third, etc tap in a tap sequence.
@@ -918,6 +931,10 @@ LED support or not.
 .BI "Synaptics LED Status"
 8 bit (BOOL), the light status of the embedded LED.
 
+.TP 7
+.BI "Synaptics LED Double Tap"
+8 bit (BOOL), enable/disable the double-tap on LED.
+
 .SH "NOTES"
 There is an example hal policy file in
 .I ${sourcecode}/fdi/11-x11-synaptics.fdi
diff --git a/src/properties.c b/src/properties.c
index 57db0ec..6862a09 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -84,6 +84,7 @@ Atom prop_resolution= 0;
 Atom prop_area  = 0;
 Atom prop_led   = 0;
 Atom prop_led_status= 0;
+Atom prop_led_double_tap= 0;
 
 static Atom
 InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
@@ -283,6 +284,9 @@ InitDeviceProperties(InputInfoPtr pInfo)
 
 prop_led = InitAtom(local->dev, SYNAPTICS_PROP_LED, 8, 1, ¶->has_led);
 prop_led_status = InitAtom(local->dev, SYNAPTICS_PROP_LED_STATUS, 8, 1, 
¶->led_status);
+
+prop_led_double_tap = InitAtom(local->dev, SYNAPTICS_PROP_LED_DOUBLE_TAP, 
8, 1, ¶->led_double_tap);
+
 }
 
 int
@@ -508,6 +512,19 @@ SetProperty(DeviceIntPtr dev, Atom property, 
XIPropertyValuePtr prop,
 return BadValue;
 
 para->touchpad_off = off;
+if (!checkonly && para->has_led &&
+   para->led_status != para->touchpad_off) {
+para->led_status = para->touchpad_off;
+if (priv->proto_ops && priv->proto_ops->UpdateLED)
+priv->proto_ops->UpdateLED(local);
+}
+} else if (property == prop_led_double_tap)
+{
+if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
+return BadMatch;
+ 
+para->led_double_tap = *(CARD8*)prop->data;
+
 } else if (property == prop_gestures)
 {
 BOOL *gestures;
@@ -669,3 +686,13 @@ SetProperty(DeviceIntPtr dev, Atom property, 
XIPropertyValuePtr prop,
 return Success;
 }
 
+void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off)
+{
+uint8_t val;
+
+if (!prop_off)
+return;
+val = off;
+XIChangeDeviceProperty(dev, prop_off, XA_INTEGER, 8,
+   PropModeReplace, 1, &val, FALSE);
+}
diff --git a/src/synaptics.c b/src/synaptics.c
index 6f57d81..26ef666 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -142,6 +142,7 @@ static void CalculateScalingCoeffs(SynapticsPrivate *priv);
 void InitDeviceProperties(InputInfoPtr pInfo);
 int SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
 BOOL checkonly);
+void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off);
 
 InputDriverRec SYNAPTICS = {
 1,
@@ -608,6 +609,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
 pars->tap_and_drag_gesture = xf86SetBoolOption(opts, "TapAndDragGesture", 
TRUE);
 pars->resolution_horiz = xf86SetIntOption(opts, "HorizResolution", 
horizResolution);
 pars->

[PATCH 09/18] Avoid bogus coord reporting with clickpad devices

2010-10-08 Thread Takashi Iwai
When a clickpad device without multi-touch kernel support is used,
it gives wrong positions and leads to the annoying pointer jumps.
Filter the bogus events out in such a case.

Signed-off-by: Takashi Iwai 
---
 src/eventcomm.c |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/src/eventcomm.c b/src/eventcomm.c
index 517e6c3..76ff69d 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -363,6 +363,21 @@ EventReadHwState(InputInfoPtr pInfo,
 struct SynapticsHwState *hw = &(comm->hwState);
 SynapticsPrivate *priv = (SynapticsPrivate *)pInfo->private;
 SynapticsParameters *para = &priv->synpara;
+int minx, miny, maxx, maxy, x;
+
+/* range to filger out; a bit wider range is allowed since some devices
+ * are too fuzzy and give slightly shifted positions
+ */
+minx = priv->minx;
+maxx = priv->maxx;
+x = (maxx - minx) / 5;
+minx -= (minx > x) ? x : minx;
+maxx += x;
+miny = priv->miny;
+maxy = priv->maxy;
+x = (maxy - miny) / 5;
+maxy += miny;
+miny -= (miny > x) ? x : miny;
 
 while (SynapticsReadEvent(pInfo, &ev)) {
switch (ev.type) {
@@ -377,6 +392,9 @@ EventReadHwState(InputInfoPtr pInfo,
hw->numFingers = 3;
else
hw->numFingers = 0;
+   /* if the coord is out of range, we filter it out */
+   if (priv->is_clickpad && hw->z > 0 && (hw->x < minx || hw->x > 
maxx || hw->y < miny || hw->y > maxy))
+   return FALSE;
*hwRet = *hw;
return TRUE;
}
-- 
1.7.3.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 08/18] Avoid unexpected jumps

2010-10-08 Thread Takashi Iwai
Limit the movement size for avoiding the unexpected pointer jumps.

Signed-off-by: Takashi Iwai 
---
 src/synaptics.c|   10 ++
 src/synapticsstr.h |1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/src/synaptics.c b/src/synaptics.c
index 3ba918a..bd52730 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -467,6 +467,8 @@ static void set_default_parameters(InputInfoPtr pInfo)
 edgeMotionMaxSpeed = diag * .080;
 accelFactor = 200.0 / diag; /* trial-and-error */
 
+priv->move_ptr_threshold = width / 5;
+
 range = priv->maxp - priv->minp;
 
 /* scaling based on defaults and a pressure of 256 */
@@ -1949,6 +1951,14 @@ ComputeDeltas(SynapticsPrivate *priv, const struct 
SynapticsHwState *hw,
break;
}
 }
+
+if (moving_state && priv->count_packet_finger > 0 &&
+   priv->move_ptr_threshold > 0 ) {
+   int d = move_distance(HIST(0).x - hw->x, HIST(0).y - hw->y);
+   if (d > priv->move_ptr_threshold)
+   priv->count_packet_finger = 0; /* to avoid unexpected jumps */
+}
+
 if (inside_area && moving_state && !priv->palm &&
!priv->vert_scroll_edge_on && !priv->horiz_scroll_edge_on &&
!priv->vert_scroll_twofinger_on && !priv->horiz_scroll_twofinger_on &&
diff --git a/src/synapticsstr.h b/src/synapticsstr.h
index 44140f2..44925e5 100644
--- a/src/synapticsstr.h
+++ b/src/synapticsstr.h
@@ -245,6 +245,7 @@ typedef struct _SynapticsPrivateRec
 unsigned int clickpad_threshold;
 int clickpad_dx, clickpad_dy;
 struct SynapticsHwState prev_hw;   /* previous h/w state (for clickpad) */
+int move_ptr_threshold;
 int prop_change_pending;
 Bool led_touch_state;
 Bool led_tapped;
-- 
1.7.3.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 01/18] Add Clickpad support

2010-10-08 Thread Takashi Iwai
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 is already included in linux 2.6.34.

When the kernel driver sets only the left-button bit evbit and no
multi-finger is possible, 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 
---
 src/eventcomm.c|6 +
 src/synaptics.c|   58 
 src/synapticsstr.h |2 +
 3 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/src/eventcomm.c b/src/eventcomm.c
index 85dfd09..fc5055b 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -269,6 +269,12 @@ event_query_axis_ranges(InputInfoPtr pInfo)
}
 
xf86Msg(X_PROBED, "%s: buttons:%s\n", pInfo->name, buf);
+
+   /* clickpad device reports only the single left button mask */
+   if (priv->has_left && !priv->has_right && !priv->has_middle && 
!priv->has_double) {
+   priv->is_clickpad = TRUE;
+   xf86Msg(X_PROBED, "%s: is Clickpad device\n", local->name);
+   }
 }
 }
 
diff --git a/src/synaptics.c b/src/synaptics.c
index 4267c88..6f57d81 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -515,6 +515,18 @@ static void set_default_parameters(InputInfoPtr pInfo)
 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);
@@ -523,6 +535,10 @@ static void set_default_parameters(InputInfoPtr pInfo)
 
 pars->area_top_edge = set_percent_option(opts, "AreaTopEdge", height, 
priv->miny);
 pars->area_bottom_edge = set_percent_option(opts, "AreaBottomEdge", 
height, priv->miny);
+/* in clickpad mode, we don't want to sense the button area as default */
+if (pars->area_bottom_edge == 0 && priv->is_clickpad)
+   pars->area_bottom_edge = b;
+
 pars->area_left_edge = set_percent_option(opts, "AreaLeftEdge", width, 
priv->minx);
 pars->area_right_edge = set_percent_option(opts, "AreaRightEdge", width, 
priv->minx);
 
@@ -1085,6 +1101,44 @@ DeviceInit(DeviceIntPtr dev)
 return Success;
 }
 
+/* clickpad event handling */
+static void
+handle_clickpad(LocalDevicePtr local, struct SynapticsHwState *hw)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
+SynapticsParameters *para = &priv->synpara;
+
+if (hw->left) { /* clicked? */
+   if (hw->y > para->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
+*/
+   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;
+   } else {
+   /* dragging */
+   hw->left = priv->prev_hw.left;
+   hw->right = priv->prev_hw.right;
+   hw->middle = priv->prev_hw.middle;
+   }
+}
+priv->prev_hw = *hw;
+}
 
 /*
  * Convert from absolute X/Y coordinates to a coordinate system where
@@ -2251,6 +2305,10 @@ update_hw_button_state(const InputInfoPtr pInfo, struct 
SynapticsHwState *hw, in
 SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private);
 SynapticsParameters *para = &priv->synpara;
 
+/* Clickpad handling for button area */
+if (priv->is_clickpad)
+   handle_clickpad(local, hw);
+
 /* Treat the first two multi buttons as up/down for now. */
 hw->up |=

[PATCH 10/18] Add multi-touch support

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 src/eventcomm.c|   56 +++--
 src/synaptics.c|  136 
 src/synapticsstr.h |   12 +
 src/synproto.h |6 ++
 4 files changed, 196 insertions(+), 14 deletions(-)

diff --git a/src/eventcomm.c b/src/eventcomm.c
index 76ff69d..5969448 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -53,6 +53,17 @@
 
 #define SYNAPTICS_LED_SYS_FILE "/sys/class/leds/psmouse::synaptics/brightness"
 
+#ifndef SYN_MT_REPORT
+#define SYN_MT_REPORT  2
+#endif
+#ifndef ABS_MT_POSITION_X
+#define ABS_MT_POSITION_X  0x35
+#define ABS_MT_POSITION_Y  0x36
+#endif
+#ifndef ABS_MT_PRESSURE
+#define ABS_MT_PRESSURE0x3a
+#endif
+
 /*
  * Function Definitions
  /
@@ -168,6 +179,22 @@ event_query_info(InputInfoPtr pInfo)
 }
 }
 
+static void event_query_multi_touch(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+unsigned long absbits[NBITS(ABS_MAX)] = {0};
+int rc;
+
+priv->can_multi_touch = FALSE;
+if (priv->model != MODEL_SYNAPTICS)
+   return;
+SYSCALL(rc = ioctl(local->fd, EVIOCGBIT(EV_ABS, sizeof(absbits)), 
absbits));
+if (rc >= 0 && TEST_BIT(ABS_MT_POSITION_X, absbits)) {
+   priv->can_multi_touch = TRUE;
+   xf86Msg(X_INFO, "%s: supports multi-touch finger detection\n", 
local->name);
+}
+}
+
 static void
 event_query_clickpad(LocalDevicePtr local)
 {
@@ -175,7 +202,7 @@ event_query_clickpad(LocalDevicePtr local)
 
 /* clickpad device reports only the single left button mask */
 if (priv->has_left && !priv->has_right && !priv->has_middle &&
-   !priv->has_double &&
+   (!priv->has_double || priv->can_multi_touch) &&
priv->model == MODEL_SYNAPTICS) {
priv->is_clickpad = TRUE;
/* enable right/middle button caps; otherwise gnome-settings-daemon
@@ -383,21 +410,27 @@ EventReadHwState(InputInfoPtr pInfo,
switch (ev.type) {
case EV_SYN:
switch (ev.code) {
+   case SYN_MT_REPORT:
+   hw->multi_touch_count++;
+   break;
case SYN_REPORT:
if (comm->oneFinger)
-   hw->numFingers = 1;
+   hw->numFingers = hw->multi_touch_count ? 
hw->multi_touch_count : 1;
else if (comm->twoFingers)
hw->numFingers = 2;
else if (comm->threeFingers)
hw->numFingers = 3;
else
hw->numFingers = 0;
+   hw->multi_touch = hw->multi_touch_count;
+   hw->multi_touch_count = 0;
/* if the coord is out of range, we filter it out */
if (priv->is_clickpad && hw->z > 0 && (hw->x < minx || hw->x > 
maxx || hw->y < miny || hw->y > maxy))
return FALSE;
*hwRet = *hw;
return TRUE;
}
+   break;
case EV_KEY:
v = (ev.value ? TRUE : FALSE);
switch (ev.code) {
@@ -458,13 +491,25 @@ EventReadHwState(InputInfoPtr pInfo,
case EV_ABS:
switch (ev.code) {
case ABS_X:
-   hw->x = ev.value;
+   case ABS_MT_POSITION_X:
+   if (hw->multi_touch_count)
+   hw->multi_touch_x = ev.value;
+   else
+   hw->x = ev.value;
break;
case ABS_Y:
-   hw->y = ev.value;
+   case ABS_MT_POSITION_Y:
+   if (hw->multi_touch_count)
+   hw->multi_touch_y = ev.value;
+   else
+   hw->y = ev.value;
break;
case ABS_PRESSURE:
-   hw->z = ev.value;
+   case ABS_MT_PRESSURE:
+   if (hw->multi_touch_count)
+   hw->multi_touch_z = ev.value;
+   else
+   hw->z = ev.value;
break;
case ABS_TOOL_WIDTH:
hw->fingerWidth = ev.value;
@@ -493,6 +538,7 @@ EventReadDevDimensions(InputInfoPtr pInfo)
 if (event_query_is_touchpad(pInfo->fd, (need_grab) ? *need_grab : TRUE))
event_query_axis_ranges(pInfo);
 event_query_info(pInfo);
+event_query_multi_touch(local);
 event_query_clickpad(local);
 event_query_led(local);
 }
diff --git a/src/synaptics.c b/src/synaptics.c
index bd52730..05df1c8 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -1198,6 +1198,37 @@ static inline int get_touch_button_area(

[PATCH 03/18] Add the embedded LED support

2010-10-08 Thread Takashi Iwai
This patch adds the control of the embedded LED on the top-left corner
of new Synaptics devices.  For LED control, it requires the patch to
Linux synaptics input driver,
https://patchwork.kernel.org/patch/92434/

When a sysfs file /sys/class/leds/psmouse::synaptics exists, the driver
assumes it supports the embeded LED control.

The LED can be controlled via new properties, "Synaptics LED" and
"Synaptics LED Status".

Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |6 ++
 man/synaptics.man  |9 +
 src/eventcomm.c|   32 +++-
 src/properties.c   |   15 +++
 src/synapticsstr.h |2 ++
 src/synproto.h |1 +
 tools/synclient.c  |1 +
 7 files changed, 65 insertions(+), 1 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index 9c6a2ee..dd7e259 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -155,4 +155,10 @@
 /* 32 bit, 4 values, left, right, top, bottom */
 #define SYNAPTICS_PROP_AREA "Synaptics Area"
 
+/* 8 bit (BOOL, read-only), has_led */
+#define SYNAPTICS_PROP_LED "Synaptics LED"
+
+/* 8 bit (BOOL), led_status (on/off) */
+#define SYNAPTICS_PROP_LED_STATUS "Synaptics LED Status"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/man/synaptics.man b/man/synaptics.man
index 1561e19..8a9767d 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -909,6 +909,15 @@ right button, two-finger detection, three-finger 
detection, pressure detection,
 .BI "Synaptics Pad Resolution"
 32 bit unsigned, 2 values (read-only), vertical, horizontal in 
units/millimeter.
 
+.TP 7
+.BI "Synaptics LED"
+8 bit (BOOL, read-only), indicating whether the device has an embedded
+LED support or not.
+
+.TP 7
+.BI "Synaptics LED Status"
+8 bit (BOOL), the light status of the embedded LED.
+
 .SH "NOTES"
 There is an example hal policy file in
 .I ${sourcecode}/fdi/11-x11-synaptics.fdi
diff --git a/src/eventcomm.c b/src/eventcomm.c
index fc5055b..1fc41ef 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -51,6 +51,8 @@
 #define LONG(x)  ((x) / LONG_BITS)
 #define TEST_BIT(bit, array) (array[LONG(bit)] & (1 << OFF(bit)))
 
+#define SYNAPTICS_LED_SYS_FILE "/sys/class/leds/psmouse::synaptics/brightness"
+
 /*
  * Function Definitions
  /
@@ -166,6 +168,32 @@ event_query_info(InputInfoPtr pInfo)
 }
 }
 
+static void
+event_query_led(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+
+priv->synpara.has_led = !access(SYNAPTICS_LED_SYS_FILE, W_OK);
+}
+
+static void EventUpdateLED(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+
+if (priv->synpara.has_led) {
+char *val = priv->synpara.led_status ? "255" : "0";
+int fd = open(SYNAPTICS_LED_SYS_FILE, O_WRONLY);
+int err;
+
+if (fd < 0)
+return;
+err = write(fd, val, strlen(val));
+close(fd);
+if (err < 0)
+   xf86Msg(X_WARNING, "%s can't write LED value %s\n", local->name, 
val);
+}
+}
+
 /* Query device for axis ranges */
 static void
 event_query_axis_ranges(InputInfoPtr pInfo)
@@ -434,6 +462,7 @@ EventReadDevDimensions(InputInfoPtr pInfo)
 if (event_query_is_touchpad(pInfo->fd, (need_grab) ? *need_grab : TRUE))
event_query_axis_ranges(pInfo);
 event_query_info(pInfo);
+event_query_led(local);
 }
 
 static Bool
@@ -493,5 +522,6 @@ struct SynapticsProtocolOperations event_proto_operations = 
{
 EventQueryHardware,
 EventReadHwState,
 EventAutoDevProbe,
-EventReadDevDimensions
+EventReadDevDimensions,
+EventUpdateLED,
 };
diff --git a/src/properties.c b/src/properties.c
index 5400928..57db0ec 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -82,6 +82,8 @@ Atom prop_gestures  = 0;
 Atom prop_capabilities  = 0;
 Atom prop_resolution= 0;
 Atom prop_area  = 0;
+Atom prop_led   = 0;
+Atom prop_led_status= 0;
 
 static Atom
 InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
@@ -278,6 +280,9 @@ InitDeviceProperties(InputInfoPtr pInfo)
 values[2] = para->area_top_edge;
 values[3] = para->area_bottom_edge;
 prop_area = InitAtom(pInfo->dev, SYNAPTICS_PROP_AREA, 32, 4, values);
+
+prop_led = InitAtom(local->dev, SYNAPTICS_PROP_LED, 8, 1, ¶->has_led);
+prop_led_status = InitAtom(local->dev, SYNAPTICS_PROP_LED_STATUS, 8, 1, 
¶->led_status);
 }
 
 in

[PATCH 05/18] Fix 64bit arch issue in synaptics eventcomm.c

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 src/eventcomm.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/eventcomm.c b/src/eventcomm.c
index 1fc41ef..8a77788 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -49,7 +49,7 @@
 #define NBITS(x) (((x) + LONG_BITS - 1) / LONG_BITS)
 #define OFF(x)   ((x) % LONG_BITS)
 #define LONG(x)  ((x) / LONG_BITS)
-#define TEST_BIT(bit, array) (array[LONG(bit)] & (1 << OFF(bit)))
+#define TEST_BIT(bit, array) ((array[LONG(bit)] >> OFF(bit)) & 1)
 
 #define SYNAPTICS_LED_SYS_FILE "/sys/class/leds/psmouse::synaptics/brightness"
 
-- 
1.7.3.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 00/18] Synaptics patches

2010-10-08 Thread Takashi Iwai
Hi,

[dropped linux-* ML since these are pure X patches]

here is a series of patches for xorg synaptics driver, mainly supporting
Clickpad devices and multi-touch features.  It also contains some fixes,
improvements of the current behavior, also the extension for sending key
events, etc.

Not all patches are in quite good form, and definitely need discussions
and/or fixes.  The whole bunch of patches have been pending for so
long time by some (irritating) reason, and I feel relieved now.
Everyone knows the same feeling in daily life, I guess, so, let me
flush the patches from my queue first :)

(BTW, I'm on vacation until Monday; will reply after that)

thanks,

Takashi

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 06/18] Fix the check of Clickpad capabilities

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 src/eventcomm.c |   26 --
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/eventcomm.c b/src/eventcomm.c
index 8a77788..517e6c3 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -169,6 +169,25 @@ event_query_info(InputInfoPtr pInfo)
 }
 
 static void
+event_query_clickpad(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+
+/* clickpad device reports only the single left button mask */
+if (priv->has_left && !priv->has_right && !priv->has_middle &&
+   !priv->has_double &&
+   priv->model == MODEL_SYNAPTICS) {
+   priv->is_clickpad = TRUE;
+   /* enable right/middle button caps; otherwise gnome-settings-daemon
+* will ignore this device for left/right-hand setup because of a
+* single-button
+*/
+   priv->has_right = priv->has_middle = TRUE;
+   xf86Msg(X_INFO, "%s: is Clickpad device\n", local->name);
+}
+}
+
+static void
 event_query_led(LocalDevicePtr local)
 {
 SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
@@ -297,12 +316,6 @@ event_query_axis_ranges(InputInfoPtr pInfo)
}
 
xf86Msg(X_PROBED, "%s: buttons:%s\n", pInfo->name, buf);
-
-   /* clickpad device reports only the single left button mask */
-   if (priv->has_left && !priv->has_right && !priv->has_middle && 
!priv->has_double) {
-   priv->is_clickpad = TRUE;
-   xf86Msg(X_PROBED, "%s: is Clickpad device\n", local->name);
-   }
 }
 }
 
@@ -462,6 +475,7 @@ EventReadDevDimensions(InputInfoPtr pInfo)
 if (event_query_is_touchpad(pInfo->fd, (need_grab) ? *need_grab : TRUE))
event_query_axis_ranges(pInfo);
 event_query_info(pInfo);
+event_query_clickpad(local);
 event_query_led(local);
 }
 
-- 
1.7.3.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 02/18] Add a brief description of Clickpad to man page

2010-10-08 Thread Takashi Iwai
Signed-off-by: Takashi Iwai 
---
 man/synaptics.man |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/man/synaptics.man b/man/synaptics.man
index 3f1ca9d..1561e19 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -726,6 +726,15 @@ and finger movement distance.
 Trackstick mode is exited when the finger pressure drops below
 FingerLow or when the finger is moved further than MaxTapMove away
 from the initial position.
+.
+When the input device reports a device with a single left button
+and without multi-fingers, the driver assumes it's a Synaptics
+Clickpad device and handles it in ClickZone mode.  In this mode,
+a left/right/middle button event is generated according to the
+position you click.  When Clickpad is enabled, the touchpad area
+is shrunk as default in 20% and the bottom area is used as the
+click-button area.  The area can be configurable via options or
+properties below.
 
 .SH "DEVICE PROPERTIES"
 Synaptics 1.0 and higher support input device properties if the driver is
-- 
1.7.3.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-08 Thread Takashi Iwai
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 
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 

---
 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,

Re: [PATCH 0/3] Input: synaptics - multitouch and multifinger support

2010-10-08 Thread Takashi Iwai
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 
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 

---
 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 C

Re: [PATCH v3] Add Clickpad support

2010-05-02 Thread Takashi Iwai
At Thu, 29 Apr 2010 17:03:59 +1000,
Peter Hutterer wrote:
> 
> On Fri, Apr 23, 2010 at 05:39:04PM +0200, Takashi Iwai wrote:
> > 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 is already included in linux-input GIT tree, which
> > will hit Linus tree sooner or later.
> > 
> > When the kernel driver sets only the left-button bit evbit and no
> > multi-finger is possible, 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 
> > ---
> > 
> > v2->v3: Fix the mis-detection of Clickpad device with double-tap feature
> > (e.g. MacBook)
> > Fix one forgotten spacing issue Peter suggested
> > 
> >  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..6b44778 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->has_double) {
> > +   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..554f7a3 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 

[PATCH v3] Support LED and tap-on-LED feature

2010-04-23 Thread Takashi Iwai
This patch adds the control of the embedded LED on the top-left corner
of new Synaptics devices.  For LED control, it requires the patch to
Linux synaptics input driver,
https://patchwork.kernel.org/patch/92434/

When a sysfs file /sys/class/leds/psmouse::synaptics exists, the driver
assumes it supports the embeded LED control.  This corresponds to the
touchpad-off state.  When touchpad is disabled, LED is turned on.

For linking between the touchpad state and the LED state, a new callback
UpdateHardware is introduced.  Only eventcomm.c supports this (naturally).

A new feature for the LED-equipped device is that user can double-tap
on the LED to toggle the touchpad state on the fly.  This is also linked
with the touchpad-off state.

There is a new parameter for controlling the LED double-tap behavior, too.
It specifies the double-tap time.  Passing zero disables the double-tap
feature.

Signed-off-by: Takashi Iwai 
---

v2->v3: Fix a fd leak in write error path in eventcomm.c

 include/synaptics-properties.h |3 +
 man/synaptics.man  |   13 ++
 src/eventcomm.c|   32 ++-
 src/properties.c   |   26 
 src/synaptics.c|   85 +++-
 src/synapticsstr.h |6 +++
 src/synproto.h |1 +
 tools/synclient.c  |1 +
 8 files changed, 164 insertions(+), 3 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index cf330d8..d24ed9e 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -155,4 +155,7 @@
 /* 32 bit, 4 values, left, right, top, bottom */
 #define SYNAPTICS_PROP_AREA "Synaptics Area"
 
+/* 32 bit */
+#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Double Tap"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/man/synaptics.man b/man/synaptics.man
index 59fbaac..0c79721 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -509,6 +509,19 @@ Ignore movements, scrolling and tapping which take place 
below this edge.
 The option is disabled by default and can be enabled by setting the
 AreaBottomEdge option to any integer value other than zero. Property: 
"Synaptics Area"
 .
+.TP
+.BI "Option \*qLEDDoubleTap\*q \*q" integer \*q
+.
+The double-tap time for toggling the touchpad-control on the top-left
+corner LED.
+.
+Some devices have an LED on the top-left corner to indicate the
+touchpad state.  User can double-tap on the LED to toggle the touchpad
+state.  This option controls the double-tap time in milli-seconds.
+When it's zero, the LED-tapping feature is disabled.  The default
+value is 400.
+Property: "Synaptics LED Double Tap"
+.
 .LP
 A tap event happens when the finger is touched and released in a time
 interval shorter than MaxTapTime, and the touch and release
diff --git a/src/eventcomm.c b/src/eventcomm.c
index 6b44778..3948d9a 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -51,6 +51,8 @@
 #define LONG(x)  ((x) / LONG_BITS)
 #define TEST_BIT(bit, array) (array[LONG(bit)] & (1 << OFF(bit)))
 
+#define SYNAPTICS_LED_SYS_FILE "/sys/class/leds/psmouse::synaptics/brightness"
+
 /*
  * Function Definitions
  /
@@ -166,6 +168,32 @@ event_query_info(LocalDevicePtr local)
 }
 }
 
+static void
+event_query_led(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+
+priv->has_led = !access(SYNAPTICS_LED_SYS_FILE, W_OK);
+}
+
+static void EventUpdateHardware(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+
+if (priv->has_led) {
+char *val = priv->synpara.touchpad_off ? "255" : "0";
+int fd = open(SYNAPTICS_LED_SYS_FILE, O_WRONLY);
+int err;
+
+if (fd < 0)
+return;
+err = write(fd, val, strlen(val));
+close(fd);
+if (err < 0)
+xf86Msg(X_WARNING, "%s can't write LED value %s\n", val);
+}
+}
+
 /* Query device for axis ranges */
 static void
 event_query_axis_ranges(LocalDevicePtr local)
@@ -434,6 +462,7 @@ EventReadDevDimensions(LocalDevicePtr local)
 if (event_query_is_touchpad(local->fd, (need_grab) ? *need_grab : TRUE))
event_query_axis_ranges(local);
 event_query_info(local);
+event_query_led(local);
 }
 
 static Bool
@@ -493,5 +522,6 @@ struct SynapticsProtocolOperations event_proto_operations = 
{
 EventQueryHardware,
 EventReadHwState,
 EventAutoDevProbe,
-EventReadDevDimensions
+EventReadDevDimensions,
+EventUpdateHardware,
 };
diff --git a/src/properties.c b/src/properties.c
index 4366034..d7bc3bd 100644
--- a/src/properties.c

[PATCH v3] Add Clickpad support

2010-04-23 Thread Takashi Iwai
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 is already included in linux-input GIT tree, which
will hit Linus tree sooner or later.

When the kernel driver sets only the left-button bit evbit and no
multi-finger is possible, 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 
---

v2->v3: Fix the mis-detection of Clickpad device with double-tap feature
(e.g. MacBook)
Fix one forgotten spacing issue Peter suggested

 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..6b44778 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->has_double) {
+   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..554f7a3 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 =

[PATCH v2] Support LED and tap-on-LED feature

2010-04-21 Thread Takashi Iwai
This patch adds the control of the embedded LED on the top-left corner
of new Synaptics devices.  For LED control, it requires the patch to
Linux synaptics input driver,
https://patchwork.kernel.org/patch/93858/

When a sysfs file /sys/class/leds/psmouse::synaptics exists, the driver
assumes it supports the embeded LED control.  This corresponds to the
touchpad-off state.  When touchpad is disabled, LED is turned on.

For linking between the touchpad state and the LED state, a new callback
UpdateHardware is introduced.  Only eventcomm.c supports this (naturally).

A new feature for the LED-equipped device is that user can double-tap
on the LED to toggle the touchpad state on the fly.  This is also linked
with the touchpad-off state.

There is a new parameter for controlling the LED double-tap behavior, too.
It specifies the double-tap time.  Passing zero disables the double-tap
feature.

Signed-off-by: Takashi Iwai 
---

The new patch is based on the sysfs LED class interface we chose
instead of input evdev LED bits.  Also including a few clean-ups and
untabified.

 include/synaptics-properties.h |3 +
 man/synaptics.man  |   13 ++
 src/eventcomm.c|   29 +-
 src/properties.c   |   26 
 src/synaptics.c|   85 +++-
 src/synapticsstr.h |6 +++
 src/synproto.h |1 +
 tools/synclient.c  |1 +
 8 files changed, 161 insertions(+), 3 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index cf330d8..d24ed9e 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -155,4 +155,7 @@
 /* 32 bit, 4 values, left, right, top, bottom */
 #define SYNAPTICS_PROP_AREA "Synaptics Area"
 
+/* 32 bit */
+#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Double Tap"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/man/synaptics.man b/man/synaptics.man
index 59fbaac..0c79721 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -509,6 +509,19 @@ Ignore movements, scrolling and tapping which take place 
below this edge.
 The option is disabled by default and can be enabled by setting the
 AreaBottomEdge option to any integer value other than zero. Property: 
"Synaptics Area"
 .
+.TP
+.BI "Option \*qLEDDoubleTap\*q \*q" integer \*q
+.
+The double-tap time for toggling the touchpad-control on the top-left
+corner LED.
+.
+Some devices have an LED on the top-left corner to indicate the
+touchpad state.  User can double-tap on the LED to toggle the touchpad
+state.  This option controls the double-tap time in milli-seconds.
+When it's zero, the LED-tapping feature is disabled.  The default
+value is 400.
+Property: "Synaptics LED Double Tap"
+.
 .LP
 A tap event happens when the finger is touched and released in a time
 interval shorter than MaxTapTime, and the touch and release
diff --git a/src/eventcomm.c b/src/eventcomm.c
index 10d183d..ffcacf1 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -51,6 +51,8 @@
 #define LONG(x)  ((x) / LONG_BITS)
 #define TEST_BIT(bit, array) (array[LONG(bit)] & (1 << OFF(bit)))
 
+#define SYNAPTICS_LED_SYS_FILE "/sys/class/leds/psmouse::synaptics/brightness"
+
 /*
  * Function Definitions
  /
@@ -166,6 +168,29 @@ event_query_info(LocalDevicePtr local)
 }
 }
 
+static void
+event_query_led(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+
+priv->has_led = !access(SYNAPTICS_LED_SYS_FILE, W_OK);
+}
+
+static void EventUpdateHardware(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+
+if (priv->has_led) {
+char *val = priv->synpara.touchpad_off ? "255" : "0";
+int fd = open(SYNAPTICS_LED_SYS_FILE, O_WRONLY);
+if (fd < 0)
+return;
+if (write(fd, val, strlen(val)) < 0)
+return;
+close(fd);
+}
+}
+
 /* Query device for axis ranges */
 static void
 event_query_axis_ranges(LocalDevicePtr local)
@@ -434,6 +459,7 @@ EventReadDevDimensions(LocalDevicePtr local)
 if (event_query_is_touchpad(local->fd, (need_grab) ? *need_grab : TRUE))
event_query_axis_ranges(local);
 event_query_info(local);
+event_query_led(local);
 }
 
 static Bool
@@ -493,5 +519,6 @@ struct SynapticsProtocolOperations event_proto_operations = 
{
 EventQueryHardware,
 EventReadHwState,
 EventAutoDevProbe,
-EventReadDevDimensions
+EventReadDevDimensions,
+EventUpdateHardware,
 };
diff --git a/src/properties.c b/src/properties.c
index 4366034..d7bc3bd 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -84,6 +

[PATCH] Add Clickpad support (v2)

2010-04-21 Thread Takashi Iwai
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 is already included in linux-input GIT tree, which
will hit Linus tree sooner 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 
---

In this version, I dropped the parameters and simplified as much as
possible.  I tried to remove the code to disable pointer moves, but
it results in the non-working dragging, as it seems relevant with
multi-tapping stuff.  So it's still there.


 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)

Re: [PATCH 1/2] Add Clickpad support

2010-04-21 Thread Takashi Iwai
At Wed, 21 Apr 2010 16:31:29 +1000,
Peter Hutterer wrote:
> 
> On Wed, Apr 21, 2010 at 08:16:40AM +0200, Takashi Iwai wrote:
> > At Wed, 21 Apr 2010 14:16:41 +1000,
> > Peter Hutterer wrote:
> > > 
> > > Thank you for the patch. Just as a heads-up, I'd like to wait with this
> > > until there's at least one kernel released with this support.
> > > My comments to the patch are inline.
> > 
> > Thanks for review!
> > 
> > > On Thu, Apr 15, 2010 at 06:12:10PM +0200, Takashi Iwai wrote:
> > > > 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/
> > > > 
> > > > 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.
> > > > 
> > > > There are two new parameters added.  One is a parameter to specify
> > > > the size of the bottom button-area, and another is to toggle the touch-
> > > > sensing in the button area.
> > > > 
> > > > Signed-off-by: Takashi Iwai 
> > > > ---
> > > >  include/synaptics-properties.h |6 
> > > >  man/synaptics.man  |   21 +++
> > > >  src/eventcomm.c|   15 ++
> > > >  src/properties.c   |   19 +
> > > >  src/synaptics.c|   56 
> > > > 
> > > >  src/synapticsstr.h |4 +++
> > > >  tools/synclient.c  |2 +
> > > >  7 files changed, 123 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/include/synaptics-properties.h 
> > > > b/include/synaptics-properties.h
> > > > index cf330d8..c77afd3 100644
> > > > --- a/include/synaptics-properties.h
> > > > +++ b/include/synaptics-properties.h
> > > > @@ -155,4 +155,10 @@
> > > >  /* 32 bit, 4 values, left, right, top, bottom */
> > > >  #define SYNAPTICS_PROP_AREA "Synaptics Area"
> > > >  
> > > > +/* 8 bit (0-100) */
> > > > +#define SYNAPTICS_PROP_TOUCH_BUTTON_AREA "Synaptics Touch Button Area"
> > > 
> > > > +
> > > > +/* 8 bit (BOOL) */
> > > > +#define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button 
> > > > Sense"
> > > > +
> > > >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > > > diff --git a/man/synaptics.man b/man/synaptics.man
> > > > index 59fbaac..f92ea0c 100644
> > > > --- a/man/synaptics.man
> > > > +++ b/man/synaptics.man
> > > > @@ -509,6 +509,27 @@ Ignore movements, scrolling and tapping which take 
> > > > place below this edge.
> > > >  The option is disabled by default and can be enabled by setting the
> > > >  AreaBottomEdge option to any integer value other than zero. Property: 
> > > > "Synaptics Area"
> > > >  .
> > > > +.TP
> > > > +.BI "Option \*qTouchButtonArea\*q \*q" integer \*q
> > > > +The percentage of touch-button area of Clickpad device.
> > > > +.
> > > > +The synaptics driver supports "ClickZone" mode, which emulates the 
> > > > buttons
> > > > +to click at the bottom of touchpad area.  This option specifies how 
> > > > large
> > > > +is the area to be used as the button area.  Passing zero to this 
> > > > disables
> > > > +the Clickpad behavior.  The default value is 20.
> > > 
> > > where does the 0-100 come from? what unit is this? it's answered in the
> > > header file but this should be in the man page as well.
> > 
> > Right.
> > 
> > > also, I'd rather have this stored in device units and leave things like
> > > "this is 20% of the area" to higher-level tools.
> > 
> > I don't mind in any way.  Maybe this parameter is rarely changed.
> > 
> > > > +Property: "Synaptics Touch Button Area"
> > > 
> > > please make this so that the Synaptics Area is re-used instead of
> > > introducing another are

Re: [PATCH 2/2] Support LED and tap-on-LED feature

2010-04-21 Thread Takashi Iwai
At Wed, 21 Apr 2010 16:53:14 +1000,
Peter Hutterer wrote:
> 
> On Wed, Apr 21, 2010 at 08:26:41AM +0200, Takashi Iwai wrote:
> > At Wed, 21 Apr 2010 14:16:29 +1000,
> > Peter Hutterer wrote:
> > > 
> > > On Thu, Apr 15, 2010 at 06:12:11PM +0200, Takashi Iwai wrote:
> > > > This patch adds the control of the embedded LED on the top-left corner
> > > > of new Synaptics devices.  For LED control, it requires the patch to
> > > > Linux synaptics input driver,
> > > > https://patchwork.kernel.org/patch/92434/
> > > > 
> > > > When evdev reports the presense of LED and LED_MUTE bits, the driver
> > > > assumes it supports the embeded LED control.  This corresponds to the
> > > > touchpad-off state.  When touchpad is disabled, LED is turned on.
> > > > 
> > > > For linking between the touchpad state and the LED state, a new callback
> > > > UpdateHardware is introduced.  Only eventcomm.c supports this 
> > > > (naturally).
> > > > 
> > > > A new feature for the LED-equipped device is that user can double-tap
> > > > on the LED to toggle the touchpad state on the fly.  This is also linked
> > > > with the touchpad-off state.
> > > > 
> > > > There is a new parameter for controlling the LED double-tap behavior, 
> > > > too.
> > > > It specifies the double-tap time.  Passing zero disables the double-tap
> > > > feature.
> > > > 
> > > > Signed-off-by: Takashi Iwai 
> > > > ---
> > > >  include/synaptics-properties.h |3 ++
> > > >  man/synaptics.man  |   13 +++
> > > >  src/eventcomm.c|   39 +-
> > > >  src/properties.c   |   26 ++
> > > >  src/synaptics.c|   73 
> > > > ++-
> > > >  src/synapticsstr.h |6 +++
> > > >  src/synproto.h |1 +
> > > >  tools/synclient.c  |1 +
> > > >  8 files changed, 159 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/include/synaptics-properties.h 
> > > > b/include/synaptics-properties.h
> > > > index c77afd3..56c3e1d 100644
> > > > --- a/include/synaptics-properties.h
> > > > +++ b/include/synaptics-properties.h
> > > > @@ -161,4 +161,7 @@
> > > >  /* 8 bit (BOOL) */
> > > >  #define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button 
> > > > Sense"
> > > >  
> > > > +/* 32 bit */
> > > > +#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Double Tap"
> > > > +
> > > >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > > > diff --git a/man/synaptics.man b/man/synaptics.man
> > > > index f92ea0c..baa7e8f 100644
> > > > --- a/man/synaptics.man
> > > > +++ b/man/synaptics.man
> > > > @@ -530,6 +530,19 @@ clicking the button.  On the other hand, this 
> > > > reduces the available
> > > >  space on the touchpad.  The default is on.
> > > >  Property: "Synaptics Touch Button Sense"
> > > >  .
> > > > +.TP
> > > > +.BI "Option \*qLEDDoubleTap\*q \*q" integer \*q
> > > > +.
> > > > +The double-tap time for toggling the touchpad-control on the top-left
> > > > +corner LED.
> > > > +.
> > > > +Some devices have an LED on the top-left corner to indicate the
> > > > +touchpad state.  User can double-tap on the LED to toggle the touchpad
> > > > +state.  This option controls the double-tap time in milli-seconds.
> > > > +When it's zero, the LED-tapping feature is disabled.  The default
> > > > +value is 400.
> > > > +Property: "Synaptics LED Double Tap"
> > > 
> > > a few questions to the principle:
> > > - any specific reason this is a double-tap?
> > 
> > A single-tap is often too sensitive.  User may toggle it mistakenly,
> > and wonders what happens suddenly.  Also, Windows driver uses the
> > double-tap, too.
> > 
> > > - the corner might be used for the same feature even if there is no LED
> > >   present, right?
> > 
> > The check is done only when has_led flag is set, so it's exclusive for
> > the device with LED control.
> > 
> > > as I 

Re: [PATCH 2/2] Support LED and tap-on-LED feature

2010-04-20 Thread Takashi Iwai
At Wed, 21 Apr 2010 14:16:29 +1000,
Peter Hutterer wrote:
> 
> On Thu, Apr 15, 2010 at 06:12:11PM +0200, Takashi Iwai wrote:
> > This patch adds the control of the embedded LED on the top-left corner
> > of new Synaptics devices.  For LED control, it requires the patch to
> > Linux synaptics input driver,
> > https://patchwork.kernel.org/patch/92434/
> > 
> > When evdev reports the presense of LED and LED_MUTE bits, the driver
> > assumes it supports the embeded LED control.  This corresponds to the
> > touchpad-off state.  When touchpad is disabled, LED is turned on.
> > 
> > For linking between the touchpad state and the LED state, a new callback
> > UpdateHardware is introduced.  Only eventcomm.c supports this (naturally).
> > 
> > A new feature for the LED-equipped device is that user can double-tap
> > on the LED to toggle the touchpad state on the fly.  This is also linked
> > with the touchpad-off state.
> > 
> > There is a new parameter for controlling the LED double-tap behavior, too.
> > It specifies the double-tap time.  Passing zero disables the double-tap
> > feature.
> > 
> > Signed-off-by: Takashi Iwai 
> > ---
> >  include/synaptics-properties.h |3 ++
> >  man/synaptics.man  |   13 +++
> >  src/eventcomm.c|   39 +-
> >  src/properties.c   |   26 ++
> >  src/synaptics.c|   73 
> > ++-
> >  src/synapticsstr.h |6 +++
> >  src/synproto.h |1 +
> >  tools/synclient.c  |1 +
> >  8 files changed, 159 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > index c77afd3..56c3e1d 100644
> > --- a/include/synaptics-properties.h
> > +++ b/include/synaptics-properties.h
> > @@ -161,4 +161,7 @@
> >  /* 8 bit (BOOL) */
> >  #define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button Sense"
> >  
> > +/* 32 bit */
> > +#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Double Tap"
> > +
> >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > diff --git a/man/synaptics.man b/man/synaptics.man
> > index f92ea0c..baa7e8f 100644
> > --- a/man/synaptics.man
> > +++ b/man/synaptics.man
> > @@ -530,6 +530,19 @@ clicking the button.  On the other hand, this reduces 
> > the available
> >  space on the touchpad.  The default is on.
> >  Property: "Synaptics Touch Button Sense"
> >  .
> > +.TP
> > +.BI "Option \*qLEDDoubleTap\*q \*q" integer \*q
> > +.
> > +The double-tap time for toggling the touchpad-control on the top-left
> > +corner LED.
> > +.
> > +Some devices have an LED on the top-left corner to indicate the
> > +touchpad state.  User can double-tap on the LED to toggle the touchpad
> > +state.  This option controls the double-tap time in milli-seconds.
> > +When it's zero, the LED-tapping feature is disabled.  The default
> > +value is 400.
> > +Property: "Synaptics LED Double Tap"
> 
> a few questions to the principle:
> - any specific reason this is a double-tap?

A single-tap is often too sensitive.  User may toggle it mistakenly,
and wonders what happens suddenly.  Also, Windows driver uses the
double-tap, too.

> - the corner might be used for the same feature even if there is no LED
>   present, right?

The check is done only when has_led flag is set, so it's exclusive for
the device with LED control.

> as I read this patch, the LED updating could be split out
>   from the actual gesture of disabling the touchpad by clicking into the
>   corner.

In general yes, the double-tapping to toggle touchpad is an individual
feature.  But this becomes intuitive when coupled with LED, thus
rather somewhat special for LED-equipped device, I suppose.


> 
> > +.
> >  .LP
> >  A tap event happens when the finger is touched and released in a time
> >  interval shorter than MaxTapTime, and the touch and release
> > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > index ca99f3a..292e64e 100644
> > --- a/src/eventcomm.c
> > +++ b/src/eventcomm.c
> > @@ -179,6 +179,39 @@ static void event_query_clickpad(LocalDevicePtr local)
> >  }
> >  }
> >  
> > +/* LED support: the kernel driver should give EV_LED and LED_MUTE bits */
> > +static void event_query_led(LocalDevicePtr local)
> > +{
> > +SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
> > +unsigned

Re: [PATCH 1/2] Add Clickpad support

2010-04-20 Thread Takashi Iwai
At Wed, 21 Apr 2010 14:16:41 +1000,
Peter Hutterer wrote:
> 
> Thank you for the patch. Just as a heads-up, I'd like to wait with this
> until there's at least one kernel released with this support.
> My comments to the patch are inline.

Thanks for review!

> On Thu, Apr 15, 2010 at 06:12:10PM +0200, Takashi Iwai wrote:
> > 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/
> > 
> > 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.
> > 
> > There are two new parameters added.  One is a parameter to specify
> > the size of the bottom button-area, and another is to toggle the touch-
> > sensing in the button area.
> > 
> > Signed-off-by: Takashi Iwai 
> > ---
> >  include/synaptics-properties.h |6 
> >  man/synaptics.man  |   21 +++
> >  src/eventcomm.c|   15 ++
> >  src/properties.c   |   19 +
> >  src/synaptics.c|   56 
> > 
> >  src/synapticsstr.h |4 +++
> >  tools/synclient.c  |2 +
> >  7 files changed, 123 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > index cf330d8..c77afd3 100644
> > --- a/include/synaptics-properties.h
> > +++ b/include/synaptics-properties.h
> > @@ -155,4 +155,10 @@
> >  /* 32 bit, 4 values, left, right, top, bottom */
> >  #define SYNAPTICS_PROP_AREA "Synaptics Area"
> >  
> > +/* 8 bit (0-100) */
> > +#define SYNAPTICS_PROP_TOUCH_BUTTON_AREA "Synaptics Touch Button Area"
> 
> > +
> > +/* 8 bit (BOOL) */
> > +#define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button Sense"
> > +
> >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > diff --git a/man/synaptics.man b/man/synaptics.man
> > index 59fbaac..f92ea0c 100644
> > --- a/man/synaptics.man
> > +++ b/man/synaptics.man
> > @@ -509,6 +509,27 @@ Ignore movements, scrolling and tapping which take 
> > place below this edge.
> >  The option is disabled by default and can be enabled by setting the
> >  AreaBottomEdge option to any integer value other than zero. Property: 
> > "Synaptics Area"
> >  .
> > +.TP
> > +.BI "Option \*qTouchButtonArea\*q \*q" integer \*q
> > +The percentage of touch-button area of Clickpad device.
> > +.
> > +The synaptics driver supports "ClickZone" mode, which emulates the buttons
> > +to click at the bottom of touchpad area.  This option specifies how large
> > +is the area to be used as the button area.  Passing zero to this disables
> > +the Clickpad behavior.  The default value is 20.
> 
> where does the 0-100 come from? what unit is this? it's answered in the
> header file but this should be in the man page as well.

Right.

> also, I'd rather have this stored in device units and leave things like
> "this is 20% of the area" to higher-level tools.

I don't mind in any way.  Maybe this parameter is rarely changed.

> > +Property: "Synaptics Touch Button Area"
> 
> please make this so that the Synaptics Area is re-used instead of
> introducing another area property. This should remove the need for this
> property altogether and just need the enable/disable value in the other one.

Do you mean to extend "Synaptics Area" property?

I find it might be better to split from Synaptics Area, because this
is pretty specific to Clickpad mode while Synaptics Area is a more
generic one.  But, mixing them up is technically no problem, of
course.

> how do you get the default of 20? isn't this something that should be
> auto-scaled from the device in case future devices have a different
> resolution?

It looks same regardless of the device size.  I checked several
clickpad devices in different sizes, and all of them show mostly same
button area, ca 20% of the touchpad size at the bottom.


> > +.TP
> > +.BI "Option \*qTouchButtonSense\*q \*q" integer \*q
> > +Disable the touch-sensing in the Clickpad button area.
> > +.
> > +Since the touchpad is often quite sensitive, the pointer would move
> > +slightly when you click via Clickpad, and user may mis

[PATCH 2/2] Support LED and tap-on-LED feature

2010-04-15 Thread Takashi Iwai
This patch adds the control of the embedded LED on the top-left corner
of new Synaptics devices.  For LED control, it requires the patch to
Linux synaptics input driver,
https://patchwork.kernel.org/patch/92434/

When evdev reports the presense of LED and LED_MUTE bits, the driver
assumes it supports the embeded LED control.  This corresponds to the
touchpad-off state.  When touchpad is disabled, LED is turned on.

For linking between the touchpad state and the LED state, a new callback
UpdateHardware is introduced.  Only eventcomm.c supports this (naturally).

A new feature for the LED-equipped device is that user can double-tap
on the LED to toggle the touchpad state on the fly.  This is also linked
with the touchpad-off state.

There is a new parameter for controlling the LED double-tap behavior, too.
It specifies the double-tap time.  Passing zero disables the double-tap
feature.

Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |3 ++
 man/synaptics.man  |   13 +++
 src/eventcomm.c|   39 +-
 src/properties.c   |   26 ++
 src/synaptics.c|   73 ++-
 src/synapticsstr.h |6 +++
 src/synproto.h |1 +
 tools/synclient.c  |1 +
 8 files changed, 159 insertions(+), 3 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index c77afd3..56c3e1d 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -161,4 +161,7 @@
 /* 8 bit (BOOL) */
 #define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button Sense"
 
+/* 32 bit */
+#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Double Tap"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/man/synaptics.man b/man/synaptics.man
index f92ea0c..baa7e8f 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -530,6 +530,19 @@ clicking the button.  On the other hand, this reduces the 
available
 space on the touchpad.  The default is on.
 Property: "Synaptics Touch Button Sense"
 .
+.TP
+.BI "Option \*qLEDDoubleTap\*q \*q" integer \*q
+.
+The double-tap time for toggling the touchpad-control on the top-left
+corner LED.
+.
+Some devices have an LED on the top-left corner to indicate the
+touchpad state.  User can double-tap on the LED to toggle the touchpad
+state.  This option controls the double-tap time in milli-seconds.
+When it's zero, the LED-tapping feature is disabled.  The default
+value is 400.
+Property: "Synaptics LED Double Tap"
+.
 .LP
 A tap event happens when the finger is touched and released in a time
 interval shorter than MaxTapTime, and the touch and release
diff --git a/src/eventcomm.c b/src/eventcomm.c
index ca99f3a..292e64e 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -179,6 +179,39 @@ static void event_query_clickpad(LocalDevicePtr local)
 }
 }
 
+/* LED support: the kernel driver should give EV_LED and LED_MUTE bits */
+static void event_query_led(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+unsigned long evbits[NBITS(EV_MAX)] = {0};
+int rc;
+
+priv->has_led = FALSE;
+
+SYSCALL(rc = ioctl(local->fd, EVIOCGBIT(0, sizeof(evbits)), evbits));
+if (TEST_BIT(EV_LED, evbits)) {
+   unsigned long ledbits[NBITS(LED_MAX)] = {0};
+   SYSCALL(rc = ioctl(local->fd, EVIOCGBIT(EV_LED, sizeof(ledbits)), 
ledbits));
+   if (TEST_BIT(LED_MUTE, ledbits)) {
+   xf86Msg(X_INFO, "%s: has LED control\n", local->name);
+   priv->has_led = TRUE;
+   }
+}
+}
+
+static void EventUpdateHardware(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+if (priv->has_led) {
+   struct input_event ev;
+   gettimeofday(&ev.time, NULL);
+   ev.type = EV_LED;
+   ev.code = LED_MUTE;
+   ev.value = priv->synpara.touchpad_off != 0;
+   (void)write(local->fd, &ev, sizeof(ev));
+}
+}
+
 /* Query device for axis ranges */
 static void
 event_query_axis_ranges(LocalDevicePtr local)
@@ -268,6 +301,7 @@ event_query_axis_ranges(LocalDevicePtr local)
 }
 
 event_query_clickpad(local);
+event_query_led(local);
 }
 
 static Bool
@@ -421,6 +455,8 @@ EventReadHwState(LocalDevicePtr local,
break;
}
break;
+   case EV_LED:
+   return TRUE;
}
 }
 return FALSE;
@@ -502,5 +538,6 @@ struct SynapticsProtocolOperations event_proto_operations = 
{
 EventQueryHardware,
 EventReadHwState,
 EventAutoDevProbe,
-EventReadDevDimensions
+EventReadDevDimensions,
+EventUpdateHardware,
 };
diff --git a/src/properties.c b/src/properties.c
index a579479..57f11f0 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -86,6 +86,7 @@ Atom prop_resol

[PATCH 0/2] Synaptics Clickpad support

2010-04-15 Thread Takashi Iwai
Hi,

the following patches are for supporting Synaptics Clickpad devices
equipped on recent HP laptops.
 [PATCH 1/2] Add Clickpad support
 [PATCH 2/2] Support LED and tap-on-LED feature

The patches require the changes in Linux-kernel synaptics input driver.
The corresponding patches have been posted to linux-input and
linux-kernel MLs.  See the changelog in each patch.

While the first one is relativey easy, the second patch isn't so
straight-forward, because it needs to handle double-tapping and link
between the driver's touchpad state and LED state.

Both patches don't change the driver behavior unless the kernel reports
the clickpad or LED device support.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 1/2] Add Clickpad support

2010-04-15 Thread Takashi Iwai
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/

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.

There are two new parameters added.  One is a parameter to specify
the size of the bottom button-area, and another is to toggle the touch-
sensing in the button area.

Signed-off-by: Takashi Iwai 
---
 include/synaptics-properties.h |6 
 man/synaptics.man  |   21 +++
 src/eventcomm.c|   15 ++
 src/properties.c   |   19 +
 src/synaptics.c|   56 
 src/synapticsstr.h |4 +++
 tools/synclient.c  |2 +
 7 files changed, 123 insertions(+), 0 deletions(-)

diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index cf330d8..c77afd3 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -155,4 +155,10 @@
 /* 32 bit, 4 values, left, right, top, bottom */
 #define SYNAPTICS_PROP_AREA "Synaptics Area"
 
+/* 8 bit (0-100) */
+#define SYNAPTICS_PROP_TOUCH_BUTTON_AREA "Synaptics Touch Button Area"
+
+/* 8 bit (BOOL) */
+#define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button Sense"
+
 #endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/man/synaptics.man b/man/synaptics.man
index 59fbaac..f92ea0c 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -509,6 +509,27 @@ Ignore movements, scrolling and tapping which take place 
below this edge.
 The option is disabled by default and can be enabled by setting the
 AreaBottomEdge option to any integer value other than zero. Property: 
"Synaptics Area"
 .
+.TP
+.BI "Option \*qTouchButtonArea\*q \*q" integer \*q
+The percentage of touch-button area of Clickpad device.
+.
+The synaptics driver supports "ClickZone" mode, which emulates the buttons
+to click at the bottom of touchpad area.  This option specifies how large
+is the area to be used as the button area.  Passing zero to this disables
+the Clickpad behavior.  The default value is 20.
+Property: "Synaptics Touch Button Area"
+.TP
+.BI "Option \*qTouchButtonSense\*q \*q" integer \*q
+Disable the touch-sensing in the Clickpad button area.
+.
+Since the touchpad is often quite sensitive, the pointer would move
+slightly when you click via Clickpad, and user may miss the aimed
+position.  When this option is set, the cursor movement in the
+Clickpad button area is suppressed, thus the point won't move while
+clicking the button.  On the other hand, this reduces the available
+space on the touchpad.  The default is on.
+Property: "Synaptics Touch Button Sense"
+.
 .LP
 A tap event happens when the finger is touched and released in a time
 interval shorter than MaxTapTime, and the touch and release
diff --git a/src/eventcomm.c b/src/eventcomm.c
index d00d810..ca99f3a 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -166,6 +166,19 @@ event_query_info(LocalDevicePtr local)
 }
 }
 
+static void event_query_clickpad(LocalDevicePtr local)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
+
+priv->is_clickpad = FALSE;
+
+/* 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);
+}
+}
+
 /* Query device for axis ranges */
 static void
 event_query_axis_ranges(LocalDevicePtr local)
@@ -253,6 +266,8 @@ event_query_axis_ranges(LocalDevicePtr local)
   strcat(buf, " triple");
xf86Msg(X_INFO, "%s: buttons:%s\n", local->name, buf);
 }
+
+event_query_clickpad(local);
 }
 
 static Bool
diff --git a/src/properties.c b/src/properties.c
index 4366034..a579479 100644
--- a/src/properties.c
+++ b/src/properties.c
@@ -84,6 +84,8 @@ Atom prop_gestures  = 0;
 Atom prop_capabilities  = 0;
 Atom prop_resolution= 0;
 Atom prop_area  = 0;
+Atom prop_touch_button_area = 0;
+Atom prop_touch_button_sense= 0;
 
 static Atom
 InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
@@ -274,6 +276,10 @@ InitDeviceProperties(LocalDevicePtr local)
 values[2] = para->area_top_edge;
 values[3] = para->area_bottom_edge;
 prop_area = InitAtom(local->dev, SYNAPTICS_PROP_AREA, 32, 4, values);
+
+prop_touch_button_area = InitAtom(local->dev, 
SYNAPTICS_PROP_TOUCH_BUTTON_AREA, 8, 1, ¶->touch_button_a