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.

Sign up for GitHub

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

Jump to bottom

feat(color): Use DMA for SPI flash access #4478

Merged
merged 9 commits into from
Jan 21, 2024
Merged

feat(color): Use DMA for SPI flash access #4478

merged 9 commits into from
Jan 21, 2024

Conversation

richardclli
Copy link
Collaborator

@richardclli richardclli commented Dec 29, 2023 •
edited
Loading

This may somehow solve the sound hiccup problem.

@richardclli richardclli self-assigned this Dec 29, 2023
@richardclli richardclli added this to the 2.10 milestone Dec 29, 2023
@richardclli
Copy link
Collaborator Author

@raphaelcoeffic I am not sure why I need to force to use the scratch buffer in your SPI driver for this to work properly. Need your input for this.

@richardclli richardclli changed the title enh(pl18): Use DMA for SPI flash access enh(color): Use DMA for SPI flash access Dec 29, 2023
@richardclli richardclli marked this pull request as draft December 29, 2023 06:35
@richardclli richardclli marked this pull request as ready for review December 30, 2023 02:18
@richardclli
Copy link
Collaborator Author

@raphaelcoeffic I figured out that DMA to SDRAM will be failed with FIFO mode, and I have added changes so that the FIFO mode can be selected for DMA configuration, please check if this change good for you.

@raphaelcoeffic I am not sure why I need to force to use the scratch buffer in your SPI driver for this to work properly. Need your input for this.

@richardclli
Copy link
Collaborator Author

After some testing, this PR will boost the R/W performance of the SPI flash to 3-4 times and free the MCU from a lot of works, it is quite promising. Will do more tests to see if this change can resolve the sound hiccup problem.

@richardclli richardclli mentioned this pull request Dec 30, 2023
Merged
@pfeerick pfeerick changed the title enh(color): Use DMA for SPI flash access feat(color): Use DMA for SPI flash access Jan 3, 2024
@pfeerick pfeerick added the enhancement ✨ New feature or request label Jan 3, 2024
@richardclli
Copy link
Collaborator Author

@pfeerick I am doing some extensive tests to see if this PR is stable enough for production.

@richardclli
Copy link
Collaborator Author

I have added the use of block erase to further improve write performance. The change in FrFTL already passes all integrity checks. And seems everything is stable, I tried to copy, overwrite, delete, format in many different sequences and did not found any data corruptions.

@pfeerick
Copy link
Member

pfeerick commented Jan 5, 2024

I'll play with it tomorrow then. And hopefully that unnamed french dude will be happy with it also 🤭

@richardclli
Copy link
Collaborator Author

@pfeerick And you can spot that I have added some handling during erase flash in BL, because the FTL can get corrupted if user did not reboot after erase flash and plugin USB to access the filesystem immediately.

@pfeerick pfeerick self-requested a review January 6, 2024 06:25
pfeerick
pfeerick approved these changes Jan 6, 2024
View reviewed changes
Copy link
Member

@pfeerick pfeerick left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Yes, indeed... I saw that last change doing SD re-init ;)

Functionally seems to be working just great on PL18EV. Was able to do exactly what you implied... erase and write to the SPI flash without needing to reboot. Copied part of the audio pack onto the SPI flash while main firmware was running... ran at a perfectly reasonable speed. LGTM

@richardclli
Copy link
Collaborator Author

richardclli commented Jan 9, 2024 •
edited
Loading

@pfeerick @raphaelcoeffic I managed to make FIFO mode works properly as well, and configured to align with SDRAM data width for best performance.

@richardclli
Copy link
Collaborator Author

I suspected that this issue may related to DMA settings, since it make use of SPI SD card driver and I think the DMA settings is wrong.

#4530

I changed the settings, and see if it can resolve the problem.

@richardclli
Copy link
Collaborator Author

richardclli commented Jan 13, 2024 •
edited
Loading

@pfeerick I have finished some extensive tests with DMA FIFO enabled, seems it is stable in my PL18EV and TX16S, in which these 2 models use different flash chips. 128MB and 32MB.

@pfeerick pfeerick mentioned this pull request Jan 14, 2024
Open
1 task
raphaelcoeffic
raphaelcoeffic approved these changes Jan 14, 2024
View reviewed changes
Copy link
Member

@raphaelcoeffic raphaelcoeffic left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me, if everyone is ok with it, let's merge. Thx for all the digging into the datasheets :-D

@pfeerick pfeerick self-requested a review January 14, 2024 23:04
@richardclli
Copy link
Collaborator Author

Let me clean up the No sdcard tests commits first.

@richardclli
Copy link
Collaborator Author

Cleaned up, ready for merge.

@pfeerick
Copy link
Member

Not seeing any issues on a variety of B&W and colorlcd radios, with assortment of SD cards. PL18EV w/ internal SPI flash looking good also.

@pfeerick pfeerick merged commit 4c2ea7a into main Jan 21, 2024
39 checks passed
@pfeerick pfeerick deleted the spi-flash-dma branch January 21, 2024 02:01
@davetheghost
Copy link

does this apply to SD cards as well?!

@gagarinlg
Copy link
Member

SD card access is DMA anyways, this is for SPI attached flash chips, not SD cards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raphaelcoeffic raphaelcoeffic raphaelcoeffic approved these changes

@pfeerick pfeerick Awaiting requested review from pfeerick

Assignees

@richardclli richardclli

Labels
enhancement ✨ New feature or request
Projects
None yet
Milestone
2.10
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@richardclli @pfeerick @davetheghost @gagarinlg @raphaelcoeffic