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

Update ST7789 to support writing to larger SPIDEV buffer #420

Merged
merged 3 commits into from
Aug 4, 2023

Conversation

newtonick
Copy link
Collaborator

@newtonick newtonick commented Aug 3, 2023

This PR provides a modest performance increase when an operation requires rendering an updated image to the waveshare LCD. Data is transferred to the LCD using spidev (user space interface) using python. This PR does not make any significant changes to this process.

The performance gain on this PR is done by writing out the data via a larger buffer and removing the previously required loop of write operations. Prior to this PR, the ShowImage method draws the entire LCD display via 29 4096 byte operations. By increasing the buffer size the entire display can be refreshed in a single operation instead of 29. The spidev kernel module supports increasing the buffer size default from 4096 via cmdline.txt. This increase is required to write out the bytes in a single operation. SeedSigner currently uses 115200 bytes (240 * 240 * 2, 240x240@16bit color). So this PR only shows performance gains after the spidev buffer has been increased. writebytes2 accepts arbitrary large lists. If list size exceeds the buffer size (which is read from /sys/module/spidev/parameters/bufsiz), data will be split into smaller chunks and sent in multiple operations. I will also be submitting at PR to increase the buffer size to 128k in SeedSignerOS. SeedSignerOS PR 55 makes this change.

To Do:

  • Add documentation in manual install instructions on how to update the cmdline.txt file to increase spider buffer size
  • Create SeedSignerOS PR to increase SPIDEV buffer to 128k

@newtonick
Copy link
Collaborator Author

Note: if the spidev buffer size is not increased. This code change will not cause any issues/conflicts. writebytes2 automatically splits data into smaller chunks to fit the spidev buffer size (unlike writebytes).

@newtonick newtonick added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 3, 2023
@kdmukai
Copy link
Contributor

kdmukai commented Aug 3, 2023

Tested ACK!

On my faster Pi Zero 1.3 running a manual install (therefore Raspi OS):

  • dev: 3.8fps
  • This PR: 5.0fps

Also seeing modest QR decoder framerate improvements: 4.8fps increased to 5.3fps.

@kdmukai
Copy link
Contributor

kdmukai commented Aug 3, 2023

Tested in my local SeedSigner OS build w/cmdline.txt change applied:

  • My livepreview_fps branch @ 2b20537 (latest from dev): 3.0fps | 3.5fps decoder
  • livepreview_fps @ e486099 (includes this PR): 6.0fps | 4.2fps decoder

And this is on my slightly slower Pi Zero (vs the above Raspi OS results)!

@kdmukai
Copy link
Contributor

kdmukai commented Aug 3, 2023

I recommend we set the ScanScreen.framerate to 6.

I tested different framerate settings in my local SeedSigner OS (w/this PR applied):

(live preview fps / QR decoder fps)
framerate  3: 6.3fps / 4.5fps
framerate  4: 6.2fps / 4.5fps
framerate  5: 6.0fps / 4.2fps
framerate  6: 5.8fps / 4.0fps
framerate  7: 5.6fps / 4.0fps
framerate  8: 5.3fps / 4.0fps
framerate 12: 4.8fps / 3.5fps

framerate=3 claimed the highest live preview fps but was visibly choppy due to the inherently low camera framerate. In other words, the fps metric was being fooled because the screen was rendering duplicate frames. I didn't test it, but have to assume the QR decoder fps are similarly wasting cycles on reprocessing the duplicate frames.

Presumably we get the same duplication issue fooling the fps metric until framerate >= reported fps.

So I wouldn't trust the metrics for the current value of framerate=5.

And then at higher framerates we see negative diminishing returns as presumably we pay a resource penalty for generating more frames than the display or QR decoder can consume.

Therefore framerate=6 seems the most sensible choice.

YMMV. Likely/definitely different optimal results for a Pi Zero 2W.

@jdlcdl
Copy link

jdlcdl commented Aug 3, 2023

ACK tested as of 32eda52

Manual Instructions were easy to follow, worked perfectly. Wow what a difference!
Also tested on ssos for pi2-dev, noticeably faster!
Also tested on ssos for pi0, much faster (but not as fast as keith's mod image from this morning).

@newtonick newtonick merged commit 886391b into SeedSigner:dev Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants