diff mbox

Input: synaptics - disable fuzz when using mt slots

Message ID 1359707312-12986-1-git-send-email-djkurtz@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Kurtz Feb. 1, 2013, 8:28 a.m. UTC
The input fuzz algorithm does a pretty good job filtering out small
jitter on input samples.

However, there is a subtle problem when fuzz is used with mt drivers that
also use input_mt_report_pointer_emulation() to report legacy single
touch coordinates.  The reported single touch coordinates are taken from
the active MT slot with smallest tracking id.  However, since the MT data
is sent first, it has already had FUZZ applied.  Thus, the ST coordinates
actually get FUZZ applied a second time, with the just-fuzzed MT value as
the 'old value'.

It appears that it is non-trivial to fix this in the mt layer/input core,
so, for now, just disable fuzz for the synaptics IMAGE_SENSOR path that
uses input_mt_report_pointer_emulation().

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Dmitry Torokhov Feb. 13, 2013, 9:52 p.m. UTC | #1
On Fri, Feb 01, 2013 at 04:28:32PM +0800, Daniel Kurtz wrote:
> The input fuzz algorithm does a pretty good job filtering out small
> jitter on input samples.
> 
> However, there is a subtle problem when fuzz is used with mt drivers that
> also use input_mt_report_pointer_emulation() to report legacy single
> touch coordinates.  The reported single touch coordinates are taken from
> the active MT slot with smallest tracking id.  However, since the MT data
> is sent first, it has already had FUZZ applied.  Thus, the ST coordinates
> actually get FUZZ applied a second time, with the just-fuzzed MT value as
> the 'old value'.
> 
> It appears that it is non-trivial to fix this in the mt layer/input core,
> so, for now, just disable fuzz for the synaptics IMAGE_SENSOR path that
> uses input_mt_report_pointer_emulation().
> 
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  drivers/input/mouse/synaptics.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 12d12ca..b6c6838 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -1211,7 +1211,8 @@ static void set_abs_position_params(struct input_dev *dev,
>  	int x_max = priv->x_max ?: XMAX_NOMINAL;
>  	int y_min = priv->y_min ?: YMIN_NOMINAL;
>  	int y_max = priv->y_max ?: YMAX_NOMINAL;
> -	int fuzz = SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) ?
> +	int fuzz = (SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) &&
> +			!SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) ?
>  			SYN_REDUCED_FILTER_FUZZ : 0;

Hmm, instead of disabling fuzz completely maybe we should disable it
only on the derived axes (ABS_X/ABS_Y) for multitouch devices?

Thanks.
Daniel Kurtz Feb. 14, 2013, 5:15 p.m. UTC | #2
On Wed, Feb 13, 2013 at 1:52 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Feb 01, 2013 at 04:28:32PM +0800, Daniel Kurtz wrote:
>> The input fuzz algorithm does a pretty good job filtering out small
>> jitter on input samples.
>>
>> However, there is a subtle problem when fuzz is used with mt drivers that
>> also use input_mt_report_pointer_emulation() to report legacy single
>> touch coordinates.  The reported single touch coordinates are taken from
>> the active MT slot with smallest tracking id.  However, since the MT data
>> is sent first, it has already had FUZZ applied.  Thus, the ST coordinates
>> actually get FUZZ applied a second time, with the just-fuzzed MT value as
>> the 'old value'.
>>
>> It appears that it is non-trivial to fix this in the mt layer/input core,
>> so, for now, just disable fuzz for the synaptics IMAGE_SENSOR path that
>> uses input_mt_report_pointer_emulation().
>>
>> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>> ---
>>  drivers/input/mouse/synaptics.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> index 12d12ca..b6c6838 100644
>> --- a/drivers/input/mouse/synaptics.c
>> +++ b/drivers/input/mouse/synaptics.c
>> @@ -1211,7 +1211,8 @@ static void set_abs_position_params(struct input_dev *dev,
>>       int x_max = priv->x_max ?: XMAX_NOMINAL;
>>       int y_min = priv->y_min ?: YMIN_NOMINAL;
>>       int y_max = priv->y_max ?: YMAX_NOMINAL;
>> -     int fuzz = SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) ?
>> +     int fuzz = (SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) &&
>> +                     !SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) ?
>>                       SYN_REDUCED_FILTER_FUZZ : 0;
>
> Hmm, instead of disabling fuzz completely maybe we should disable it
> only on the derived axes (ABS_X/ABS_Y) for multitouch devices?

Yes, that should work...  both would end up fuzzed to the same value,
for a not-so-obvious reason, IMHO :).
It's a bit tricky, and I like it, but is that what you intended by the
suggestion?

-Daniel

>
> Thanks.
>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Henrik Rydberg Feb. 14, 2013, 5:22 p.m. UTC | #3
> > Hmm, instead of disabling fuzz completely maybe we should disable it
> > only on the derived axes (ABS_X/ABS_Y) for multitouch devices?
> 
> Yes, that should work...  both would end up fuzzed to the same value,
> for a not-so-obvious reason, IMHO :).
> It's a bit tricky, and I like it, but is that what you intended by the
> suggestion?

Since this is inherently a problem when using the mt helper functions,
turning fuzz off there ought to solve this problem transparently, and
for all users.

Thanks, Daniel, for reminding me of this year-old glitch. :-)

Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 12d12ca..b6c6838 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1211,7 +1211,8 @@  static void set_abs_position_params(struct input_dev *dev,
 	int x_max = priv->x_max ?: XMAX_NOMINAL;
 	int y_min = priv->y_min ?: YMIN_NOMINAL;
 	int y_max = priv->y_max ?: YMAX_NOMINAL;
-	int fuzz = SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) ?
+	int fuzz = (SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) &&
+			!SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) ?
 			SYN_REDUCED_FILTER_FUZZ : 0;
 
 	input_set_abs_params(dev, x_code, x_min, x_max, fuzz, 0);