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

dma: don't swap buffers #367

Merged
merged 17 commits into from
Jun 4, 2021
Merged

dma: don't swap buffers #367

merged 17 commits into from
Jun 4, 2021

Conversation

jordens
Copy link
Member

@jordens jordens commented May 24, 2021

  • This uses a new closure-based method to the DMA HAL implementation which
    gives access to the inactive buffer directly.
  • It removes changing addresses, the third buffer for DBM, the inactive
    address poisoning, and allows the cancellation of the redundant repeat
    memory barriers and compiler fences.
  • This is now around 20 instructions per buffer down from about 100 cycles
    before.
  • Also introduces a new SampleBuffer type alias.
  • The required unpacking of the resources structure is a bit annoying
    but could probably abstraced away.
  • Reduced pounder capture rate to the batch rate using the prescaler.
  • Removes the Pounder Timestamper DMA (close Remove Pounder timestamping DMA #260)

TODO:

  • Tested that dual-iir still works
  • Tested that DMA overflows are signaled as panics (batch size 1 at full rate)
  • Adapt lockin
  • Tested on FLS without pounder timestamp DMA.

* This uses a new closure-based method to the DMA HAL implementation which
  gives access to the inactive buffer directly.
* It removes changing addresses, the third buffer for DBM, the inactive
  address poisoning, and allows the cancellation of the redundant repeat
  memory barriers and compiler fences.
* This is now around 20 instructions per buffer down from about 100 cycles
  before.
* Also introduces a new `SampleBuffer` type alias.
* The required unpacking of the resources structure is a bit annoying
  but could probably abstraced away.

TODO:

* Test
* Adapt `lockin`
* master:
  adapt to new heapless/serde-json-core after const-generics
  Bump serde-json-core from 0.3.0 to 0.4.0
  build(deps): bump heapless from 0.6.1 to 0.7.1
  setup: cleanup
  itcm: add some comments, make it safe
  build(deps): bump ndarray from 0.15.1 to 0.15.2
  Updating dependencies
  Updating the embedded-nal
  Removing spurious settings updates
  deps: add rationales for git dependencies
  itcm: implement in rust and execute during setup()
  remove duplicate linker option
  gha: install gcc
  fmt
  dependencies: align with master
  bump cortex-m-rt to 0.6.13+git
  memory.x: remove comment about old cortex-m-rt
  enable itcm/dtcm explicitly
  load process into itcm
* master:
  pounder: clippy
  pounder: add comment on channel enum
  ad9959: refactor pad()
  pounder: enum for gpio ext pins
  pounder: fix attenuator indices (latch and shiftreg)
  pounder io extender: hack around some bug
  rf_power: fix measurement
  attenuators: use robust latching sequence
  deps: use mcp23017 release
  pounder: simplify attenuator spi interface
@jordens jordens marked this pull request as ready for review June 1, 2021 12:27
* One sample per batch is typical and sufficient.
* DMA has more overhead than direct read for one sample.
This reverts commit 26b2613.

First needs to reduce capture rate to batch interval. Otherwise it's
jittery due to polling alignment.
@jordens jordens requested a review from ryan-summers June 1, 2021 15:56
Copy link
Member

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

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

Other comments:

We need to update the SPI ISRs in the main apps to either listen for SPI errors, or just remove them entirely. It's my understanding that these are now unused as they are covered by the DMA DBM mode and unwrapped DMA overflow error

src/lib.rs Show resolved Hide resolved
src/hardware/pounder/dds_output.rs Show resolved Hide resolved
src/hardware/timers.rs Show resolved Hide resolved
src/hardware/pounder/timestamp.rs Outdated Show resolved Hide resolved
@jordens
Copy link
Member Author

jordens commented Jun 4, 2021

Other comments:

We need to update the SPI ISRs in the main apps to either listen for SPI errors, or just remove them entirely. It's my understanding that these are now unused as they are covered by the DMA DBM mode and unwrapped DMA overflow error

Yes. process() being slow and loosing the race to the DMA buffer should not cause those ISRs to fire anymore (instead panic as tested).
But we were and are still listening for errors from the SPI peripheral. I expect them to be triggered if the DMA peripheral is too slow serving them or badly configured. But I don't know how to trigger them. I've updated the panics to explicitly state that the error is from the SPI peripheral and not DBM. But let's keep them. Apart from the code duplication I don't think they hurt us.

@jordens
Copy link
Member Author

jordens commented Jun 4, 2021

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 4, 2021

Build succeeded:

@bors bors bot merged commit 5e2c2c8 into master Jun 4, 2021
@bors bors bot deleted the rj/fast-dbm branch June 4, 2021 09:02
@ryan-summers
Copy link
Member

But we were and are still listening for errors from the SPI peripheral. I expect them to be triggered if the DMA peripheral is too slow serving them or badly configured. But I don't know how to trigger them. I've updated the panics to explicitly state that the error is from the SPI peripheral and not DBM. But let's keep them. Apart from the code duplication I don't think they hurt us.

While we can still listen for these errors, we don't have to bind an ISR handler in the app. If we don't bind an ISR, they will use the default hardfault handler, which would clean up the apps a bit. In any case, it's a cosmetic difference imo.

@jordens
Copy link
Member Author

jordens commented Jun 4, 2021

Ack. I'll leave them in dual-iir as the prime debugging app and remove them from lockin and fls.

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.

Remove Pounder timestamping DMA
2 participants