Skip to content

Commit 3f38d97

Browse files
DrTobeDrTobe
committed
Rewrite dma::CircBuffer as a single-writer-single-reader queue as suggested in #205.
1 parent aa1b554 commit 3f38d97

File tree

2 files changed

+110
-94
lines changed

2 files changed

+110
-94
lines changed

‎src/dma.rs

Lines changed: 108 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -318,23 +318,23 @@ pub struct CircBuffer<BUFFER, PAYLOAD>
318318
where
319319
BUFFER: 'static,
320320
{
321-
buffer: &'static mut [BUFFER; 2],
321+
buffer: &'static mut BUFFER,
322322
payload: PAYLOAD,
323-
readable_half: Half,
324-
consumed_offset: usize,
323+
read_index: usize,
324+
write_previous: usize,
325325
}
326326

327327
impl<BUFFER, PAYLOAD> CircBuffer<BUFFER, PAYLOAD>
328328
where
329-
&'static mut [BUFFER; 2]: StaticWriteBuffer,
329+
&'static mut BUFFER: StaticWriteBuffer,
330330
BUFFER: 'static,
331331
{
332-
pub(crate) fn new(buf: &'static mut [BUFFER; 2], payload: PAYLOAD) -> Self {
332+
pub(crate) fn new(buf: &'static mut BUFFER, payload: PAYLOAD) -> Self {
333333
CircBuffer {
334334
buffer: buf,
335335
payload,
336-
readable_half: Half::Second,
337-
consumed_offset: 0,
336+
read_index: 0,
337+
write_previous: 0,
338338
}
339339
}
340340
}
@@ -861,101 +861,117 @@ macro_rules! dma {
861861
where
862862
RxDma<PAYLOAD, $CX>: TransferPayload,
863863
{
864-
865-
/// Return the partial contents of the buffer half being written
866-
pub fn partial_peek<R, F, T>(&mut self, f: F) -> Result<R, Error>
867-
where
868-
F: FnOnce(&[T], Half) -> Result<(usize, R), ()>,
869-
B: AsRef<[T]>,
870-
{
871-
// this inverts expectation and returns the half being _written_
872-
let buf = match self.readable_half {
873-
Half::First => &self.buffer[1],
874-
Half::Second => &self.buffer[0],
875-
};
876-
// ,- half-buffer
877-
// [ x x x x y y y y y z | z z z z z z z z z z ]
878-
// ^- pending=11
879-
let pending = self.payload.channel.get_cndtr() as usize; // available transfers (= bytes) in _whole_ buffer
880-
let slice = buf.as_ref();
881-
let capacity = slice.len(); // capacity of _half_ a buffer
882-
// <--- capacity=10 --->
883-
// [ x x x x y y y y y z | z z z z z z z z z z ]
884-
let pending = if pending > capacity {
885-
pending - capacity
886-
} else {
887-
pending
888-
};
889-
// ,- half-buffer
890-
// [ x x x x y y y y y z | z z z z z z z z z z ]
891-
// ^- pending=1
892-
let end = capacity - pending;
893-
// [ x x x x y y y y y z | z z z z z z z z z z ]
894-
// ^- end=9
895-
// ^- consumed_offset=4
896-
// [y y y y y] <-- slice
897-
let slice = &buf.as_ref()[self.consumed_offset..end];
898-
match f(slice, self.readable_half) {
899-
Ok((l, r)) => { self.consumed_offset += l; Ok(r) },
900-
Err(_) => Err(Error::BufferError),
864+
/// Determines if the write index passed the given `mark` when it moved
865+
/// from `previous` to `current` with wrapping behaviour at `wrap`.
866+
/// When `current` reaches `mark` (`current == mark`), this method already
867+
/// returns `true`.
868+
fn passed_mark(&self, mut previous: usize, mut current: usize, mark: usize, wrap: usize) -> bool {
869+
// We have three indexes mark (m), previous (p) and current (c) so
870+
// there are fac(3)=6 possibilities how those can be ordered. For both
871+
// cases (passed, !passed), there are three wrapped variations each:
872+
// !passed: 1. m <= p <= c, 2. c < m <= p, 3. p <= c < m
873+
// passed: 1. m <= c < p, 2. p < m <= c, 3. c < p < m
874+
// By enforcing p >= m and c >= m (reverting the wrap), we only have to
875+
// check the first case.
876+
if previous < mark {
877+
previous += wrap;
901878
}
879+
if current < mark {
880+
current += wrap;
881+
}
882+
current < previous && current >= mark
902883
}
903884

904-
/// Peeks into the readable half of the buffer
905-
/// Returns the result of the closure
906-
pub fn peek<R, F, T>(&mut self, f: F) -> Result<R, Error>
885+
/// Reads and removes the available contents of the dma buffer into `buf`.
886+
/// Returns `Err(Error::Overrun)` if an overrun is detected but there is no
887+
/// guarantee that every overrun can be detected.
888+
/// On success, returns the number of words written to `buf`.
889+
pub fn read<T>(&mut self, buf: &mut [T]) -> Result<usize, Error>
907890
where
908-
F: FnOnce(&[T], Half) -> R,
909891
B: AsRef<[T]>,
892+
T: Copy,
910893
{
911-
let half_being_read = self.readable_half()?;
912-
let buf = match half_being_read {
913-
Half::First => &self.buffer[0],
914-
Half::Second => &self.buffer[1],
915-
};
916-
let slice = &buf.as_ref()[self.consumed_offset..];
917-
self.consumed_offset = 0;
918-
Ok(f(slice, half_being_read))
919-
}
920-
921-
/// Returns the `Half` of the buffer that can be read
922-
pub fn readable_half(&mut self) -> Result<Half, Error> {
894+
// We do our best to figure out when an overrun occurred but without
895+
// risking false positives.
896+
//
897+
// One possibility to detect an overrun is by examining the read- and
898+
// write-indexes: If the write-index passes the read-index, that is an
899+
// overrun condition because an unread value is overwritten. This check
900+
// can fail if the write-index passed the read-index but due to
901+
// wrapping, this can not be detected. For example, this can happen if
902+
// `capacity` many words were written since the last read which looks
903+
// like no word has been written.
904+
//
905+
// Another possibility to detect overruns is by checking the
906+
// TransferComplete and HalfTransferComplete flags. For example, the
907+
// TransferComplete flag is set when the write-index wraps around so
908+
// whenever we encounter this flag the new write-index should be
909+
// smaller than the previous one. If it is not, more than `capacity`
910+
// many words must have been written which definitely must be an
911+
// overrun. Another possibility to formulate this condition is to check
912+
// wheter the write-index passed index 0. A similar condition can be
913+
// formulated for the HalfTransferComplete flag, i.e. check whether the
914+
// write-index passed index capacity/2.
915+
//
916+
// Unfortunately, even both checks together can not guarantee that we
917+
// detect all overruns.
918+
// Example:
919+
// read = 2, write = 3, 2*capacity-2 words written => write = 1.
920+
let capacity = self.buffer.as_ref().len();
921+
// We read the flags before reading the current write-index because if
922+
// another word is written between those two accesses, this ordering
923+
// prevents a false positive overrun error.
923924
let isr = self.payload.channel.isr();
924-
let first_half_is_done = isr.$htifX().bit_is_set();
925-
let second_half_is_done = isr.$tcifX().bit_is_set();
926-
927-
if first_half_is_done && second_half_is_done {
928-
return Err(Error::Overrun);
925+
let half_complete_flag = isr.$htifX().bit_is_set();
926+
if half_complete_flag {
927+
self.payload.channel.ifcr().write(|w| w.$chtifX().set_bit());
929928
}
930-
931-
let last_read_half = self.readable_half;
932-
933-
Ok(match last_read_half {
934-
Half::First => {
935-
if second_half_is_done {
936-
self.payload.channel.ifcr().write(|w| w.$ctcifX().set_bit());
937-
938-
self.readable_half = Half::Second;
939-
Half::Second
940-
} else {
941-
last_read_half
942-
}
943-
}
944-
Half::Second => {
945-
if first_half_is_done {
946-
self.payload.channel.ifcr().write(|w| w.$chtifX().set_bit());
947-
948-
self.readable_half = Half::First;
949-
Half::First
950-
} else {
951-
last_read_half
952-
}
929+
let transfer_complete_flag = isr.$tcifX().bit_is_set();
930+
if transfer_complete_flag {
931+
self.payload.channel.ifcr().write(|w| w.$ctcifX().set_bit());
932+
}
933+
let write_current = capacity - self.payload.channel.get_cndtr() as usize;
934+
// Copy the data before examining the overrun conditions. If the
935+
// overrun happens shortly after the flags and write-index were read,
936+
// we can not detect it anyways. So we can only hope that we have
937+
// already read the word(s) that will be overwritten.
938+
let available = if write_current >= self.read_index {
939+
write_current - self.read_index
940+
} else {
941+
capacity + write_current - self.read_index
942+
};
943+
let read_len = core::cmp::min(available, buf.len());
944+
if self.read_index + read_len <= capacity {
945+
// non-wrapping read
946+
buf[..read_len].copy_from_slice(&self.buffer.as_ref()[self.read_index..self.read_index+read_len]);
947+
} else {
948+
// wrapping read
949+
let first_read_len = capacity - self.read_index;
950+
let second_read_len = read_len - first_read_len;
951+
buf[..first_read_len].copy_from_slice(&self.buffer.as_ref()[self.read_index..]);
952+
buf[first_read_len..read_len].copy_from_slice(&self.buffer.as_ref()[..second_read_len]);
953+
}
954+
// For checking the overrun conditions, it is important that we use the
955+
// old read_index so do not increment it yet but check overrun
956+
// conditions first.
957+
// TODO When is the half-complete flag written exactly? Especially for
958+
// odd buffer capacities.
959+
let overrun = self.passed_mark(self.write_previous, write_current, self.read_index, capacity) || (transfer_complete_flag && !self.passed_mark(self.write_previous, write_current, 0, capacity)) || (half_complete_flag && !self.passed_mark(self.write_previous, write_current, capacity/2, capacity));
960+
self.write_previous = write_current;
961+
if overrun {
962+
self.read_index = write_current;
963+
Err(Error::Overrun)
964+
} else {
965+
self.read_index += read_len;
966+
if self.read_index >= capacity {
967+
self.read_index -= capacity;
953968
}
954-
})
969+
Ok(read_len)
970+
}
955971
}
956972

957973
/// Stops the transfer and returns the underlying buffer and RxDma
958-
pub fn stop(mut self) -> (&'static mut [B; 2], RxDma<PAYLOAD, $CX>) {
974+
pub fn stop(mut self) -> (&'static mut B, RxDma<PAYLOAD, $CX>) {
959975
self.payload.stop();
960976

961977
(self.buffer, self.payload)
@@ -1243,11 +1259,11 @@ pub trait ReceiveTransmit {
12431259
/// Trait for circular DMA readings from peripheral to memory.
12441260
pub trait CircReadDma<B, RS>: Receive
12451261
where
1246-
&'static mut [B; 2]: StaticWriteBuffer<Word = RS>,
1262+
&'static mut B: StaticWriteBuffer<Word = RS>,
12471263
B: 'static,
12481264
Self: core::marker::Sized,
12491265
{
1250-
fn circ_read(self, buffer: &'static mut [B; 2]) -> CircBuffer<B, Self>;
1266+
fn circ_read(self, buffer: &'static mut B) -> CircBuffer<B, Self>;
12511267
}
12521268

12531269
/// Trait for DMA readings from peripheral to memory.

‎src/serial.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -679,11 +679,11 @@ macro_rules! hal {
679679

680680
impl<B> crate::dma::CircReadDma<B, u8> for $rxdma
681681
where
682-
&'static mut [B; 2]: StaticWriteBuffer<Word = u8>,
682+
&'static mut B: StaticWriteBuffer<Word = u8>,
683683
B: 'static,
684684
Self: core::marker::Sized,
685685
{
686-
fn circ_read(mut self, mut buffer: &'static mut [B; 2],
686+
fn circ_read(mut self, mut buffer: &'static mut B,
687687
) -> CircBuffer<B, Self>
688688
{
689689
let (ptr, len) = unsafe { buffer.static_write_buffer() };

0 commit comments

Comments
 (0)