diff mbox series

[2/4] HID: wiiu-drc: Implement touch reports

Message ID 20210502232836.26134-3-linkmauve@linkmauve.fr
State New
Headers show
Series RFC: HID: wiiu-drc: Add a driver for the Wii U gamepad | expand

Commit Message

Link Mauve May 2, 2021, 11:28 p.m. UTC 
There is a 100×200 inaccessible border on each side, and the Y axis is
inverted, these are the two main quirks of this touch panel.

I’ve been testing with weston-simple-touch mostly, but it also with the
rest of Weston.

Signed-off-by: Ash Logan <ash@heyquark.com>
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
---
 drivers/hid/hid-wiiu-drc.c | 83 +++++++++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 5 deletions(-)

Comments

Jonathan Neuschäfer May 5, 2021, 10:43 p.m. UTC | #1 
Hi,

some more comments below. Enjoy :)

On Mon, May 03, 2021 at 01:28:33AM +0200, Emmanuel Gil Peyrot wrote:
> There is a 100×200 inaccessible border on each side, and the Y axis is

> inverted, these are the two main quirks of this touch panel.


Does that mean 100 px borders left and right, and 200 px borders top and
bottom?

100×200 evokes the image of a rectangle of that size, which I found
confusing for a moment.

> 

> I’ve been testing with weston-simple-touch mostly, but it also with the

> rest of Weston.

> 

> Signed-off-by: Ash Logan <ash@heyquark.com>

> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>

> ---

>  drivers/hid/hid-wiiu-drc.c | 83 +++++++++++++++++++++++++++++++++++---

>  1 file changed, 78 insertions(+), 5 deletions(-)

> 

> diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c

> index 018cbdb53a2c..77e70827c37d 100644

> --- a/drivers/hid/hid-wiiu-drc.c

> +++ b/drivers/hid/hid-wiiu-drc.c

> @@ -49,13 +49,27 @@

>  

>  #define BUTTON_POWER	BIT(25)

>  

> +/* Touch constants */

> +/* Resolution in pixels */

> +#define RES_X		854

> +#define RES_Y		480

> +/* Display/touch size in mm */

> +#define WIDTH		138

> +#define HEIGHT		79

> +#define NUM_TOUCH_POINTS 10

> +#define MAX_TOUCH_RES	(1 << 12)

> +#define TOUCH_BORDER_X	100

> +#define TOUCH_BORDER_Y	200


[...]
> +	/* touch */

> +	/* Average touch points for improved accuracy. */

> +	x = y = 0;

> +	for (i = 0; i < NUM_TOUCH_POINTS; i++) {

> +		base = 36 + 4 * i;

> +

> +		x += ((data[base + 1] & 0xF) << 8) | data[base];

> +		y += ((data[base + 3] & 0xF) << 8) | data[base + 2];

> +	}

> +	x /= NUM_TOUCH_POINTS;

> +	y /= NUM_TOUCH_POINTS;


Given that there are 10 possible touch points: Does the gamepad actually
support multitouch (usefully)?

If so, I think it would be better to report all touch points
individually to userspace, to allow for multitouch gestures;
userspace can still implement averaging if desired.



Thanks,
Jonathan
Link Mauve May 6, 2021, 10:20 a.m. UTC | #2 
On Wed, May 05, 2021 at 10:43:55PM +0000, Jonathan Neuschäfer wrote:
> Hi,

> 

> some more comments below. Enjoy :)

> 

> On Mon, May 03, 2021 at 01:28:33AM +0200, Emmanuel Gil Peyrot wrote:

> > There is a 100×200 inaccessible border on each side, and the Y axis is

> > inverted, these are the two main quirks of this touch panel.

> 

> Does that mean 100 px borders left and right, and 200 px borders top and

> bottom?


Correct, I’ll reformulate in v2. :)

> 

> 100×200 evokes the image of a rectangle of that size, which I found

> confusing for a moment.

> 

> > 

> > I’ve been testing with weston-simple-touch mostly, but it also with the

> > rest of Weston.

> > 

> > Signed-off-by: Ash Logan <ash@heyquark.com>

> > Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>

> > ---

> >  drivers/hid/hid-wiiu-drc.c | 83 +++++++++++++++++++++++++++++++++++---

> >  1 file changed, 78 insertions(+), 5 deletions(-)

> > 

> > diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c

> > index 018cbdb53a2c..77e70827c37d 100644

> > --- a/drivers/hid/hid-wiiu-drc.c

> > +++ b/drivers/hid/hid-wiiu-drc.c

> > @@ -49,13 +49,27 @@

> >  

> >  #define BUTTON_POWER	BIT(25)

> >  

> > +/* Touch constants */

> > +/* Resolution in pixels */

> > +#define RES_X		854

> > +#define RES_Y		480

> > +/* Display/touch size in mm */

> > +#define WIDTH		138

> > +#define HEIGHT		79

> > +#define NUM_TOUCH_POINTS 10

> > +#define MAX_TOUCH_RES	(1 << 12)

> > +#define TOUCH_BORDER_X	100

> > +#define TOUCH_BORDER_Y	200

> 

> [...]

> > +	/* touch */

> > +	/* Average touch points for improved accuracy. */

> > +	x = y = 0;

> > +	for (i = 0; i < NUM_TOUCH_POINTS; i++) {

> > +		base = 36 + 4 * i;

> > +

> > +		x += ((data[base + 1] & 0xF) << 8) | data[base];

> > +		y += ((data[base + 3] & 0xF) << 8) | data[base + 2];

> > +	}

> > +	x /= NUM_TOUCH_POINTS;

> > +	y /= NUM_TOUCH_POINTS;

> 

> Given that there are 10 possible touch points: Does the gamepad actually

