| Message ID | 1313632629-23603-3-git-send-email-jj_ding@emc.com.tw (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 18, 2011 at 9:57 AM, JJ Ding <jj_ding@emc.com.tw> wrote: > With newer hardware, the touchpad provides range info. > Let's use it. > > Signed-off-by: JJ Ding <jj_ding@emc.com.tw> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org> -- 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 Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: > + > + i = (etd->fw_version > 0x020800 && > + etd->fw_version < 0x020900) ? 1 : 2; > + *x_max = (etd->capabilities[1] - i) * 64; > + *y_max = (etd->capabilities[2] - i) * 64; > + *y_2ft_max = (*y_max - i) * 64 / 4; Hmm, we should have the same range for ST and MT data and scale MT data if it has lower resolution to match ST. Thanks.
Op 19-08-11 11:47, JJ Ding schreef: > Hi Dmitry, > > Sorry for late reply. I missed this one somehow. > > On Thu, 18 Aug 2011 00:47:56 -0700, Dmitry Torokhov<dmitry.torokhov@gmail.com> wrote: >> On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: >>> + >>> + i = (etd->fw_version> 0x020800&& >>> + etd->fw_version< 0x020900) ? 1 : 2; >>> + *x_max = (etd->capabilities[1] - i) * 64; >>> + *y_max = (etd->capabilities[2] - i) * 64; >>> + *y_2ft_max = (*y_max - i) * 64 / 4; >> >> Hmm, we should have the same range for ST and MT data and scale MT data >> if it has lower resolution to match ST. > So I should just remove y_2ft_max and those ETP_2FT_XXXX in elantech.h, > and do the scale in elantech_report_absolute_v2? Humm, yes, I think what Dmitry wants is that both ABS_MT_POSITION_Y and ABS_Y have the same min and max, and the scaling is done when reading the data. However, it seems this already what is being tried to be done, excepted that I mess it up in the latest patch set I sent. I just noticed it now, sorry :-S You can see in elantech_report_absolute_v2() in case of 2 fingers: The part for updating ABS_X, ABS_Y is correct: input_report_abs(dev, ABS_X, x1 << 2); input_report_abs(dev, ABS_Y, y1 << 2); But I forgot to do the same for MT: elantech_report_semi_mt_data(dev, fingers, x1, y1, x2, y2); That should be: elantech_report_semi_mt_data(dev, fingers, x1 << 2, y1 << 2, x2 << 2, y2 << 2); Or, even more clean, just move the shift directly into the computation, like: y1 = etd->y_max - ((((packet[0] & 0x20) << 3) | packet[2]) << 2); In such case you can drop completely y_2ft_max, and move the input_report_abs() outside of the switch. In addition, I have a couple of more remarks on this patch: > + *x_max = (etd->capabilities[1] - i) * 64; > + *y_max = (etd->capabilities[2] - i) * 64; > + *y_2ft_max = (*y_max - i) * 64 / 4; This last line is probably wrong as I think it should be: *y_2ft_max = *y_max / 4; But if you drop y_2ft_max, that shouldn't matter anymore! > + case 1: > + *x_min = ETP_XMIN_V1; > + *y_min = ETP_YMIN_V1; > + *x_max = ETP_XMAX_V1; > + *y_max = ETP_YMAX_V1; > + break; > + > + case 2: > + if (etd->fw_version == 0x020800 || > + etd->fw_version == 0x020b00 || > + etd->fw_version == 0x020030) { > + *x_min = ETP_XMIN_V2; > + *y_min = ETP_YMIN_V2; > + *x_max = ETP_XMAX_V2; > + *y_max = ETP_YMAX_V2; > + *y_2ft_max = ETP_2FT_YMAX; > + break; > + } Actually these variables are defined as: #define ETP_YMAX_V2 ( 768 - ETP_EDGE_FUZZ_V2) I'd suggest to remove trying being too clever and remove the ETP_EDGE_FUZZ_V2. They should be just the raw resolution of the device. Otherwise, they can cause underflow on the Y axis. Finally, a minor style suggestion, in "case 2:" above, only use one single "break;" and put the two part in a complete "if-else" statement, with xmin, y_min all explicit. E.g.: case 2: if (etd->fw_version == 0x020800 || etd->fw_version == 0x020b00 || etd->fw_version == 0x020030) { *x_min = ETP_XMIN_V2; *y_min = ETP_YMIN_V2; *x_max = ETP_XMAX_V2; *y_max = ETP_YMAX_V2; } else { i = (etd->fw_version > 0x020800 && etd->fw_version < 0x020900) ? 1 : 2; *x_min = 0; *y_min = 0; *x_max = (etd->capabilities[1] - i) * 64; *y_max = (etd->capabilities[2] - i) * 64; } break; > If so, I will create another patch for this change. > If you could send a new version of this patch with these changes, it'd be great :-) Cheers, Éric -- 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 08/18/2011 12:47 AM, Dmitry Torokhov wrote: > On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: >> + >> + i = (etd->fw_version > 0x020800 && >> + etd->fw_version < 0x020900) ? 1 : 2; >> + *x_max = (etd->capabilities[1] - i) * 64; >> + *y_max = (etd->capabilities[2] - i) * 64; >> + *y_2ft_max = (*y_max - i) * 64 / 4; > > Hmm, we should have the same range for ST and MT data and scale MT data > if it has lower resolution to match ST. I saw this go by a while back and it made sense to me at the time. However, I've had some thoughts that give me pause. Seth Forshee has been working on getting a semi-mt driver for ALPS devices. The ALPS devices have an interesting mechanism for providing multitouch data, but it boils down to having a resolution of only 15 values in the X axis and 11 in the Y axis (it looks possible to extrapolate and get double the resolution, but my point will remain). Let's take the X synaptics module as an example of the repercussions of in-kernel axis scaling. The X synaptics module translates two touch drags into scroll events. Synaptics will want to use the highest resolution axis for generating scroll events. If both the MT and ST axes have the same resolution, it might pick the MT axes for scrolling. On ALPS devices with in-kernel axis scaling that would be a bad choice. It's trivial to project the MT and ST axes onto each other in userspace. I suggest we report the real range and resolution of ST and MT axes independently. -- 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 Chase, On Thu, 01 Sep 2011 11:26:32 -0700, Chase Douglas <chase.douglas@canonical.com> wrote: > On 08/18/2011 12:47 AM, Dmitry Torokhov wrote: > > On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: > >> + > >> + i = (etd->fw_version > 0x020800 && > >> + etd->fw_version < 0x020900) ? 1 : 2; > >> + *x_max = (etd->capabilities[1] - i) * 64; > >> + *y_max = (etd->capabilities[2] - i) * 64; > >> + *y_2ft_max = (*y_max - i) * 64 / 4; > > > > Hmm, we should have the same range for ST and MT data and scale MT data > > if it has lower resolution to match ST. > > I saw this go by a while back and it made sense to me at the time. > However, I've had some thoughts that give me pause. > > Seth Forshee has been working on getting a semi-mt driver for ALPS > devices. The ALPS devices have an interesting mechanism for providing > multitouch data, but it boils down to having a resolution of only 15 > values in the X axis and 11 in the Y axis (it looks possible to > extrapolate and get double the resolution, but my point will remain). > > Let's take the X synaptics module as an example of the repercussions of > in-kernel axis scaling. The X synaptics module translates two touch > drags into scroll events. Synaptics will want to use the highest > resolution axis for generating scroll events. If both the MT and ST axes > have the same resolution, it might pick the MT axes for scrolling. On > ALPS devices with in-kernel axis scaling that would be a bad choice. I don't know about the ALPS devices, but since we already report ABS_MT_POSITION_{X,Y} with elantech v2, we have to do the scaling in kernel anyway to adhere to multitouch protocol. So I would say it is still more appropriate to have the same resolution for ST and MT with respect to elantech v2. Maybe ALPS should be considered an exception to this? > It's trivial to project the MT and ST axes onto each other in userspace. > I suggest we report the real range and resolution of ST and MT axes > independently. > -- 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 09/04/2011 08:22 PM, JJ Ding wrote: > Hi Chase, > > On Thu, 01 Sep 2011 11:26:32 -0700, Chase Douglas <chase.douglas@canonical.com> wrote: >> On 08/18/2011 12:47 AM, Dmitry Torokhov wrote: >>> On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: >>>> + >>>> + i = (etd->fw_version > 0x020800 && >>>> + etd->fw_version < 0x020900) ? 1 : 2; >>>> + *x_max = (etd->capabilities[1] - i) * 64; >>>> + *y_max = (etd->capabilities[2] - i) * 64; >>>> + *y_2ft_max = (*y_max - i) * 64 / 4; >>> >>> Hmm, we should have the same range for ST and MT data and scale MT data >>> if it has lower resolution to match ST. >> >> I saw this go by a while back and it made sense to me at the time. >> However, I've had some thoughts that give me pause. >> >> Seth Forshee has been working on getting a semi-mt driver for ALPS >> devices. The ALPS devices have an interesting mechanism for providing >> multitouch data, but it boils down to having a resolution of only 15 >> values in the X axis and 11 in the Y axis (it looks possible to >> extrapolate and get double the resolution, but my point will remain). >> >> Let's take the X synaptics module as an example of the repercussions of >> in-kernel axis scaling. The X synaptics module translates two touch >> drags into scroll events. Synaptics will want to use the highest >> resolution axis for generating scroll events. If both the MT and ST axes >> have the same resolution, it might pick the MT axes for scrolling. On >> ALPS devices with in-kernel axis scaling that would be a bad choice. > I don't know about the ALPS devices, but since we already report > ABS_MT_POSITION_{X,Y} with elantech v2, we have to do the scaling in > kernel anyway to adhere to multitouch protocol. So I would say it is > still more appropriate to have the same resolution for ST and MT with > respect to elantech v2. Maybe ALPS should be considered an exception to this? The multitouch protocol doesn't require scaling of axes to match, at least not according to the protocol documentation. I see that the current code scales the coordinates for v2, but it's only half-resolution. That's not a huge deal since the resolution of modern touchpads is very high. We could leave it scaled to not break abi, if that was a concern. However, with new devices it makes sense to state the ranges in terms of what the device actually supports. Otherwise, we're just masking out useful data that userspace could be using. -- 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, Sep 06, 2011 at 10:03:05AM -0700, Chase Douglas wrote: > On 09/04/2011 08:22 PM, JJ Ding wrote: > > Hi Chase, > > > > On Thu, 01 Sep 2011 11:26:32 -0700, Chase Douglas <chase.douglas@canonical.com> wrote: > >> On 08/18/2011 12:47 AM, Dmitry Torokhov wrote: > >>> On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: > >>>> + > >>>> + i = (etd->fw_version > 0x020800 && > >>>> + etd->fw_version < 0x020900) ? 1 : 2; > >>>> + *x_max = (etd->capabilities[1] - i) * 64; > >>>> + *y_max = (etd->capabilities[2] - i) * 64; > >>>> + *y_2ft_max = (*y_max - i) * 64 / 4; > >>> > >>> Hmm, we should have the same range for ST and MT data and scale MT data > >>> if it has lower resolution to match ST. > >> > >> I saw this go by a while back and it made sense to me at the time. > >> However, I've had some thoughts that give me pause. > >> > >> Seth Forshee has been working on getting a semi-mt driver for ALPS > >> devices. The ALPS devices have an interesting mechanism for providing > >> multitouch data, but it boils down to having a resolution of only 15 > >> values in the X axis and 11 in the Y axis (it looks possible to > >> extrapolate and get double the resolution, but my point will remain). > >> > >> Let's take the X synaptics module as an example of the repercussions of > >> in-kernel axis scaling. The X synaptics module translates two touch > >> drags into scroll events. Synaptics will want to use the highest > >> resolution axis for generating scroll events. If both the MT and ST axes > >> have the same resolution, it might pick the MT axes for scrolling. On > >> ALPS devices with in-kernel axis scaling that would be a bad choice. > > I don't know about the ALPS devices, but since we already report > > ABS_MT_POSITION_{X,Y} with elantech v2, we have to do the scaling in > > kernel anyway to adhere to multitouch protocol. So I would say it is > > still more appropriate to have the same resolution for ST and MT with > > respect to elantech v2. Maybe ALPS should be considered an exception to this? > > The multitouch protocol doesn't require scaling of axes to match, at > least not according to the protocol documentation. > > I see that the current code scales the coordinates for v2, but it's only > half-resolution. That's not a huge deal since the resolution of modern > touchpads is very high. We could leave it scaled to not break abi, if > that was a concern. However, with new devices it makes sense to state > the ranges in terms of what the device actually supports. Otherwise, > we're just masking out useful data that userspace could be using. > I disagree. I believe that ST and MT ranges reported for the same working surface should match, especially since many devices derive ST data from MT. As far as devices that have ranges 0-15 in MT mode - I am not sure how useful such MT steam anyway and if we are better of just ignore them (maybe just use the data to report number of fingers on the surface but otherwise use standard ST protocol). Thanks.
On 09/06/2011 10:36 AM, Dmitry Torokhov wrote: > On Tue, Sep 06, 2011 at 10:03:05AM -0700, Chase Douglas wrote: >> On 09/04/2011 08:22 PM, JJ Ding wrote: >>> Hi Chase, >>> >>> On Thu, 01 Sep 2011 11:26:32 -0700, Chase Douglas <chase.douglas@canonical.com> wrote: >>>> On 08/18/2011 12:47 AM, Dmitry Torokhov wrote: >>>>> On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: >>>>>> + >>>>>> + i = (etd->fw_version > 0x020800 && >>>>>> + etd->fw_version < 0x020900) ? 1 : 2; >>>>>> + *x_max = (etd->capabilities[1] - i) * 64; >>>>>> + *y_max = (etd->capabilities[2] - i) * 64; >>>>>> + *y_2ft_max = (*y_max - i) * 64 / 4; >>>>> >>>>> Hmm, we should have the same range for ST and MT data and scale MT data >>>>> if it has lower resolution to match ST. >>>> >>>> I saw this go by a while back and it made sense to me at the time. >>>> However, I've had some thoughts that give me pause. >>>> >>>> Seth Forshee has been working on getting a semi-mt driver for ALPS >>>> devices. The ALPS devices have an interesting mechanism for providing >>>> multitouch data, but it boils down to having a resolution of only 15 >>>> values in the X axis and 11 in the Y axis (it looks possible to >>>> extrapolate and get double the resolution, but my point will remain). >>>> >>>> Let's take the X synaptics module as an example of the repercussions of >>>> in-kernel axis scaling. The X synaptics module translates two touch >>>> drags into scroll events. Synaptics will want to use the highest >>>> resolution axis for generating scroll events. If both the MT and ST axes >>>> have the same resolution, it might pick the MT axes for scrolling. On >>>> ALPS devices with in-kernel axis scaling that would be a bad choice. >>> I don't know about the ALPS devices, but since we already report >>> ABS_MT_POSITION_{X,Y} with elantech v2, we have to do the scaling in >>> kernel anyway to adhere to multitouch protocol. So I would say it is >>> still more appropriate to have the same resolution for ST and MT with >>> respect to elantech v2. Maybe ALPS should be considered an exception to this? >> >> The multitouch protocol doesn't require scaling of axes to match, at >> least not according to the protocol documentation. >> >> I see that the current code scales the coordinates for v2, but it's only >> half-resolution. That's not a huge deal since the resolution of modern >> touchpads is very high. We could leave it scaled to not break abi, if >> that was a concern. However, with new devices it makes sense to state >> the ranges in terms of what the device actually supports. Otherwise, >> we're just masking out useful data that userspace could be using. >> > > I disagree. I believe that ST and MT ranges reported for the same > working surface should match, especially since many devices derive ST > data from MT. > > As far as devices that have ranges 0-15 in MT mode - I am not sure how > useful such MT steam anyway and if we are better of just ignore them > (maybe just use the data to report number of fingers on the surface but > otherwise use standard ST protocol). The MT data could still be useful for pinch to zoom or potentially rotate (though most low res devices probably are only semi-mt). I don't want to forsake pinch to zoom just because we can't pass on the resolution of MT data properly. -- 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, Sep 06, 2011 at 11:05:11AM -0700, Chase Douglas wrote: > On 09/06/2011 10:36 AM, Dmitry Torokhov wrote: > > On Tue, Sep 06, 2011 at 10:03:05AM -0700, Chase Douglas wrote: > >> On 09/04/2011 08:22 PM, JJ Ding wrote: > >>> Hi Chase, > >>> > >>> On Thu, 01 Sep 2011 11:26:32 -0700, Chase Douglas <chase.douglas@canonical.com> wrote: > >>>> On 08/18/2011 12:47 AM, Dmitry Torokhov wrote: > >>>>> On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: > >>>>>> + > >>>>>> + i = (etd->fw_version > 0x020800 && > >>>>>> + etd->fw_version < 0x020900) ? 1 : 2; > >>>>>> + *x_max = (etd->capabilities[1] - i) * 64; > >>>>>> + *y_max = (etd->capabilities[2] - i) * 64; > >>>>>> + *y_2ft_max = (*y_max - i) * 64 / 4; > >>>>> > >>>>> Hmm, we should have the same range for ST and MT data and scale MT data > >>>>> if it has lower resolution to match ST. > >>>> > >>>> I saw this go by a while back and it made sense to me at the time. > >>>> However, I've had some thoughts that give me pause. > >>>> > >>>> Seth Forshee has been working on getting a semi-mt driver for ALPS > >>>> devices. The ALPS devices have an interesting mechanism for providing > >>>> multitouch data, but it boils down to having a resolution of only 15 > >>>> values in the X axis and 11 in the Y axis (it looks possible to > >>>> extrapolate and get double the resolution, but my point will remain). > >>>> > >>>> Let's take the X synaptics module as an example of the repercussions of > >>>> in-kernel axis scaling. The X synaptics module translates two touch > >>>> drags into scroll events. Synaptics will want to use the highest > >>>> resolution axis for generating scroll events. If both the MT and ST axes > >>>> have the same resolution, it might pick the MT axes for scrolling. On > >>>> ALPS devices with in-kernel axis scaling that would be a bad choice. > >>> I don't know about the ALPS devices, but since we already report > >>> ABS_MT_POSITION_{X,Y} with elantech v2, we have to do the scaling in > >>> kernel anyway to adhere to multitouch protocol. So I would say it is > >>> still more appropriate to have the same resolution for ST and MT with > >>> respect to elantech v2. Maybe ALPS should be considered an exception to this? > >> > >> The multitouch protocol doesn't require scaling of axes to match, at > >> least not according to the protocol documentation. > >> > >> I see that the current code scales the coordinates for v2, but it's only > >> half-resolution. That's not a huge deal since the resolution of modern > >> touchpads is very high. We could leave it scaled to not break abi, if > >> that was a concern. However, with new devices it makes sense to state > >> the ranges in terms of what the device actually supports. Otherwise, > >> we're just masking out useful data that userspace could be using. > >> > > > > I disagree. I believe that ST and MT ranges reported for the same > > working surface should match, especially since many devices derive ST > > data from MT. > > > > As far as devices that have ranges 0-15 in MT mode - I am not sure how > > useful such MT steam anyway and if we are better of just ignore them > > (maybe just use the data to report number of fingers on the surface but > > otherwise use standard ST protocol). > > The MT data could still be useful for pinch to zoom or potentially > rotate (though most low res devices probably are only semi-mt). I don't > want to forsake pinch to zoom just because we can't pass on the > resolution of MT data properly. How would userspace know that MT data should only be used for gestures but nothign else? By examining range? What is the "too small range" then? It would be different for different devices. I do not want userspace portions of the drivers to turn into unmanageble collection of quirks. Some hardware is just hopeless... And pinch to zoom is cute but hardly most used function on a laptop (as opposed to phone/tablet), I'd just leave it be. Thanks.
> >> I see that the current code scales the coordinates for v2, but it's only > >> half-resolution. That's not a huge deal since the resolution of modern > >> touchpads is very high. We could leave it scaled to not break abi, if > >> that was a concern. However, with new devices it makes sense to state > >> the ranges in terms of what the device actually supports. Otherwise, > >> we're just masking out useful data that userspace could be using. > >> > > > > I disagree. I believe that ST and MT ranges reported for the same > > working surface should match, especially since many devices derive ST > > data from MT. > > > > As far as devices that have ranges 0-15 in MT mode - I am not sure how > > useful such MT steam anyway and if we are better of just ignore them > > (maybe just use the data to report number of fingers on the surface but > > otherwise use standard ST protocol). > > The MT data could still be useful for pinch to zoom or potentially > rotate (though most low res devices probably are only semi-mt). I don't > want to forsake pinch to zoom just because we can't pass on the > resolution of MT data properly. We are not forsaking anything. At best, the detail missed out by using the same scaling for ST and MT is the ability to distinguish the axis with the best or poorest resolution. However, some devices report higher MT than ST resolution, not because it is better (it is indeed worse), but simply to not lose bits in the filtering process. In other words: the logic is not without corner cases, and it only makes a difference to the transitional devices that are not fully MT capable. If this really was a concern, userspace could use the ST coordinates for some transformations instead. 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 09/06/2011 11:47 AM, Henrik Rydberg wrote: >>>> I see that the current code scales the coordinates for v2, but it's only >>>> half-resolution. That's not a huge deal since the resolution of modern >>>> touchpads is very high. We could leave it scaled to not break abi, if >>>> that was a concern. However, with new devices it makes sense to state >>>> the ranges in terms of what the device actually supports. Otherwise, >>>> we're just masking out useful data that userspace could be using. >>>> >>> >>> I disagree. I believe that ST and MT ranges reported for the same >>> working surface should match, especially since many devices derive ST >>> data from MT. >>> >>> As far as devices that have ranges 0-15 in MT mode - I am not sure how >>> useful such MT steam anyway and if we are better of just ignore them >>> (maybe just use the data to report number of fingers on the surface but >>> otherwise use standard ST protocol). >> >> The MT data could still be useful for pinch to zoom or potentially >> rotate (though most low res devices probably are only semi-mt). I don't >> want to forsake pinch to zoom just because we can't pass on the >> resolution of MT data properly. > > We are not forsaking anything. At best, the detail missed out by using > the same scaling for ST and MT is the ability to distinguish the axis > with the best or poorest resolution. However, some devices report > higher MT than ST resolution, not because it is better (it is indeed > worse), but simply to not lose bits in the filtering process. In other > words: the logic is not without corner cases, and it only makes a > difference to the transitional devices that are not fully MT > capable. If this really was a concern, userspace could use the ST > coordinates for some transformations instead. So what do we want userspace to do? In the absence of supplying this information we need to provide guidelines. We could say: * Always use ST coords + finger count for drag gestures (i.e. scrolling) * Use MT coords for pinch and rotate If this is acceptable, we should add it to the documentation. Thoughts? -- 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 09/06/2011 11:20 AM, Dmitry Torokhov wrote: > On Tue, Sep 06, 2011 at 11:05:11AM -0700, Chase Douglas wrote: >> On 09/06/2011 10:36 AM, Dmitry Torokhov wrote: >>> On Tue, Sep 06, 2011 at 10:03:05AM -0700, Chase Douglas wrote: >>>> On 09/04/2011 08:22 PM, JJ Ding wrote: >>>>> Hi Chase, >>>>> >>>>> On Thu, 01 Sep 2011 11:26:32 -0700, Chase Douglas <chase.douglas@canonical.com> wrote: >>>>>> On 08/18/2011 12:47 AM, Dmitry Torokhov wrote: >>>>>>> On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: >>>>>>>> + >>>>>>>> + i = (etd->fw_version > 0x020800 && >>>>>>>> + etd->fw_version < 0x020900) ? 1 : 2; >>>>>>>> + *x_max = (etd->capabilities[1] - i) * 64; >>>>>>>> + *y_max = (etd->capabilities[2] - i) * 64; >>>>>>>> + *y_2ft_max = (*y_max - i) * 64 / 4; >>>>>>> >>>>>>> Hmm, we should have the same range for ST and MT data and scale MT data >>>>>>> if it has lower resolution to match ST. >>>>>> >>>>>> I saw this go by a while back and it made sense to me at the time. >>>>>> However, I've had some thoughts that give me pause. >>>>>> >>>>>> Seth Forshee has been working on getting a semi-mt driver for ALPS >>>>>> devices. The ALPS devices have an interesting mechanism for providing >>>>>> multitouch data, but it boils down to having a resolution of only 15 >>>>>> values in the X axis and 11 in the Y axis (it looks possible to >>>>>> extrapolate and get double the resolution, but my point will remain). >>>>>> >>>>>> Let's take the X synaptics module as an example of the repercussions of >>>>>> in-kernel axis scaling. The X synaptics module translates two touch >>>>>> drags into scroll events. Synaptics will want to use the highest >>>>>> resolution axis for generating scroll events. If both the MT and ST axes >>>>>> have the same resolution, it might pick the MT axes for scrolling. On >>>>>> ALPS devices with in-kernel axis scaling that would be a bad choice. >>>>> I don't know about the ALPS devices, but since we already report >>>>> ABS_MT_POSITION_{X,Y} with elantech v2, we have to do the scaling in >>>>> kernel anyway to adhere to multitouch protocol. So I would say it is >>>>> still more appropriate to have the same resolution for ST and MT with >>>>> respect to elantech v2. Maybe ALPS should be considered an exception to this? >>>> >>>> The multitouch protocol doesn't require scaling of axes to match, at >>>> least not according to the protocol documentation. >>>> >>>> I see that the current code scales the coordinates for v2, but it's only >>>> half-resolution. That's not a huge deal since the resolution of modern >>>> touchpads is very high. We could leave it scaled to not break abi, if >>>> that was a concern. However, with new devices it makes sense to state >>>> the ranges in terms of what the device actually supports. Otherwise, >>>> we're just masking out useful data that userspace could be using. >>>> >>> >>> I disagree. I believe that ST and MT ranges reported for the same >>> working surface should match, especially since many devices derive ST >>> data from MT. >>> >>> As far as devices that have ranges 0-15 in MT mode - I am not sure how >>> useful such MT steam anyway and if we are better of just ignore them >>> (maybe just use the data to report number of fingers on the surface but >>> otherwise use standard ST protocol). >> >> The MT data could still be useful for pinch to zoom or potentially >> rotate (though most low res devices probably are only semi-mt). I don't >> want to forsake pinch to zoom just because we can't pass on the >> resolution of MT data properly. > > How would userspace know that MT data should only be used for gestures > but nothign else? By examining range? What is the "too small range" > then? It would be different for different devices. I do not want > userspace portions of the drivers to turn into unmanageble collection of > quirks. I wasn't suggesting that it have a big switch that enables or disables gestures. I just want userspace to be able to figure out whether ST or MT data would be better for a given task. If the range of the ST and MT axes could provide this data, then it makes sense to do so. The test wouldn't be like "is MT range big enough", it would be "is MT range better than ST or vice versa". However, Henrik pointed out that some devices report ranges that aren't representative of how accurate or precise they really are. That invalidates this approach. > Some hardware is just hopeless... And pinch to zoom is cute but hardly > most used function on a laptop (as opposed to phone/tablet), I'd just > leave it be. Just because a piece of hardware is imprecise does not mean it is useless for gestures. Sure, it may not work that great for precision zooming, but it would be good enough for threshold matching. The Unity window manager uses these thresholds to fire actions like spread (or "expose" in OS X terms). -- 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, On Wed, Sep 7, 2011 at 3:29 AM, Chase Douglas <chase.douglas@canonical.com> wrote: > > On 09/06/2011 11:20 AM, Dmitry Torokhov wrote: > > On Tue, Sep 06, 2011 at 11:05:11AM -0700, Chase Douglas wrote: > >> On 09/06/2011 10:36 AM, Dmitry Torokhov wrote: > >>> On Tue, Sep 06, 2011 at 10:03:05AM -0700, Chase Douglas wrote: > >>>> On 09/04/2011 08:22 PM, JJ Ding wrote: > >>>>> Hi Chase, > >>>>> > >>>>> On Thu, 01 Sep 2011 11:26:32 -0700, Chase Douglas <chase.douglas@canonical.com> wrote: > >>>>>> On 08/18/2011 12:47 AM, Dmitry Torokhov wrote: > >>>>>>> On Thu, Aug 18, 2011 at 09:57:05AM +0800, JJ Ding wrote: > >>>>>>>> + > >>>>>>>> + i = (etd->fw_version > 0x020800 && > >>>>>>>> + etd->fw_version < 0x020900) ? 1 : 2; > >>>>>>>> + *x_max = (etd->capabilities[1] - i) * 64; > >>>>>>>> + *y_max = (etd->capabilities[2] - i) * 64; > >>>>>>>> + *y_2ft_max = (*y_max - i) * 64 / 4; > >>>>>>> > >>>>>>> Hmm, we should have the same range for ST and MT data and scale MT data > >>>>>>> if it has lower resolution to match ST. > >>>>>> > >>>>>> I saw this go by a while back and it made sense to me at the time. > >>>>>> However, I've had some thoughts that give me pause. > >>>>>> > >>>>>> Seth Forshee has been working on getting a semi-mt driver for ALPS > >>>>>> devices. The ALPS devices have an interesting mechanism for providing > >>>>>> multitouch data, but it boils down to having a resolution of only 15 > >>>>>> values in the X axis and 11 in the Y axis (it looks possible to > >>>>>> extrapolate and get double the resolution, but my point will remain). > >>>>>> > >>>>>> Let's take the X synaptics module as an example of the repercussions of > >>>>>> in-kernel axis scaling. The X synaptics module translates two touch > >>>>>> drags into scroll events. Synaptics will want to use the highest > >>>>>> resolution axis for generating scroll events. If both the MT and ST axes > >>>>>> have the same resolution, it might pick the MT axes for scrolling. On > >>>>>> ALPS devices with in-kernel axis scaling that would be a bad choice. > >>>>> I don't know about the ALPS devices, but since we already report > >>>>> ABS_MT_POSITION_{X,Y} with elantech v2, we have to do the scaling in > >>>>> kernel anyway to adhere to multitouch protocol. So I would say it is > >>>>> still more appropriate to have the same resolution for ST and MT with > >>>>> respect to elantech v2. Maybe ALPS should be considered an exception to this? > >>>> > >>>> The multitouch protocol doesn't require scaling of axes to match, at > >>>> least not according to the protocol documentation. > >>>> > >>>> I see that the current code scales the coordinates for v2, but it's only > >>>> half-resolution. That's not a huge deal since the resolution of modern > >>>> touchpads is very high. We could leave it scaled to not break abi, if > >>>> that was a concern. However, with new devices it makes sense to state > >>>> the ranges in terms of what the device actually supports. Otherwise, > >>>> we're just masking out useful data that userspace could be using. > >>>> > >>> > >>> I disagree. I believe that ST and MT ranges reported for the same > >>> working surface should match, especially since many devices derive ST > >>> data from MT. > >>> > >>> As far as devices that have ranges 0-15 in MT mode - I am not sure how > >>> useful such MT steam anyway and if we are better of just ignore them > >>> (maybe just use the data to report number of fingers on the surface but > >>> otherwise use standard ST protocol). > >> > >> The MT data could still be useful for pinch to zoom or potentially > >> rotate (though most low res devices probably are only semi-mt). I don't > >> want to forsake pinch to zoom just because we can't pass on the > >> resolution of MT data properly. > > > > How would userspace know that MT data should only be used for gestures > > but nothign else? By examining range? What is the "too small range" > > then? It would be different for different devices. I do not want > > userspace portions of the drivers to turn into unmanageble collection of > > quirks. > > I wasn't suggesting that it have a big switch that enables or disables > gestures. I just want userspace to be able to figure out whether ST or > MT data would be better for a given task. If the range of the ST and MT > axes could provide this data, then it makes sense to do so. The test > wouldn't be like "is MT range big enough", it would be "is MT range > better than ST or vice versa". > > However, Henrik pointed out that some devices report ranges that aren't > representative of how accurate or precise they really are. That > invalidates this approach. > > > Some hardware is just hopeless... And pinch to zoom is cute but hardly > > most used function on a laptop (as opposed to phone/tablet), I'd just > > leave it be. > > Just because a piece of hardware is imprecise does not mean it is > useless for gestures. Sure, it may not work that great for precision > zooming, but it would be good enough for threshold matching. The Unity > window manager uses these thresholds to fire actions like spread (or > "expose" in OS X terms). > > -- Chase As far as I can tell, when the pad reports exactly 2 fingers, it reports them both with half the resolution with which it would report a single finger (in the 1 finger or 3 fingers cases). It sounded like Chase was recommending to report "ST" at full resolution and "MT" at half resolution. Thus, if there is 1 finger, we would throw away 1 bit for the MT report. If there were 2 fingers, we would inflate the ST reported finger by one bit. I prefer the original behavior of this patch, which just always reports ST and MT using the ST scale. -Daniel -- 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/elantech.c b/drivers/input/mouse/elantech.c index da161da..cf41f23 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -223,7 +223,7 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse) input_report_abs(dev, ABS_X, ((packet[1] & 0x0c) << 6) | packet[2]); input_report_abs(dev, ABS_Y, - ETP_YMAX_V1 - (((packet[1] & 0x03) << 8) | packet[3])); + etd->y_max - (((packet[1] & 0x03) << 8) | packet[3])); } input_report_key(dev, BTN_TOOL_FINGER, fingers == 1); @@ -233,7 +233,7 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse) input_report_key(dev, BTN_RIGHT, packet[0] & 0x02); if (etd->fw_version < 0x020000 && - (etd->capabilities & ETP_CAP_HAS_ROCKER)) { + (etd->capabilities[0] & ETP_CAP_HAS_ROCKER)) { /* rocker up */ input_report_key(dev, BTN_FORWARD, packet[0] & 0x40); /* rocker down */ @@ -298,7 +298,7 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse) * byte 4: . . . . y11 y10 y9 y8 * byte 5: y7 y6 y5 y4 y3 y2 y1 y0 */ - y1 = ETP_YMAX_V2 - (((packet[4] & 0x0f) << 8) | packet[5]); + y1 = etd->y_max - (((packet[4] & 0x0f) << 8) | packet[5]); input_report_abs(dev, ABS_X, x1); input_report_abs(dev, ABS_Y, y1); @@ -316,14 +316,14 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse) */ x1 = ((packet[0] & 0x10) << 4) | packet[1]; /* byte 2: ay7 ay6 ay5 ay4 ay3 ay2 ay1 ay0 */ - y1 = ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2]); + y1 = etd->y_2ft_max - (((packet[0] & 0x20) << 3) | packet[2]); /* * byte 3: . . by8 bx8 . . . . * byte 4: bx7 bx6 bx5 bx4 bx3 bx2 bx1 bx0 */ x2 = ((packet[3] & 0x10) << 4) | packet[4]; /* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */ - y2 = ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5]); + y2 = etd->y_2ft_max - (((packet[3] & 0x20) << 3) | packet[5]); /* * For compatibility with the X Synaptics driver scale up * one coordinate and report as ordinary mouse movent @@ -470,6 +470,42 @@ static int elantech_set_absolute_mode(struct psmouse *psmouse) return rc; } +static void set_range(struct psmouse *psmouse, unsigned int *x_min, + unsigned int *y_min, unsigned int *x_max, + unsigned int *y_max, unsigned int *y_2ft_max) +{ + struct elantech_data *etd = psmouse->private; + int i; + + switch (etd->hw_version) { + case 1: + *x_min = ETP_XMIN_V1; + *y_min = ETP_YMIN_V1; + *x_max = ETP_XMAX_V1; + *y_max = ETP_YMAX_V1; + break; + + case 2: + if (etd->fw_version == 0x020800 || + etd->fw_version == 0x020b00 || + etd->fw_version == 0x020030) { + *x_min = ETP_XMIN_V2; + *y_min = ETP_YMIN_V2; + *x_max = ETP_XMAX_V2; + *y_max = ETP_YMAX_V2; + *y_2ft_max = ETP_2FT_YMAX; + break; + } + + i = (etd->fw_version > 0x020800 && + etd->fw_version < 0x020900) ? 1 : 2; + *x_max = (etd->capabilities[1] - i) * 64; + *y_max = (etd->capabilities[2] - i) * 64; + *y_2ft_max = (*y_max - i) * 64 / 4; + break; + } +} + /* * Set the appropriate event bits for the input subsystem */ @@ -477,6 +513,9 @@ static void elantech_set_input_params(struct psmouse *psmouse) { struct input_dev *dev = psmouse->dev; struct elantech_data *etd = psmouse->private; + unsigned int x_min = 0, y_min = 0, x_max = 0, y_max = 0, y_2ft_max = 0; + + set_range(psmouse, &x_min, &y_min, &x_max, &y_max, &y_2ft_max); __set_bit(EV_KEY, dev->evbit); __set_bit(EV_ABS, dev->evbit); @@ -494,18 +533,18 @@ static void elantech_set_input_params(struct psmouse *psmouse) case 1: /* Rocker button */ if (etd->fw_version < 0x020000 && - (etd->capabilities & ETP_CAP_HAS_ROCKER)) { + (etd->capabilities[0] & ETP_CAP_HAS_ROCKER)) { __set_bit(BTN_FORWARD, dev->keybit); __set_bit(BTN_BACK, dev->keybit); } - input_set_abs_params(dev, ABS_X, ETP_XMIN_V1, ETP_XMAX_V1, 0, 0); - input_set_abs_params(dev, ABS_Y, ETP_YMIN_V1, ETP_YMAX_V1, 0, 0); + input_set_abs_params(dev, ABS_X, x_min, x_max, 0, 0); + input_set_abs_params(dev, ABS_Y, y_min, y_max, 0, 0); break; case 2: __set_bit(BTN_TOOL_QUADTAP, dev->keybit); - input_set_abs_params(dev, ABS_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0); - input_set_abs_params(dev, ABS_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 0); + input_set_abs_params(dev, ABS_X, x_min, x_max, 0, 0); + input_set_abs_params(dev, ABS_Y, y_min, y_max, 0, 0); if (etd->reports_pressure) { input_set_abs_params(dev, ABS_PRESSURE, ETP_PMIN_V2, ETP_PMAX_V2, 0, 0); @@ -514,10 +553,13 @@ static void elantech_set_input_params(struct psmouse *psmouse) } __set_bit(INPUT_PROP_SEMI_MT, dev->propbit); input_mt_init_slots(dev, 2); - input_set_abs_params(dev, ABS_MT_POSITION_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0); - input_set_abs_params(dev, ABS_MT_POSITION_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 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; } + + etd->y_max = y_max; + etd->y_2ft_max = y_2ft_max; } struct elantech_attr_data { @@ -771,13 +813,14 @@ int elantech_init(struct psmouse *psmouse) pr_info("assuming hardware version %d, firmware version %d.%d.%d\n", etd->hw_version, param[0], param[1], param[2]); - if (synaptics_send_cmd(psmouse, ETP_CAPABILITIES_QUERY, param)) { + if (synaptics_send_cmd(psmouse, ETP_CAPABILITIES_QUERY, + etd->capabilities)) { pr_err("failed to query capabilities.\n"); goto init_fail; } pr_info("Synaptics capabilities query result 0x%02x, 0x%02x, 0x%02x.\n", - param[0], param[1], param[2]); - etd->capabilities = param[0]; + etd->capabilities[0], etd->capabilities[1], + etd->capabilities[2]); /* * This firmware suffers from misreporting coordinates when diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h index fabb2b9..4b7447e 100644 --- a/drivers/input/mouse/elantech.h +++ b/drivers/input/mouse/elantech.h @@ -104,13 +104,15 @@ struct elantech_data { unsigned char reg_25; unsigned char reg_26; unsigned char debug; - unsigned char capabilities; + unsigned char capabilities[3]; bool paritycheck; bool jumpy_cursor; bool reports_pressure; unsigned char hw_version; unsigned int fw_version; unsigned int single_finger_reports; + unsigned int y_max; + unsigned int y_2ft_max; unsigned char parity[256]; };
With newer hardware, the touchpad provides range info. Let's use it. Signed-off-by: JJ Ding <jj_ding@emc.com.tw> --- drivers/input/mouse/elantech.c | 73 +++++++++++++++++++++++++++++++-------- drivers/input/mouse/elantech.h | 4 ++- 2 files changed, 61 insertions(+), 16 deletions(-)