onitake / gsl-firmware

Firmware repository for Silead touchscreen controllers
323 stars 200 forks source link

Add Positivo C4128B #217

Closed ghost closed 12 months ago

ghost commented 12 months ago

Seems to use the same panel as the Positivo C464C, although I tweaked the values a bit after running evtest.

One major annoyance is that it seems impossible to be able to swipe into the screen from the top or the left borders, since the device does not send a zero position from either axis. If there is a way to solve this, please let me know :-p

I also tried patching the kernel to use the mainline silead module, and it works fine, so I may try sending the patch in the future.

onitake commented 12 months ago

The Linux touchscreen driver supports min and max properties: https://elixir.bootlin.com/linux/v6.5.5/source/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml Would that help?

(Note: requires kernel 5.x or later. 4.x had different properties)

onitake commented 12 months ago

Is this PR ready for merge, or should I wait until you've submitted the kernel patch?

jwrdegoede commented 12 months ago

I also tried patching the kernel to use the mainline silead module, and it works fine, so I may try sending the patch in the future.

Upstream maintainer of drivers/platform/x86/touchscreen_dmi.c here. As @onitake mentioned with the mainline kernel driver you can fix the not being able to swipe from the top / left side issue by setting min x/y properties, see e.g. : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/platform/x86/touchscreen_dmi.c#n99

Once you have this working please do submit your touchscreen_dmi.c changes upstream. I know the whole upstream patch submission process can be a bit daunting if that is a problem you can also just send me your modified touchscreen_dmi.c at hdegoede@redhat.com and then I can turn it into a patch and submit it myself.

I really want this to work out of the box as much as possible for users, which requires a touchscreen_dmi.c entry. Unfortunately we cannot add the firmwares to linux-firmware proper so users still need to manually download the fw file and drop it into /lib/firmware/silead .

jwrdegoede commented 12 months ago

p.s. you can find the min values just like how you find the max values, run evtest and see what the minimum reported values are when you (repeatedly) swipe over the edges.

ghost commented 12 months ago

Thanks for the tips, everyone! I will try tweaking the touchscreen-min-x/y values, then I'll submit the kernel patch as soon as my spare time allows.

Is this PR ready for merge, or should I wait until you've submitted the kernel patch?

I believe it is ready for merging, I just did a small update to the commit to adjust the firmware file name for the silead driver. The silead_ts.fw is also working with the gslx680-acpi driver, so the files should already be useful for someone :-p

ghost commented 12 months ago

For completeness' sake, here's the kernel patch as it currently stands:

--- a/drivers/platform/x86/touchscreen_dmi.c.orig   2023-10-03 16:36:24.190623912 -0400
+++ b/drivers/platform/x86/touchscreen_dmi.c    2023-10-03 16:33:00.842254721 -0400
@@ -756,6 +756,19 @@
    .properties = pipo_w11_props,
 };

+static const struct property_entry positivo_c4128b_props[] = {
+   PROPERTY_ENTRY_U32("touchscreen-size-x", 1920),
+   PROPERTY_ENTRY_U32("touchscreen-size-y", 1270),
+   PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-positivo_c4128b.fw"),
+   PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+   { }
+};
+
+static const struct ts_dmi_data positivo_c4128b_data = {
+   .acpi_name  = "MSSL1680:00",
+   .properties = positivo_c4128b_props,
+};
+
 static const struct property_entry pov_mobii_wintab_p800w_v20_props[] = {
    PROPERTY_ENTRY_U32("touchscreen-min-x", 32),
    PROPERTY_ENTRY_U32("touchscreen-min-y", 16),
@@ -1481,6 +1494,14 @@
        },
    },
    {
+       /* Positivo C4128B */
+       .driver_data = (void *)&positivo_c4128b_data,
+       .matches = {
+           DMI_MATCH(DMI_SYS_VENDOR, "Positivo Tecnologia SA"),
+           DMI_MATCH(DMI_PRODUCT_NAME, "C4128B-1"),
+       },
+   },
+   {
        /* Point of View mobii wintab p800w (v2.0) */
        .driver_data = (void *)&pov_mobii_wintab_p800w_v20_data,
        .matches = {

My only concern is that a quick online search revealed that there is also a C4128B-3 model available. I would like to believe that it uses the same panel, but I'm not sure. Is it OK to match only to a C4128B-1?

ghost commented 12 months ago

I've sent the patch to the kernel: https://lore.kernel.org/linux-input/20231004235900.426240-1-japareaggae@gmail.com/

I've decided to only match to the C4128B-1, since I'm unsure about the panel on the -3 variant.

onitake commented 12 months ago

All right!

To complete the PR, could you please add these things?

As for the patch, it should normally also be sent to the platform-driver-x86 list, but I think @jwrdegoede can pick it up for now.

ghost commented 12 months ago

I've just pushed an update with the changes you've requested!

onitake commented 12 months ago

Perfect, thank you very much!