[PATCH] hid: 3m: Simplify touchsreen emulation logic

[Chase Douglas]

On Sat, 2010-08-14 at 17:29 +0100, Henrik Rydberg wrote:
> The ten-finger 3M screen M2256PW has a kernel driver already
> (hid-3m-pct), but with a couple of glitches:
> 
> 1. The one-touch down/up, which emits BTN_TOUCH events, sometimes does
> not work properly. This results in buttons getting stuck in the
> button-down state in the window manager.
> 
> 2. The action after touch-up in one spot followed by a touch-down in
> another spot sometimes causes a sequence of strange finger-tracing
> events along the perimeter of the rectangle between the two spots. The
> root of the problem lies in how the kernel inteprets the HID hybrid
> event type which is used by the 3M device [1].
> 
> While a proper HID solution is being worked out upstream, this patch
> simplifies the driver logic such that BTN_TOUCH and ABS_X/Y events are
> emitted properly, in effect papering over the worst problems. Suggested
> for inclusion in Maverick.
> 
> [1] Suggested by Stephane Chatty, who also confirms the observed problems.
> 
> Signed-off-by: Henrik Rydberg <rydberg at euromail.se>
> ---
>  drivers/hid/hid-3m-pct.c |   47 +++++++++------------------------------------
>  1 files changed, 10 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/hid/hid-3m-pct.c b/drivers/hid/hid-3m-pct.c
> index 2a0d56b..fc57c0c 100644
> --- a/drivers/hid/hid-3m-pct.c
> +++ b/drivers/hid/hid-3m-pct.c
> @@ -125,9 +125,7 @@ static int mmm_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>   */
>  static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
>  {
> -	struct mmm_finger *oldest = 0;
> -	bool pressed = false, released = false;
> -	int i;
> +	int i, index = -1;
>  
>  	/*
>  	 * we need to iterate on all fingers to decide if we have a press
> @@ -149,45 +147,20 @@ static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
>  			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR,
>  						wide ? f->h : f->w);
>  			input_mt_sync(input);
> -			/*
> -			 * touchscreen emulation: maintain the age rank
> -			 * of this finger, decide if we have a press
> -			 */
> -			if (f->rank == 0) {
> -				f->rank = ++(md->num);
> -				if (f->rank == 1)
> -					pressed = true;
> -			}
> -			if (f->rank == 1)
> -				oldest = f;
> -		} else {
> -			/* this finger took off the screen */
> -			/* touchscreen emulation: maintain age rank of others */
> -			int j;
> -
> -			for (j = 0; j < 10; ++j) {
> -				struct mmm_finger *g = &md->f[j];
> -				if (g->rank > f->rank) {
> -					g->rank--;
> -					if (g->rank == 1)
> -						oldest = g;
> -				}
> -			}
> -			f->rank = 0;
> -			--(md->num);
> -			if (md->num == 0)
> -				released = true;
> +
> +			if (index < 0)
> +				index = i;
>  		}
>  		f->valid = 0;
>  	}
>  
>  	/* touchscreen emulation */
> -	if (oldest) {
> -		if (pressed)
> -			input_event(input, EV_KEY, BTN_TOUCH, 1);
> -		input_event(input, EV_ABS, ABS_X, oldest->x);
> -		input_event(input, EV_ABS, ABS_Y, oldest->y);
> -	} else if (released) {
> +	if (index >= 0) {
> +		struct mmm_finger *f = &md->f[index];
> +		input_event(input, EV_KEY, BTN_TOUCH, 1);
> +		input_event(input, EV_ABS, ABS_X, f->x);
> +		input_event(input, EV_ABS, ABS_Y, f->y);
> +	} else {
>  		input_event(input, EV_KEY, BTN_TOUCH, 0);
>  	}
>  }
> -- 
> 1.7.1

Won't this emit a button touch down and ABS_{X,Y} event every time any
finger moves? I can think of many ways this could break things,
including the ABS_{X,Y} values flittering back and forth between all
fingers touching the screen. This would make dragging and dropping a
nightmare if you accidentally or intentionally touch more than one
finger to the display at a time.

Is the one-touch button down/up issue caused by driver logic or hardware
issues?

I wonder if a better solution would be single-touch emulation by only
sending BTN_TOUCH and ABS_{X,Y} events on the first touch started and
not sending these events on the second touch started when the first is
released.

-- Chase