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

From: "Jonathan Neuschäfer" <j.ne@posteo.net>
To: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
Cc: linux-input@vger.kernel.org, "Ash Logan" <ash@heyquark.com>,
	"Jonathan Neuschäfer" <j.ne@posteo.net>,
	"Jiri Kosina" <jikos@kernel.org>,
	"Benjamin Tissoires" <benjamin.tissoires@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] HID: wiiu-drc: Implement touch reports
Date: Wed,  5 May 2021 22:43:55 +0000	[thread overview]
Message-ID: <YJMfq9MbY2guEhpv@latitude> (raw)
In-Reply-To: <20210502232836.26134-3-linkmauve@linkmauve.fr>

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]