| Message ID | 20220215001253.1109876-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Input: zinitix - Do not report shadow fingers | expand |
Hi Linus, On Tue, Feb 15, 2022 at 01:12:53AM +0100, Linus Walleij wrote: > I observed the following problem with the BT404 touch pad > running the Phosh UI: > > When e.g. typing on the virtual keyboard pressing "g" would > produce "ggg". > > After some analysis it turns out the firmware reports that three > fingers hit that coordinate at the same time, finger 0, 2 and > 4 (of the five available 0,1,2,3,4). > > DOWN > Zinitix-TS 3-0020: finger 0 down (246, 395) > Zinitix-TS 3-0020: finger 1 up (0, 0) > Zinitix-TS 3-0020: finger 2 down (246, 395) > Zinitix-TS 3-0020: finger 3 up (0, 0) > Zinitix-TS 3-0020: finger 4 down (246, 395) > UP > Zinitix-TS 3-0020: finger 0 up (246, 395) > Zinitix-TS 3-0020: finger 2 up (246, 395) > Zinitix-TS 3-0020: finger 4 up (246, 395) > > This is one touch and release: i.e. this is all reported on > touch (down) and release. > > After augmenting the driver to remember all fingers we report in > a single touch event and filter out any duplicates we get this > debug print: > > DOWN > Zinitix-TS 3-0020: finger 0 down (257, 664) > Zinitix-TS 3-0020: finger 1 up (0, 0) > Zinitix-TS 3-0020: ignore shadow finger 2 at (257, 664) > Zinitix-TS 3-0020: ignore shadow finger 3 at (0, 0) > Zinitix-TS 3-0020: ignore shadow finger 4 at (257, 664) > UP > Zinitix-TS 3-0020: finger 0 up (257, 664) > Zinitix-TS 3-0020: ignore shadow finger 2 at (257, 664) > Zinitix-TS 3-0020: ignore shadow finger 4 at (257, 664) > > As it is physically impossible to place two fingers at the same > point at the screen this seems safe to do. > > The "finger 1 up (0, 0)" type messages of releaseing ghost > fingers does not go away, and even though this is mostly > incorrect too, we cannot rule out some finger being released > at (0, 0) in the generic case, so it needs to stay. > > Notice that the ghostly release of fingers 1 and 3 only > happens on finger down events, not on finger up. > > This appears to me as the best we can do. After this my > screen is nicely interactive. I see that there is "finger_cnt" that we completely ignore in the driver. I wonder if we actually pay attention to it we would not need to do all this? Thanks. > > Cc: Michael Srba <Michael.Srba@seznam.cz> > Cc: Nikita Travkin <nikita@trvn.ru> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/input/touchscreen/zinitix.c | 78 ++++++++++++++++++++++++----- > 1 file changed, 66 insertions(+), 12 deletions(-) > > diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c > index 129ebc810de8..7655a09b65bc 100644 > --- a/drivers/input/touchscreen/zinitix.c > +++ b/drivers/input/touchscreen/zinitix.c > @@ -319,14 +319,72 @@ static int zinitix_send_power_on_sequence(struct bt541_ts_data *bt541) > return 0; > } > > -static void zinitix_report_finger(struct bt541_ts_data *bt541, int slot, > - const struct point_coord *p) > +static void zinitix_report_fingers(struct bt541_ts_data *bt541, struct touch_event *te) > { > - input_mt_slot(bt541->input_dev, slot); > - input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, true); > - touchscreen_report_pos(bt541->input_dev, &bt541->prop, > - le16_to_cpu(p->x), le16_to_cpu(p->y), true); > - input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width); > + struct point_coord *p; > + u16 reported_x[MAX_SUPPORTED_FINGER_NUM]; > + u16 reported_y[MAX_SUPPORTED_FINGER_NUM]; > + u16 x, y; > + int i, j; > + int ridx = 0; > + bool ignore; > + > + for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) { > + p = &te->point_coord[i]; > + > + /* Skip nonexisting fingers */ > + if (!p->sub_status & SUB_BIT_EXIST) > + continue; > + > + x = le16_to_cpu(p->x); > + y = le16_to_cpu(p->y); > + > + /* > + * Check if this has already been reported and is just a shadow > + * finger > + */ > + ignore = false; > + for (j = 0; j < ridx; j++) { > + if (x == reported_x[j] && y == reported_y[j]) { > + ignore = true; > + break; > + } > + } > + > + if (ignore) { > + dev_dbg(&bt541->client->dev, > + "ignore shadow finger %d at (%u, %u)\n", i, x, y); > + continue; > + } > + > + input_mt_slot(bt541->input_dev, i); > + > + if (p->sub_status & BIT_DOWN) { > + /* Finger down */ > + input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, true); > + touchscreen_report_pos(bt541->input_dev, &bt541->prop, x, y, true); > + input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width); > + dev_dbg(&bt541->client->dev, "finger %d down (%u, %u)\n", i, x, y); > + } else if (p->sub_status & BIT_UP) { > + /* Release finger */ > + input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, false); > + touchscreen_report_pos(bt541->input_dev, &bt541->prop, x, y, true); > + input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, 0); > + dev_dbg(&bt541->client->dev, "finger %d up (%u, %u)\n", i, x, y); > + } else if (p->sub_status & BIT_MOVE) { > + /* Finger moves while pressed down */ > + input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, true); > + touchscreen_report_pos(bt541->input_dev, &bt541->prop, x, y, true); > + input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width); > + dev_dbg(&bt541->client->dev, "finger %d move (%u, %u)\n", i, x, y); > + } else { > + dev_dbg(&bt541->client->dev, "unknown finger event\n"); > + } > + > + reported_x[ridx] = x; > + reported_y[ridx] = y; > + ridx++; > + } > } > > static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) > @@ -335,7 +393,6 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) > struct i2c_client *client = bt541->client; > struct touch_event touch_event; > int error; > - int i; > > memset(&touch_event, 0, sizeof(struct touch_event)); > > @@ -346,10 +403,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) > goto out; > } > > - for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) > - if (touch_event.point_coord[i].sub_status & SUB_BIT_EXIST) > - zinitix_report_finger(bt541, i, > - &touch_event.point_coord[i]); > + zinitix_report_fingers(bt541, &touch_event); > > input_mt_sync_frame(bt541->input_dev); > input_sync(bt541->input_dev); > -- > 2.34.1 >
diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c index 129ebc810de8..7655a09b65bc 100644 --- a/drivers/input/touchscreen/zinitix.c +++ b/drivers/input/touchscreen/zinitix.c @@ -319,14 +319,72 @@ static int zinitix_send_power_on_sequence(struct bt541_ts_data *bt541) return 0; } -static void zinitix_report_finger(struct bt541_ts_data *bt541, int slot, - const struct point_coord *p) +static void zinitix_report_fingers(struct bt541_ts_data *bt541, struct touch_event *te) { - input_mt_slot(bt541->input_dev, slot); - input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, true); - touchscreen_report_pos(bt541->input_dev, &bt541->prop, - le16_to_cpu(p->x), le16_to_cpu(p->y), true); - input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width); + struct point_coord *p; + u16 reported_x[MAX_SUPPORTED_FINGER_NUM]; + u16 reported_y[MAX_SUPPORTED_FINGER_NUM]; + u16 x, y; + int i, j; + int ridx = 0; + bool ignore; + + for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) { + p = &te->point_coord[i]; + + /* Skip nonexisting fingers */ + if (!p->sub_status & SUB_BIT_EXIST) + continue; + + x = le16_to_cpu(p->x); + y = le16_to_cpu(p->y); + + /* + * Check if this has already been reported and is just a shadow + * finger + */ + ignore = false; + for (j = 0; j < ridx; j++) { + if (x == reported_x[j] && y == reported_y[j]) { + ignore = true; + break; + } + } + + if (ignore) { + dev_dbg(&bt541->client->dev, + "ignore shadow finger %d at (%u, %u)\n", i, x, y); + continue; + } + + input_mt_slot(bt541->input_dev, i); + + if (p->sub_status & BIT_DOWN) { + /* Finger down */ + input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, true); + touchscreen_report_pos(bt541->input_dev, &bt541->prop, x, y, true); + input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width); + dev_dbg(&bt541->client->dev, "finger %d down (%u, %u)\n", i, x, y); + } else if (p->sub_status & BIT_UP) { + /* Release finger */ + input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, false); + touchscreen_report_pos(bt541->input_dev, &bt541->prop, x, y, true); + input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, 0); + dev_dbg(&bt541->client->dev, "finger %d up (%u, %u)\n", i, x, y); + } else if (p->sub_status & BIT_MOVE) { + /* Finger moves while pressed down */ + input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, true); + touchscreen_report_pos(bt541->input_dev, &bt541->prop, x, y, true); + input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width); + dev_dbg(&bt541->client->dev, "finger %d move (%u, %u)\n", i, x, y); + } else { + dev_dbg(&bt541->client->dev, "unknown finger event\n"); + } + + reported_x[ridx] = x; + reported_y[ridx] = y; + ridx++; + } } static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) @@ -335,7 +393,6 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) struct i2c_client *client = bt541->client; struct touch_event touch_event; int error; - int i; memset(&touch_event, 0, sizeof(struct touch_event)); @@ -346,10 +403,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler) goto out; } - for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) - if (touch_event.point_coord[i].sub_status & SUB_BIT_EXIST) - zinitix_report_finger(bt541, i, - &touch_event.point_coord[i]); + zinitix_report_fingers(bt541, &touch_event); input_mt_sync_frame(bt541->input_dev); input_sync(bt541->input_dev);
I observed the following problem with the BT404 touch pad running the Phosh UI: When e.g. typing on the virtual keyboard pressing "g" would produce "ggg". After some analysis it turns out the firmware reports that three fingers hit that coordinate at the same time, finger 0, 2 and 4 (of the five available 0,1,2,3,4). DOWN Zinitix-TS 3-0020: finger 0 down (246, 395) Zinitix-TS 3-0020: finger 1 up (0, 0) Zinitix-TS 3-0020: finger 2 down (246, 395) Zinitix-TS 3-0020: finger 3 up (0, 0) Zinitix-TS 3-0020: finger 4 down (246, 395) UP Zinitix-TS 3-0020: finger 0 up (246, 395) Zinitix-TS 3-0020: finger 2 up (246, 395) Zinitix-TS 3-0020: finger 4 up (246, 395) This is one touch and release: i.e. this is all reported on touch (down) and release. After augmenting the driver to remember all fingers we report in a single touch event and filter out any duplicates we get this debug print: DOWN Zinitix-TS 3-0020: finger 0 down (257, 664) Zinitix-TS 3-0020: finger 1 up (0, 0) Zinitix-TS 3-0020: ignore shadow finger 2 at (257, 664) Zinitix-TS 3-0020: ignore shadow finger 3 at (0, 0) Zinitix-TS 3-0020: ignore shadow finger 4 at (257, 664) UP Zinitix-TS 3-0020: finger 0 up (257, 664) Zinitix-TS 3-0020: ignore shadow finger 2 at (257, 664) Zinitix-TS 3-0020: ignore shadow finger 4 at (257, 664) As it is physically impossible to place two fingers at the same point at the screen this seems safe to do. The "finger 1 up (0, 0)" type messages of releaseing ghost fingers does not go away, and even though this is mostly incorrect too, we cannot rule out some finger being released at (0, 0) in the generic case, so it needs to stay. Notice that the ghostly release of fingers 1 and 3 only happens on finger down events, not on finger up. This appears to me as the best we can do. After this my screen is nicely interactive. Cc: Michael Srba <Michael.Srba@seznam.cz> Cc: Nikita Travkin <nikita@trvn.ru> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/input/touchscreen/zinitix.c | 78 ++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 12 deletions(-)