Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened [v2]
On Sun, 13 Dec 2020 16:21:52 GMT, Johan Vos wrote: >> John Neffenger has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains two additional >> commits since the last revision: >> >> - Merge branch 'master' into zforce-command-overrun >> - 8201568: zForce touchscreen input device fails when closed and >> immediately reopened > > Marked as reviewed by jvos (Reviewer). It seems there was an additional merge commit, which didn't change your code, but it triggered a re-approval request that I missed. I re-approved it, so this should be ready to integrate. - PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened [v2]
On Fri, 25 Sep 2020 22:21:36 GMT, John Neffenger wrote: >> Fixes [JDK-8201568](https://bugs.openjdk.java.net/browse/JDK-8201568). > > John Neffenger has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains two additional > commits since the last revision: > > - Merge branch 'master' into zforce-command-overrun > - 8201568: zForce touchscreen input device fails when closed and immediately > reopened Marked as reviewed by jvos (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened [v2]
On Tue, 29 Sep 2020 19:47:54 GMT, John Neffenger
wrote:
>> I don't have access to a Pi right now, so I can't test this (I'll be able to
>> test in about 10 days from now though)
>
> @johanvos, would you mind approving this pull request again? It looks as if
> the *ready* label was removed when I merged the master branch. All of the
> tests I ran this week worked as expected and are described below.
>
> ### Tests
>
> I ran additional tests on the following devices after merging the master
> branch:
>
> * Kobo Touch Model N905C,
> * Kobo Glo HD Model N437, and
> * Raspberry Pi 2 Model B.
>
> I ran the tests:
>
> * without this fix and without the workaround,
> * with this fix but without the workaround,
> * with this fix and with the workaround (representing this pull request).
>
> The workaround is the code in `EPDInputDeviceRegistry.createDevice` that lets
> the current JavaFX 15 release avoid the problem even without this fix. The
> goal of this pull request is to be able eventually to remove the
> `EPDInputDeviceRegistry` subclass entirely.
>
> The test results are shown below. The system call trace was captured with the
> following command using the [epd-javafx][1] application.
>
> $ sudo strace -f -e trace=open,openat,close -o strace.log \
> bin/run.sh --pattern=3 --loops=1
>
> **Note:** In the messages below from the kernel ring buffer, the final two
> lines with `zforce_i2c_close` and `Deactivate` are printed when the program
> terminates. That call made by the kernel on behalf of the program is not
> recorded in the program's system call trace.
>
> ### Kobo Touch Model N905C
>
> This device has the "command overrun" bug.
>
> Without fix, without workaround
>
> The system call trace shows:
>
> 3576 open("/dev/input/event1", O_RDONLY) = 12
> 3576 close(12) = 0
> 3576 open("/dev/input/event1", O_RDONLY) = 12
>
> The error occurs, and the touchscreen is disabled:
>
> $ dmesg | grep -i zforce
> [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open()
> [zForce_ir_touch_recv_data-145] command Activate (0) ...
> [zForce_ir_touch_recv_data-154] command Resolution (0) ...
> [zForce_ir_touch_recv_data-179] command Frequency (0) ...
> [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close()
> [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open()
> [zForce_ir_touch_recv_data-142] command Deactivate ...
> [zForce_ir_touch_recv_data-198] command overrun (8) ...
> [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close()
> [zForce_ir_touch_recv_data-142] command Deactivate ...
>
> With fix, without workaround
>
> The system call trace shows:
>
> 3997 open("/dev/input/event1", O_RDONLY) = 12
> 3997 open("/dev/input/event1", O_RDONLY) = 13
> 3997 close(13) = 0
>
> The error does not occur, and the touchscreen works as expected:
>
> $ dmesg | grep -i zforce
> [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open()
> [zForce_ir_touch_recv_data-145] command Activate (0) ...
> [zForce_ir_touch_recv_data-154] command Resolution (0) ...
> [zForce_ir_touch_recv_data-179] command Frequency (0) ...
> [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close()
> [zForce_ir_touch_recv_data-142] command Deactivate ...
>
> With fix, with workaround
>
> The system call trace shows the additional *open* call due to the workaround:
>
> 4123 open("/dev/input/event1", O_RDONLY) = 13
> 4123 open("/dev/input/event1", O_RDONLY) = 14
> 4123 open("/dev/input/event1", O_RDONLY) = 15
> 4123 close(15) = 0
>
> The error does not occur, and the touchscreen works as expected:
>
> $ dmesg | grep -i zforce
> [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open()
> [zForce_ir_touch_recv_data-145] command Activate (0) ...
> [zForce_ir_touch_recv_data-154] command Resolution (0) ...
> [zForce_ir_touch_recv_data-179] command Frequency (0) ...
> [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close()
> [zForce_ir_touch_recv_data-142] command Deactivate ...
>
> ### Kobo Glo HD Model N437
>
> This device does not have the "command overrun" bug.
>
> Without fix, without workaround
>
> The system call trace shows:
>
> 3396 open("/dev/input/event1", O_RDONLY) = 11
> 3396 close(11) = 0
> 3396 open("/dev/input/event1", O_RDONLY) = 11
>
> The error does not occur, and the touchscreen works as expected:
>
> $ dmesg | grep -i zforce
> [drivers/input/touchscreen/zforce_i2c.c-740] zforce_i2c_open()
> [zForce_ir_touch_recv_data-233] command Activate (0) ...
> ...
> [zForce_ir_touch_recv_data-250] command Resolution (0) ...
> [zForce_ir_touch_recv_data-278] command Frequency (0) ...
> [zForce_ir_touch_recv_data-260] command Configuration ...
> [drivers/input/touchscreen/zforce_i2c.c-756] zforce_i2c_close()
> [zForce_ir_touch_recv_data-230] command Deactivate ...
>
> With fix, without workaround
>
> The system call trace shows:
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened [v2]
On Mon, 21 Sep 2020 18:34:29 GMT, John Neffenger
wrote:
>> Based on my notes below, I believe this change is safe for any Linux input
>> device driver because the driver shouldn't
>> receive the intermediate *open* and *close* calls at all.
>> Nonetheless, it would be reassuring if someone could try [this
>> change](https://github.com/openjdk/jfx/pull/258/files)
>> just once on a mainstream device, such as the Raspberry Pi with a
>> touchscreen LCD panel. I only have six obscure ARM
>> devices and a headless QEMU ARM virtual machine for testing. @johanvos or
>> @dellgreen - Is that something you could
>> test? If you think it's overkill and not worth it, that's fine, too.
>> Notes
>> The Linux kernel documentation about [opening and
>> closing](https://www.kernel.org/doc/html/latest/input/input-programming.html#dev-open-and-dev-close)
>> input devices
>> states:
>>> Note that input core keeps track of number of users for the device and
>>> makes sure that dev->open() is called only when
>>> the first user connects to the device and that dev->close() is called when
>>> the very last user disconnects. Calls to
>>> both callbacks are serialized.
>>
>> Also, the Linux Programmer's Manual for the *close* system call (`man 2
>> close`) states:
>>
>>> If *fd* is the last file descriptor referring to the underlying open file
>>> description (see **open**(2)), the resources
>>> associated with the open file description are freed.
>>
>> Running a JavaFX program with `strace -f -e trace=open,close -o strace.log`
>> shows the one *open* for the *event0*
>> keyboard, and the *open, open, close* for the *event1* touchscreen:
>> 5847 open("/dev/input/event0", O_RDONLY) = 11
>> ...
>> 5847 open("/dev/input/event1", O_RDONLY) = 12
>> 5847 open("/dev/input/event1", O_RDONLY) = 13
>> 5847 close(13) = 0
>>
>> Both devices are actually closed by the kernel when the JavaFX program ends.
>
>> Nonetheless, it would be reassuring if someone could try [this
>> change](https://github.com/openjdk/jfx/pull/258/files)
>> just once on a mainstream device, such as the Raspberry Pi with a
>> touchscreen LCD panel.
>
> I just placed an order for a Raspberry Pi 2 Model B -- the older model with
> the 32-bit ARMv7-A architecture. It's the
> oldest, slowest, and coolest-running Raspberry Pi supported by Ubuntu, and
> it's also the closest match to the
> processors found in devices with e-paper displays. The Raspberry Pi should
> finally let me test changes like this to the
> Linux Monocle platform on which the EPD platform is based. I may eventually
> add a touchscreen, but I'm hoping to get by
> with just the mouse for now.
@johanvos, would you mind approving this pull request again? It looks as if
your prior approval was removed when I
merged the master branch. All of the tests I ran this week worked as expected
and are described below.
### Tests
I ran additional tests on the following devices after merging the master branch:
* Kobo Touch Model N905C,
* Kobo Glo HD Model N437, and
* Raspberry Pi 2 Model B.
I ran the tests:
* without this fix and without the workaround,
* with this fix but without the workaround,
* with this fix and with the workaround (representing this pull request).
The workaround is the code in `EPDInputDeviceRegistry.createDevice` that lets
the current JavaFX 15 release avoid the
problem even without this fix. The goal of this pull request is to be able
eventually to remove the
`EPDInputDeviceRegistry` subclass entirely.
The test results are shown below. The system call trace was captured with the
following command using the
[epd-javafx][1] application.
$ sudo strace -f -e trace=open,openat,close -o strace.log \
bin/run.sh --pattern=3 --loops=1
**Note:** In the messages below from the kernel ring buffer, the final two
lines with `zforce_i2c_close` and `Deactivate`
are printed when the program terminates. That call made by the kernel on
behalf of the program is not recorded in the
program's system call trace.
### Kobo Touch Model N905C
This device has the "command overrun" bug.
Without fix, without workaround
The system call trace shows:
3576 open("/dev/input/event1", O_RDONLY) = 12
3576 close(12) = 0
3576 open("/dev/input/event1", O_RDONLY) = 12
The error occurs, and the touchscreen is disabled:
$ dmesg | grep -i zforce
[drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open()
[zForce_ir_touch_recv_data-145] command Activate (0) ...
[zForce_ir_touch_recv_data-154] command Resolution (0) ...
[zForce_ir_touch_recv_data-179] command Frequency (0) ...
[drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close()
[drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open()
[zForce_ir_touch_recv_data-142] command Deactivate ...
[zForce_ir_touch_recv_data-198] command overrun (8) ...
[drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close()
[zForce_ir_touch_recv_data-142] command Deactivate
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened [v2]
> Fixes [JDK-8201568](https://bugs.openjdk.java.net/browse/JDK-8201568). John Neffenger has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision: - Merge branch 'master' into zforce-command-overrun - 8201568: zForce touchscreen input device fails when closed and immediately reopened - Changes: - all: https://git.openjdk.java.net/jfx/pull/258/files - new: https://git.openjdk.java.net/jfx/pull/258/files/54631e7f..68c05234 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=258=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=258=00-01 Stats: 391615 lines in 5768 files changed: 194160 ins; 135443 del; 62012 mod Patch: https://git.openjdk.java.net/jfx/pull/258.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/258/head:pull/258 PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Mon, 29 Jun 2020 18:49:33 GMT, John Neffenger wrote: > Nonetheless, it would be reassuring if someone could try [this > change](https://github.com/openjdk/jfx/pull/258/files) > just once on a mainstream device, such as the Raspberry Pi with a touchscreen > LCD panel. I just placed an order for a Raspberry Pi 2 Model B -- the older model with the 32-bit ARMv7-A architecture. It's the oldest, slowest, and coolest-running Raspberry Pi supported by Ubuntu, and it's also the closest match to the processors found in devices with e-paper displays. The Raspberry Pi should finally let me test changes like this to the Linux Monocle platform on which the EPD platform is based. I may eventually add a touchscreen, but I'm hoping to get by with just the mouse for now. - PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Mon, 27 Jul 2020 16:05:02 GMT, John Neffenger wrote: >>> By the way, do you know whether I would be able to use a PinePhone >> >> I have been trying to run JavaFX on the PinePhone ever since I got it. The >> OS I'm using is Fedora and it is aarch64, >> which means I had to add that arch to build.gradle to compile it. As is the >> touchscreen doesn't respond correctly at >> all. Every touch reports as a mouse_entered, mouse_exited event. Though >> this is due to using the Desktop JavaFX. Is >> there somewhere I can find an embedded aarch64 version? Since desktop >> javafx/OpenJFX only runs monocle in headless >> according to this: https://wiki.openjdk.java.net/display/OpenJFX/Monocle > >> Is there somewhere I can find an embedded aarch64 version? > > If I had a PinePhone, I would try to get the 32-bit *armhf* build of JavaFX > working. That's the one you build with > `gradle -PCOMPILE_TARGETS=armv6hf sdk jmod`. You may need to switch operating > systems. See the [Ask Ubuntu > question](https://askubuntu.com/q/1090351) "Can I run an ARM32 bit App on an > ARM64bit platform which is running Ubuntu > 16.04?" I do not know of a currently supported 32-bit armhf OS for the PinePhone. This issue affects Desktop JavaFX linux touch as well, so I suppose I'll need to dig into adding that in, which was mentioned on this older issue: https://github.com/javafxports/openjdk-jfx/issues/329#issuecomment-459433106 Thanks for the quick reply! - PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Mon, 27 Jul 2020 14:26:44 GMT, Tor (torbuntu) wrote: > Is there somewhere I can find an embedded aarch64 version? If I had a PinePhone, I would try to get the 32-bit *armhf* build of JavaFX working. That's the one you build with `gradle -PCOMPILE_TARGETS=armv6hf sdk jmod`. You may need to switch operating systems. See the [Ask Ubuntu question](https://askubuntu.com/q/1090351) "Can I run an ARM32 bit App on an ARM64bit platform which is running Ubuntu 16.04?" - PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Thu, 9 Jul 2020 22:35:01 GMT, John Neffenger wrote: > By the way, do you know whether I would be able to use a PinePhone I have been trying to run JavaFX on the PinePhone ever since I got it. The OS I'm using is Fedora and it is aarch64, which means I had to add that arch to build.gradle to compile it. As is the touchscreen doesn't respond correctly at all. Every touch reports as a mouse_entered, mouse_exited event. How can I set the `monocle.platform=Linux` to test? - PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Wed, 1 Jul 2020 12:10:23 GMT, Johan Vos wrote: > I don't have access to a Pi right now, so I can't test this (I'll be able to > test in about 10 days from now though) Thank you, Johan. If you have the time, that would be great. Otherwise, merging this for early-access builds of JavaFX 16 should provide enough time to find any regression errors on other devices. By the way, do you know whether I would be able to use a [PinePhone](https://www.pine64.org/pinephone/) instead of a Raspberry Pi for testing the Monocle Linux platform (`monocle.platform=Linux`)? The PinePhone has a quad-core ARM Cortex A53 processor instead of the quad-core ARM Cortex-A72 on the Raspberry Pi 4, but it also has the advantage of a built-in LCD touchscreen. - PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Mon, 29 Jun 2020 18:49:33 GMT, John Neffenger
wrote:
>> The rationale makes sense (open/open/close) instead of (open/close/open)
>
> Based on my notes below, I believe this change is safe for any Linux input
> device driver because the driver shouldn't
> receive the intermediate *open* and *close* calls at all.
> Nonetheless, it would be reassuring if someone could try [this
> change](https://github.com/openjdk/jfx/pull/258/files)
> just once on a mainstream device, such as the Raspberry Pi with a touchscreen
> LCD panel. I only have six obscure ARM
> devices and a headless QEMU ARM virtual machine for testing. @johanvos or
> @dellgreen - Is that something you could
> test? If you think it's overkill and not worth it, that's fine, too.
> Notes
> The Linux kernel documentation about [opening and
> closing](https://www.kernel.org/doc/html/latest/input/input-programming.html#dev-open-and-dev-close)
> input devices
> states:
>> Note that input core keeps track of number of users for the device and makes
>> sure that dev->open() is called only when
>> the first user connects to the device and that dev->close() is called when
>> the very last user disconnects. Calls to
>> both callbacks are serialized.
>
> Also, the Linux Programmer's Manual for the *close* system call (`man 2
> close`) states:
>
>> If *fd* is the last file descriptor referring to the underlying open file
>> description (see **open**(2)), the resources
>> associated with the open file description are freed.
>
> Running a JavaFX program with `strace -f -e trace=open,close -o strace.log`
> shows the one *open* for the *event0*
> keyboard, and the *open, open, close* for the *event1* touchscreen:
> 5847 open("/dev/input/event0", O_RDONLY) = 11
> ...
> 5847 open("/dev/input/event1", O_RDONLY) = 12
> 5847 open("/dev/input/event1", O_RDONLY) = 13
> 5847 close(13) = 0
>
> Both devices are actually closed by the kernel when the JavaFX program ends.
I don't have access to a Pi right now, so I can't test this (I'll be able to
test in about 10 days from now though)
-
PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.
--
On Mon, 29 Jun 2020 11:26:50 GMT, Johan Vos wrote:
>> Fixes [JDK-8201568](https://bugs.openjdk.java.net/browse/JDK-8201568).
>
> The rationale makes sense (open/open/close) instead of (open/close/open)
Based on my notes below, I believe this change is safe for any Linux input
device driver because the driver shouldn't
receive the intermediate *open* and *close* calls at all.
Nonetheless, it would be reassuring if someone could try [this
change](https://github.com/openjdk/jfx/pull/258/files)
just once on a mainstream device, such as the Raspberry Pi with a touchscreen
LCD panel. I only have six obscure ARM
devices and a headless QEMU ARM virtual machine for testing. @johanvos or
@dellgreen - Is that something you could
test? If you think it's overkill and not worth it, that's fine, too.
Notes
The Linux kernel documentation about [opening and
closing](https://www.kernel.org/doc/html/latest/input/input-programming.html#dev-open-and-dev-close)
input devices
states:
> Note that input core keeps track of number of users for the device and makes
> sure that dev->open() is called only when
> the first user connects to the device and that dev->close() is called when
> the very last user disconnects. Calls to
> both callbacks are serialized.
Also, the Linux Programmer's Manual for the *close* system call (`man 2 close`)
states:
> If *fd* is the last file descriptor referring to the underlying open file
> description (see **open**(2)), the resources
> associated with the open file description are freed.
Running a JavaFX program with `strace -f -e trace=open,close -o strace.log`
shows the one *open* for the *event0*
keyboard, and the *open, open, close* for the *event1* touchscreen:
5847 open("/dev/input/event0", O_RDONLY) = 11
...
5847 open("/dev/input/event1", O_RDONLY) = 12
5847 open("/dev/input/event1", O_RDONLY) = 13
5847 close(13) = 0
Both devices are actually closed by the kernel when the JavaFX program ends.
-
PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Sat, 27 Jun 2020 00:21:06 GMT, John Neffenger wrote: > Fixes [JDK-8201568](https://bugs.openjdk.java.net/browse/JDK-8201568). The rationale makes sense (open/open/close) instead of (open/close/open) - Marked as reviewed by jvos (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/258
Re: RFR: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Sat, 27 Jun 2020 00:21:06 GMT, John Neffenger wrote: > Fixes [JDK-8201568](https://bugs.openjdk.java.net/browse/JDK-8201568). I thought I could avoid fixing this bug simply by waiting a few years, but the old devices from 2012 and 2013 where the problem occurs are still supported and still even being sold (refurbished) by the manufacturer. The Monocle EPD platform has a work-around for the bug in the method `EPDInputDeviceRegistry.createDevice`. The problem does not occur on newer 2015 and 2018 devices that I tested. ### Background The Neonode zForce touchscreen input device almost always fails when it is opened immediately after being closed. Once it fails, no input events are received. The device driver logs the following messages to the kernel ring buffer: [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open() [zForce_ir_touch_recv_data-145] command Activate (0) ... [zForce_ir_touch_recv_data-154] command Resolution (0) ... [zForce_ir_touch_recv_data-179] command Frequency (0) ... [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close() [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open() [zForce_ir_touch_recv_data-142] command Deactivate ... [zForce_ir_touch_recv_data-198] command overrun (8) ... This pull request reorders the initialization sequence to be "open, open, close" instead of "open, close, open" so that the input device is never actually closed. With this change, the file reference count remains above zero so the call to close it is ignored, and the device driver logs the following messages to the ring buffer: [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open() [zForce_ir_touch_recv_data-145] command Activate (0) ... [zForce_ir_touch_recv_data-154] command Resolution (0) ... [zForce_ir_touch_recv_data-179] command Frequency (0) ... In both cases, when the program exits or is canceled with `Ctrl-C`, the input device is closed normally: [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close() [zForce_ir_touch_recv_data-142] command Deactivate ... ### Summary I decided to go ahead and submit this pull request for the following reasons: 1. we know of at least one input device where the code causes problems, so there could be others; 2. this makes progress towards removing 48 lines of work-around code from `EPDInputDeviceRegistry`; and 3. opening and closing an input device is expensive enough that we probably shouldn't close it when we know we're going to open it again only two statements later. - PR: https://git.openjdk.java.net/jfx/pull/258
