| Message ID | 20180511133040.7793-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Hans, On Fri, May 11, 2018 at 03:30:40PM +0200, Hans de Goede wrote: > Some Silead bases touchscreens (depending on the firmware and/or the > regulator) report coordinates which never reach 0 along one or both > of their axis. > > This commit adds support for setting a "silead,min-x" and "min-y" property > which indicates the minimum x/y values reported. > > When enabled this fixes e.g. not being able to click things in the GNOME3 > top-bar on same tablets. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > .../input/touchscreen/silead_gsl1680.txt | 3 +++ > drivers/input/touchscreen/silead.c | 23 ++++++++++++++++--- > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt > index 84752de12412..07e19ef46c56 100644 > --- a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt > +++ b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt > @@ -25,6 +25,9 @@ Optional properties: > - silead,max-fingers : maximum number of fingers the touchscreen can detect > - silead,home-button : Boolean, set to true on devices which have a > capacitive home-button build into the touchscreen > +- silead,min-x : minimum raw x value reported by the touchscreen, set > + this for firmwares/digitizers where this is not 0 > +- silead,min-y : minimum raw y value reported by the touchscreen I'd rather this go into generic touchscreen bindings as "touchscreen-min-x" and "touchscreen-min-y". Rob? > - vddio-supply : regulator phandle for controller VDDIO > - avdd-supply : regulator phandle for controller AVDD > > diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c > index ff7043f74a3d..024d15ba6d1b 100644 > --- a/drivers/input/touchscreen/silead.c > +++ b/drivers/input/touchscreen/silead.c > @@ -77,6 +77,8 @@ struct silead_ts_data { > struct regulator_bulk_data regulators[2]; > char fw_name[64]; > struct touchscreen_properties prop; > + int min_x; > + int min_y; > u32 max_fingers; > u32 chip_id; > struct input_mt_pos pos[SILEAD_MAX_FINGERS]; > @@ -144,6 +146,7 @@ static void silead_ts_read_data(struct i2c_client *client) > u8 *bufp, buf[SILEAD_TS_DATA_LEN]; > int touch_nr, softbutton, error, i; > bool softbutton_pressed = false; > + int x, y; > > error = i2c_smbus_read_i2c_block_data(client, SILEAD_REG_DATA, > SILEAD_TS_DATA_LEN, buf); > @@ -182,9 +185,14 @@ static void silead_ts_read_data(struct i2c_client *client) > */ > data->id[touch_nr] = (bufp[SILEAD_POINT_X_MSB_OFF] & > SILEAD_EXTRA_DATA_MASK) >> 4; > - touchscreen_set_mt_pos(&data->pos[touch_nr], &data->prop, > - get_unaligned_le16(&bufp[SILEAD_POINT_X_OFF]) & 0xfff, > - get_unaligned_le16(&bufp[SILEAD_POINT_Y_OFF]) & 0xfff); > + > + x = get_unaligned_le16(&bufp[SILEAD_POINT_X_OFF]) & 0xfff; > + x = max(x - data->min_x, 0); > + > + y = get_unaligned_le16(&bufp[SILEAD_POINT_Y_OFF]) & 0xfff; > + y = max(y - data->min_y, 0); "Normalizing" the coordinates is not what kernel normally does, but is better left for userpsace. Peter, will the userspace DTRT when presented with X/Y axis with non-zero minimum supplied? Thanks.
On Tue, May 15, 2018 at 10:18:25AM -0700, Dmitry Torokhov wrote: > Hi Hans, > > On Fri, May 11, 2018 at 03:30:40PM +0200, Hans de Goede wrote: > > Some Silead bases touchscreens (depending on the firmware and/or the > > regulator) report coordinates which never reach 0 along one or both > > of their axis. > > > > This commit adds support for setting a "silead,min-x" and "min-y" property > > which indicates the minimum x/y values reported. > > > > When enabled this fixes e.g. not being able to click things in the GNOME3 > > top-bar on same tablets. > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > --- > > .../input/touchscreen/silead_gsl1680.txt | 3 +++ > > drivers/input/touchscreen/silead.c | 23 ++++++++++++++++--- > > 2 files changed, 23 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt > > index 84752de12412..07e19ef46c56 100644 > > --- a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt > > +++ b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt > > @@ -25,6 +25,9 @@ Optional properties: > > - silead,max-fingers : maximum number of fingers the touchscreen can detect > > - silead,home-button : Boolean, set to true on devices which have a > > capacitive home-button build into the touchscreen > > +- silead,min-x : minimum raw x value reported by the touchscreen, set > > + this for firmwares/digitizers where this is not 0 > > +- silead,min-y : minimum raw y value reported by the touchscreen > > I'd rather this go into generic touchscreen bindings as > "touchscreen-min-x" and "touchscreen-min-y". Rob? > > > - vddio-supply : regulator phandle for controller VDDIO > > - avdd-supply : regulator phandle for controller AVDD > > > > diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c > > index ff7043f74a3d..024d15ba6d1b 100644 > > --- a/drivers/input/touchscreen/silead.c > > +++ b/drivers/input/touchscreen/silead.c > > @@ -77,6 +77,8 @@ struct silead_ts_data { > > struct regulator_bulk_data regulators[2]; > > char fw_name[64]; > > struct touchscreen_properties prop; > > + int min_x; > > + int min_y; > > u32 max_fingers; > > u32 chip_id; > > struct input_mt_pos pos[SILEAD_MAX_FINGERS]; > > @@ -144,6 +146,7 @@ static void silead_ts_read_data(struct i2c_client *client) > > u8 *bufp, buf[SILEAD_TS_DATA_LEN]; > > int touch_nr, softbutton, error, i; > > bool softbutton_pressed = false; > > + int x, y; > > > > error = i2c_smbus_read_i2c_block_data(client, SILEAD_REG_DATA, > > SILEAD_TS_DATA_LEN, buf); > > @@ -182,9 +185,14 @@ static void silead_ts_read_data(struct i2c_client *client) > > */ > > data->id[touch_nr] = (bufp[SILEAD_POINT_X_MSB_OFF] & > > SILEAD_EXTRA_DATA_MASK) >> 4; > > - touchscreen_set_mt_pos(&data->pos[touch_nr], &data->prop, > > - get_unaligned_le16(&bufp[SILEAD_POINT_X_OFF]) & 0xfff, > > - get_unaligned_le16(&bufp[SILEAD_POINT_Y_OFF]) & 0xfff); > > + > > + x = get_unaligned_le16(&bufp[SILEAD_POINT_X_OFF]) & 0xfff; > > + x = max(x - data->min_x, 0); > > + > > + y = get_unaligned_le16(&bufp[SILEAD_POINT_Y_OFF]) & 0xfff; > > + y = max(y - data->min_y, 0); > > "Normalizing" the coordinates is not what kernel normally does, but > is better left for userpsace. > > Peter, will the userspace DTRT when presented with X/Y axis with > non-zero minimum supplied? yeah, we've been assuming non-zero minimums for axes for few years now. Only exceptions are ABS_MT_SLOT and the axes where zero is a defined magic value like ABS_MT_ORIENTATION. There's also the LIBINPUT_CALIBRATION_MATRIX udev property which is intended for devices that report correct-ish coordinates but are mounted wrong, e.g. upside down. Cheers, Peter -- 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 15-05-18 19:18, Dmitry Torokhov wrote: > Hi Hans, > > On Fri, May 11, 2018 at 03:30:40PM +0200, Hans de Goede wrote: >> Some Silead bases touchscreens (depending on the firmware and/or the >> regulator) report coordinates which never reach 0 along one or both >> of their axis. >> >> This commit adds support for setting a "silead,min-x" and "min-y" property >> which indicates the minimum x/y values reported. >> >> When enabled this fixes e.g. not being able to click things in the GNOME3 >> top-bar on same tablets. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> .../input/touchscreen/silead_gsl1680.txt | 3 +++ >> drivers/input/touchscreen/silead.c | 23 ++++++++++++++++--- >> 2 files changed, 23 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt >> index 84752de12412..07e19ef46c56 100644 >> --- a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt >> +++ b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt >> @@ -25,6 +25,9 @@ Optional properties: >> - silead,max-fingers : maximum number of fingers the touchscreen can detect >> - silead,home-button : Boolean, set to true on devices which have a >> capacitive home-button build into the touchscreen >> +- silead,min-x : minimum raw x value reported by the touchscreen, set >> + this for firmwares/digitizers where this is not 0 >> +- silead,min-y : minimum raw y value reported by the touchscreen > > I'd rather this go into generic touchscreen bindings as > "touchscreen-min-x" and "touchscreen-min-y". Rob? Ah yes, together with just putting this as min in absinfo and supporting "touchscreen-min-x" and "touchscreen-min-y" in the generic of_touchscreen code, this would nicely support setting dealing with screens with non 0 minimum coordinates for all touchscreen drivers using the of_touchscreen helpers. I'll submit a new patch with that approach. Regards, Hans >> - vddio-supply : regulator phandle for controller VDDIO >> - avdd-supply : regulator phandle for controller AVDD >> >> diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c >> index ff7043f74a3d..024d15ba6d1b 100644 >> --- a/drivers/input/touchscreen/silead.c >> +++ b/drivers/input/touchscreen/silead.c >> @@ -77,6 +77,8 @@ struct silead_ts_data { >> struct regulator_bulk_data regulators[2]; >> char fw_name[64]; >> struct touchscreen_properties prop; >> + int min_x; >> + int min_y; >> u32 max_fingers; >> u32 chip_id; >> struct input_mt_pos pos[SILEAD_MAX_FINGERS]; >> @@ -144,6 +146,7 @@ static void silead_ts_read_data(struct i2c_client *client) >> u8 *bufp, buf[SILEAD_TS_DATA_LEN]; >> int touch_nr, softbutton, error, i; >> bool softbutton_pressed = false; >> + int x, y; >> >> error = i2c_smbus_read_i2c_block_data(client, SILEAD_REG_DATA, >> SILEAD_TS_DATA_LEN, buf); >> @@ -182,9 +185,14 @@ static void silead_ts_read_data(struct i2c_client *client) >> */ >> data->id[touch_nr] = (bufp[SILEAD_POINT_X_MSB_OFF] & >> SILEAD_EXTRA_DATA_MASK) >> 4; >> - touchscreen_set_mt_pos(&data->pos[touch_nr], &data->prop, >> - get_unaligned_le16(&bufp[SILEAD_POINT_X_OFF]) & 0xfff, >> - get_unaligned_le16(&bufp[SILEAD_POINT_Y_OFF]) & 0xfff); >> + >> + x = get_unaligned_le16(&bufp[SILEAD_POINT_X_OFF]) & 0xfff; >> + x = max(x - data->min_x, 0); >> + >> + y = get_unaligned_le16(&bufp[SILEAD_POINT_Y_OFF]) & 0xfff; >> + y = max(y - data->min_y, 0); > > "Normalizing" the coordinates is not what kernel normally does, but > is better left for userpsace. > > Peter, will the userspace DTRT when presented with X/Y axis with > non-zero minimum supplied? > > Thanks. > -- 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/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt index 84752de12412..07e19ef46c56 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt +++ b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt @@ -25,6 +25,9 @@ Optional properties: - silead,max-fingers : maximum number of fingers the touchscreen can detect - silead,home-button : Boolean, set to true on devices which have a capacitive home-button build into the touchscreen +- silead,min-x : minimum raw x value reported by the touchscreen, set + this for firmwares/digitizers where this is not 0 +- silead,min-y : minimum raw y value reported by the touchscreen - vddio-supply : regulator phandle for controller VDDIO - avdd-supply : regulator phandle for controller AVDD diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c index ff7043f74a3d..024d15ba6d1b 100644 --- a/drivers/input/touchscreen/silead.c +++ b/drivers/input/touchscreen/silead.c @@ -77,6 +77,8 @@ struct silead_ts_data { struct regulator_bulk_data regulators[2]; char fw_name[64]; struct touchscreen_properties prop; + int min_x; + int min_y; u32 max_fingers; u32 chip_id; struct input_mt_pos pos[SILEAD_MAX_FINGERS]; @@ -144,6 +146,7 @@ static void silead_ts_read_data(struct i2c_client *client) u8 *bufp, buf[SILEAD_TS_DATA_LEN]; int touch_nr, softbutton, error, i; bool softbutton_pressed = false; + int x, y; error = i2c_smbus_read_i2c_block_data(client, SILEAD_REG_DATA, SILEAD_TS_DATA_LEN, buf); @@ -182,9 +185,14 @@ static void silead_ts_read_data(struct i2c_client *client) */ data->id[touch_nr] = (bufp[SILEAD_POINT_X_MSB_OFF] & SILEAD_EXTRA_DATA_MASK) >> 4; - touchscreen_set_mt_pos(&data->pos[touch_nr], &data->prop, - get_unaligned_le16(&bufp[SILEAD_POINT_X_OFF]) & 0xfff, - get_unaligned_le16(&bufp[SILEAD_POINT_Y_OFF]) & 0xfff); + + x = get_unaligned_le16(&bufp[SILEAD_POINT_X_OFF]) & 0xfff; + x = max(x - data->min_x, 0); + + y = get_unaligned_le16(&bufp[SILEAD_POINT_Y_OFF]) & 0xfff; + y = max(y - data->min_y, 0); + + touchscreen_set_mt_pos(&data->pos[touch_nr], &data->prop, x, y); touch_nr++; } @@ -408,6 +416,7 @@ static void silead_ts_read_props(struct i2c_client *client) struct device *dev = &client->dev; const char *str; int error; + u32 val; error = device_property_read_u32(dev, "silead,max-fingers", &data->max_fingers); @@ -422,6 +431,14 @@ static void silead_ts_read_props(struct i2c_client *client) "silead/%s", str); else dev_dbg(dev, "Firmware file name read error. Using default."); + + error = device_property_read_u32(dev, "silead,min-x", &val); + if (!error) + data->min_x = val; + + error = device_property_read_u32(dev, "silead,min-y", &val); + if (!error) + data->min_y = val; } #ifdef CONFIG_ACPI
Some Silead bases touchscreens (depending on the firmware and/or the regulator) report coordinates which never reach 0 along one or both of their axis. This commit adds support for setting a "silead,min-x" and "min-y" property which indicates the minimum x/y values reported. When enabled this fixes e.g. not being able to click things in the GNOME3 top-bar on same tablets. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- .../input/touchscreen/silead_gsl1680.txt | 3 +++ drivers/input/touchscreen/silead.c | 23 ++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)