Skip to content

SDCard speedup : don't re-calculate CRC table, allow faster SPI data rate #23

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

Merged
merged 1 commit into from
Oct 18, 2019

Conversation

Anton-2
Copy link
Contributor

@Anton-2 Anton-2 commented Oct 16, 2019

Tested on a pyportal, with CircuitPython 5.0.0 alpha 4.

My ad-hoc benchmark is 20 refresh of the screen, with a 64x64 OnDiskBitmap on the SDCard.
Original adafruit_sdcard.py takes 19 s.
By removing the CRC table re-calculation, we are down to 1s.
Maxing the data rate at 24Mhz makes it 0.7s.

Other changes involve reading status bytes first before testing them. It's marginally faster, and more correct on line 257 where it was not initialized.

@ladyada
Copy link
Member

ladyada commented Oct 16, 2019

i wonder if we'd be able to play audio off sd card, something that did not work before :)

@Anton-2
Copy link
Contributor Author

Anton-2 commented Oct 16, 2019

For audio playback, it’s something I’m going to try in the next days, as it would help me in my project. But I fear that reading samples in the background won’t work : auto-refreshing the display always lock the board up. I had to use manual refresh to make it work.

I fear that we can’t call python code in sdcard.py in the background while running python code in the main loop. That’s why a C version of sdcard.py would be usefull, more than speed improvements.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thanks for this fix! A couple suggestions/questions about the non-CRC changes and then it should be good to go.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for the speed up.

@tannewt tannewt merged commit 21b7528 into adafruit:master Oct 18, 2019
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 18, 2019
Updating https://github.com/adafruit/Adafruit_CircuitPython_SD to 3.2.5 from 3.2.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_SD#29 from dhalbert/avoid-longint
  > Merge pull request adafruit/Adafruit_CircuitPython_SD#26 from Anton-2/master
  > Merge pull request adafruit/Adafruit_CircuitPython_SD#24 from adafruit/dherrada-patch-1
  > Merge pull request adafruit/Adafruit_CircuitPython_SD#23 from Anton-2/master
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.

3 participants