[PATCH RFC 0/2] Input: Goodix - add axis swapping and inversion support

[PATCH RFC 0/2] Input: Goodix - add axis swapping and inversion support

[Karsten Merker]

Hello,

the goodix touchscreen driver has gained device-tree support in kernel
4.1, but doesn't currently support the touchscreen-swapped-x-y,
touchscreen-inverted-x and touchscreen-inverted-y properties.

On systems which combine a portrait-mode display with a landscape-mode
touchscreen, such as e.g. the MSI Primo 81 tablet, support for these
features is necessary to have the touchscreen and the display use the
same coordinate system.

With support for axis inversion, the "rotated_screen" flag in the
driver can also be removed, as "rotated_screen" is just a special case
of x/y axis inversion.

This patchset sits on top of the "[PATCH v6 0/9] Goodix touchscreen
enhancements" series by Irina Tirdea:
http://www.spinics.net/lists/linux-input/msg40992.html

I have successfully tested the axis swapping on an (arm-based) MSI
Primo 81 tablet, but I lack appropriate hardware to do a real-world
test of the "rotated_screen" code path, so I would apprechiate very
much if somebody with appropriate hardware could give it a try.

This is my first go at modifying an input driver; any kind of
feedback and hints is welcome.

Regards,
Karsten

Karsten Merker (2):
  Input: goodix - add dt axis swapping and axis inversion support
  Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"

 drivers/input/touchscreen/goodix.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

-- 
2.1.4

[PATCH 1/2] Input: goodix - add dt axis swapping and axis inversion support

[Karsten Merker]

Implement support for the following device-tree properties
in the goodix touchscreen driver:

 - touchscreen-inverted-x:  X axis is inverted (boolean)
 - touchscreen-inverted-y:  Y axis is inverted (boolean)
 - touchscreen-swapped-x-y: X and Y axis are swapped (boolean)

These are necessary on tablets which have a display in portrait
format while the touchscreen is in landscape format, such as e.g.
the MSI Primo 81.

Signed-off-by: Karsten Merker <merker@debian.org>
---
 drivers/input/touchscreen/goodix.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f8e5b49..f2b1590 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -2,6 +2,7 @@
  *  Driver for Goodix Touchscreens
  *
  *  Copyright (c) 2014 Red Hat Inc.
+ *  Copyright (c) 2015 K. Merker <merker@debian.org>
  *
  *  This code is based on gt9xx.c authored by andrew@goodix.com:
  *
@@ -48,6 +49,9 @@ struct goodix_ts_data {
 	unsigned long irq_flags;
 	atomic_t esd_timeout;
 	struct delayed_work esd_work;
+	bool swapped_x_y;
+	bool inverted_x;
+	bool inverted_y;
 };
 
 #define GOODIX_DEVICE_ESD_TIMEOUT_PROPERTY     "esd-recovery-timeout-ms"
@@ -262,6 +266,14 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
 		input_y = ts->abs_y_max - input_y;
 	}
 
+	/* Inversions have to happen before axis swapping */
+	if (ts->inverted_x)
+		input_x = ts->abs_x_max - input_x;
+	if (ts->inverted_y)
+		input_y = ts->abs_y_max - input_y;
+	if (ts->swapped_x_y)
+		swap(input_x, input_y);
+
 	input_mt_slot(ts->input_dev, id);
 	input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
 	input_report_abs(ts->input_dev, ABS_MT_POSITION_X, input_x);
@@ -670,6 +682,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 			 error);
 		ts->abs_x_max = GOODIX_MAX_WIDTH;
 		ts->abs_y_max = GOODIX_MAX_HEIGHT;
+		if (ts->swapped_x_y)
+			swap(ts->abs_x_max, ts->abs_y_max);
 		ts->int_trigger_type = GOODIX_INT_TRIGGER;
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 		return;
@@ -677,6 +691,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 
 	ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
 	ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
+	if (ts->swapped_x_y)
+		swap(ts->abs_x_max, ts->abs_y_max);
 	ts->int_trigger_type = config[TRIGGER_LOC] & 0x03;
 	ts->max_touch_num = config[MAX_CONTACTS_LOC] & 0x0f;
 	if (!ts->abs_x_max || !ts->abs_y_max || !ts->max_touch_num) {
@@ -684,6 +700,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 			"Invalid config, using defaults\n");
 		ts->abs_x_max = GOODIX_MAX_WIDTH;
 		ts->abs_y_max = GOODIX_MAX_HEIGHT;
+		if (ts->swapped_x_y)
+			swap(ts->abs_x_max, ts->abs_y_max);
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 	}
 
@@ -913,6 +931,15 @@ static int goodix_ts_probe(struct i2c_client *client,
 
 	ts->cfg_len = goodix_get_cfg_len(ts->id);
 
+#ifdef CONFIG_OF
+	ts->swapped_x_y = of_property_read_bool(client->dev.of_node,
+						"touchscreen-swapped-x-y");
+	ts->inverted_x = of_property_read_bool(client->dev.of_node,
+					       "touchscreen-inverted-x");
+	ts->inverted_y = of_property_read_bool(client->dev.of_node,
+					       "touchscreen-inverted-y");
+#endif
+
 	goodix_read_config(ts);
 
 	if (ts->gpiod_int && ts->gpiod_rst) {
-- 
2.1.4

[PATCH 2/2] Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"

[Karsten Merker]

The goodix touchscreen driver uses a "rotated_screen" flag for
systems on which the touchscreen is mounted rotated by 180
degrees with respect to the display.  With the addition of
support for the dt properties "touchscreen-inverted-x" and
"touchscreen-inverted-y", a separate "rotated_screen" flag
is not necessary any more. This patch replaces it by setting
the inverted-x and inverted-x flags instead.

Signed-off-by: Karsten Merker <merker@debian.org>
---
 drivers/input/touchscreen/goodix.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f2b1590..6ce909c 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -39,7 +39,6 @@ struct goodix_ts_data {
 	int abs_y_max;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
-	bool rotated_screen;
 	int cfg_len;
 	struct gpio_desc *gpiod_int;
 	struct gpio_desc *gpiod_rst;
@@ -261,11 +260,6 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
 	int input_y = get_unaligned_le16(&coor_data[3]);
 	int input_w = get_unaligned_le16(&coor_data[5]);
 
-	if (ts->rotated_screen) {
-		input_x = ts->abs_x_max - input_x;
-		input_y = ts->abs_y_max - input_y;
-	}
-
 	/* Inversions have to happen before axis swapping */
 	if (ts->inverted_x)
 		input_x = ts->abs_x_max - input_x;
@@ -705,10 +699,12 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 	}
 
-	ts->rotated_screen = dmi_check_system(rotated_screen);
-	if (ts->rotated_screen)
+	if (dmi_check_system(rotated_screen)) {
+		ts->inverted_x = true;
+		ts->inverted_y = true;
 		dev_dbg(&ts->client->dev,
 			 "Applying '180 degrees rotated screen' quirk\n");
+	}
 }
 
 /**
-- 
2.1.4

Re: [PATCH RFC 0/2] Input: Goodix - add axis swapping and inversion support

[Bastien Nocera]

On Thu, 2015-10-01 at 00:29 +0200, Karsten Merker wrote:
> Hello,
> 
> the goodix touchscreen driver has gained device-tree support in
> kernel
> 4.1, but doesn't currently support the touchscreen-swapped-x-y,
> touchscreen-inverted-x and touchscreen-inverted-y properties.

Can't test for now, but:

Reviewed-by: Bastien Nocera <hadess@hadess.net>

We'll have to wait until Irina's patchset lands though, and we'll test
at that point.

Cheers