Skip to content

Commit ae73526

Browse files
committed
fix(lcd_cam): address PR review feedback and CI failures
Review fixes: - Update send_bounce_buffered() docs to reflect polling-only API (ISR binding was removed earlier; docs still mentioned it) - Validate back buffer size in set_back_buffer() to prevent OOB reads when framebuffer pointer is swapped - Break swap_buffers() spin loop on is_done() to avoid deadlock if DMA enters error state CI fixes: - Gate all bounce buffer types behind #[cfg(feature = "unstable")] so critical_section (an optional dep) isn't required in non-unstable builds — fixes esp-hal (xtensa), msrv (xtensa), docs (xtensa) - Fix clippy: map_or(false, ...) -> is_some_and(...)
1 parent 1013682 commit ae73526

1 file changed

Lines changed: 25 additions & 7 deletions

File tree

esp-hal/src/lcd_cam/lcd/dpi.rs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,12 @@
9696
//! ```
9797
9898
use core::{
99-
cell::RefCell,
10099
marker::PhantomData,
101100
mem::ManuallyDrop,
102101
ops::{Deref, DerefMut},
103102
};
104103

104+
#[cfg(feature = "unstable")]
105105
use critical_section::Mutex;
106106

107107
use crate::{
@@ -133,7 +133,9 @@ use crate::{
133133
time::Rate,
134134
};
135135

136-
static BOUNCE_STATE: Mutex<RefCell<Option<DmaBounceBuffer>>> = Mutex::new(RefCell::new(None));
136+
#[cfg(feature = "unstable")]
137+
static BOUNCE_STATE: Mutex<core::cell::RefCell<Option<DmaBounceBuffer>>> =
138+
Mutex::new(core::cell::RefCell::new(None));
137139

138140
/// Errors that can occur when configuring the DPI peripheral.
139141
#[derive(Debug, Clone, Copy, PartialEq)]
@@ -146,6 +148,7 @@ pub enum ConfigError {
146148
InvalidBounceBufferSize,
147149
}
148150

151+
#[cfg(feature = "unstable")]
149152
/// Bounce buffer state for continuous RGB DPI output from a PSRAM framebuffer.
150153
///
151154
/// Holds two SRAM bounce buffers and their circular DMA descriptor chain.
@@ -170,6 +173,7 @@ pub struct DmaBounceBuffer {
170173
pub(crate) eof_count: usize,
171174
}
172175

176+
#[cfg(feature = "unstable")]
173177
#[allow(dead_code)]
174178
impl DmaBounceBuffer {
175179
/// Create a new bounce buffer state.
@@ -310,6 +314,7 @@ impl DmaBounceBuffer {
310314
}
311315
}
312316

317+
#[cfg(feature = "unstable")]
313318
// SAFETY: `DmaBounceBuffer` contains a raw pointer (`*mut [DmaDescriptor; 2]`) which prevents
314319
// auto-derived `Send`/`Sync`. The pointer is constructed from a `&'static mut [DmaDescriptor; 2]`
315320
// and is valid for the `'static` lifetime. Access to the pointed-to data is controlled
@@ -318,8 +323,10 @@ impl DmaBounceBuffer {
318323
// and access behind a shared reference when protected by a critical section.
319324
unsafe impl Send for DmaBounceBuffer {}
320325
// SAFETY: See the safety rationale above for `Send`.
326+
#[cfg(feature = "unstable")]
321327
unsafe impl Sync for DmaBounceBuffer {}
322328

329+
#[cfg(feature = "unstable")]
323330
/// Refill bounce buffers from the PSRAM framebuffer.
324331
///
325332
/// Called by [`DpiBounceTransfer::poll`] to check for completed DMA
@@ -724,16 +731,16 @@ where
724731
/// Start a continuous bounce-buffered RGB DPI transfer from a PSRAM framebuffer.
725732
///
726733
/// Sets up a circular two-descriptor DMA chain that continuously streams data from
727-
/// `bounce_state`'s framebuffer to the LCD_CAM peripheral, refilling each bounce
728-
/// buffer on every GDMA EOF interrupt.
734+
/// `bounce_state`'s framebuffer to the LCD_CAM peripheral. Bounce buffers are
735+
/// refilled in software as part of the transfer's polling API.
729736
///
730737
/// After calling this method, call [`DpiBounceTransfer::poll`] periodically to
731-
/// refill bounce buffers, or rely on the automatically bound GDMA TX interrupt
732-
/// handler for interrupt-driven operation.
738+
/// refill bounce buffers for continuous operation.
733739
///
734740
/// # Errors
735741
///
736742
/// Returns `(DmaError, Dpi, DmaBounceBuffer)` on DMA setup failure.
743+
#[cfg(feature = "unstable")]
737744
#[instability::unstable]
738745
pub fn send_bounce_buffered(
739746
mut self,
@@ -823,6 +830,7 @@ where
823830
}
824831
}
825832

833+
#[cfg(feature = "unstable")]
826834
unsafe impl DmaTxBuffer for DmaBounceBuffer {
827835
type View = ();
828836
type Final = ();
@@ -844,6 +852,7 @@ unsafe impl DmaTxBuffer for DmaBounceBuffer {
844852
fn from_view(_view: Self::View) -> Self::Final {}
845853
}
846854

855+
#[cfg(feature = "unstable")]
847856
#[instability::unstable]
848857
/// Represents an ongoing continuous (bounce-buffered) DPI RGB transfer.
849858
///
@@ -854,6 +863,7 @@ pub struct DpiBounceTransfer<'d, Dm: DriverMode> {
854863
back_buffer: Option<(*mut u8, usize)>,
855864
}
856865

866+
#[cfg(feature = "unstable")]
857867
impl<'d, Dm: DriverMode> DpiBounceTransfer<'d, Dm> {
858868
/// Returns true if the transfer is no longer active.
859869
#[instability::unstable]
@@ -890,6 +900,13 @@ impl<'d, Dm: DriverMode> DpiBounceTransfer<'d, Dm> {
890900
/// boundary.
891901
#[instability::unstable]
892902
pub fn set_back_buffer(&mut self, buf: &'static mut [u8]) {
903+
let valid = critical_section::with(|cs| {
904+
BOUNCE_STATE
905+
.borrow_ref(cs)
906+
.as_ref()
907+
.is_some_and(|state| buf.len() == state.fb_len)
908+
});
909+
assert!(valid, "back buffer length must match front buffer length");
893910
self.back_buffer = Some((buf.as_mut_ptr(), buf.len()));
894911
}
895912

@@ -921,7 +938,7 @@ impl<'d, Dm: DriverMode> DpiBounceTransfer<'d, Dm> {
921938
};
922939

923940
loop {
924-
if self.poll() {
941+
if self.poll() || self.is_done() {
925942
break;
926943
}
927944
}
@@ -951,6 +968,7 @@ impl<'d, Dm: DriverMode> DpiBounceTransfer<'d, Dm> {
951968
}
952969
}
953970

971+
#[cfg(feature = "unstable")]
954972
impl<Dm: DriverMode> Drop for DpiBounceTransfer<'_, Dm> {
955973
fn drop(&mut self) {
956974
self.stop_peripheral();

0 commit comments

Comments
 (0)