Skip to content

Add SAI/I2S audio support with Audio PLL and SGTL5000 examples#183

Closed
tacertain wants to merge 1 commit intomciantyre:masterfrom
tacertain:audio-support
Closed

Add SAI/I2S audio support with Audio PLL and SGTL5000 examples#183
tacertain wants to merge 1 commit intomciantyre:masterfrom
tacertain:audio-support

Conversation

@tacertain
Copy link
Copy Markdown
Contributor

Enable SAI1-3 clock gates, configure Audio PLL (PLL4) for 44.1 kHz sample rates, expose SAI peripherals in the BSP, and add two working audio examples for the Teensy 4.1 + Audio Shield Rev D (SGTL5000):

  • rtic_sai_poll_tone: interrupt-driven FIFO polling (no DMA)
  • rtic_sai_dma_tone: DMA-driven I2S transfers

Also updates dependencies to imxrt-hal v0.6 / imxrt-ral v0.6 and fixes LPSPI PCS0 removal and usb-device v0.3 API changes.

Enable SAI1-3 clock gates, configure Audio PLL (PLL4) for 44.1 kHz
sample rates, expose SAI peripherals in the BSP, and add two working
audio examples for the Teensy 4.1 + Audio Shield Rev D (SGTL5000):

- rtic_sai_poll_tone: interrupt-driven FIFO polling (no DMA)
- rtic_sai_dma_tone: DMA-driven I2S transfers

Also updates dependencies to imxrt-hal v0.6 / imxrt-ral v0.6 and
fixes LPSPI PCS0 removal and usb-device v0.3 API changes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tacertain
Copy link
Copy Markdown
Contributor Author

Here's what I did to enable the audio shield. Obviously, this isn't mergeable with the current Cargo.toml - just putting it out there to let you know the path I'm on

Copy link
Copy Markdown
Owner

@mciantyre mciantyre left a comment

Choose a reason for hiding this comment

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

First pass review. Any chance we could break out the SAI additions to the board? Those changes should be usable without the updated dependencies, and folks in #182 would benefit.

Comment on lines +101 to +112
// ── Static DMA buffer ────────────────────────────────────────────
// Place in OCRAM (.uninit region) which is non-cached and
// DMA-accessible. Using .uninit means the section is NOLOAD so it
// won't bloat the flash image / hex file. The buffer is filled
// before every DMA transfer, so uninitialised contents are fine.
#[link_section = ".uninit.dmabuffers"]
static mut DMA_BUF: core::mem::MaybeUninit<[u32; DMA_BUF_LEN]> =
core::mem::MaybeUninit::uninit();

// ── Init ─────────────────────────────────────────────────────────

#[init]
Copy link
Copy Markdown
Owner

@mciantyre mciantyre Mar 9, 2026

Choose a reason for hiding this comment

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

Suggested change
// ── Static DMA buffer ────────────────────────────────────────────
// Place in OCRAM (.uninit region) which is non-cached and
// DMA-accessible. Using .uninit means the section is NOLOAD so it
// won't bloat the flash image / hex file. The buffer is filled
// before every DMA transfer, so uninitialised contents are fine.
#[link_section = ".uninit.dmabuffers"]
static mut DMA_BUF: core::mem::MaybeUninit<[u32; DMA_BUF_LEN]> =
core::mem::MaybeUninit::uninit();
// ── Init ─────────────────────────────────────────────────────────
#[init]
#[init(local = [
dma_buf: [u32; DMA_BUF_LEN] = [0; DMA_BUF_LEN]
])]

RTIC's init can initialize large objects in global memory. We're provided a &'static mut [u32; _] which we can safely pass around through Shared or Local. Once implemented, I don't think we'll need any unsafe code to access the buffer.

Learn more here. You can also study the examples in this repository, like this one:

