| Message ID | 1342606923-9997-1-git-send-email-cywang@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/18/2012 03:22 AM, Chung-yih Wang wrote: > The profile sensor clickpad in a Cr-48 Chromebook does a reasonable job of > tracking individual fingers. This tracking isn't perfect, but, experiments > show that it works better than just passing "semi-mt" data to userspace, > and making userspace try to deduce where the fingers are given a bounding box. > > This patch tries to report correct two-finger positions instead of the > {(min_x, min_y), (max_x, max_y)} for profile sensor clickpads on Cr-48 > chromebooks. Note that this device's firmware always reports the higher > (smaller y) finger in the "sgm" packet, and the lower (larger y) finger in the > "agm" packet. Thus, when a new finger arrives on the pad, the kernel driver > uses a simple Euclidean distance measure to deduce which of the two new fingers > should keep the tracking ID of the previous single finger. Similarly, when one > finger is removed, the same measure is used to determine which finger remained > on the pad. Can it keep track of the touches as you rotate them past each other in both the X and Y axes? If not, then it should remain a semi-mt device. Even if you can guess which touch is which when a second touch is added, you will lose track of it when the user attempts to perform a rotation. Semi-mt is our only mechanism for telling userspace that the device can't accurately tell us about rotations. We could create a new device property to say: "This device kinda sorta tells us enough info usually to know where two touches are initially." I don't think the effort is worth it though. What is the point of providing the exact locations on a trackpad if they can't be used for rotation? -- 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
Hi Chung-Yih, On Thu, Jul 19, 2012 at 11:02:23AM +0800, Chung-Yih Wang (???) wrote: > Hi Chase, > > Thanks for your great comments. You are right, it is impossible to get > correct finger tracking if both fingers are moving. However, we think it > still worth to have the firmware tracking of the fingers as they could > perform well for most one-stationary-one-moving cases. This will be good > enough for the one-stationary-one-moving gestures we want to provide on > Cr-48. And that's why we want to make the patch specific to Cr-48. > First of all if the patch makes sense for Cr-48 then other boxes using the same touchpad with the same sensor would also benefit from it. However it is still not clear to me what coordinates are being reported by the device? You say that it always reports finger with smaller Y in SGM packet, but what about X coordinate? Could it be that the most benefit from your patch is because it reports proper slot from 2->1 finger transitions? BTW, you seem to have butchered single-touch protocol reporting for your device. Thanks.
> Thanks for your great comments. You are right, it is impossible to get > correct finger tracking if both fingers are moving. However, we think it > still worth to have the firmware tracking of the fingers as they could > perform well for most one-stationary-one-moving cases. This will be good > enough for the one-stationary-one-moving gestures we want to provide on > Cr-48. And that's why we want to make the patch specific to Cr-48. If one finger is stationary, it is trivial to find out where the other finger is using the available semi-mt data. The general feeling about this patch is that it is very similar to where we started off two years ago. The problems we saw then led to the implementation of the semi-mt protocol. I doubt things have changed much since then. Thanks, Henrik -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/18/2012 08:02 PM, Chung-Yih Wang (???) wrote: > Hi Chase, > > Thanks for your great comments. You are right, it is impossible to > get correct finger tracking if both fingers are moving. However, we > think it still worth to have the firmware tracking of the fingers as > they could perform well for most one-stationary-one-moving cases. This > will be good enough for the one-stationary-one-moving gestures we want > to provide on Cr-48. And that's why we want to make the patch specific > to Cr-48. Can you provide more details on what you are attempting to accomplish? The only thing you can't do easily with semi-mt is: * Rotations (but the hardware can't do it anyway, so this is moot) * Pinch directions at 45 degree angles (i.e. (pinch up and left, down and right) or (pinch down and left, up and right)) You should be able to do everything else with semi-mt, such as pinch horizontally vs pinch vertically, movement, etc. The pinch directions at 45 degree angles could be possible with the Cr-48 trackpad, but I would want to know that there's a real use case for it before undertaking an effort to support it. -- 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 Fri, Jul 20, 2012 at 12:16 AM, Chase Douglas <chase.douglas@canonical.com> wrote: > > On 07/18/2012 08:02 PM, Chung-Yih Wang (???) wrote: >> >> Hi Chase, >> >> Thanks for your great comments. You are right, it is impossible to >> get correct finger tracking if both fingers are moving. However, we >> think it still worth to have the firmware tracking of the fingers as >> they could perform well for most one-stationary-one-moving cases. This >> will be good enough for the one-stationary-one-moving gestures we want >> to provide on Cr-48. And that's why we want to make the patch specific >> to Cr-48. > > > Can you provide more details on what you are attempting to accomplish? The Cr-48 profile sensor is also a Clickpad. Thus, one of the most common user gestures is to click the pad with one finger (to start a selection) and then swipe a second (almost always "upper") finger across the pad to extend the selection (or move a selected object). The semi-mt approach is breaking down when the swiping finger moves horizontally and "crosses over top" of a lower "stationary" finger. For example, if a user clicks their finger in the bottom center of the pad, and while using a second finger to horizontally extend a selection, the fingers cross in the X direction. As the moving finger approaches the same X coordinate as the stationary finger, the reported position of the bottom finger will start to move significantly towards the upper finger due to a "pulling" affect of the profile sensor. Thus, that bottom, stationary finger starts to move towards the finger that is actually moving. Eventually, when the moving finger gets close enough to the stationary finger, the reported X coordinate of the two fingers becomes the same - the two fingers' reported X coordinates "merge". As the moving finger continues to move towards, over and past the lower finger, the reported position of the two fingers moves together, until the moving finger gets significantly far enough away (in X) from the lower finger, at which point the lower finger's reported position starts moving back to its actual position. Due to this effect, when using semi-mt, it is very difficult to know at which point the "finger pattern" of the bounding box changes; in other words, when the fingers change from "BottomLeft / TopRight" to "BottomRight / TopLeft". It can be approximated by assuming that the "finger pattern" changes when the merged X coordinate passes over the original starting point of the bottom finger. However, this approximation only holds when the bottom finger is perfectly stationary. In the real world, the bottom finger rolls/wiggles or otherwise moves, causing the actual crossing point to change. Or, as is also likely, the lower finger is already being pulled when the upper finger starts moving, so its reported position is already not at the correct crossing point. Thus, using semi-mt with this profile sensor clickpad, we have not been able to generate smooth pointer motion when an upper finger crosses a lower finger. What we found, though, is that the firmware does do a much better job of tracking such horizontal crosses. Therefore, we would like to get this raw data from the kernel. It is trivial for userspace to convert the raw finger position data to a bounding box format for use with other gestures. In any case, we really aren't that familiar with how other "synaptics semi-mt-compatible" touchpads work. Using this patch may or may not be better than semi-mt. Or, their firmware may or may not follow the same "always report upper finger in sgm" rule (we have seen other Synaptics trackpads that do "always report oldest finger in sgm"). This is why we chose to isolate this change to just one hardware type, so that the change, which improves the experience of a Cr-48 user, has no deleterious affect on users of any other hardware. If others try this method with their hardware and it works for them, then great! We would be happy to help review additional patches that extend it to additional systems. -Daniel > The only thing you can't do easily with semi-mt is: > > * Rotations (but the hardware can't do it anyway, so this is moot) > * Pinch directions at 45 degree angles (i.e. (pinch up and left, down and > right) or (pinch down and left, up and right)) > > You should be able to do everything else with semi-mt, such as pinch > horizontally vs pinch vertically, movement, etc. > > The pinch directions at 45 degree angles could be possible with the Cr-48 > trackpad, but I would want to know that there's a real use case for it > before undertaking an effort to support it. > > -- 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 07/19/2012 10:05 AM, Daniel Kurtz wrote: > On Fri, Jul 20, 2012 at 12:16 AM, Chase Douglas > <chase.douglas@canonical.com> wrote: >> >> On 07/18/2012 08:02 PM, Chung-Yih Wang (???) wrote: >>> >>> Hi Chase, >>> >>> Thanks for your great comments. You are right, it is impossible to >>> get correct finger tracking if both fingers are moving. However, we >>> think it still worth to have the firmware tracking of the fingers as >>> they could perform well for most one-stationary-one-moving cases. This >>> will be good enough for the one-stationary-one-moving gestures we want >>> to provide on Cr-48. And that's why we want to make the patch specific >>> to Cr-48. >> >> >> Can you provide more details on what you are attempting to accomplish? > > The Cr-48 profile sensor is also a Clickpad. Thus, one of the most > common user gestures is to click the pad with one finger (to start a > selection) and then swipe a second (almost always "upper") finger > across the pad to extend the selection (or move a selected object). > > The semi-mt approach is breaking down when the swiping finger moves > horizontally and "crosses over top" of a lower "stationary" finger. > For example, if a user clicks their finger in the bottom center of the > pad, and while using a second finger to horizontally extend a > selection, the fingers cross in the X direction. > > As the moving finger approaches the same X coordinate as the > stationary finger, the reported position of the bottom finger will > start to move significantly towards the upper finger due to a > "pulling" affect of the profile sensor. Thus, that bottom, stationary > finger starts to move towards the finger that is actually moving. > Eventually, when the moving finger gets close enough to the stationary > finger, the reported X coordinate of the two fingers becomes the same > - the two fingers' reported X coordinates "merge". As the moving > finger continues to move towards, over and past the lower finger, the > reported position of the two fingers moves together, until the moving > finger gets significantly far enough away (in X) from the lower > finger, at which point the lower finger's reported position starts > moving back to its actual position. > > Due to this effect, when using semi-mt, it is very difficult to know > at which point the "finger pattern" of the bounding box changes; in > other words, when the fingers change from "BottomLeft / TopRight" to > "BottomRight / TopLeft". It can be approximated by assuming that the > "finger pattern" changes when the merged X coordinate passes over the > original starting point of the bottom finger. However, this > approximation only holds when the bottom finger is perfectly > stationary. In the real world, the bottom finger rolls/wiggles or > otherwise moves, causing the actual crossing point to change. Or, as > is also likely, the lower finger is already being pulled when the > upper finger starts moving, so its reported position is already not at > the correct crossing point. Thus, using semi-mt with this profile > sensor clickpad, we have not been able to generate smooth pointer > motion when an upper finger crosses a lower finger. > > What we found, though, is that the firmware does do a much better job > of tracking such horizontal crosses. Therefore, we would like to get > this raw data from the kernel. It is trivial for userspace to convert > the raw finger position data to a bounding box format for use with > other gestures. > > In any case, we really aren't that familiar with how other "synaptics > semi-mt-compatible" touchpads work. Using this patch may or may not > be better than semi-mt. Or, their firmware may or may not follow the > same "always report upper finger in sgm" rule (we have seen other > Synaptics trackpads that do "always report oldest finger in sgm"). > This is why we chose to isolate this change to just one hardware type, > so that the change, which improves the experience of a Cr-48 user, has > no deleterious affect on users of any other hardware. > > If others try this method with their hardware and it works for them, > then great! We would be happy to help review additional patches that > extend it to additional systems. Ok, that's very helpful to know, thanks for providing such a detailed response! So the problem is that you want to support the kernel providing two touches, but the touch locations are only valid if they do not cross in the Y axis. That's a very nuanced property of the device, one that is only specific to certain touchpads from one manufacturer. I understand the usefulness of this functionality, but I also worry about proliferating the number of properties for devices (there are only 32 bits we can use, IIRC). I see four options off the top of my head: * Don't do anything, leave it as SEMI_MT. Obviously this would suck, but it is an option. * Make the trackpad advertise itself as *not* SEMI_MT. This would be broken, however, if the user performs a rotation where the touches cross in the Y axis. I feel this is plain wrong according to the stated protocol documentation and previous behavior, so I don't want to do this. * Add a new device property (INVALID_Y_AXIS_CROSSING?) that describes the exact behavior of this device. I would be ok with this if everyone else is, but only because proper clickpad behavior (which I consider very importand) is broken without this knowledge. * Leave the device as SEMI_MT, but provide the real locations, and allow userspace to determine the device vendor/model/etc. If userspace knows that a specific device behaves in a specific way, it can do its own quirking handling. Given the specificity of this behavior to only some devices of one brand, this would be my suggested resolution to the issue. -- 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
> I understand the usefulness of this functionality, but I also worry > about proliferating the number of properties for devices (there are > only 32 bits we can use, IIRC). I see four options off the top of my > head: > > * Don't do anything, leave it as SEMI_MT. Obviously this would suck, > but it is an option. > > * Make the trackpad advertise itself as *not* SEMI_MT. This would be > broken, however, if the user performs a rotation where the touches > cross in the Y axis. I feel this is plain wrong according to the > stated protocol documentation and previous behavior, so I don't want > to do this. > > * Add a new device property (INVALID_Y_AXIS_CROSSING?) that > describes the exact behavior of this device. I would be ok with this > if everyone else is, but only because proper clickpad behavior > (which I consider very importand) is broken without this knowledge. > > * Leave the device as SEMI_MT, but provide the real locations, and > allow userspace to determine the device vendor/model/etc. If > userspace knows that a specific device behaves in a specific way, it > can do its own quirking handling. Given the specificity of this > behavior to only some devices of one brand, this would be my > suggested resolution to the issue. A fifth option is to quirk the driver to remove the pulling effect from the reported bounding box, such that the simple userspace scheme for determining the position of the moving finger actually works. For instance, consider the simple algorithm "the slowest corner is the stationary finger". As long as this is true, the behavior will be smooth. If the sensor data clutters this behavior, it shows up in the driver as a mismatch between the point as computed above and the better estimate available in the driver. For frames where this happens, one can simply alter the bounding box slightly so that the algorithm works. It should be possible to formulate such a scheme in a couple of lines. Henrik -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 20, 2012 at 11:14:25AM +0800, Chung-Yih Wang (???) wrote: > From our experiments, the assumption of "the slowest corner is the > stationary finger" is not always true. That is the major reason we want to > report the firmware data instead of semi-mt. Oh, but that was precisely the point; the reason it does not hold true is due to sensor and discretization errors. If we can improve upon this situation, we get a better model of reality. > The problem here will be how > to remove the pulling effect, we measured the pulling effect before and can > not get a good result as there could be IIR in firmware as well. It seems > not an easy job to remove the pulling effect cleanly. Probably a simple filter will work. If the bounding box is moving too fast for the tracked point to stay in the right corner, the solution is to use a smaller time step. In practise, keeping the tracked point as a state in the driver, and updating the bounding box using box -> (1 - m) box + m box_new. If the tracked point is in the right corner, let m = 1. If not, choose a smaller m. > > > * Add a new device property (INVALID_Y_AXIS_CROSSING?) that > > > describes the exact behavior of this device. I would be ok with this > > > if everyone else is, but only because proper clickpad behavior > > > (which I consider very importand) is broken without this knowledge. > > > Sounds good to me(but I would rather to have INVALID_CROSSING instead, > depending on the relative finger positions, it could still have wrong > tracking either in X or Y axis crossing) Propagating information about various sensor defects to userspace sounds horrid to me. The sooner we can forget about these devices, the better. > > > * Leave the device as SEMI_MT, but provide the real locations, and > > > allow userspace to determine the device vendor/model/etc. If > > > userspace knows that a specific device behaves in a specific way, it > > > can do its own quirking handling. Given the specificity of this > > > behavior to only some devices of one brand, this would be my > > > suggested resolution to the issue. > > > > A bit confused here, do you mean we report the real locations instead of > bounding box the current driver have? I am not quite sure if this will > affect other existing works in userspace for this semi-mt driver. I am not entirely opposed to this solution, but I would much rather see an attempt to improve the bounding box in the driver, since such a solution could be useful for other devices as well. Thanks, Henrik -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 20, 2012 at 3:25 PM, Henrik Rydberg <rydberg@euromail.se> wrote: > On Fri, Jul 20, 2012 at 11:14:25AM +0800, Chung-Yih Wang (???) wrote: >> From our experiments, the assumption of "the slowest corner is the >> stationary finger" is not always true. That is the major reason we want to >> report the firmware data instead of semi-mt. > > Oh, but that was precisely the point; the reason it does not hold true > is due to sensor and discretization errors. If we can improve upon > this situation, we get a better model of reality. > >> The problem here will be how >> to remove the pulling effect, we measured the pulling effect before and can >> not get a good result as there could be IIR in firmware as well. It seems >> not an easy job to remove the pulling effect cleanly. > > Probably a simple filter will work. If the bounding box is moving too > fast for the tracked point to stay in the right corner, the solution > is to use a smaller time step. In practise, keeping the tracked point > as a state in the driver, and updating the bounding box using box -> > (1 - m) box + m box_new. If the tracked point is in the right corner, > let m = 1. If not, choose a smaller m. > >> > > * Add a new device property (INVALID_Y_AXIS_CROSSING?) that >> > > describes the exact behavior of this device. I would be ok with this >> > > if everyone else is, but only because proper clickpad behavior >> > > (which I consider very importand) is broken without this knowledge. >> > >> Sounds good to me(but I would rather to have INVALID_CROSSING instead, >> depending on the relative finger positions, it could still have wrong >> tracking either in X or Y axis crossing) > > Propagating information about various sensor defects to userspace > sounds horrid to me. The sooner we can forget about these devices, the > better. Not providing the userspace driver with enough information to give users the best experience possible sounds horrid to me. It turns out that using a bounding box with fixed "[(min_x, min_y), (max_x, max_y)]", and no per-finger pressure information, instead of the coordinates and pressures provided by the firmware, is throwing away useful data that could be used by the userspace driver. What we would like is a way to tell userspace what the firmware originally intended, but with a caveat that the firmware can't be 100% trusted. And, since this is for a relatively small class of hardware, to do it in a way that doesn't consume precious resources, like additional input properties. > >> > > * Leave the device as SEMI_MT, but provide the real locations, and >> > > allow userspace to determine the device vendor/model/etc. If >> > > userspace knows that a specific device behaves in a specific way, it >> > > can do its own quirking handling. Given the specificity of this >> > > behavior to only some devices of one brand, this would be my >> > > suggested resolution to the issue. >> > This is essentially what this patch does. It sets the SEMI_MT flag to indicate that the kernel data cannot be totally trusted, and then provides real MT-B (including per-finger pressures), instead of a fixed bounding box. It leaves it to userspace to treat the two slots worth of coordinates as a bounding box or as actual fingers using its own heuristics. By limiting to only one hardware type (using DMI), any breakage caused by this alternative use of the SEMI_MT flag is limited. Hopefully it is clear what we are trying to accomplish. I don't see how we can make a bounding box, even an improved bounding box, work. Perhaps Henrik has a really good idea, but I haven't been able to figure out what he is suggesting. Thank you for the nice discussion and alternative suggestions about better ways of doing this! -Daniel >> >> A bit confused here, do you mean we report the real locations instead of >> bounding box the current driver have? I am not quite sure if this will >> affect other existing works in userspace for this semi-mt driver. > > I am not entirely opposed to this solution, but I would much rather > see an attempt to improve the bounding box in the driver, since such a > solution could be useful for other devices as well. > > Thanks, > Henrik -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Propagating information about various sensor defects to userspace > > sounds horrid to me. The sooner we can forget about these devices, the > > better. > > Not providing the userspace driver with enough information to give > users the best experience possible sounds horrid to me. The question was whether we should add those to the long-lived input interface or not, sorry if that sounded like a rant. > It turns out > that using a bounding box with fixed "[(min_x, min_y), (max_x, > max_y)]", and no per-finger pressure information, instead of the > coordinates and pressures provided by the firmware, is throwing away > useful data that could be used by the userspace driver. So far, we have provided the baseline of reliable data from similar devices. > What we would like is a way to tell userspace what the firmware > originally intended, but with a caveat that the firmware can't be 100% > trusted. And, since this is for a relatively small class of hardware, > to do it in a way that doesn't consume precious resources, like > additional input properties. Input properties are not a precious resource, there is no limit on the bitmask values or anything like that, but there is no rush to add new ones. > >> > > * Leave the device as SEMI_MT, but provide the real locations, and > >> > > allow userspace to determine the device vendor/model/etc. If > >> > > userspace knows that a specific device behaves in a specific way, it > >> > > can do its own quirking handling. Given the specificity of this > >> > > behavior to only some devices of one brand, this would be my > >> > > suggested resolution to the issue. > >> > > > This is essentially what this patch does. I am interpreting Chase's suggestion as simply reporting the raw values instead of min/max. > It sets the SEMI_MT flag to > indicate that the kernel data cannot be totally trusted, and then > provides real MT-B (including per-finger pressures), instead of a > fixed bounding box. It leaves it to userspace to treat the two slots > worth of coordinates as a bounding box or as actual fingers using its > own heuristics. By limiting to only one hardware type (using DMI), > any breakage caused by this alternative use of the SEMI_MT flag is > limited. So it seems there is no need to add logic to the driver, only change one line from min/max to raw data for this particular hardware. That would solve your problem, yes? > Hopefully it is clear what we are trying to accomplish. I don't see > how we can make a bounding box, even an improved bounding box, work. > Perhaps Henrik has a really good idea, but I haven't been able to > figure out what he is suggesting. I understand you are not interested in looking into this, and your main objective is quite clear. Thanks, Henrik -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/20/2012 02:03 AM, Daniel Kurtz wrote: >>>>> * Leave the device as SEMI_MT, but provide the real locations, and >>>>> allow userspace to determine the device vendor/model/etc. If >>>>> userspace knows that a specific device behaves in a specific way, it >>>>> can do its own quirking handling. Given the specificity of this >>>>> behavior to only some devices of one brand, this would be my >>>>> suggested resolution to the issue. > > This is essentially what this patch does. It sets the SEMI_MT flag to > indicate that the kernel data cannot be totally trusted, and then > provides real MT-B (including per-finger pressures), instead of a > fixed bounding box. It leaves it to userspace to treat the two slots > worth of coordinates as a bounding box or as actual fingers using its > own heuristics. By limiting to only one hardware type (using DMI), > any breakage caused by this alternative use of the SEMI_MT flag is > limited. So I was worried that you were trying to remove the SEMI_MT flag, and I apologise for not looking closely enough to notice that wasn't the case. The documentation for the flag says: """ Some touchpads, most common between 2008 and 2011, can detect the presence of multiple contacts without resolving the individual positions; only the number of contacts and a rectangular shape is known. For such touchpads, the semi-mt property should be set. Depending on the device, the rectangle may enclose all touches, like a bounding box, or just some of them, for instance the two most recent touches. The diversity makes the rectangle of limited use, but some gestures can normally be extracted from it. """ Since the documentation doesn't say the data must be provided as min/max values, this patch actually appears to be perfectly fine as is. My next question is: how are you going to tell from userspace if the hardware actually provides correct data? IIRC, it was decided that we wouldn't provide sysfs nodes for the device IDs. -- 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 Sat, Jul 21, 2012 at 2:31 AM, Chase Douglas <chase.douglas@canonical.com> wrote: > > On 07/20/2012 02:03 AM, Daniel Kurtz wrote: >>>>>> >>>>>> * Leave the device as SEMI_MT, but provide the real locations, and >>>>>> allow userspace to determine the device vendor/model/etc. If >>>>>> userspace knows that a specific device behaves in a specific way, it >>>>>> can do its own quirking handling. Given the specificity of this >>>>>> behavior to only some devices of one brand, this would be my >>>>>> suggested resolution to the issue. >> >> >> This is essentially what this patch does. It sets the SEMI_MT flag to >> indicate that the kernel data cannot be totally trusted, and then >> provides real MT-B (including per-finger pressures), instead of a >> fixed bounding box. It leaves it to userspace to treat the two slots >> worth of coordinates as a bounding box or as actual fingers using its >> own heuristics. By limiting to only one hardware type (using DMI), >> any breakage caused by this alternative use of the SEMI_MT flag is >> limited. > > > So I was worried that you were trying to remove the SEMI_MT flag, and I apologise for not looking closely enough to notice that wasn't the case. The documentation for the flag says: > > """ > Some touchpads, most common between 2008 and 2011, can detect the presence of multiple contacts without resolving the individual positions; only the number of contacts and a rectangular shape is known. For such touchpads, the semi-mt property should be set. > > Depending on the device, the rectangle may enclose all touches, like a bounding box, or just some of them, for instance the two most recent touches. The diversity makes the rectangle of limited use, but some gestures can normally be extracted from it. > """ > > Since the documentation doesn't say the data must be provided as min/max values, this patch actually appears to be perfectly fine as is. > > My next question is: how are you going to tell from userspace if the hardware actually provides correct data? IIRC, it was decided that we wouldn't provide sysfs nodes for the device IDs. > Excellent question. We haven't solved this in any elegant way. When building images for this particular hardware platform, we set a flag in our user-space touchpad driver. It then knows to process this device's data as "non-bounding box semi-mt". -Daniel > -- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index d5b390f..0215c7c 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -45,6 +45,7 @@ #define YMIN_NOMINAL 1408 #define YMAX_NOMINAL 4448 +static bool cr48_profile_sensor; /***************************************************************************** * Stuff we need even when we do not want native Synaptics support @@ -1003,6 +1004,75 @@ static void synaptics_image_sensor_process(struct psmouse *psmouse, priv->agm_pending = false; } +static int synaptics_distsq(const struct input_mt_slot *slot, + const struct synaptics_hw_state *hw) +{ + int slot_x = input_mt_get_value(slot, ABS_MT_POSITION_X); + int slot_y = input_mt_get_value(slot, ABS_MT_POSITION_Y); + int dx = hw->x - slot_x; + int dy = synaptics_invert_y(hw->y) - slot_y; + return dx * dx + dy * dy; +} + +static bool synaptics_is_sgm_slot(const struct input_mt_slot *slot, + const struct synaptics_hw_state *sgm, + const struct synaptics_hw_state *agm) +{ + return (synaptics_distsq(slot, sgm) < synaptics_distsq(slot, agm)); +} + +static int synaptics_get_sgm_slot(const struct input_mt_slot *slots, + const struct synaptics_hw_state *sgm) +{ + int distsq_slot0 = synaptics_distsq(&slots[0], sgm); + int distsq_slot1 = synaptics_distsq(&slots[1], sgm); + return (distsq_slot0 < distsq_slot1 ? 0 : 1); +} + +static void synaptics_profile_sensor_process(struct psmouse *psmouse, + struct synaptics_hw_state *sgm, + int num_fingers) +{ + struct input_dev *dev = psmouse->dev; + struct synaptics_data *priv = psmouse->private; + struct synaptics_hw_state *agm = &priv->agm; + struct synaptics_mt_state mt_state; + + /* Initialize using current mt_state (as updated by last agm) */ + mt_state = agm->mt_state; + + if (num_fingers >= 2) { + /* Get previous sgm slot if exists */ + int sgm_slot = (mt_state.count != 0) ? mt_state.sgm : 0; + if (mt_state.count == 1) { + const struct input_mt_slot *mt = &dev->mt[sgm_slot]; + if (!synaptics_is_sgm_slot(mt, sgm, agm)) + sgm_slot = 1 - sgm_slot; + } + synaptics_report_slot(dev, sgm_slot, sgm); + synaptics_report_slot(dev, 1 - sgm_slot, agm); + synaptics_mt_state_set(&mt_state, num_fingers, + sgm_slot, 1 - sgm_slot); + } else if (num_fingers == 1) { + int sgm_slot = (mt_state.count != 0) ? mt_state.sgm : 0; + if (mt_state.count >= 2) + sgm_slot = synaptics_get_sgm_slot(dev->mt, sgm); + synaptics_report_slot(dev, sgm_slot, sgm); + synaptics_report_slot(dev, 1 - sgm_slot, NULL); + synaptics_mt_state_set(&mt_state, 1, sgm_slot, -1); + } else { + synaptics_report_slot(dev, 0, NULL); + synaptics_report_slot(dev, 1, NULL); + synaptics_mt_state_set(&mt_state, 0, -1, -1); + } + /* Store updated mt_state */ + priv->mt_state = agm->mt_state = mt_state; + /* Send the number of fingers reported by touchpad itself. */ + input_mt_report_finger_count(dev, mt_state.count); + synaptics_report_buttons(psmouse, sgm); + input_sync(dev); +} + /* * called for each full received packet from the touchpad */ @@ -1066,6 +1136,12 @@ static void synaptics_process_packet(struct psmouse *psmouse) finger_width = 0; } + if (cr48_profile_sensor) { + synaptics_profile_sensor_process(psmouse, &hw, num_fingers); + return; + } + + if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) synaptics_report_semi_mt_data(dev, &hw, &priv->agm, num_fingers); @@ -1227,6 +1303,9 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv) ABS_MT_POSITION_Y); } + if (cr48_profile_sensor) + input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0); + if (SYN_CAP_PALMDETECT(priv->capabilities)) input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0); @@ -1422,10 +1501,24 @@ static const struct dmi_system_id __initconst olpc_dmi_table[] = { { } }; +static const struct dmi_system_id __initconst cr48_dmi_table[] = { +#if defined(CONFIG_DMI) && defined(CONFIG_X86) + { + /* Cr-48 Chromebook (Codename Mario) */ + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "IEC"), + DMI_MATCH(DMI_PRODUCT_NAME, "Mario"), + }, + }, +#endif + { } +}; + void __init synaptics_module_init(void) { impaired_toshiba_kbc = dmi_check_system(toshiba_dmi_table); broken_olpc_ec = dmi_check_system(olpc_dmi_table); + cr48_profile_sensor = dmi_check_system(cr48_dmi_table); } static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
The profile sensor clickpad in a Cr-48 Chromebook does a reasonable job of tracking individual fingers. This tracking isn't perfect, but, experiments show that it works better than just passing "semi-mt" data to userspace, and making userspace try to deduce where the fingers are given a bounding box. This patch tries to report correct two-finger positions instead of the {(min_x, min_y), (max_x, max_y)} for profile sensor clickpads on Cr-48 chromebooks. Note that this device's firmware always reports the higher (smaller y) finger in the "sgm" packet, and the lower (larger y) finger in the "agm" packet. Thus, when a new finger arrives on the pad, the kernel driver uses a simple Euclidean distance measure to deduce which of the two new fingers should keep the tracking ID of the previous single finger. Similarly, when one finger is removed, the same measure is used to determine which finger remained on the pad. Signed-off-by: Chung-yih Wang <cywang@chromium.org> --- drivers/input/mouse/synaptics.c | 93 +++++++++++++++++++++++++++++++++++++++ 1 files changed, 93 insertions(+), 0 deletions(-)