| Message ID | 1301691819-2090-1-git-send-email-chase.douglas@canonical.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | cc5e0f08ca2a66fc4c6984ccff74fd529e969fac |
| Headers | show |
On 04/01/2011 07:51 PM, Jeffrey Brown wrote: > I've got another change in the works that fixes this problem more > systematically. That would be great :). I still would like to see this patch added so it's released with the 2.6.39 kernel and backported to earlier stable releases. I hope that's not a problem. Thanks, -- Chase > On Fri, Apr 1, 2011 at 2:03 PM, Chase Douglas > <chase.douglas@canonical.com> wrote: >> The evdev buffer isn't big enough when you get many fingers on the >> device. Bump up the buffer to a reasonable size, matching what other >> multitouch devices use. Without this change, events may be discarded in >> the evdev buffer before they are read. >> >> Reported-by: Simon Budig <simon@budig.de> >> Cc: Henrik Rydberg <rydberg@euromail.se> >> Cc: Jiri Kosina <jkosina@suse.cz> >> Cc: stable@kernel.org >> Signed-off-by: Chase Douglas <chase.douglas@canonical.com> >> --- >> Forgot to Cc the mailing lists on the first send. >> >> drivers/hid/hid-magicmouse.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c >> index 698e645..404dcbc 100644 >> --- a/drivers/hid/hid-magicmouse.c >> +++ b/drivers/hid/hid-magicmouse.c >> @@ -418,6 +418,8 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h >> input_set_abs_params(input, ABS_MT_POSITION_Y, -2456, >> 2565, 4, 0); >> } >> + >> + input_set_events_per_packet(input, 60); >> } >> >> if (report_undeciphered) { >> -- >> 1.7.4.1 >> >> -- >> 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 >> > -- 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
Sorry, I originally sent a reply to the list from my phone but it was HTML formatted for some reason and the list server rejected it. Meh. On Mon, Apr 4, 2011 at 5:43 AM, Jiri Kosina <jkosina@suse.cz> wrote: > Could you just briefly describe what your solution is? I have sent a series of 4 patches to linux-input: * [PATCH v2 1/4] input: Set default events per packet. This patch modifies input_register_device to calculate the number of events per packet if the driver did not already supply a hint. Roughly speaking, it examines the number of MT_SLOTs or ABS_MT_TRACKING_IDs and multiplies that by the total number of MT axes, then adds in a bit for non-MT axes, REL axes and SYN events. The intent is to systematically solve the problem of determining the number of events per packet so that most drivers won't need to set hints themselves. This fixes the magic mouse issue and related problems in other drivers. * [PATCH v2 2/4] hid: hid-input: Remove obsolete default events per packet setting. Given the first patch, the HID driver no longer needs to hardcode a default events per packet value. * [PATCH v2 3/4] input: evdev: Indicate buffer overrun with SYN_DROPPED. I first discovered that the evdev buffer was too small because of the Apple Magic Trackpad. With more than 3 fingers, some points would regularly get dropped and would interrupt the gestures I was trying to detect. User space code got really confused since there was no indication that anything unusual had happened and the event streams effectively appeared to be truncated and concatenated arbitrarily. So this patch introduces a new type of SYN event code, SYN_DROPPED, to mark the point in the event stream where events were dropped. The most recent version of this patch also takes care to drop the entire last packet, so the next event received by user-space will be the first event of the following compete packet. * [PATCH v2 4/4] input: evdev: Make device readable only when it contains a complete packet. There is a significant performance issue on SMP systems where the driver and user-space code can race against one another to fill and drain the evdev buffer in parallel. It is possible for a client waiting on poll() to wake up and read each event published by the driver immediately as it becomes available. That could mean 60 or more calls to poll() and read() to read an entire packet. It would be preferable if the client were only awoken when the entire packet were available so it could just make 1 call to poll() and read() everything all at once. No changes are required on the part of the client. This patch changes when evdev considers itself readable. Essentially evdev is made to be readable only up to the last non-MT SYN event it contains. That way the client can only read events that belong to packets which have been completely buffered. The client can still read events one at a time if it likes. The client will only wake from poll() when the buffer contains a non-MT SYN. This patch works well in combination with the SYN_DROPPED change above. > I'd happuly take Chase's patch, but want to make sure that we don't cause > any changes that would make backwards compatilibity an issue later. There should be no compatibility issues. However, we might be better off in the long term taking (some variation of) these patches instead. Jeff. -- 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
On Mon, Apr 04, 2011 at 02:55:03PM -0400, Chase Douglas wrote: > On 04/04/2011 02:13 PM, Jeffrey Brown wrote: > > On Mon, Apr 4, 2011 at 5:43 AM, Jiri Kosina <jkosina@suse.cz> wrote: > >> I'd happuly take Chase's patch, but want to make sure that we don't cause > >> any changes that would make backwards compatilibity an issue later. > > > > There should be no compatibility issues. However, we might be better > > off in the long term taking (some variation of) these patches instead. > > I like the proposed changes, but I want to ensure stable kernel releases > aren't left out of the fix for hid-magicmouse. I don't know the best way > forward, but here's one possibility: > > 1. Apply my patch to manually set the buffer size hint > 2. It gets sent to stable trees due to the 'Cc: stable@kernel.org' line > 3. Apply Jeffrey's patches, including a reversion of my buffer size hint > > Obviously the extra application and reversion is odd, but this seems the > easiest way forward given that the patches already exist and can be > applied without issue. > Or, once Jeffrey's patches hit mainline, send your change to stable with the explanation why it is needed for stable but not for mainline. Thanks.
On 04/04/2011 05:39 PM, Dmitry Torokhov wrote: > On Mon, Apr 04, 2011 at 02:55:03PM -0400, Chase Douglas wrote: >> On 04/04/2011 02:13 PM, Jeffrey Brown wrote: >>> On Mon, Apr 4, 2011 at 5:43 AM, Jiri Kosina <jkosina@suse.cz> wrote: >>>> I'd happuly take Chase's patch, but want to make sure that we don't cause >>>> any changes that would make backwards compatilibity an issue later. >>> >>> There should be no compatibility issues. However, we might be better >>> off in the long term taking (some variation of) these patches instead. >> >> I like the proposed changes, but I want to ensure stable kernel releases >> aren't left out of the fix for hid-magicmouse. I don't know the best way >> forward, but here's one possibility: >> >> 1. Apply my patch to manually set the buffer size hint >> 2. It gets sent to stable trees due to the 'Cc: stable@kernel.org' line >> 3. Apply Jeffrey's patches, including a reversion of my buffer size hint >> >> Obviously the extra application and reversion is odd, but this seems the >> easiest way forward given that the patches already exist and can be >> applied without issue. >> > > Or, once Jeffrey's patches hit mainline, send your change to stable with > the explanation why it is needed for stable but not for mainline. I'm fine with this too. I'll watch the list to see what happens. Thanks, -- Chase -- 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
On 04/04/2011 05:39 PM, Dmitry Torokhov wrote: > On Mon, Apr 04, 2011 at 02:55:03PM -0400, Chase Douglas wrote: >> On 04/04/2011 02:13 PM, Jeffrey Brown wrote: >>> On Mon, Apr 4, 2011 at 5:43 AM, Jiri Kosina <jkosina@suse.cz> wrote: >>>> I'd happuly take Chase's patch, but want to make sure that we don't cause >>>> any changes that would make backwards compatilibity an issue later. >>> >>> There should be no compatibility issues. However, we might be better >>> off in the long term taking (some variation of) these patches instead. >> >> I like the proposed changes, but I want to ensure stable kernel releases >> aren't left out of the fix for hid-magicmouse. I don't know the best way >> forward, but here's one possibility: >> >> 1. Apply my patch to manually set the buffer size hint >> 2. It gets sent to stable trees due to the 'Cc: stable@kernel.org' line >> 3. Apply Jeffrey's patches, including a reversion of my buffer size hint >> >> Obviously the extra application and reversion is odd, but this seems the >> easiest way forward given that the patches already exist and can be >> applied without issue. >> > > Or, once Jeffrey's patches hit mainline, send your change to stable with > the explanation why it is needed for stable but not for mainline. One thing that crossed my mind is that Jeffrey's patches wouldn't be merged until 2.6.40, right? If so, even 2.6.39 will be released with this bug, which makes me want to go with the plan I outlined above. Thanks, -- Chase -- 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
On Tue, 5 Apr 2011, Chase Douglas wrote: > >>>> I'd happuly take Chase's patch, but want to make sure that we don't cause > >>>> any changes that would make backwards compatilibity an issue later. > >>> > >>> There should be no compatibility issues. However, we might be better > >>> off in the long term taking (some variation of) these patches instead. > >> > >> I like the proposed changes, but I want to ensure stable kernel releases > >> aren't left out of the fix for hid-magicmouse. I don't know the best way > >> forward, but here's one possibility: > >> > >> 1. Apply my patch to manually set the buffer size hint > >> 2. It gets sent to stable trees due to the 'Cc: stable@kernel.org' line > >> 3. Apply Jeffrey's patches, including a reversion of my buffer size hint > >> > >> Obviously the extra application and reversion is odd, but this seems the > >> easiest way forward given that the patches already exist and can be > >> applied without issue. > >> > > > > Or, once Jeffrey's patches hit mainline, send your change to stable with > > the explanation why it is needed for stable but not for mainline. > > One thing that crossed my mind is that Jeffrey's patches wouldn't be > merged until 2.6.40, right? If so, even 2.6.39 will be released with > this bug, which makes me want to go with the plan I outlined above. OK, applied. Thanks everybody.
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index 698e645..404dcbc 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -418,6 +418,8 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h input_set_abs_params(input, ABS_MT_POSITION_Y, -2456, 2565, 4, 0); } + + input_set_events_per_packet(input, 60); } if (report_undeciphered) {