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

Add SpiDmaBus::split for moving between manual & automatic DMA buffers #2824

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

i404788
Copy link
Contributor

@i404788 i404788 commented Dec 17, 2024

Description

This pull-request adds SpiDmaBus::deconstruct for moving between manual & automatic DMA buffers.
If an SpiDmaBus is made you are stuck with automatic buffer handling (unless drop, steal & re-init the SPI peripheral again).

Here is an example snippet (for the FT8xx series display driver series) of it's usage:

  pub fn new(inner: SpiDma<'static, Blocking, esp_hal::spi::AnySpi>) -> Self {
    let (_, tx_desc) = esp_hal::dma_descriptors_chunk_size!(1, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE);

    let tx_buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize);
    let display_buf = DmaTxBuf::new_with_config(tx_desc, tx_buffer, DMA_ALIGNMENT).unwrap();

    let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = esp_hal::dma_buffers!(128, 128);
    let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap();
    let dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap();

    Self {
      inner: Some(inner.with_buffers(dma_rx_buf, dma_tx_buf)),
      display_buf: Some(display_buf),
      size: embedded_graphics::prelude::Size::new(WIDTH as u32, HEIGHT as u32),
    }
  }
  pub fn after_render(&mut self) -> anyhow::Result<()> {
    let addr_pixels = 0 | (0b10 << 22);
    let addr_spi_mode = EngineRegister::SpiWidth.addr() | (0b10 << 22);
    let mode = esp_hal::spi::SpiDataMode::Quad;
    use esp_hal::spi::master::{Address, Command};
    let mut spidma = self.inner.take().unwrap();

    // Configure Quad SPI on slave
    spidma
      .half_duplex_write(
        esp_hal::spi::SpiDataMode::Single,
        Command::None,
        Address::Address24(addr_spi_mode, esp_hal::spi::SpiDataMode::Single),
        0u8,
        &[2u8],
      )
      .map_err(|e| anyhow::anyhow!("Failed to set spi width: {:?}", e))?;

    // NEW: deconstruct
    let (spidma, rx_buf, tx_buf) = spidma.split();

    // Transfer large DMA buffer (>=100kB)
    let transfer = spidma
      .half_duplex_write(
        mode,
        Command::None,
        Address::Address24(addr_pixels, mode),
        0u8,
        WIDTH * HEIGHT,
        self.display_buf.take().unwrap(),
      )
      .map_err(|e| anyhow::anyhow!("Could not fill screen: {:?}", e))?;

    let (spidma, display_buf) = transfer.wait();
    self.display_buf.replace(display_buf);

    // Re-use old dma bufs
    self.inner.replace(spidma.with_buffers(rx_buf, tx_buf));

    // Disable Quad SPI on slave
    self
      .inner
      .as_mut()
      .unwrap()
      .half_duplex_write(
        mode,
        Command::None,
        Address::Address24(addr_spi_mode, mode),
        0u8,
        &[0u8],
      )
      .map_err(|e| anyhow::anyhow!("Failed to reset spi width: {:?}", e))
}

Testing

Testing is still on-going but since SpiDmaBus is effectively a dumb-wrapper I don't expect any issues.

@Dominaezzz
Copy link
Collaborator

Change LGTM, I'm just gonna ask you to rename deconstruct to split.
We already use split for this sort of thing, like DmaTxBuf::split, DmaRxBuf::split

@bugadani
Copy link
Contributor

bugadani commented Dec 17, 2024

Using an SPI bus to drive a display is slightly problematic (you won't be able to share the bus with other devices) and you won't be able to do this with an SPI device, so I'm not sure what the point is.

@i404788
Copy link
Contributor Author

i404788 commented Dec 17, 2024

@bugadani The display chip does allow for CS/bus sharing, so I'm not entirely sure what you mean. It's not a problem in our case anyway, we have dedicated lines for the display since we need to the entire bandwidth.

Copy link
Collaborator

@Dominaezzz Dominaezzz left a comment

Choose a reason for hiding this comment

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

Missing flush

esp-hal/src/spi/master.rs Outdated Show resolved Hide resolved
esp-hal/src/spi/master.rs Show resolved Hide resolved
@Dominaezzz Dominaezzz dismissed their stale review December 17, 2024 14:19

flush added

@Dominaezzz Dominaezzz changed the title Add SpiDmaBus::deconstruct for moving between manual & automatic DMA buffers Add SpiDmaBus::split for moving between manual & automatic DMA buffers Dec 17, 2024
Copy link
Collaborator

@Dominaezzz Dominaezzz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

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.

3 participants