On Wed, Dec 29, 2010 at 2:30 PM, Ping Cheng <pinglinux@xxxxxxxxx> wrote:
> 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.
After revisited the change you made, I think I can accept both
suggestions. The legacy client support may introduce issue. But it
won't hurt too much, I hope ;). The MT scaling offers a consistent
support between one and two finger touch, which is good.
So, the only change left is to scale the raw x,y of 2FGT to max_pen. I
can make a version 3 of this patch for you or you can change those two
lines if you don't mind. Just let me know.
Thank you.
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
