Skip to contentSkip to conversation

Add generic driver for Silead tochscreens

PATCH Gregor Riepl 3 participants 11 messages Jul 11, 2015 – Jul 21, 2015 HID

lore
Share & tools
lore
#0 lore raw

Hello.

You beat me to it - I noticed the lack of a generic Silead driver that works with Baytrail tablets (and ACPI) and started writing one myself, adapting the code by Joe Burmeister (https://github.com/jabjoe/sunxi-gslx680). You can find my endeavors (so far) here: https://github.com/onitake/gslx680-acpi

While writing the driver, I noticed that my controller or the firmware (which I got from the Windows 8 driver for the tablet) does not seem to support finger tracking. If I look at the data packets, almost all of them carry 0 as the finger ID, while some carry 4. Lifting a finger simply removes the record, without providing a way to correlate it with the next data set.

The code I worked with seems to support some kind of finger tracking, but it still uses the ID field of each touch record as finger ID. And that does not work with my touchscreen.

In the meantime, I just implemented type A multitouch support, leaving the finger tracking out.

Can you help me look into the problem?

#1 lore raw
Quoted 13 lines · Gregor Riepl

Hello.

You beat me to it - I noticed the lack of a generic Silead driver that works with Baytrail tablets (and ACPI) and started writing one myself, adapting the code by Joe Burmeister (https://github.com/jabjoe/sunxi-gslx680). You can find my endeavors (so far) here: https://github.com/onitake/gslx680-acpi

While writing the driver, I noticed that my controller or the firmware (which I got from the Windows 8 driver for the tablet) does not seem to support finger tracking. If I look at the data packets, almost all of them carry 0 as the finger ID, while some carry 4. Lifting a finger simply removes the record, without providing a way to correlate it with the next data set.

Quoted 8 lines

The code I worked with seems to support some kind of finger tracking, but it still uses the ID field of each touch record as finger ID. And that does not work with my touchscreen.

In the meantime, I just implemented type A multitouch support, leaving the finger tracking out.

Can you help me look into the problem?

Input core does have facilities to perform contact tracking, see input_mt_assign_slots().

Thanks.

Signature · 1 line
Dmitry
#2 lore raw
In reply to Gregor Riepl
Quoted 13 lines · Gregor Riepl

Hello.

You beat me to it - I noticed the lack of a generic Silead driver that works with Baytrail tablets (and ACPI) and started writing one myself, adapting the code by Joe Burmeister (https://github.com/jabjoe/sunxi-gslx680). You can find my endeavors (so far) here: https://github.com/onitake/gslx680-acpi

While writing the driver, I noticed that my controller or the firmware (which I got from the Windows 8 driver for the tablet) does not seem to support finger tracking. If I look at the data packets, almost all of them carry 0 as the finger ID, while some carry 4. Lifting a finger simply removes the record, without providing a way to correlate it with the next data set.

Quoted 8 lines

The code I worked with seems to support some kind of finger tracking, but it still uses the ID field of each touch record as finger ID. And that does not work with my touchscreen.

In the meantime, I just implemented type A multitouch support, leaving the finger tracking out.

Can you help me look into the problem?

The firmware has to be calibrated by the vendor for each chip-panel combination. If you change the panel even if you keep the same chip it is very likely to need some calibration. This might be the reason why the finger ID is not working properly.

#3 lore raw
Quoted 4 lines

The firmware has to be calibrated by the vendor for each chip-panel combination. If you change the panel even if you keep the same chip it is very likely to need some calibration. This might be the reason why the finger ID is not working properly.

I see.

So, while your new driver is sufficiently generic, it would still need a device specific firmware image and there is no way to work around that?

That's kind of a bummer.

Would it be possible to modify the driver in a way that it would work with different types of panels/chips, even those where finger tracking does not work correctly?

Also, I see one particular problem concerning existing hardware. You're retrieving the dimensions, maximum number of trackable fingers and the name of the firmware image from the DSDT or DTS using device_property_read_*. The Baytrail tablet I'm testing on does not include such information, and injecting a modified DSDT is maybe not the best option. Also, the DSDT shipped I have here does not use GPIO definitions for the control pins, but instead executes the shutdown/wakeup sequences through _PS3/_PS0 methods. At least that was my assumption when I looked at it. There are GPIO definitions as well, but it's not clear what those are used for. Please take a look at the stanza yourself, if you can:
https://github.com/onitake/gslx680-acpi/blob/master/acpi/gsl-dsdt.aml
I can also send you the complete decompiled DSDT if it helps.

#4 lore raw
In reply to Robert Dolca
Quoted 4 lines

The firmware has to be calibrated by the vendor for each chip-panel combination. If you change the panel even if you keep the same chip it is very likely to need some calibration. This might be the reason why the finger ID is not working properly.

Thinking about the device parameters... Wouldn't it be possible to read them from the Silead controller? According to comments in the firmware shipped with the old Android platform driver by Basewin, all the configurable parameters are contained in the firmware.[1]

I don't have a complete data sheet, but maybe we could just read back the panel width and height and number of tracking points from a hardware register?

That would not solve the problem of transferring the correct, platform specific firmware image to the device, however.

[1]
https://code.google.com/p/yuandao-n90-window-dual-core-2/source/browse/drivers/input/touchscreen/gslx680_ts.h
- the comments are in Chinese, a translation can be found here:
https://github.com/jabjoe/sunxi-gslx680/blob/master/firmware/firmware_information.txt

#5 lore raw
Quoted 10 lines · Gregor Riepl
Quoted 4 lines

The firmware has to be calibrated by the vendor for each chip-panel combination. If you change the panel even if you keep the same chip it is very likely to need some calibration. This might be the reason why the finger ID is not working properly.

Thinking about the device parameters... Wouldn't it be possible to read them from the Silead controller? According to comments in the firmware shipped with the old Android platform driver by Basewin, all the configurable parameters are contained in the firmware.[1]

Quoted 5 lines

I don't have a complete data sheet, but maybe we could just read back the panel width and height and number of tracking points from a hardware register?

That would not solve the problem of transferring the correct, platform specific firmware image to the device, however.

In the datasheet there is no register map. Silead gave us a partial register map in order to implement the driver. In the map they provided there is no register for resolution or anything that would help with this issue. The information in [1] is incomplete and can not be considered reliable since the vendor does not say anything about it.

Regards,
Robert

Quoted 6 lines

[1]
https://code.google.com/p/yuandao-n90-window-dual-core-2/source/browse/drivers/input/touchscreen/gslx680_ts.h
- the comments are in Chinese, a translation can be found here:
https://github.com/jabjoe/sunxi-gslx680/blob/master/firmware/firmware_information.txt

#6 lore raw
Quoted 6 lines

In the datasheet there is no register map. Silead gave us a partial register map in order to implement the driver. In the map they provided there is no register for resolution or anything that would help with this issue. The information in [1] is incomplete and can not be considered reliable since the vendor does not say anything about it.

That is indeed true.
Relying on parameters read from the ACPI DSDT or the DeviceTree would certainly be the "clean" way to go on new hardware, but it will not work on existing hardware where the device parameters are fixed.

Do you think it would make sense to also allow passing the parameters as module options, or via sysfs for cases where they can't be autodetected?

Also, while going over your code, I found something that might be incorrect. When you evaluate the point data from the device, you are interpreting it with this code:

#define SILEAD_POINT_DATA_LEN 0x04
#define SILEAD_POINT_X_OFF 0x02
#define SILEAD_POINT_ID_OFF 0x03
#define SILEAD_X_HSB_MASK 0xF0
#define SILEAD_TOUCH_ID_MASK 0x0F

for (i = 1; i <= touch_nr; i++) {
	offset = i * SILEAD_POINT_DATA_LEN;

	/* The last 4 bits are the touch id */
	id = buf[offset + SILEAD_POINT_ID_OFF] & SILEAD_TOUCH_ID_MASK;

	/* The 1st 4 bits are part of X */
	buf[offset + SILEAD_POINT_ID_OFF] =
		(buf[offset + SILEAD_POINT_ID_OFF] & SILEAD_X_HSB_MASK)
		>> 4;

	y = le16_to_cpup((__le16 *)(buf + offset));
	x = le16_to_cpup((__le16 *)(buf + offset + SILEAD_POINT_X_OFF));
}

The Basewin driver and all the ones derived from it interpreted it like this:

ts->dd = {
	.x_index = 6,
	.y_index = 4,
	.z_index = 5,
	.id_index = 7,
}
for(i= 0;i < (touches > MAX_FINGERS ? MAX_FINGERS : touches);i ++) {
	x = join_bytes( ( ts->touch_data[ts->dd->x_index + 4 * i + 1] & 0xf),
	ts->touch_data[ts->dd->x_index + 4 * i]);
	y = join_bytes(ts->touch_data[ts->dd->y_index + 4 * i + 1],
	ts->touch_data[ts->dd->y_index + 4 * i ]);
	id = ts->touch_data[ts->dd->id_index + 4 * i] >> 4;
}

Or, in other words, your code uses the second-to-last nibble of each record as the touch id, while the Basewin code uses the last nibble (in little-endian order).
Please correct me if I am wrong.

#7 lore raw

Please excuse me if I'm being to brash. I'd love to see complete Silead touchscreen controller support in the mainline kernel, but with the hardware being what it is and the lack of complete public documentation, I understand this is a bit difficult.

Quoted 10 lines

/* The last 4 bits are the touch id */
id = buf[offset + SILEAD_POINT_ID_OFF] & SILEAD_TOUCH_ID_MASK;

/* The 1st 4 bits are part of X */
buf[offset + SILEAD_POINT_ID_OFF] =

	(buf[offset + SILEAD_POINT_ID_OFF] & SILEAD_X_HSB_MASK)
	>> 4;

y = le16_to_cpup((__le16 *)(buf + offset));
x = le16_to_cpup((__le16 *)(buf + offset + SILEAD_POINT_X_OFF));

Can you confirm that your data sheet/hardware does in fact use the lower 4 bits of byte 3 of each point record as the touch id, and not the upper 4 bits?

I just checked, both the Basewin driver (and all drivers derived from it) and my testing hardware attribute it to the high nibble. The lower nibble is part of the x coordinate.

Aside from this, would you be interested in making your driver compatible with legacy/"broken" hardware? I'd start working on a patch if this is ok to you.

This would encompass the following:
- Adding fallbacks for unavailable information (width, height, number of touch points, finger tracking availability, axis swapping and mirroring, firmware name), maybe keeping the device uninitialized until all parameters are set
- Adding a sysfs interface and/or module options to set these attributes, or possibly adding platform quirks that autoconfigurate depending on DMI data (better ideas are welcome)
- Adding code paths for the missing functionality (finger tracking, swapping/mirroring, other quirks)

#8 lore raw
Quoted 23 lines · Gregor Riepl

Please excuse me if I'm being to brash. I'd love to see complete Silead touchscreen controller support in the mainline kernel, but with the hardware being what it is and the lack of complete public documentation, I understand this is a bit difficult.

Quoted 10 lines

/* The last 4 bits are the touch id */
id = buf[offset + SILEAD_POINT_ID_OFF] & SILEAD_TOUCH_ID_MASK;

/* The 1st 4 bits are part of X */
buf[offset + SILEAD_POINT_ID_OFF] =
(buf[offset + SILEAD_POINT_ID_OFF] & SILEAD_X_HSB_MASK) >> 4;

y = le16_to_cpup((__le16 *)(buf + offset));
x = le16_to_cpup((__le16 *)(buf + offset + SILEAD_POINT_X_OFF));

Can you confirm that your data sheet/hardware does in fact use the lower 4 bits of byte 3 of each point record as the touch id, and not the upper 4 bits?

I just checked, both the Basewin driver (and all drivers derived from it) and my testing hardware attribute it to the high nibble. The lower nibble is part of the x coordinate.

Quoted 7 lines

Aside from this, would you be interested in making your driver compatible with legacy/"broken" hardware? I'd start working on a patch if this is ok to you.

This would encompass the following:
- Adding fallbacks for unavailable information (width, height, number of touch points, finger tracking availability, axis swapping and mirroring, firmware name), maybe keeping the device uninitialized until all parameters are set

axis swapping, mirroring sound like a good addon.
there is currently support for width, height, number of touch points and firmware name

Quoted 3 lines

- Adding a sysfs interface and/or module options to set these attributes, or possibly adding platform quirks that autoconfigurate depending on DMI data (better ideas are welcome)

IMHO it's not a good idea to do the config using sysfs.

Quoted 2 lines

- Adding code paths for the missing functionality (finger tracking, swapping/mirroring, other quirks)

I am not sure I think Dmitry could answer this one.

#9 lore raw
In reply to Gregor Riepl

Hi Gregor,

Quoted 7 lines · Gregor Riepl
Quoted 6 lines

In the datasheet there is no register map. Silead gave us a partial register map in order to implement the driver. In the map they provided there is no register for resolution or anything that would help with this issue. The information in [1] is incomplete and can not be considered reliable since the vendor does not say anything about it.

Quoted 7 lines

That is indeed true.
Relying on parameters read from the ACPI DSDT or the DeviceTree would certainly be the "clean" way to go on new hardware, but it will not work on existing hardware where the device parameters are fixed.

Do you think it would make sense to also allow passing the parameters as module options, or via sysfs for cases where they can't be autodetected?

For the existing devices you can override the ACPI tables using initrd.
https://www.kernel.org/doc/Documentation/acpi/initrd_table_override.txt

There is some work being done that will allow to amend (no need to completely override) the ACPI tables with custom data.
https://lkml.org/lkml/2015/6/18/947

Device tree already has support for overlays.

Giving this I think that we should keep the driver as simple as possible.

Quoted 41 lines

Also, while going over your code, I found something that might be incorrect. When you evaluate the point data from the device, you are interpreting it with this code:

#define SILEAD_POINT_DATA_LEN   0x04
#define SILEAD_POINT_X_OFF      0x02
#define SILEAD_POINT_ID_OFF     0x03
#define SILEAD_X_HSB_MASK       0xF0
#define SILEAD_TOUCH_ID_MASK    0x0F
for (i = 1; i <= touch_nr; i++) {
        offset = i * SILEAD_POINT_DATA_LEN;

        /* The last 4 bits are the touch id */
        id = buf[offset + SILEAD_POINT_ID_OFF] & SILEAD_TOUCH_ID_MASK;

        /* The 1st 4 bits are part of X */
        buf[offset + SILEAD_POINT_ID_OFF] =
                (buf[offset + SILEAD_POINT_ID_OFF] & SILEAD_X_HSB_MASK)
                >> 4;

        y = le16_to_cpup((__le16 *)(buf + offset));
        x = le16_to_cpup((__le16 *)(buf + offset + SILEAD_POINT_X_OFF));
}

The Basewin driver and all the ones derived from it interpreted it like this:

ts->dd = {
        .x_index = 6,
        .y_index = 4,
        .z_index = 5,
        .id_index = 7,
}
for(i= 0;i < (touches > MAX_FINGERS ? MAX_FINGERS : touches);i ++) {
        x = join_bytes( ( ts->touch_data[ts->dd->x_index + 4 * i + 1] & 0xf),
        ts->touch_data[ts->dd->x_index + 4 * i]);
        y = join_bytes(ts->touch_data[ts->dd->y_index + 4 * i + 1],
        ts->touch_data[ts->dd->y_index + 4 * i ]);
        id = ts->touch_data[ts->dd->id_index + 4 * i] >> 4;
}
Quoted 4 lines

Or, in other words, your code uses the second-to-last nibble of each record as the touch id, while the Basewin code uses the last nibble (in little-endian order).
Please correct me if I am wrong.

I will come back with the info on the touch id after I check the docs.

#10 lore raw

Hi Robert

Quoted 6 lines

For the existing devices you can override the ACPI tables using initrd.
https://www.kernel.org/doc/Documentation/acpi/initrd_table_override.txt

There is some work being done that will allow to amend (no need to completely override) the ACPI tables with custom data.
https://lkml.org/lkml/2015/6/18/947

I see.
I was under the impression that modifying the DSDT is generally frowned upon and will result in a tainted kernel.

Quoted 3 lines

Device tree already has support for overlays.

Giving this I think that we should keep the driver as simple as possible.

I absolutely agree. However, as long as there is no alternative way to pass the necessary parameters, a driver will not work on all machines, particularly those that were designed to be shipped with Microsoft Windows. I suspect that vendors will be reluctant to change their (UEFI) BIOSes to accommodate full Linux support in such devices.

Quoted 1 line

I will come back with the info on the touch id after I check the docs.

Thanks.

Quoted 3 lines

axis swapping, mirroring sound like a good addon.
there is currently support for width, height, number of touch points and firmware name

Those features are fairly easy to implement, and thanks to the great work by the linux-input team, even finger tracking is not difficult to integrate.

Some example code is included below.

Quoted 1 line

IMHO it's not a good idea to do the config using sysfs.

You are probably right, yes.

In the meantime, I had a different idea. Instead of passing the panel and firmware parameters (width, height, number of touch areas, mirroring, swapping, tracking support) through device_property_read, they could be contained in the firmware, in a special header.
As practically every single device seems to require its own firmware, users and platform builders would need to obtain it anyway. Why not include all the information the driver requires with the firmware image itself?

Reading the firmware name from the DSDT/DT like all the other parameters is an option, of course. However... If you have, let's say, 100 different devices on the market, all with different firmware, you'd have to install 100 different firmware images to make automatic detection and device startup possible. This doesn't seem very reasonable. On the other hand, just logging an error that a specific firmware is required and requiring the user to manually download and install that firmware isn't much better than logging vendor/device data from the DMI, then provide a tool or website that provides the appropriate firmware for that device.

Another obvious approach would be device quirks. Many drivers in the Linux kernel follow this approach, but I'm not sure if it's a preferable solution. Again, assuming there are a 100 different devices with a limited DSDT out there, one would need 100 different device quirks.

That number is, obviously, purely hypothetical. But I hope you get the picture.

Code example: Swapping, mirroring and finger tracking support (error checking omitted for simplicity):

/* 0 = unlimited distance */
#define MAX_DISTANCE 0
#define MAX_CONTACTS 10

struct gsl_ts_data {
	struct input_dev *input;
	/* ... */
	unsigned int x_max;
	unsigned int y_max;
	unsigned int multi_touches;
	bool x_reversed;
	bool y_reversed;
	bool xy_swapped;
	bool soft_tracking;
};
static int gsl_ts_probe(struct i2c_client *client, const struct i2c_device_id *id)
{
	struct gsl_ts_data *ts;
	/* ... */
	ts = devm_kzalloc(&client->dev, sizeof(struct gsl_ts_data), GFP_KERNEL);
	ts->input = devm_input_allocate_device(&client->dev);
	if (ts->soft_tracking) {
		input_mt_init_slots(ts->input, ts->multi_touches, INPUT_MT_DIRECT |
INPUT_MT_DROP_UNUSED | INPUT_MT_TRACK);
	} else {
		input_mt_init_slots(ts->input, ts->multi_touches, INPUT_MT_DIRECT |
INPUT_MT_DROP_UNUSED);
	}
	/* ... */
}

static void gsl_ts_mt_event(struct gsl_ts_data *ts, u8 *buf)

{
	size_t i;
	struct input_mt_pos positions[MAX_CONTACTS];
	int slots[MAX_CONTACTS];
	/* ... */
	if (num_touches > MAX_CONTACTS) {
		num_touches = MAX_CONTACTS;
	}
	for (i = 0; i < num_touches; i++) {
		/* ... */
		if (ts->xy_swapped) {
			swap(x, y);
		}
		if (ts->x_reversed) {
			x = ts->x_max - x;
		}
		if (ts->y_reversed) {
			y = ts->y_max - y;
		}
		positions[i].x = x;
		positions[i].y = y;
		if (!ts->soft_tracking) {
			slots[i] = id;
		}
	}
	if (ts->soft_tracking) {
#if LINUX_VERSION_CODE < KERNEL_VERSION(4,0,0)
		rc = input_mt_assign_slots(input, slots, positions, touches);
#else
		rc = input_mt_assign_slots(input, slots, positions, touches, MAX_DISTANCE);
#endif
		if (rc < 0) {
			dev_err(dev, "%s: input_mt_assign_slots returned %d\n", __func__, rc);
			return;
		}
	}
	for (i = 0; i < num_touches; i++) {
		input_mt_slot(input, slots[i]);
		input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
		input_report_abs(input, ABS_MT_POSITION_X, positions[i].x);
		input_report_abs(input, ABS_MT_POSITION_Y, positions[i].y);
	}
	input_mt_sync_frame(input);
	input_sync(input);
}