Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kobo: Allow poking at the EPDC PM directly #1645

Merged
merged 19 commits into from Aug 1, 2023
Merged

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Aug 1, 2023

On devices where it's available (~Mk. 8+), Nickel does that on touch events every ~1.5s. For some mysterious reason (it may be a red herring), this seems to help with the mysterious hangs we get on, strangely enough, all the NXP boards where this feature is available...

It does not help with the timeouts in the wait__for_update_complete ioctl these boards also suffer from, despite Nickel also doing something weird with those, which I tried to replicate for shit'n giggles. Unsurprisingly so, as these timeouts have been reproduced in Nickel (and, in fact, I did just that myself in the course of testing, c.f., 729fa5a).

Speaking of testing, this was all done thanks to @ezuk, who donated a Clara 2E to try to get to the bottom of this mess. Thanks again ;).


This change is Reviewable

Will poke at it some more later...
I mean, the waiting sort oif achieved the desired effect, but it never
really helped with the issue at hand, so it's just extra delay for no
good reason.

Effectively reverts koreader#1576
Not that the sscanf call in question is actually checked, of course... ;D.
(Because, yeah, I just managed to repro it with all the other "fixes" in
place)
Or maybe it does, a bit; who knows, the whole thing is super weird.
```
[pid  2694<nickel>] 04:49:19.851633 [718bb4ec] ioctl(3</dev/fb0>, MXCFB_SEND_UPDATE_V2, {update_region={top=0, left=0, width=1072, height=1448}, waveform_mode=NTX_WFM_MODE_GLR16 => NTX_WFM_MODE_GLR16, update_mode=UPDATE_MODE_PARTIAL, update_marker=589, temp=TEMP_USE_AMBIENT, flags=0, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=NULL, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}) = 0 <0.000585>
[pid  2694<nickel>] 04:49:20.265339 [718bb4ec] ioctl(3</dev/fb0>, MXCFB_SEND_UPDATE_V2, {update_region={top=0, left=0, width=1072, height=1448}, waveform_mode=NTX_WFM_MODE_GC16 => NTX_WFM_MODE_GC16, update_mode=UPDATE_MODE_FULL, update_marker=590, temp=TEMP_USE_AMBIENT, flags=0, dither_mode=EPDC_FLAG_USE_DITHERING_PASSTHROUGH, quant_bit=0, alt_buffer_data={phys_addr=NULL, width=0, height=0, alt_update_region={top=0, left=0, width=0, height=0}}}) = 0 <0.000572>
[pid  2694<nickel>] 04:49:20.267273 [718bb4ec] ioctl(3</dev/fb0>, HWTCON_WAIT_FOR_UPDATE_COMPLETE or MXCFB_WAIT_FOR_UPDATE_COMPLETE_V3, {update_marker=589, collision_test=0 => 0}) = 0 <0.000101>
[pid  2694<nickel>] 04:49:20.269302 [718bb4ec] ioctl(3</dev/fb0>, HWTCON_WAIT_FOR_UPDATE_COMPLETE or MXCFB_WAIT_FOR_UPDATE_COMPLETE_V3, {update_marker=590, collision_test=0 <unfinished ...>
[pid  3950<Qt HTTP thread>] 04:49:22.082990 [718bb4ec] ioctl(33<socket:[14639]>, FIONREAD, [397]) = 0 <0.000458>
[pid  2694<nickel>] 04:49:25.269320 [718bb4ec] <... ioctl resumed>) = -1 ETIMEDOUT (Connection timed out) <4.999146>
```
Let's hope that won't reintroduce tolerance issues because of the
tighter timings, making the whole thing racier and bring back the hangs...
Copy link
Member Author

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @NiLuJe)

-- e.g., I mostly see it before the refresh on NXP & sunxi, but I mostly see it in front of everything on MTK).
-- TL;DR: Because, on devices flagged !hasReliableMxcWaitFor, we were seeing deadlock issues with refreshes that aren't necessarily tied to input,
-- (and for simplicity's sake), we simply unconditionally do this here as early possible.
-- NOTE: This might be a gigantic red herring, and simply a case of the very few extra cpu cycles involved throwing off the race...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheee \o/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the upside, I genuinely haven't managed to hang the Clara 2E with jump markers since ;).

Copy link
Member Author

@NiLuJe NiLuJe Aug 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The downside being I never actually managed to hang it with anything else (specifically, Notifications).

My only other hang was extremely overenginereed, as it involved more than a solid minute of ftrace, which will positively spam refresh ioctls ;).

@NiLuJe
Copy link
Member Author

NiLuJe commented Aug 1, 2023

FWIW, I have yet to validate whether this actually helps with the missed refreshes on wakeup from standby on sunxi, i.e., koreader/koreader#10743 (comment)

Mainly because these are fairly rare, which means I actually need time to read on that device ;p.

@NiLuJe NiLuJe merged commit afbabf2 into koreader:master Aug 1, 2023
2 of 3 checks passed
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Aug 1, 2023
NiLuJe added a commit to koreader/koreader that referenced this pull request Aug 1, 2023
…XP boards (#10771)

* Notification: Drop the fencing from #10083; it never actually helped, and had subtle side-effects we could do without.
* VirtualKeyBoard: Flash on close, otherwise, some of the fast refresh glitches may be burned into the working buffer until a flash. Making sure we flash ourselves prevent it from sticking around on the page ;).
* util: Move `writeToSysfs` to base (i.e., `ffi/util`), as we need it there (and it actually makes more sense there anyway ;p).
* Bump base for koreader/koreader-base#1645, which is where the actual workaround (hopefully) lives.

Re #8414, #9806, #10558
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants