Hi,
On 12/24/20 3:02 PM, Christopher Piggott wrote:
> Hans, I have the almost the exact same problem with a Xenarc 705T that has an eGalax controller on it. In this case, it's not just a swap of X and Y, it's actually rotated 90 degrees. I have been searching for a more generic solution to this problem. I'm wondering if there's a more generic way to do this, up in maybe hid-input or somewhere, and I'm also wondering if it's at all feasible to do it so that you don't specify swapxy, rotation, and size transformations. In my mind this calls for an affine transform, where if somebody really needs to manipulate this they specify a matrix. I can see wanting to have parameters like you added to make it easier for people, but I would just make those parameters formulate an affine transformation matrix that could also be specified explicitly as x1, x2, y1, y1.
>
> At one point in time, the eGalax driver handled this with a swapxy. Somebody somewhere along the line thought that the eGalax driver could be discarded in lieu of standard handling in hid, but without the ability to transform that's not the case. In my particular case, I'm actually doing this for android, where these drivers aren't even loaded as modules (so it's kind of a pain).
>
> What do you think?
Any transformation which you need because of the touchscreen and LCD panel orientation
not matching can be achieved through the swap-x-y and invert-x and invert-y device properties
defined in:
Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
Note that swap-x-y will also swap the width/height and min/max properties for the
touchscreen. So you specify the min and max values as reported by the controller and
since the touchscreen is mounted 90 degrees rotated those will then be swapped
before being reported to userspace.
This is all handled by the generic touchscreen-properties helpers from:
drivers/input/touchscreen/of_touchscreen.c
(touchscreen_parse_properties, touchscreen_report_pos and others)
You can find plenty examples of drivers using those.
The proper solution for this would be to convert the driver which you are
using to use these helpers, see e.g. commit fafef982c735 ("Input: goodix -
use generic touchscreen_properties"), in combination with setting the
properties needed for your device in the devicetree of your device.
And yes if you need this with a standard HID device then the
drivers/hid/hid-multitouch.c properly needs to get support for the
helpers from drivers/input/touchscreen/of_touchscreen.c . Which is not
entirely trivial, but should be not that hard to do.
Regards,
Hans
>
>
>
> On Thu, Dec 24, 2020 at 8:55 AM Hans de Goede <hdegoede@xxxxxxxxxx <mailto:hdegoede@xxxxxxxxxx>> wrote:
>
> The Estar Beauty HD (MID 7316R) tablet uses a Goodix touchscreen,
> with the X and Y coordinates swapped compared to the LCD panel.
>
> Add a touchscreen_dmi entry for this adding a "touchscreen-swapped-x-y"
> device-property to the i2c-client instantiated for this device before
> the driver binds.
>
> This is the first entry of a Goodix touchscreen to touchscreen_dmi.c,
> so far DMI quirks for Goodix touchscreen's have been added directly
> to drivers/input/touchscreen/goodix.c. Currently there are 3
> DMI tables in goodix.c:
> 1. rotated_screen[] for devices where the touchscreen is rotated
> 180 degrees vs the LCD panel
> 2. inverted_x_screen[] for devices where the X axis is inverted
> 3. nine_bytes_report[] for devices which use a non standard touch
> report size
>
> Arguably only 3. really needs to be inside the driver and the other
> 2 cases are better handled through the generic touchscreen DMI quirk
> mechanism from touchscreen_dmi.c, which allows adding device-props to
> any i2c-client. Esp. now that goodix.c is using the generic
> touchscreen_properties code.
>
> Alternative to the approach from this patch we could add a 4th
> dmi_system_id table for devices with swapped-x-y axis to goodix.c,
> but that seems undesirable.
>
> Cc: Bastien Nocera <hadess@xxxxxxxxxx <mailto:hadess@xxxxxxxxxx>>
> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx <mailto:dmitry.torokhov@xxxxxxxxx>>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx <mailto:hdegoede@xxxxxxxxxx>>
> ---
> drivers/platform/x86/touchscreen_dmi.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
> index 5783139d0a11..c4de932302d6 100644
> --- a/drivers/platform/x86/touchscreen_dmi.c
> +++ b/drivers/platform/x86/touchscreen_dmi.c
> @@ -263,6 +263,16 @@ static const struct ts_dmi_data digma_citi_e200_data = {
> .properties = digma_citi_e200_props,
> };
>
> +static const struct property_entry estar_beauty_hd_props[] = {
> + PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
> + { }
> +};
> +
> +static const struct ts_dmi_data estar_beauty_hd_data = {
> + .acpi_name = "GDIX1001:00",
> + .properties = estar_beauty_hd_props,
> +};
> +
> static const struct property_entry gp_electronic_t701_props[] = {
> PROPERTY_ENTRY_U32("touchscreen-size-x", 960),
> PROPERTY_ENTRY_U32("touchscreen-size-y", 640),
> @@ -942,6 +952,14 @@ const struct dmi_system_id touchscreen_dmi_table[] = {
> DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"),
> },
> },
> + {
> + /* Estar Beauty HD (MID 7316R) */
> + .driver_data = (void *)&estar_beauty_hd_data,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Estar"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "eSTAR BEAUTY HD Intel Quad core"),
> + },
> + },
> {
> /* GP-electronic T701 */
> .driver_data = (void *)&gp_electronic_t701_data,
> --
> 2.28.0
>
