* [PATCH 01/19] Input: Break out MT data
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 02/19] Input: Improve the events-per-packet estimate Henrik Rydberg
` (17 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Move all MT-related things to a separate place. This saves some
bytes for non-mt input devices, and prepares for new MT features.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/evdev.c | 10 ++++++----
drivers/input/input-mt.c | 47 +++++++++++++++++++++++++++--------------------
drivers/input/input.c | 9 ++++-----
include/linux/input.h | 9 ++-------
include/linux/input/mt.h | 16 ++++++++++++++--
5 files changed, 53 insertions(+), 38 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 6c58bff..a0692c5 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -653,20 +653,22 @@ static int evdev_handle_mt_request(struct input_dev *dev,
unsigned int size,
int __user *ip)
{
- const struct input_mt_slot *mt = dev->mt;
+ const struct input_mt *mt = dev->mt;
unsigned int code;
int max_slots;
int i;
if (get_user(code, &ip[0]))
return -EFAULT;
- if (!input_is_mt_value(code))
+ if (!mt || !input_is_mt_value(code))
return -EINVAL;
max_slots = (size - sizeof(__u32)) / sizeof(__s32);
- for (i = 0; i < dev->mtsize && i < max_slots; i++)
- if (put_user(input_mt_get_value(&mt[i], code), &ip[1 + i]))
+ for (i = 0; i < mt->num_slots && i < max_slots; i++) {
+ int value = input_mt_get_value(&mt->slots[i], code);
+ if (put_user(value, &ip[1 + i]))
return -EFAULT;
+ }
return 0;
}
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 70a16c7..c6df704 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -27,26 +27,28 @@
*/
int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots)
{
+ struct input_mt *mt = dev->mt;
int i;
if (!num_slots)
return 0;
- if (dev->mt)
- return dev->mtsize != num_slots ? -EINVAL : 0;
+ if (mt)
+ return mt->num_slots != num_slots ? -EINVAL : 0;
- dev->mt = kcalloc(num_slots, sizeof(struct input_mt_slot), GFP_KERNEL);
- if (!dev->mt)
+ mt = kzalloc(sizeof(*mt) + num_slots * sizeof(*mt->slots), GFP_KERNEL);
+ if (!mt)
return -ENOMEM;
- dev->mtsize = num_slots;
+ mt->num_slots = num_slots;
input_set_abs_params(dev, ABS_MT_SLOT, 0, num_slots - 1, 0, 0);
input_set_abs_params(dev, ABS_MT_TRACKING_ID, 0, TRKID_MAX, 0, 0);
input_set_events_per_packet(dev, 6 * num_slots);
/* Mark slots as 'unused' */
for (i = 0; i < num_slots; i++)
- input_mt_set_value(&dev->mt[i], ABS_MT_TRACKING_ID, -1);
+ input_mt_set_value(&mt->slots[i], ABS_MT_TRACKING_ID, -1);
+ dev->mt = mt;
return 0;
}
EXPORT_SYMBOL(input_mt_init_slots);
@@ -62,9 +64,7 @@ void input_mt_destroy_slots(struct input_dev *dev)
{
kfree(dev->mt);
dev->mt = NULL;
- dev->mtsize = 0;
dev->slot = 0;
- dev->trkid = 0;
}
EXPORT_SYMBOL(input_mt_destroy_slots);
@@ -83,18 +83,19 @@ EXPORT_SYMBOL(input_mt_destroy_slots);
void input_mt_report_slot_state(struct input_dev *dev,
unsigned int tool_type, bool active)
{
- struct input_mt_slot *mt;
+ struct input_mt *mt = dev->mt;
+ struct input_mt_slot *slot;
int id;
- if (!dev->mt || !active) {
+ if (!mt || !active) {
input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
return;
}
- mt = &dev->mt[dev->slot];
- id = input_mt_get_value(mt, ABS_MT_TRACKING_ID);
- if (id < 0 || input_mt_get_value(mt, ABS_MT_TOOL_TYPE) != tool_type)
- id = input_mt_new_trkid(dev);
+ slot = &mt->slots[dev->slot];
+ id = input_mt_get_value(slot, ABS_MT_TRACKING_ID);
+ if (id < 0 || input_mt_get_value(slot, ABS_MT_TOOL_TYPE) != tool_type)
+ id = input_mt_new_trkid(mt);
input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, id);
input_event(dev, EV_ABS, ABS_MT_TOOL_TYPE, tool_type);
@@ -135,13 +136,19 @@ EXPORT_SYMBOL(input_mt_report_finger_count);
*/
void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
{
- struct input_mt_slot *oldest = NULL;
- int oldid = dev->trkid;
- int count = 0;
- int i;
+ struct input_mt *mt = dev->mt;
+ struct input_mt_slot *oldest;
+ int oldid, count, i;
+
+ if (!mt)
+ return;
+
+ oldest = 0;
+ oldid = mt->trkid;
+ count = 0;
- for (i = 0; i < dev->mtsize; ++i) {
- struct input_mt_slot *ps = &dev->mt[i];
+ for (i = 0; i < mt->num_slots; ++i) {
+ struct input_mt_slot *ps = &mt->slots[i];
int id = input_mt_get_value(ps, ABS_MT_TRACKING_ID);
if (id < 0)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 8921c61..6e90705 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -174,7 +174,7 @@ static int input_handle_abs_event(struct input_dev *dev,
* "Stage" the event; we'll flush it later, when we
* get actual touch data.
*/
- if (*pval >= 0 && *pval < dev->mtsize)
+ if (dev->mt && *pval >= 0 && *pval < dev->mt->num_slots)
dev->slot = *pval;
return INPUT_IGNORE_EVENT;
@@ -185,8 +185,7 @@ static int input_handle_abs_event(struct input_dev *dev,
if (!is_mt_event) {
pold = &dev->absinfo[code].value;
} else if (dev->mt) {
- struct input_mt_slot *mtslot = &dev->mt[dev->slot];
- pold = &mtslot->abs[code - ABS_MT_FIRST];
+ pold = &dev->mt->slots[dev->slot].abs[code - ABS_MT_FIRST];
} else {
/*
* Bypass filtering for multi-touch events when
@@ -1751,8 +1750,8 @@ static unsigned int input_estimate_events_per_packet(struct input_dev *dev)
int i;
unsigned int events;
- if (dev->mtsize) {
- mt_slots = dev->mtsize;
+ if (dev->mt) {
+ mt_slots = dev->mt->num_slots;
} else if (test_bit(ABS_MT_TRACKING_ID, dev->absbit)) {
mt_slots = dev->absinfo[ABS_MT_TRACKING_ID].maximum -
dev->absinfo[ABS_MT_TRACKING_ID].minimum + 1,
diff --git a/include/linux/input.h b/include/linux/input.h
index 725dcd0..76d6788 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -1203,11 +1203,8 @@ struct ff_effect {
* software autorepeat
* @timer: timer for software autorepeat
* @rep: current values for autorepeat parameters (delay, rate)
- * @mt: pointer to array of struct input_mt_slot holding current values
- * of tracked contacts
- * @mtsize: number of MT slots the device uses
+ * @mt: pointer to multitouch state
* @slot: MT slot currently being transmitted
- * @trkid: stores MT tracking ID for the current contact
* @absinfo: array of &struct input_absinfo elements holding information
* about absolute axes (current value, min, max, flat, fuzz,
* resolution)
@@ -1287,10 +1284,8 @@ struct input_dev {
int rep[REP_CNT];
- struct input_mt_slot *mt;
- int mtsize;
+ struct input_mt *mt;
int slot;
- int trkid;
struct input_absinfo *absinfo;
diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
index f867375..4ae275c 100644
--- a/include/linux/input/mt.h
+++ b/include/linux/input/mt.h
@@ -23,6 +23,18 @@ struct input_mt_slot {
int abs[ABS_MT_LAST - ABS_MT_FIRST + 1];
};
+/**
+ * struct input_mt - state of tracked contacts
+ * @trkid: stores MT tracking ID for the next contact
+ * @num_slots: number of MT slots the device uses
+ * @slots: array of slots holding current values of tracked contacts
+ */
+struct input_mt {
+ int trkid;
+ int num_slots;
+ struct input_mt_slot slots[];
+};
+
static inline void input_mt_set_value(struct input_mt_slot *slot,
unsigned code, int value)
{
@@ -38,9 +50,9 @@ static inline int input_mt_get_value(const struct input_mt_slot *slot,
int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots);
void input_mt_destroy_slots(struct input_dev *dev);
-static inline int input_mt_new_trkid(struct input_dev *dev)
+static inline int input_mt_new_trkid(struct input_mt *mt)
{
- return dev->trkid++ & TRKID_MAX;
+ return mt->trkid++ & TRKID_MAX;
}
static inline void input_mt_slot(struct input_dev *dev, int slot)
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 02/19] Input: Improve the events-per-packet estimate
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
2012-08-12 21:42 ` [PATCH 01/19] Input: Break out MT data Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-14 19:32 ` Ping Cheng
2012-08-12 21:42 ` [PATCH 03/19] Input: Remove redundant packet estimates Henrik Rydberg
` (16 subsequent siblings)
18 siblings, 1 reply; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Many MT devices send a number of keys along with the mt information.
This patch makes sure that there is room for them in the packet
buffer.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/input.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 6e90705..8ebf116 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1777,6 +1777,9 @@ static unsigned int input_estimate_events_per_packet(struct input_dev *dev)
if (test_bit(i, dev->relbit))
events++;
+ /* Make room for KEY and MSC events */
+ events += 7;
+
return events;
}
@@ -1815,6 +1818,7 @@ int input_register_device(struct input_dev *dev)
{
static atomic_t input_no = ATOMIC_INIT(0);
struct input_handler *handler;
+ unsigned int packet_size;
const char *path;
int error;
@@ -1827,9 +1831,9 @@ int input_register_device(struct input_dev *dev)
/* Make sure that bitmasks not mentioned in dev->evbit are clean. */
input_cleanse_bitmasks(dev);
- if (!dev->hint_events_per_packet)
- dev->hint_events_per_packet =
- input_estimate_events_per_packet(dev);
+ packet_size = input_estimate_events_per_packet(dev);
+ if (dev->hint_events_per_packet < packet_size)
+ dev->hint_events_per_packet = packet_size;
/*
* If delay and period are pre-set by the driver, then autorepeating
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
2012-08-12 21:42 ` [PATCH 02/19] Input: Improve the events-per-packet estimate Henrik Rydberg
@ 2012-08-14 19:32 ` Ping Cheng
2012-08-14 19:53 ` Dmitry Torokhov
2012-08-14 20:01 ` Henrik Rydberg
0 siblings, 2 replies; 45+ messages in thread
From: Ping Cheng @ 2012-08-14 19:32 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Many MT devices send a number of keys along with the mt information.
> This patch makes sure that there is room for them in the packet
> buffer.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> drivers/input/input.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 6e90705..8ebf116 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1777,6 +1777,9 @@ static unsigned int input_estimate_events_per_packet(struct input_dev *dev)
> if (test_bit(i, dev->relbit))
> events++;
>
> + /* Make room for KEY and MSC events */
> + events += 7;
Hi Henrik,
It is nice to get rid of the redundant pieces and to incorporate
common functions. Thank you.
I have a question about the code above though. Why do we use 7
instead of going through the keys like:
for (i = 0; i < KEY_MAX; i++)
if (test_bit(i, dev->keybit))
events++;
Ping
> +
> return events;
> }
>
> @@ -1815,6 +1818,7 @@ int input_register_device(struct input_dev *dev)
> {
> static atomic_t input_no = ATOMIC_INIT(0);
> struct input_handler *handler;
> + unsigned int packet_size;
> const char *path;
> int error;
>
> @@ -1827,9 +1831,9 @@ int input_register_device(struct input_dev *dev)
> /* Make sure that bitmasks not mentioned in dev->evbit are clean. */
> input_cleanse_bitmasks(dev);
>
> - if (!dev->hint_events_per_packet)
> - dev->hint_events_per_packet =
> - input_estimate_events_per_packet(dev);
> + packet_size = input_estimate_events_per_packet(dev);
> + if (dev->hint_events_per_packet < packet_size)
> + dev->hint_events_per_packet = packet_size;
>
> /*
> * If delay and period are pre-set by the driver, then autorepeating
> --
> 1.7.11.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 [flat|nested] 45+ messages in thread* Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
2012-08-14 19:32 ` Ping Cheng
@ 2012-08-14 19:53 ` Dmitry Torokhov
2012-08-14 20:50 ` Ping Cheng
2012-08-14 20:01 ` Henrik Rydberg
1 sibling, 1 reply; 45+ messages in thread
From: Dmitry Torokhov @ 2012-08-14 19:53 UTC (permalink / raw)
To: Ping Cheng; +Cc: Henrik Rydberg, Jiri Kosina, linux-input, linux-kernel
On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote:
> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> > Many MT devices send a number of keys along with the mt information.
> > This patch makes sure that there is room for them in the packet
> > buffer.
> >
> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> > ---
> >
> > drivers/input/input.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > index 6e90705..8ebf116 100644
> > --- a/drivers/input/input.c
> > +++ b/drivers/input/input.c
> > @@ -1777,6 +1777,9 @@ static unsigned int
> > input_estimate_events_per_packet(struct input_dev *dev)>
> > if (test_bit(i, dev->relbit))
> >
> > events++;
> >
> > + /* Make room for KEY and MSC events */
> > + events += 7;
>
> Hi Henrik,
>
> It is nice to get rid of the redundant pieces and to incorporate
> common functions. Thank you.
>
> I have a question about the code above though. Why do we use 7
> instead of going through the keys like:
>
> for (i = 0; i < KEY_MAX; i++)
> if (test_bit(i, dev->keybit))
> events++;
Because that would result in gross over-estimation for many devices -
my keyboard has 100+ keys but it never sends all of them in one event
frame, not even if I can get a cat to lay on it ;)
--
Dmitry
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
2012-08-14 19:53 ` Dmitry Torokhov
@ 2012-08-14 20:50 ` Ping Cheng
2012-08-14 21:12 ` Dmitry Torokhov
0 siblings, 1 reply; 45+ messages in thread
From: Ping Cheng @ 2012-08-14 20:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, Jiri Kosina, linux-input, linux-kernel
On Tue, Aug 14, 2012 at 12:53 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote:
>> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> > Many MT devices send a number of keys along with the mt information.
>> > This patch makes sure that there is room for them in the packet
>> > buffer.
>> >
>> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
>> > ---
>> >
>> > drivers/input/input.c | 10 +++++++---
>> > 1 file changed, 7 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/input/input.c b/drivers/input/input.c
>> > index 6e90705..8ebf116 100644
>> > --- a/drivers/input/input.c
>> > +++ b/drivers/input/input.c
>> > @@ -1777,6 +1777,9 @@ static unsigned int
>> > input_estimate_events_per_packet(struct input_dev *dev)>
>> > if (test_bit(i, dev->relbit))
>> >
>> > events++;
>> >
>> > + /* Make room for KEY and MSC events */
>> > + events += 7;
>>
>> Hi Henrik,
>>
>> It is nice to get rid of the redundant pieces and to incorporate
>> common functions. Thank you.
>>
>> I have a question about the code above though. Why do we use 7
>> instead of going through the keys like:
>>
>> for (i = 0; i < KEY_MAX; i++)
>> if (test_bit(i, dev->keybit))
>> events++;
>
> Because that would result in gross over-estimation for many devices -
> my keyboard has 100+ keys but it never sends all of them in one event
> frame, not even if I can get a cat to lay on it ;)
Thanks for the prompt reply. I thought you were on vacation ;-).
So, what device are we talking about here? I thought it is a touch
device with a few extra buttons, which are reported as key events. Am
I missing something?
If it is a touch device, we won't have too many buttons. So,
test_bit(i, dev->keybit) won't be true for more than the number of
buttons that declared by __set_bit().
I would think we could play a keyboard (this keyboard does not have
letters on it ;-) with ten fingers.
Ping
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
2012-08-14 20:50 ` Ping Cheng
@ 2012-08-14 21:12 ` Dmitry Torokhov
2012-08-15 0:54 ` Ping Cheng
0 siblings, 1 reply; 45+ messages in thread
From: Dmitry Torokhov @ 2012-08-14 21:12 UTC (permalink / raw)
To: Ping Cheng; +Cc: Henrik Rydberg, Jiri Kosina, linux-input, linux-kernel
On Tue, Aug 14, 2012 at 01:50:38PM -0700, Ping Cheng wrote:
> On Tue, Aug 14, 2012 at 12:53 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote:
> >> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> >> > Many MT devices send a number of keys along with the mt information.
> >> > This patch makes sure that there is room for them in the packet
> >> > buffer.
> >> >
> >> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> >> > ---
> >> >
> >> > drivers/input/input.c | 10 +++++++---
> >> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/input/input.c b/drivers/input/input.c
> >> > index 6e90705..8ebf116 100644
> >> > --- a/drivers/input/input.c
> >> > +++ b/drivers/input/input.c
> >> > @@ -1777,6 +1777,9 @@ static unsigned int
> >> > input_estimate_events_per_packet(struct input_dev *dev)>
> >> > if (test_bit(i, dev->relbit))
> >> >
> >> > events++;
> >> >
> >> > + /* Make room for KEY and MSC events */
> >> > + events += 7;
> >>
> >> Hi Henrik,
> >>
> >> It is nice to get rid of the redundant pieces and to incorporate
> >> common functions. Thank you.
> >>
> >> I have a question about the code above though. Why do we use 7
> >> instead of going through the keys like:
> >>
> >> for (i = 0; i < KEY_MAX; i++)
> >> if (test_bit(i, dev->keybit))
> >> events++;
> >
> > Because that would result in gross over-estimation for many devices -
> > my keyboard has 100+ keys but it never sends all of them in one event
> > frame, not even if I can get a cat to lay on it ;)
>
> Thanks for the prompt reply. I thought you were on vacation ;-).
No, just generally busy ;(
>
> So, what device are we talking about here? I thought it is a touch
> device with a few extra buttons, which are reported as key events. Am
> I missing something?
I was talking about a bog-standard computer keyboard here.
>
> If it is a touch device, we won't have too many buttons. So,
> test_bit(i, dev->keybit) won't be true for more than the number of
> buttons that declared by __set_bit().
input_estimate_events_per_packet() is a generic routine that is used for
all devices, not only [multi]touch.
>
> I would think we could play a keyboard (this keyboard does not have
> letters on it ;-) with ten fingers.
But even that keyboard would have more than 10 keys, right? So even
though max_events should be 10 + 10 + 1 (10 keys, 10 msc, syn) your loop
would produce what 88 + 88 + 1 for full size music keyboard?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
2012-08-14 21:12 ` Dmitry Torokhov
@ 2012-08-15 0:54 ` Ping Cheng
0 siblings, 0 replies; 45+ messages in thread
From: Ping Cheng @ 2012-08-15 0:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, Jiri Kosina, linux-input, linux-kernel
On Tue, Aug 14, 2012 at 2:12 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> >> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> >> > Many MT devices send a number of keys along with the mt information.
> >> > This patch makes sure that there is room for them in the packet
> >> > buffer.
>>
>> So, what device are we talking about here? I thought it is a touch
>> device with a few extra buttons, which are reported as key events. Am
>> I missing something?
>
> I was talking about a bog-standard computer keyboard here.
>
>>
>> If it is a touch device, we won't have too many buttons. So,
>> test_bit(i, dev->keybit) won't be true for more than the number of
>> buttons that declared by __set_bit().
>
> input_estimate_events_per_packet() is a generic routine that is used for
> all devices, not only [multi]touch.
I understand you are talking about standard keyboard. And I know this
routine is for all devices.
However, from the commit comments, the patch is to address an MT
issue. If it is not just for MT, we need either to make it clear in
the comments or to verify the type of the device in the code.
>> I would think we could play a keyboard (this keyboard does not have
>> letters on it ;-) with ten fingers.
>
> But even that keyboard would have more than 10 keys, right? So even
> though max_events should be 10 + 10 + 1 (10 keys, 10 msc, syn) your loop
> would produce what 88 + 88 + 1 for full size music keyboard?
No, I was not talking about implementing full music keyboard functions
in the kernel. My point was: why do we take 7 instead of 10, or
another number?
In fact, 7 works for me as long as we explain the rationale behind the
decision. I do not have a device that needs to post 10 button events
simultaneously, yet ;-).
Ping
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
2012-08-14 19:32 ` Ping Cheng
2012-08-14 19:53 ` Dmitry Torokhov
@ 2012-08-14 20:01 ` Henrik Rydberg
2012-08-14 21:06 ` Ping Cheng
1 sibling, 1 reply; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-14 20:01 UTC (permalink / raw)
To: Ping Cheng; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
Hi Ping,
Long time no see. :-)
> > + /* Make room for KEY and MSC events */
> > + events += 7;
>
> It is nice to get rid of the redundant pieces and to incorporate
> common functions. Thank you.
>
> I have a question about the code above though. Why do we use 7
> instead of going through the keys like:
>
> for (i = 0; i < KEY_MAX; i++)
> if (test_bit(i, dev->keybit))
> events++;
Keyboards register a large amount of different keys, but seldom send
more than one or two at a time. The value 7 is ad hoc, admittedly, but
it makes the default buffer 8 bytes, which happens to precisely match
the default buffer in evdev.
Thanks,
Henrik
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
2012-08-14 20:01 ` Henrik Rydberg
@ 2012-08-14 21:06 ` Ping Cheng
0 siblings, 0 replies; 45+ messages in thread
From: Ping Cheng @ 2012-08-14 21:06 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
On Tue, Aug 14, 2012 at 1:01 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Ping,
>
> Long time no see. :-)
>
>> > + /* Make room for KEY and MSC events */
>> > + events += 7;
>>
>> It is nice to get rid of the redundant pieces and to incorporate
>> common functions. Thank you.
>>
>> I have a question about the code above though. Why do we use 7
>> instead of going through the keys like:
>>
>> for (i = 0; i < KEY_MAX; i++)
>> if (test_bit(i, dev->keybit))
>> events++;
>
> Keyboards register a large amount of different keys, but seldom send
> more than one or two at a time. The value 7 is ad hoc, admittedly, but
> it makes the default buffer 8 bytes, which happens to precisely match
> the default buffer in evdev.
That can be a valid reason until we need to report more keys
simultaneously. Please update the comments so we know why we end up
with 7.
Thank you.
Ping
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 03/19] Input: Remove redundant packet estimates
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
2012-08-12 21:42 ` [PATCH 01/19] Input: Break out MT data Henrik Rydberg
2012-08-12 21:42 ` [PATCH 02/19] Input: Improve the events-per-packet estimate Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 04/19] Input: Make sure we follow all EV_KEY events Henrik Rydberg
` (15 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Packet estimates are now better handled in the input core. Remove the
redundant estimates from those drivers.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hid/hid-input.c | 4 ----
drivers/hid/hid-magicmouse.c | 2 --
drivers/input/input-mt.c | 1 -
drivers/input/mouse/bcm5974.c | 2 --
4 files changed, 9 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 811bfad..ad5cbcf 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -911,10 +911,6 @@ mapped:
input_abs_set_res(input, usage->code,
hidinput_calc_abs_res(field, usage->code));
-
- /* use a larger default input buffer for MT devices */
- if (usage->code == ABS_MT_POSITION_X && input->hint_events_per_packet == 0)
- input_set_events_per_packet(input, 60);
}
if (usage->type == EV_ABS &&
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 7364726..e425de4 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -435,8 +435,6 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd
TRACKPAD_RES_Y);
}
- input_set_events_per_packet(input, 60);
-
if (report_undeciphered) {
__set_bit(EV_MSC, input->evbit);
__set_bit(MSC_RAW, input->mscbit);
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index c6df704..58bde77 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -42,7 +42,6 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots)
mt->num_slots = num_slots;
input_set_abs_params(dev, ABS_MT_SLOT, 0, num_slots - 1, 0, 0);
input_set_abs_params(dev, ABS_MT_TRACKING_ID, 0, TRKID_MAX, 0, 0);
- input_set_events_per_packet(dev, 6 * num_slots);
/* Mark slots as 'unused' */
for (i = 0; i < num_slots; i++)
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index d528c23..8234098 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -456,8 +456,6 @@ static void setup_events_to_report(struct input_dev *input_dev,
__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
if (cfg->caps & HAS_INTEGRATED_BUTTON)
__set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
-
- input_set_events_per_packet(input_dev, 60);
}
/* report button data as logical button state */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 04/19] Input: Make sure we follow all EV_KEY events
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (2 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 03/19] Input: Remove redundant packet estimates Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 05/19] Input: Move autorepeat to the event-passing phase Henrik Rydberg
` (14 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
For some EV_KEY types, sending a larger-than-one value causes the
input state to oscillate. This patch makes sure this cannot happen,
clearing up the autorepeat bypass logic in the process.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/input.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 8ebf116..4d64500 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -239,24 +239,30 @@ static void input_handle_event(struct input_dev *dev,
break;
case EV_KEY:
- if (is_event_supported(code, dev->keybit, KEY_MAX) &&
- !!test_bit(code, dev->key) != value) {
+ if (is_event_supported(code, dev->keybit, KEY_MAX)) {
+
+ /* auto-repeat bypasses state updates */
+ if (value == 2) {
+ disposition = INPUT_PASS_TO_HANDLERS;
+ break;
+ }
+
+ if (!!test_bit(code, dev->key) != !!value) {
- if (value != 2) {
__change_bit(code, dev->key);
+ disposition = INPUT_PASS_TO_HANDLERS;
+
if (value)
input_start_autorepeat(dev, code);
else
input_stop_autorepeat(dev);
}
-
- disposition = INPUT_PASS_TO_HANDLERS;
}
break;
case EV_SW:
if (is_event_supported(code, dev->swbit, SW_MAX) &&
- !!test_bit(code, dev->sw) != value) {
+ !!test_bit(code, dev->sw) != !!value) {
__change_bit(code, dev->sw);
disposition = INPUT_PASS_TO_HANDLERS;
@@ -283,7 +289,7 @@ static void input_handle_event(struct input_dev *dev,
case EV_LED:
if (is_event_supported(code, dev->ledbit, LED_MAX) &&
- !!test_bit(code, dev->led) != value) {
+ !!test_bit(code, dev->led) != !!value) {
__change_bit(code, dev->led);
disposition = INPUT_PASS_TO_ALL;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 05/19] Input: Move autorepeat to the event-passing phase
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (3 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 04/19] Input: Make sure we follow all EV_KEY events Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 06/19] Input: Send events one packet at a time Henrik Rydberg
` (13 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Preparing to split event filtering and event passing, move the
autorepeat function to the point where the event is actually passed.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/input.c | 46 +++++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 21 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 4d64500..a57c4a5 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -69,6 +69,22 @@ static int input_defuzz_abs_event(int value, int old_val, int fuzz)
return value;
}
+static void input_start_autorepeat(struct input_dev *dev, int code)
+{
+ if (test_bit(EV_REP, dev->evbit) &&
+ dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
+ dev->timer.data) {
+ dev->repeat_key = code;
+ mod_timer(&dev->timer,
+ jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
+ }
+}
+
+static void input_stop_autorepeat(struct input_dev *dev)
+{
+ del_timer(&dev->timer);
+}
+
/*
* Pass event first through all filters and then, if event has not been
* filtered out, through all open handles. This function is called with
@@ -105,6 +121,15 @@ static void input_pass_event(struct input_dev *dev,
}
rcu_read_unlock();
+
+ /* trigger auto repeat for key events */
+ if (type == EV_KEY && value != 2) {
+ if (value)
+ input_start_autorepeat(dev, code);
+ else
+ input_stop_autorepeat(dev);
+ }
+
}
/*
@@ -142,22 +167,6 @@ static void input_repeat_key(unsigned long data)
spin_unlock_irqrestore(&dev->event_lock, flags);
}
-static void input_start_autorepeat(struct input_dev *dev, int code)
-{
- if (test_bit(EV_REP, dev->evbit) &&
- dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
- dev->timer.data) {
- dev->repeat_key = code;
- mod_timer(&dev->timer,
- jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
- }
-}
-
-static void input_stop_autorepeat(struct input_dev *dev)
-{
- del_timer(&dev->timer);
-}
-
#define INPUT_IGNORE_EVENT 0
#define INPUT_PASS_TO_HANDLERS 1
#define INPUT_PASS_TO_DEVICE 2
@@ -251,11 +260,6 @@ static void input_handle_event(struct input_dev *dev,
__change_bit(code, dev->key);
disposition = INPUT_PASS_TO_HANDLERS;
-
- if (value)
- input_start_autorepeat(dev, code);
- else
- input_stop_autorepeat(dev);
}
}
break;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 06/19] Input: Send events one packet at a time
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (4 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 05/19] Input: Move autorepeat to the event-passing phase Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-24 4:03 ` Daniel Kurtz
2012-08-12 21:42 ` [PATCH 07/19] Input: evdev - Add the events() callback Henrik Rydberg
` (12 subsequent siblings)
18 siblings, 1 reply; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
On heavy event loads, such as a multitouch driver, the irqsoff latency
can be as high as 250 us. By accumulating a frame worth of data
before passing it on, the latency can be dramatically reduced. As a
side effect, the special EV_SYN handling can be removed, since the
frame is now atomic.
This patch adds the events() handler callback and uses it if it
exists. The latency is improved by 50 us even without the callback.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/input-mt.c | 3 +-
drivers/input/input.c | 187 ++++++++++++++++++++++++++++-------------------
include/linux/input.h | 26 +++++--
3 files changed, 132 insertions(+), 84 deletions(-)
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 58bde77..f956b27 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -63,7 +63,6 @@ void input_mt_destroy_slots(struct input_dev *dev)
{
kfree(dev->mt);
dev->mt = NULL;
- dev->slot = 0;
}
EXPORT_SYMBOL(input_mt_destroy_slots);
@@ -91,7 +90,7 @@ void input_mt_report_slot_state(struct input_dev *dev,
return;
}
- slot = &mt->slots[dev->slot];
+ slot = &mt->slots[input_abs_get_val(dev, ABS_MT_SLOT)];
id = input_mt_get_value(slot, ABS_MT_TRACKING_ID);
if (id < 0 || input_mt_get_value(slot, ABS_MT_TOOL_TYPE) != tool_type)
id = input_mt_new_trkid(mt);
diff --git a/drivers/input/input.c b/drivers/input/input.c
index a57c4a5..9b6aa15 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -90,46 +90,81 @@ static void input_stop_autorepeat(struct input_dev *dev)
* filtered out, through all open handles. This function is called with
* dev->event_lock held and interrupts disabled.
*/
-static void input_pass_event(struct input_dev *dev,
- unsigned int type, unsigned int code, int value)
+static size_t input_to_handler(struct input_handle *handle,
+ struct input_value *vals, size_t count)
{
- struct input_handler *handler;
- struct input_handle *handle;
+ struct input_handler *handler = handle->handler;
+ struct input_value *end = vals;
+ struct input_value *v;
- rcu_read_lock();
+ 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++;
+ }
- handle = rcu_dereference(dev->grab);
- if (handle)
- handle->handler->event(handle, type, code, value);
- else {
- bool filtered = false;
+ count = end - vals;
+ if (!count)
+ return 0;
- list_for_each_entry_rcu(handle, &dev->h_list, d_node) {
- if (!handle->open)
- continue;
+ if (handler->events)
+ handler->events(handle, vals, count);
+ else
+ for (v = vals; v != end; v++)
+ handler->event(handle, v->type, v->code, v->value);
+
+ return count;
+}
+
+/*
+ * Pass values first through all filters and then, if event has not been
+ * filtered out, through all open handles. This function is called with
+ * dev->event_lock held and interrupts disabled.
+ */
+static void input_pass_values(struct input_dev *dev,
+ struct input_value *vals, size_t count)
+{
+ struct input_handle *handle;
+ struct input_value *v;
- handler = handle->handler;
- if (!handler->filter) {
- if (filtered)
- break;
+ if (!count)
+ return;
- handler->event(handle, type, code, value);
+ rcu_read_lock();
- } else if (handler->filter(handle, type, code, value))
- filtered = true;
- }
+ handle = rcu_dereference(dev->grab);
+ if (handle) {
+ count = input_to_handler(handle, vals, count);
+ } else {
+ list_for_each_entry_rcu(handle, &dev->h_list, d_node)
+ if (handle->open)
+ count = input_to_handler(handle, vals, count);
}
rcu_read_unlock();
+ add_input_randomness(vals->type, vals->code, vals->value);
+
/* trigger auto repeat for key events */
- if (type == EV_KEY && value != 2) {
- if (value)
- input_start_autorepeat(dev, code);
- else
- input_stop_autorepeat(dev);
+ 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);
+ }
}
+}
+
+static void input_pass_event(struct input_dev *dev,
+ unsigned int type, unsigned int code, int value)
+{
+ struct input_value vals[] = { { type, code, value } };
+ input_pass_values(dev, vals, 1);
}
/*
@@ -146,18 +181,12 @@ static void input_repeat_key(unsigned long data)
if (test_bit(dev->repeat_key, dev->key) &&
is_event_supported(dev->repeat_key, dev->keybit, KEY_MAX)) {
+ struct input_value vals[] = {
+ { EV_KEY, dev->repeat_key, 2 },
+ { EV_SYN, SYN_REPORT, 1 },
+ };
- input_pass_event(dev, EV_KEY, dev->repeat_key, 2);
-
- if (dev->sync) {
- /*
- * Only send SYN_REPORT if we are not in a middle
- * of driver parsing a new hardware packet.
- * Otherwise assume that the driver will send
- * SYN_REPORT once it's done.
- */
- input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
- }
+ input_pass_values(dev, vals, 2);
if (dev->rep[REP_PERIOD])
mod_timer(&dev->timer, jiffies +
@@ -170,37 +199,23 @@ static void input_repeat_key(unsigned long data)
#define INPUT_IGNORE_EVENT 0
#define INPUT_PASS_TO_HANDLERS 1
#define INPUT_PASS_TO_DEVICE 2
+#define INPUT_FLUSH 4
#define INPUT_PASS_TO_ALL (INPUT_PASS_TO_HANDLERS | INPUT_PASS_TO_DEVICE)
static int input_handle_abs_event(struct input_dev *dev,
unsigned int code, int *pval)
{
bool is_mt_event;
- int *pold;
-
- if (code == ABS_MT_SLOT) {
- /*
- * "Stage" the event; we'll flush it later, when we
- * get actual touch data.
- */
- if (dev->mt && *pval >= 0 && *pval < dev->mt->num_slots)
- dev->slot = *pval;
-
- return INPUT_IGNORE_EVENT;
- }
+ int *pold = NULL;
is_mt_event = input_is_mt_value(code);
if (!is_mt_event) {
pold = &dev->absinfo[code].value;
} else if (dev->mt) {
- pold = &dev->mt->slots[dev->slot].abs[code - ABS_MT_FIRST];
- } else {
- /*
- * Bypass filtering for multi-touch events when
- * not employing slots.
- */
- pold = NULL;
+ int slot = dev->absinfo[ABS_MT_SLOT].value;
+ if (slot >= 0 && slot < dev->mt->num_slots)
+ pold = &dev->mt->slots[slot].abs[code - ABS_MT_FIRST];
}
if (pold) {
@@ -212,17 +227,11 @@ static int input_handle_abs_event(struct input_dev *dev,
*pold = *pval;
}
- /* Flush pending "slot" event */
- if (is_mt_event && dev->slot != input_abs_get_val(dev, ABS_MT_SLOT)) {
- input_abs_set_val(dev, ABS_MT_SLOT, dev->slot);
- input_pass_event(dev, EV_ABS, ABS_MT_SLOT, dev->slot);
- }
-
return INPUT_PASS_TO_HANDLERS;
}
-static void input_handle_event(struct input_dev *dev,
- unsigned int type, unsigned int code, int value)
+static int input_get_disposition(struct input_dev *dev,
+ unsigned int type, unsigned int code, int value)
{
int disposition = INPUT_IGNORE_EVENT;
@@ -235,13 +244,9 @@ static void input_handle_event(struct input_dev *dev,
break;
case SYN_REPORT:
- if (!dev->sync) {
- dev->sync = true;
- disposition = INPUT_PASS_TO_HANDLERS;
- }
+ disposition = INPUT_PASS_TO_HANDLERS | INPUT_FLUSH;
break;
case SYN_MT_REPORT:
- dev->sync = false;
disposition = INPUT_PASS_TO_HANDLERS;
break;
}
@@ -326,14 +331,35 @@ static void input_handle_event(struct input_dev *dev,
break;
}
- if (disposition != INPUT_IGNORE_EVENT && type != EV_SYN)
- dev->sync = false;
+ return disposition;
+}
+
+static void input_handle_event(struct input_dev *dev,
+ unsigned int type, unsigned int code, int value)
+{
+ struct input_value *v;
+ int disp;
+
+ disp = input_get_disposition(dev, type, code, value);
- if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event)
+ if ((disp & INPUT_PASS_TO_DEVICE) && dev->event)
dev->event(dev, type, code, value);
- if (disposition & INPUT_PASS_TO_HANDLERS)
- input_pass_event(dev, type, code, value);
+ if (!dev->vals)
+ return;
+
+ if (disp & INPUT_PASS_TO_HANDLERS) {
+ v = &dev->vals[dev->num_vals++];
+ v->type = type;
+ v->code = code;
+ v->value = value;
+ }
+
+ if ((disp & INPUT_FLUSH) || (dev->num_vals >= dev->max_vals)) {
+ if (dev->num_vals >= 2)
+ input_pass_values(dev, dev->vals, dev->num_vals);
+ dev->num_vals = 0;
+ }
}
/**
@@ -361,7 +387,6 @@ void input_event(struct input_dev *dev,
if (is_event_supported(type, dev->evbit, EV_MAX)) {
spin_lock_irqsave(&dev->event_lock, flags);
- add_input_randomness(type, code, value);
input_handle_event(dev, type, code, value);
spin_unlock_irqrestore(&dev->event_lock, flags);
}
@@ -842,8 +867,7 @@ int input_set_keycode(struct input_dev *dev,
__test_and_clear_bit(old_keycode, dev->key)) {
input_pass_event(dev, EV_KEY, old_keycode, 0);
- if (dev->sync)
- input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
+ input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
}
out:
@@ -1425,6 +1449,7 @@ static void input_dev_release(struct device *device)
input_ff_destroy(dev);
input_mt_destroy_slots(dev);
kfree(dev->absinfo);
+ kfree(dev->vals);
kfree(dev);
module_put(THIS_MODULE);
@@ -1845,6 +1870,14 @@ int input_register_device(struct input_dev *dev)
if (dev->hint_events_per_packet < packet_size)
dev->hint_events_per_packet = packet_size;
+ dev->num_vals = 0;
+ dev->max_vals = max(dev->hint_events_per_packet, packet_size);
+
+ kfree(dev->vals);
+ dev->vals = kcalloc(dev->max_vals, sizeof(*dev->vals), GFP_KERNEL);
+ if (!dev->vals)
+ return -ENOMEM;
+
/*
* If delay and period are pre-set by the driver, then autorepeating
* is handled by the driver itself and we don't do it in input.c.
diff --git a/include/linux/input.h b/include/linux/input.h
index 76d6788..1f7f172 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -1169,6 +1169,18 @@ struct ff_effect {
#include <linux/mod_devicetable.h>
/**
+ * struct input_value - input value representation
+ * @type: type of value (EV_KEY, EV_ABS, etc)
+ * @code: the value code
+ * @value: the value
+ */
+struct input_value {
+ __u16 type;
+ __u16 code;
+ __s32 value;
+};
+
+/**
* struct input_dev - represents an input device
* @name: name of the device
* @phys: physical path to the device in the system hierarchy
@@ -1204,7 +1216,6 @@ struct ff_effect {
* @timer: timer for software autorepeat
* @rep: current values for autorepeat parameters (delay, rate)
* @mt: pointer to multitouch state
- * @slot: MT slot currently being transmitted
* @absinfo: array of &struct input_absinfo elements holding information
* about absolute axes (current value, min, max, flat, fuzz,
* resolution)
@@ -1241,7 +1252,6 @@ struct ff_effect {
* last user closes the device
* @going_away: marks devices that are in a middle of unregistering and
* causes input_open_device*() fail with -ENODEV.
- * @sync: set to %true when there were no new events since last EV_SYN
* @dev: driver model's view of this device
* @h_list: list of input handles associated with the device. When
* accessing the list dev->mutex must be held
@@ -1285,7 +1295,6 @@ struct input_dev {
int rep[REP_CNT];
struct input_mt *mt;
- int slot;
struct input_absinfo *absinfo;
@@ -1307,12 +1316,14 @@ struct input_dev {
unsigned int users;
bool going_away;
- bool sync;
-
struct device dev;
struct list_head h_list;
struct list_head node;
+
+ size_t num_vals;
+ size_t max_vals;
+ struct input_value *vals;
};
#define to_input_dev(d) container_of(d, struct input_dev, dev)
@@ -1373,6 +1384,9 @@ struct input_handle;
* @event: event handler. This method is being called by input core with
* interrupts disabled and dev->event_lock spinlock held and so
* it may not sleep
+ * @events: event sequence handler. This method is being called by
+ * input core with interrupts disabled and dev->event_lock
+ * spinlock held and so it may not sleep
* @filter: similar to @event; separates normal event handlers from
* "filters".
* @match: called after comparing device's id with handler's id_table
@@ -1409,6 +1423,8 @@ struct input_handler {
void *private;
void (*event)(struct input_handle *handle, unsigned int type, unsigned int code, int value);
+ void (*events)(struct input_handle *handle,
+ const struct input_value *vals, size_t count);
bool (*filter)(struct input_handle *handle, unsigned int type, unsigned int code, int value);
bool (*match)(struct input_handler *handler, struct input_dev *dev);
int (*connect)(struct input_handler *handler, struct input_dev *dev, const struct input_device_id *id);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 06/19] Input: Send events one packet at a time
2012-08-12 21:42 ` [PATCH 06/19] Input: Send events one packet at a time Henrik Rydberg
@ 2012-08-24 4:03 ` Daniel Kurtz
2012-08-25 19:38 ` Henrik Rydberg
0 siblings, 1 reply; 45+ messages in thread
From: Daniel Kurtz @ 2012-08-24 4:03 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
On Mon, Aug 13, 2012 at 5:42 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> On heavy event loads, such as a multitouch driver, the irqsoff latency
> can be as high as 250 us. By accumulating a frame worth of data
> before passing it on, the latency can be dramatically reduced. As a
> side effect, the special EV_SYN handling can be removed, since the
> frame is now atomic.
This patch(set) is very interesting and exciting. Thanks!
Some questions and comments inline...
>
> This patch adds the events() handler callback and uses it if it
> exists. The latency is improved by 50 us even without the callback.
How much of the savings is just from reducing the number of
add_input_randomness() calls from 1-per-input_value to 1-per-frame?
Could you achieve similar savings by only calling add_input_randomness
on the first input_value after each EV_SYN/SYN_REPORT (ie "when sync =
true")?
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> drivers/input/input-mt.c | 3 +-
> drivers/input/input.c | 187 ++++++++++++++++++++++++++++-------------------
> include/linux/input.h | 26 +++++--
> 3 files changed, 132 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index 58bde77..f956b27 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -63,7 +63,6 @@ void input_mt_destroy_slots(struct input_dev *dev)
> {
> kfree(dev->mt);
> dev->mt = NULL;
> - dev->slot = 0;
> }
> EXPORT_SYMBOL(input_mt_destroy_slots);
>
> @@ -91,7 +90,7 @@ void input_mt_report_slot_state(struct input_dev *dev,
> return;
> }
>
> - slot = &mt->slots[dev->slot];
> + slot = &mt->slots[input_abs_get_val(dev, ABS_MT_SLOT)];
> id = input_mt_get_value(slot, ABS_MT_TRACKING_ID);
> if (id < 0 || input_mt_get_value(slot, ABS_MT_TOOL_TYPE) != tool_type)
> id = input_mt_new_trkid(mt);
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index a57c4a5..9b6aa15 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -90,46 +90,81 @@ static void input_stop_autorepeat(struct input_dev *dev)
> * filtered out, through all open handles. This function is called with
> * dev->event_lock held and interrupts disabled.
> */
> -static void input_pass_event(struct input_dev *dev,
> - unsigned int type, unsigned int code, int value)
> +static size_t input_to_handler(struct input_handle *handle,
> + struct input_value *vals, size_t count)
The only thing that is a little strange with this function is that it
actually changes the 'vals' array due to in-place filtering. It means
that input_to_handler can't handle const arrays of vals, which may
have a performance impact in some cases (like key repeat). You are
relying on this behavior since you want to pass the final filtered
input_value array to ->events() without copying, but this seems to be
optimizing the 'filtered' case relative to the (normal?) unfiltered
behavior. Probably not worth changing, though.
> {
> - struct input_handler *handler;
> - struct input_handle *handle;
> + struct input_handler *handler = handle->handler;
> + struct input_value *end = vals;
> + struct input_value *v;
>
> - rcu_read_lock();
> + for (v = vals; v != vals + count; v++) {
> + if (handler->filter &&
if (handler->filter == false), you could skip the whole loop and just
assign end = vals + count.
Also, the original version assumed that if a handler had the grab, it
couldn't be a filter, and would skip filtering entirely.
> + handler->filter(handle, v->type, v->code, v->value))
Maybe we can have a handler->filter_events(handle, vals, count) that
returns the number of events left after filtering.
This would allow more sophisticated filtering that could inspect an
entire frame.
> + continue;
> + if (end != v)
> + *end = *v;
> + end++;
> + }
>
> - handle = rcu_dereference(dev->grab);
> - if (handle)
> - handle->handler->event(handle, type, code, value);
> - else {
> - bool filtered = false;
> + count = end - vals;
> + if (!count)
> + return 0;
>
> - list_for_each_entry_rcu(handle, &dev->h_list, d_node) {
> - if (!handle->open)
> - continue;
In the original version, one handler would not call both ->filter()
and ->event().
I'm not sure if that was a bug or a feature. But, you now make it possible.
However, this opens up the possibility of a filter handler processing
events via its ->event that would get filtered out by a later
handler's filter.
In sum, I think if we assume a handler only has either ->filter or
->event (->events), then we can split this function into two, one that
only does filtering on filters, and one that only passes the resulting
filtered events.
> + if (handler->events)
> + handler->events(handle, vals, count);
> + else
> + for (v = vals; v != end; v++)
> + handler->event(handle, v->type, v->code, v->value);
> +
> + return count;
> +}
> +
> +/*
> + * Pass values first through all filters and then, if event has not been
> + * filtered out, through all open handles. This function is called with
> + * dev->event_lock held and interrupts disabled.
> + */
> +static void input_pass_values(struct input_dev *dev,
> + struct input_value *vals, size_t count)
> +{
> + struct input_handle *handle;
> + struct input_value *v;
>
> - handler = handle->handler;
> - if (!handler->filter) {
> - if (filtered)
> - break;
> + if (!count)
> + return;
>
> - handler->event(handle, type, code, value);
> + rcu_read_lock();
>
> - } else if (handler->filter(handle, type, code, value))
> - filtered = true;
> - }
> + handle = rcu_dereference(dev->grab);
> + if (handle) {
> + count = input_to_handler(handle, vals, count);
> + } else {
> + list_for_each_entry_rcu(handle, &dev->h_list, d_node)
> + if (handle->open)
> + count = input_to_handler(handle, vals, count);
> }
>
> rcu_read_unlock();
>
> + add_input_randomness(vals->type, vals->code, vals->value);
> +
> /* trigger auto repeat for key events */
> - if (type == EV_KEY && value != 2) {
> - if (value)
> - input_start_autorepeat(dev, code);
> - else
> - input_stop_autorepeat(dev);
> + 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);
> + }
> }
> +}
> +
> +static void input_pass_event(struct input_dev *dev,
> + unsigned int type, unsigned int code, int value)
> +{
> + struct input_value vals[] = { { type, code, value } };
>
> + input_pass_values(dev, vals, 1);
> }
>
> /*
> @@ -146,18 +181,12 @@ static void input_repeat_key(unsigned long data)
>
> if (test_bit(dev->repeat_key, dev->key) &&
> is_event_supported(dev->repeat_key, dev->keybit, KEY_MAX)) {
> + struct input_value vals[] = {
> + { EV_KEY, dev->repeat_key, 2 },
> + { EV_SYN, SYN_REPORT, 1 },
> + };
>
> - input_pass_event(dev, EV_KEY, dev->repeat_key, 2);
> -
> - if (dev->sync) {
> - /*
> - * Only send SYN_REPORT if we are not in a middle
> - * of driver parsing a new hardware packet.
> - * Otherwise assume that the driver will send
> - * SYN_REPORT once it's done.
> - */
> - input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
> - }
> + input_pass_values(dev, vals, 2);
>
> if (dev->rep[REP_PERIOD])
> mod_timer(&dev->timer, jiffies +
> @@ -170,37 +199,23 @@ static void input_repeat_key(unsigned long data)
> #define INPUT_IGNORE_EVENT 0
> #define INPUT_PASS_TO_HANDLERS 1
> #define INPUT_PASS_TO_DEVICE 2
> +#define INPUT_FLUSH 4
> #define INPUT_PASS_TO_ALL (INPUT_PASS_TO_HANDLERS | INPUT_PASS_TO_DEVICE)
>
> static int input_handle_abs_event(struct input_dev *dev,
> unsigned int code, int *pval)
> {
> bool is_mt_event;
> - int *pold;
> -
> - if (code == ABS_MT_SLOT) {
> - /*
> - * "Stage" the event; we'll flush it later, when we
> - * get actual touch data.
> - */
> - if (dev->mt && *pval >= 0 && *pval < dev->mt->num_slots)
> - dev->slot = *pval;
> -
> - return INPUT_IGNORE_EVENT;
> - }
> + int *pold = NULL;
>
> is_mt_event = input_is_mt_value(code);
>
> if (!is_mt_event) {
> pold = &dev->absinfo[code].value;
> } else if (dev->mt) {
> - pold = &dev->mt->slots[dev->slot].abs[code - ABS_MT_FIRST];
> - } else {
> - /*
> - * Bypass filtering for multi-touch events when
> - * not employing slots.
> - */
> - pold = NULL;
> + int slot = dev->absinfo[ABS_MT_SLOT].value;
> + if (slot >= 0 && slot < dev->mt->num_slots)
> + pold = &dev->mt->slots[slot].abs[code - ABS_MT_FIRST];
> }
>
> if (pold) {
> @@ -212,17 +227,11 @@ static int input_handle_abs_event(struct input_dev *dev,
> *pold = *pval;
> }
>
> - /* Flush pending "slot" event */
> - if (is_mt_event && dev->slot != input_abs_get_val(dev, ABS_MT_SLOT)) {
> - input_abs_set_val(dev, ABS_MT_SLOT, dev->slot);
> - input_pass_event(dev, EV_ABS, ABS_MT_SLOT, dev->slot);
> - }
> -
> return INPUT_PASS_TO_HANDLERS;
> }
>
> -static void input_handle_event(struct input_dev *dev,
> - unsigned int type, unsigned int code, int value)
> +static int input_get_disposition(struct input_dev *dev,
> + unsigned int type, unsigned int code, int value)
> {
> int disposition = INPUT_IGNORE_EVENT;
>
> @@ -235,13 +244,9 @@ static void input_handle_event(struct input_dev *dev,
> break;
>
> case SYN_REPORT:
> - if (!dev->sync) {
> - dev->sync = true;
> - disposition = INPUT_PASS_TO_HANDLERS;
> - }
> + disposition = INPUT_PASS_TO_HANDLERS | INPUT_FLUSH;
> break;
> case SYN_MT_REPORT:
> - dev->sync = false;
> disposition = INPUT_PASS_TO_HANDLERS;
> break;
> }
> @@ -326,14 +331,35 @@ static void input_handle_event(struct input_dev *dev,
> break;
> }
>
> - if (disposition != INPUT_IGNORE_EVENT && type != EV_SYN)
> - dev->sync = false;
> + return disposition;
> +}
> +
> +static void input_handle_event(struct input_dev *dev,
> + unsigned int type, unsigned int code, int value)
> +{
> + struct input_value *v;
> + int disp;
> +
> + disp = input_get_disposition(dev, type, code, value);
>
> - if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event)
> + if ((disp & INPUT_PASS_TO_DEVICE) && dev->event)
> dev->event(dev, type, code, value);
>
> - if (disposition & INPUT_PASS_TO_HANDLERS)
> - input_pass_event(dev, type, code, value);
> + if (!dev->vals)
> + return;
> +
> + if (disp & INPUT_PASS_TO_HANDLERS) {
> + v = &dev->vals[dev->num_vals++];
> + v->type = type;
> + v->code = code;
> + v->value = value;
> + }
> +
> + if ((disp & INPUT_FLUSH) || (dev->num_vals >= dev->max_vals)) {
> + if (dev->num_vals >= 2)
I'm not sure about this check. What if the previous "frame" had
dev->max_vals + 1 events, and so dev->max_vals of them (all but the
SYN_REPORT) were already passed.
We would not get that frame's SYN_REPORT all by itself, so "disp &
INPUT_FLUSH" is true, but dev->num_vals == 1. We still want to pass
the SYN_REPORT immediately, and not save until we get another full
frame.
Is this even possible?
> + input_pass_values(dev, dev->vals, dev->num_vals);
> + dev->num_vals = 0;
> + }
> }
>
> /**
> @@ -361,7 +387,6 @@ void input_event(struct input_dev *dev,
> if (is_event_supported(type, dev->evbit, EV_MAX)) {
>
> spin_lock_irqsave(&dev->event_lock, flags);
> - add_input_randomness(type, code, value);
> input_handle_event(dev, type, code, value);
> spin_unlock_irqrestore(&dev->event_lock, flags);
> }
> @@ -842,8 +867,7 @@ int input_set_keycode(struct input_dev *dev,
> __test_and_clear_bit(old_keycode, dev->key)) {
>
> input_pass_event(dev, EV_KEY, old_keycode, 0);
> - if (dev->sync)
> - input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
> + input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
> }
>
> out:
> @@ -1425,6 +1449,7 @@ static void input_dev_release(struct device *device)
> input_ff_destroy(dev);
> input_mt_destroy_slots(dev);
> kfree(dev->absinfo);
> + kfree(dev->vals);
> kfree(dev);
>
> module_put(THIS_MODULE);
> @@ -1845,6 +1870,14 @@ int input_register_device(struct input_dev *dev)
> if (dev->hint_events_per_packet < packet_size)
> dev->hint_events_per_packet = packet_size;
>
> + dev->num_vals = 0;
> + dev->max_vals = max(dev->hint_events_per_packet, packet_size);
> +
> + kfree(dev->vals);
How could this already be non-NULL? Is it possible to re-register a device?
A huge optimization to input event processing is pretty exciting!
-Daniel
> + dev->vals = kcalloc(dev->max_vals, sizeof(*dev->vals), GFP_KERNEL);
> + if (!dev->vals)
> + return -ENOMEM;
> +
> /*
> * If delay and period are pre-set by the driver, then autorepeating
> * is handled by the driver itself and we don't do it in input.c.
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 76d6788..1f7f172 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -1169,6 +1169,18 @@ struct ff_effect {
> #include <linux/mod_devicetable.h>
>
> /**
> + * struct input_value - input value representation
> + * @type: type of value (EV_KEY, EV_ABS, etc)
> + * @code: the value code
> + * @value: the value
> + */
> +struct input_value {
> + __u16 type;
> + __u16 code;
> + __s32 value;
> +};
> +
> +/**
> * struct input_dev - represents an input device
> * @name: name of the device
> * @phys: physical path to the device in the system hierarchy
> @@ -1204,7 +1216,6 @@ struct ff_effect {
> * @timer: timer for software autorepeat
> * @rep: current values for autorepeat parameters (delay, rate)
> * @mt: pointer to multitouch state
> - * @slot: MT slot currently being transmitted
> * @absinfo: array of &struct input_absinfo elements holding information
> * about absolute axes (current value, min, max, flat, fuzz,
> * resolution)
> @@ -1241,7 +1252,6 @@ struct ff_effect {
> * last user closes the device
> * @going_away: marks devices that are in a middle of unregistering and
> * causes input_open_device*() fail with -ENODEV.
> - * @sync: set to %true when there were no new events since last EV_SYN
> * @dev: driver model's view of this device
> * @h_list: list of input handles associated with the device. When
> * accessing the list dev->mutex must be held
> @@ -1285,7 +1295,6 @@ struct input_dev {
> int rep[REP_CNT];
>
> struct input_mt *mt;
> - int slot;
>
> struct input_absinfo *absinfo;
>
> @@ -1307,12 +1316,14 @@ struct input_dev {
> unsigned int users;
> bool going_away;
>
> - bool sync;
> -
> struct device dev;
>
> struct list_head h_list;
> struct list_head node;
> +
> + size_t num_vals;
> + size_t max_vals;
> + struct input_value *vals;
> };
> #define to_input_dev(d) container_of(d, struct input_dev, dev)
>
> @@ -1373,6 +1384,9 @@ struct input_handle;
> * @event: event handler. This method is being called by input core with
> * interrupts disabled and dev->event_lock spinlock held and so
> * it may not sleep
> + * @events: event sequence handler. This method is being called by
> + * input core with interrupts disabled and dev->event_lock
> + * spinlock held and so it may not sleep
> * @filter: similar to @event; separates normal event handlers from
> * "filters".
> * @match: called after comparing device's id with handler's id_table
> @@ -1409,6 +1423,8 @@ struct input_handler {
> void *private;
>
> void (*event)(struct input_handle *handle, unsigned int type, unsigned int code, int value);
> + void (*events)(struct input_handle *handle,
> + const struct input_value *vals, size_t count);
> bool (*filter)(struct input_handle *handle, unsigned int type, unsigned int code, int value);
> bool (*match)(struct input_handler *handler, struct input_dev *dev);
> int (*connect)(struct input_handler *handler, struct input_dev *dev, const struct input_device_id *id);
> --
> 1.7.11.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 [flat|nested] 45+ messages in thread* Re: [PATCH 06/19] Input: Send events one packet at a time
2012-08-24 4:03 ` Daniel Kurtz
@ 2012-08-25 19:38 ` Henrik Rydberg
0 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-25 19:38 UTC (permalink / raw)
To: Daniel Kurtz; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
Hi Daniel,
> On Mon, Aug 13, 2012 at 5:42 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> > On heavy event loads, such as a multitouch driver, the irqsoff latency
> > can be as high as 250 us. By accumulating a frame worth of data
> > before passing it on, the latency can be dramatically reduced. As a
> > side effect, the special EV_SYN handling can be removed, since the
> > frame is now atomic.
>
> This patch(set) is very interesting and exciting. Thanks!
> Some questions and comments inline...
>
> >
> > This patch adds the events() handler callback and uses it if it
> > exists. The latency is improved by 50 us even without the callback.
>
> How much of the savings is just from reducing the number of
> add_input_randomness() calls from 1-per-input_value to 1-per-frame?
Some, but the largest saving comes from calling down to evdev more sparsely.
> Could you achieve similar savings by only calling add_input_randomness
> on the first input_value after each EV_SYN/SYN_REPORT (ie "when sync =
> true")?
It might make a bit of a difference, because of the additional locks,
but I have not tried it explicitly.
> > @@ -90,46 +90,81 @@ static void input_stop_autorepeat(struct input_dev *dev)
> > * filtered out, through all open handles. This function is called with
> > * dev->event_lock held and interrupts disabled.
> > */
> > -static void input_pass_event(struct input_dev *dev,
> > - unsigned int type, unsigned int code, int value)
> > +static size_t input_to_handler(struct input_handle *handle,
> > + struct input_value *vals, size_t count)
>
> The only thing that is a little strange with this function is that it
> actually changes the 'vals' array due to in-place filtering.
Hm, yes, I did not want to allocate additional memory for the
filtering stuff. It is only used in a few (one?) place, and TBH, it is
not on my list of favorite pieces of code. I would rather see that
api modified than working towards more elaborate filtering schemes.
> It means
> that input_to_handler can't handle const arrays of vals, which may
> have a performance impact in some cases (like key repeat). You are
> relying on this behavior since you want to pass the final filtered
> input_value array to ->events() without copying, but this seems to be
> optimizing the 'filtered' case relative to the (normal?) unfiltered
> behavior. Probably not worth changing, though.
Right.
>
> > {
> > - struct input_handler *handler;
> > - struct input_handle *handle;
> > + struct input_handler *handler = handle->handler;
> > + struct input_value *end = vals;
> > + struct input_value *v;
> >
> > - rcu_read_lock();
> > + for (v = vals; v != vals + count; v++) {
> > + if (handler->filter &&
>
> if (handler->filter == false), you could skip the whole loop and just
> assign end = vals + count.
True - in principle, but currently a suboptimization.
> Also, the original version assumed that if a handler had the grab, it
> couldn't be a filter, and would skip filtering entirely.
>
> > + handler->filter(handle, v->type, v->code, v->value))
>
> Maybe we can have a handler->filter_events(handle, vals, count) that
> returns the number of events left after filtering.
> This would allow more sophisticated filtering that could inspect an
> entire frame.
Possibly. Still, the notion of filtering as information-sharing
between drivers on the input bus is not one of my favorites. IMHO,
focus should be on getting the data out of the kernel as quickly as
possible.
>
> > + continue;
> > + if (end != v)
> > + *end = *v;
> > + end++;
> > + }
> >
> > - handle = rcu_dereference(dev->grab);
> > - if (handle)
> > - handle->handler->event(handle, type, code, value);
> > - else {
> > - bool filtered = false;
> > + count = end - vals;
> > + if (!count)
> > + return 0;
> >
> > - list_for_each_entry_rcu(handle, &dev->h_list, d_node) {
> > - if (!handle->open)
> > - continue;
>
> In the original version, one handler would not call both ->filter()
> and ->event().
> I'm not sure if that was a bug or a feature. But, you now make it possible.
> However, this opens up the possibility of a filter handler processing
> events via its ->event that would get filtered out by a later
> handler's filter.
True, but it does not change any of the existing usages of filtering.
> In sum, I think if we assume a handler only has either ->filter or
> ->event (->events), then we can split this function into two, one that
> only does filtering on filters, and one that only passes the resulting
> filtered events.
>
> > + if (handler->events)
> > + handler->events(handle, vals, count);
> > + else
> > + for (v = vals; v != end; v++)
> > + handler->event(handle, v->type, v->code, v->value);
> > +
> > + return count;
> > +}
My standpoint is clear by now, so I shall not repeat myself. :-)
> > @@ -326,14 +331,35 @@ static void input_handle_event(struct input_dev *dev,
> > break;
> > }
> >
> > - if (disposition != INPUT_IGNORE_EVENT && type != EV_SYN)
> > - dev->sync = false;
> > + return disposition;
> > +}
> > +
> > +static void input_handle_event(struct input_dev *dev,
> > + unsigned int type, unsigned int code, int value)
> > +{
> > + struct input_value *v;
> > + int disp;
> > +
> > + disp = input_get_disposition(dev, type, code, value);
> >
> > - if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event)
> > + if ((disp & INPUT_PASS_TO_DEVICE) && dev->event)
> > dev->event(dev, type, code, value);
> >
> > - if (disposition & INPUT_PASS_TO_HANDLERS)
> > - input_pass_event(dev, type, code, value);
> > + if (!dev->vals)
> > + return;
> > +
> > + if (disp & INPUT_PASS_TO_HANDLERS) {
> > + v = &dev->vals[dev->num_vals++];
> > + v->type = type;
> > + v->code = code;
> > + v->value = value;
> > + }
> > +
> > + if ((disp & INPUT_FLUSH) || (dev->num_vals >= dev->max_vals)) {
> > + if (dev->num_vals >= 2)
>
> I'm not sure about this check. What if the previous "frame" had
> dev->max_vals + 1 events, and so dev->max_vals of them (all but the
> SYN_REPORT) were already passed.
> We would not get that frame's SYN_REPORT all by itself, so "disp &
> INPUT_FLUSH" is true, but dev->num_vals == 1. We still want to pass
> the SYN_REPORT immediately, and not save until we get another full
> frame.
>
> Is this even possible?
Yes, it is possible, if the driver is misconfigured with respect to
the input buffer size. I have ignored that possibility in a few other
places as well (keyboard repeat for one). You are probably right in
that it should be handled somehow, but I would rather make sure the
buffer is always large enough. The atomicity of the frame is really
what makes things go faster.
>
> > + input_pass_values(dev, dev->vals, dev->num_vals);
> > + dev->num_vals = 0;
> > + }
> > }
> >
> > /**
> > @@ -361,7 +387,6 @@ void input_event(struct input_dev *dev,
> > if (is_event_supported(type, dev->evbit, EV_MAX)) {
> >
> > spin_lock_irqsave(&dev->event_lock, flags);
> > - add_input_randomness(type, code, value);
> > input_handle_event(dev, type, code, value);
> > spin_unlock_irqrestore(&dev->event_lock, flags);
> > }
> > @@ -842,8 +867,7 @@ int input_set_keycode(struct input_dev *dev,
> > __test_and_clear_bit(old_keycode, dev->key)) {
> >
> > input_pass_event(dev, EV_KEY, old_keycode, 0);
> > - if (dev->sync)
> > - input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
> > + input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
> > }
> >
> > out:
> > @@ -1425,6 +1449,7 @@ static void input_dev_release(struct device *device)
> > input_ff_destroy(dev);
> > input_mt_destroy_slots(dev);
> > kfree(dev->absinfo);
> > + kfree(dev->vals);
> > kfree(dev);
> >
> > module_put(THIS_MODULE);
> > @@ -1845,6 +1870,14 @@ int input_register_device(struct input_dev *dev)
> > if (dev->hint_events_per_packet < packet_size)
> > dev->hint_events_per_packet = packet_size;
> >
> > + dev->num_vals = 0;
> > + dev->max_vals = max(dev->hint_events_per_packet, packet_size);
> > +
> > + kfree(dev->vals);
> How could this already be non-NULL? Is it possible to re-register a device?
Uhm, you are probably right.
> A huge optimization to input event processing is pretty exciting!
Thanks for the review, I will send out a new patchset taking all the
response so far into account.
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 07/19] Input: evdev - Add the events() callback
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (5 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 06/19] Input: Send events one packet at a time Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-24 4:07 ` Daniel Kurtz
2012-08-12 21:42 ` [PATCH 08/19] Input: MT - Add flags to input_mt_init_slots() Henrik Rydberg
` (11 subsequent siblings)
18 siblings, 1 reply; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
By sending a full frame of events at the same time, the irqsoff
latency at heavy load is brought down from 200 us to 100 us.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/evdev.c | 68 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 47 insertions(+), 21 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index a0692c5..1bf4ce5 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -54,16 +54,9 @@ struct evdev_client {
static struct evdev *evdev_table[EVDEV_MINORS];
static DEFINE_MUTEX(evdev_table_mutex);
-static void evdev_pass_event(struct evdev_client *client,
- struct input_event *event,
- ktime_t mono, ktime_t real)
+static void __pass_event(struct evdev_client *client,
+ const struct input_event *event)
{
- event->time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
- mono : real);
-
- /* Interrupts are disabled, just acquire the lock. */
- spin_lock(&client->buffer_lock);
-
client->buffer[client->head++] = *event;
client->head &= client->bufsize - 1;
@@ -86,42 +79,74 @@ static void evdev_pass_event(struct evdev_client *client,
client->packet_head = client->head;
kill_fasync(&client->fasync, SIGIO, POLL_IN);
}
+}
+
+static void evdev_pass_values(struct evdev_client *client,
+ const struct input_value *vals, size_t count,
+ ktime_t mono, ktime_t real)
+{
+ struct evdev *evdev = client->evdev;
+ const struct input_value *v;
+ struct input_event event;
+ bool wakeup = false;
+
+ event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
+ mono : real);
+
+ /* Interrupts are disabled, just acquire the lock. */
+ spin_lock(&client->buffer_lock);
+
+ for (v = vals; v != vals + count; v++) {
+ event.type = v->type;
+ event.code = v->code;
+ event.value = v->value;
+ __pass_event(client, &event);
+ if (v->type == EV_SYN && v->code == SYN_REPORT)
+ wakeup = true;
+ }
spin_unlock(&client->buffer_lock);
+
+ if (wakeup)
+ wake_up_interruptible(&evdev->wait);
}
/*
- * Pass incoming event to all connected clients.
+ * Pass incoming events to all connected clients.
*/
-static void evdev_event(struct input_handle *handle,
- unsigned int type, unsigned int code, int value)
+static void evdev_events(struct input_handle *handle,
+ const struct input_value *vals, size_t count)
{
struct evdev *evdev = handle->private;
struct evdev_client *client;
- struct input_event event;
ktime_t time_mono, time_real;
time_mono = ktime_get();
time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());
- event.type = type;
- event.code = code;
- event.value = value;
-
rcu_read_lock();
client = rcu_dereference(evdev->grab);
if (client)
- evdev_pass_event(client, &event, time_mono, time_real);
+ evdev_pass_values(client, vals, count, time_mono, time_real);
else
list_for_each_entry_rcu(client, &evdev->client_list, node)
- evdev_pass_event(client, &event, time_mono, time_real);
+ evdev_pass_values(client, vals, count,
+ time_mono, time_real);
rcu_read_unlock();
+}
- if (type == EV_SYN && code == SYN_REPORT)
- wake_up_interruptible(&evdev->wait);
+/*
+ * Pass incoming event to all connected clients.
+ */
+static void evdev_event(struct input_handle *handle,
+ unsigned int type, unsigned int code, int value)
+{
+ struct input_value vals[] = { { type, code, value } };
+
+ evdev_events(handle, vals, 1);
}
static int evdev_fasync(int fd, struct file *file, int on)
@@ -1050,6 +1075,7 @@ MODULE_DEVICE_TABLE(input, evdev_ids);
static struct input_handler evdev_handler = {
.event = evdev_event,
+ .events = evdev_events,
.connect = evdev_connect,
.disconnect = evdev_disconnect,
.fops = &evdev_fops,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 07/19] Input: evdev - Add the events() callback
2012-08-12 21:42 ` [PATCH 07/19] Input: evdev - Add the events() callback Henrik Rydberg
@ 2012-08-24 4:07 ` Daniel Kurtz
2012-08-25 19:46 ` Henrik Rydberg
0 siblings, 1 reply; 45+ messages in thread
From: Daniel Kurtz @ 2012-08-24 4:07 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
On Mon, Aug 13, 2012 at 5:42 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> By sending a full frame of events at the same time, the irqsoff
> latency at heavy load is brought down from 200 us to 100 us.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> drivers/input/evdev.c | 68 +++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 47 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index a0692c5..1bf4ce5 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -54,16 +54,9 @@ struct evdev_client {
> static struct evdev *evdev_table[EVDEV_MINORS];
> static DEFINE_MUTEX(evdev_table_mutex);
>
> -static void evdev_pass_event(struct evdev_client *client,
> - struct input_event *event,
> - ktime_t mono, ktime_t real)
> +static void __pass_event(struct evdev_client *client,
> + const struct input_event *event)
> {
> - event->time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
> - mono : real);
> -
> - /* Interrupts are disabled, just acquire the lock. */
> - spin_lock(&client->buffer_lock);
> -
> client->buffer[client->head++] = *event;
> client->head &= client->bufsize - 1;
>
> @@ -86,42 +79,74 @@ static void evdev_pass_event(struct evdev_client *client,
> client->packet_head = client->head;
> kill_fasync(&client->fasync, SIGIO, POLL_IN);
> }
> +}
> +
> +static void evdev_pass_values(struct evdev_client *client,
> + const struct input_value *vals, size_t count,
> + ktime_t mono, ktime_t real)
> +{
> + struct evdev *evdev = client->evdev;
> + const struct input_value *v;
> + struct input_event event;
> + bool wakeup = false;
> +
> + event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
> + mono : real);
> +
> + /* Interrupts are disabled, just acquire the lock. */
> + spin_lock(&client->buffer_lock);
> +
> + for (v = vals; v != vals + count; v++) {
> + event.type = v->type;
> + event.code = v->code;
> + event.value = v->value;
> + __pass_event(client, &event);
> + if (v->type == EV_SYN && v->code == SYN_REPORT)
> + wakeup = true;
> + }
>
> spin_unlock(&client->buffer_lock);
> +
> + if (wakeup)
> + wake_up_interruptible(&evdev->wait);
> }
>
> /*
> - * Pass incoming event to all connected clients.
> + * Pass incoming events to all connected clients.
> */
> -static void evdev_event(struct input_handle *handle,
> - unsigned int type, unsigned int code, int value)
> +static void evdev_events(struct input_handle *handle,
> + const struct input_value *vals, size_t count)
> {
> struct evdev *evdev = handle->private;
> struct evdev_client *client;
> - struct input_event event;
> ktime_t time_mono, time_real;
>
> time_mono = ktime_get();
> time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());
>
> - event.type = type;
> - event.code = code;
> - event.value = value;
> -
> rcu_read_lock();
>
> client = rcu_dereference(evdev->grab);
>
> if (client)
> - evdev_pass_event(client, &event, time_mono, time_real);
> + evdev_pass_values(client, vals, count, time_mono, time_real);
> else
> list_for_each_entry_rcu(client, &evdev->client_list, node)
> - evdev_pass_event(client, &event, time_mono, time_real);
> + evdev_pass_values(client, vals, count,
> + time_mono, time_real);
Hi Henrik,
Reading the time just once and applying it as the timestamp to an
entire frame is very nice.
However, is it ever possible for the SYN_REPORT to get delayed until
the next batch of input_values, therefore breaking the assumption that
the SYN_REPORT timestamp applies to the rest of the input_values for
its frame?
Also, bonus points if the input driver could set this input frame
timestamp based on when it first saw a hardware interrupt rather then
when evdev gets around to sending the frame to userspace. This could
potentially remove a lot of the timing jitter userspace sees when
computing ballistics based on input event timestamps.
Thanks!
-Daniel
>
> rcu_read_unlock();
> +}
>
> - if (type == EV_SYN && code == SYN_REPORT)
> - wake_up_interruptible(&evdev->wait);
> +/*
> + * Pass incoming event to all connected clients.
> + */
> +static void evdev_event(struct input_handle *handle,
> + unsigned int type, unsigned int code, int value)
> +{
> + struct input_value vals[] = { { type, code, value } };
> +
> + evdev_events(handle, vals, 1);
> }
>
> static int evdev_fasync(int fd, struct file *file, int on)
> @@ -1050,6 +1075,7 @@ MODULE_DEVICE_TABLE(input, evdev_ids);
>
> static struct input_handler evdev_handler = {
> .event = evdev_event,
> + .events = evdev_events,
> .connect = evdev_connect,
> .disconnect = evdev_disconnect,
> .fops = &evdev_fops,
> --
> 1.7.11.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 [flat|nested] 45+ messages in thread* Re: [PATCH 07/19] Input: evdev - Add the events() callback
2012-08-24 4:07 ` Daniel Kurtz
@ 2012-08-25 19:46 ` Henrik Rydberg
0 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-25 19:46 UTC (permalink / raw)
To: Daniel Kurtz; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
> Reading the time just once and applying it as the timestamp to an
> entire frame is very nice.
> However, is it ever possible for the SYN_REPORT to get delayed until
> the next batch of input_values, therefore breaking the assumption that
> the SYN_REPORT timestamp applies to the rest of the input_values for
> its frame?
Yes, but see reply to previous patch.
> Also, bonus points if the input driver could set this input frame
> timestamp based on when it first saw a hardware interrupt rather then
> when evdev gets around to sending the frame to userspace. This could
> potentially remove a lot of the timing jitter userspace sees when
> computing ballistics based on input event timestamps.
In principle, yes (it has been discussed before), but in practise some
devices provide timestamps and some not, and the scale and granularity
may vary. In addition, desktop userland (read X input) does not even
use the kernel timestamp, so the effect would not even be seen without
a synchronized effort. I am not saying it is a bad idea, but it has
some details to get straight before it becomes useful.
Thanks,
Henrik
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 08/19] Input: MT - Add flags to input_mt_init_slots()
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (6 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 07/19] Input: evdev - Add the events() callback Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 09/19] Input: MT - Handle frame synchronization in core Henrik Rydberg
` (10 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Preparing to move more repeated code into the mt core, and a flags
argument to the input_mt_slots_init() function.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hid/hid-magicmouse.c | 2 +-
drivers/hid/hid-multitouch.c | 2 +-
drivers/input/input-mt.c | 4 +++-
drivers/input/misc/uinput.c | 2 +-
drivers/input/mouse/alps.c | 2 +-
drivers/input/mouse/elantech.c | 4 ++--
drivers/input/mouse/sentelic.c | 2 +-
drivers/input/mouse/synaptics.c | 4 ++--
drivers/input/tablet/wacom_wac.c | 6 +++---
drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
drivers/input/touchscreen/cyttsp_core.c | 2 +-
drivers/input/touchscreen/edt-ft5x06.c | 2 +-
drivers/input/touchscreen/egalax_ts.c | 2 +-
drivers/input/touchscreen/ili210x.c | 2 +-
drivers/input/touchscreen/mms114.c | 2 +-
drivers/input/touchscreen/penmount.c | 2 +-
drivers/input/touchscreen/wacom_w8001.c | 2 +-
include/linux/input/mt.h | 5 ++++-
18 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index e425de4..eae3aaa 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -392,7 +392,7 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd
__set_bit(EV_ABS, input->evbit);
- error = input_mt_init_slots(input, 16);
+ error = input_mt_init_slots(input, 16, 0);
if (error)
return error;
input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255 << 2,
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 59c8b5c..c400d90 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -390,7 +390,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
case HID_DG_CONTACTID:
if (!td->maxcontacts)
td->maxcontacts = MT_DEFAULT_MAXCONTACT;
- input_mt_init_slots(hi->input, td->maxcontacts);
+ input_mt_init_slots(hi->input, td->maxcontacts, 0);
mt_store_field(usage, td, hi);
td->last_field_index = field->index;
td->touches_by_report++;
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index f956b27..bbb3304 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -25,7 +25,8 @@
* May be called repeatedly. Returns -EINVAL if attempting to
* reinitialize with a different number of slots.
*/
-int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots)
+int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
+ unsigned int flags)
{
struct input_mt *mt = dev->mt;
int i;
@@ -40,6 +41,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots)
return -ENOMEM;
mt->num_slots = num_slots;
+ mt->flags = flags;
input_set_abs_params(dev, ABS_MT_SLOT, 0, num_slots - 1, 0, 0);
input_set_abs_params(dev, ABS_MT_TRACKING_ID, 0, TRKID_MAX, 0, 0);
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 7360568..6b17975 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -405,7 +405,7 @@ static int uinput_setup_device(struct uinput_device *udev, const char __user *bu
goto exit;
if (test_bit(ABS_MT_SLOT, dev->absbit)) {
int nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1;
- input_mt_init_slots(dev, nslot);
+ input_mt_init_slots(dev, nslot, 0);
} else if (test_bit(ABS_MT_POSITION_X, dev->absbit)) {
input_set_events_per_packet(dev, 60);
}
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 4a1347e..cf5af1f 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1620,7 +1620,7 @@ int alps_init(struct psmouse *psmouse)
case ALPS_PROTO_V3:
case ALPS_PROTO_V4:
set_bit(INPUT_PROP_SEMI_MT, dev1->propbit);
- input_mt_init_slots(dev1, 2);
+ input_mt_init_slots(dev1, 2, 0);
input_set_abs_params(dev1, ABS_MT_POSITION_X, 0, ALPS_V3_X_MAX, 0, 0);
input_set_abs_params(dev1, ABS_MT_POSITION_Y, 0, ALPS_V3_Y_MAX, 0, 0);
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 4790110..1e8e42f 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1004,7 +1004,7 @@ static int elantech_set_input_params(struct psmouse *psmouse)
input_set_abs_params(dev, ABS_TOOL_WIDTH, ETP_WMIN_V2,
ETP_WMAX_V2, 0, 0);
}
- input_mt_init_slots(dev, 2);
+ input_mt_init_slots(dev, 2, 0);
input_set_abs_params(dev, ABS_MT_POSITION_X, x_min, x_max, 0, 0);
input_set_abs_params(dev, ABS_MT_POSITION_Y, y_min, y_max, 0, 0);
break;
@@ -1035,7 +1035,7 @@ static int elantech_set_input_params(struct psmouse *psmouse)
input_set_abs_params(dev, ABS_TOOL_WIDTH, ETP_WMIN_V2,
ETP_WMAX_V2, 0, 0);
/* Multitouch capable pad, up to 5 fingers. */
- input_mt_init_slots(dev, ETP_MAX_FINGERS);
+ input_mt_init_slots(dev, ETP_MAX_FINGERS, 0);
input_set_abs_params(dev, ABS_MT_POSITION_X, x_min, x_max, 0, 0);
input_set_abs_params(dev, ABS_MT_POSITION_Y, y_min, y_max, 0, 0);
input_abs_set_res(dev, ABS_MT_POSITION_X, x_res);
diff --git a/drivers/input/mouse/sentelic.c b/drivers/input/mouse/sentelic.c
index 3f5649f..b47155d 100644
--- a/drivers/input/mouse/sentelic.c
+++ b/drivers/input/mouse/sentelic.c
@@ -960,7 +960,7 @@ static int fsp_set_input_params(struct psmouse *psmouse)
input_set_abs_params(dev, ABS_X, 0, abs_x, 0, 0);
input_set_abs_params(dev, ABS_Y, 0, abs_y, 0, 0);
- input_mt_init_slots(dev, 2);
+ input_mt_init_slots(dev, 2, 0);
input_set_abs_params(dev, ABS_MT_POSITION_X, 0, abs_x, 0, 0);
input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, abs_y, 0, 0);
}
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 14eaece..37033ad 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1232,7 +1232,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
- input_mt_init_slots(dev, 2);
+ input_mt_init_slots(dev, 2, 0);
set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
ABS_MT_POSITION_Y);
/* Image sensors can report per-contact pressure */
@@ -1244,7 +1244,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
/* Non-image sensors with AGM use semi-mt */
__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
- input_mt_init_slots(dev, 2);
+ input_mt_init_slots(dev, 2, 0);
set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
ABS_MT_POSITION_Y);
}
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 0020419..5837d07 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -1530,7 +1530,7 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
__set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
__set_bit(BTN_TOOL_QUADTAP, input_dev->keybit);
- input_mt_init_slots(input_dev, features->touch_max);
+ input_mt_init_slots(input_dev, features->touch_max, 0);
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
0, 255, 0, 0);
@@ -1575,7 +1575,7 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
case TABLETPC2FG:
if (features->device_type == BTN_TOOL_FINGER) {
- input_mt_init_slots(input_dev, features->touch_max);
+ input_mt_init_slots(input_dev, features->touch_max, 0);
input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE,
0, MT_TOOL_MAX, 0, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
@@ -1631,7 +1631,7 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
__set_bit(BTN_TOOL_FINGER, input_dev->keybit);
__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
- input_mt_init_slots(input_dev, features->touch_max);
+ input_mt_init_slots(input_dev, features->touch_max, 0);
if (features->pktlen == WACOM_PKGLEN_BBTOUCH3) {
__set_bit(BTN_TOOL_TRIPLETAP,
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 4623cc6..e92615d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1152,7 +1152,7 @@ static int __devinit mxt_probe(struct i2c_client *client,
/* For multi touch */
num_mt_slots = data->T9_reportid_max - data->T9_reportid_min + 1;
- error = input_mt_init_slots(input_dev, num_mt_slots);
+ error = input_mt_init_slots(input_dev, num_mt_slots, 0);
if (error)
goto err_free_object;
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
index f030d9e..8e60437 100644
--- a/drivers/input/touchscreen/cyttsp_core.c
+++ b/drivers/input/touchscreen/cyttsp_core.c
@@ -571,7 +571,7 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
0, CY_MAXZ, 0, 0);
- input_mt_init_slots(input_dev, CY_MAX_ID);
+ input_mt_init_slots(input_dev, CY_MAX_ID, 0);
error = request_threaded_irq(ts->irq, NULL, cyttsp_irq,
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 9afc777..7b786e7 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -778,7 +778,7 @@ static int __devinit edt_ft5x06_ts_probe(struct i2c_client *client,
0, tsdata->num_x * 64 - 1, 0, 0);
input_set_abs_params(input, ABS_MT_POSITION_Y,
0, tsdata->num_y * 64 - 1, 0, 0);
- error = input_mt_init_slots(input, MAX_SUPPORT_POINTS);
+ error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, 0);
if (error) {
dev_err(&client->dev, "Unable to init MT slots.\n");
goto err_free_mem;
diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
index 70524dd..c1e3460 100644
--- a/drivers/input/touchscreen/egalax_ts.c
+++ b/drivers/input/touchscreen/egalax_ts.c
@@ -204,7 +204,7 @@ static int __devinit egalax_ts_probe(struct i2c_client *client,
ABS_MT_POSITION_X, 0, EGALAX_MAX_X, 0, 0);
input_set_abs_params(input_dev,
ABS_MT_POSITION_X, 0, EGALAX_MAX_Y, 0, 0);
- input_mt_init_slots(input_dev, MAX_SUPPORT_POINTS);
+ input_mt_init_slots(input_dev, MAX_SUPPORT_POINTS, 0);
input_set_drvdata(input_dev, ts);
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index c004417..4ac6976 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -252,7 +252,7 @@ static int __devinit ili210x_i2c_probe(struct i2c_client *client,
input_set_abs_params(input, ABS_Y, 0, ymax, 0, 0);
/* Multi touch */
- input_mt_init_slots(input, MAX_TOUCHES);
+ input_mt_init_slots(input, MAX_TOUCHES, 0);
input_set_abs_params(input, ABS_MT_POSITION_X, 0, xmax, 0, 0);
input_set_abs_params(input, ABS_MT_POSITION_Y, 0, ymax, 0, 0);
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 49c44bb..560cf09 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -404,7 +404,7 @@ static int __devinit mms114_probe(struct i2c_client *client,
input_set_abs_params(input_dev, ABS_Y, 0, data->pdata->y_size, 0, 0);
/* For multi touch */
- input_mt_init_slots(input_dev, MMS114_MAX_TOUCH);
+ input_mt_init_slots(input_dev, MMS114_MAX_TOUCH, 0);
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
0, MMS114_MAX_AREA, 0, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
diff --git a/drivers/input/touchscreen/penmount.c b/drivers/input/touchscreen/penmount.c
index 4ccde45..b49f0b8 100644
--- a/drivers/input/touchscreen/penmount.c
+++ b/drivers/input/touchscreen/penmount.c
@@ -264,7 +264,7 @@ static int pm_connect(struct serio *serio, struct serio_driver *drv)
input_set_abs_params(pm->dev, ABS_Y, 0, max_y, 0, 0);
if (pm->maxcontacts > 1) {
- input_mt_init_slots(pm->dev, pm->maxcontacts);
+ input_mt_init_slots(pm->dev, pm->maxcontacts, 0);
input_set_abs_params(pm->dev,
ABS_MT_POSITION_X, 0, max_x, 0, 0);
input_set_abs_params(pm->dev,
diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 8f9ad2f..9a83be6 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -471,7 +471,7 @@ static int w8001_setup(struct w8001 *w8001)
case 5:
w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
- input_mt_init_slots(dev, 2);
+ input_mt_init_slots(dev, 2, 0);
input_set_abs_params(dev, ABS_MT_POSITION_X,
0, touch.x, 0, 0);
input_set_abs_params(dev, ABS_MT_POSITION_Y,
diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
index 4ae275c..03c52ce 100644
--- a/include/linux/input/mt.h
+++ b/include/linux/input/mt.h
@@ -27,11 +27,13 @@ struct input_mt_slot {
* struct input_mt - state of tracked contacts
* @trkid: stores MT tracking ID for the next contact
* @num_slots: number of MT slots the device uses
+ * @flags: input_mt operation flags
* @slots: array of slots holding current values of tracked contacts
*/
struct input_mt {
int trkid;
int num_slots;
+ unsigned int flags;
struct input_mt_slot slots[];
};
@@ -47,7 +49,8 @@ static inline int input_mt_get_value(const struct input_mt_slot *slot,
return slot->abs[code - ABS_MT_FIRST];
}
-int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots);
+int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
+ unsigned int flags);
void input_mt_destroy_slots(struct input_dev *dev);
static inline int input_mt_new_trkid(struct input_mt *mt)
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 09/19] Input: MT - Handle frame synchronization in core
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (7 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 08/19] Input: MT - Add flags to input_mt_init_slots() Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-15 23:28 ` Ping Cheng
2012-08-20 13:36 ` Benjamin Tissoires
2012-08-12 21:42 ` [PATCH 10/19] Input: MT - Add in-kernel tracking Henrik Rydberg
` (9 subsequent siblings)
18 siblings, 2 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Collect common frame synchronization tasks in a new function,
input_mt_sync_frame(). Depending on the flags set, it drops
unseen contacts and performs pointer emulation.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/input-mt.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++--
include/linux/input/mt.h | 9 ++++++
2 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index bbb3304..21b105e 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -14,6 +14,14 @@
#define TRKID_SGN ((TRKID_MAX + 1) >> 1)
+static void copy_abs(struct input_dev *dev, unsigned int dst, unsigned int src)
+{
+ if (dev->absinfo && test_bit(src, dev->absbit)) {
+ dev->absinfo[dst] = dev->absinfo[src];
+ dev->absbit[BIT_WORD(dst)] |= BIT_MASK(dst);
+ }
+}
+
/**
* input_mt_init_slots() - initialize MT input slots
* @dev: input device supporting MT events and finger tracking
@@ -45,6 +53,28 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
input_set_abs_params(dev, ABS_MT_SLOT, 0, num_slots - 1, 0, 0);
input_set_abs_params(dev, ABS_MT_TRACKING_ID, 0, TRKID_MAX, 0, 0);
+ if (flags & (INPUT_MT_POINTER | INPUT_MT_DIRECT)) {
+ __set_bit(EV_KEY, dev->evbit);
+ __set_bit(BTN_TOUCH, dev->keybit);
+
+ copy_abs(dev, ABS_X, ABS_MT_POSITION_X);
+ copy_abs(dev, ABS_Y, ABS_MT_POSITION_Y);
+ copy_abs(dev, ABS_PRESSURE, ABS_MT_PRESSURE);
+ }
+ if (flags & INPUT_MT_POINTER) {
+ __set_bit(BTN_TOOL_FINGER, dev->keybit);
+ __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+ if (num_slots >= 3)
+ __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
+ if (num_slots >= 4)
+ __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
+ if (num_slots >= 5)
+ __set_bit(BTN_TOOL_QUINTTAP, dev->keybit);
+ __set_bit(INPUT_PROP_POINTER, dev->propbit);
+ }
+ if (flags & INPUT_MT_DIRECT)
+ __set_bit(INPUT_PROP_DIRECT, dev->propbit);
+
/* Mark slots as 'unused' */
for (i = 0; i < num_slots; i++)
input_mt_set_value(&mt->slots[i], ABS_MT_TRACKING_ID, -1);
@@ -87,12 +117,17 @@ void input_mt_report_slot_state(struct input_dev *dev,
struct input_mt_slot *slot;
int id;
- if (!mt || !active) {
+ if (!mt)
+ return;
+
+ slot = &mt->slots[input_abs_get_val(dev, ABS_MT_SLOT)];
+ slot->frame = mt->frame;
+
+ if (!active) {
input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
return;
}
- slot = &mt->slots[input_abs_get_val(dev, ABS_MT_SLOT)];
id = input_mt_get_value(slot, ABS_MT_TRACKING_ID);
if (id < 0 || input_mt_get_value(slot, ABS_MT_TOOL_TYPE) != tool_type)
id = input_mt_new_trkid(mt);
@@ -177,3 +212,38 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
}
}
EXPORT_SYMBOL(input_mt_report_pointer_emulation);
+
+/**
+ * input_mt_sync_frame() - synchronize mt frame
+ * @dev: input device with allocated MT slots
+ *
+ * Close the frame and prepare the internal state for a new one.
+ * Depending on the flags, marks unused slots as inactive and performs
+ * pointer emulation.
+ */
+void input_mt_sync_frame(struct input_dev *dev)
+{
+ struct input_mt *mt = dev->mt;
+ struct input_mt_slot *s;
+
+ if (!mt)
+ return;
+
+ if (mt->flags & INPUT_MT_DROP_UNUSED) {
+ for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
+ if (s->frame == mt->frame)
+ continue;
+ input_mt_slot(dev, s - mt->slots);
+ input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+ }
+ }
+
+ if (mt->flags & INPUT_MT_POINTER)
+ input_mt_report_pointer_emulation(dev, true);
+
+ if (mt->flags & INPUT_MT_DIRECT)
+ input_mt_report_pointer_emulation(dev, false);
+
+ mt->frame++;
+}
+EXPORT_SYMBOL(input_mt_sync_frame);
diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
index 03c52ce..a11d485 100644
--- a/include/linux/input/mt.h
+++ b/include/linux/input/mt.h
@@ -15,12 +15,17 @@
#define TRKID_MAX 0xffff
+#define INPUT_MT_POINTER 0x0001 /* pointer device, e.g. trackpad */
+#define INPUT_MT_DIRECT 0x0002 /* direct device, e.g. touchscreen */
+#define INPUT_MT_DROP_UNUSED 0x0004 /* drop contacts not seen in frame */
/**
* struct input_mt_slot - represents the state of an input MT slot
* @abs: holds current values of ABS_MT axes for this slot
+ * @frame: last frame at which input_mt_report_slot_state() was called
*/
struct input_mt_slot {
int abs[ABS_MT_LAST - ABS_MT_FIRST + 1];
+ unsigned int frame;
};
/**
@@ -28,12 +33,14 @@ struct input_mt_slot {
* @trkid: stores MT tracking ID for the next contact
* @num_slots: number of MT slots the device uses
* @flags: input_mt operation flags
+ * @frame: increases every time input_mt_sync_frame() is called
* @slots: array of slots holding current values of tracked contacts
*/
struct input_mt {
int trkid;
int num_slots;
unsigned int flags;
+ unsigned int frame;
struct input_mt_slot slots[];
};
@@ -79,4 +86,6 @@ void input_mt_report_slot_state(struct input_dev *dev,
void input_mt_report_finger_count(struct input_dev *dev, int count);
void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count);
+void input_mt_sync_frame(struct input_dev *dev);
+
#endif
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 09/19] Input: MT - Handle frame synchronization in core
2012-08-12 21:42 ` [PATCH 09/19] Input: MT - Handle frame synchronization in core Henrik Rydberg
@ 2012-08-15 23:28 ` Ping Cheng
2012-08-16 18:07 ` Henrik Rydberg
2012-08-20 13:36 ` Benjamin Tissoires
1 sibling, 1 reply; 45+ messages in thread
From: Ping Cheng @ 2012-08-15 23:28 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Collect common frame synchronization tasks in a new function,
> input_mt_sync_frame(). Depending on the flags set, it drops
> unseen contacts and performs pointer emulation.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
I went through the patchset except those for bcm5974. Since there are
changes that affect other drivers, do you plan to update the affected
drivers as well?
I have a minor question below inline. You can add my Reviewed-by tag
after updating commit comments for 02/19.
Thank you for your effort.
Ping
> ---
> drivers/input/input-mt.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++--
> include/linux/input/mt.h | 9 ++++++
> 2 files changed, 81 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index bbb3304..21b105e 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -14,6 +14,14 @@
>
> #define TRKID_SGN ((TRKID_MAX + 1) >> 1)
>
> +static void copy_abs(struct input_dev *dev, unsigned int dst, unsigned int src)
> +{
> + if (dev->absinfo && test_bit(src, dev->absbit)) {
> + dev->absinfo[dst] = dev->absinfo[src];
> + dev->absbit[BIT_WORD(dst)] |= BIT_MASK(dst);
> + }
> +}
> +
> /**
> * input_mt_init_slots() - initialize MT input slots
> * @dev: input device supporting MT events and finger tracking
> @@ -45,6 +53,28 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
> input_set_abs_params(dev, ABS_MT_SLOT, 0, num_slots - 1, 0, 0);
> input_set_abs_params(dev, ABS_MT_TRACKING_ID, 0, TRKID_MAX, 0, 0);
>
> + if (flags & (INPUT_MT_POINTER | INPUT_MT_DIRECT)) {
> + __set_bit(EV_KEY, dev->evbit);
> + __set_bit(BTN_TOUCH, dev->keybit);
> +
> + copy_abs(dev, ABS_X, ABS_MT_POSITION_X);
> + copy_abs(dev, ABS_Y, ABS_MT_POSITION_Y);
> + copy_abs(dev, ABS_PRESSURE, ABS_MT_PRESSURE);
> + }
> + if (flags & INPUT_MT_POINTER) {
> + __set_bit(BTN_TOOL_FINGER, dev->keybit);
> + __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
> + if (num_slots >= 3)
> + __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
> + if (num_slots >= 4)
> + __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> + if (num_slots >= 5)
> + __set_bit(BTN_TOOL_QUINTTAP, dev->keybit);
> + __set_bit(INPUT_PROP_POINTER, dev->propbit);
> + }
> + if (flags & INPUT_MT_DIRECT)
> + __set_bit(INPUT_PROP_DIRECT, dev->propbit);
> +
> /* Mark slots as 'unused' */
> for (i = 0; i < num_slots; i++)
> input_mt_set_value(&mt->slots[i], ABS_MT_TRACKING_ID, -1);
> @@ -87,12 +117,17 @@ void input_mt_report_slot_state(struct input_dev *dev,
> struct input_mt_slot *slot;
> int id;
>
> - if (!mt || !active) {
> + if (!mt)
> + return;
> +
> + slot = &mt->slots[input_abs_get_val(dev, ABS_MT_SLOT)];
> + slot->frame = mt->frame;
> +
> + if (!active) {
> input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> return;
> }
>
> - slot = &mt->slots[input_abs_get_val(dev, ABS_MT_SLOT)];
> id = input_mt_get_value(slot, ABS_MT_TRACKING_ID);
> if (id < 0 || input_mt_get_value(slot, ABS_MT_TOOL_TYPE) != tool_type)
> id = input_mt_new_trkid(mt);
> @@ -177,3 +212,38 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
> }
> }
> EXPORT_SYMBOL(input_mt_report_pointer_emulation);
> +
> +/**
> + * input_mt_sync_frame() - synchronize mt frame
> + * @dev: input device with allocated MT slots
> + *
> + * Close the frame and prepare the internal state for a new one.
> + * Depending on the flags, marks unused slots as inactive and performs
> + * pointer emulation.
> + */
> +void input_mt_sync_frame(struct input_dev *dev)
> +{
> + struct input_mt *mt = dev->mt;
> + struct input_mt_slot *s;
> +
> + if (!mt)
> + return;
> +
> + if (mt->flags & INPUT_MT_DROP_UNUSED) {
> + for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
> + if (s->frame == mt->frame)
> + continue;
> + input_mt_slot(dev, s - mt->slots);
> + input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + }
> + }
> +
> + if (mt->flags & INPUT_MT_POINTER)
> + input_mt_report_pointer_emulation(dev, true);
> +
> + if (mt->flags & INPUT_MT_DIRECT)
> + input_mt_report_pointer_emulation(dev, false);
> +
> + mt->frame++;
Where do we reset this frame counter?
> +}
> +EXPORT_SYMBOL(input_mt_sync_frame);
> diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
> index 03c52ce..a11d485 100644
> --- a/include/linux/input/mt.h
> +++ b/include/linux/input/mt.h
> @@ -15,12 +15,17 @@
>
> #define TRKID_MAX 0xffff
>
> +#define INPUT_MT_POINTER 0x0001 /* pointer device, e.g. trackpad */
> +#define INPUT_MT_DIRECT 0x0002 /* direct device, e.g. touchscreen */
> +#define INPUT_MT_DROP_UNUSED 0x0004 /* drop contacts not seen in frame */
>
> /**
> * struct input_mt_slot - represents the state of an input MT slot
> * @abs: holds current values of ABS_MT axes for this slot
> + * @frame: last frame at which input_mt_report_slot_state() was called
> */
> struct input_mt_slot {
> int abs[ABS_MT_LAST - ABS_MT_FIRST + 1];
> + unsigned int frame;
> };
>
> /**
> @@ -28,12 +33,14 @@ struct input_mt_slot {
> * @trkid: stores MT tracking ID for the next contact
> * @num_slots: number of MT slots the device uses
> * @flags: input_mt operation flags
> + * @frame: increases every time input_mt_sync_frame() is called
> * @slots: array of slots holding current values of tracked contacts
> */
> struct input_mt {
> int trkid;
> int num_slots;
> unsigned int flags;
> + unsigned int frame;
> struct input_mt_slot slots[];
> };
>
> @@ -79,4 +86,6 @@ void input_mt_report_slot_state(struct input_dev *dev,
> void input_mt_report_finger_count(struct input_dev *dev, int count);
> void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count);
>
> +void input_mt_sync_frame(struct input_dev *dev);
> +
> #endif
> --
> 1.7.11.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 [flat|nested] 45+ messages in thread* Re: [PATCH 09/19] Input: MT - Handle frame synchronization in core
2012-08-15 23:28 ` Ping Cheng
@ 2012-08-16 18:07 ` Henrik Rydberg
2012-08-16 19:22 ` Ping Cheng
2012-08-16 19:58 ` Ping Cheng
0 siblings, 2 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-16 18:07 UTC (permalink / raw)
To: Ping Cheng; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
On Wed, Aug 15, 2012 at 04:28:17PM -0700, Ping Cheng wrote:
> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> > Collect common frame synchronization tasks in a new function,
> > input_mt_sync_frame(). Depending on the flags set, it drops
> > unseen contacts and performs pointer emulation.
> >
> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
>
> I went through the patchset except those for bcm5974. Since there are
> changes that affect other drivers, do you plan to update the affected
> drivers as well?
I am not sure what you mean here? There is a patch with in-kernel api
changes, which also changes all drivers using the api. Some of those
drivers will benefit from further changes, but that is a different
story.
> > +void input_mt_sync_frame(struct input_dev *dev)
> > +{
> > + struct input_mt *mt = dev->mt;
> > + struct input_mt_slot *s;
> > +
> > + if (!mt)
> > + return;
> > +
> > + if (mt->flags & INPUT_MT_DROP_UNUSED) {
> > + for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
> > + if (s->frame == mt->frame)
> > + continue;
> > + input_mt_slot(dev, s - mt->slots);
> > + input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> > + }
> > + }
> > +
> > + if (mt->flags & INPUT_MT_POINTER)
> > + input_mt_report_pointer_emulation(dev, true);
> > +
> > + if (mt->flags & INPUT_MT_DIRECT)
> > + input_mt_report_pointer_emulation(dev, false);
> > +
> > + mt->frame++;
>
> Where do we reset this frame counter?
Why would we reset it? It is used in the core to keep track of changes
per frame, and may wrap around without issues.
Henrik
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 09/19] Input: MT - Handle frame synchronization in core
2012-08-16 18:07 ` Henrik Rydberg
@ 2012-08-16 19:22 ` Ping Cheng
2012-08-16 20:05 ` Henrik Rydberg
2012-08-16 19:58 ` Ping Cheng
1 sibling, 1 reply; 45+ messages in thread
From: Ping Cheng @ 2012-08-16 19:22 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
On Thu, Aug 16, 2012 at 11:07 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> On Wed, Aug 15, 2012 at 04:28:17PM -0700, Ping Cheng wrote:
>> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> > Collect common frame synchronization tasks in a new function,
>> > input_mt_sync_frame(). Depending on the flags set, it drops
>> > unseen contacts and performs pointer emulation.
>> >
>> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
>>
>> I went through the patchset except those for bcm5974. Since there are
>> changes that affect other drivers, do you plan to update the affected
>> drivers as well?
>
> I am not sure what you mean here? There is a patch with in-kernel api
> changes, which also changes all drivers using the api. Some of those
> drivers will benefit from further changes, but that is a different
> story.
I meant that some new routines require individual MT drivers to be
updated to adapt to the new implementation.
For example, the new input_mt_init_slots() takes care of the
__set_bit() functions in one place. That is great. But, it requires
wacom_wac.c to be updated since it has an interface change. I guess
there are other drivers calling input_mt_init_slots as well.
I was wondering if you plan to update all drivers after this patchset
is accepted or if you need us to chime in.
>> > +void input_mt_sync_frame(struct input_dev *dev)
>> > +{
>> > + struct input_mt *mt = dev->mt;
>> > + struct input_mt_slot *s;
>> > +
>> > + if (!mt)
>> > + return;
>> > +
>> > + if (mt->flags & INPUT_MT_DROP_UNUSED) {
>> > + for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
>> > + if (s->frame == mt->frame)
>> > + continue;
>> > + input_mt_slot(dev, s - mt->slots);
>> > + input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
>> > + }
>> > + }
>> > +
>> > + if (mt->flags & INPUT_MT_POINTER)
>> > + input_mt_report_pointer_emulation(dev, true);
>> > +
>> > + if (mt->flags & INPUT_MT_DIRECT)
>> > + input_mt_report_pointer_emulation(dev, false);
>> > +
>> > + mt->frame++;
>>
>> Where do we reset this frame counter?
>
> Why would we reset it? It is used in the core to keep track of changes
> per frame, and may wrap around without issues.
>From what I see, frame/mt is only initialized when driver starts.
Frame will be increased by MT events while the driver running. If this
is true, won't it be possible the value gets too large?
I might have missed a detail about frame somewhere in the patchset.
Ping
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 09/19] Input: MT - Handle frame synchronization in core
2012-08-16 19:22 ` Ping Cheng
@ 2012-08-16 20:05 ` Henrik Rydberg
0 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-16 20:05 UTC (permalink / raw)
To: Ping Cheng; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
> For example, the new input_mt_init_slots() takes care of the
> __set_bit() functions in one place. That is great. But, it requires
> wacom_wac.c to be updated since it has an interface change. I guess
> there are other drivers calling input_mt_init_slots as well.
The default behavior in the "new" input_mt_init_slots(), i.e. with the
zero flags argument added to all the drivers, is exactly the same as
before. No further changes are necessary.
Yes, some drivers could definitely start using some of the flags to
good advantage, but there is no rush to make that happen. Such changes
would of course involve the driver maintainers.
> I was wondering if you plan to update all drivers after this patchset
> is accepted or if you need us to chime in.
Thank you, the best is definitely if you want to do it yourself. Let's
just see where the latency-related work takes us first, since it might
affect how to send data from drivers.
> From what I see, frame/mt is only initialized when driver starts.
> Frame will be increased by MT events while the driver running. If this
> is true, won't it be possible the value gets too large?
Yes, but as is apparent from the code, the semantics of that variable
is not to count the number of frames since the dawn of times, but to
keep track of changes per frame.
Thanks,
Henrik
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 09/19] Input: MT - Handle frame synchronization in core
2012-08-16 18:07 ` Henrik Rydberg
2012-08-16 19:22 ` Ping Cheng
@ 2012-08-16 19:58 ` Ping Cheng
1 sibling, 0 replies; 45+ messages in thread
From: Ping Cheng @ 2012-08-16 19:58 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
On Thu, Aug 16, 2012 at 11:07 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> On Wed, Aug 15, 2012 at 04:28:17PM -0700, Ping Cheng wrote:
>> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> > Collect common frame synchronization tasks in a new function,
>> > input_mt_sync_frame(). Depending on the flags set, it drops
>> > unseen contacts and performs pointer emulation.
>> >
>> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
>>
>> I went through the patchset except those for bcm5974. Since there are
>> changes that affect other drivers, do you plan to update the affected
>> drivers as well?
>
> I am not sure what you mean here? There is a patch with in-kernel api
> changes, which also changes all drivers using the api. Some of those
> drivers will benefit from further changes, but that is a different
> story.
Just to clarify a little bit more. Patch 08/19 was only a generic
update. A complete update should introduce INPUT_MT_POINTER and
INPUT_MT_DIRECT so the drivers can fully utilize the new feature.
Now I understand you expect us to update the "different story".
Thanks.
Ping
Ping
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 09/19] Input: MT - Handle frame synchronization in core
2012-08-12 21:42 ` [PATCH 09/19] Input: MT - Handle frame synchronization in core Henrik Rydberg
2012-08-15 23:28 ` Ping Cheng
@ 2012-08-20 13:36 ` Benjamin Tissoires
2012-08-20 15:53 ` Henrik Rydberg
1 sibling, 1 reply; 45+ messages in thread
From: Benjamin Tissoires @ 2012-08-20 13:36 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
Hi Henrik,
thanks for this patch set.
I'm also happy when someone tries to factorize and optimize some code.
I have a few concerns though:
On Sun, Aug 12, 2012 at 11:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Collect common frame synchronization tasks in a new function,
> input_mt_sync_frame(). Depending on the flags set, it drops
> unseen contacts and performs pointer emulation.
I was really wondering why you needed to put in input-mt something
that appeared only in hid-multitouch.... until I noted that you are
going to use it for bcm5974.
Maybe you should add a comment on it (otherwise, it seams like you're
just adding unused code). Maybe this would also help people
understanding the *frame thing.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> drivers/input/input-mt.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++--
> include/linux/input/mt.h | 9 ++++++
> 2 files changed, 81 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index bbb3304..21b105e 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -14,6 +14,14 @@
>
> #define TRKID_SGN ((TRKID_MAX + 1) >> 1)
>
> +static void copy_abs(struct input_dev *dev, unsigned int dst, unsigned int src)
> +{
> + if (dev->absinfo && test_bit(src, dev->absbit)) {
> + dev->absinfo[dst] = dev->absinfo[src];
> + dev->absbit[BIT_WORD(dst)] |= BIT_MASK(dst);
> + }
> +}
> +
> /**
> * input_mt_init_slots() - initialize MT input slots
> * @dev: input device supporting MT events and finger tracking
> @@ -45,6 +53,28 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
> input_set_abs_params(dev, ABS_MT_SLOT, 0, num_slots - 1, 0, 0);
> input_set_abs_params(dev, ABS_MT_TRACKING_ID, 0, TRKID_MAX, 0, 0);
>
> + if (flags & (INPUT_MT_POINTER | INPUT_MT_DIRECT)) {
> + __set_bit(EV_KEY, dev->evbit);
> + __set_bit(BTN_TOUCH, dev->keybit);
> +
> + copy_abs(dev, ABS_X, ABS_MT_POSITION_X);
> + copy_abs(dev, ABS_Y, ABS_MT_POSITION_Y);
> + copy_abs(dev, ABS_PRESSURE, ABS_MT_PRESSURE);
> + }
> + if (flags & INPUT_MT_POINTER) {
> + __set_bit(BTN_TOOL_FINGER, dev->keybit);
> + __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
> + if (num_slots >= 3)
> + __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
> + if (num_slots >= 4)
> + __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> + if (num_slots >= 5)
> + __set_bit(BTN_TOOL_QUINTTAP, dev->keybit);
> + __set_bit(INPUT_PROP_POINTER, dev->propbit);
> + }
> + if (flags & INPUT_MT_DIRECT)
> + __set_bit(INPUT_PROP_DIRECT, dev->propbit);
> +
> /* Mark slots as 'unused' */
> for (i = 0; i < num_slots; i++)
> input_mt_set_value(&mt->slots[i], ABS_MT_TRACKING_ID, -1);
> @@ -87,12 +117,17 @@ void input_mt_report_slot_state(struct input_dev *dev,
> struct input_mt_slot *slot;
> int id;
>
> - if (!mt || !active) {
> + if (!mt)
> + return;
> +
> + slot = &mt->slots[input_abs_get_val(dev, ABS_MT_SLOT)];
> + slot->frame = mt->frame;
> +
> + if (!active) {
> input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> return;
> }
>
> - slot = &mt->slots[input_abs_get_val(dev, ABS_MT_SLOT)];
> id = input_mt_get_value(slot, ABS_MT_TRACKING_ID);
> if (id < 0 || input_mt_get_value(slot, ABS_MT_TOOL_TYPE) != tool_type)
> id = input_mt_new_trkid(mt);
> @@ -177,3 +212,38 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
> }
> }
> EXPORT_SYMBOL(input_mt_report_pointer_emulation);
> +
> +/**
> + * input_mt_sync_frame() - synchronize mt frame
> + * @dev: input device with allocated MT slots
> + *
> + * Close the frame and prepare the internal state for a new one.
> + * Depending on the flags, marks unused slots as inactive and performs
> + * pointer emulation.
> + */
> +void input_mt_sync_frame(struct input_dev *dev)
> +{
> + struct input_mt *mt = dev->mt;
> + struct input_mt_slot *s;
> +
> + if (!mt)
> + return;
> +
> + if (mt->flags & INPUT_MT_DROP_UNUSED) {
> + for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
> + if (s->frame == mt->frame)
> + continue;
> + input_mt_slot(dev, s - mt->slots);
> + input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
Shouldn't we rely on input_mt_report_slot_state instead of doing it by hand?
> + }
> + }
> +
> + if (mt->flags & INPUT_MT_POINTER)
> + input_mt_report_pointer_emulation(dev, true);
> +
> + if (mt->flags & INPUT_MT_DIRECT)
> + input_mt_report_pointer_emulation(dev, false);
The function input_mt_report_pointer_emulation could be called twice
if the driver has both INPUT_MT_POINTER and INPUT_MT_DIRECT flags. Are
they mutual exclusive?
Cheers,
Benjamin
> +
> + mt->frame++;
> +}
> +EXPORT_SYMBOL(input_mt_sync_frame);
> diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
> index 03c52ce..a11d485 100644
> --- a/include/linux/input/mt.h
> +++ b/include/linux/input/mt.h
> @@ -15,12 +15,17 @@
>
> #define TRKID_MAX 0xffff
>
> +#define INPUT_MT_POINTER 0x0001 /* pointer device, e.g. trackpad */
> +#define INPUT_MT_DIRECT 0x0002 /* direct device, e.g. touchscreen */
> +#define INPUT_MT_DROP_UNUSED 0x0004 /* drop contacts not seen in frame */
> /**
> * struct input_mt_slot - represents the state of an input MT slot
> * @abs: holds current values of ABS_MT axes for this slot
> + * @frame: last frame at which input_mt_report_slot_state() was called
> */
> struct input_mt_slot {
> int abs[ABS_MT_LAST - ABS_MT_FIRST + 1];
> + unsigned int frame;
> };
>
> /**
> @@ -28,12 +33,14 @@ struct input_mt_slot {
> * @trkid: stores MT tracking ID for the next contact
> * @num_slots: number of MT slots the device uses
> * @flags: input_mt operation flags
> + * @frame: increases every time input_mt_sync_frame() is called
> * @slots: array of slots holding current values of tracked contacts
> */
> struct input_mt {
> int trkid;
> int num_slots;
> unsigned int flags;
> + unsigned int frame;
> struct input_mt_slot slots[];
> };
>
> @@ -79,4 +86,6 @@ void input_mt_report_slot_state(struct input_dev *dev,
> void input_mt_report_finger_count(struct input_dev *dev, int count);
> void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count);
>
> +void input_mt_sync_frame(struct input_dev *dev);
> +
> #endif
> --
> 1.7.11.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 [flat|nested] 45+ messages in thread* Re: [PATCH 09/19] Input: MT - Handle frame synchronization in core
2012-08-20 13:36 ` Benjamin Tissoires
@ 2012-08-20 15:53 ` Henrik Rydberg
0 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-20 15:53 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
> > Collect common frame synchronization tasks in a new function,
> > input_mt_sync_frame(). Depending on the flags set, it drops
> > unseen contacts and performs pointer emulation.
>
> I was really wondering why you needed to put in input-mt something
> that appeared only in hid-multitouch.... until I noted that you are
> going to use it for bcm5974.
True, you were only copied in on the patch specific to
hid-multitouch. The core changes will naturally be used for some other
drivers as well.
> Maybe you should add a comment on it (otherwise, it seams like you're
> just adding unused code). Maybe this would also help people
> understanding the *frame thing.
More comments on those plans, agreed. The "frame thing" is really only
an input core change; it can most likely be better explained as well,
but it really should not matter to drivers.
> > +void input_mt_sync_frame(struct input_dev *dev)
> > +{
> > + struct input_mt *mt = dev->mt;
> > + struct input_mt_slot *s;
> > +
> > + if (!mt)
> > + return;
> > +
> > + if (mt->flags & INPUT_MT_DROP_UNUSED) {
> > + for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
> > + if (s->frame == mt->frame)
> > + continue;
> > + input_mt_slot(dev, s - mt->slots);
> > + input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
>
> Shouldn't we rely on input_mt_report_slot_state instead of doing it by hand?
No, input_mt_report_slot_state() is a driver api function with side
effects which are not desired here.
> > + }
> > + }
> > +
> > + if (mt->flags & INPUT_MT_POINTER)
> > + input_mt_report_pointer_emulation(dev, true);
> > +
> > + if (mt->flags & INPUT_MT_DIRECT)
> > + input_mt_report_pointer_emulation(dev, false);
>
> The function input_mt_report_pointer_emulation could be called twice
> if the driver has both INPUT_MT_POINTER and INPUT_MT_DIRECT flags. Are
> they mutual exclusive?
You are right, and they are not. Will fix.
Thanks,
Henrik
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 10/19] Input: MT - Add in-kernel tracking
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (8 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 09/19] Input: MT - Handle frame synchronization in core Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 11/19] Input: MT - Add slot assignment by id Henrik Rydberg
` (8 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
With the INPUT_MT_TRACK flag set, the function input_mt_assign_slots()
can be used to match a new set of contacts against the currently used
slots. The algorithm used is based on Lagrange relaxation, and performs
very well in practice; slower than mtdev for a few corner cases, but
faster in most commonly occuring cases.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/input-mt.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/input/mt.h | 21 +++++++
2 files changed, 163 insertions(+), 2 deletions(-)
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 21b105e..36c93d7 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -46,7 +46,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
mt = kzalloc(sizeof(*mt) + num_slots * sizeof(*mt->slots), GFP_KERNEL);
if (!mt)
- return -ENOMEM;
+ goto err_mem;
mt->num_slots = num_slots;
mt->flags = flags;
@@ -74,6 +74,12 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
}
if (flags & INPUT_MT_DIRECT)
__set_bit(INPUT_PROP_DIRECT, dev->propbit);
+ if (flags & INPUT_MT_TRACK) {
+ unsigned int n2 = num_slots * num_slots;
+ mt->red = kcalloc(n2, sizeof(*mt->red), GFP_KERNEL);
+ if (!mt->red)
+ goto err_mem;
+ }
/* Mark slots as 'unused' */
for (i = 0; i < num_slots; i++)
@@ -81,6 +87,9 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
dev->mt = mt;
return 0;
+err_mem:
+ kfree(mt);
+ return -ENOMEM;
}
EXPORT_SYMBOL(input_mt_init_slots);
@@ -93,7 +102,10 @@ EXPORT_SYMBOL(input_mt_init_slots);
*/
void input_mt_destroy_slots(struct input_dev *dev)
{
- kfree(dev->mt);
+ if (dev->mt) {
+ kfree(dev->mt->red);
+ kfree(dev->mt);
+ }
dev->mt = NULL;
}
EXPORT_SYMBOL(input_mt_destroy_slots);
@@ -247,3 +259,131 @@ void input_mt_sync_frame(struct input_dev *dev)
mt->frame++;
}
EXPORT_SYMBOL(input_mt_sync_frame);
+
+static int adjust_dual(int *begin, int step, int *end, int eq)
+{
+ int f, *p, s, c;
+
+ if (begin == end)
+ return 0;
+
+ f = *begin;
+ p = begin + step;
+ s = p == end ? f + 1 : *p;
+
+ for (; p != end; p += step)
+ if (*p < f)
+ s = f, f = *p;
+ else if (*p < s)
+ s = *p;
+
+ c = (f + s + 1) / 2;
+ if (c == 0 || (c > 0 && !eq))
+ return 0;
+ if (s < 0)
+ c *= 2;
+
+ for (p = begin; p != end; p += step)
+ *p -= c;
+
+ return (c < s && s <= 0) || (f >= 0 && f < c);
+}
+
+static void find_reduced_matrix(int *w, int nr, int nc, int nrc)
+{
+ int i, k, sum;
+
+ for (k = 0; k < nrc; k++) {
+ for (i = 0; i < nr; i++)
+ adjust_dual(w + i, nr, w + i + nrc, nr <= nc);
+ sum = 0;
+ for (i = 0; i < nrc; i += nr)
+ sum += adjust_dual(w + i, 1, w + i + nr, nc <= nr);
+ if (!sum)
+ break;
+ }
+}
+
+static int input_mt_set_matrix(struct input_mt *mt,
+ const struct input_mt_pos *pos, int num_pos)
+{
+ const struct input_mt_pos *p;
+ struct input_mt_slot *s;
+ int *w = mt->red;
+ int x, y;
+
+ for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
+ if (!input_mt_is_active(s))
+ continue;
+ x = input_mt_get_value(s, ABS_MT_POSITION_X);
+ y = input_mt_get_value(s, ABS_MT_POSITION_Y);
+ for (p = pos; p != pos + num_pos; p++) {
+ int dx = x - p->x, dy = y - p->y;
+ *w++ = dx * dx + dy * dy;
+ }
+ }
+
+ return w - mt->red;
+}
+
+static void input_mt_set_slots(struct input_mt *mt,
+ int *slots, int num_pos)
+{
+ struct input_mt_slot *s;
+ int *w = mt->red, *p;
+
+ for (p = slots; p != slots + num_pos; p++)
+ *p = -1;
+
+ for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
+ if (!input_mt_is_active(s))
+ continue;
+ for (p = slots; p != slots + num_pos; p++)
+ if (*w++ < 0)
+ *p = s - mt->slots;
+ }
+
+ for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
+ if (input_mt_is_active(s))
+ continue;
+ for (p = slots; p != slots + num_pos; p++)
+ if (*p < 0) {
+ *p = s - mt->slots;
+ break;
+ }
+ }
+}
+
+/**
+ * input_mt_assign_slots() - perform a best-match assignment
+ * @dev: input device with allocated MT slots
+ * @slots: the slot assignment to be filled
+ * @pos: the position array to match
+ * @num_pos: number of positions
+ *
+ * Performs a best match against the current contacts and returns
+ * the slot assignment list. New contacts are assigned to unused
+ * slots.
+ *
+ * Returns zero on success, or negative error in case of failure.
+ */
+int input_mt_assign_slots(struct input_dev *dev, int *slots,
+ const struct input_mt_pos *pos, int num_pos)
+{
+ struct input_mt *mt = dev->mt;
+ int nrc;
+
+ if (!mt || !mt->red)
+ return -ENXIO;
+ if (num_pos > mt->num_slots)
+ return -EINVAL;
+ if (num_pos < 1)
+ return 0;
+
+ nrc = input_mt_set_matrix(mt, pos, num_pos);
+ find_reduced_matrix(mt->red, num_pos, nrc / num_pos, nrc);
+ input_mt_set_slots(mt, slots, num_pos);
+
+ return 0;
+}
+EXPORT_SYMBOL(input_mt_assign_slots);
diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
index a11d485..10bb77c 100644
--- a/include/linux/input/mt.h
+++ b/include/linux/input/mt.h
@@ -18,6 +18,8 @@
#define INPUT_MT_POINTER 0x0001 /* pointer device, e.g. trackpad */
#define INPUT_MT_DIRECT 0x0002 /* direct device, e.g. touchscreen */
#define INPUT_MT_DROP_UNUSED 0x0004 /* drop contacts not seen in frame */
+#define INPUT_MT_TRACK 0x0008 /* use in-kernel tracking */
+
/**
* struct input_mt_slot - represents the state of an input MT slot
* @abs: holds current values of ABS_MT axes for this slot
@@ -34,6 +36,7 @@ struct input_mt_slot {
* @num_slots: number of MT slots the device uses
* @flags: input_mt operation flags
* @frame: increases every time input_mt_sync_frame() is called
+ * @red: reduced cost matrix for in-kernel tracking
* @slots: array of slots holding current values of tracked contacts
*/
struct input_mt {
@@ -41,6 +44,7 @@ struct input_mt {
int num_slots;
unsigned int flags;
unsigned int frame;
+ int *red;
struct input_mt_slot slots[];
};
@@ -56,6 +60,11 @@ static inline int input_mt_get_value(const struct input_mt_slot *slot,
return slot->abs[code - ABS_MT_FIRST];
}
+static inline bool input_mt_is_active(const struct input_mt_slot *slot)
+{
+ return input_mt_get_value(slot, ABS_MT_TRACKING_ID) >= 0;
+}
+
int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
unsigned int flags);
void input_mt_destroy_slots(struct input_dev *dev);
@@ -88,4 +97,16 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count);
void input_mt_sync_frame(struct input_dev *dev);
+/**
+ * struct input_mt_pos - contact position
+ * @x: horizontal coordinate
+ * @y: vertical coordinate
+ */
+struct input_mt_pos {
+ s16 x, y;
+};
+
+int input_mt_assign_slots(struct input_dev *dev, int *slots,
+ const struct input_mt_pos *pos, int num_pos);
+
#endif
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 11/19] Input: MT - Add slot assignment by id
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (9 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 10/19] Input: MT - Add in-kernel tracking Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 12/19] Input: bcm5974 - Preparatory renames Henrik Rydberg
` (7 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Some drivers produce their own tracking ids, which needs to be mapped
to slots. This patch provides that function.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/input-mt.c | 30 ++++++++++++++++++++++++++++++
include/linux/input/mt.h | 2 ++
2 files changed, 32 insertions(+)
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 36c93d7..1f3e110 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -387,3 +387,33 @@ int input_mt_assign_slots(struct input_dev *dev, int *slots,
return 0;
}
EXPORT_SYMBOL(input_mt_assign_slots);
+
+/**
+ * input_mt_assign_slot_by_id() - return matching slot
+ * @dev: input device with allocated MT slots
+ * @id: the sought tracking id
+ *
+ * Returns the slot of the given tracking id, if it exists. Otherwise,
+ * the first unused slot is returned.
+ *
+ * If no available slot can be found, -1 is returned.
+ */
+int input_mt_assign_slot_by_id(struct input_dev *dev, int id)
+{
+ struct input_mt *mt = dev->mt;
+ struct input_mt_slot *s;
+
+ if (!mt)
+ return -1;
+
+ for (s = mt->slots; s != mt->slots + mt->num_slots; s++)
+ if (input_mt_get_value(s, ABS_MT_TRACKING_ID) == id)
+ return s - mt->slots;
+
+ for (s = mt->slots; s != mt->slots + mt->num_slots; s++)
+ if (!input_mt_is_active(s))
+ return s - mt->slots;
+
+ return -1;
+}
+EXPORT_SYMBOL(input_mt_assign_slot_by_id);
diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
index 10bb77c..54b640b 100644
--- a/include/linux/input/mt.h
+++ b/include/linux/input/mt.h
@@ -109,4 +109,6 @@ struct input_mt_pos {
int input_mt_assign_slots(struct input_dev *dev, int *slots,
const struct input_mt_pos *pos, int num_pos);
+int input_mt_assign_slot_by_id(struct input_dev *dev, int id);
+
#endif
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 12/19] Input: bcm5974 - Preparatory renames
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (10 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 11/19] Input: MT - Add slot assignment by id Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 13/19] Input: bcm5974 - Drop pressure and width emulation Henrik Rydberg
` (6 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Rename touch properties to match established nomenclature, and define
the maximum number of fingers.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/mouse/bcm5974.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index 8234098..2d6468f 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -183,18 +183,19 @@ struct tp_finger {
__le16 abs_y; /* absolute y coodinate */
__le16 rel_x; /* relative x coodinate */
__le16 rel_y; /* relative y coodinate */
- __le16 size_major; /* finger size, major axis? */
- __le16 size_minor; /* finger size, minor axis? */
+ __le16 tool_major; /* tool area, major axis */
+ __le16 tool_minor; /* tool area, minor axis */
__le16 orientation; /* 16384 when point, else 15 bit angle */
- __le16 force_major; /* trackpad force, major axis? */
- __le16 force_minor; /* trackpad force, minor axis? */
+ __le16 touch_major; /* touch area, major axis */
+ __le16 touch_minor; /* touch area, minor axis */
__le16 unused[3]; /* zeros */
__le16 multi; /* one finger: varies, more fingers: constant */
} __attribute__((packed,aligned(2)));
/* trackpad finger data size, empirically at least ten fingers */
+#define MAX_FINGERS 16
#define SIZEOF_FINGER sizeof(struct tp_finger)
-#define SIZEOF_ALL_FINGERS (16 * SIZEOF_FINGER)
+#define SIZEOF_ALL_FINGERS (MAX_FINGERS * SIZEOF_FINGER)
#define MAX_FINGER_ORIENTATION 16384
/* device-specific parameters */
@@ -480,13 +481,13 @@ static void report_finger_data(struct input_dev *input,
const struct tp_finger *f)
{
input_report_abs(input, ABS_MT_TOUCH_MAJOR,
- raw2int(f->force_major) << 1);
+ raw2int(f->touch_major) << 1);
input_report_abs(input, ABS_MT_TOUCH_MINOR,
- raw2int(f->force_minor) << 1);
+ raw2int(f->touch_minor) << 1);
input_report_abs(input, ABS_MT_WIDTH_MAJOR,
- raw2int(f->size_major) << 1);
+ raw2int(f->tool_major) << 1);
input_report_abs(input, ABS_MT_WIDTH_MINOR,
- raw2int(f->size_minor) << 1);
+ raw2int(f->tool_minor) << 1);
input_report_abs(input, ABS_MT_ORIENTATION,
MAX_FINGER_ORIENTATION - raw2int(f->orientation));
input_report_abs(input, ABS_MT_POSITION_X, raw2int(f->abs_x));
@@ -519,8 +520,8 @@ static int report_tp_state(struct bcm5974 *dev, int size)
for (i = 0; i < raw_n; i++)
report_finger_data(input, c, &f[i]);
- raw_p = raw2int(f->force_major);
- raw_w = raw2int(f->size_major);
+ raw_p = raw2int(f->touch_major);
+ raw_w = raw2int(f->tool_major);
raw_x = raw2int(f->abs_x);
raw_y = raw2int(f->abs_y);
@@ -540,7 +541,7 @@ static int report_tp_state(struct bcm5974 *dev, int size)
abs_y = int2bound(&c->y, c->y.devmax - raw_y);
while (raw_n--) {
ptest = int2bound(&c->p,
- raw2int(f->force_major));
+ raw2int(f->touch_major));
if (ptest > PRESSURE_LOW)
nmax++;
if (ptest > PRESSURE_HIGH)
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 13/19] Input: bcm5974 - Drop pressure and width emulation
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (11 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 12/19] Input: bcm5974 - Preparatory renames Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 14/19] Input: bcm5974 - Drop the logical dimensions Henrik Rydberg
` (5 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
The ABS_PRESSURE and ABS_WIDTH have special scales, and were initially
added solely for thumb and palm recognition in the synaptics driver.
This never really get used, however, and userspace quickly moved to
MT solutions instead. This patch drops the unused events.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/mouse/bcm5974.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index 2d6468f..8443872 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -417,10 +417,6 @@ static void setup_events_to_report(struct input_dev *input_dev,
{
__set_bit(EV_ABS, input_dev->evbit);
- input_set_abs_params(input_dev, ABS_PRESSURE,
- 0, cfg->p.dim, cfg->p.fuzz, 0);
- input_set_abs_params(input_dev, ABS_TOOL_WIDTH,
- 0, cfg->w.dim, cfg->w.fuzz, 0);
input_set_abs_params(input_dev, ABS_X,
0, cfg->x.dim, cfg->x.fuzz, 0);
input_set_abs_params(input_dev, ABS_Y,
@@ -502,9 +498,9 @@ static int report_tp_state(struct bcm5974 *dev, int size)
const struct bcm5974_config *c = &dev->cfg;
const struct tp_finger *f;
struct input_dev *input = dev->input;
- int raw_p, raw_w, raw_x, raw_y, raw_n, i;
+ int raw_p, raw_x, raw_y, raw_n, i;
int ptest, origin, ibt = 0, nmin = 0, nmax = 0;
- int abs_p = 0, abs_w = 0, abs_x = 0, abs_y = 0;
+ int abs_x = 0, abs_y = 0;
if (size < c->tp_offset || (size - c->tp_offset) % SIZEOF_FINGER != 0)
return -EIO;
@@ -521,22 +517,19 @@ static int report_tp_state(struct bcm5974 *dev, int size)
report_finger_data(input, c, &f[i]);
raw_p = raw2int(f->touch_major);
- raw_w = raw2int(f->tool_major);
raw_x = raw2int(f->abs_x);
raw_y = raw2int(f->abs_y);
dprintk(9,
"bcm5974: "
- "raw: p: %+05d w: %+05d x: %+05d y: %+05d n: %d\n",
- raw_p, raw_w, raw_x, raw_y, raw_n);
+ "raw: p: %+05d x: %+05d y: %+05d n: %d\n",
+ raw_p, raw_x, raw_y, raw_n);
ptest = int2bound(&c->p, raw_p);
origin = raw2int(f->origin);
/* while tracking finger still valid, count all fingers */
if (ptest > PRESSURE_LOW && origin) {
- abs_p = ptest;
- abs_w = int2bound(&c->w, raw_w);
abs_x = int2bound(&c->x, raw_x - c->x.devmin);
abs_y = int2bound(&c->y, c->y.devmax - raw_y);
while (raw_n--) {
@@ -566,18 +559,9 @@ static int report_tp_state(struct bcm5974 *dev, int size)
input_report_key(input, BTN_TOOL_TRIPLETAP, dev->fingers == 3);
input_report_key(input, BTN_TOOL_QUADTAP, dev->fingers > 3);
- input_report_abs(input, ABS_PRESSURE, abs_p);
- input_report_abs(input, ABS_TOOL_WIDTH, abs_w);
-
- if (abs_p) {
+ if (dev->fingers > 0) {
input_report_abs(input, ABS_X, abs_x);
input_report_abs(input, ABS_Y, abs_y);
-
- dprintk(8,
- "bcm5974: abs: p: %+05d w: %+05d x: %+05d y: %+05d "
- "nmin: %d nmax: %d n: %d ibt: %d\n", abs_p, abs_w,
- abs_x, abs_y, nmin, nmax, dev->fingers, ibt);
-
}
/* type 2 reports button events via ibt only */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 14/19] Input: bcm5974 - Drop the logical dimensions
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (12 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 13/19] Input: bcm5974 - Drop pressure and width emulation Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 15/19] Input: bcm5974 - Convert to MT-B Henrik Rydberg
` (4 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
The logical scale was previously used to produce special width values
to userspace, and its only present use is to put "pressure" hysteresis
on a common scale. However, bcm5974 trackpads are very accurate and
work well without hysteresis.
This patch simplifies the driver and device data by removing the logical
scale altogether. While at it, the fake pressure range is replaced by the
orientation range, which will be used in a subsequent patch.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/mouse/bcm5974.c | 193 ++++++++++++++++--------------------------
1 file changed, 75 insertions(+), 118 deletions(-)
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index 8443872..fb8ec52 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -200,10 +200,9 @@ struct tp_finger {
/* device-specific parameters */
struct bcm5974_param {
- int dim; /* logical dimension */
- int fuzz; /* logical noise value */
- int devmin; /* device minimum reading */
- int devmax; /* device maximum reading */
+ int snratio; /* signal-to-noise ratio */
+ int min; /* device minimum reading */
+ int max; /* device maximum reading */
};
/* device-specific configuration */
@@ -216,7 +215,7 @@ struct bcm5974_config {
enum tp_type tp_type; /* type of trackpad interface */
int tp_offset; /* offset to trackpad finger data */
int tp_datalen; /* data length of the trackpad interface */
- struct bcm5974_param p; /* finger pressure limits */
+ struct bcm5974_param o; /* orientation limits */
struct bcm5974_param w; /* finger width limits */
struct bcm5974_param x; /* horizontal limits */
struct bcm5974_param y; /* vertical limits */
@@ -235,24 +234,13 @@ struct bcm5974 {
struct bt_data *bt_data; /* button transferred data */
struct urb *tp_urb; /* trackpad usb request block */
u8 *tp_data; /* trackpad transferred data */
- int fingers; /* number of fingers on trackpad */
};
-/* logical dimensions */
-#define DIM_PRESSURE 256 /* maximum finger pressure */
-#define DIM_WIDTH 16 /* maximum finger width */
-#define DIM_X 1280 /* maximum trackpad x value */
-#define DIM_Y 800 /* maximum trackpad y value */
-
/* logical signal quality */
-#define SN_PRESSURE 45 /* pressure signal-to-noise ratio */
+#define SN_ORIENT 10 /* orientation signal-to-noise ratio */
#define SN_WIDTH 100 /* width signal-to-noise ratio */
#define SN_COORD 250 /* coordinate signal-to-noise ratio */
-/* pressure thresholds */
-#define PRESSURE_LOW (2 * DIM_PRESSURE / SN_PRESSURE)
-#define PRESSURE_HIGH (3 * PRESSURE_LOW)
-
/* device constants */
static const struct bcm5974_config bcm5974_config_table[] = {
{
@@ -262,10 +250,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
0,
0x84, sizeof(struct bt_data),
0x81, TYPE1, FINGER_TYPE1, FINGER_TYPE1 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 256 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4824, 5342 },
- { DIM_Y, DIM_Y / SN_COORD, -172, 5820 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4824, 5342 },
+ { SN_COORD, -172, 5820 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI,
@@ -274,10 +262,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
0,
0x84, sizeof(struct bt_data),
0x81, TYPE1, FINGER_TYPE1, FINGER_TYPE1 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 256 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4824, 4824 },
- { DIM_Y, DIM_Y / SN_COORD, -172, 4290 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4824, 4824 },
+ { SN_COORD, -172, 4290 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING3_ANSI,
@@ -286,10 +274,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
HAS_INTEGRATED_BUTTON,
0x84, sizeof(struct bt_data),
0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 300 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4460, 5166 },
- { DIM_Y, DIM_Y / SN_COORD, -75, 6700 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4460, 5166 },
+ { SN_COORD, -75, 6700 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI,
@@ -298,10 +286,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
HAS_INTEGRATED_BUTTON,
0x84, sizeof(struct bt_data),
0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 300 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4620, 5140 },
- { DIM_Y, DIM_Y / SN_COORD, -150, 6600 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4620, 5140 },
+ { SN_COORD, -150, 6600 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING4A_ANSI,
@@ -310,10 +298,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
HAS_INTEGRATED_BUTTON,
0x84, sizeof(struct bt_data),
0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 300 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4616, 5112 },
- { DIM_Y, DIM_Y / SN_COORD, -142, 5234 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4616, 5112 },
+ { SN_COORD, -142, 5234 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING5_ANSI,
@@ -322,10 +310,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
HAS_INTEGRATED_BUTTON,
0x84, sizeof(struct bt_data),
0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 300 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4415, 5050 },
- { DIM_Y, DIM_Y / SN_COORD, -55, 6680 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4415, 5050 },
+ { SN_COORD, -55, 6680 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING6_ANSI,
@@ -334,10 +322,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
HAS_INTEGRATED_BUTTON,
0x84, sizeof(struct bt_data),
0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 300 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4620, 5140 },
- { DIM_Y, DIM_Y / SN_COORD, -150, 6600 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4620, 5140 },
+ { SN_COORD, -150, 6600 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING5A_ANSI,
@@ -346,10 +334,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
HAS_INTEGRATED_BUTTON,
0x84, sizeof(struct bt_data),
0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 300 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4750, 5280 },
- { DIM_Y, DIM_Y / SN_COORD, -150, 6730 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4750, 5280 },
+ { SN_COORD, -150, 6730 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING6A_ANSI,
@@ -358,10 +346,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
HAS_INTEGRATED_BUTTON,
0x84, sizeof(struct bt_data),
0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 300 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4620, 5140 },
- { DIM_Y, DIM_Y / SN_COORD, -150, 6600 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4620, 5140 },
+ { SN_COORD, -150, 6600 }
},
{
USB_DEVICE_ID_APPLE_WELLSPRING7_ANSI,
@@ -370,10 +358,10 @@ static const struct bcm5974_config bcm5974_config_table[] = {
HAS_INTEGRATED_BUTTON,
0x84, sizeof(struct bt_data),
0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS,
- { DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 300 },
- { DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
- { DIM_X, DIM_X / SN_COORD, -4750, 5280 },
- { DIM_Y, DIM_Y / SN_COORD, -150, 6730 }
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -4750, 5280 },
+ { SN_COORD, -150, 6730 }
},
{}
};
@@ -397,18 +385,11 @@ static inline int raw2int(__le16 x)
return (signed short)le16_to_cpu(x);
}
-/* scale device data to logical dimensions (asserts devmin < devmax) */
-static inline int int2scale(const struct bcm5974_param *p, int x)
-{
- return x * p->dim / (p->devmax - p->devmin);
-}
-
-/* all logical value ranges are [0,dim). */
-static inline int int2bound(const struct bcm5974_param *p, int x)
+static void set_abs(struct input_dev *input, unsigned int code,
+ const struct bcm5974_param *p)
{
- int s = int2scale(p, x);
-
- return clamp_val(s, 0, p->dim - 1);
+ int fuzz = p->snratio ? (p->max - p->min) / p->snratio : 0;
+ input_set_abs_params(input, code, p->min, p->max, fuzz, 0);
}
/* setup which logical events to report */
@@ -417,30 +398,21 @@ static void setup_events_to_report(struct input_dev *input_dev,
{
__set_bit(EV_ABS, input_dev->evbit);
- input_set_abs_params(input_dev, ABS_X,
- 0, cfg->x.dim, cfg->x.fuzz, 0);
- input_set_abs_params(input_dev, ABS_Y,
- 0, cfg->y.dim, cfg->y.fuzz, 0);
+ /* pointer emulation */
+ set_abs(input_dev, ABS_X, &cfg->w);
+ set_abs(input_dev, ABS_Y, &cfg->w);
/* finger touch area */
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
- cfg->w.devmin, cfg->w.devmax, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR,
- cfg->w.devmin, cfg->w.devmax, 0, 0);
+ set_abs(input_dev, ABS_MT_TOUCH_MAJOR, &cfg->w);
+ set_abs(input_dev, ABS_MT_TOUCH_MINOR, &cfg->w);
/* finger approach area */
- input_set_abs_params(input_dev, ABS_MT_WIDTH_MAJOR,
- cfg->w.devmin, cfg->w.devmax, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_WIDTH_MINOR,
- cfg->w.devmin, cfg->w.devmax, 0, 0);
+ set_abs(input_dev, ABS_MT_WIDTH_MAJOR, &cfg->w);
+ set_abs(input_dev, ABS_MT_WIDTH_MINOR, &cfg->w);
/* finger orientation */
- input_set_abs_params(input_dev, ABS_MT_ORIENTATION,
- -MAX_FINGER_ORIENTATION,
- MAX_FINGER_ORIENTATION, 0, 0);
+ set_abs(input_dev, ABS_MT_ORIENTATION, &cfg->o);
/* finger position */
- input_set_abs_params(input_dev, ABS_MT_POSITION_X,
- cfg->x.devmin, cfg->x.devmax, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
- cfg->y.devmin, cfg->y.devmax, 0, 0);
+ set_abs(input_dev, ABS_MT_POSITION_X, &cfg->x);
+ set_abs(input_dev, ABS_MT_POSITION_Y, &cfg->y);
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(BTN_TOUCH, input_dev->keybit);
@@ -488,7 +460,7 @@ static void report_finger_data(struct input_dev *input,
MAX_FINGER_ORIENTATION - raw2int(f->orientation));
input_report_abs(input, ABS_MT_POSITION_X, raw2int(f->abs_x));
input_report_abs(input, ABS_MT_POSITION_Y,
- cfg->y.devmin + cfg->y.devmax - raw2int(f->abs_y));
+ cfg->y.min + cfg->y.max - raw2int(f->abs_y));
input_mt_sync(input);
}
@@ -499,8 +471,7 @@ static int report_tp_state(struct bcm5974 *dev, int size)
const struct tp_finger *f;
struct input_dev *input = dev->input;
int raw_p, raw_x, raw_y, raw_n, i;
- int ptest, origin, ibt = 0, nmin = 0, nmax = 0;
- int abs_x = 0, abs_y = 0;
+ int abs_x = 0, abs_y = 0, n = 0;
if (size < c->tp_offset || (size - c->tp_offset) % SIZEOF_FINGER != 0)
return -EIO;
@@ -525,48 +496,34 @@ static int report_tp_state(struct bcm5974 *dev, int size)
"raw: p: %+05d x: %+05d y: %+05d n: %d\n",
raw_p, raw_x, raw_y, raw_n);
- ptest = int2bound(&c->p, raw_p);
- origin = raw2int(f->origin);
-
/* while tracking finger still valid, count all fingers */
- if (ptest > PRESSURE_LOW && origin) {
- abs_x = int2bound(&c->x, raw_x - c->x.devmin);
- abs_y = int2bound(&c->y, c->y.devmax - raw_y);
+ if (raw_p > 0 && raw2int(f->origin)) {
+ abs_x = raw_x;
+ abs_y = c->y.min + c->y.max - raw_y;
while (raw_n--) {
- ptest = int2bound(&c->p,
- raw2int(f->touch_major));
- if (ptest > PRESSURE_LOW)
- nmax++;
- if (ptest > PRESSURE_HIGH)
- nmin++;
+ if (raw2int(f->touch_major) > 0)
+ n++;
f++;
}
}
}
- /* set the integrated button if applicable */
- if (c->tp_type == TYPE2)
- ibt = raw2int(dev->tp_data[BUTTON_TYPE2]);
-
- if (dev->fingers < nmin)
- dev->fingers = nmin;
- if (dev->fingers > nmax)
- dev->fingers = nmax;
+ input_report_key(input, BTN_TOUCH, n > 0);
+ input_report_key(input, BTN_TOOL_FINGER, n == 1);
+ input_report_key(input, BTN_TOOL_DOUBLETAP, n == 2);
+ input_report_key(input, BTN_TOOL_TRIPLETAP, n == 3);
+ input_report_key(input, BTN_TOOL_QUADTAP, n > 3);
- input_report_key(input, BTN_TOUCH, dev->fingers > 0);
- input_report_key(input, BTN_TOOL_FINGER, dev->fingers == 1);
- input_report_key(input, BTN_TOOL_DOUBLETAP, dev->fingers == 2);
- input_report_key(input, BTN_TOOL_TRIPLETAP, dev->fingers == 3);
- input_report_key(input, BTN_TOOL_QUADTAP, dev->fingers > 3);
-
- if (dev->fingers > 0) {
+ if (n > 0) {
input_report_abs(input, ABS_X, abs_x);
input_report_abs(input, ABS_Y, abs_y);
}
/* type 2 reports button events via ibt only */
- if (c->tp_type == TYPE2)
+ if (c->tp_type == TYPE2) {
+ int ibt = raw2int(dev->tp_data[BUTTON_TYPE2]);
input_report_key(input, BTN_LEFT, ibt);
+ }
input_sync(input);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 15/19] Input: bcm5974 - Convert to MT-B
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (13 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 14/19] Input: bcm5974 - Drop the logical dimensions Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 16/19] HID: hid-multitouch: Remove misleading null test Henrik Rydberg
` (3 subsequent siblings)
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Use of the in-kernel tracking code to convert the driver to MT-B.
With ten fingers on the pad, the in-kernel tracking adds approximately
25 us to the maximum irqsoff latency. Under normal workloads, however,
the tracking has no measurable effect.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/input/mouse/bcm5974.c | 80 +++++++++++++++----------------------------
1 file changed, 27 insertions(+), 53 deletions(-)
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index fb8ec52..6fddee6 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -40,6 +40,7 @@
#include <linux/usb/input.h>
#include <linux/hid.h>
#include <linux/mutex.h>
+#include <linux/input/mt.h>
#define USB_VENDOR_ID_APPLE 0x05ac
@@ -234,6 +235,9 @@ struct bcm5974 {
struct bt_data *bt_data; /* button transferred data */
struct urb *tp_urb; /* trackpad usb request block */
u8 *tp_data; /* trackpad transferred data */
+ const struct tp_finger *index[MAX_FINGERS]; /* finger index data */
+ struct input_mt_pos pos[MAX_FINGERS]; /* position array */
+ int slots[MAX_FINGERS]; /* slot assignments */
};
/* logical signal quality */
@@ -398,10 +402,6 @@ static void setup_events_to_report(struct input_dev *input_dev,
{
__set_bit(EV_ABS, input_dev->evbit);
- /* pointer emulation */
- set_abs(input_dev, ABS_X, &cfg->w);
- set_abs(input_dev, ABS_Y, &cfg->w);
-
/* finger touch area */
set_abs(input_dev, ABS_MT_TOUCH_MAJOR, &cfg->w);
set_abs(input_dev, ABS_MT_TOUCH_MINOR, &cfg->w);
@@ -415,16 +415,13 @@ static void setup_events_to_report(struct input_dev *input_dev,
set_abs(input_dev, ABS_MT_POSITION_Y, &cfg->y);
__set_bit(EV_KEY, input_dev->evbit);
- __set_bit(BTN_TOUCH, input_dev->keybit);
- __set_bit(BTN_TOOL_FINGER, input_dev->keybit);
- __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
- __set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
- __set_bit(BTN_TOOL_QUADTAP, input_dev->keybit);
__set_bit(BTN_LEFT, input_dev->keybit);
- __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
if (cfg->caps & HAS_INTEGRATED_BUTTON)
__set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
+
+ input_mt_init_slots(input_dev, MAX_FINGERS,
+ INPUT_MT_POINTER | INPUT_MT_DROP_UNUSED | INPUT_MT_TRACK);
}
/* report button data as logical button state */
@@ -444,10 +441,13 @@ static int report_bt_state(struct bcm5974 *dev, int size)
return 0;
}
-static void report_finger_data(struct input_dev *input,
- const struct bcm5974_config *cfg,
+static void report_finger_data(struct input_dev *input, int slot,
+ const struct input_mt_pos *pos,
const struct tp_finger *f)
{
+ input_mt_slot(input, slot);
+ input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
+
input_report_abs(input, ABS_MT_TOUCH_MAJOR,
raw2int(f->touch_major) << 1);
input_report_abs(input, ABS_MT_TOUCH_MINOR,
@@ -458,10 +458,8 @@ static void report_finger_data(struct input_dev *input,
raw2int(f->tool_minor) << 1);
input_report_abs(input, ABS_MT_ORIENTATION,
MAX_FINGER_ORIENTATION - raw2int(f->orientation));
- input_report_abs(input, ABS_MT_POSITION_X, raw2int(f->abs_x));
- input_report_abs(input, ABS_MT_POSITION_Y,
- cfg->y.min + cfg->y.max - raw2int(f->abs_y));
- input_mt_sync(input);
+ input_report_abs(input, ABS_MT_POSITION_X, pos->x);
+ input_report_abs(input, ABS_MT_POSITION_Y, pos->y);
}
/* report trackpad data as logical trackpad state */
@@ -470,8 +468,7 @@ static int report_tp_state(struct bcm5974 *dev, int size)
const struct bcm5974_config *c = &dev->cfg;
const struct tp_finger *f;
struct input_dev *input = dev->input;
- int raw_p, raw_x, raw_y, raw_n, i;
- int abs_x = 0, abs_y = 0, n = 0;
+ int raw_n, i, n = 0;
if (size < c->tp_offset || (size - c->tp_offset) % SIZEOF_FINGER != 0)
return -EIO;
@@ -480,44 +477,21 @@ static int report_tp_state(struct bcm5974 *dev, int size)
f = (const struct tp_finger *)(dev->tp_data + c->tp_offset);
raw_n = (size - c->tp_offset) / SIZEOF_FINGER;
- /* always track the first finger; when detached, start over */
- if (raw_n) {
-
- /* report raw trackpad data */
- for (i = 0; i < raw_n; i++)
- report_finger_data(input, c, &f[i]);
-
- raw_p = raw2int(f->touch_major);
- raw_x = raw2int(f->abs_x);
- raw_y = raw2int(f->abs_y);
-
- dprintk(9,
- "bcm5974: "
- "raw: p: %+05d x: %+05d y: %+05d n: %d\n",
- raw_p, raw_x, raw_y, raw_n);
-
- /* while tracking finger still valid, count all fingers */
- if (raw_p > 0 && raw2int(f->origin)) {
- abs_x = raw_x;
- abs_y = c->y.min + c->y.max - raw_y;
- while (raw_n--) {
- if (raw2int(f->touch_major) > 0)
- n++;
- f++;
- }
- }
+ for (i = 0; i < raw_n; i++) {
+ if (raw2int(f[i].touch_major) == 0)
+ continue;
+ dev->pos[n].x = raw2int(f[i].abs_x);
+ dev->pos[n].y = c->y.min + c->y.max - raw2int(f[i].abs_y);
+ dev->index[n++] = &f[i];
}
- input_report_key(input, BTN_TOUCH, n > 0);
- input_report_key(input, BTN_TOOL_FINGER, n == 1);
- input_report_key(input, BTN_TOOL_DOUBLETAP, n == 2);
- input_report_key(input, BTN_TOOL_TRIPLETAP, n == 3);
- input_report_key(input, BTN_TOOL_QUADTAP, n > 3);
+ input_mt_assign_slots(input, dev->slots, dev->pos, n);
- if (n > 0) {
- input_report_abs(input, ABS_X, abs_x);
- input_report_abs(input, ABS_Y, abs_y);
- }
+ for (i = 0; i < n; i++)
+ report_finger_data(input, dev->slots[i],
+ &dev->pos[i], dev->index[i]);
+
+ input_mt_sync_frame(input);
/* type 2 reports button events via ibt only */
if (c->tp_type == TYPE2) {
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 16/19] HID: hid-multitouch: Remove misleading null test
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (14 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 15/19] Input: bcm5974 - Convert to MT-B Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-20 13:35 ` Benjamin Tissoires
2012-08-12 21:42 ` [PATCH 17/19] HID: Only dump input if someone is listening Henrik Rydberg
` (2 subsequent siblings)
18 siblings, 1 reply; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
A null test was left behind during the autoloading work;
the test was introduced by 8d179a9e, but was never completely
reverted.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hid/hid-multitouch.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c400d90..b15133c 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -691,12 +691,10 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct mt_device *td;
struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
- if (id) {
- for (i = 0; mt_classes[i].name ; i++) {
- if (id->driver_data == mt_classes[i].name) {
- mtclass = &(mt_classes[i]);
- break;
- }
+ for (i = 0; mt_classes[i].name ; i++) {
+ if (id->driver_data == mt_classes[i].name) {
+ mtclass = &(mt_classes[i]);
+ break;
}
}
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 16/19] HID: hid-multitouch: Remove misleading null test
2012-08-12 21:42 ` [PATCH 16/19] HID: hid-multitouch: Remove misleading null test Henrik Rydberg
@ 2012-08-20 13:35 ` Benjamin Tissoires
0 siblings, 0 replies; 45+ messages in thread
From: Benjamin Tissoires @ 2012-08-20 13:35 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
Acked-by: Benjamin Tissoires <benjamin.tissoires@enac.fr>
On Sun, Aug 12, 2012 at 11:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> A null test was left behind during the autoloading work;
> the test was introduced by 8d179a9e, but was never completely
> reverted.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> drivers/hid/hid-multitouch.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index c400d90..b15133c 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -691,12 +691,10 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> struct mt_device *td;
> struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
>
> - if (id) {
> - for (i = 0; mt_classes[i].name ; i++) {
> - if (id->driver_data == mt_classes[i].name) {
> - mtclass = &(mt_classes[i]);
> - break;
> - }
> + for (i = 0; mt_classes[i].name ; i++) {
> + if (id->driver_data == mt_classes[i].name) {
> + mtclass = &(mt_classes[i]);
> + break;
> }
> }
>
> --
> 1.7.11.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 [flat|nested] 45+ messages in thread
* [PATCH 17/19] HID: Only dump input if someone is listening
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (15 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 16/19] HID: hid-multitouch: Remove misleading null test Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 18/19] HID: Add an input configured notification callback Henrik Rydberg
2012-08-12 21:42 ` [PATCH 19/19] HID: multitouch: Remove the redundant touch state Henrik Rydberg
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
Going through the motions of printing the debug message information
takes a long time; using the keyboard can lead to a 160 us irqsoff
latency. This patch skips hid_dump_input() when there are no open
handles, which brings latency down to 100 us.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hid/hid-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 60ea284..5b74e78 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -996,7 +996,8 @@ static void hid_process_event(struct hid_device *hid, struct hid_field *field,
struct hid_driver *hdrv = hid->driver;
int ret;
- hid_dump_input(hid, usage, value);
+ if (!list_empty(&hid->debug_list))
+ hid_dump_input(hid, usage, value);
if (hdrv && hdrv->event && hid_match_usage(hid, usage)) {
ret = hdrv->event(hid, field, usage, value);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 18/19] HID: Add an input configured notification callback
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (16 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 17/19] HID: Only dump input if someone is listening Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-12 21:42 ` [PATCH 19/19] HID: multitouch: Remove the redundant touch state Henrik Rydberg
18 siblings, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina; +Cc: linux-input, linux-kernel, Henrik Rydberg
A hid device may create several input devices, and a driver may need
to prepare or finalize the configuration per input device. Currently,
there is no sane way for a driver to know when a device has been
configured. This patch adds a callback providing that information.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hid/hid-input.c | 11 +++++++++--
include/linux/hid.h | 3 +++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index ad5cbcf..49141db 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1150,6 +1150,7 @@ static void report_features(struct hid_device *hid)
int hidinput_connect(struct hid_device *hid, unsigned int force)
{
+ struct hid_driver *drv = hid->driver;
struct hid_report *report;
struct hid_input *hidinput = NULL;
struct input_dev *input_dev;
@@ -1224,6 +1225,8 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
* UGCI) cram a lot of unrelated inputs into the
* same interface. */
hidinput->report = report;
+ if (drv->input_configured)
+ drv->input_configured(hid, hidinput);
if (input_register_device(hidinput->input))
goto out_cleanup;
hidinput = NULL;
@@ -1231,8 +1234,12 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
}
}
- if (hidinput && input_register_device(hidinput->input))
- goto out_cleanup;
+ if (hidinput) {
+ if (drv->input_configured)
+ drv->input_configured(hid, hidinput);
+ if (input_register_device(hidinput->input))
+ goto out_cleanup;
+ }
return 0;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 42970de..f37da28 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -626,6 +626,7 @@ struct hid_usage_id {
* @report_fixup: called before report descriptor parsing (NULL means nop)
* @input_mapping: invoked on input registering before mapping an usage
* @input_mapped: invoked on input registering after mapping an usage
+ * @input_configured: invoked just before the device is registered
* @feature_mapping: invoked on feature registering
* @suspend: invoked on suspend (NULL means nop)
* @resume: invoked on resume if device was not reset (NULL means nop)
@@ -670,6 +671,8 @@ struct hid_driver {
int (*input_mapped)(struct hid_device *hdev,
struct hid_input *hidinput, struct hid_field *field,
struct hid_usage *usage, unsigned long **bit, int *max);
+ void (*input_configured)(struct hid_device *hdev,
+ struct hid_input *hidinput);
void (*feature_mapping)(struct hid_device *hdev,
struct hid_field *field,
struct hid_usage *usage);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 19/19] HID: multitouch: Remove the redundant touch state
2012-08-12 21:42 [PATCH 00/19] Input and HID updates for 3.7 Henrik Rydberg
` (17 preceding siblings ...)
2012-08-12 21:42 ` [PATCH 18/19] HID: Add an input configured notification callback Henrik Rydberg
@ 2012-08-12 21:42 ` Henrik Rydberg
2012-08-20 13:36 ` Benjamin Tissoires
18 siblings, 1 reply; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-12 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina
Cc: linux-input, linux-kernel, Henrik Rydberg, Benjamin Tissoires
With the input_mt_sync_frame() function in place, there is no longer
any need to keep the full touch state in the driver. This patch
removes the slot state and replaces the lookup code with the input-mt
equivalent. The initialization code is moved to mt_input_configured(),
to make sure the full HID report has been seen.
Cc: Benjamin Tissoires <benjamin.tissoires@enac.fr>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hid/hid-multitouch.c | 162 ++++++++++++++++++-------------------------
1 file changed, 66 insertions(+), 96 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index b15133c..bd4bc3c 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -52,13 +52,6 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 6)
#define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE (1 << 8)
-struct mt_slot {
- __s32 x, y, p, w, h;
- __s32 contactid; /* the device ContactID assigned to this slot */
- bool touch_state; /* is the touch valid? */
- bool seen_in_this_frame;/* has this slot been updated */
-};
-
struct mt_class {
__s32 name; /* MT_CLS */
__s32 quirks;
@@ -76,7 +69,6 @@ struct mt_fields {
};
struct mt_device {
- struct mt_slot curdata; /* placeholder of incoming data */
struct mt_class mtclass; /* our mt device class */
struct mt_fields *fields; /* temporary placeholder for storing the
multitouch fields */
@@ -93,7 +85,9 @@ struct mt_device {
* 1 means we should use a serial protocol
* > 1 means hybrid (multitouch) protocol */
bool curvalid; /* is the current contact valid? */
- struct mt_slot *slots;
+ __s32 x, y, p, w, h;
+ __s32 contactid; /* the device ContactID assigned to this slot */
+ bool touch_state; /* is the touch valid? */
};
/* classes of device behavior */
@@ -128,31 +122,12 @@ struct mt_device {
static int cypress_compute_slot(struct mt_device *td)
{
- if (td->curdata.contactid != 0 || td->num_received == 0)
- return td->curdata.contactid;
+ if (td->contactid != 0 || td->num_received == 0)
+ return td->contactid;
else
return -1;
}
-static int find_slot_from_contactid(struct mt_device *td)
-{
- int i;
- for (i = 0; i < td->maxcontacts; ++i) {
- if (td->slots[i].contactid == td->curdata.contactid &&
- td->slots[i].touch_state)
- return i;
- }
- for (i = 0; i < td->maxcontacts; ++i) {
- if (!td->slots[i].seen_in_this_frame &&
- !td->slots[i].touch_state)
- return i;
- }
- /* should not occurs. If this happens that means
- * that the device sent more touches that it says
- * in the report descriptor. It is ignored then. */
- return -1;
-}
-
static struct mt_class mt_classes[] = {
{ .name = MT_CLS_DEFAULT,
.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
@@ -390,7 +365,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
case HID_DG_CONTACTID:
if (!td->maxcontacts)
td->maxcontacts = MT_DEFAULT_MAXCONTACT;
- input_mt_init_slots(hi->input, td->maxcontacts, 0);
mt_store_field(usage, td, hi);
td->last_field_index = field->index;
td->touches_by_report++;
@@ -464,12 +438,31 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
return -1;
}
-static int mt_compute_slot(struct mt_device *td)
+static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
+
+{
+ struct mt_device *td = hid_get_drvdata(hdev);
+ struct mt_class *cls = &td->mtclass;
+ struct input_dev *input = hi->input;
+ unsigned int flags = 0;
+
+ if (test_bit(INPUT_PROP_POINTER, input->propbit))
+ flags |= INPUT_MT_POINTER;
+ if (test_bit(INPUT_PROP_DIRECT, input->propbit))
+ flags |= INPUT_MT_DIRECT;
+
+ if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
+ flags |= INPUT_MT_DROP_UNUSED;
+
+ input_mt_init_slots(input, td->maxcontacts, flags);
+}
+
+static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
{
__s32 quirks = td->mtclass.quirks;
if (quirks & MT_QUIRK_SLOT_IS_CONTACTID)
- return td->curdata.contactid;
+ return td->contactid;
if (quirks & MT_QUIRK_CYPRESS)
return cypress_compute_slot(td);
@@ -478,25 +471,43 @@ static int mt_compute_slot(struct mt_device *td)
return td->num_received;
if (quirks & MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE)
- return td->curdata.contactid - 1;
+ return td->contactid - 1;
- return find_slot_from_contactid(td);
+ return input_mt_assign_slot_by_id(input, td->contactid);
}
/*
* this function is called when a whole contact has been processed,
* so that it can assign it to a slot and store the data there
*/
-static void mt_complete_slot(struct mt_device *td)
+static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
{
- td->curdata.seen_in_this_frame = true;
- if (td->curvalid) {
- int slotnum = mt_compute_slot(td);
+ int slot;
- if (slotnum >= 0 && slotnum < td->maxcontacts)
- td->slots[slotnum] = td->curdata;
- }
td->num_received++;
+ if (!td->curvalid)
+ return;
+
+ slot = mt_compute_slot(td, input);
+ if (slot < 0 || slot >= td->maxcontacts)
+ return;
+
+ input_mt_slot(input, slot);
+ input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch_state);
+ if (td->touch_state) {
+ /* this finger is on the screen */
+ int wide = (td->w > td->h);
+ /* divided by two to match visual scale of touch */
+ int major = max(td->w, td->h) >> 1;
+ int minor = min(td->w, td->h) >> 1;
+
+ input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
+ input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
+ input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
+ input_event(input, EV_ABS, ABS_MT_PRESSURE, td->p);
+ input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
+ input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
+ }
}
@@ -504,52 +515,20 @@ static void mt_complete_slot(struct mt_device *td)
* this function is called when a whole packet has been received and processed,
* so that it can decide what to send to the input layer.
*/
-static void mt_emit_event(struct mt_device *td, struct input_dev *input)
+static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
{
- int i;
-
- for (i = 0; i < td->maxcontacts; ++i) {
- struct mt_slot *s = &(td->slots[i]);
- if ((td->mtclass.quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) &&
- !s->seen_in_this_frame) {
- s->touch_state = false;
- }
-
- input_mt_slot(input, i);
- input_mt_report_slot_state(input, MT_TOOL_FINGER,
- s->touch_state);
- if (s->touch_state) {
- /* this finger is on the screen */
- int wide = (s->w > s->h);
- /* divided by two to match visual scale of touch */
- int major = max(s->w, s->h) >> 1;
- int minor = min(s->w, s->h) >> 1;
-
- input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
- input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
- input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
- input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
- input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
- }
- s->seen_in_this_frame = false;
-
- }
-
- input_mt_report_pointer_emulation(input, true);
+ input_mt_sync_frame(input);
input_sync(input);
td->num_received = 0;
}
-
-
static int mt_event(struct hid_device *hid, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
struct mt_device *td = hid_get_drvdata(hid);
__s32 quirks = td->mtclass.quirks;
- if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {
+ if (hid->claimed & HID_CLAIMED_INPUT) {
switch (usage->hid) {
case HID_DG_INRANGE:
if (quirks & MT_QUIRK_ALWAYS_VALID)
@@ -560,29 +539,29 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
case HID_DG_TIPSWITCH:
if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
td->curvalid = value;
- td->curdata.touch_state = value;
+ td->touch_state = value;
break;
case HID_DG_CONFIDENCE:
if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
td->curvalid = value;
break;
case HID_DG_CONTACTID:
- td->curdata.contactid = value;
+ td->contactid = value;
break;
case HID_DG_TIPPRESSURE:
- td->curdata.p = value;
+ td->p = value;
break;
case HID_GD_X:
- td->curdata.x = value;
+ td->x = value;
break;
case HID_GD_Y:
- td->curdata.y = value;
+ td->y = value;
break;
case HID_DG_WIDTH:
- td->curdata.w = value;
+ td->w = value;
break;
case HID_DG_HEIGHT:
- td->curdata.h = value;
+ td->h = value;
break;
case HID_DG_CONTACTCOUNT:
/*
@@ -602,11 +581,11 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
}
if (usage->hid == td->last_slot_field)
- mt_complete_slot(td);
+ mt_complete_slot(td, field->hidinput->input);
if (field->index == td->last_field_index
&& td->num_received >= td->num_expected)
- mt_emit_event(td, field->hidinput->input);
+ mt_sync_frame(td, field->hidinput->input);
}
@@ -733,15 +712,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
mt_post_parse_default_settings(td);
- td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
- GFP_KERNEL);
- if (!td->slots) {
- dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
- hid_hw_stop(hdev);
- ret = -ENOMEM;
- goto fail;
- }
-
ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
mt_set_maxcontacts(hdev);
@@ -772,7 +742,6 @@ static void mt_remove(struct hid_device *hdev)
struct mt_device *td = hid_get_drvdata(hdev);
sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
hid_hw_stop(hdev);
- kfree(td->slots);
kfree(td);
hid_set_drvdata(hdev, NULL);
}
@@ -1085,6 +1054,7 @@ static struct hid_driver mt_driver = {
.remove = mt_remove,
.input_mapping = mt_input_mapping,
.input_mapped = mt_input_mapped,
+ .input_configured = mt_input_configured,
.feature_mapping = mt_feature_mapping,
.usage_table = mt_grabbed_usages,
.event = mt_event,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 19/19] HID: multitouch: Remove the redundant touch state
2012-08-12 21:42 ` [PATCH 19/19] HID: multitouch: Remove the redundant touch state Henrik Rydberg
@ 2012-08-20 13:36 ` Benjamin Tissoires
2012-08-20 16:01 ` Henrik Rydberg
2012-08-22 20:58 ` [PATCH v2] " Henrik Rydberg
0 siblings, 2 replies; 45+ messages in thread
From: Benjamin Tissoires @ 2012-08-20 13:36 UTC (permalink / raw)
To: Henrik Rydberg; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
Hello Henrik,
On Sun, Aug 12, 2012 at 11:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> With the input_mt_sync_frame() function in place, there is no longer
> any need to keep the full touch state in the driver. This patch
> removes the slot state and replaces the lookup code with the input-mt
> equivalent. The initialization code is moved to mt_input_configured(),
> to make sure the full HID report has been seen.
This patch seems to be a little bit complex.
It has very good things, but also many things that hinders the readability.
And you should also remove the /* touchscreen emulation */ things in
mt_input_mapping as input_mt_init_slots handles now the init of ABS_X
ABS_Y and ABS_PRESSURE.
>
> Cc: Benjamin Tissoires <benjamin.tissoires@enac.fr>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> drivers/hid/hid-multitouch.c | 162 ++++++++++++++++++-------------------------
> 1 file changed, 66 insertions(+), 96 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index b15133c..bd4bc3c 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -52,13 +52,6 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 6)
> #define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE (1 << 8)
>
> -struct mt_slot {
> - __s32 x, y, p, w, h;
> - __s32 contactid; /* the device ContactID assigned to this slot */
> - bool touch_state; /* is the touch valid? */
> - bool seen_in_this_frame;/* has this slot been updated */
> -};
Why removing this struct?
Removing it infers a lot of unneeded changes in the patch.
As the mt_sync_frame handle the quirk NOT_SEEN_MEANS_UP, we should
just remove the field seen_in_this_frame.
> -
> struct mt_class {
> __s32 name; /* MT_CLS */
> __s32 quirks;
> @@ -76,7 +69,6 @@ struct mt_fields {
> };
>
> struct mt_device {
> - struct mt_slot curdata; /* placeholder of incoming data */
> struct mt_class mtclass; /* our mt device class */
> struct mt_fields *fields; /* temporary placeholder for storing the
> multitouch fields */
> @@ -93,7 +85,9 @@ struct mt_device {
> * 1 means we should use a serial protocol
> * > 1 means hybrid (multitouch) protocol */
> bool curvalid; /* is the current contact valid? */
> - struct mt_slot *slots;
> + __s32 x, y, p, w, h;
> + __s32 contactid; /* the device ContactID assigned to this slot */
> + bool touch_state; /* is the touch valid? */
Again, by keeping the first field, you don't lose any memory and
things are packed and organized!
(having x, y, etc... at the root of the device struct is kind of weird...)
> };
>
> /* classes of device behavior */
> @@ -128,31 +122,12 @@ struct mt_device {
>
> static int cypress_compute_slot(struct mt_device *td)
> {
> - if (td->curdata.contactid != 0 || td->num_received == 0)
> - return td->curdata.contactid;
> + if (td->contactid != 0 || td->num_received == 0)
> + return td->contactid;
This patch contains many unneeded modifications of this type.... :-(
> else
> return -1;
> }
>
> -static int find_slot_from_contactid(struct mt_device *td)
> -{
> - int i;
> - for (i = 0; i < td->maxcontacts; ++i) {
> - if (td->slots[i].contactid == td->curdata.contactid &&
> - td->slots[i].touch_state)
> - return i;
> - }
> - for (i = 0; i < td->maxcontacts; ++i) {
> - if (!td->slots[i].seen_in_this_frame &&
> - !td->slots[i].touch_state)
> - return i;
> - }
> - /* should not occurs. If this happens that means
> - * that the device sent more touches that it says
> - * in the report descriptor. It is ignored then. */
> - return -1;
> -}
> -
I'm always happy to remove code.... ;-)
> static struct mt_class mt_classes[] = {
> { .name = MT_CLS_DEFAULT,
> .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
> @@ -390,7 +365,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> case HID_DG_CONTACTID:
> if (!td->maxcontacts)
> td->maxcontacts = MT_DEFAULT_MAXCONTACT;
> - input_mt_init_slots(hi->input, td->maxcontacts, 0);
this thing was worrying me, so it's better this way.
> mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> td->touches_by_report++;
> @@ -464,12 +438,31 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
> return -1;
> }
>
> -static int mt_compute_slot(struct mt_device *td)
> +static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
> +
> +{
> + struct mt_device *td = hid_get_drvdata(hdev);
> + struct mt_class *cls = &td->mtclass;
> + struct input_dev *input = hi->input;
> + unsigned int flags = 0;
> +
> + if (test_bit(INPUT_PROP_POINTER, input->propbit))
> + flags |= INPUT_MT_POINTER;
> + if (test_bit(INPUT_PROP_DIRECT, input->propbit))
> + flags |= INPUT_MT_DIRECT;
These two tests are really strange: the function input_mt_init_slots
already sets those bits....
Maybe we should handle INPUT_PROP_POINTER, INPUT_PROP_DIRECT by
keeping the flag instead of setting the bits and re-read them to
finally re-set them...
And yes, I know that input_mt_init_slots does a lot more than just
setting those two bits, but it's the impression I feel when reading
this.
Anyway, besides this things, I'm happy with the input_configured callback.
> +
> + if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
> + flags |= INPUT_MT_DROP_UNUSED;
> +
> + input_mt_init_slots(input, td->maxcontacts, flags);
> +}
> +
> +static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
> {
> __s32 quirks = td->mtclass.quirks;
>
> if (quirks & MT_QUIRK_SLOT_IS_CONTACTID)
> - return td->curdata.contactid;
> + return td->contactid;
>
> if (quirks & MT_QUIRK_CYPRESS)
> return cypress_compute_slot(td);
> @@ -478,25 +471,43 @@ static int mt_compute_slot(struct mt_device *td)
> return td->num_received;
>
> if (quirks & MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE)
> - return td->curdata.contactid - 1;
> + return td->contactid - 1;
>
> - return find_slot_from_contactid(td);
> + return input_mt_assign_slot_by_id(input, td->contactid);
> }
>
> /*
> * this function is called when a whole contact has been processed,
> * so that it can assign it to a slot and store the data there
> */
> -static void mt_complete_slot(struct mt_device *td)
> +static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> {
> - td->curdata.seen_in_this_frame = true;
> - if (td->curvalid) {
> - int slotnum = mt_compute_slot(td);
> + int slot;
>
> - if (slotnum >= 0 && slotnum < td->maxcontacts)
> - td->slots[slotnum] = td->curdata;
> - }
> td->num_received++;
> + if (!td->curvalid)
> + return;
> +
> + slot = mt_compute_slot(td, input);
> + if (slot < 0 || slot >= td->maxcontacts)
> + return;
> +
> + input_mt_slot(input, slot);
> + input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch_state);
> + if (td->touch_state) {
> + /* this finger is on the screen */
> + int wide = (td->w > td->h);
> + /* divided by two to match visual scale of touch */
> + int major = max(td->w, td->h) >> 1;
> + int minor = min(td->w, td->h) >> 1;
> +
> + input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
> + input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
> + input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
> + input_event(input, EV_ABS, ABS_MT_PRESSURE, td->p);
> + input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
> + input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> + }
> }
>
>
> @@ -504,52 +515,20 @@ static void mt_complete_slot(struct mt_device *td)
> * this function is called when a whole packet has been received and processed,
> * so that it can decide what to send to the input layer.
> */
> -static void mt_emit_event(struct mt_device *td, struct input_dev *input)
> +static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
> {
> - int i;
> -
> - for (i = 0; i < td->maxcontacts; ++i) {
> - struct mt_slot *s = &(td->slots[i]);
> - if ((td->mtclass.quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) &&
> - !s->seen_in_this_frame) {
> - s->touch_state = false;
> - }
> -
> - input_mt_slot(input, i);
> - input_mt_report_slot_state(input, MT_TOOL_FINGER,
> - s->touch_state);
> - if (s->touch_state) {
> - /* this finger is on the screen */
> - int wide = (s->w > s->h);
> - /* divided by two to match visual scale of touch */
> - int major = max(s->w, s->h) >> 1;
> - int minor = min(s->w, s->h) >> 1;
> -
> - input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);
> - input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
> - input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
> - input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
> - input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
> - input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> - }
> - s->seen_in_this_frame = false;
> -
> - }
> -
> - input_mt_report_pointer_emulation(input, true);
> + input_mt_sync_frame(input);
> input_sync(input);
> td->num_received = 0;
> }
>
> -
> -
> static int mt_event(struct hid_device *hid, struct hid_field *field,
> struct hid_usage *usage, __s32 value)
> {
> struct mt_device *td = hid_get_drvdata(hid);
> __s32 quirks = td->mtclass.quirks;
>
> - if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {
> + if (hid->claimed & HID_CLAIMED_INPUT) {
the "td->slots" test was ugly ;-)
Thanks,
Benjamin
> switch (usage->hid) {
> case HID_DG_INRANGE:
> if (quirks & MT_QUIRK_ALWAYS_VALID)
> @@ -560,29 +539,29 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> case HID_DG_TIPSWITCH:
> if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
> td->curvalid = value;
> - td->curdata.touch_state = value;
> + td->touch_state = value;
> break;
> case HID_DG_CONFIDENCE:
> if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
> td->curvalid = value;
> break;
> case HID_DG_CONTACTID:
> - td->curdata.contactid = value;
> + td->contactid = value;
> break;
> case HID_DG_TIPPRESSURE:
> - td->curdata.p = value;
> + td->p = value;
> break;
> case HID_GD_X:
> - td->curdata.x = value;
> + td->x = value;
> break;
> case HID_GD_Y:
> - td->curdata.y = value;
> + td->y = value;
> break;
> case HID_DG_WIDTH:
> - td->curdata.w = value;
> + td->w = value;
> break;
> case HID_DG_HEIGHT:
> - td->curdata.h = value;
> + td->h = value;
> break;
> case HID_DG_CONTACTCOUNT:
> /*
> @@ -602,11 +581,11 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> }
>
> if (usage->hid == td->last_slot_field)
> - mt_complete_slot(td);
> + mt_complete_slot(td, field->hidinput->input);
>
> if (field->index == td->last_field_index
> && td->num_received >= td->num_expected)
> - mt_emit_event(td, field->hidinput->input);
> + mt_sync_frame(td, field->hidinput->input);
>
> }
>
> @@ -733,15 +712,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
> mt_post_parse_default_settings(td);
>
> - td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
> - GFP_KERNEL);
> - if (!td->slots) {
> - dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
> - hid_hw_stop(hdev);
> - ret = -ENOMEM;
> - goto fail;
> - }
> -
> ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
>
> mt_set_maxcontacts(hdev);
> @@ -772,7 +742,6 @@ static void mt_remove(struct hid_device *hdev)
> struct mt_device *td = hid_get_drvdata(hdev);
> sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
> hid_hw_stop(hdev);
> - kfree(td->slots);
> kfree(td);
> hid_set_drvdata(hdev, NULL);
> }
> @@ -1085,6 +1054,7 @@ static struct hid_driver mt_driver = {
> .remove = mt_remove,
> .input_mapping = mt_input_mapping,
> .input_mapped = mt_input_mapped,
> + .input_configured = mt_input_configured,
> .feature_mapping = mt_feature_mapping,
> .usage_table = mt_grabbed_usages,
> .event = mt_event,
> --
> 1.7.11.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 [flat|nested] 45+ messages in thread* Re: [PATCH 19/19] HID: multitouch: Remove the redundant touch state
2012-08-20 13:36 ` Benjamin Tissoires
@ 2012-08-20 16:01 ` Henrik Rydberg
2012-08-22 20:58 ` [PATCH v2] " Henrik Rydberg
1 sibling, 0 replies; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-20 16:01 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
Hi Benjamin,
> This patch seems to be a little bit complex.
> It has very good things, but also many things that hinders the readability.
>
> And you should also remove the /* touchscreen emulation */ things in
> mt_input_mapping as input_mt_init_slots handles now the init of ABS_X
> ABS_Y and ABS_PRESSURE.
Yes, it could be changed as well, like the bit patterns. As you
mention below, the logic around the bits could be enhanced, and the
ABS_X/Y should go in that set of changes.
> > -struct mt_slot {
> > - __s32 x, y, p, w, h;
> > - __s32 contactid; /* the device ContactID assigned to this slot */
> > - bool touch_state; /* is the touch valid? */
> > - bool seen_in_this_frame;/* has this slot been updated */
> > -};
>
> Why removing this struct?
> Removing it infers a lot of unneeded changes in the patch.
>
> As the mt_sync_frame handle the quirk NOT_SEEN_MEANS_UP, we should
> just remove the field seen_in_this_frame.
Well, it is no longer needed, but sure, one could keep it and just
remove the unused fields.
> > @@ -464,12 +438,31 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
> > return -1;
> > }
> >
> > -static int mt_compute_slot(struct mt_device *td)
> > +static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
> > +
> > +{
> > + struct mt_device *td = hid_get_drvdata(hdev);
> > + struct mt_class *cls = &td->mtclass;
> > + struct input_dev *input = hi->input;
> > + unsigned int flags = 0;
> > +
> > + if (test_bit(INPUT_PROP_POINTER, input->propbit))
> > + flags |= INPUT_MT_POINTER;
> > + if (test_bit(INPUT_PROP_DIRECT, input->propbit))
> > + flags |= INPUT_MT_DIRECT;
>
> These two tests are really strange: the function input_mt_init_slots
> already sets those bits....
> Maybe we should handle INPUT_PROP_POINTER, INPUT_PROP_DIRECT by
> keeping the flag instead of setting the bits and re-read them to
> finally re-set them...
Ok, I will extend the patchset for hid-multitouch to include such
changes as well.
Thanks,
Henrik
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH v2] HID: multitouch: Remove the redundant touch state
2012-08-20 13:36 ` Benjamin Tissoires
2012-08-20 16:01 ` Henrik Rydberg
@ 2012-08-22 20:58 ` Henrik Rydberg
2012-08-28 22:25 ` Jiri Kosina
1 sibling, 1 reply; 45+ messages in thread
From: Henrik Rydberg @ 2012-08-22 20:58 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Jiri Kosina, linux-input, linux-kernel
With the input_mt_sync_frame() function in place, there is no longer
any need to keep the full touch state in the driver. This patch
removes the slot state and replaces the lookup code with the input-mt
equivalent. The initialization code is moved to mt_input_configured(),
to make sure the full HID report has been seen.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
Hi Benjamin,
Maybe this patch works better? It has received limited testing so far.
Henrik
drivers/hid/hid-multitouch.c | 133 +++++++++++++++----------------------------
1 file changed, 46 insertions(+), 87 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c400d90..dc08a4e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -56,7 +56,6 @@ struct mt_slot {
__s32 x, y, p, w, h;
__s32 contactid; /* the device ContactID assigned to this slot */
bool touch_state; /* is the touch valid? */
- bool seen_in_this_frame;/* has this slot been updated */
};
struct mt_class {
@@ -93,7 +92,7 @@ struct mt_device {
* 1 means we should use a serial protocol
* > 1 means hybrid (multitouch) protocol */
bool curvalid; /* is the current contact valid? */
- struct mt_slot *slots;
+ unsigned mt_flags; /* flags to pass to input-mt */
};
/* classes of device behavior */
@@ -134,25 +133,6 @@ static int cypress_compute_slot(struct mt_device *td)
return -1;
}
-static int find_slot_from_contactid(struct mt_device *td)
-{
- int i;
- for (i = 0; i < td->maxcontacts; ++i) {
- if (td->slots[i].contactid == td->curdata.contactid &&
- td->slots[i].touch_state)
- return i;
- }
- for (i = 0; i < td->maxcontacts; ++i) {
- if (!td->slots[i].seen_in_this_frame &&
- !td->slots[i].touch_state)
- return i;
- }
- /* should not occurs. If this happens that means
- * that the device sent more touches that it says
- * in the report descriptor. It is ignored then. */
- return -1;
-}
-
static struct mt_class mt_classes[] = {
{ .name = MT_CLS_DEFAULT,
.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
@@ -319,24 +299,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
* We need to ignore fields that belong to other collections
* such as Mouse that might have the same GenericDesktop usages. */
if (field->application == HID_DG_TOUCHSCREEN)
- set_bit(INPUT_PROP_DIRECT, hi->input->propbit);
+ td->mt_flags |= INPUT_MT_DIRECT;
else if (field->application != HID_DG_TOUCHPAD)
return 0;
- /* In case of an indirect device (touchpad), we need to add
- * specific BTN_TOOL_* to be handled by the synaptics xorg
- * driver.
- * We also consider that touchscreens providing buttons are touchpads.
+ /*
+ * Model touchscreens providing buttons as touchpads.
*/
if (field->application == HID_DG_TOUCHPAD ||
- (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON ||
- cls->is_indirect) {
- set_bit(INPUT_PROP_POINTER, hi->input->propbit);
- set_bit(BTN_TOOL_FINGER, hi->input->keybit);
- set_bit(BTN_TOOL_DOUBLETAP, hi->input->keybit);
- set_bit(BTN_TOOL_TRIPLETAP, hi->input->keybit);
- set_bit(BTN_TOOL_QUADTAP, hi->input->keybit);
- }
+ (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON)
+ td->mt_flags |= INPUT_MT_POINTER;
/* eGalax devices provide a Digitizer.Stylus input which overrides
* the correct Digitizers.Finger X/Y ranges.
@@ -353,8 +325,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
EV_ABS, ABS_MT_POSITION_X);
set_abs(hi->input, ABS_MT_POSITION_X, field,
cls->sn_move);
- /* touchscreen emulation */
- set_abs(hi->input, ABS_X, field, cls->sn_move);
mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;
@@ -363,8 +333,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
EV_ABS, ABS_MT_POSITION_Y);
set_abs(hi->input, ABS_MT_POSITION_Y, field,
cls->sn_move);
- /* touchscreen emulation */
- set_abs(hi->input, ABS_Y, field, cls->sn_move);
mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;
@@ -390,7 +358,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
case HID_DG_CONTACTID:
if (!td->maxcontacts)
td->maxcontacts = MT_DEFAULT_MAXCONTACT;
- input_mt_init_slots(hi->input, td->maxcontacts, 0);
mt_store_field(usage, td, hi);
td->last_field_index = field->index;
td->touches_by_report++;
@@ -418,9 +385,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
EV_ABS, ABS_MT_PRESSURE);
set_abs(hi->input, ABS_MT_PRESSURE, field,
cls->sn_pressure);
- /* touchscreen emulation */
- set_abs(hi->input, ABS_PRESSURE, field,
- cls->sn_pressure);
mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;
@@ -464,7 +428,24 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
return -1;
}
-static int mt_compute_slot(struct mt_device *td)
+static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
+
+{
+ struct mt_device *td = hid_get_drvdata(hdev);
+ struct mt_class *cls = &td->mtclass;
+
+ if (cls->is_indirect)
+ td->mt_flags |= INPUT_MT_POINTER;
+
+ if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
+ td->mt_flags |= INPUT_MT_DROP_UNUSED;
+
+ input_mt_init_slots(hi->input, td->maxcontacts, td->mt_flags);
+
+ td->mt_flags = 0;
+}
+
+static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
{
__s32 quirks = td->mtclass.quirks;
@@ -480,42 +461,23 @@ static int mt_compute_slot(struct mt_device *td)
if (quirks & MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE)
return td->curdata.contactid - 1;
- return find_slot_from_contactid(td);
+ return input_mt_assign_slot_by_id(input, td->curdata.contactid);
}
/*
* this function is called when a whole contact has been processed,
* so that it can assign it to a slot and store the data there
*/
-static void mt_complete_slot(struct mt_device *td)
+static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
{
- td->curdata.seen_in_this_frame = true;
if (td->curvalid) {
- int slotnum = mt_compute_slot(td);
-
- if (slotnum >= 0 && slotnum < td->maxcontacts)
- td->slots[slotnum] = td->curdata;
- }
- td->num_received++;
-}
+ int slotnum = mt_compute_slot(td, input);
+ struct mt_slot *s = &td->curdata;
+ if (slotnum < 0 || slotnum >= td->maxcontacts)
+ return;
-/*
- * this function is called when a whole packet has been received and processed,
- * so that it can decide what to send to the input layer.
- */
-static void mt_emit_event(struct mt_device *td, struct input_dev *input)
-{
- int i;
-
- for (i = 0; i < td->maxcontacts; ++i) {
- struct mt_slot *s = &(td->slots[i]);
- if ((td->mtclass.quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) &&
- !s->seen_in_this_frame) {
- s->touch_state = false;
- }
-
- input_mt_slot(input, i);
+ input_mt_slot(input, slotnum);
input_mt_report_slot_state(input, MT_TOOL_FINGER,
s->touch_state);
if (s->touch_state) {
@@ -532,16 +494,22 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
}
- s->seen_in_this_frame = false;
-
}
- input_mt_report_pointer_emulation(input, true);
- input_sync(input);
- td->num_received = 0;
+ td->num_received++;
}
+/*
+ * this function is called when a whole packet has been received and processed,
+ * so that it can decide what to send to the input layer.
+ */
+static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
+{
+ input_mt_sync_frame(input);
+ input_sync(input);
+ td->num_received = 0;
+}
static int mt_event(struct hid_device *hid, struct hid_field *field,
struct hid_usage *usage, __s32 value)
@@ -549,7 +517,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
struct mt_device *td = hid_get_drvdata(hid);
__s32 quirks = td->mtclass.quirks;
- if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {
+ if (hid->claimed & HID_CLAIMED_INPUT) {
switch (usage->hid) {
case HID_DG_INRANGE:
if (quirks & MT_QUIRK_ALWAYS_VALID)
@@ -602,11 +570,11 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
}
if (usage->hid == td->last_slot_field)
- mt_complete_slot(td);
+ mt_complete_slot(td, field->hidinput->input);
if (field->index == td->last_field_index
&& td->num_received >= td->num_expected)
- mt_emit_event(td, field->hidinput->input);
+ mt_sync_frame(td, field->hidinput->input);
}
@@ -735,15 +703,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
mt_post_parse_default_settings(td);
- td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
- GFP_KERNEL);
- if (!td->slots) {
- dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
- hid_hw_stop(hdev);
- ret = -ENOMEM;
- goto fail;
- }
-
ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
mt_set_maxcontacts(hdev);
@@ -774,7 +733,6 @@ static void mt_remove(struct hid_device *hdev)
struct mt_device *td = hid_get_drvdata(hdev);
sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
hid_hw_stop(hdev);
- kfree(td->slots);
kfree(td);
hid_set_drvdata(hdev, NULL);
}
@@ -1087,6 +1045,7 @@ static struct hid_driver mt_driver = {
.remove = mt_remove,
.input_mapping = mt_input_mapping,
.input_mapped = mt_input_mapped,
+ .input_configured = mt_input_configured,
.feature_mapping = mt_feature_mapping,
.usage_table = mt_grabbed_usages,
.event = mt_event,
--
1.7.11.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v2] HID: multitouch: Remove the redundant touch state
2012-08-22 20:58 ` [PATCH v2] " Henrik Rydberg
@ 2012-08-28 22:25 ` Jiri Kosina
2012-08-29 13:36 ` Benjamin Tissoires
0 siblings, 1 reply; 45+ messages in thread
From: Jiri Kosina @ 2012-08-28 22:25 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Benjamin Tissoires, Dmitry Torokhov, linux-input, linux-kernel
On Wed, 22 Aug 2012, Henrik Rydberg wrote:
> With the input_mt_sync_frame() function in place, there is no longer
> any need to keep the full touch state in the driver. This patch
> removes the slot state and replaces the lookup code with the input-mt
> equivalent. The initialization code is moved to mt_input_configured(),
> to make sure the full HID report has been seen.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> Hi Benjamin,
>
> Maybe this patch works better? It has received limited testing so far.
What is the status of this patch please? Henrik, Benjamin?
>
> Henrik
>
> drivers/hid/hid-multitouch.c | 133 +++++++++++++++----------------------------
> 1 file changed, 46 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index c400d90..dc08a4e 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -56,7 +56,6 @@ struct mt_slot {
> __s32 x, y, p, w, h;
> __s32 contactid; /* the device ContactID assigned to this slot */
> bool touch_state; /* is the touch valid? */
> - bool seen_in_this_frame;/* has this slot been updated */
> };
>
> struct mt_class {
> @@ -93,7 +92,7 @@ struct mt_device {
> * 1 means we should use a serial protocol
> * > 1 means hybrid (multitouch) protocol */
> bool curvalid; /* is the current contact valid? */
> - struct mt_slot *slots;
> + unsigned mt_flags; /* flags to pass to input-mt */
> };
>
> /* classes of device behavior */
> @@ -134,25 +133,6 @@ static int cypress_compute_slot(struct mt_device *td)
> return -1;
> }
>
> -static int find_slot_from_contactid(struct mt_device *td)
> -{
> - int i;
> - for (i = 0; i < td->maxcontacts; ++i) {
> - if (td->slots[i].contactid == td->curdata.contactid &&
> - td->slots[i].touch_state)
> - return i;
> - }
> - for (i = 0; i < td->maxcontacts; ++i) {
> - if (!td->slots[i].seen_in_this_frame &&
> - !td->slots[i].touch_state)
> - return i;
> - }
> - /* should not occurs. If this happens that means
> - * that the device sent more touches that it says
> - * in the report descriptor. It is ignored then. */
> - return -1;
> -}
> -
> static struct mt_class mt_classes[] = {
> { .name = MT_CLS_DEFAULT,
> .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
> @@ -319,24 +299,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> * We need to ignore fields that belong to other collections
> * such as Mouse that might have the same GenericDesktop usages. */
> if (field->application == HID_DG_TOUCHSCREEN)
> - set_bit(INPUT_PROP_DIRECT, hi->input->propbit);
> + td->mt_flags |= INPUT_MT_DIRECT;
> else if (field->application != HID_DG_TOUCHPAD)
> return 0;
>
> - /* In case of an indirect device (touchpad), we need to add
> - * specific BTN_TOOL_* to be handled by the synaptics xorg
> - * driver.
> - * We also consider that touchscreens providing buttons are touchpads.
> + /*
> + * Model touchscreens providing buttons as touchpads.
> */
> if (field->application == HID_DG_TOUCHPAD ||
> - (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON ||
> - cls->is_indirect) {
> - set_bit(INPUT_PROP_POINTER, hi->input->propbit);
> - set_bit(BTN_TOOL_FINGER, hi->input->keybit);
> - set_bit(BTN_TOOL_DOUBLETAP, hi->input->keybit);
> - set_bit(BTN_TOOL_TRIPLETAP, hi->input->keybit);
> - set_bit(BTN_TOOL_QUADTAP, hi->input->keybit);
> - }
> + (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON)
> + td->mt_flags |= INPUT_MT_POINTER;
>
> /* eGalax devices provide a Digitizer.Stylus input which overrides
> * the correct Digitizers.Finger X/Y ranges.
> @@ -353,8 +325,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> EV_ABS, ABS_MT_POSITION_X);
> set_abs(hi->input, ABS_MT_POSITION_X, field,
> cls->sn_move);
> - /* touchscreen emulation */
> - set_abs(hi->input, ABS_X, field, cls->sn_move);
> mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> @@ -363,8 +333,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> EV_ABS, ABS_MT_POSITION_Y);
> set_abs(hi->input, ABS_MT_POSITION_Y, field,
> cls->sn_move);
> - /* touchscreen emulation */
> - set_abs(hi->input, ABS_Y, field, cls->sn_move);
> mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> @@ -390,7 +358,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> case HID_DG_CONTACTID:
> if (!td->maxcontacts)
> td->maxcontacts = MT_DEFAULT_MAXCONTACT;
> - input_mt_init_slots(hi->input, td->maxcontacts, 0);
> mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> td->touches_by_report++;
> @@ -418,9 +385,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> EV_ABS, ABS_MT_PRESSURE);
> set_abs(hi->input, ABS_MT_PRESSURE, field,
> cls->sn_pressure);
> - /* touchscreen emulation */
> - set_abs(hi->input, ABS_PRESSURE, field,
> - cls->sn_pressure);
> mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> @@ -464,7 +428,24 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
> return -1;
> }
>
> -static int mt_compute_slot(struct mt_device *td)
> +static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
> +
> +{
> + struct mt_device *td = hid_get_drvdata(hdev);
> + struct mt_class *cls = &td->mtclass;
> +
> + if (cls->is_indirect)
> + td->mt_flags |= INPUT_MT_POINTER;
> +
> + if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
> + td->mt_flags |= INPUT_MT_DROP_UNUSED;
> +
> + input_mt_init_slots(hi->input, td->maxcontacts, td->mt_flags);
> +
> + td->mt_flags = 0;
> +}
> +
> +static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
> {
> __s32 quirks = td->mtclass.quirks;
>
> @@ -480,42 +461,23 @@ static int mt_compute_slot(struct mt_device *td)
> if (quirks & MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE)
> return td->curdata.contactid - 1;
>
> - return find_slot_from_contactid(td);
> + return input_mt_assign_slot_by_id(input, td->curdata.contactid);
> }
>
> /*
> * this function is called when a whole contact has been processed,
> * so that it can assign it to a slot and store the data there
> */
> -static void mt_complete_slot(struct mt_device *td)
> +static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> {
> - td->curdata.seen_in_this_frame = true;
> if (td->curvalid) {
> - int slotnum = mt_compute_slot(td);
> -
> - if (slotnum >= 0 && slotnum < td->maxcontacts)
> - td->slots[slotnum] = td->curdata;
> - }
> - td->num_received++;
> -}
> + int slotnum = mt_compute_slot(td, input);
> + struct mt_slot *s = &td->curdata;
>
> + if (slotnum < 0 || slotnum >= td->maxcontacts)
> + return;
>
> -/*
> - * this function is called when a whole packet has been received and processed,
> - * so that it can decide what to send to the input layer.
> - */
> -static void mt_emit_event(struct mt_device *td, struct input_dev *input)
> -{
> - int i;
> -
> - for (i = 0; i < td->maxcontacts; ++i) {
> - struct mt_slot *s = &(td->slots[i]);
> - if ((td->mtclass.quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) &&
> - !s->seen_in_this_frame) {
> - s->touch_state = false;
> - }
> -
> - input_mt_slot(input, i);
> + input_mt_slot(input, slotnum);
> input_mt_report_slot_state(input, MT_TOOL_FINGER,
> s->touch_state);
> if (s->touch_state) {
> @@ -532,16 +494,22 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
> input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> }
> - s->seen_in_this_frame = false;
> -
> }
>
> - input_mt_report_pointer_emulation(input, true);
> - input_sync(input);
> - td->num_received = 0;
> + td->num_received++;
> }
>
>
> +/*
> + * this function is called when a whole packet has been received and processed,
> + * so that it can decide what to send to the input layer.
> + */
> +static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
> +{
> + input_mt_sync_frame(input);
> + input_sync(input);
> + td->num_received = 0;
> +}
>
> static int mt_event(struct hid_device *hid, struct hid_field *field,
> struct hid_usage *usage, __s32 value)
> @@ -549,7 +517,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> struct mt_device *td = hid_get_drvdata(hid);
> __s32 quirks = td->mtclass.quirks;
>
> - if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {
> + if (hid->claimed & HID_CLAIMED_INPUT) {
> switch (usage->hid) {
> case HID_DG_INRANGE:
> if (quirks & MT_QUIRK_ALWAYS_VALID)
> @@ -602,11 +570,11 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> }
>
> if (usage->hid == td->last_slot_field)
> - mt_complete_slot(td);
> + mt_complete_slot(td, field->hidinput->input);
>
> if (field->index == td->last_field_index
> && td->num_received >= td->num_expected)
> - mt_emit_event(td, field->hidinput->input);
> + mt_sync_frame(td, field->hidinput->input);
>
> }
>
> @@ -735,15 +703,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
> mt_post_parse_default_settings(td);
>
> - td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
> - GFP_KERNEL);
> - if (!td->slots) {
> - dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
> - hid_hw_stop(hdev);
> - ret = -ENOMEM;
> - goto fail;
> - }
> -
> ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
>
> mt_set_maxcontacts(hdev);
> @@ -774,7 +733,6 @@ static void mt_remove(struct hid_device *hdev)
> struct mt_device *td = hid_get_drvdata(hdev);
> sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
> hid_hw_stop(hdev);
> - kfree(td->slots);
> kfree(td);
> hid_set_drvdata(hdev, NULL);
> }
> @@ -1087,6 +1045,7 @@ static struct hid_driver mt_driver = {
> .remove = mt_remove,
> .input_mapping = mt_input_mapping,
> .input_mapped = mt_input_mapped,
> + .input_configured = mt_input_configured,
> .feature_mapping = mt_feature_mapping,
> .usage_table = mt_grabbed_usages,
> .event = mt_event,
> --
> 1.7.11.5
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v2] HID: multitouch: Remove the redundant touch state
2012-08-28 22:25 ` Jiri Kosina
@ 2012-08-29 13:36 ` Benjamin Tissoires
2012-08-29 17:18 ` Jiri Kosina
0 siblings, 1 reply; 45+ messages in thread
From: Benjamin Tissoires @ 2012-08-29 13:36 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Henrik Rydberg, Dmitry Torokhov, linux-input, linux-kernel
Hi Jiri,
On Wed, Aug 29, 2012 at 12:25 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 22 Aug 2012, Henrik Rydberg wrote:
>
>> With the input_mt_sync_frame() function in place, there is no longer
>> any need to keep the full touch state in the driver. This patch
>> removes the slot state and replaces the lookup code with the input-mt
>> equivalent. The initialization code is moved to mt_input_configured(),
>> to make sure the full HID report has been seen.
>>
>> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
>> ---
>> Hi Benjamin,
>>
>> Maybe this patch works better? It has received limited testing so far.
>
> What is the status of this patch please? Henrik, Benjamin?
Well, Henrik submitted a new release a few days ago (including this version).
I just didn't found the time to test the whole thing on our different devices.
It's now on the top of my TODO list.
Cheers,
Benjamin
>
>>
>> Henrik
>>
>> drivers/hid/hid-multitouch.c | 133 +++++++++++++++----------------------------
>> 1 file changed, 46 insertions(+), 87 deletions(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index c400d90..dc08a4e 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -56,7 +56,6 @@ struct mt_slot {
>> __s32 x, y, p, w, h;
>> __s32 contactid; /* the device ContactID assigned to this slot */
>> bool touch_state; /* is the touch valid? */
>> - bool seen_in_this_frame;/* has this slot been updated */
>> };
>>
>> struct mt_class {
>> @@ -93,7 +92,7 @@ struct mt_device {
>> * 1 means we should use a serial protocol
>> * > 1 means hybrid (multitouch) protocol */
>> bool curvalid; /* is the current contact valid? */
>> - struct mt_slot *slots;
>> + unsigned mt_flags; /* flags to pass to input-mt */
>> };
>>
>> /* classes of device behavior */
>> @@ -134,25 +133,6 @@ static int cypress_compute_slot(struct mt_device *td)
>> return -1;
>> }
>>
>> -static int find_slot_from_contactid(struct mt_device *td)
>> -{
>> - int i;
>> - for (i = 0; i < td->maxcontacts; ++i) {
>> - if (td->slots[i].contactid == td->curdata.contactid &&
>> - td->slots[i].touch_state)
>> - return i;
>> - }
>> - for (i = 0; i < td->maxcontacts; ++i) {
>> - if (!td->slots[i].seen_in_this_frame &&
>> - !td->slots[i].touch_state)
>> - return i;
>> - }
>> - /* should not occurs. If this happens that means
>> - * that the device sent more touches that it says
>> - * in the report descriptor. It is ignored then. */
>> - return -1;
>> -}
>> -
>> static struct mt_class mt_classes[] = {
>> { .name = MT_CLS_DEFAULT,
>> .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
>> @@ -319,24 +299,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> * We need to ignore fields that belong to other collections
>> * such as Mouse that might have the same GenericDesktop usages. */
>> if (field->application == HID_DG_TOUCHSCREEN)
>> - set_bit(INPUT_PROP_DIRECT, hi->input->propbit);
>> + td->mt_flags |= INPUT_MT_DIRECT;
>> else if (field->application != HID_DG_TOUCHPAD)
>> return 0;
>>
>> - /* In case of an indirect device (touchpad), we need to add
>> - * specific BTN_TOOL_* to be handled by the synaptics xorg
>> - * driver.
>> - * We also consider that touchscreens providing buttons are touchpads.
>> + /*
>> + * Model touchscreens providing buttons as touchpads.
>> */
>> if (field->application == HID_DG_TOUCHPAD ||
>> - (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON ||
>> - cls->is_indirect) {
>> - set_bit(INPUT_PROP_POINTER, hi->input->propbit);
>> - set_bit(BTN_TOOL_FINGER, hi->input->keybit);
>> - set_bit(BTN_TOOL_DOUBLETAP, hi->input->keybit);
>> - set_bit(BTN_TOOL_TRIPLETAP, hi->input->keybit);
>> - set_bit(BTN_TOOL_QUADTAP, hi->input->keybit);
>> - }
>> + (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON)
>> + td->mt_flags |= INPUT_MT_POINTER;
>>
>> /* eGalax devices provide a Digitizer.Stylus input which overrides
>> * the correct Digitizers.Finger X/Y ranges.
>> @@ -353,8 +325,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> EV_ABS, ABS_MT_POSITION_X);
>> set_abs(hi->input, ABS_MT_POSITION_X, field,
>> cls->sn_move);
>> - /* touchscreen emulation */
>> - set_abs(hi->input, ABS_X, field, cls->sn_move);
>> mt_store_field(usage, td, hi);
>> td->last_field_index = field->index;
>> return 1;
>> @@ -363,8 +333,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> EV_ABS, ABS_MT_POSITION_Y);
>> set_abs(hi->input, ABS_MT_POSITION_Y, field,
>> cls->sn_move);
>> - /* touchscreen emulation */
>> - set_abs(hi->input, ABS_Y, field, cls->sn_move);
>> mt_store_field(usage, td, hi);
>> td->last_field_index = field->index;
>> return 1;
>> @@ -390,7 +358,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> case HID_DG_CONTACTID:
>> if (!td->maxcontacts)
>> td->maxcontacts = MT_DEFAULT_MAXCONTACT;
>> - input_mt_init_slots(hi->input, td->maxcontacts, 0);
>> mt_store_field(usage, td, hi);
>> td->last_field_index = field->index;
>> td->touches_by_report++;
>> @@ -418,9 +385,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> EV_ABS, ABS_MT_PRESSURE);
>> set_abs(hi->input, ABS_MT_PRESSURE, field,
>> cls->sn_pressure);
>> - /* touchscreen emulation */
>> - set_abs(hi->input, ABS_PRESSURE, field,
>> - cls->sn_pressure);
>> mt_store_field(usage, td, hi);
>> td->last_field_index = field->index;
>> return 1;
>> @@ -464,7 +428,24 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>> return -1;
>> }
>>
>> -static int mt_compute_slot(struct mt_device *td)
>> +static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
>> +
>> +{
>> + struct mt_device *td = hid_get_drvdata(hdev);
>> + struct mt_class *cls = &td->mtclass;
>> +
>> + if (cls->is_indirect)
>> + td->mt_flags |= INPUT_MT_POINTER;
>> +
>> + if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
>> + td->mt_flags |= INPUT_MT_DROP_UNUSED;
>> +
>> + input_mt_init_slots(hi->input, td->maxcontacts, td->mt_flags);
>> +
>> + td->mt_flags = 0;
>> +}
>> +
>> +static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
>> {
>> __s32 quirks = td->mtclass.quirks;
>>
>> @@ -480,42 +461,23 @@ static int mt_compute_slot(struct mt_device *td)
>> if (quirks & MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE)
>> return td->curdata.contactid - 1;
>>
>> - return find_slot_from_contactid(td);
>> + return input_mt_assign_slot_by_id(input, td->curdata.contactid);
>> }
>>
>> /*
>> * this function is called when a whole contact has been processed,
>> * so that it can assign it to a slot and store the data there
>> */
>> -static void mt_complete_slot(struct mt_device *td)
>> +static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> {
>> - td->curdata.seen_in_this_frame = true;
>> if (td->curvalid) {
>> - int slotnum = mt_compute_slot(td);
>> -
>> - if (slotnum >= 0 && slotnum < td->maxcontacts)
>> - td->slots[slotnum] = td->curdata;
>> - }
>> - td->num_received++;
>> -}
>> + int slotnum = mt_compute_slot(td, input);
>> + struct mt_slot *s = &td->curdata;
>>
>> + if (slotnum < 0 || slotnum >= td->maxcontacts)
>> + return;
>>
>> -/*
>> - * this function is called when a whole packet has been received and processed,
>> - * so that it can decide what to send to the input layer.
>> - */
>> -static void mt_emit_event(struct mt_device *td, struct input_dev *input)
>> -{
>> - int i;
>> -
>> - for (i = 0; i < td->maxcontacts; ++i) {
>> - struct mt_slot *s = &(td->slots[i]);
>> - if ((td->mtclass.quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) &&
>> - !s->seen_in_this_frame) {
>> - s->touch_state = false;
>> - }
>> -
>> - input_mt_slot(input, i);
>> + input_mt_slot(input, slotnum);
>> input_mt_report_slot_state(input, MT_TOOL_FINGER,
>> s->touch_state);
>> if (s->touch_state) {
>> @@ -532,16 +494,22 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
>> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>> input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
>> }
>> - s->seen_in_this_frame = false;
>> -
>> }
>>
>> - input_mt_report_pointer_emulation(input, true);
>> - input_sync(input);
>> - td->num_received = 0;
>> + td->num_received++;
>> }
>>
>>
>> +/*
>> + * this function is called when a whole packet has been received and processed,
>> + * so that it can decide what to send to the input layer.
>> + */
>> +static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
>> +{
>> + input_mt_sync_frame(input);
>> + input_sync(input);
>> + td->num_received = 0;
>> +}
>>
>> static int mt_event(struct hid_device *hid, struct hid_field *field,
>> struct hid_usage *usage, __s32 value)
>> @@ -549,7 +517,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>> struct mt_device *td = hid_get_drvdata(hid);
>> __s32 quirks = td->mtclass.quirks;
>>
>> - if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {
>> + if (hid->claimed & HID_CLAIMED_INPUT) {
>> switch (usage->hid) {
>> case HID_DG_INRANGE:
>> if (quirks & MT_QUIRK_ALWAYS_VALID)
>> @@ -602,11 +570,11 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>> }
>>
>> if (usage->hid == td->last_slot_field)
>> - mt_complete_slot(td);
>> + mt_complete_slot(td, field->hidinput->input);
>>
>> if (field->index == td->last_field_index
>> && td->num_received >= td->num_expected)
>> - mt_emit_event(td, field->hidinput->input);
>> + mt_sync_frame(td, field->hidinput->input);
>>
>> }
>>
>> @@ -735,15 +703,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
>> mt_post_parse_default_settings(td);
>>
>> - td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
>> - GFP_KERNEL);
>> - if (!td->slots) {
>> - dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
>> - hid_hw_stop(hdev);
>> - ret = -ENOMEM;
>> - goto fail;
>> - }
>> -
>> ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
>>
>> mt_set_maxcontacts(hdev);
>> @@ -774,7 +733,6 @@ static void mt_remove(struct hid_device *hdev)
>> struct mt_device *td = hid_get_drvdata(hdev);
>> sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
>> hid_hw_stop(hdev);
>> - kfree(td->slots);
>> kfree(td);
>> hid_set_drvdata(hdev, NULL);
>> }
>> @@ -1087,6 +1045,7 @@ static struct hid_driver mt_driver = {
>> .remove = mt_remove,
>> .input_mapping = mt_input_mapping,
>> .input_mapped = mt_input_mapped,
>> + .input_configured = mt_input_configured,
>> .feature_mapping = mt_feature_mapping,
>> .usage_table = mt_grabbed_usages,
>> .event = mt_event,
>> --
>> 1.7.11.5
>>
>
> --
> Jiri Kosina
> SUSE Labs
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v2] HID: multitouch: Remove the redundant touch state
2012-08-29 13:36 ` Benjamin Tissoires
@ 2012-08-29 17:18 ` Jiri Kosina
0 siblings, 0 replies; 45+ messages in thread
From: Jiri Kosina @ 2012-08-29 17:18 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Henrik Rydberg, Dmitry Torokhov, linux-input, linux-kernel
On Wed, 29 Aug 2012, Benjamin Tissoires wrote:
> >> With the input_mt_sync_frame() function in place, there is no longer
> >> any need to keep the full touch state in the driver. This patch
> >> removes the slot state and replaces the lookup code with the input-mt
> >> equivalent. The initialization code is moved to mt_input_configured(),
> >> to make sure the full HID report has been seen.
> >>
> >> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> >> ---
> >> Hi Benjamin,
> >>
> >> Maybe this patch works better? It has received limited testing so far.
> >
> > What is the status of this patch please? Henrik, Benjamin?
>
> Well, Henrik submitted a new release a few days ago (including this version).
> I just didn't found the time to test the whole thing on our different devices.
> It's now on the top of my TODO list.
Ah, I have missed the fact that this one is also part of Henrik's series,
sorry for the noise.
I haven't unfortunately reviewed the series yet due to kernel summit &
related events, but I'll get to it shortly.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 45+ messages in thread