DMA Futures #65
Merged
DMA Futures #65
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
astapleton
force-pushed
the
as/gpdma-async
branch
from
564a91b to
36701a2
Compare
astapleton
force-pushed
the
as/gpdma-async
branch
5 times, most recently
from
34418a0 to
fe59cf0
Compare
astapleton
force-pushed
the
as/gpdma-async
branch
from
fe59cf0 to
48acbcd
Compare
usbalbin
reviewed
Aug 8, 2025
src/gpdma/future.rs
Outdated
| DmaTransfer, Error, Instance, Word, | ||
| }; | ||
|
|
||
| // This trait is defined to be bound by ChannelWaker when futures are enabled via the |
Member
There was a problem hiding this comment.
Are those supposed to be // and not ///?
Member
Author
There was a problem hiding this comment.
I think it also needs a /// block for public consumption. This was a note differentiating the definition with and without the gpdma-future feature flag
usbalbin
reviewed
Aug 8, 2025
Comment on lines
+153
to
+157
| // This creates a DmaChannelRef instance for channel N, which is be a duplicate to the | ||
| // DmaChannelRef instance that is held as a mutable reference in the DmaTransfer struct | ||
| // while a transfer is in progress. However, it is only used to disable the transfer | ||
| // interrupts for the channel and wake up the task that is waiting for the transfer to | ||
| // complete, which can be done safely. |
Member
There was a problem hiding this comment.
This feels like the the motivation of an unsafe block. Should DmaChannelRef::new be made unsafe?
Member
Author
There was a problem hiding this comment.
Yeah, I was thinking the same while writing it, so I think it makes sense to mark new as unsafe
usbalbin
reviewed
Aug 10, 2025
usbalbin
reviewed
Aug 10, 2025
usbalbin
approved these changes
Aug 11, 2025
Member
usbalbin
left a comment
usbalbin
left a comment
There was a problem hiding this comment.
Very cool stuff!
This looks good to me now as far as I can tell :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements futures for GPDMA transfers by adding a wrapper for
DmaTransfer,DmaTransferFuture, implementing Future forDmaTransferFuturein thegpdma::futuremodule, which is created when aDmaTransferis awaited via theIntoFuturetrait. Under the hood, the implementation uses anAtomicWakerfrom thefutures-utilcrate and channel interrupts to drive the future. Because the wakers need to be defined statically in order to be accessible from the channel interrupts, and to prevent accidental colliding implementations of interrupt handlers for the DMA channels, the futures implementation is hidden behind thegpdma-futurefeature flag.