Input: goodix - Fix touch coords on WinBook TW100 and TW700

PATCH Superseded Bastien Nocera 2 participants 5 messages Jul 23, 2015 – Jul 24, 2015

Subsystems (3)

HID Input drivers Goodix touchscreen

lore

Share & tools

lore

Bastien Nocera

author

2015-07-23 14:14 UTC #0 lore raw

PATCH v2 1/1 Input: goodix - Fix touch coords on WinBook TW100 and TW700

The touchscreen on the WinBook TW100 and TW700 don't match the default display, with 0,0 touches being reported when touching at the bottom right of the screen.

1280,800 0,800

         +-------------+
         |             |
         |             |
         |             |
         +-------------+
    1280,0             0,0

It's unfortunately impossible to detect this problem with data from the DSDT, or other auxiliary metadata, so fallback to quirking this specific model of tablet instead.

Signed-off-by Bastien Nocera <[email protected]>

Reviewed-by Benjamin Tissoires <[email protected]>

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

drivers/input/touchscreen/goodix.c +31 -0

index b4d12e2..3722806 100644
@@ -15,6 +15,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/dmi.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>

@@ -34,6 +35,7 @@ struct goodix_ts_data {
 	int abs_y_max;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
+	bool rotated_screen;
 };
 
 #define GOODIX_MAX_HEIGHT		4096

@@ -60,6 +62,24 @@ static const unsigned long goodix_irq_flags[] = {
 	IRQ_TYPE_LEVEL_HIGH,
 };
 
+/* Those tablets have their coords origin at the bottom right
+ * of the tablet, as if rotated 180 degrees */
+static const struct dmi_system_id rotated_screen[] = {
+	{
+		.ident = "WinBook TW100",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
+		},
+		.ident = "WinBook TW700",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
+		},
+	},
+	{}
+};
+
 /**
  * goodix_i2c_read - read data from a register of the i2c slave device.
  *

@@ -129,6 +149,11 @@ 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;
+	}
+
 	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);

@@ -223,6 +248,12 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 		ts->abs_y_max = GOODIX_MAX_HEIGHT;
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 	}
+
+	ts->rotated_screen = dmi_check_system(rotated_screen);
+	if (ts->rotated_screen) {
+		dev_dbg(&ts->client->dev,
+			 "Applying '180 degrees rotated screen' quirk\n");
+	}
 }
 
 /**

Signature · 1 line

2.4.3

Dmitry Torokhov

maintainer

2015-07-23 21:56 UTC #1 lore raw

Hi Bastien,

Quoted 43 lines · Bastien Nocera

The touchscreen on the WinBook TW100 and TW700 don't match the default display, with 0,0 touches being reported when touching at the bottom right of the screen.
1280,800 0,800
         +-------------+
         |             |
         |             |
         |             |
         +-------------+
    1280,0             0,0
It's unfortunately impossible to detect this problem with data from the DSDT, or other auxiliary metadata, so fallback to quirking this specific model of tablet instead.
Signed-off-by Bastien Nocera <[email protected]>
Reviewed-by Benjamin Tissoires <[email protected]>
---
 drivers/input/touchscreen/goodix.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
drivers/input/touchscreen/goodix.c +2 -0
index b4d12e2..3722806 100644
@@ -15,6 +15,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/dmi.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
@@ -34,6 +35,7 @@ struct goodix_ts_data {
 	int abs_y_max;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
+	bool rotated_screen;
 };
 
 #define GOODIX_MAX_HEIGHT		4096

drivers/input/touchscreen/goodix.c static const unsigned long goodix_irq_flags[] = { · line 66

Quoted 5 lines

drivers/input/touchscreen/goodix.c +2 -0

@@ -60,6 +62,24 @@ static const unsigned long goodix_irq_flags[] = {
 	IRQ_TYPE_LEVEL_HIGH,
 };
 
+/* Those tablets have their coords origin at the bottom right
+ * of the tablet, as if rotated 180 degrees */

/*
* Multi
* line
* comment
*/

please.