#[init(local = [
// This allocation is used for USB endpoint I/O, and it's shared
// across all USB endpoints.
ep_memory: EndpointMemory<1024> = EndpointMemory::new(),
// This allocation is for the endpoints themselves. It's sized for
// all possible endpoints, even if we're not using them in this
// example.
ep_state: EndpointState = EndpointState::max_endpoints(),
// And this is the USB bus itself. It's set to Some(...) below.
usb_bus: Option<UsbBusAllocator<BusAdapter>> = None,
])]
fn init(cx: init::Context) -> (Shared, Local) {

Comment on lines +127 to +130
unsafe {
let gpr = ral::iomuxc_gpr::IOMUXC_GPR::instance();
ral::modify_reg!(ral::iomuxc_gpr, gpr, GPR1, SAI1_MCLK_DIR: 1);
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The IOMUXC_GPR instance should be returned through board::Resources. I appreciate that we did that for the SAI instances. Keep following that pattern.

Comment on lines +254 to +255
channel::set_source_linear_buffer(&mut *dma_chan, buf);
dma_chan.set_transfer_iterations(DMA_BUF_LEN as u16);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

set_source_linear_buffer instructs the DMA channel to return to the buffer's start address once the transfer completes. See here. And typically, the transfer iterations are unchanged in the DMA channel, too.

Since hardware already achieves the two objectives of these calls, we could remove them. This makes the interrupt a littler faster, and the code a little smaller.

Suggested change
channel::set_source_linear_buffer(&mut *dma_chan, buf);
dma_chan.set_transfer_iterations(DMA_BUF_LEN as u16);

Comment thread src/board.rs
Comment on lines +292 to +299
/// The register block for SAI1 (I2S audio).
///
/// SAI1 is the primary audio interface used by the Teensy Audio Shield.
pub sai1: ral::sai::SAI1,
/// The register block for SAI2.
pub sai2: ral::sai::SAI2,
/// The register block for SAI3.
pub sai3: ral::sai::SAI3,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

#182 would appreciate if we broke this out into a separate commit, along with the SAI clock setup.

Comment thread src/clock_power.rs
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

See other comments regarding #182 support. This isolated change is worthy of its own commit.

Comment thread src/clock_power.rs
}

/// Configure SAI1 clock root to derive from Audio PLL.
fn setup_sai1_clk(ccm: &mut ral::ccm::CCM) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What about SAI2 and / or SAI3?

If my interpretation of the 4.0 pinout is correct, the official documentation is advertising, at least, SAI2 capabilities. It doesn't seem that SAI3 is usefully broken out, at least on the common pins.

The SAI root clocks are distinct, but it looks like we can mux them all the same. I'm looking at figure 14-2 in the reference manual (rev 3). Picking the same audio PLL would seem useful.

Comment thread Cargo.toml
Comment on lines +68 to +71
imxrt-hal = { path = "../imxrt-hal", features = ["imxrt1060"] }
imxrt-iomuxc = { version = "0.3.0", features = ["imxrt1060"] }
imxrt-log = { path = "../imxrt-hal/logging", default-features = false }
imxrt-ral = { path = "../imxrt-ral", features = ["imxrt1062"] }
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If the unreleased packages were expressed as [patch] directives pointing to git repositories, we might have an easier time getting this code building in CI / other folk's development environments. The change would look like

Suggested change
imxrt-hal = { path = "../imxrt-hal", features = ["imxrt1060"] }
imxrt-iomuxc = { version = "0.3.0", features = ["imxrt1060"] }
imxrt-log = { path = "../imxrt-hal/logging", default-features = false }
imxrt-ral = { path = "../imxrt-ral", features = ["imxrt1062"] }
imxrt-hal = { version = "0.6.0", features = ["imxrt1060"] }
imxrt-iomuxc = { version = "0.3.0", features = ["imxrt1060"] }
imxrt-log = { version = "0.2" }
imxrt-ral = { version = "0.6", features = ["imxrt1062"] }

with supplements like

[patch.crates-io.imxrt-ral]
git = "https://github.com/imxrt-rs/imxrt-ral"

[patch.crates-io.imxrt-iomuxc]
git = "https://github.com/imxrt-rs/imxrt-iomuxc"

# etc...

@tacertain
Copy link
Copy Markdown
Contributor Author

Thanks so much for all the thoughtful comments. I'll get on these. Top priority will be to get as much as I can committed without version changes.

@tacertain
Copy link
Copy Markdown
Contributor Author

New PRs incoming....

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.

2 participants