Re: [Linuxwacom-devel] [PATCH 4/4 v2] BTN_TOOL_FINGER is not for PAD if MT is supported

2012-12-16 Thread Chris Bagwell
On Thu, Dec 13, 2012 at 2:19 PM, Ping Cheng pingli...@gmail.com wrote:

 BTN_TOOL_FINGER indicates single touch/first finger if MT is enabled

 Signed-off-by: Ping Cheng pi...@wacom.com
 Reviewed-by: Jason Gerecke killert...@gmail.com


Oh, good point.  I assume the new Intuos5's are tracked as Protocol 5
devices but also with MT.

 If this is true then probably all the checks for WCM_PROTOCOL_GENERIC in
wcmUSB.c need to be reviewed.

Reviewed-by: Chris Bagwell ch...@cnpbagwell.com


---
  src/wcmUSB.c |   13 -
  1 file changed, 8 insertions(+), 5 deletions(-)

 diff --git a/src/wcmUSB.c b/src/wcmUSB.c
 index ef19b51..1f1db35 100644
 --- a/src/wcmUSB.c
 +++ b/src/wcmUSB.c
 @@ -1441,13 +1441,15 @@ static void usbParseBTNEvent(WacomCommonPtr common,
   * Translates a tool code from the kernel (e.g. BTN_TOOL_PEN) into the
   * corresponding device type for the driver (e.g. STYLUS_ID).
   *
 + * @param[in] common
   * @param[in] type  Linux input tool type (e.g. EV_KEY)
   * @param[in] code  Linux input tool code (e.g. BTN_STYLUS_PEN)
 - * @param[in] protocol  Wacom protocol type (e.g. WCM_PROTOCOL_GENERIC)
   * @return  Wacom device ID (e.g. STYLUS_ID) or 0 if no match.
   */
 -static int toolTypeToDeviceType(int type, int code, int protocol)
 +static int toolTypeToDeviceType(WacomCommonPtr common, int type, int code)
  {
 +   wcmUSBData* private = common-private;
 +
 if (type == EV_KEY) {
 switch(code) {
 case BTN_TOOL_PEN:
 @@ -1457,7 +1459,8 @@ static int toolTypeToDeviceType(int type, int code,
 int protocol)
 return STYLUS_ID;

 case BTN_TOOL_FINGER:
 -   if (protocol != WCM_PROTOCOL_GENERIC)
 +   if ((common-wcmProtocolLevel !=
 WCM_PROTOCOL_GENERIC)
 +!private-wcmUseMT)
 return PAD_ID;
 else
 return TOUCH_ID;
 @@ -1499,7 +1502,7 @@ static int refreshDeviceType(WacomCommonPtr common)
 for (i = 0; i  KEY_MAX; i++)
 {
 if (ISBITSET(keys, i))
 -   device_type = toolTypeToDeviceType(EV_KEY, i,
 common-wcmProtocolLevel);
 +   device_type = toolTypeToDeviceType(common, EV_KEY,
 i);
 if (device_type)
 return device_type;
 }
 @@ -1529,7 +1532,7 @@ static int usbInitToolType(WacomCommonPtr common,
 const struct input_event *even

 for (i = 0; (i  nevents)  !device_type; ++i, event_ptr++)
 {
 -   device_type = toolTypeToDeviceType(event_ptr-type,
 event_ptr-code, common-wcmProtocolLevel);
 +   device_type = toolTypeToDeviceType(common,
 event_ptr-type, event_ptr-code);
 }

 if (!device_type)
 --
 1.7.10.4



 --
 LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
 Remotely access PCs and mobile devices and provide instant support
 Improve your efficiency, and focus on delivering more value-add services
 Discover what IT Professionals Know. Rescue delivers
 http://p.sf.net/sfu/logmein_12329d2d
 ___
 Linuxwacom-devel mailing list
 Linuxwacom-devel@lists.sourceforge.net
 https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


Re: [Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-16 Thread Chris Bagwell
On Thu, Dec 13, 2012 at 2:16 PM, Ping Cheng pingli...@gmail.com wrote:

 We use true MT protocol for MT devices in kernel now. This code
 was introduced to deal with ABS_TOOL_*TAP events loss issue. It
 is uncessary any more. And its existence makes it hard to support
 generic PAD cleanly.


I can't think of any negative effects removing these will do with protocol
4 and generic devices with MT events using the latest input-wacom drivers.
There is the theoritical issue the code is solving so if its possible to
solve what ever the bug is without deleting this then that would be my
preference.

Since I don't know exact issue your seeing, I can't really comment on if
its a good idea to delete this.

If you do decide to delete this logic then I'd say we official do not
support the *old* 2 finger drivers and so I'd delete the Protocol 4
DOUBLETAP and TRIPLETAP logic in wcmUSB.c and  wcmValidateDevice.c as part
of this patch as well.  Touchpad's using old drivers but without this code
become close to un-usable with all the cursor jumping that occurs.

Chris


Signed-off-by: Ping Cheng pi...@wacom.com
 Acked-by: Jason Gerecke killert...@gmail.com
 ---
 v2: only patch 2/4 has code change from Jason's comments. To ease the
 review
 and merge effort, I updated all to v2 with acked-by and reviewed-by tags.
 ---
  src/wcmUSB.c |   34 ++
  1 file changed, 2 insertions(+), 32 deletions(-)

 diff --git a/src/wcmUSB.c b/src/wcmUSB.c
 index e192489..4b5f53b 100644
 --- a/src/wcmUSB.c
 +++ b/src/wcmUSB.c
 @@ -37,7 +37,6 @@ typedef struct {
 Bool wcmPenTouch;
 Bool wcmUseMT;
 int wcmMTChannel;
 -   int wcmPrevChannel;
 int wcmEventCnt;
 struct input_event wcmEvents[MAX_USB_EVENTS];
 int nbuttons;/* total number of buttons */
 @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
 return;
 }

 -   /* Protocol 5 devices have some complications related to DUALINPUT
 -* support and can not use below logic to recover from input
 -* event filtering.  Instead, just live with occasional dropped
 -* event.  Since tools are dynamically assigned a channel #, the
 -* structure must be initialized to known starting values
 +   /* Protocol 5 tools are dynamically assigned with channel numbers.
 +* The structure must be initialized to known starting values
  * when first entering proximity to discard invalid data.
  */
 if (common-wcmProtocolLevel == WCM_PROTOCOL_5)
 @@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
 memset(common-wcmChannel[channel],0,
sizeof(WacomChannel));
 }
 -   else
 -   {
 -   /* Because of linux input filtering, each switch to a new
 -* tool is required to have its initial values match values
 -* of previous tool.
 -*
 -* For normal case, all tools are in channel 0 and so
 -* no issue.  Protocol 4 2FGT devices split between
 -* two channels though and so need to copy data between
 -* channels to prevent loss of events; which could
 -* lead to cursor jumps.
 -*
 -* PAD device is special.  It shares no events
 -* with other channels and is always in proximity.
 -* So it requires no copying of data from other
 -* channels.
 -*/
 -   if (private-wcmPrevChannel != channel 
 -   channel != PAD_CHANNEL 
 -   private-wcmPrevChannel != PAD_CHANNEL)
 -   {
 -   common-wcmChannel[channel].work =
 -
 common-wcmChannel[private-wcmPrevChannel].work;
 -   private-wcmPrevChannel = channel;
 -   }
 -   }

 ds = common-wcmChannel[channel].work;
 dslast = common-wcmChannel[channel].valid.state;
 --
 1.7.10.4



 --
 LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
 Remotely access PCs and mobile devices and provide instant support
 Improve your efficiency, and focus on delivering more value-add services
 Discover what IT Professionals Know. Rescue delivers
 http://p.sf.net/sfu/logmein_12329d2d
 ___
 Linuxwacom-devel mailing list
 Linuxwacom-devel@lists.sourceforge.net
 https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT 

Re: [Linuxwacom-devel] [PATCH 3/4 v2] Send generic PAD events before other events

2012-12-16 Thread Ping Cheng
On Sun, Dec 16, 2012 at 1:40 PM, Chris Bagwell ch...@cnpbagwell.com wrote:




 On Thu, Dec 13, 2012 at 2:18 PM, Ping Cheng pingli...@gmail.com wrote:

 If we wait until we finish other verifications, we could miss
 PAD events since they will be filtered out when there are no
 motion events sent simultaneously.



 Signed-off-by: Ping Cheng pingli...@gmail.com
 Acked-by: Jason Gerecke killert...@gmail.com
 ---
  src/wcmUSB.c |   13 +
  1 file changed, 13 insertions(+)

 diff --git a/src/wcmUSB.c b/src/wcmUSB.c
 index 0ea2093..f15a6a2 100644
 --- a/src/wcmUSB.c
 +++ b/src/wcmUSB.c
 @@ -1655,6 +1655,19 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
 {
 usbParseKeyEvent(common, event, channel);
 usbParseBTNEvent(common, event,
 private-wcmBTNChannel);
 +
 +   /* send PAD events now for generic devices.
 Otherwise,
 +* they are filtered out when there are no motion
 events.
 +*/
 +   if ((common-wcmProtocolLevel ==
 WCM_PROTOCOL_GENERIC)
 +   
 (common-wcmChannel[private-wcmBTNChannel].dirty))
 +{
 +   DBG(10, common, Dirty flag set on
 channel %d; 
 +   sending event.\n,
 private-wcmBTNChannel);
 +
 common-wcmChannel[private-wcmBTNChannel].dirty = FALSE;
 +   wcmEvent(common, private-wcmBTNChannel,
 +
  common-wcmChannel[private-wcmBTNChannel].work);
 +   }



 I don't understand this one.  How are they filtered out?  Is
 usbDispatchEvents() returning early or is it some other function?


It is returned immediarely by the next if statement since both device_type
and proximity are zero when no touch events.

I can tell from the if() that its a Generic device... which one though?


All generic devices that support PAD the new way in the kernel.



 I suspect the events get ignored because we are not initlizing the ds
 structure correctly.  I'd prefer to get that resolved instead of
 duplicating logic in two places.


For generic PAD, we can not initialize ds correctly. There is no ds for
generic PAD. It is ride on one of the touch points, normally the first one.


 Or if we decide this route is best/easiest then I'd prefer if we at least
 move all Button processing earlier and then at the for() loop below we skip
 over button channel always.


Other PADs do not need this route since they have their own channel (ds).

Ping
--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


Re: [Linuxwacom-devel] [PATCH 3/4 v2] Send generic PAD events before other events

2012-12-16 Thread Chris Bagwell
On Sun, Dec 16, 2012 at 7:17 PM, Ping Cheng pingli...@gmail.com wrote:

 On Sun, Dec 16, 2012 at 1:40 PM, Chris Bagwell ch...@cnpbagwell.comwrote:




 On Thu, Dec 13, 2012 at 2:18 PM, Ping Cheng pingli...@gmail.com wrote:

 If we wait until we finish other verifications, we could miss
 PAD events since they will be filtered out when there are no
 motion events sent simultaneously.



 Signed-off-by: Ping Cheng pingli...@gmail.com
 Acked-by: Jason Gerecke killert...@gmail.com
 ---
  src/wcmUSB.c |   13 +
  1 file changed, 13 insertions(+)

 diff --git a/src/wcmUSB.c b/src/wcmUSB.c
 index 0ea2093..f15a6a2 100644
 --- a/src/wcmUSB.c
 +++ b/src/wcmUSB.c
 @@ -1655,6 +1655,19 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
 {
 usbParseKeyEvent(common, event, channel);
 usbParseBTNEvent(common, event,
 private-wcmBTNChannel);
 +
 +   /* send PAD events now for generic devices.
 Otherwise,
 +* they are filtered out when there are no
 motion events.
 +*/
 +   if ((common-wcmProtocolLevel ==
 WCM_PROTOCOL_GENERIC)
 +   
 (common-wcmChannel[private-wcmBTNChannel].dirty))
 +{
 +   DBG(10, common, Dirty flag set on
 channel %d; 
 +   sending event.\n,
 private-wcmBTNChannel);
 +
 common-wcmChannel[private-wcmBTNChannel].dirty = FALSE;
 +   wcmEvent(common, private-wcmBTNChannel,
 +
  common-wcmChannel[private-wcmBTNChannel].work);
 +   }



 I don't understand this one.  How are they filtered out?  Is
 usbDispatchEvents() returning early or is it some other function?


 It is returned immediarely by the next if statement since both device_type
 and proximity are zero when no touch events.


I see.  Now I can understand why you had patch 1/4 as well.

The intent was that the button channel's ds-device_type and ds-proximity
is fixed and initialized one time up front by usbWcmInitPadState(). That
function was added specifically because its known that no BTN_TOOL_* like
event will kick the code to set up PAD's device_type and proximity for
generic devices.

The code in 1/4  patch was being to agressive and setting these fields to
zero and then confusing all kinds of stuff from there.  If we submit patch
1/4 then can this patch be dropped?  Does it do anything useful once
device_type and proximity stay at good values for PAD?

Chris


 I can tell from the if() that its a Generic device... which one though?


 All generic devices that support PAD the new way in the kernel.



 I suspect the events get ignored because we are not initlizing the ds
 structure correctly.  I'd prefer to get that resolved instead of
 duplicating logic in two places.


 For generic PAD, we can not initialize ds correctly. There is no ds for
 generic PAD. It is ride on one of the touch points, normally the first one.


 Or if we decide this route is best/easiest then I'd prefer if we at least
 move all Button processing earlier and then at the for() loop below we skip
 over button channel always.


 Other PADs do not need this route since they have their own channel (ds).

 Ping

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


Re: [Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-16 Thread Ping Cheng
Hi Chris,

Thank you for your reply and your comments. Most importantly, thank you for
your time.

The patch does not have to be merged. Let's figure out if the duplicated
channel is necessary or not.


On Sun, Dec 16, 2012 at 2:56 PM, Chris Bagwell ch...@cnpbagwell.com wrote:


 On Thu, Dec 13, 2012 at 2:16 PM, Ping Cheng pingli...@gmail.com wrote:

 We use true MT protocol for MT devices in kernel now. This code
 was introduced to deal with ABS_TOOL_*TAP events loss issue. It
 is uncessary any more. And its existence makes it hard to support
 generic PAD cleanly.


 I can't think of any negative effects removing these will do with protocol
 4 and generic devices with MT events using the latest input-wacom drivers.
 There is the theoritical issue the code is solving so if its possible to
 solve what ever the bug is without deleting this then that would be my
 preference.

 Since I don't know exact issue your seeing, I can't really comment on if
 its a good idea to delete this.

 If you do decide to delete this logic


We can keep this logic. But it does not provide the information required
for generic device any more.

For P5 devices, we do not use it anyway.

The theoritical issues only happen:

if incoming tool lands on outgoing tool's axes, As you mentioned we deal
with absolute values, this case is hard to happen, plus ignoring the first
motion events would avoid this issue;

if user pressing a button while bringing tool out of prox, then pressing
the same button while bringing it back in. In this case, we want to send
the last button up since we do not know if the tool is going to come back
or not.

In both cases, duplicated channel is unnecessary.


then I'd say we official do not support the *old* 2 finger drivers and so
 I'd delete the Protocol 4 DOUBLETAP and TRIPLETAP logic in wcmUSB.c and
 wcmValidateDevice.c as part of this patch as well.


Those TAP code are used by ISDV4 touch devices, which I can not afford to
delete.


 Touchpad's using old drivers but without this code become close to
 un-usable with all the cursor jumping that occurs.


Which old driver are we talking about, in input-wacom or in old kernel
releases? Which versions?

The current 2.6.30 kernel driver for bamboo in input-wacom is un-usable
with the current xf86-input-wacom. We have to use xf86-input-synaptics. I
thought that was on purpose.

Ping




 Chris


 Signed-off-by: Ping Cheng pi...@wacom.com
 Acked-by: Jason Gerecke killert...@gmail.com
 ---
 v2: only patch 2/4 has code change from Jason's comments. To ease the
 review
 and merge effort, I updated all to v2 with acked-by and reviewed-by tags.
 ---
  src/wcmUSB.c |   34 ++
  1 file changed, 2 insertions(+), 32 deletions(-)

 diff --git a/src/wcmUSB.c b/src/wcmUSB.c
 index e192489..4b5f53b 100644
 --- a/src/wcmUSB.c
 +++ b/src/wcmUSB.c
 @@ -37,7 +37,6 @@ typedef struct {
 Bool wcmPenTouch;
 Bool wcmUseMT;
 int wcmMTChannel;
 -   int wcmPrevChannel;
 int wcmEventCnt;
 struct input_event wcmEvents[MAX_USB_EVENTS];
 int nbuttons;/* total number of buttons */
 @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
 return;
 }

 -   /* Protocol 5 devices have some complications related to DUALINPUT
 -* support and can not use below logic to recover from input
 -* event filtering.  Instead, just live with occasional dropped
 -* event.  Since tools are dynamically assigned a channel #, the
 -* structure must be initialized to known starting values
 +   /* Protocol 5 tools are dynamically assigned with channel numbers.
 +* The structure must be initialized to known starting values
  * when first entering proximity to discard invalid data.
  */
 if (common-wcmProtocolLevel == WCM_PROTOCOL_5)
 @@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
 memset(common-wcmChannel[channel],0,
sizeof(WacomChannel));
 }
 -   else
 -   {
 -   /* Because of linux input filtering, each switch to a new
 -* tool is required to have its initial values match
 values
 -* of previous tool.
 -*
 -* For normal case, all tools are in channel 0 and so
 -* no issue.  Protocol 4 2FGT devices split between
 -* two channels though and so need to copy data between
 -* channels to prevent loss of events; which could
 -* lead to cursor jumps.
 -*
 -* PAD device is special.  It shares no events
 -* with other channels and is always in proximity.
 -* So it requires no copying of data from other
 -* channels.
 -*/
 -   if (private-wcmPrevChannel != channel 
 -   

Re: [Linuxwacom-devel] [PATCH 3/4 v2] Send generic PAD events before other events

2012-12-16 Thread Ping Cheng
On Sunday, December 16, 2012, Chris Bagwell wrote:


 On Sun, Dec 16, 2012 at 7:17 PM, Ping Cheng 
 pingli...@gmail.comjavascript:_e({}, 'cvml', 'pingli...@gmail.com');
  wrote:

 On Sun, Dec 16, 2012 at 1:40 PM, Chris Bagwell 
 ch...@cnpbagwell.comjavascript:_e({}, 'cvml', 'ch...@cnpbagwell.com');
  wrote:




 On Thu, Dec 13, 2012 at 2:18 PM, Ping Cheng 
 pingli...@gmail.comjavascript:_e({}, 'cvml', 'pingli...@gmail.com');
  wrote:

 If we wait until we finish other verifications, we could miss
 PAD events since they will be filtered out when there are no
 motion events sent simultaneously.



 Signed-off-by: Ping Cheng pingli...@gmail.com javascript:_e({},
 'cvml', 'pingli...@gmail.com');
 Acked-by: Jason Gerecke killert...@gmail.com javascript:_e({},
 'cvml', 'killert...@gmail.com');
 ---
  src/wcmUSB.c |   13 +
  1 file changed, 13 insertions(+)

 diff --git a/src/wcmUSB.c b/src/wcmUSB.c
 index 0ea2093..f15a6a2 100644
 --- a/src/wcmUSB.c
 +++ b/src/wcmUSB.c
 @@ -1655,6 +1655,19 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
 {
 usbParseKeyEvent(common, event, channel);
 usbParseBTNEvent(common, event,
 private-wcmBTNChannel);
 +
 +   /* send PAD events now for generic devices.
 Otherwise,
 +* they are filtered out when there are no
 motion events.
 +*/
 +   if ((common-wcmProtocolLevel ==
 WCM_PROTOCOL_GENERIC)
 +   
 (common-wcmChannel[private-wcmBTNChannel].dirty))
 +{
 +   DBG(10, common, Dirty flag set on
 channel %d; 
 +   sending event.\n,
 private-wcmBTNChannel);
 +
 common-wcmChannel[private-wcmBTNChannel].dirty = FALSE;
 +   wcmEvent(common, private-wcmBTNChannel,
 +
  common-wcmChannel[private-wcmBTNChannel].work);
 +   }



 I don't understand this one.  How are they filtered out?  Is
 usbDispatchEvents() returning early or is it some other function?


 It is returned immediarely by the next if statement since both
 device_type and proximity are zero when no touch events.


 I see.  Now I can understand why you had patch 1/4 as well.

 The intent was that the button channel's ds-device_type and ds-proximity
 is fixed and initialized one time up front by usbWcmInitPadState(). That
 function was added specifically because its known that no BTN_TOOL_* like
 event will kick the code to set up PAD's device_type and proximity for
 generic devices.

 The code in 1/4  patch was being to agressive and setting these fields to
 zero and then confusing all kinds of stuff from there.  If we submit patch
 1/4 then can this patch be dropped?  Does it do anything useful once
 device_type and proximity stay at good values for PAD?


I have to test it again to be sure. I think this patch was needed too. Will
do it tomorrow.

Thanks.

Ping

I can tell from the if() that its a Generic device... which one though?


 All generic devices that support PAD the new way in the kernel.



 I suspect the events get ignored because we are not initlizing the ds
 structure correctly.  I'd prefer to get that resolved instead of
 duplicating logic in two places.


 For generic PAD, we can not initialize ds correctly. There is no ds for
 generic PAD. It is ride on one of the touch points, normally the first one.


 Or if we decide this route is best/easiest then I'd prefer if we at
 least move all Button processing earlier and then at the for() loop below
 we skip over button channel always.


 Other PADs do not need this route since they have their own channel (ds).

 Ping



--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


Re: [Linuxwacom-devel] [PATCH 3/4 v2] Send generic PAD events before other events

2012-12-16 Thread Ping Cheng
On Sunday, December 16, 2012, Ping Cheng wrote:

 On Sunday, December 16, 2012, Chris Bagwell wrote:


 On Sun, Dec 16, 2012 at 7:17 PM, Ping Cheng pingli...@gmail.com wrote:

 On Sun, Dec 16, 2012 at 1:40 PM, Chris Bagwell ch...@cnpbagwell.comwrote:




 On Thu, Dec 13, 2012 at 2:18 PM, Ping Cheng pingli...@gmail.comwrote:

 If we wait until we finish other verifications, we could miss
 PAD events since they will be filtered out when there are no
 motion events sent simultaneously.



 Signed-off-by: Ping Cheng pingli...@gmail.com
 Acked-by: Jason Gerecke killert...@gmail.com
 ---
  src/wcmUSB.c |   13 +
  1 file changed, 13 insertions(+)

 diff --git a/src/wcmUSB.c b/src/wcmUSB.c
 index 0ea2093..f15a6a2 100644
 --- a/src/wcmUSB.c
 +++ b/src/wcmUSB.c
 @@ -1655,6 +1655,19 @@ static void usbDispatchEvents(InputInfoPtr
 pInfo)
 {
 usbParseKeyEvent(common, event, channel);
 usbParseBTNEvent(common, event,
 private-wcmBTNChannel);
 +
 +   /* send PAD events now for generic devices.
 Otherwise,
 +* they are filtered out when there are no
 motion events.
 +*/
 +   if ((common-wcmProtocolLevel ==
 WCM_PROTOCOL_GENERIC)
 +   
 (common-wcmChannel[private-wcmBTNChannel].dirty))
 +{
 +   DBG(10, common, Dirty flag set on
 channel %d; 
 +   sending event.\n,
 private-wcmBTNChannel);
 +
 common-wcmChannel[private-wcmBTNChannel].dirty = FALSE;
 +   wcmEvent(common,
 private-wcmBTNChannel,
 +
  common-wcmChannel[private-wcmBTNChannel].work);
 +   }



 I don't understand this one.  How are they filtered out?  Is
 usbDispatchEvents() returning early or is it some other function?


 It is returned immediarely by the next if statement since both
 device_type and proximity are zero when no touch events.


 I see.  Now I can understand why you had patch 1/4 as well.

 The intent was that the button channel's ds-device_type and
 ds-proximity is fixed and initialized one time up front by
 usbWcmInitPadState(). That function was added specifically because its
 known that no BTN_TOOL_* like event will kick the code to set up PAD's
 device_type and proximity for generic devices.

 The code in 1/4  patch was being to agressive and setting these fields to
 zero and then confusing all kinds of stuff from there.  If we submit patch
 1/4 then can this patch be dropped?  Does it do anything useful once
 device_type and proximity stay at good values for PAD?


I can not sleep well without getting the job done ;-).

No, we can not drop this patch. Although type and prox are good for PAD, ds
will never be on PAD channel for generic devices. We do not know if there
are touch events from a packet or not until after we parsed the packet. So,
calling wcmEvent immediately is the best option if we do not want to go
through all the other channels here.

Chris, can I assume your Acked/Reviewed-by for the patch set?

Ping


 I can tell from the if() that its a Generic device... which one though?


 All generic devices that support PAD the new way in the kernel.



 I suspect the events get ignored because we are not initlizing the ds
 structure correctly.  I'd prefer to get that resolved instead of
 duplicating logic in two places.


 For generic PAD, we can not initialize ds correctly. There is no ds for
 generic PAD. It is ride on one of the touch points, normally the first one.


 Or if we decide this route is best/easiest then I'd prefer if we at
 least move all Button processing earlier and then at the for() loop below
 we skip over button channel always.


 Other PADs do not need this route since they have their own channel (ds).

 Ping



--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


[Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-16 Thread Ping Cheng
On Sunday, December 16, 2012, Peter Hutterer wrote:

 On Thu, Dec 13, 2012 at 10:19:09PM -0800, Ping Cheng wrote:
  On Thursday, December 13, 2012, Peter Hutterer wrote:
 
   On Thu, Dec 13, 2012 at 12:16:52PM -0800, Ping Cheng wrote:
We use true MT protocol for MT devices in kernel now. This code
was introduced to deal with ABS_TOOL_*TAP events loss issue. It
is uncessary any more. And its existence makes it hard to support
generic PAD cleanly.
   
Signed-off-by: Ping Cheng pi...@wacom.com javascript:;
Acked-by: Jason Gerecke killert...@gmail.com javascript:;
---
v2: only patch 2/4 has code change from Jason's comments. To ease the
   review
and merge effort, I updated all to v2 with acked-by and reviewed-by
 tags.
  
   I honestly don't know this code well enough to do more than a cursory
   review (on all 4 patches), but is there any chance we can have
 test-cases
   for this? or does it rely on kernel versions too much?
 
 
  Yes, it relies on the kernel version and the device (Bamboo2, the 2FG
 touch
  series).
 
  How much chance do we have for a user to run kernel 2.6.30 with our
 latest
  X driver which does not support X servers older than 1.7?

 I personally don't care about 2.6.30, but I do about 2.6.32 (for obvious
 reasons :).


2.6.32 is important to me for the same reason. TBH, RHEL6 is one
of the systems I have to test ...

Like it or not, we are in the same boat ;-).

Ping


 Note that the kernel is somewhat of a special case too, you can
 run latest userspace on an old kernel, so in theory. we can't run an older
 X
 server with the new driver, but i'm sure we can run on most 2.6 kernels.

 Cheers,
Peter

---
 src/wcmUSB.c |   34 ++
 1 file changed, 2 insertions(+), 32 deletions(-)
   
diff --git a/src/wcmUSB.c b/src/wcmUSB.c
index e192489..4b5f53b 100644
--- a/src/wcmUSB.c
+++ b/src/wcmUSB.c
@@ -37,7 +37,6 @@ typedef struct {
  Bool wcmPenTouch;
  Bool wcmUseMT;
  int wcmMTChannel;
- int wcmPrevChannel;
  int wcmEventCnt;
  struct input_event wcmEvents[MAX_USB_EVENTS];
  int nbuttons;/* total number of buttons */
@@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr
 pInfo)
  return;
  }
   
- /* Protocol 5 devices have some complications related to
 DUALINPUT
-  * support and can not use below logic to recover from input
-  * event filtering.  Instead, just live with occasional dropped
-  * event.  Since tools are dynamically assigned a channel #,
 the
-  * structure must be initialized to known starting values
+ /* Protocol 5 tools are dynamically assigned with channel
 numbers.
+  * The structure must be initialized to known starting values
   * when first entering proximity to discard invalid data.
   */
  if (common-wcmProtocolLevel == WCM_PROTOCOL_5)
@@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr
 pInfo)
  memset(common-wcmChannel[channel],0,
 sizeof(WacomChannel));
  }
- else
- {
- /* Because of linux input filtering, each switch to a
 new
-  * tool is required to have its initial values match
 values
-  * of previous tool.
-  *
-  * For normal case, all tools are in channel 0 and so
-  * no issue.  Protocol 4 2FGT devices split between
-  * two channels though and so need to copy data between
-  * channels to prevent loss of events; which could
-  * lead to cursor jumps.
-  *
-  * PAD device is special.  It shares no events
-  * with other channels and is always in proximity.
-  * So it requires no copying of data from other
-  * channels.
-  */
- if (private-wcmPrevChannel != channel 
- channel != PAD_CHANNEL 
- private-wcmPrevChannel != PAD_CHANNEL)
- {
- common-wcmChannel[channel].work =
-
   common-wcmChannel[private-wcmPrevChannel].work;
-
--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel