* Re: [PATCH] Input: Optimize input_to_handler and input_pass_values function
From: Anshul Garg @ 2015-01-05 16:10 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: anshul.g@samsung.com, linux-input, p.shailesh
In-Reply-To: <20150104225733.GE31987@dtor-ws>
Dear Mr Dimtry ,
Thanks for the suggestions.
I will send you the updated patch soon.
On Mon, Jan 5, 2015 at 4:27 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Dec 30, 2014 at 11:19:53AM +0530, anshul.g@samsung.com wrote:
>> From: Anshul Garg <anshul.g@samsung.com>
>>
>> As input_pass_values function is called everytime when EV_SYN is sent from
>> input driver or input event buffer becomes full.
>>
>> 1. In case of regular handler, event filter code should not run so added check
>> whether handler supports filter or not.
>> 2. If input device doesn't support EV_KEY event type avoid running auto repeat code.
>
> Can you please split it in 2 patches?
>
>>
>> Signed-off-by: Anshul Garg <anshul.g@samsung.com>
>> ---
>> drivers/input/input.c | 37 +++++++++++++++++++++----------------
>> 1 file changed, 21 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/input/input.c b/drivers/input/input.c
>> index bbec2dc..9d6609b 100644
>> --- a/drivers/input/input.c
>> +++ b/drivers/input/input.c
>> @@ -100,23 +100,24 @@ static unsigned int input_to_handler(struct input_handle *handle,
>> struct input_value *end = vals;
>> struct input_value *v;
>>
>> - for (v = vals; v != vals + count; v++) {
>> - if (handler->filter &&
>> - handler->filter(handle, v->type, v->code, v->value))
>> - continue;
>> - if (end != v)
>> - *end = *v;
>> - end++;
>> + if (handler->filter) {
>> + for (v = vals; v != vals + count; v++) {
>> + if (handler->filter(handle, v->type, v->code, v->value))
>> + continue;
>> + if (end != v)
>> + *end = *v;
>> + end++;
>> + }
>> + count = end - vals;
>> }
>>
>> - count = end - vals;
>> if (!count)
>> return 0;
>>
>> if (handler->events)
>> handler->events(handle, vals, count);
>> else if (handler->event)
>> - for (v = vals; v != end; v++)
>> + for (v = vals; v != vals + count; v++)
>> handler->event(handle, v->type, v->code, v->value);
>>
>> return count;
>> @@ -143,8 +144,10 @@ static void input_pass_values(struct input_dev *dev,
>> count = input_to_handler(handle, vals, count);
>> } else {
>> list_for_each_entry_rcu(handle, &dev->h_list, d_node)
>> - if (handle->open)
>> + if (handle->open && count)
>> count = input_to_handler(handle, vals, count);
>> + else
>> + break;
>
> This is not correct. You'll stop delivering events at the very first
> closed handle, which is not what we want. We need:
>
> if (handle->open) {
> count = input_to_handler(handle, vals, count);
> if (count == 0)
> break;
> }
>
>> }
>>
>> rcu_read_unlock();
>> @@ -152,12 +155,14 @@ static void input_pass_values(struct input_dev *dev,
>> add_input_randomness(vals->type, vals->code, vals->value);
>>
>> /* trigger auto repeat for key events */
>> - for (v = vals; v != vals + count; v++) {
>> - if (v->type == EV_KEY && v->value != 2) {
>> - if (v->value)
>> - input_start_autorepeat(dev, v->code);
>> - else
>> - input_stop_autorepeat(dev);
>> + if (test_bit(EV_KEY, dev->evbit)) {
>
> You might want to test for presence of EV_REP as well.
>
>> + for (v = vals; v != vals + count; v++) {
>> + if (v->type == EV_KEY && v->value != 2) {
>> + if (v->value)
>> + input_start_autorepeat(dev, v->code);
>> + else
>> + input_stop_autorepeat(dev);
>> + }
>> }
>> }
>> }
>> --
>> 1.7.9.5
>>
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* [PATCH] Input: Optimize Auto Repeat Loop
From: Anshul Garg @ 2015-01-05 16:12 UTC (permalink / raw)
To: dmitry.torokhov, dtor, linux-input; +Cc: aksgarg1989, anshul.g, p.shailesh
From: Anshul Garg <aksgarg1989@gmail.com>
As input_pass_values function is called everytime when EV_SYN is sent
from input driver or input event buffer becomes full. So avoid running
auto repeat code if input device doesn't support EV_KEY and EV_REP
event type.
Signed-off-by: Anshul Garg <anshul.g@samsung.com>
---
drivers/input/input.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 213e3a1..601171b 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -152,12 +152,14 @@ static void input_pass_values(struct input_dev *dev,
add_input_randomness(vals->type, vals->code, vals->value);
/* trigger auto repeat for key events */
- for (v = vals; v != vals + count; v++) {
- if (v->type == EV_KEY && v->value != 2) {
- if (v->value)
- input_start_autorepeat(dev, v->code);
- else
- input_stop_autorepeat(dev);
+ if (test_bit(EV_KEY, dev->evbit) && test_bit(EV_REP, dev->evbit)) {
+ for (v = vals; v != vals + count; v++) {
+ if (v->type == EV_KEY && v->value != 2) {
+ if (v->value)
+ input_start_autorepeat(dev, v->code);
+ else
+ input_stop_autorepeat(dev);
+ }
}
}
}
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
^ permalink raw reply related
* [PATCH] Input: Optimize Event Filter functionality
From: Anshul Garg @ 2015-01-05 16:27 UTC (permalink / raw)
To: dmitry.torokhov, dtor, linux-input; +Cc: aksgarg1989, anshul.g, p.shailesh
From: Anshul Garg <aksgarg1989@gmail.com>
As input_pass_values function is called everytime when EV_SYN is sent
from input driver or input event buffer becomes full.
So in case of regular handler event filter code should not run so added
check whether handler supports filter or not.
Signed-off-by: Anshul Garg <anshul.g@samsung.com>
---
drivers/input/input.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 213e3a1..de55279 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -100,23 +100,24 @@ static unsigned int input_to_handler(struct input_handle *handle,
struct input_value *end = vals;
struct input_value *v;
- for (v = vals; v != vals + count; v++) {
- if (handler->filter &&
- handler->filter(handle, v->type, v->code, v->value))
- continue;
- if (end != v)
- *end = *v;
- end++;
+ if (handler->filter) {
+ for (v = vals; v != vals + count; v++) {
+ if (handler->filter(handle, v->type, v->code, v->value))
+ continue;
+ if (end != v)
+ *end = *v;
+ end++;
+ }
+ count = end - vals;
}
- count = end - vals;
if (!count)
return 0;
if (handler->events)
handler->events(handle, vals, count);
else if (handler->event)
- for (v = vals; v != end; v++)
+ for (v = vals; v != vals + count; v++)
handler->event(handle, v->type, v->code, v->value);
return count;
@@ -143,8 +144,11 @@ static void input_pass_values(struct input_dev *dev,
count = input_to_handler(handle, vals, count);
} else {
list_for_each_entry_rcu(handle, &dev->h_list, d_node)
- if (handle->open)
+ if (handle->open) {
count = input_to_handler(handle, vals, count);
+ if (!count)
+ break;
+ }
}
rcu_read_unlock();
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
^ permalink raw reply related
* Re: [PATCH 2/2] input: synaptics - fix width calculation on image sensors
From: Benjamin Tissoires @ 2015-01-05 18:25 UTC (permalink / raw)
To: Gabriele Mazzotta
Cc: Dmitry Torokhov, Henrik Rydberg, linux-input,
linux-kernel@vger.kernel.org, silverhammermba
In-Reply-To: <1419679889-6582-3-git-send-email-gabriele.mzt@gmail.com>
Hi Gabriele,
On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
<gabriele.mzt@gmail.com> wrote:
> When two or more fingers are on the touchpad, the 'w' slot holds the
> finger count rather than the width. Retrieve the correct value encoded
> in the lower bits of 'x', 'y' and 'z'.
>
> The minimum width reported is 8 rather than 4 in this case, while the
> maximum remains 15.
>
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> ---
> drivers/input/mouse/synaptics.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index ea0563e..5ff4c5b 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -593,7 +593,9 @@ static void synaptics_parse_agm(const unsigned char buf[],
> switch (agm_packet_type) {
> case 1:
> /* Gesture packet: (x, y, z) half resolution */
> - agm->w = hw->w;
> + agm->w = ((buf[1] & 0x01) |
> + ((buf[2] & 0x01) << 1) |
> + ((buf[5] & 0x01) << 2)) + 8;
> agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
For completeness, I think, we should also removes the w bits from x, y
and z (by masking buf[1], buf[2] and buf[5] with 0xfe).
Cheers,
Benjamin
> agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
> agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 2/2] input: synaptics - fix width calculation on image sensors
From: Gabriele Mazzotta @ 2015-01-05 18:37 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Henrik Rydberg, linux-input,
linux-kernel@vger.kernel.org, silverhammermba
In-Reply-To: <CAN+gG=FmtDcMQR0RzNwdy65riaL=hF5F18opwUTJV-oPESJ5JA@mail.gmail.com>
On Monday 05 January 2015 13:25:38 Benjamin Tissoires wrote:
> Hi Gabriele,
>
> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
> <gabriele.mzt@gmail.com> wrote:
> > When two or more fingers are on the touchpad, the 'w' slot holds the
> > finger count rather than the width. Retrieve the correct value encoded
> > in the lower bits of 'x', 'y' and 'z'.
> >
> > The minimum width reported is 8 rather than 4 in this case, while the
> > maximum remains 15.
> >
> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > ---
> > drivers/input/mouse/synaptics.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > index ea0563e..5ff4c5b 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -593,7 +593,9 @@ static void synaptics_parse_agm(const unsigned char buf[],
> > switch (agm_packet_type) {
> > case 1:
> > /* Gesture packet: (x, y, z) half resolution */
> > - agm->w = hw->w;
> > + agm->w = ((buf[1] & 0x01) |
> > + ((buf[2] & 0x01) << 1) |
> > + ((buf[5] & 0x01) << 2)) + 8;
> > agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
>
> For completeness, I think, we should also removes the w bits from x, y
> and z (by masking buf[1], buf[2] and buf[5] with 0xfe).
You are right, I forgot about it.
I was going to re-submit these patches anyway since there have been
quite a few changes and these can no longer be applied on next.
> Cheers,
> Benjamin
>
> > agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
> > agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
> > --
> > 2.1.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 2/2] input: synaptics - fix width calculation on image sensors
From: Benjamin Tissoires @ 2015-01-05 18:42 UTC (permalink / raw)
To: Gabriele Mazzotta
Cc: Dmitry Torokhov, Henrik Rydberg, linux-input,
linux-kernel@vger.kernel.org, silverhammermba
In-Reply-To: <4167786.IbP0CPQIld@xps13>
On Mon, Jan 5, 2015 at 1:37 PM, Gabriele Mazzotta
<gabriele.mzt@gmail.com> wrote:
> On Monday 05 January 2015 13:25:38 Benjamin Tissoires wrote:
>> Hi Gabriele,
>>
>> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
>> <gabriele.mzt@gmail.com> wrote:
>> > When two or more fingers are on the touchpad, the 'w' slot holds the
>> > finger count rather than the width. Retrieve the correct value encoded
>> > in the lower bits of 'x', 'y' and 'z'.
>> >
>> > The minimum width reported is 8 rather than 4 in this case, while the
>> > maximum remains 15.
>> >
>> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
>> > ---
>> > drivers/input/mouse/synaptics.c | 4 +++-
>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> > index ea0563e..5ff4c5b 100644
>> > --- a/drivers/input/mouse/synaptics.c
>> > +++ b/drivers/input/mouse/synaptics.c
>> > @@ -593,7 +593,9 @@ static void synaptics_parse_agm(const unsigned char buf[],
>> > switch (agm_packet_type) {
>> > case 1:
>> > /* Gesture packet: (x, y, z) half resolution */
>> > - agm->w = hw->w;
>> > + agm->w = ((buf[1] & 0x01) |
>> > + ((buf[2] & 0x01) << 1) |
>> > + ((buf[5] & 0x01) << 2)) + 8;
>> > agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
>>
>> For completeness, I think, we should also removes the w bits from x, y
>> and z (by masking buf[1], buf[2] and buf[5] with 0xfe).
>
> You are right, I forgot about it.
>
> I was going to re-submit these patches anyway since there have been
> quite a few changes and these can no longer be applied on next.
OK, I am trying to put together my thoughts for 1/2 right now. If you
could just wait a little before resubmitting, I would be grateful.
Thanks,
Benjamin
>
>> Cheers,
>> Benjamin
>>
>> > agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
>> > agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
>> > --
>> > 2.1.4
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> > Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply
* Re: [PATCH 2/2] input: synaptics - fix width calculation on image sensors
From: Dmitry Torokhov @ 2015-01-05 19:04 UTC (permalink / raw)
To: Benjamin Tissoires, Gabriele Mazzotta
Cc: Henrik Rydberg, linux-input, linux-kernel@vger.kernel.org,
silverhammermba
In-Reply-To: <CAN+gG=FLOD5C3N0q-i=4Sm6fH+oYTV27nEOKn8nxaw7GvFJbkQ@mail.gmail.com>
On January 5, 2015 10:42:13 AM PST, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote:
>On Mon, Jan 5, 2015 at 1:37 PM, Gabriele Mazzotta
><gabriele.mzt@gmail.com> wrote:
>> On Monday 05 January 2015 13:25:38 Benjamin Tissoires wrote:
>>> Hi Gabriele,
>>>
>>> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
>>> <gabriele.mzt@gmail.com> wrote:
>>> > When two or more fingers are on the touchpad, the 'w' slot holds
>the
>>> > finger count rather than the width. Retrieve the correct value
>encoded
>>> > in the lower bits of 'x', 'y' and 'z'.
>>> >
>>> > The minimum width reported is 8 rather than 4 in this case, while
>the
>>> > maximum remains 15.
>>> >
>>> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
>>> > ---
>>> > drivers/input/mouse/synaptics.c | 4 +++-
>>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/drivers/input/mouse/synaptics.c
>b/drivers/input/mouse/synaptics.c
>>> > index ea0563e..5ff4c5b 100644
>>> > --- a/drivers/input/mouse/synaptics.c
>>> > +++ b/drivers/input/mouse/synaptics.c
>>> > @@ -593,7 +593,9 @@ static void synaptics_parse_agm(const unsigned
>char buf[],
>>> > switch (agm_packet_type) {
>>> > case 1:
>>> > /* Gesture packet: (x, y, z) half resolution */
>>> > - agm->w = hw->w;
>>> > + agm->w = ((buf[1] & 0x01) |
>>> > + ((buf[2] & 0x01) << 1) |
>>> > + ((buf[5] & 0x01) << 2)) + 8;
>>> > agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
>>>
>>> For completeness, I think, we should also removes the w bits from x,
>y
>>> and z (by masking buf[1], buf[2] and buf[5] with 0xfe).
>>
>> You are right, I forgot about it.
>>
>> I was going to re-submit these patches anyway since there have been
>> quite a few changes and these can no longer be applied on next.
>
>OK, I am trying to put together my thoughts for 1/2 right now. If you
>could just wait a little before resubmitting, I would be grateful.
>
BTW, do we really get different widths four different contacts?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] input: synaptics - fix width calculation on image sensors
From: Benjamin Tissoires @ 2015-01-05 19:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Gabriele Mazzotta, Henrik Rydberg, linux-input,
linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <3ACB879C-A553-49A0-B170-CFF14A494BEE@gmail.com>
On Mon, Jan 5, 2015 at 2:04 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On January 5, 2015 10:42:13 AM PST, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote:
>>On Mon, Jan 5, 2015 at 1:37 PM, Gabriele Mazzotta
>><gabriele.mzt@gmail.com> wrote:
>>> On Monday 05 January 2015 13:25:38 Benjamin Tissoires wrote:
>>>> Hi Gabriele,
>>>>
>>>> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
>>>> <gabriele.mzt@gmail.com> wrote:
>>>> > When two or more fingers are on the touchpad, the 'w' slot holds
>>the
>>>> > finger count rather than the width. Retrieve the correct value
>>encoded
>>>> > in the lower bits of 'x', 'y' and 'z'.
>>>> >
>>>> > The minimum width reported is 8 rather than 4 in this case, while
>>the
>>>> > maximum remains 15.
>>>> >
>>>> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
>>>> > ---
>>>> > drivers/input/mouse/synaptics.c | 4 +++-
>>>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>>>> >
>>>> > diff --git a/drivers/input/mouse/synaptics.c
>>b/drivers/input/mouse/synaptics.c
>>>> > index ea0563e..5ff4c5b 100644
>>>> > --- a/drivers/input/mouse/synaptics.c
>>>> > +++ b/drivers/input/mouse/synaptics.c
>>>> > @@ -593,7 +593,9 @@ static void synaptics_parse_agm(const unsigned
>>char buf[],
>>>> > switch (agm_packet_type) {
>>>> > case 1:
>>>> > /* Gesture packet: (x, y, z) half resolution */
>>>> > - agm->w = hw->w;
>>>> > + agm->w = ((buf[1] & 0x01) |
>>>> > + ((buf[2] & 0x01) << 1) |
>>>> > + ((buf[5] & 0x01) << 2)) + 8;
>>>> > agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
>>>>
>>>> For completeness, I think, we should also removes the w bits from x,
>>y
>>>> and z (by masking buf[1], buf[2] and buf[5] with 0xfe).
>>>
>>> You are right, I forgot about it.
>>>
>>> I was going to re-submit these patches anyway since there have been
>>> quite a few changes and these can no longer be applied on next.
>>
>>OK, I am trying to put together my thoughts for 1/2 right now. If you
>>could just wait a little before resubmitting, I would be grateful.
>>
>
> BTW, do we really get different widths four different contacts?
>
I think we should. However, on capacitive sensors, the "width" and the
"pressure" fields are 2 ways of sending the same initial information.
Given that we already have two different pressure info, we should thus
get 2 different widths.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH 2/2] input: synaptics - fix width calculation on image sensors
From: Gabriele Mazzotta @ 2015-01-05 19:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Henrik Rydberg, linux-input,
linux-kernel@vger.kernel.org, silverhammermba
In-Reply-To: <3ACB879C-A553-49A0-B170-CFF14A494BEE@gmail.com>
On Monday 05 January 2015 11:04:07 Dmitry Torokhov wrote:
> On January 5, 2015 10:42:13 AM PST, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote:
> >On Mon, Jan 5, 2015 at 1:37 PM, Gabriele Mazzotta
> ><gabriele.mzt@gmail.com> wrote:
> >> On Monday 05 January 2015 13:25:38 Benjamin Tissoires wrote:
> >>> Hi Gabriele,
> >>>
> >>> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
> >>> <gabriele.mzt@gmail.com> wrote:
> >>> > When two or more fingers are on the touchpad, the 'w' slot holds
> >the
> >>> > finger count rather than the width. Retrieve the correct value
> >encoded
> >>> > in the lower bits of 'x', 'y' and 'z'.
> >>> >
> >>> > The minimum width reported is 8 rather than 4 in this case, while
> >the
> >>> > maximum remains 15.
> >>> >
> >>> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> >>> > ---
> >>> > drivers/input/mouse/synaptics.c | 4 +++-
> >>> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >>> >
> >>> > diff --git a/drivers/input/mouse/synaptics.c
> >b/drivers/input/mouse/synaptics.c
> >>> > index ea0563e..5ff4c5b 100644
> >>> > --- a/drivers/input/mouse/synaptics.c
> >>> > +++ b/drivers/input/mouse/synaptics.c
> >>> > @@ -593,7 +593,9 @@ static void synaptics_parse_agm(const unsigned
> >char buf[],
> >>> > switch (agm_packet_type) {
> >>> > case 1:
> >>> > /* Gesture packet: (x, y, z) half resolution */
> >>> > - agm->w = hw->w;
> >>> > + agm->w = ((buf[1] & 0x01) |
> >>> > + ((buf[2] & 0x01) << 1) |
> >>> > + ((buf[5] & 0x01) << 2)) + 8;
> >>> > agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
> >>>
> >>> For completeness, I think, we should also removes the w bits from x,
> >y
> >>> and z (by masking buf[1], buf[2] and buf[5] with 0xfe).
> >>
> >> You are right, I forgot about it.
> >>
> >> I was going to re-submit these patches anyway since there have been
> >> quite a few changes and these can no longer be applied on next.
> >
> >OK, I am trying to put together my thoughts for 1/2 right now. If you
> >could just wait a little before resubmitting, I would be grateful.
> >
>
> BTW, do we really get different widths four different contacts?
>
> Thanks.
I think that only the width of the last finger that was laid on the
touchpad is reported.
^ permalink raw reply
* Re: [PATCH 1/2] input: synaptics - make image sensors report ABS_MT_TOUCH_MAJOR
From: Benjamin Tissoires @ 2015-01-05 19:24 UTC (permalink / raw)
To: Gabriele Mazzotta, Peter Hutterer, Hans de Goede
Cc: Dmitry Torokhov, Henrik Rydberg, linux-input,
linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <1419679889-6582-2-git-send-email-gabriele.mzt@gmail.com>
Hi Gabriele,
[Adding Peter and Hans as this change will impact both
xf86-input-synaptics and libinput]
On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
<gabriele.mzt@gmail.com> wrote:
> Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors
> were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR
> instead ABS_TOOL_WIDTH.
It looks like the current xorg-synaptics code already handles
ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in
replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter
confirm this because xorg-synaptics still relies a lot on the single
touch emulation.
>
> Since the 'w' slot is used to report the finger count when two or more
> fingers are on the touchpad, make sure that only meaningful values are
> emitted, i.e. values greater than or equal to 4, and assign the correct
> range to ABS_MT_TOUCH_MAJOR.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> ---
> drivers/input/mouse/synaptics.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index f947292..ea0563e 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot,
Just FYI, this does not apply anymore on top of Dmitry's tree.
synaptics_report_slot() has been removed, and you should now report
the slot state in synaptics_report_mt_data().
> input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
> input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
> input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
> + if (hw->w >= 4)
That I don't like. IMO, at this point, .w should only contain the
finger width, unconditionally.
Also, with 2/2, .w is computed accurately for the second finger, but
not for the first.
I tried to figure out a way to properly extract the actual width
information from the sgm packet when the w is 0 or 1, and the only way
I found was to do the fix in synaptics_image_sensor_process(). I would
have preferred dealing with that in synaptics_parse_hw_state()
directly, but I think the final code would be more and more ugly.
Dealing with the true finger width in synaptics_image_sensor_process()
is not a problem for cr48 sensors, because they will not have the
ABS_MT_TOUCH_MAJOR event exported.
Anyway, something like that "should" work (sorry for the tab/space,
gmail in use), of course, this is untested when I send the mail :) :
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 95c15451..caf7ccd 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -852,6 +852,17 @@ static void synaptics_image_sensor_process(struct
psmouse *psmouse,
else
num_fingers = 4;
+ /* When multiple fingers are on the TouchPad, the width is encoded in
+ * X, Y and Z */
+ if (sgm->w == 0 || sgm->w == 1) {
+ sgm->w = ((sgm->x & BIT(1)) >> 1) |
+ (sgm->y & BIT(1)) |
+ ((sgm->z & BIT(0)) << 1);
+ sgm->x &= ~BIT(1);
+ sgm->y &= ~BIT(1);
+ sgm->z &= ~BIT(0);
+ }
+
/* Send resulting input events to user space */
synaptics_report_mt_data(psmouse, sgm, num_fingers);
}
> + input_report_abs(dev, ABS_MT_TOUCH_MAJOR, hw->w);
> }
>
> static void synaptics_report_mt_data(struct psmouse *psmouse,
> @@ -1462,8 +1464,13 @@ static void set_input_params(struct psmouse *psmouse,
> INPUT_MT_TRACK : INPUT_MT_SEMI_MT));
> }
>
> - if (SYN_CAP_PALMDETECT(priv->capabilities))
> - input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> + if (SYN_CAP_PALMDETECT(priv->capabilities)) {
> + if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
> + input_set_abs_params(dev,
> + ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> + else
> + input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> + }
Again, I'd prefer having Peter's point of view here.
If we really need both, we will need to change input-mt to also
emulate ABS_TOOL_WIDTH from ABS_MT_TOUCH_MAJOR like we do between
ABS_MT_POSITION_X|Y and ABS_X|Y.
Cheers,
Benjamin
>
> __set_bit(BTN_TOUCH, dev->keybit);
> __set_bit(BTN_TOOL_FINGER, dev->keybit);
> --
> 2.1.4
>
> --
> 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
^ permalink raw reply related
* Re: [PATCH 2/2] input: synaptics - fix width calculation on image sensors
From: Benjamin Tissoires @ 2015-01-05 19:26 UTC (permalink / raw)
To: Gabriele Mazzotta
Cc: Dmitry Torokhov, Henrik Rydberg, linux-input,
linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <3368304.jm6jgTz8dd@xps13>
On Mon, Jan 5, 2015 at 2:15 PM, Gabriele Mazzotta
<gabriele.mzt@gmail.com> wrote:
> On Monday 05 January 2015 11:04:07 Dmitry Torokhov wrote:
>> On January 5, 2015 10:42:13 AM PST, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote:
>> >On Mon, Jan 5, 2015 at 1:37 PM, Gabriele Mazzotta
>> ><gabriele.mzt@gmail.com> wrote:
>> >> On Monday 05 January 2015 13:25:38 Benjamin Tissoires wrote:
>> >>> Hi Gabriele,
>> >>>
>> >>> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
>> >>> <gabriele.mzt@gmail.com> wrote:
>> >>> > When two or more fingers are on the touchpad, the 'w' slot holds
>> >the
>> >>> > finger count rather than the width. Retrieve the correct value
>> >encoded
>> >>> > in the lower bits of 'x', 'y' and 'z'.
>> >>> >
>> >>> > The minimum width reported is 8 rather than 4 in this case, while
>> >the
>> >>> > maximum remains 15.
>> >>> >
>> >>> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
>> >>> > ---
>> >>> > drivers/input/mouse/synaptics.c | 4 +++-
>> >>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>> >>> >
>> >>> > diff --git a/drivers/input/mouse/synaptics.c
>> >b/drivers/input/mouse/synaptics.c
>> >>> > index ea0563e..5ff4c5b 100644
>> >>> > --- a/drivers/input/mouse/synaptics.c
>> >>> > +++ b/drivers/input/mouse/synaptics.c
>> >>> > @@ -593,7 +593,9 @@ static void synaptics_parse_agm(const unsigned
>> >char buf[],
>> >>> > switch (agm_packet_type) {
>> >>> > case 1:
>> >>> > /* Gesture packet: (x, y, z) half resolution */
>> >>> > - agm->w = hw->w;
>> >>> > + agm->w = ((buf[1] & 0x01) |
>> >>> > + ((buf[2] & 0x01) << 1) |
>> >>> > + ((buf[5] & 0x01) << 2)) + 8;
>> >>> > agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
>> >>>
>> >>> For completeness, I think, we should also removes the w bits from x,
>> >y
>> >>> and z (by masking buf[1], buf[2] and buf[5] with 0xfe).
>> >>
>> >> You are right, I forgot about it.
>> >>
>> >> I was going to re-submit these patches anyway since there have been
>> >> quite a few changes and these can no longer be applied on next.
>> >
>> >OK, I am trying to put together my thoughts for 1/2 right now. If you
>> >could just wait a little before resubmitting, I would be grateful.
>> >
>>
>> BTW, do we really get different widths four different contacts?
>>
>> Thanks.
>
> I think that only the width of the last finger that was laid on the
> touchpad is reported.
According to the Synaptics PS2 Interfacing guide, the width of the
primary finger is also reported. See
http://www.synaptics.com/sites/default/files/511-000275-01_RevB.pdf
section 3.2.10. page 32)
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH 1/2] input: synaptics - make image sensors report ABS_MT_TOUCH_MAJOR
From: Gabriele Mazzotta @ 2015-01-05 20:13 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Peter Hutterer, Hans de Goede, Dmitry Torokhov, Henrik Rydberg,
linux-input, linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <CAN+gG=Fhaj2aQM7gxXZBaXx3MZQbdMdCZ7KO=nmhzufp4YqnmA@mail.gmail.com>
On Monday 05 January 2015 14:24:30 Benjamin Tissoires wrote:
> Hi Gabriele,
>
> [Adding Peter and Hans as this change will impact both
> xf86-input-synaptics and libinput]
>
> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
> <gabriele.mzt@gmail.com> wrote:
> > Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors
> > were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR
> > instead ABS_TOOL_WIDTH.
>
> It looks like the current xorg-synaptics code already handles
> ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in
> replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter
> confirm this because xorg-synaptics still relies a lot on the single
> touch emulation.
Yes, xorg-synaptics handles ABS_MT_TOUCH_MAJOR, I sent the patch for it.
The width of the fingers is used only for the two fingers emulation and
the palm detection, both of which are broken on image sensors as the
width is never reported.
I'm normally using hid-rmi for my touchpad which never sends
ABS_TOOL_WIDTH and everything seems to work correctly. Same for
synaptics with this patch, although it seems that the pressure drops
drastically when there are more than one finger, so this must be taken
into account when defining the threshold values. I'll double check that
everything is correct for 'z'.
> >
> > Since the 'w' slot is used to report the finger count when two or more
> > fingers are on the touchpad, make sure that only meaningful values are
> > emitted, i.e. values greater than or equal to 4, and assign the correct
> > range to ABS_MT_TOUCH_MAJOR.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > ---
> > drivers/input/mouse/synaptics.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > index f947292..ea0563e 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot,
>
> Just FYI, this does not apply anymore on top of Dmitry's tree.
> synaptics_report_slot() has been removed, and you should now report
> the slot state in synaptics_report_mt_data().
>
> > input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
> > input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
> > input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
> > + if (hw->w >= 4)
>
> That I don't like. IMO, at this point, .w should only contain the
> finger width, unconditionally.
> Also, with 2/2, .w is computed accurately for the second finger, but
> not for the first.
>
> I tried to figure out a way to properly extract the actual width
> information from the sgm packet when the w is 0 or 1, and the only way
> I found was to do the fix in synaptics_image_sensor_process(). I would
> have preferred dealing with that in synaptics_parse_hw_state()
> directly, but I think the final code would be more and more ugly.
> Dealing with the true finger width in synaptics_image_sensor_process()
> is not a problem for cr48 sensors, because they will not have the
> ABS_MT_TOUCH_MAJOR event exported.
>
> Anyway, something like that "should" work (sorry for the tab/space,
> gmail in use), of course, this is untested when I send the mail :) :
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 95c15451..caf7ccd 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -852,6 +852,17 @@ static void synaptics_image_sensor_process(struct
> psmouse *psmouse,
> else
> num_fingers = 4;
>
> + /* When multiple fingers are on the TouchPad, the width is encoded in
> + * X, Y and Z */
> + if (sgm->w == 0 || sgm->w == 1) {
> + sgm->w = ((sgm->x & BIT(1)) >> 1) |
> + (sgm->y & BIT(1)) |
> + ((sgm->z & BIT(0)) << 1);
> + sgm->x &= ~BIT(1);
> + sgm->y &= ~BIT(1);
> + sgm->z &= ~BIT(0);
> + }
> +
It works perfectly with some adjustments:
/* When multiple fingers are on the TouchPad, the width is encoded in
* X, Y and Z */
if (sgm->w == 0 || sgm->w == 1) {
sgm->w = (((sgm->x & BIT(1)) >> 1) |
(sgm->y & BIT(1)) |
((sgm->z & BIT(0)) << 2)) + 8;
sgm->x &= ~BIT(1);
sgm->y &= ~BIT(1);
sgm->z &= ~BIT(0);
}
> /* Send resulting input events to user space */
> synaptics_report_mt_data(psmouse, sgm, num_fingers);
> }
>
>
> > + input_report_abs(dev, ABS_MT_TOUCH_MAJOR, hw->w);
> > }
> >
> > static void synaptics_report_mt_data(struct psmouse *psmouse,
> > @@ -1462,8 +1464,13 @@ static void set_input_params(struct psmouse *psmouse,
> > INPUT_MT_TRACK : INPUT_MT_SEMI_MT));
> > }
> >
> > - if (SYN_CAP_PALMDETECT(priv->capabilities))
> > - input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> > + if (SYN_CAP_PALMDETECT(priv->capabilities)) {
> > + if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
> > + input_set_abs_params(dev,
> > + ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> > + else
> > + input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> > + }
>
> Again, I'd prefer having Peter's point of view here.
>
> If we really need both, we will need to change input-mt to also
> emulate ABS_TOOL_WIDTH from ABS_MT_TOUCH_MAJOR like we do between
> ABS_MT_POSITION_X|Y and ABS_X|Y.
>
> Cheers,
> Benjamin
>
> >
> > __set_bit(BTN_TOUCH, dev->keybit);
> > __set_bit(BTN_TOOL_FINGER, dev->keybit);
> > --
> > 2.1.4
> >
> > --
> > 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
^ permalink raw reply
* Re: [PATCH 1/2] input: synaptics - make image sensors report ABS_MT_TOUCH_MAJOR
From: Gabriele Mazzotta @ 2015-01-05 20:18 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Peter Hutterer, Hans de Goede, Dmitry Torokhov, Henrik Rydberg,
linux-input, linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <4526075.fzTQl3K7DK@xps13>
On Monday 05 January 2015 21:13:09 Gabriele Mazzotta wrote:
> On Monday 05 January 2015 14:24:30 Benjamin Tissoires wrote:
> > Hi Gabriele,
> >
> > [Adding Peter and Hans as this change will impact both
> > xf86-input-synaptics and libinput]
> >
> > On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
> > <gabriele.mzt@gmail.com> wrote:
> > > Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors
> > > were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR
> > > instead ABS_TOOL_WIDTH.
> >
> > It looks like the current xorg-synaptics code already handles
> > ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in
> > replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter
> > confirm this because xorg-synaptics still relies a lot on the single
> > touch emulation.
>
> Yes, xorg-synaptics handles ABS_MT_TOUCH_MAJOR, I sent the patch for it.
> The width of the fingers is used only for the two fingers emulation and
> the palm detection, both of which are broken on image sensors as the
> width is never reported.
>
> I'm normally using hid-rmi for my touchpad which never sends
> ABS_TOOL_WIDTH and everything seems to work correctly. Same for
> synaptics with this patch, although it seems that the pressure drops
> drastically when there are more than one finger, so this must be taken
> into account when defining the threshold values. I'll double check that
> everything is correct for 'z'.
It was wrong
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 808c8e3..b3aed2e 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -590,7 +590,7 @@ static void synaptics_parse_agm(const unsigned char buf[],
((buf[5] & 0x01) << 2)) + 8;
agm->x = (((buf[4] & 0x0f) << 8) | (buf[1] & 0xfe)) << 1;
agm->y = (((buf[4] & 0xf0) << 4) | (buf[2] & 0xfe)) << 1;
- agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0e)) << 1;
+ agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0e)) << 2;
break;
case 2:
> > >
> > > Since the 'w' slot is used to report the finger count when two or more
> > > fingers are on the touchpad, make sure that only meaningful values are
> > > emitted, i.e. values greater than or equal to 4, and assign the correct
> > > range to ABS_MT_TOUCH_MAJOR.
> > >
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> > > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > > ---
> > > drivers/input/mouse/synaptics.c | 11 +++++++++--
> > > 1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > > index f947292..ea0563e 100644
> > > --- a/drivers/input/mouse/synaptics.c
> > > +++ b/drivers/input/mouse/synaptics.c
> > > @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot,
> >
> > Just FYI, this does not apply anymore on top of Dmitry's tree.
> > synaptics_report_slot() has been removed, and you should now report
> > the slot state in synaptics_report_mt_data().
> >
> > > input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
> > > input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
> > > input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
> > > + if (hw->w >= 4)
> >
> > That I don't like. IMO, at this point, .w should only contain the
> > finger width, unconditionally.
> > Also, with 2/2, .w is computed accurately for the second finger, but
> > not for the first.
> >
> > I tried to figure out a way to properly extract the actual width
> > information from the sgm packet when the w is 0 or 1, and the only way
> > I found was to do the fix in synaptics_image_sensor_process(). I would
> > have preferred dealing with that in synaptics_parse_hw_state()
> > directly, but I think the final code would be more and more ugly.
> > Dealing with the true finger width in synaptics_image_sensor_process()
> > is not a problem for cr48 sensors, because they will not have the
> > ABS_MT_TOUCH_MAJOR event exported.
> >
> > Anyway, something like that "should" work (sorry for the tab/space,
> > gmail in use), of course, this is untested when I send the mail :) :
> >
> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > index 95c15451..caf7ccd 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -852,6 +852,17 @@ static void synaptics_image_sensor_process(struct
> > psmouse *psmouse,
> > else
> > num_fingers = 4;
> >
> > + /* When multiple fingers are on the TouchPad, the width is encoded in
> > + * X, Y and Z */
> > + if (sgm->w == 0 || sgm->w == 1) {
> > + sgm->w = ((sgm->x & BIT(1)) >> 1) |
> > + (sgm->y & BIT(1)) |
> > + ((sgm->z & BIT(0)) << 1);
> > + sgm->x &= ~BIT(1);
> > + sgm->y &= ~BIT(1);
> > + sgm->z &= ~BIT(0);
> > + }
> > +
>
> It works perfectly with some adjustments:
>
> /* When multiple fingers are on the TouchPad, the width is encoded in
> * X, Y and Z */
> if (sgm->w == 0 || sgm->w == 1) {
> sgm->w = (((sgm->x & BIT(1)) >> 1) |
> (sgm->y & BIT(1)) |
> ((sgm->z & BIT(0)) << 2)) + 8;
> sgm->x &= ~BIT(1);
> sgm->y &= ~BIT(1);
> sgm->z &= ~BIT(0);
> }
>
> > /* Send resulting input events to user space */
> > synaptics_report_mt_data(psmouse, sgm, num_fingers);
> > }
> >
> >
> > > + input_report_abs(dev, ABS_MT_TOUCH_MAJOR, hw->w);
> > > }
> > >
> > > static void synaptics_report_mt_data(struct psmouse *psmouse,
> > > @@ -1462,8 +1464,13 @@ static void set_input_params(struct psmouse *psmouse,
> > > INPUT_MT_TRACK : INPUT_MT_SEMI_MT));
> > > }
> > >
> > > - if (SYN_CAP_PALMDETECT(priv->capabilities))
> > > - input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> > > + if (SYN_CAP_PALMDETECT(priv->capabilities)) {
> > > + if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
> > > + input_set_abs_params(dev,
> > > + ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> > > + else
> > > + input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> > > + }
> >
> > Again, I'd prefer having Peter's point of view here.
> >
> > If we really need both, we will need to change input-mt to also
> > emulate ABS_TOOL_WIDTH from ABS_MT_TOUCH_MAJOR like we do between
> > ABS_MT_POSITION_X|Y and ABS_X|Y.
> >
> > Cheers,
> > Benjamin
> >
> > >
> > > __set_bit(BTN_TOUCH, dev->keybit);
> > > __set_bit(BTN_TOOL_FINGER, dev->keybit);
> > > --
> > > 2.1.4
> > >
> > > --
> > > 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
^ permalink raw reply related
* Re: hid-replay captured data
From: Benjamin Tissoires @ 2015-01-05 21:07 UTC (permalink / raw)
To: josep.sanchez.ferreres
Cc: linux-input, Ping Cheng, Jason Gerecke, Aaron Skomra
In-Reply-To: <CAN+gG=H5Zxfn1mRRYoypnMetfWzzOtc6XuvoJmKmJC8AW+9Wwg@mail.gmail.com>
Hi Josep,
On Mon, Dec 8, 2014 at 5:04 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> Thanks for the logs (both way arrived in my mailbox, attached file or pastebin).
>
> I'll try to have a look at them on Wednesday.
Actually, it's nearly been a month. Thanks for the reminder. (However,
it's easier to track the various bugs with only one thread, not
several).
So apologies for the delay.
>
> On Mon, Dec 8, 2014 at 3:52 PM, <josep.sanchez.ferreres@est.fib.upc.edu> wrote:
>> I attach a .tar.gz with all the captures I took, the filenames should be
>> self explanatory. If you prefer me to dump all that into the e-mail text
>> field just ask, but it's quite a big amount of data.
>>
>> Both 3.16 and 3.18 kernels seemed to respond to the same events. Oddly
>> enough I've noticed that although 3 devices for rawhid were listed, only two
>> captured events. Here's the output from hid-replay that shows my devices:
>>
>> Available devices:
>> /dev/hidraw0: USB Keyboard
>> /dev/hidraw1: USB Keyboard
>> /dev/hidraw2: USB OPTICAL MOUSE
>> /dev/hidraw3: Wacom Co.,Ltd. Bamboo Pad, USB
>> /dev/hidraw4: Wacom Co.,Ltd. Bamboo Pad, USB
>> /dev/hidraw5: Wacom Co.,Ltd. Bamboo Pad, USB
>>
>> Only hidraw4 and 5 reported events to hid-replay. Also note that hidraw5
>> corresponds to the touchpad while hidraw5 corresponds to the stylus.
Hmm, I'd like to see the evemu-record output of hidraw3 still. hidraw4
is for now completely ignored by wacom on purpose because all the rest
of the wacom tablets have a mouse emulation mode which looks like this
one. I think we might be able to report true multitouch data through
hidraw3, but I need to have a look at it.
>>
>> I also just found out that the stylus hardware button is captured by the
>> touchpad device and not by the stylus one (that actually makes more sense as
>> it acts like a mouse button), so all the data I put about the stylus key
>> should just contain stylus movement data. I added a file stylus-key.hid
>> which is actually capturing the clicks for that button.
Weird. I managed to make the stylus working properly with the current
code base (patches to follow soon).
The stylus secondary button was send through the stylus interface, so
I am not quite following you here.
>>
>> I should also say that some events, like clicking the screen with the stylus
>> will be mixed with the mouse movement (I tried to avoid that by clicking
>> fast enough but I guess it's impossible to completely avoid it).
That's not a problem per se. It's easy to have a look at the events
and filter out what is not required. But thanks for the effort though,
it simplifies part of the filtering!
Cheers,
Benjamin
>>
>> PD: I see you're using a gmail address to reply, should I reply to that
>> address better?
>
> I think that's better to keep all the people in CC of the thread. we
> generally have rules to filter linux-input/lkml mails, so it avoids
> losing traces of a thread.
>
> Cheers,
> Benjamin
^ permalink raw reply
* [PATCH 2/2] HID: wacom: add support of the Pen of the Bamboo Pad
From: Benjamin Tissoires @ 2015-01-05 21:32 UTC (permalink / raw)
To: Jiri Kosina, Jason Gerecke
Cc: josep.sanchez.ferreres, Ping Cheng, linux-input, linux-kernel
In-Reply-To: <1420493533-11639-1-git-send-email-benjamin.tissoires@redhat.com>
Bamboo Pads are using the generic processing but their report descriptors
differ from the ISDv* line. The pen fields are marked with the .physical
as Digitizer_Pen, which makes also sense.
Add this field to the checks and enable for free Bamboo Pads.
Reported-by: Josep Sanchez Ferreres <josep.sanchez.ferreres@est.fib.upc.edu>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Hi Jason,
I still did not managed to put in shape the regression tests for the Wacom
recordings we have. I do not think this will impact the current supported
devices (none that I have present the HID_DG_PEN application/logical/physical
tag).
It would be good still if you could have a look and validate the patch series.
Cheers,
Benjamin
drivers/hid/wacom_wac.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 7436f2b..7afd929 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -74,6 +74,7 @@
#define WACOM_PEN_FIELD(f) (((f)->logical == HID_DG_STYLUS) || \
((f)->physical == HID_DG_STYLUS) || \
+ ((f)->physical == HID_DG_PEN) || \
((f)->application == HID_DG_PEN))
#define WACOM_FINGER_FIELD(f) (((f)->logical == HID_DG_FINGER) || \
((f)->physical == HID_DG_FINGER) || \
--
2.1.0
^ permalink raw reply related
* [PATCH 1/2] HID: wacom: use WACOM_*_FIELD macros in wacom_usage_mapping()
From: Benjamin Tissoires @ 2015-01-05 21:32 UTC (permalink / raw)
To: Jiri Kosina, Jason Gerecke
Cc: josep.sanchez.ferreres, Ping Cheng, linux-input, linux-kernel
We introduced nice macros in wacom_wac.c to check whether a field is
a pen or a touch one.
wacom_usage_mapping() still uses it's own tests, which are not in sync with
the wacom_wac tests (.application is not checked).
That means that some legitimate fields might be filtered out from the
usage mapping, and thus will not be used properly while receiving the
events.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/wacom_sys.c | 6 ++----
drivers/hid/wacom_wac.c | 8 --------
drivers/hid/wacom_wac.h | 8 ++++++++
3 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 6542029..f01ab3a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -173,10 +173,8 @@ static void wacom_usage_mapping(struct hid_device *hdev,
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_features *features = &wacom->wacom_wac.features;
- bool finger = (field->logical == HID_DG_FINGER) ||
- (field->physical == HID_DG_FINGER);
- bool pen = (field->logical == HID_DG_STYLUS) ||
- (field->physical == HID_DG_STYLUS);
+ bool finger = WACOM_FINGER_FIELD(field);
+ bool pen = WACOM_PEN_FIELD(field);
/*
* Requiring Stylus Usage will ignore boot mouse
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index ac7447c..596a6fb5 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -15,7 +15,6 @@
#include "wacom_wac.h"
#include "wacom.h"
#include <linux/input/mt.h>
-#include <linux/hid.h>
/* resolution for penabled devices */
#define WACOM_PL_RES 20
@@ -1514,13 +1513,6 @@ static void wacom_wac_finger_report(struct hid_device *hdev,
wacom_wac->shared->touch_down = wacom_wac_finger_count_touches(hdev);
}
-#define WACOM_PEN_FIELD(f) (((f)->logical == HID_DG_STYLUS) || \
- ((f)->physical == HID_DG_STYLUS) || \
- ((f)->application == HID_DG_PEN))
-#define WACOM_FINGER_FIELD(f) (((f)->logical == HID_DG_FINGER) || \
- ((f)->physical == HID_DG_FINGER) || \
- ((f)->application == HID_DG_TOUCHSCREEN))
-
void wacom_wac_usage_mapping(struct hid_device *hdev,
struct hid_field *field, struct hid_usage *usage)
{
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index bfad815..7436f2b 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -10,6 +10,7 @@
#define WACOM_WAC_H
#include <linux/types.h>
+#include <linux/hid.h>
/* maximum packet length for USB devices */
#define WACOM_PKGLEN_MAX 68
@@ -71,6 +72,13 @@
#define WACOM_QUIRK_MONITOR 0x0008
#define WACOM_QUIRK_BATTERY 0x0010
+#define WACOM_PEN_FIELD(f) (((f)->logical == HID_DG_STYLUS) || \
+ ((f)->physical == HID_DG_STYLUS) || \
+ ((f)->application == HID_DG_PEN))
+#define WACOM_FINGER_FIELD(f) (((f)->logical == HID_DG_FINGER) || \
+ ((f)->physical == HID_DG_FINGER) || \
+ ((f)->application == HID_DG_TOUCHSCREEN))
+
enum {
PENPARTNER = 0,
GRAPHIRE,
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 1/2] input: synaptics - make image sensors report ABS_MT_TOUCH_MAJOR
From: Gabriele Mazzotta @ 2015-01-05 22:00 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Peter Hutterer, Hans de Goede, Dmitry Torokhov, Henrik Rydberg,
linux-input, linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <CAN+gG=Fhaj2aQM7gxXZBaXx3MZQbdMdCZ7KO=nmhzufp4YqnmA@mail.gmail.com>
On Monday 05 January 2015 14:24:30 Benjamin Tissoires wrote:
> Hi Gabriele,
>
> [Adding Peter and Hans as this change will impact both
> xf86-input-synaptics and libinput]
>
> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
> <gabriele.mzt@gmail.com> wrote:
> > Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors
> > were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR
> > instead ABS_TOOL_WIDTH.
>
> It looks like the current xorg-synaptics code already handles
> ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in
> replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter
> confirm this because xorg-synaptics still relies a lot on the single
> touch emulation.
>
> >
> > Since the 'w' slot is used to report the finger count when two or more
> > fingers are on the touchpad, make sure that only meaningful values are
> > emitted, i.e. values greater than or equal to 4, and assign the correct
> > range to ABS_MT_TOUCH_MAJOR.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > ---
> > drivers/input/mouse/synaptics.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > index f947292..ea0563e 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot,
>
> Just FYI, this does not apply anymore on top of Dmitry's tree.
> synaptics_report_slot() has been removed, and you should now report
> the slot state in synaptics_report_mt_data().
>
> > input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
> > input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
> > input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
> > + if (hw->w >= 4)
>
> That I don't like. IMO, at this point, .w should only contain the
> finger width, unconditionally.
> Also, with 2/2, .w is computed accurately for the second finger, but
> not for the first.
>
> I tried to figure out a way to properly extract the actual width
> information from the sgm packet when the w is 0 or 1, and the only way
> I found was to do the fix in synaptics_image_sensor_process(). I would
> have preferred dealing with that in synaptics_parse_hw_state()
> directly, but I think the final code would be more and more ugly.
> Dealing with the true finger width in synaptics_image_sensor_process()
> is not a problem for cr48 sensors, because they will not have the
> ABS_MT_TOUCH_MAJOR event exported.
Regarding the last part on cr48 sensors.
Currently these sensors are not reporting widths through ABS_TOOL_WIDTH
and I don't see what could go wrong if they start reporting
ABS_MT_TOUCH_MAJOR. If I understood correctly, they can report widths
only when one finger is on the touchpad. This means that they will
report widths through slot 0, but they won't through slot 1. Nothing
bad should happen.
I've just rebased my patches and included the support for cr48 sensors.
The only change I made was to change the default width value from
5 to 4 since this is the minimum values these sensors can report
(according to the existing code).
Anyway, should I simply include the changes you suggested to handle
widths of sgm packets in what will be the new 2/2?
^ permalink raw reply
* Re: [PATCH 1/2] input: synaptics - make image sensors report ABS_MT_TOUCH_MAJOR
From: Benjamin Tissoires @ 2015-01-05 22:04 UTC (permalink / raw)
To: Gabriele Mazzotta
Cc: Peter Hutterer, Hans de Goede, Dmitry Torokhov, Henrik Rydberg,
linux-input, linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <1777845.g1sZJx65aj@xps13>
On Mon, Jan 5, 2015 at 5:00 PM, Gabriele Mazzotta
<gabriele.mzt@gmail.com> wrote:
> On Monday 05 January 2015 14:24:30 Benjamin Tissoires wrote:
>> Hi Gabriele,
>>
>> [Adding Peter and Hans as this change will impact both
>> xf86-input-synaptics and libinput]
>>
>> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
>> <gabriele.mzt@gmail.com> wrote:
>> > Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors
>> > were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR
>> > instead ABS_TOOL_WIDTH.
>>
>> It looks like the current xorg-synaptics code already handles
>> ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in
>> replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter
>> confirm this because xorg-synaptics still relies a lot on the single
>> touch emulation.
>>
>> >
>> > Since the 'w' slot is used to report the finger count when two or more
>> > fingers are on the touchpad, make sure that only meaningful values are
>> > emitted, i.e. values greater than or equal to 4, and assign the correct
>> > range to ABS_MT_TOUCH_MAJOR.
>> >
>> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
>> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
>> > ---
>> > drivers/input/mouse/synaptics.c | 11 +++++++++--
>> > 1 file changed, 9 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> > index f947292..ea0563e 100644
>> > --- a/drivers/input/mouse/synaptics.c
>> > +++ b/drivers/input/mouse/synaptics.c
>> > @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot,
>>
>> Just FYI, this does not apply anymore on top of Dmitry's tree.
>> synaptics_report_slot() has been removed, and you should now report
>> the slot state in synaptics_report_mt_data().
>>
>> > input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
>> > input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
>> > input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
>> > + if (hw->w >= 4)
>>
>> That I don't like. IMO, at this point, .w should only contain the
>> finger width, unconditionally.
>> Also, with 2/2, .w is computed accurately for the second finger, but
>> not for the first.
>>
>> I tried to figure out a way to properly extract the actual width
>> information from the sgm packet when the w is 0 or 1, and the only way
>> I found was to do the fix in synaptics_image_sensor_process(). I would
>> have preferred dealing with that in synaptics_parse_hw_state()
>> directly, but I think the final code would be more and more ugly.
>> Dealing with the true finger width in synaptics_image_sensor_process()
>> is not a problem for cr48 sensors, because they will not have the
>> ABS_MT_TOUCH_MAJOR event exported.
>
> Regarding the last part on cr48 sensors.
> Currently these sensors are not reporting widths through ABS_TOOL_WIDTH
> and I don't see what could go wrong if they start reporting
> ABS_MT_TOUCH_MAJOR. If I understood correctly, they can report widths
> only when one finger is on the touchpad. This means that they will
> report widths through slot 0, but they won't through slot 1. Nothing
> bad should happen.
I am not entirely sure. The entire purpose of having widht for palm
detection is to filter palm from true finger events. So if we only
have the width info on the first slot, it would be useless IMO.
Still I agree with "nothing bad should happen" :)
>
> I've just rebased my patches and included the support for cr48 sensors.
> The only change I made was to change the default width value from
> 5 to 4 since this is the minimum values these sensors can report
> (according to the existing code).
>
> Anyway, should I simply include the changes you suggested to handle
> widths of sgm packets in what will be the new 2/2?
Go ahead and include my changes in your 2/2. It's better to have a
consistent commit.
Cheers,
Benjamin
^ permalink raw reply
* [PATCH v2 0/4] input: synaptics - report correct width and pressure values
From: Gabriele Mazzotta @ 2015-01-05 22:28 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, rydberg, linux-kernel, silverhammermba,
peter.hutterer, hdegoede, benjamin.tissoires, Gabriele Mazzotta
Make image sensors and cr48 sensors report widths and fix the
calculation of width and pressure values in some cases.
Changes since v1:
- Rebased on 35393dcb2ed3
- Get widths from AGM packets too
- Include support for cr48 sensors
- Fix pressure values on image sensors
Gabriele Mazzotta (4):
input: synaptics - fix pressure values calculation on image sensors
input: synaptics - fix width values calculation on image sensors
input: synaptics - change default width value of cr48 sensors
input: synaptics - make semi-mt touchpads report widths
drivers/input/mouse/synaptics.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH v2 1/4] input: synaptics - fix pressure values calculation on image sensors
From: Gabriele Mazzotta @ 2015-01-05 22:28 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, rydberg, linux-kernel, silverhammermba,
peter.hutterer, hdegoede, benjamin.tissoires, Gabriele Mazzotta
In-Reply-To: <1420496915-31560-1-git-send-email-gabriele.mzt@gmail.com>
The pressure values retrieved from secondary packets was incorrectly
shifted, making them lower than what they actually were.
Since this only happened with secondary packets, the values reported
when only one finger was present on the touchpad were correct.
Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
drivers/input/mouse/synaptics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index f89de89..4d22ebd 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -588,7 +588,7 @@ static void synaptics_parse_agm(const unsigned char buf[],
agm->w = hw->w;
agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
- agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
+ agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 2;
break;
case 2:
--
2.1.4
^ permalink raw reply related
* [PATCH v2 3/4] input: synaptics - change default width value of cr48 sensors
From: Gabriele Mazzotta @ 2015-01-05 22:28 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, rydberg, linux-kernel, silverhammermba,
peter.hutterer, hdegoede, benjamin.tissoires, Gabriele Mazzotta
In-Reply-To: <1420496915-31560-1-git-send-email-gabriele.mzt@gmail.com>
The minimum value these sensors can report is 4, so this should be the
value used when W is not reporting the width.
Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
drivers/input/mouse/synaptics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 918a727..5008e8c 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -910,7 +910,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
if (hw.z > 0 && hw.x > 1) {
num_fingers = 1;
- finger_width = 5;
+ finger_width = 4;
if (SYN_CAP_EXTENDED(priv->capabilities)) {
switch (hw.w) {
case 0 ... 1:
--
2.1.4
^ permalink raw reply related
* [PATCH v2 4/4] input: synaptics - make semi-mt touchpads report widths
From: Gabriele Mazzotta @ 2015-01-05 22:28 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, rydberg, linux-kernel, silverhammermba,
peter.hutterer, hdegoede, benjamin.tissoires, Gabriele Mazzotta
In-Reply-To: <1420496915-31560-1-git-send-email-gabriele.mzt@gmail.com>
Despite claiming to report finger widths, semi-mt touchpads were not
doing it. Make them report widths using ABS_MT_TOUCH_MAJOR instead
of ABS_TOOL_WIDTH.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
drivers/input/mouse/synaptics.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 5008e8c..7b95a0b 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -819,6 +819,7 @@ static void synaptics_report_mt_data(struct psmouse *psmouse,
input_report_abs(dev, ABS_MT_POSITION_X, pos[i].x);
input_report_abs(dev, ABS_MT_POSITION_Y, pos[i].y);
input_report_abs(dev, ABS_MT_PRESSURE, hw[i]->z);
+ input_report_abs(dev, ABS_MT_TOUCH_MAJOR, hw[i]->w);
}
input_mt_drop_unused(dev);
@@ -954,8 +955,14 @@ static void synaptics_process_packet(struct psmouse *psmouse)
}
input_report_abs(dev, ABS_PRESSURE, hw.z);
- if (SYN_CAP_PALMDETECT(priv->capabilities))
- input_report_abs(dev, ABS_TOOL_WIDTH, finger_width);
+ if (SYN_CAP_PALMDETECT(priv->capabilities)) {
+ if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c) ||
+ cr48_profile_sensor)
+ input_set_abs_params(dev,
+ ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
+ else
+ input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
+ }
input_report_key(dev, BTN_TOOL_FINGER, num_fingers == 1);
if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
--
2.1.4
^ permalink raw reply related
* [PATCH v2 2/4] input: synaptics - fix width values calculation on image sensors
From: Gabriele Mazzotta @ 2015-01-05 22:28 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, rydberg, linux-kernel, silverhammermba,
peter.hutterer, hdegoede, benjamin.tissoires, Gabriele Mazzotta
In-Reply-To: <1420496915-31560-1-git-send-email-gabriele.mzt@gmail.com>
When multiple fingers are on the touchpad, W reports the finger
count rather than the width. Retrieve the correct value that is
encoded in X, Y and Z of AGM and SGM packets.
The minimum width reported is 8 rather than 4 in this case, while the
maximum remains 15.
Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
drivers/input/mouse/synaptics.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 4d22ebd..918a727 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -585,10 +585,12 @@ static void synaptics_parse_agm(const unsigned char buf[],
switch (agm_packet_type) {
case 1:
/* Gesture packet: (x, y, z) half resolution */
- agm->w = hw->w;
- agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
- agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
- agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 2;
+ agm->w = ((buf[1] & 0x01) |
+ ((buf[2] & 0x01) << 1) |
+ ((buf[5] & 0x01) << 2)) + 8;
+ agm->x = (((buf[4] & 0x0f) << 8) | (buf[1] & 0xfe)) << 1;
+ agm->y = (((buf[4] & 0xf0) << 4) | (buf[2] & 0xfe)) << 1;
+ agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0e)) << 2;
break;
case 2:
@@ -852,6 +854,17 @@ static void synaptics_image_sensor_process(struct psmouse *psmouse,
else
num_fingers = 4;
+ /* When multiple fingers are on the TouchPad, the width is encoded in
+ * X, Y and Z */
+ if (sgm->w == 0 || sgm->w == 1) {
+ sgm->w = (((sgm->x & BIT(1)) >> 1) |
+ (sgm->y & BIT(1)) |
+ ((sgm->z & BIT(0)) << 2)) + 8;
+ sgm->x &= ~BIT(1);
+ sgm->y &= ~BIT(1);
+ sgm->z &= ~BIT(0);
+ }
+
/* Send resulting input events to user space */
synaptics_report_mt_data(psmouse, sgm, num_fingers);
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH 2/2] HID: wacom: add support of the Pen of the Bamboo Pad
From: Jason Gerecke @ 2015-01-06 1:49 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, josep.sanchez.ferreres, Ping Cheng, Linux Input,
linux-kernel
In-Reply-To: <1420493533-11639-2-git-send-email-benjamin.tissoires@redhat.com>
On Mon, Jan 5, 2015 at 1:32 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Bamboo Pads are using the generic processing but their report descriptors
> differ from the ISDv* line. The pen fields are marked with the .physical
> as Digitizer_Pen, which makes also sense.
>
> Add this field to the checks and enable for free Bamboo Pads.
>
> Reported-by: Josep Sanchez Ferreres <josep.sanchez.ferreres@est.fib.upc.edu>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>
> Hi Jason,
>
> I still did not managed to put in shape the regression tests for the Wacom
> recordings we have. I do not think this will impact the current supported
> devices (none that I have present the HID_DG_PEN application/logical/physical
> tag).
>
> It would be good still if you could have a look and validate the patch series.
>
> Cheers,
> Benjamin
>
None of the units I've collected descriptors for appear to have a
HD_DG_PEN physical collection, so I also don't see any potential for
regression. For both patches:
Reviewed-by: Jason Gerecke <killertofu@gmail.com>
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
> drivers/hid/wacom_wac.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 7436f2b..7afd929 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -74,6 +74,7 @@
>
> #define WACOM_PEN_FIELD(f) (((f)->logical == HID_DG_STYLUS) || \
> ((f)->physical == HID_DG_STYLUS) || \
> + ((f)->physical == HID_DG_PEN) || \
> ((f)->application == HID_DG_PEN))
> #define WACOM_FINGER_FIELD(f) (((f)->logical == HID_DG_FINGER) || \
> ((f)->physical == HID_DG_FINGER) || \
> --
> 2.1.0
>
^ permalink raw reply
* [PATCH 02/17] clk: sunxi: Propagate rate changes to parent for mux clocks
From: Chen-Yu Tsai @ 2015-01-06 2:35 UTC (permalink / raw)
To: Maxime Ripard, Dmitry Torokhov, Zhang Rui, Eduardo Valentin
Cc: Chen-Yu Tsai, Hans de Goede, linux-input, linux-arm-kernel,
linux-pm
In-Reply-To: <1420511727-8242-1-git-send-email-wens@csie.org>
The cpu clock on sunxi machines is just a mux clock, which is normally
fed by the main PLL, but can be muxed to the main or low power oscillator.
Make the mux clock propagate rate changes to its parent, so we can
change the clock rate of the PLL, and thus actually implement rate
changing on the cpu clock.
This patch also removes the no reparenting limit.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/clk/sunxi/clk-sunxi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index d8f17bbcb8e3..df18a2af33ad 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -778,7 +778,7 @@ static void __init sunxi_mux_clk_setup(struct device_node *node,
of_property_read_string(node, "clock-output-names", &clk_name);
clk = clk_register_mux(NULL, clk_name, parents, i,
- CLK_SET_RATE_NO_REPARENT, reg,
+ CLK_SET_RATE_PARENT, reg,
data->shift, SUNXI_MUX_GATE_WIDTH,
0, &clk_lock);
--
2.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox