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@xxxxxxxxx>
> Cc: Nikita Travkin <nikita@xxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> 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
>
--
Dmitry
