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

Revert "Reduce firmware USB transfer size from 16KB to 8KB." #1412

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

martinling
Copy link
Member

This reverts PR #1203, which introduced bug #1363.

The problem is as summarised here:

The 32KB sample buffer straddles a block boundary such that each 16KB half-buffer is in a separate SRAM block which has its own independent AHB bus connection. With 16KB transfers, the M0 and the USB peripheral were always accessing different blocks. This change to 8KB transfers allowed the USB peripheral to start reading from the first 8KB of a 16KB half-buffer while the M0 core was still writing to the second 8KB. The contention caused by having two bus masters accessing the same SRAM block added intermittent latency, causing the M0 to sometimes miss its SGPIO timing deadlines.

Reverting this PR will eliminate the glitches, but will also cause a regression to host throughput.

We have a plan for future improvement as set out here:

There is enough space in ram_local1 (which stores the M4 text section) to add a 32 KiB or 48 KiB USB buffer separate from the M0/SGPIO buffer. Instead of having USB operations read and write directly from/to the SGPIO buffer, we can have DMA shuttle data between the SGPIO buffer and this new USB buffer. DMA transfers should complete very quickly, long before the M0 is done with the other half of the SGPIO buffer. USB reads and writes would contend only with M4 instruction reads, never with the M0.

Until this new scheme is implemented however, we should go ahead and revert the previous change.

@martinling martinling linked an issue Feb 13, 2024 that may be closed by this pull request
@mossmann mossmann merged commit 840b6f4 into greatscottgadgets:master Feb 15, 2024
17 checks passed
@martinling martinling mentioned this pull request Feb 21, 2024
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.

Periodic small glitch in the received signal
2 participants