> support multitouch (usefully)?

> 

> If so, I think it would be better to report all touch points

> individually to userspace, to allow for multitouch gestures;

> userspace can still implement averaging if desired.


Sadly no, in my testing all ten reports are always within a few units
from each other, even if I press two (or more) different points on the
touchscreen at the same time.

My guess would be, the firmware and report format got written before
Nintendo decided whether to go for a capacitive or resistive touch
panel, and they didn’t get changed once the final decision was made to
go for a non-multitouch-aware resistive panel.

I’ll add some factual comment about this in v2.

> 

> 

> 

> Thanks,

> Jonathan


Thanks!

-- 
Emmanuel Gil Peyrot
diff mbox series

Patch

diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
index 018cbdb53a2c..77e70827c37d 100644
--- a/drivers/hid/hid-wiiu-drc.c
+++ b/drivers/hid/hid-wiiu-drc.c
@@ -49,13 +49,27 @@ 
 
 #define BUTTON_POWER	BIT(25)
 
+/* Touch constants */
+/* Resolution in pixels */
+#define RES_X		854
+#define RES_Y		480
+/* Display/touch size in mm */
+#define WIDTH		138
+#define HEIGHT		79
+#define NUM_TOUCH_POINTS 10
+#define MAX_TOUCH_RES	(1 << 12)
+#define TOUCH_BORDER_X	100
+#define TOUCH_BORDER_Y	200
+
 /*
  * The device is setup with multiple input devices:
  * - A joypad with the buttons and sticks.
+ * - The touch area which works as a touchscreen.
  */
 
 struct drc {
 	struct input_dev *joy_input_dev;
+	struct input_dev *touch_input_dev;
 	struct hid_device *hdev;
 };
 
@@ -63,7 +77,7 @@  static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
 	 u8 *data, int len)
 {
 	struct drc *drc = hid_get_drvdata(hdev);
-	int i;
+	int i, x, y, pressure, base;
 	u32 buttons;
 
 	if (len != 128)
@@ -126,6 +140,37 @@  static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	input_sync(drc->joy_input_dev);
 
+	/* touch */
+	/* Average touch points for improved accuracy. */
+	x = y = 0;
+	for (i = 0; i < NUM_TOUCH_POINTS; i++) {
+		base = 36 + 4 * i;
+
+		x += ((data[base + 1] & 0xF) << 8) | data[base];
+		y += ((data[base + 3] & 0xF) << 8) | data[base + 2];
+	}
+	x /= NUM_TOUCH_POINTS;
+	y /= NUM_TOUCH_POINTS;
+
+	/* Pressure reporting isn’t properly understood, so we don’t report it yet. */
+	pressure = 0;
+	pressure |= ((data[37] >> 4) & 7) << 0;
+	pressure |= ((data[39] >> 4) & 7) << 3;
+	pressure |= ((data[41] >> 4) & 7) << 6;
+	pressure |= ((data[43] >> 4) & 7) << 9;
+
+	if (pressure != 0) {
+		input_report_key(drc->touch_input_dev, BTN_TOUCH, 1);
+		input_report_key(drc->touch_input_dev, BTN_TOOL_FINGER, 1);
+
+		input_report_abs(drc->touch_input_dev, ABS_X, x);
+		input_report_abs(drc->touch_input_dev, ABS_Y, MAX_TOUCH_RES - y);
+	} else {
+		input_report_key(drc->touch_input_dev, BTN_TOUCH, 0);
+		input_report_key(drc->touch_input_dev, BTN_TOOL_FINGER, 0);
+	}
+	input_sync(drc->touch_input_dev);
+
 	/* let hidraw and hiddev handle the report */
 	return 0;
 }
@@ -168,6 +213,32 @@  static struct input_dev *allocate_and_setup(struct hid_device *hdev,
 	return input_dev;
 }
 
+static bool drc_setup_touch(struct drc *drc,
+		struct hid_device *hdev)
+{
+	struct input_dev *input_dev;
+
+	input_dev = allocate_and_setup(hdev, DEVICE_NAME " Touch");
+	if (!input_dev)
+		return false;
+
+	input_dev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY);
+
+	input_set_abs_params(input_dev, ABS_X, TOUCH_BORDER_X, MAX_TOUCH_RES - TOUCH_BORDER_X, 20, 0);
+	input_abs_set_res(input_dev, ABS_X, RES_X / WIDTH);
+	input_set_abs_params(input_dev, ABS_Y, TOUCH_BORDER_Y, MAX_TOUCH_RES - TOUCH_BORDER_Y, 20, 0);
+	input_abs_set_res(input_dev, ABS_Y, RES_Y / HEIGHT);
+
+	set_bit(BTN_TOUCH, input_dev->keybit);
+	set_bit(BTN_TOOL_FINGER, input_dev->keybit);
+
+	set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
+
+	drc->touch_input_dev = input_dev;
+
+	return true;
+}
+
 static bool drc_setup_joypad(struct drc *drc,
 		struct hid_device *hdev)
 {
@@ -231,14 +302,16 @@  static int drc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
-	if (!drc_setup_joypad(drc, hdev)) {
-		hid_err(hdev, "could not allocate interface\n");
+	if (!drc_setup_joypad(drc, hdev) ||
+	    !drc_setup_touch(drc, hdev)) {
+		hid_err(hdev, "could not allocate interfaces\n");
 		return -ENOMEM;
 	}
 
-	ret = input_register_device(drc->joy_input_dev);
+	ret = input_register_device(drc->joy_input_dev) ||
+		input_register_device(drc->touch_input_dev);
 	if (ret) {
-		hid_err(hdev, "failed to register interface\n");
+		hid_err(hdev, "failed to register interfaces\n");
 		return ret;
 	}