Dmitry,
Thank you for your effort in updating the patch. Most changes work for
me. I do have comments related to raw touch data scaling for MT and
single touch legacy client support. Details are inline.
Ping
On Tue, Dec 28, 2010 at 11:40 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> Hi Ping,
>
> On Fri, Dec 17, 2010 at 09:37:54AM -0800, Ping Cheng wrote:
>> Signed-off-by: Ping Cheng <pingc@xxxxxxxxx>
>> ---
>> Âdrivers/input/touchscreen/wacom_w8001.c | Â 89 ++++++++++++++++++++++++++++---
>> Â1 files changed, 82 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>> index 59664a8..763eb8f 100644
>> --- a/drivers/input/touchscreen/wacom_w8001.c
>> +++ b/drivers/input/touchscreen/wacom_w8001.c
>> @@ -3,6 +3,7 @@
>> Â *
>> Â * Copyright (c) 2008 Jaya Kumar
>> Â * Copyright (c) 2010 Red Hat, Inc.
>> + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@xxxxxxxxx>
>> Â *
>> Â * This file is subject to the terms and conditions of the GNU General Public
>> Â * License. See the file COPYING in the main directory of this archive for
>> @@ -86,6 +87,12 @@ struct w8001 {
>> Â Â Â char phys[32];
>> Â Â Â int type;
>> Â Â Â unsigned int pktlen;
>> + Â Â bool pen_in_prox;
>> + Â Â bool has_touch;
>
> We already have type, why do we need these 2 fields a well?
>
> Actually, I tried massaging the patch a bit, could you please tell me if
> the patch below still works for you?
>
> Thanks.
>
> --
> Dmitry
>
>
> Input: wacom_w8001 - add one finger touch support
>
> From: Ping Cheng <pinglinux@xxxxxxxxx>
>
> Signed-off-by: Ping Cheng <pingc@xxxxxxxxx>
> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxx>
> ---
>
> Âdrivers/input/touchscreen/wacom_w8001.c | Â123 +++++++++++++++++++++++++------
> Â1 files changed, 101 insertions(+), 22 deletions(-)
>
>
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 4774c09..ce8c8e1 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -3,6 +3,7 @@
> Â*
> Â* Copyright (c) 2008 Jaya Kumar
> Â* Copyright (c) 2010 Red Hat, Inc.
> + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@xxxxxxxxx>
> Â*
> Â* This file is subject to the terms and conditions of the GNU General Public
> Â* License. See the file COPYING in the main directory of this archive for
> @@ -63,11 +64,11 @@ struct w8001_coord {
>
> Â/* touch query reply packet */
> Âstruct w8001_touch_query {
> + Â Â Â u16 x;
> + Â Â Â u16 y;
> Â Â Â Âu8 panel_res;
> Â Â Â Âu8 capacity_res;
> Â Â Â Âu8 sensor_id;
> - Â Â Â u16 x;
> - Â Â Â u16 y;
> Â};
>
> Â/*
> @@ -86,9 +87,13 @@ struct w8001 {
> Â Â Â Âchar phys[32];
> Â Â Â Âint type;
> Â Â Â Âunsigned int pktlen;
> + Â Â Â u16 max_touch_x;
> + Â Â Â u16 max_touch_y;
> + Â Â Â u16 max_pen_x;
> + Â Â Â u16 max_pen_y;
> Â};
>
> -static void parse_data(u8 *data, struct w8001_coord *coord)
> +static void parse_pen_data(u8 *data, struct w8001_coord *coord)
> Â{
> Â Â Â Âmemset(coord, 0, sizeof(*coord));
>
> @@ -112,7 +117,14 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
> Â Â Â Âcoord->tilt_y = data[8] & 0x7F;
> Â}
>
> -static void parse_touch(struct w8001 *w8001)
> +static void parse_single_touch(u8 *data, struct w8001_coord *coord)
> +{
> + Â Â Â coord->x = (data[1] << 7) | data[2];
> + Â Â Â coord->y = (data[3] << 7) | data[4];
> + Â Â Â coord->tsw = data[0] & 0x01;
> +}
> +
> +static void parse_multi_touch(struct w8001 *w8001)
> Â{
> Â Â Â Âstruct input_dev *dev = w8001->dev;
> Â Â Â Âunsigned char *data = w8001->data;
> @@ -124,8 +136,8 @@ static void parse_touch(struct w8001 *w8001)
> Â Â Â Â Â Â Â Âinput_mt_slot(dev, i);
> Â Â Â Â Â Â Â Âinput_mt_report_slot_state(dev, MT_TOOL_FINGER, touch);
> Â Â Â Â Â Â Â Âif (touch) {
> - Â Â Â Â Â Â Â Â Â Â Â int x = (data[6 * i + 1] << 7) | (data[6 * i + 2]);
> - Â Â Â Â Â Â Â Â Â Â Â int y = (data[6 * i + 3] << 7) | (data[6 * i + 4]);
> + Â Â Â Â Â Â Â Â Â Â Â int x = (data[6 * i + 1] << 7) | data[6 * i + 2];
> + Â Â Â Â Â Â Â Â Â Â Â int y = (data[6 * i + 3] << 7) | data[6 * i + 4];
Since you scaled MT max_touch_x/y to max_pen_x/y, you would need to
scale the x and y here as well. I didn't scale MT touch data since MT
protocol can report touch resolution on its own. See more comments
below.
> Â Â Â Â Â Â Â Â Â Â Â Â/* data[5,6] and [11,12] is finger capacity */
>
> Â Â Â Â Â Â Â Â Â Â Â Âinput_report_abs(dev, ABS_MT_POSITION_X, x);
> @@ -151,6 +163,15 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
> Â Â Â Âquery->y = data[5] << 9;
> Â Â Â Âquery->y |= data[6] << 2;
> Â Â Â Âquery->y |= (data[2] >> 3) & 0x3;
> +
> + Â Â Â /* Early days' single-finger touch models need the following defaults */
> + Â Â Â if (!query->x && !query->y) {
> + Â Â Â Â Â Â Â query->x = 1024;
> + Â Â Â Â Â Â Â query->y = 1024;
> + Â Â Â Â Â Â Â if (query->panel_res)
> + Â Â Â Â Â Â Â Â Â Â Â query->x = query->y = (1 << query->panel_res);
> + Â Â Â Â Â Â Â query->panel_res = 10;
> + Â Â Â }
> Â}
>
> Âstatic void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> @@ -160,16 +181,15 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> Â Â Â Â/*
> Â Â Â Â * We have 1 bit for proximity (rdy) and 3 bits for tip, side,
> Â Â Â Â * side2/eraser. If rdy && f2 are set, this can be either pen + side2,
> - Â Â Â Â* or eraser. assume
> + Â Â Â Â* or eraser. Assume:
> Â Â Â Â * - if dev is already in proximity and f2 is toggled â pen + side2
> Â Â Â Â * - if dev comes into proximity with f2 set â eraser
> Â Â Â Â * If f2 disappears after assuming eraser, fake proximity out for
> Â Â Â Â * eraser and in for pen.
> Â Â Â Â */
>
> - Â Â Â if (!w8001->type) {
> - Â Â Â Â Â Â Â w8001->type = coord->f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
> - Â Â Â } else if (w8001->type == BTN_TOOL_RUBBER) {
> + Â Â Â switch (w8001->type) {
> + Â Â Â case BTN_TOOL_RUBBER:
> Â Â Â Â Â Â Â Âif (!coord->f2) {
> Â Â Â Â Â Â Â Â Â Â Â Âinput_report_abs(dev, ABS_PRESSURE, 0);
> Â Â Â Â Â Â Â Â Â Â Â Âinput_report_key(dev, BTN_TOUCH, 0);
> @@ -179,8 +199,21 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> Â Â Â Â Â Â Â Â Â Â Â Âinput_sync(dev);
> Â Â Â Â Â Â Â Â Â Â Â Âw8001->type = BTN_TOOL_PEN;
> Â Â Â Â Â Â Â Â}
> - Â Â Â } else {
> + Â Â Â Â Â Â Â break;
> +
> + Â Â Â case BTN_TOOL_FINGER:
> + Â Â Â Â Â Â Â input_report_key(dev, BTN_TOUCH, 0);
> + Â Â Â Â Â Â Â input_report_key(dev, BTN_TOOL_FINGER, 0);
> + Â Â Â Â Â Â Â input_sync(dev);
> + Â Â Â Â Â Â Â /* fall through */
> +
> + Â Â Â case KEY_RESERVED:
> + Â Â Â Â Â Â Â w8001->type = coord->f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
> + Â Â Â Â Â Â Â break;
> +
> + Â Â Â default:
> Â Â Â Â Â Â Â Âinput_report_key(dev, BTN_STYLUS2, coord->f2);
> + Â Â Â Â Â Â Â break;
> Â Â Â Â}
>
> Â Â Â Âinput_report_abs(dev, ABS_X, coord->x);
> @@ -192,7 +225,30 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> Â Â Â Âinput_sync(dev);
>
> Â Â Â Âif (!coord->rdy)
> - Â Â Â Â Â Â Â w8001->type = 0;
> + Â Â Â Â Â Â Â w8001->type = KEY_RESERVED;
> +}
> +
> +static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
> +{
> + Â Â Â struct input_dev *dev = w8001->dev;
> + Â Â Â unsigned int x = coord->x;
> + Â Â Â unsigned int y = coord->y;
> +
> + Â Â Â /* scale to pen maximum */
> + Â Â Â if (w8001->max_pen_x && w8001->max_touch_x)
> + Â Â Â Â Â Â Â x = x * w8001->max_pen_x / w8001->max_touch_x;
> +
> + Â Â Â if (w8001->max_pen_y && w8001->max_touch_y)
> + Â Â Â Â Â Â Â y = y * w8001->max_pen_y / w8001->max_touch_y;
> +
> + Â Â Â input_report_abs(dev, ABS_X, x);
> + Â Â Â input_report_abs(dev, ABS_Y, y);
> + Â Â Â input_report_key(dev, BTN_TOUCH, coord->tsw);
> + Â Â Â input_report_key(dev, BTN_TOOL_FINGER, coord->tsw);
> +
> + Â Â Â input_sync(dev);
> +
> + Â Â Â w8001->type = coord->tsw ? BTN_TOOL_FINGER : KEY_RESERVED;
> Â}
>
> Âstatic irqreturn_t w8001_interrupt(struct serio *serio,
> @@ -213,9 +269,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>
> Â Â Â Âcase W8001_PKTLEN_TOUCH93 - 1:
> Â Â Â Âcase W8001_PKTLEN_TOUCH9A - 1:
> - Â Â Â Â Â Â Â /* ignore one-finger touch packet. */
> - Â Â Â Â Â Â Â if (w8001->pktlen == w8001->idx)
> + Â Â Â Â Â Â Â tmp = w8001->data[0] & W8001_TOUCH_BYTE;
> + Â Â Â Â Â Â Â if (tmp != W8001_TOUCH_BYTE)
> + Â Â Â Â Â Â Â Â Â Â Â break;
> +
> + Â Â Â Â Â Â Â if (w8001->pktlen == w8001->idx) {
> Â Â Â Â Â Â Â Â Â Â Â Âw8001->idx = 0;
> + Â Â Â Â Â Â Â Â Â Â Â if (w8001->type != BTN_TOOL_PEN &&
> + Â Â Â Â Â Â Â Â Â Â Â Â Â w8001->type != BTN_TOOL_RUBBER) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â parse_single_touch(w8001->data, &coord);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â report_single_touch(w8001, &coord);
> + Â Â Â Â Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Âbreak;
>
> Â Â Â Â/* Pen coordinates packet */
> @@ -224,18 +289,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
> Â Â Â Â Â Â Â Âif (unlikely(tmp == W8001_TAB_BYTE))
> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
>
> - Â Â Â Â Â Â Â tmp = (w8001->data[0] & W8001_TOUCH_BYTE);
> + Â Â Â Â Â Â Â tmp = w8001->data[0] & W8001_TOUCH_BYTE;
> Â Â Â Â Â Â Â Âif (tmp == W8001_TOUCH_BYTE)
> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
>
> Â Â Â Â Â Â Â Âw8001->idx = 0;
> - Â Â Â Â Â Â Â parse_data(w8001->data, &coord);
> + Â Â Â Â Â Â Â parse_pen_data(w8001->data, &coord);
> Â Â Â Â Â Â Â Âreport_pen_events(w8001, &coord);
> Â Â Â Â Â Â Â Âbreak;
>
> Â Â Â Â/* control packet */
> Â Â Â Âcase W8001_PKTLEN_TPCCTL - 1:
> - Â Â Â Â Â Â Â tmp = (w8001->data[0] & W8001_TOUCH_MASK);
> + Â Â Â Â Â Â Â tmp = w8001->data[0] & W8001_TOUCH_MASK;
> Â Â Â Â Â Â Â Âif (tmp == W8001_TOUCH_BYTE)
> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
>
> @@ -248,7 +313,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
> Â Â Â Â/* 2 finger touch packet */
> Â Â Â Âcase W8001_PKTLEN_TOUCH2FG - 1:
> Â Â Â Â Â Â Â Âw8001->idx = 0;
> - Â Â Â Â Â Â Â parse_touch(w8001);
> + Â Â Â Â Â Â Â parse_multi_touch(w8001);
> Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â}
>
> @@ -278,6 +343,7 @@ static int w8001_setup(struct w8001 *w8001)
> Â{
> Â Â Â Âstruct input_dev *dev = w8001->dev;
> Â Â Â Âstruct w8001_coord coord;
> + Â Â Â struct w8001_touch_query touch;
> Â Â Â Âint error;
>
> Â Â Â Âerror = w8001_command(w8001, W8001_CMD_STOP, false);
> @@ -289,11 +355,15 @@ static int w8001_setup(struct w8001 *w8001)
> Â Â Â Â/* penabled? */
> Â Â Â Âerror = w8001_command(w8001, W8001_CMD_QUERY, true);
> Â Â Â Âif (!error) {
> + Â Â Â Â Â Â Â __set_bit(BTN_TOUCH, dev->keybit);
> Â Â Â Â Â Â Â Â__set_bit(BTN_TOOL_PEN, dev->keybit);
> Â Â Â Â Â Â Â Â__set_bit(BTN_TOOL_RUBBER, dev->keybit);
> Â Â Â Â Â Â Â Â__set_bit(BTN_STYLUS, dev->keybit);
> Â Â Â Â Â Â Â Â__set_bit(BTN_STYLUS2, dev->keybit);
> - Â Â Â Â Â Â Â parse_data(w8001->response, &coord);
> +
> + Â Â Â Â Â Â Â parse_pen_data(w8001->response, &coord);
> + Â Â Â Â Â Â Â w8001->max_pen_x = coord.x;
> + Â Â Â Â Â Â Â w8001->max_pen_y = coord.y;
>
> Â Â Â Â Â Â Â Âinput_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
> Â Â Â Â Â Â Â Âinput_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
> @@ -312,24 +382,34 @@ static int w8001_setup(struct w8001 *w8001)
> Â Â Â Â * second byte is empty, which indicates touch is not supported.
> Â Â Â Â */
> Â Â Â Âif (!error && w8001->response[1]) {
> - Â Â Â Â Â Â Â struct w8001_touch_query touch;
> + Â Â Â Â Â Â Â __set_bit(BTN_TOUCH, dev->keybit);
> + Â Â Â Â Â Â Â __set_bit(BTN_TOOL_FINGER, dev->keybit);
I do not think we want to set BTN_TOUCH and BTN_TOOL_FINGER for MT
device since we do not need to support single touch for this MT
device. Please refer to my comments for your other patch in the next
email.
> Â Â Â Â Â Â Â Âparse_touchquery(w8001->response, &touch);
> + Â Â Â Â Â Â Â w8001->max_touch_x = touch.x;
> + Â Â Â Â Â Â Â w8001->max_touch_y = touch.y;
> +
> + Â Â Â Â Â Â Â /* scale to pen maximum */
> + Â Â Â Â Â Â Â if (w8001->max_pen_x && w8001->max_pen_y) {
> + Â Â Â Â Â Â Â Â Â Â Â touch.x = w8001->max_pen_x;
> + Â Â Â Â Â Â Â Â Â Â Â touch.y = w8001->max_pen_y;
> + Â Â Â Â Â Â Â }
If we scale touch for both single and MT, we need to scale the raw
(x,y) for both too. I purposely did not want to scale MT touch since
they can be supported in different resolution from pen by MT protocol
now.
>
> Â Â Â Â Â Â Â Âinput_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
> Â Â Â Â Â Â Â Âinput_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
> - Â Â Â Â Â Â Â __set_bit(BTN_TOOL_FINGER, dev->keybit);
>
> Â Â Â Â Â Â Â Âswitch (touch.sensor_id) {
> Â Â Â Â Â Â Â Âcase 0:
> Â Â Â Â Â Â Â Âcase 2:
> Â Â Â Â Â Â Â Â Â Â Â Âw8001->pktlen = W8001_PKTLEN_TOUCH93;
> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> +
> Â Â Â Â Â Â Â Âcase 1:
> Â Â Â Â Â Â Â Âcase 3:
> Â Â Â Â Â Â Â Âcase 4:
> Â Â Â Â Â Â Â Â Â Â Â Âw8001->pktlen = W8001_PKTLEN_TOUCH9A;
> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> +
> Â Â Â Â Â Â Â Âcase 5:
> Â Â Â Â Â Â Â Â Â Â Â Âw8001->pktlen = W8001_PKTLEN_TOUCH2FG;
>
> @@ -397,7 +477,6 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
> Â Â Â Âinput_dev->dev.parent = &serio->dev;
>
> Â Â Â Âinput_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> - Â Â Â __set_bit(BTN_TOUCH, input_dev->keybit);
>
> Â Â Â Âserio_set_drvdata(serio, w8001);
> Â Â Â Âerr = serio_open(serio, drv);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
