Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Ping Cheng
On Tue, Aug 14, 2012 at 2:12 PM, Dmitry Torokhov
 wrote:
> >> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg  
> >> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Dmitry Torokhov
On Tue, Aug 14, 2012 at 01:50:38PM -0700, Ping Cheng wrote:
> On Tue, Aug 14, 2012 at 12:53 PM, Dmitry Torokhov
>  wrote:
> > On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote:
> >> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg  
> >> 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 
> >> > ---
> >> >
> >> >  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Ping Cheng
On Tue, Aug 14, 2012 at 1:01 PM, Henrik Rydberg  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Ping Cheng
On Tue, Aug 14, 2012 at 12:53 PM, Dmitry Torokhov
 wrote:
> On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote:
>> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg  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 
>> > ---
>> >
>> >  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Henrik Rydberg
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Dmitry Torokhov
On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote:
> On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg  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 
> > ---
> > 
> >  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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Ping Cheng
On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg  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 
> ---
>  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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Ping Cheng
On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg rydb...@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 rydb...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Dmitry Torokhov
On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote:
 On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg rydb...@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 rydb...@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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Henrik Rydberg
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Ping Cheng
On Tue, Aug 14, 2012 at 12:53 PM, Dmitry Torokhov
dmitry.torok...@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 rydb...@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 rydb...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Ping Cheng
On Tue, Aug 14, 2012 at 1:01 PM, Henrik Rydberg rydb...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Dmitry Torokhov
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.torok...@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 rydb...@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 rydb...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-14 Thread Ping Cheng
On Tue, Aug 14, 2012 at 2:12 PM, Dmitry Torokhov
dmitry.torok...@gmail.com wrote:
  On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg rydb...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-12 Thread 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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 02/19] Input: Improve the events-per-packet estimate

2012-08-12 Thread 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 rydb...@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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/