Open ardera opened 2 years ago
6by9
commented
2 years ago On first power up I do see spurious events being generated by the controller, but not after the first touch on the screen. Does that match with your observations?
The ts controller is reporting these touches, so it seems to be in a funny state.
ardera
commented
2 years ago It seems like they still ocurr after the first touch, even while I'm touching the screen. Maybe a slightly different display revision? It says Raspberry Pi Display V1.1 on mine
What's more weird is that it seems like the ABS_MT_SLOT events are missing, it just generates two pairs of ABS_MT_POSITION_... after another
6by9
commented
2 years ago AIUI ABS_MT_SLOT is only needed if the slot is changed. Touch with 2 fingers and you'll see slot 0 and slot 1 being selected.
Something very odd is going on with those tracking IDs. The value the driver generates is between 0 and 15 due to the masking at https://github.com/raspberrypi/linux/blob/rpi-5.10.y/drivers/input/touchscreen/edt-ft5x06.c#L271. How evtest is seeing values of 24000 is really strange.
ardera
commented
2 years ago AIUI ABS_MT_SLOT is only needed if the slot is changed. Touch with 2 fingers and you'll see slot 0 and slot 1 being selected.
That's correct. But reporting two ABS_MT_POSITION_X (or Y) events for the same multitouch slot in the same synchronization frame makes no sense:
Event: time 1638281587.294554, -------------- SYN_REPORT ------------
Event: time 1638281588.104155, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 417
Event: time 1638281588.104155, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 221
Event: time 1638281588.104155, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 799
Event: time 1638281588.104155, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 479
Event: time 1638281588.104155, -------------- SYN_REPORT ------------Either there should be a ABS_MT_SLOT event with value 1 after the first ABS_MT_POSITION_Y, or a SYN_REPORT, or something else
Something very odd is going on with those tracking IDs.
could be a red herring, seems like you only really use the id reported by the FT to set the multitouch slot, so ABS_MT_SLOT. I think ABS_MT_TRACKING_ID is automatically generated by the input subsystem. See the last sentence of the event usage section here
EDIT: Also seems like there's a flag you can set when calling input_mt_init_slots that'll make input_mt_sync_frame automatically emit touch up events for any slots that the driver didn't report coordinates for, INPUT_MT_DROP_UNUSED
6by9
commented
2 years ago The panel is producing rubbish initially. I've added logging to the driver, and in one message it's reporting
[ 51.951072] edt_ft5x06_ts_isr Slot 4 reported type 0, 2665, 3242 id 4
[ 51.951088] edt_ft5x06_ts_isr Slot 10 reported type 0, 1220, 650 id 10
[ 51.951102] edt_ft5x06_ts_isr Slot 10 reported type 2, 1084, 232 id 10
[ 51.951118] edt_ft5x06_ts_isr Slot 9 reported type 2, 4063, 3557 id 9
[ 51.951133] edt_ft5x06_ts_isr Slot 5 reported type 2, 3724, 1364 id 5
[ 51.951146] edt_ft5x06_ts_isr Slot 9 reported type 0, 539, 3518 id 9type 0 is event down, and type 2 is event on. The x and y co-ords are outside the display area, it's repeating ID/slot values. (type = TOUCH_EVENT_RESERVED events are ignored, so not logged). That converts to evtest reporting
Event: time 1638288355.364274, -------------- SYN_REPORT ------------
Event: time 1638288355.394532, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 1638288355.394532, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 4
Event: time 1638288355.394532, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -1866
Event: time 1638288355.394532, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value -2763
Event: time 1638288355.394532, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -421
Event: time 1638288355.394532, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value -171
Event: time 1638288355.394532, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -285
Event: time 1638288355.394532, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 247
Event: time 1638288355.394532, type 3 (EV_ABS), code 47 (ABS_MT_SLOT), value 9
Event: time 1638288355.394532, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 1936
Event: time 1638288355.394532, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value -3264
Event: time 1638288355.394532, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value -3078
Event: time 1638288355.394532, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 260
Event: time 1638288355.394532, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value -3039First press for me (V1.0 panel) is stopping that, and it behaves normally from then on.
EDIT: Also seems like there's a flag you can set when calling input_mt_init_slots that'll make input_mt_sync_frame automatically emit touch up events for any slots that the driver didn't report coordinates for, INPUT_MT_DROP_UNUSED
Ooh, magic. That means I can revert https://github.com/raspberrypi/linux/commit/16c756198b3f2c26e39c6c007cc0c2400b9ab33e and make it a one-liner to set that flag.
ardera
commented
2 years ago Ah I think I got it. In the old touch driver they didn't iterate up to the max supported points in this loop: https://github.com/raspberrypi/linux/blob/30bb91977d65c22cec3eaeadb06a6ff7f36ecbc5/drivers/input/touchscreen/edt-ft5x06.c#L253
but to the number of current touch contacts, reported by the FT. Which is a single byte that's reported right before the first touch coordinate (so buf[offset-1])
Ooh, magic. That means I can revert 16c7561 and make it a one-liner to set that flag.
Exactly 😄
6by9
commented
2 years ago Ah I think I got it. In the old touch driver they didn't iterate up to the max supported points in this loop:
Sadly it's not it. I have local changes that do that:
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index f9a55dd5755f..7a8b2d29b023 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -197,6 +197,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
int i, type, x, y, id;
int offset, tplen, datalen, crclen;
int error;
+ unsigned int num_points;
switch (tsdata->version) {
case EDT_M06:
@@ -244,9 +245,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
if (!edt_ft5x06_ts_check_crc(tsdata, rdbuf, datalen))
goto out;
- }
+ num_points = tsdata->max_support_points;
+ } else {
+ /* Register 2 is TD_STATUS, containing the number of touch
+ * points.
+ */
+ num_points = min(rdbuf[2] & 0xf, tsdata->max_support_points);
+ dev_err(dev, "TS reported %u points\n", num_points);
+ }
- for (i = 0; i < tsdata->max_support_points; i++) {
+ for (i = 0; i < num_points; i++) {
u8 *buf = &rdbuf[i * tplen + offset];but on first read the touchscreen is reporting 10 touch events in that register.
6by9
commented
2 years ago Ooh, magic. That means I can revert 16c7561 and make it a one-liner to set that flag.
Exactly 😄
Except it seems not to work - I'm not seeing any release events at all with that. Ignoring it for now as the existing patch does work.
ardera
commented
2 years ago Sadly it's not it. I have local changes that do that:
oh. then I have no idea
Except it seems not to work - I'm not seeing any release events at all with that. Ignoring it for now as the existing patch does work.
true, that makes sense. though just to be sure did you call input_mt_sync_frame at the end of the ISR? i think that call's not there in repo version
6by9
commented
2 years ago true, that makes sense. though just to be sure did you call input_mt_sync_frame at the end of the ISR? i think that call's not there in repo version
No I hadn't changed the input_sync to input_mt_sync_frame.
Need to drop the input_mt_report_pointer_emulation as well, as that is done by input_mt_sync_frame.
One to look at another day though.
I'm trying extending the reset pulse (currently 6ms) to see if that helps, but I'm running out of ideas. I'm still not seeing the garbage results coming back after the first touch though. I am on a Pi3B+, but that shouldn't make any difference.
thijstriemstra
commented
2 years ago upgrade kernel to latest version using rpi-update (because touch needs edt-ft5x06 touchscreen fixes #4736 to be working at all)
I upgraded the kernel:
$ sudo rpi-update
*** Raspberry Pi firmware updater by Hexxeh, enhanced by AndrewS and Dom
*** Performing self-update
*** Relaunching after update
*** Raspberry Pi firmware updater by Hexxeh, enhanced by AndrewS and Dom
*** We're running for the first time
*** Backing up files (this will take a few minutes)
*** Backing up firmware
*** Backing up modules 5.10.63-v7+
#############################################################
WARNING: This update bumps to rpi-5.10.y linux tree
See: https://www.raspberrypi.org/forums/viewtopic.php?f=29&t=288234
'rpi-update' should only be used if there is a specific
reason to do so - for example, a request by a Raspberry Pi
engineer or if you want to help the testing effort
and are comfortable with restoring if there are regressions.
DO NOT use 'rpi-update' as part of a regular update process.
##############################################################
Would you like to proceed? (y/N)
*** Downloading specific firmware revision (this will take a few minutes)
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 173 100 173 0 0 748 0 --:--:-- --:--:-- --:--:-- 748
100 121M 100 121M 0 0 3159k 0 0:00:39 0:00:39 --:--:-- 4761k
*** Updating firmware
*** Updating kernel modules
*** depmod 5.10.81+
*** depmod 5.10.81-v7l+
*** depmod 5.10.81-v7+
*** depmod 5.10.81-v8+
*** Updating VideoCore libraries
*** Using HardFP libraries
*** Updating SDK
*** Running ldconfig
*** Storing current firmware revision
*** Deleting downloaded files
*** Syncing changes to disk
*** If no errors appeared, your firmware was successfully updated to 8a74f2a62979db8793ae7d5a1b6d8f29467e1c27
*** A reboot is needed to activate the new firmwareBut this didn't fix touch unfortunately. It does a right-click, garbled something.
type 0 is event down, and type 2 is event on. The x and y co-ords are outside the display area, it's repeating ID/slot values. (type = TOUCH_EVENT_RESERVED events are ignored, so not logged).
Seeing similar crap with Raspberry pi 3b+ evtest output:
Event: time 1638311693.056529, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1638311693.056529, -------------- SYN_REPORT ------------
Event: time 1638311693.145183, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 1638311693.145183, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
Event: time 1638311693.145183, -------------- SYN_REPORT ------------
Event: time 1638311693.176296, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 188
Event: time 1638311693.176296, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1638311693.176296, -------------- SYN_REPORT ------------
Event: time 1638311693.775214, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 1638311693.775214, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
Event: time 1638311693.775214, -------------- SYN_REPORT ------------
Event: time 1638311693.806120, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 189
Event: time 1638311693.806120, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1638311693.806120, -------------- SYN_REPORT ------------
Event: time 1638311694.135243, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value -3616
Event: time 1638311694.135243, type 3 (EV_ABS), code 1 (ABS_Y), value -3616
Event: time 1638311694.135243, -------------- SYN_REPORT ------------
Event: time 1638311694.165307, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 479
Event: time 1638311694.165307, type 3 (EV_ABS), code 1 (ABS_Y), value 479
Event: time 1638311694.165307, -------------- SYN_REPORT ------------
Event: time 1638311694.345218, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 1638311694.345218, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
Event: time 1638311694.345218, -------------- SYN_REPORT ------------
Event: time 1638311694.375309, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 190
Event: time 1638311694.375309, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1638311694.375309, -------------- SYN_REPORT ------------
Event: time 1638311694.946114, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 1638311694.946114, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
Event: time 1638311694.946114, -------------- SYN_REPORT ------------
Event: time 1638311694.975306, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 191
Event: time 1638311694.975306, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 1638311694.975306, -------------- SYN_REPORT ------------Also had to add dtparam=i2c_vc_baudrate=50000 to /boot/config.txt
I'm disappointed this official touchscreen is so broken on a official OS. You would expect it to be tested at least once by someone...
Update it seems enabling the dtoverlay=vc4-fkms-v3d overlay in /boot/config.txt fixes the display..
#dtoverlay=vc4-kms-v3d
dtoverlay=vc4-fkms-v3dSee https://forums.raspberrypi.com/viewtopic.php?p=1935740#p1935740
ardera
commented
2 years ago @6by9 do you have the firmware sources so you can look at what the closed-source driver did differently? or is there no obvious deviation there?
I have a i2c sniffer that i used for debugging the last two times the DSI panel was buggy (😉), I'll use that and see if I find something
ardera
commented
2 years ago I don't know what changed but I can't reproduce it anymore.
Describe the bug
To reproduce
rpi-update(because touch needs https://github.com/raspberrypi/linux/pull/4736 to be working at all)evtest(sudo apt install evtest) and select thegeneric ft5x06deviceExpected behaviour
Actual behaviour
evtestoutput)System
raspinfo.txtLogs output of
evtest