embassy icon indicating copy to clipboard operation
embassy copied to clipboard

Add gpdma ringbuffer support

Open calebstewart opened this issue 1 year ago • 3 comments
trafficstars

This is an ongoing attempt to get the changes from #2302 merged and working. I'm by no means an expert here, and this is currently just a first-pass at getting the merge conflicts fixed. This hasn't been tested at all yet by me.

calebstewart avatar Jul 09 '24 21:07 calebstewart

@Dirbaio I'm going to try and test this soon ( :tm: ) but I'm curious if I even updated some of this correctly. The APIs within DMA seem to have changed since Tyler originally implemented this branch. There were things like this:

/// This is a Readable ring buffer. It reads data from a peripheral into a buffer. The reads happen in circular mode.
/// There are interrupts on complete and half complete. You should read half the buffer on every read.
pub struct ReadableRingBuffer<'a, C: Channel, W: Word> {
    channel: PeripheralRef<'a, C>,
    ringbuf: ReadableDmaRingBuffer<'a, W>,
}

Where self.channel was used like:

self.channel.regs().ch(self.channel.num())

This was no longer valid, and looking over the repository changes over the last few months, I realized that other similar code was now using a pattern where they had an AnyChannel variable, and accessed the DMA registers through channel.info().dma.ch(...), so I opted to modify the structs like this:

/// This is a Readable ring buffer. It reads data from a peripheral into a buffer. The reads happen in circular mode.
/// There are interrupts on complete and half complete. You should read half the buffer on every read.
pub struct ReadableRingBuffer<'a, W: Word> {
    channel: PeripheralRef<'a, AnyChannel>,
    ringbuf: ReadableDmaRingBuffer<'a, W>,
}

And now most of the references to that variable look like:

let info = self.channel.info();
info.dma.ch(info.num)

I'm not sure if that's the "correct" way to go about it, though. Given that the only place I saw the channel info exposed was AnyChannel and that AnyChannel could represent... well, any channel, the generic seemed unnecessary. Happy to be corrected though.

calebstewart avatar Jul 09 '24 22:07 calebstewart

Ah, I just had a look at dma_bdma.rs, and what I did appears to be mostly the same as what was done there with some minor modifications. :)

calebstewart avatar Jul 09 '24 23:07 calebstewart

@calebstewart I'm testing the ringbuffer support for use with USART, maybe the method request_stop should be renamed to request_pause? Since the bdma implementation uses this.

@Dirbaio do you think a trait would be in order here? since even if we're using only a single implementation for each chip (BDMA or GPDMA) the interface should be the same regardless.

Luctins avatar Oct 22 '24 13:10 Luctins

done in #3923

Dirbaio avatar Sep 05 '25 12:09 Dirbaio