From: 廖崇榮 <kt.liao@emc.com.tw> To: "'Benjamin Tissoires'" <benjamin.tissoires@redhat.com>, "'Dmitry Torokhov'" <dmitry.torokhov@gmail.com>, "'Rob Herring'" <robh+dt@kernel.org>, "'Aaron Ma'" <aaron.ma@canonical.com>, "'Hans de Goede'" <hdegoede@redhat.com> Cc: "'open list:HID CORE LAYER'" <linux-input@vger.kernel.org>, "'lkml'" <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org> Subject: RE: [PATCH v2 08/10] Input: elan_i2c - export true width/height Date: Mon, 27 May 2019 11:55:01 +0800 [thread overview] Message-ID: <00f901d5143f$f5ea8420$e1bf8c60$@emc.com.tw> (raw) In-Reply-To: <CAO-hwJJXGTZq7zRVhcFNwh-kOo0rUhZOsNtFX1yA93Km=L+ynA@mail.gmail.com> Hi -----Original Message----- From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com] Sent: Friday, May 24, 2019 5:37 PM To: Dmitry Torokhov; KT Liao; Rob Herring; Aaron Ma; Hans de Goede Cc: open list:HID CORE LAYER; lkml; devicetree@vger.kernel.org Subject: Re: [PATCH v2 08/10] Input: elan_i2c - export true width/height On Tue, May 21, 2019 at 3:28 PM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > The width/height is actually in the same unit than X and Y. So we > should not tamper the data, but just set the proper resolution, so > that userspace can correctly detect which touch is a palm or a finger. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > -- > > new in v2 > --- > drivers/input/mouse/elan_i2c_core.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/input/mouse/elan_i2c_core.c > b/drivers/input/mouse/elan_i2c_core.c > index 7ff044c6cd11..6f4feedb7765 100644 > --- a/drivers/input/mouse/elan_i2c_core.c > +++ b/drivers/input/mouse/elan_i2c_core.c > @@ -45,7 +45,6 @@ > #define DRIVER_NAME "elan_i2c" > #define ELAN_VENDOR_ID 0x04f3 > #define ETP_MAX_PRESSURE 255 > -#define ETP_FWIDTH_REDUCE 90 > #define ETP_FINGER_WIDTH 15 > #define ETP_RETRY_COUNT 3 > > @@ -915,12 +914,8 @@ static void elan_report_contact(struct elan_tp_data *data, > return; > } > > - /* > - * To avoid treating large finger as palm, let's reduce the > - * width x and y per trace. > - */ > - area_x = mk_x * (data->width_x - ETP_FWIDTH_REDUCE); > - area_y = mk_y * (data->width_y - ETP_FWIDTH_REDUCE); > + area_x = mk_x * data->width_x; > + area_y = mk_y * data->width_y; > > major = max(area_x, area_y); > minor = min(area_x, area_y); @@ -1123,8 +1118,10 @@ > static int elan_setup_input_device(struct elan_tp_data *data) > ETP_MAX_PRESSURE, 0, 0); > input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, > ETP_FINGER_WIDTH * max_width, 0, 0); > + input_abs_set_res(input, ABS_MT_TOUCH_MAJOR, data->x_res); > input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, > ETP_FINGER_WIDTH * min_width, 0, 0); > + input_abs_set_res(input, ABS_MT_TOUCH_MINOR, data->y_res); I had a chat with Peter on Wednesday, and he mentioned that this is dangerous as Major/Minor are max/min of the width and height. And given that we might have 2 different resolutions, we would need to do some computation in the kernel to ensure the data is correct with respect to the resolution. TL;DR: I don't think we should export the resolution there :( KT, should I drop the patch entirely, or is there a strong argument for keeping the ETP_FWIDTH_REDUCE around? I suggest you apply the patch, I have no idea why ETP_FWIDTH_REDUCE existed. Our FW team know nothing about ETP_FWIDTH_REDUCE ether. The only side effect will happen on Chromebook because such computation have stayed in ChromeOS' kernel for four years. Chrome's finger/palm threshold may be different from other Linux distribution. We will discuss it with Google once the patch picked by chrome and cause something wrong. Cheers, Benjamin > > data->input = input; > > -- > 2.21.0 >
WARNING: multiple messages have this Message-ID (diff)
From: 廖崇榮 <kt.liao@emc.com.tw> To: 'Benjamin Tissoires' <benjamin.tissoires@redhat.com>, 'Dmitry Torokhov' <dmitry.torokhov@gmail.com>, 'Rob Herring' <robh+dt@kernel.org>, 'Aaron Ma' <aaron.ma@canonical.com>, 'Hans de Goede' <hdegoede@redhat.com> Cc: "'open list:HID CORE LAYER'" <linux-input@vger.kernel.org>, 'lkml' <linux-kernel@vger.kernel.org>, devicetree@vger.kernel.org Subject: RE: [PATCH v2 08/10] Input: elan_i2c - export true width/height Date: Mon, 27 May 2019 11:55:01 +0800 [thread overview] Message-ID: <00f901d5143f$f5ea8420$e1bf8c60$@emc.com.tw> (raw) In-Reply-To: <CAO-hwJJXGTZq7zRVhcFNwh-kOo0rUhZOsNtFX1yA93Km=L+ynA@mail.gmail.com> Hi -----Original Message----- From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com] Sent: Friday, May 24, 2019 5:37 PM To: Dmitry Torokhov; KT Liao; Rob Herring; Aaron Ma; Hans de Goede Cc: open list:HID CORE LAYER; lkml; devicetree@vger.kernel.org Subject: Re: [PATCH v2 08/10] Input: elan_i2c - export true width/height On Tue, May 21, 2019 at 3:28 PM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > The width/height is actually in the same unit than X and Y. So we > should not tamper the data, but just set the proper resolution, so > that userspace can correctly detect which touch is a palm or a finger. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > -- > > new in v2 > --- > drivers/input/mouse/elan_i2c_core.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/input/mouse/elan_i2c_core.c > b/drivers/input/mouse/elan_i2c_core.c > index 7ff044c6cd11..6f4feedb7765 100644 > --- a/drivers/input/mouse/elan_i2c_core.c > +++ b/drivers/input/mouse/elan_i2c_core.c > @@ -45,7 +45,6 @@ > #define DRIVER_NAME "elan_i2c" > #define ELAN_VENDOR_ID 0x04f3 > #define ETP_MAX_PRESSURE 255 > -#define ETP_FWIDTH_REDUCE 90 > #define ETP_FINGER_WIDTH 15 > #define ETP_RETRY_COUNT 3 > > @@ -915,12 +914,8 @@ static void elan_report_contact(struct elan_tp_data *data, > return; > } > > - /* > - * To avoid treating large finger as palm, let's reduce the > - * width x and y per trace. > - */ > - area_x = mk_x * (data->width_x - ETP_FWIDTH_REDUCE); > - area_y = mk_y * (data->width_y - ETP_FWIDTH_REDUCE); > + area_x = mk_x * data->width_x; > + area_y = mk_y * data->width_y; > > major = max(area_x, area_y); > minor = min(area_x, area_y); @@ -1123,8 +1118,10 @@ > static int elan_setup_input_device(struct elan_tp_data *data) > ETP_MAX_PRESSURE, 0, 0); > input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, > ETP_FINGER_WIDTH * max_width, 0, 0); > + input_abs_set_res(input, ABS_MT_TOUCH_MAJOR, data->x_res); > input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, > ETP_FINGER_WIDTH * min_width, 0, 0); > + input_abs_set_res(input, ABS_MT_TOUCH_MINOR, data->y_res); I had a chat with Peter on Wednesday, and he mentioned that this is dangerous as Major/Minor are max/min of the width and height. And given that we might have 2 different resolutions, we would need to do some computation in the kernel to ensure the data is correct with respect to the resolution. TL;DR: I don't think we should export the resolution there :( KT, should I drop the patch entirely, or is there a strong argument for keeping the ETP_FWIDTH_REDUCE around? I suggest you apply the patch, I have no idea why ETP_FWIDTH_REDUCE existed. Our FW team know nothing about ETP_FWIDTH_REDUCE ether. The only side effect will happen on Chromebook because such computation have stayed in ChromeOS' kernel for four years. Chrome's finger/palm threshold may be different from other Linux distribution. We will discuss it with Google once the patch picked by chrome and cause something wrong. Cheers, Benjamin > > data->input = input; > > -- > 2.21.0 >
next prev parent reply other threads:[~2019-05-27 3:55 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-21 13:27 [PATCH v2 00/10] Fix Elan I2C touchpads in latest generation from Lenovo Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 01/10] Input: elantech - query the min/max information beforehand too Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 02/10] Input: elantech - add helper function elantech_is_buttonpad() Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 03/10] Input: elantech - detect middle button based on firmware version Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 04/10] dt-bindings: add more optional properties for elan_i2c touchpads Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 05/10] Input: elan_i2c - do not query the info if they are provided Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 06/10] Input: elantech/SMBus - export all capabilities from the PS/2 node Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 07/10] Input: elan_i2c - handle physical middle button Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 08/10] Input: elan_i2c - export true width/height Benjamin Tissoires 2019-05-24 9:37 ` Benjamin Tissoires 2019-05-27 3:55 ` 廖崇榮 [this message] 2019-05-27 3:55 ` 廖崇榮 2019-05-28 1:21 ` 'Dmitry Torokhov' 2019-05-28 18:13 ` Harry Cutts 2019-05-29 0:12 ` Sean O'Brien 2019-05-29 7:16 ` Benjamin Tissoires 2019-05-29 12:55 ` 廖崇榮 2019-05-29 12:55 ` 廖崇榮 2019-05-29 13:16 ` Benjamin Tissoires 2019-05-30 0:22 ` Peter Hutterer 2019-05-21 13:27 ` [PATCH v2 09/10] Input: elan_i2c - correct the width/size base value Benjamin Tissoires 2019-05-24 3:13 ` 廖崇榮 2019-05-24 3:13 ` 廖崇榮 2019-05-24 7:05 ` Benjamin Tissoires 2019-05-24 9:00 ` 廖崇榮 2019-05-24 9:00 ` 廖崇榮 2019-05-24 9:19 ` Benjamin Tissoires 2019-05-21 13:27 ` [PATCH v2 10/10] Input: elantech: remove P52 from SMBus blacklist Benjamin Tissoires 2019-05-23 13:25 ` [PATCH v2 00/10] Fix Elan I2C touchpads in latest generation from Lenovo Benjamin Tissoires
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='00f901d5143f$f5ea8420$e1bf8c60$@emc.com.tw' \ --to=kt.liao@emc.com.tw \ --cc=aaron.ma@canonical.com \ --cc=benjamin.tissoires@redhat.com \ --cc=devicetree@vger.kernel.org \ --cc=dmitry.torokhov@gmail.com \ --cc=hdegoede@redhat.com \ --cc=linux-input@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=robh+dt@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.