drivers/input/touchscreen/goodix.c static const unsigned long goodix_irq_flags[] = { · line 67

Quoted 1 line

drivers/input/touchscreen/goodix.c +1 -0
+static const struct dmi_system_id rotated_screen[] = {

#if defined(CONFIG_DMI) && defined(CONFIG_X86)

drivers/input/touchscreen/goodix.c static const unsigned long goodix_irq_flags[] = { · line 78

Quoted 11 lines

drivers/input/touchscreen/goodix.c +11 -0

+	{
+		.ident = "WinBook TW100",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
+		},
+		.ident = "WinBook TW700",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
+		},

This does not do what you want it to do... First of all you probably wanted TW700 on the second entry, second you need separate them into 2 entries into array (right now it is one still).

Thank you.

Signature · 1 line

Dmitry

Bastien Nocera

author

2015-07-24 11:14 UTC #2 lore raw

Quoted 65 lines · Dmitry Torokhov

Hi Bastien,
Quoted 49 lines · Bastien Nocera
The touchscreen on the WinBook TW100 and TW700 don't match the default
display, with 0,0 touches being reported when touching at the bottom
right of the screen.
1280,800 0,800
         +-------------+
         |             |
         |             |
         |             |
         +-------------+
    1280,0             0,0
It's unfortunately impossible to detect this problem with data from the
DSDT, or other auxiliary metadata, so fallback to quirking this specific
model of tablet instead.
Signed-off-by Bastien Nocera <[email protected]>
Reviewed-by Benjamin Tissoires <[email protected]>
---
 drivers/input/touchscreen/goodix.c | 31 
+++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
drivers/input/touchscreen/goodix.c +2 -0
index b4d12e2..3722806 100644
@@ -15,6 +15,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/dmi.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
@@ -34,6 +35,7 @@ struct goodix_ts_data {
 	int abs_y_max;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
+	bool rotated_screen;
 };
 
 #define GOODIX_MAX_HEIGHT		4096
drivers/input/touchscreen/goodix.c static const unsigned long goodix_irq_flags[] = · line 67
Quoted 6 lines
drivers/input/touchscreen/goodix.c +2 -0
@@ -60,6 +62,24 @@ static const unsigned long goodix_irq_flags[] = 
 {
 	IRQ_TYPE_LEVEL_HIGH,
 };
 
+/* Those tablets have their coords origin at the bottom right
+ * of the tablet, as if rotated 180 degrees */
/*
* Multi
* line
* comment
*/

Quoted 1 line

please.

Would be good if checkpatch could check for that, or is it a personal request?

Quoted 3 lines

drivers/input/touchscreen/goodix.c static const unsigned long goodix_irq_flags[] = { · line 67
Quoted 1 line
drivers/input/touchscreen/goodix.c +1 -0
+static const struct dmi_system_id rotated_screen[] = {
#if defined(CONFIG_DMI) && defined(CONFIG_X86)

Sure.

Quoted 8 lines

drivers/input/touchscreen/goodix.c +8 -0

+	{
+		.ident = "WinBook TW100",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
+		},
+		.ident = "WinBook TW700",
+		.matches = {

Quoted 8 lines

drivers/input/touchscreen/goodix.c static const unsigned long goodix_irq_flags[] = { · line 73
Quoted 3 lines
drivers/input/touchscreen/goodix.c +3 -0
+			DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
+		},
This does not do what you want it to do... First of all you probably wanted TW700 on the second entry, second you need separate them into 2
entries into array (right now it is one still).

Duh!

Thanks, coming in v3 shortly.

Dmitry Torokhov

maintainer

2015-07-24 16:08 UTC #3 lore raw

Quoted 66 lines · Bastien Nocera

Quoted 65 lines · Dmitry Torokhov
Hi Bastien,
Quoted 49 lines · Bastien Nocera
The touchscreen on the WinBook TW100 and TW700 don't match the default
display, with 0,0 touches being reported when touching at the bottom
right of the screen.
1280,800 0,800
         +-------------+
         |             |
         |             |
         |             |
         +-------------+
    1280,0             0,0
It's unfortunately impossible to detect this problem with data from the
DSDT, or other auxiliary metadata, so fallback to quirking this specific
model of tablet instead.
Signed-off-by Bastien Nocera <[email protected]>
Reviewed-by Benjamin Tissoires <[email protected]>
---
 drivers/input/touchscreen/goodix.c | 31 
+++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
drivers/input/touchscreen/goodix.c +2 -0
index b4d12e2..3722806 100644
@@ -15,6 +15,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/dmi.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
@@ -34,6 +35,7 @@ struct goodix_ts_data {
 	int abs_y_max;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
+	bool rotated_screen;
 };
 
 #define GOODIX_MAX_HEIGHT		4096
drivers/input/touchscreen/goodix.c static const unsigned long goodix_irq_flags[] = · line 67
Quoted 6 lines
drivers/input/touchscreen/goodix.c +2 -0
@@ -60,6 +62,24 @@ static const unsigned long goodix_irq_flags[] = 
 {
 	IRQ_TYPE_LEVEL_HIGH,
 };
 
+/* Those tablets have their coords origin at the bottom right
+ * of the tablet, as if rotated 180 degrees */
/*
* Multi
* line
* comment
*/

Quoted 4 lines

Quoted 1 line
please.
Would be good if checkpatch could check for that, or is it a personal request?

CodingStyle chapter 8.

Thanks.

Signature · 1 line

Dmitry

Bastien Nocera

author

2015-07-24 16:09 UTC #4 lore raw

Quoted 70 lines · Dmitry Torokhov

Quoted 69 lines · Bastien Nocera
Quoted 68 lines · Dmitry Torokhov
Hi Bastien,
Quoted 53 lines · Bastien Nocera
The touchscreen on the WinBook TW100 and TW700 don't match the default
display, with 0,0 touches being reported when touching at the bottom
right of the screen.
1280,800 0,800
         +-------------+
         |             |
         |             |
         |             |
         +-------------+
    1280,0             0,0
It's unfortunately impossible to detect this problem with data from
the
DSDT, or other auxiliary metadata, so fallback to quirking this
specific
model of tablet instead.
Signed-off-by Bastien Nocera <[email protected]>
Reviewed-by Benjamin Tissoires <[email protected]>
---
 drivers/input/touchscreen/goodix.c | 31 
+++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
drivers/input/touchscreen/goodix.c +2 -0
index b4d12e2..3722806 100644
@@ -15,6 +15,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/dmi.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
@@ -34,6 +35,7 @@ struct goodix_ts_data {
 	int abs_y_max;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
+	bool rotated_screen;
 };
 
 #define GOODIX_MAX_HEIGHT		4096
@@ -60,6 +62,24 @@ static const unsigned long 
goodix_irq_flags[] =
{
drivers/input/touchscreen/goodix.c static const unsigned long goodix_irq_flags[] = { · line 66
Quoted 5 lines
drivers/input/touchscreen/goodix.c +2 -0
 	IRQ_TYPE_LEVEL_HIGH,
 };
 
+/* Those tablets have their coords origin at the bottom right
+ * of the tablet, as if rotated 180 degrees */
/*
* Multi
* line
* comment
*/

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
please.
Would be good if checkpatch could check for that, or is it a personal
request?
CodingStyle chapter 8.

OK, thanks for the notice :)