omnibus commit. hid stats. dac trait improvements. refactor to state machine style.

Cargo.lock

@@ -2,6 +2,18 @@
 # It is not intended for manual editing.
 version = 4
 
+[[package]]
+name = "ahash"
+version = "0.8.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75"
+dependencies = [
+ "cfg-if",
+ "once_cell",
+ "version_check",
+ "zerocopy",
+]
+
 [[package]]
 name = "aho-corasick"
 version = "1.1.4"
@@ -11,6 +23,15 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "atomic"
+version = "0.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a89cbf775b137e9b968e67227ef7f775587cde3fd31b0d8599dbd0f598a48340"
+dependencies = [
+ "bytemuck",
+]
+
 [[package]]
 name = "atomic-polyfill"
 version = "1.0.3"
@@ -52,6 +73,12 @@ version = "0.13.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "46afbd2983a5d5a7bd740ccb198caf5b82f45c40c09c0eed36052d91cb92e719"
 
+[[package]]
+name = "bitfield"
+version = "0.14.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2d7e60934ceec538daadb9d8432424ed043a904d8e0243f3c6446bce549a46ac"
+
 [[package]]
 name = "bitflags"
 version = "1.3.2"
@@ -67,6 +94,26 @@ dependencies = [
  "generic-array 0.14.7",
 ]
 
+[[package]]
+name = "bytemuck"
+version = "1.25.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec"
+dependencies = [
+ "bytemuck_derive",
+]
+
+[[package]]
+name = "bytemuck_derive"
+version = "1.10.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f9abbd1bc6865053c427f7198e6af43bfdedc55ab791faed4fbd361d789575ff"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "byteorder"
 version = "1.5.0"
@@ -132,7 +179,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "8ec610d8f49840a5b376c69663b6369e71f4b34484b9b2eb29fb918d92516cb9"
 dependencies = [
  "bare-metal",
- "bitfield",
+ "bitfield 0.13.2",
  "critical-section",
  "embedded-hal 0.2.7",
  "volatile-register",
@@ -311,7 +358,9 @@ dependencies = [
 name = "guac"
 version = "0.1.0"
 dependencies = [
+ "atomic",
  "bbqueue",
+ "bytemuck",
  "cortex-m",
  "cortex-m-rt",
  "defmt 1.0.1",
@@ -325,6 +374,7 @@ dependencies = [
  "panic-probe",
  "static_cell",
  "usb-device",
+ "usbd-hid",
  "usbd-uac2",
 ]
 
@@ -346,6 +396,15 @@ dependencies = [
  "byteorder",
 ]
 
+[[package]]
+name = "hashbrown"
+version = "0.13.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "43a3c133739dddd0d2990f9a4bdf8eb4b21ef50e4851ca85ab661199821d510e"
+dependencies = [
+ "ahash",
+]
+
 [[package]]
 name = "heapless"
 version = "0.7.17"
@@ -783,6 +842,35 @@ version = "0.7.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3"
 
+[[package]]
+name = "serde"
+version = "1.0.228"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e"
+dependencies = [
+ "serde_core",
+]
+
+[[package]]
+name = "serde_core"
+version = "1.0.228"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad"
+dependencies = [
+ "serde_derive",
+]
+
+[[package]]
+name = "serde_derive"
+version = "1.0.228"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "sharded-slab"
 version = "0.1.7"
@@ -958,6 +1046,41 @@ dependencies = [
  "portable-atomic",
 ]
 
+[[package]]
+name = "usbd-hid"
+version = "0.10.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "68beab087e4971a2fe76f631478b0e91d39593f58efd2775026ce6dc07a7bac6"
+dependencies = [
+ "usb-device",
+ "usbd-hid-macros",
+]
+
+[[package]]
+name = "usbd-hid-descriptors"
+version = "0.10.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b297f021719c4308d5d0c61b6c1e7c6b3ba383deba774b49aa5484f996bdb8f1"
+dependencies = [
+ "bitfield 0.14.0",
+]
+
+[[package]]
+name = "usbd-hid-macros"
+version = "0.10.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "011a3219e0933f5b3ad7dc90d9a66541a967d084c98c067deed1cd608e557ed7"
+dependencies = [
+ "byteorder",
+ "hashbrown",
+ "log",
+ "proc-macro2",
+ "quote",
+ "serde",
+ "syn",
+ "usbd-hid-descriptors",
+]
+
 [[package]]
 name = "usbd-uac2"
 version = "0.1.0"
@@ -1026,3 +1149,23 @@ checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc"
 dependencies = [
  "windows-link",
 ]
+
+[[package]]
+name = "zerocopy"
+version = "0.8.48"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "eed437bf9d6692032087e337407a86f04cd8d6a16a37199ed57949d415bd68e9"
+dependencies = [
+ "zerocopy-derive",
+]
+
+[[package]]
+name = "zerocopy-derive"
+version = "0.8.48"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "70e3cd084b1788766f53af483dd21f93881ff30d7320490ec3ef7526d203bad4"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]

Cargo.toml

@@ -4,13 +4,16 @@ version = "0.1.0"
 edition = "2024"
 
 [features]
-default = ["cs4398", "hid"]
+default = ["nodac", "hid"]
 ak4490 = []
 cs4398 = []
-hid = []
+nodac = []
+hid = [ "dep:usbd-hid"]
 
 [dependencies]
+atomic = "0.6.1"
 bbqueue = "0.7.0"
+bytemuck = { version = "1.25.0", features = ["derive"] }
 cortex-m = { version = "0.7.7", features = ["critical-section-single-core"] }
 cortex-m-rt = "0.7.5"
 defmt = "1.0.1"
@@ -24,6 +27,7 @@ panic-halt = "1.0.0"
 panic-probe = { version = "1.0.0", features = ["print-defmt"] }
 static_cell = "2.1.1"
 usb-device = "0.3"
+usbd-hid = { version = "0.10.0", optional = true }
 usbd-uac2 = { version = "0.1.0", path = "../usbd_uac2", features = ["defmt"]}
 
 [profile.release]

scripts/guac-stats.py

@@ -0,0 +1,39 @@
+from dataclasses import dataclass
+import struct
+from time import sleep
+from typing import Self
+import hid
+
+VID = 0x1209
+PID = 0xCC1D
+INTERVAL = 0.1
+
+
+@dataclass
+class AudioTelemetry:
+    STRUCT = "<LLLLL"
+    LEN = struct.calcsize(STRUCT)
+
+    average_buffer_fill: int
+    frame_count: int
+    dac_underflow_count: int
+    usb_underflow_count: int
+    dac_overflow_count: int
+
+    def from_bytes(b: bytes) -> Self:
+        if len(b) != AudioTelemetry.LEN:
+            raise ValueError(f"wrong size report ({len(b)} != {AudioTelemetry.LEN})")
+        fields = struct.unpack(AudioTelemetry.STRUCT, b)
+        return AudioTelemetry(*fields)
+
+
+def main():
+    with hid.Device(VID, PID) as h:
+        while True:
+            report = AudioTelemetry.from_bytes(h.read(AudioTelemetry.LEN))
+            print(f"{report}")
+            sleep(INTERVAL)
+
+
+if __name__ == "__main__":
+    main()

scripts/pyproject.toml

@@ -0,0 +1,10 @@
+[project]
+name = "guac-scripts"
+version = "0.1.0"
+description = "Scripts to work with GUAC devices"
+readme = "README.md"
+requires-python = ">=3.12"
+dependencies = [
+    "hid>=1.0.9",
+    "rich-click>=1.9.7",
+]

src/dac/cs4398.rs

@@ -56,12 +56,18 @@ where
         self.pins.reset.set_high().ok();
         // power up, enable control port
         self.write_reg(RegisterAddress::MiscControl, 1 << 6);
+        self.mute();
         // set audio protocol to I2S, Single rate mode
         self.write_reg(RegisterAddress::ModeControl, 1 << 4);
-        self.pins.reset.set_high().ok();
     }
     fn change_rate(&mut self, new_rate: u32) {
         let mode_control = (1 << 4) | self.fm_for_rate(new_rate);
         self.write_reg(RegisterAddress::ModeControl, mode_control);
     }
+    fn mute(&mut self) {
+        self.write_reg(RegisterAddress::MuteControl, 0xc0 | (0b11 << 3));
+    }
+    fn unmute(&mut self) {
+        self.write_reg(RegisterAddress::MuteControl, 0xc0);
+    }
 }

src/dac/noop.rs

@@ -0,0 +1,18 @@
+use core::marker::PhantomData;
+
+use crate::traits::Dac;
+
+/// Noop DAC for debugging on the EVK without a DAC connected
+pub struct NoopDac<T> {
+    __: PhantomData<T>,
+}
+impl<T> Dac<T> for NoopDac<T> {
+    fn init(&mut self) {}
+    fn change_rate(&mut self, _new_rate: u32) {}
+    fn mute(&mut self) {}
+    fn new(_i2c: T, _pins: crate::CodecPins) -> Self {
+        Self { __: PhantomData }
+    }
+    fn set_volume(&mut self, _left: u8, _right: u8) {}
+    fn unmute(&mut self) {}
+}

src/hid.rs

@@ -0,0 +1,20 @@
+use usbd_hid::descriptor::generator_prelude::*;
+
+#[gen_hid_descriptor(
+    (collection = APPLICATION, usage_page = VENDOR_DEFINED_START, usage = 0x01, ) = {
+        average_buffer_fill=input;
+        frame_count=input;
+        dac_underflow_count=input;
+        usb_underflow_count=input;
+        dac_overflow_count=input;
+    }
+)]
+#[repr(C)]
+// Note these are all actually u32
+pub struct AudioTelemetryReport {
+    pub average_buffer_fill: i32,
+    pub frame_count: i32,
+    pub dac_underflow_count: i32,
+    pub usb_underflow_count: i32,
+    pub dac_overflow_count: i32,
+}

src/hw.rs

@@ -1,3 +1,6 @@
+use crate::pac;
+use defmt::debug;
+
 pub(crate) struct PllConstants {
     pub m: u16,   // 1-65535
     pub n: u8,    // 1-255
@@ -54,3 +57,58 @@ impl defmt::Format for PllConstants {
         );
     }
 }
+
+const SYS_PLL: PllConstants = PllConstants::new(4, 75, 1); // 150MHz
+
+pub(crate) fn init_sys_pll1() {
+    let syscon = unsafe { &*pac::SYSCON::ptr() };
+    let pmc = unsafe { &*pac::PMC::ptr() };
+    let anactrl = unsafe { &*pac::ANACTRL::ptr() };
+
+    debug!("start clk_in");
+    pmc.pdruncfg0
+        .modify(|_, w| w.pden_xtal32m().poweredon().pden_ldoxo32m().poweredon());
+    syscon.clock_ctrl.modify(|_, w| w.clkin_ena().enable());
+    anactrl
+        .xo32m_ctrl
+        .modify(|_, w| w.enable_system_clk_out().enable());
+
+    debug!("init pll1: {}", SYS_PLL);
+    pmc.pdruncfg0.modify(|_, w| w.pden_pll1().poweredoff());
+    syscon.pll1clksel.write(|w| w.sel().enum_0x1()); // clk_in
+    syscon.pll1ctrl.write(|w| unsafe {
+        w.clken()
+            .enable()
+            .seli()
+            .bits(SYS_PLL.seli)
+            .selp()
+            .bits(SYS_PLL.selp)
+    });
+
+    syscon
+        .pll1ndec
+        .write(|w| unsafe { w.ndiv().bits(SYS_PLL.n) });
+    syscon.pll1ndec.write(|w| unsafe {
+        w.ndiv().bits(SYS_PLL.n).nreq().set_bit() // latch
+    });
+    syscon
+        .pll1mdec
+        .write(|w| unsafe { w.mdiv().bits(SYS_PLL.m) });
+    syscon
+        .pll1pdec
+        .write(|w| unsafe { w.pdiv().bits(SYS_PLL.p) });
+    syscon.pll1pdec.write(|w| unsafe {
+        w.pdiv().bits(SYS_PLL.p).preq().set_bit() // latch
+    });
+
+    pmc.pdruncfg0.modify(|_, w| w.pden_pll1().poweredon());
+    debug!("pll1 wait for lock");
+    let mut i = 0usize;
+    while syscon.pll1stat.read().lock().bit_is_clear() {
+        i += 1;
+    }
+    debug!("pll1 locked after {} tries", i);
+    // switch system clock to pll1
+    syscon.fmccr.modify(|_, w| w.flashtim().flashtim11());
+    syscon.mainclkselb.modify(|_, w| w.sel().enum_0x2()); // pll1
+}

src/main.rs

@@ -7,9 +7,12 @@ fn panic() -> ! {
     panic_probe::hard_fault()
 }
 
+use atomic::Atomic;
 use bbqueue::nicknames::Churrasco;
 use bbqueue::prod_cons::stream::{StreamConsumer, StreamProducer};
 use bbqueue::traits::bbqhdl::BbqHandle;
+use bbqueue::traits::coordination::ReadGrantError;
+use bytemuck::NoUninit;
 use core::sync::atomic::{AtomicBool, AtomicI32, AtomicU32, AtomicUsize, Ordering};
 use cortex_m_rt::entry;
 use defmt;
@@ -20,15 +23,19 @@ use hal::Syscon;
 use hal::drivers::{Timer, UsbBus, pins, pins::direction::Output};
 use hal::prelude::*;
 use hal::raw as pac;
-use hal::time::Hertz;
+use hal::time::{Hertz, Microseconds};
 use hal::typestates::pin::state::Gpio;
 use lpc55_hal as hal;
+use lpc55_hal::raw::NVIC;
+use lpc55_hal::raw::sdif::FIFO;
 use pac::interrupt;
 use usb_device::{
     bus::{self},
     device::{StringDescriptors, UsbDeviceBuilder, UsbVidPid},
     endpoint::IsochronousSynchronizationType,
 };
+#[cfg(feature = "hid")]
+use usbd_hid::{descriptor::SerializedDescriptor, hid_class::HIDClass};
 use usbd_uac2::UsbIsochronousFeedback;
 use usbd_uac2::{
     self, AudioClassConfig, RangeEntry, TerminalConfig, UsbAudioClass, UsbAudioClockImpl, UsbSpeed,
@@ -37,6 +44,7 @@ use usbd_uac2::{
 };
 
 use crate::dac::DacImpl;
+use crate::hid::AudioTelemetryReport;
 use crate::traits::Dac;
 
 #[cfg(feature = "ak4490")]
@@ -49,16 +57,27 @@ pub mod dac {
     mod cs4398;
     pub use self::cs4398::Cs4398Dac as DacImpl;
 }
+#[cfg(feature = "nodac")]
+pub mod dac {
+    mod noop;
+    pub use self::noop::NoopDac as DacImpl;
+}
 
+#[cfg(feature = "hid")]
+mod hid;
+mod hw;
 mod traits;
 
 // Fo = M/(N*2*P) * Fin
 // Fo = 3072/(125*2*8) * 16MHz = 24.576MHz
 //
 const FIFO_LENGTH: usize = 256; // frames
+const QUEUE_RUNNING_UP: usize = (FIFO_LENGTH * 4) / 10; // 40%
+const QUEUE_RUNNING_DOWN: usize = (FIFO_LENGTH * 2) / 10; // 20%
+const NODATA_TIMEOUT_FRAMES: usize = SAMPLE_RATE as usize / 100; // ~100ms
 const MCLK_FREQ: u32 = 24576000;
 const SAMPLE_RATE: u32 = 88200;
-type SampleType = (i32, i32);
+const HID_INTERVAL_MS: u8 = 100;
 
 struct CodecPins {
     reset: Pin<pins::Pio0_3, Gpio<Output>>,
@@ -147,6 +166,15 @@ impl PerfCounters {
         self.queue_overflows.store(0, Ordering::Relaxed);
         self.audio_underflows.store(0, Ordering::Relaxed);
     }
+    fn build_report(&self) -> AudioTelemetryReport {
+        AudioTelemetryReport {
+            average_buffer_fill: self.avg_fill.load(Ordering::Relaxed) as i32,
+            frame_count: self.played_frames.load(Ordering::Relaxed) as i32,
+            dac_underflow_count: self.audio_underflows.load(Ordering::Relaxed) as i32,
+            usb_underflow_count: self.queue_underflows.load(Ordering::Relaxed) as i32,
+            dac_overflow_count: self.queue_overflows.load(Ordering::Relaxed) as i32,
+        }
+    }
 }
 
 impl defmt::Format for PerfCounters {
@@ -174,8 +202,8 @@ static PRODUCED: AtomicU32 = AtomicU32::new(0);
 static CONSUMED: AtomicU32 = AtomicU32::new(0);
 
 static PERF: PerfCounters = PerfCounters {
-    received_frames: AtomicUsize::new(0),
+    received_frames: AtomicUsize::new(0),    // received from USB
-    played_frames: AtomicUsize::new(0),
+    played_frames: AtomicUsize::new(0),      // played audio frames
     min_fill: AtomicUsize::new(FIFO_LENGTH), // not recording this for now, need to figure out how to make it meaningful, since the queue starts empty
     avg_fill: AtomicUsize::new(FIFO_LENGTH / 2),
     queue_underflows: AtomicUsize::new(0), // ditto here, since we underflow at startup, but we record this one as it can be trended
@@ -195,7 +223,9 @@ fn try_write_one_frame<T: BbqHandle>(
     cons: &mut StreamConsumer<T>,
     i2s: &pac::i2s7::RegisterBlock,
 ) -> bool {
-    if let Ok(rgr) = cons.read() {
+    match cons.read() {
+        Ok(rgr) => {
+            // TODO: Fix this to handle the case where frame lands on a ring buffer boundary (if it is possible)
             if rgr.len() >= BYTES_PER_FRAME {
                 let l = u32::from_le_bytes(rgr[0..4].try_into().unwrap());
                 let r = u32::from_le_bytes(rgr[4..8].try_into().unwrap());
@@ -213,14 +243,23 @@ fn try_write_one_frame<T: BbqHandle>(
                 return false;
             }
         }
+        Err(ReadGrantError::Empty) => {
+            return false;
+        }
+        Err(e) => {
+            defmt::error!("Unexpected queue read error")
+        }
+    }
     false
 }
 
 #[interrupt]
 fn FLEXCOMM7() {
     let i2s = unsafe { &*pac::I2S7::ptr() };
+    defmt::info!("isr");
 
     if i2s.fifostat.read().txlvl().bits() == 0 {
+        // ISR was not serviced before the FIFO drained
         PERF.audio_underflows.fetch_add(1, Ordering::Relaxed);
     }
 
@@ -228,7 +267,8 @@ fn FLEXCOMM7() {
     let mut cons = QUEUE.stream_consumer();
     while i2s.fifostat.read().txlvl().bits() <= 6 {
         if !try_write_one_frame(&mut cons, i2s) {
-            // No complete frame available: write silence to keep FIFO above threshold
+            // No complete frame available: write silence to keep FIFO above threshold or we will
+            // get stuck in the ISR.
             PERF.queue_underflows.fetch_add(1, Ordering::Relaxed);
             i2s.fifowr.write(|w| unsafe { w.bits(0) });
             i2s.fifowr.write(|w| unsafe { w.bits(0) });
@@ -236,25 +276,193 @@ fn FLEXCOMM7() {
         }
     }
 }
+#[repr(u8)]
+#[derive(Clone, Copy, NoUninit)]
+enum AudioState {
+    /// Knowingly stopped, ie. AltSetting=0. DAC muted, I2S disabled.
+    ///  
+    ///  AltSetting = 1 -> ARMED
+    Stopped,
+    /// Waiting for data. DAC muted, I2S running sending 0s (FIFO not serviced).
+    ///
+    ///  USB OUT data packet -> ARMED
+    ///  AltSetting = 0 -> STOPPED
+    Armed,
+    /// Filling the buffer before playback starts. Feedback does not run,
+    /// playout does not start draining the queue. Gets us better feedback
+    /// behaviour and a full buffer without a feedback rate spike at startup.
+    ///
+    ///  queue reaches <QUEUE_RUNNING_UP> -> RUNNING
+    ///  AltSetting = 0 -> STOPPED
+    ///  
+    Prefill,
+    /// Normal running state. Start servicing FIFO and begin playing out from the buffer.
+    ///
+    ///  queue reaches <QUEUE_RUNNING_DOWN> -> DRAINING
+    ///  AltSetting = 0 -> DRAINING
+    Running,
+    /// The queue is low. We will continue playout.
+    ///
+    ///  queue is empty && altSetting 1 -> NODATA
+    ///  queue is empty && altSetting 0 -> STOPPED
+    ///  queue reaches <QUEUE_RUNNING_UP> && altSetting 1 -> RUNNING
+    LowData,
+    /// There is no data in the queue. We will count underflows for a while, send 0s, and hope the host comes back, but maybe playback is done, which we should notice and shut down.
+    ///
+    ///  countdown reaches DATA_TIMEOUT -> STOPPED
+    ///  AltSetting = 0 -> STOPPED
+    NoData,
+}
+impl defmt::Format for AudioState {
+    fn format(&self, fmt: defmt::Formatter) {
+        defmt::write!(
+            fmt,
+            "{}",
+            match self {
+                Self::Stopped => "Stopped",
+                Self::Armed => "Armed",
+                Self::Prefill => "Prefill",
+                Self::Running => "Running",
+                Self::LowData => "Draining",
+                Self::NoData => "NoData",
+            }
+        )
+    }
+}
+
+struct FeedbackState {
+    correction_enabled: AtomicBool,
+    integrator: AtomicI32,
+    filtered_fill: AtomicI32,
+}
+impl FeedbackState {
+    fn start(&mut self) {
+        self.correction_enabled.store(true, Ordering::Relaxed);
+    }
+    fn reset(&mut self) {
+        self.correction_enabled.store(false, Ordering::Relaxed);
+        self.integrator.store(0, Ordering::Relaxed);
+        self.filtered_fill
+            .store(FIFO_LENGTH as i32 / 2, Ordering::Relaxed);
+    }
+}
+impl Default for FeedbackState {
+    fn default() -> Self {
+        Self {
+            correction_enabled: AtomicBool::new(false),
+            integrator: AtomicI32::new(0),
+            filtered_fill: AtomicI32::new(FIFO_LENGTH as i32 / 2),
+        }
+    }
+}
 
 struct Audio<T: BbqHandle, D: Dac<I>, I> {
-    running: AtomicBool,
+    state: Atomic<AudioState>,
+    alt_setting: u8,
     i2s: I2sTx,
     dac: D,
     producer: StreamProducer<T>,
-    integrator: AtomicI32,
+    fb: FeedbackState,
-    filtered_fill: AtomicI32,
+    nodata_timeout_frame: AtomicUsize,
     _marker: core::marker::PhantomData<I>,
 }
 impl<T: BbqHandle, D: Dac<I>, I> Audio<T, D, I> {
+    /// Perform a state transition to `state`
+    fn transition(&mut self, state: AudioState) {
+        defmt::info!(
+            "AudioState {} -> {}",
+            self.state.load(Ordering::Relaxed),
+            state
+        );
+        match state {
+            AudioState::Stopped => self.stop(),
+            AudioState::Armed => self.arm(),
+            AudioState::Prefill => self.prefill(),
+            AudioState::Running => self.run(),
+            AudioState::LowData => {}
+            AudioState::NoData => self.nodata(),
+        }
+        self.state.store(state, Ordering::SeqCst);
+    }
+
     fn init(&mut self) {
+        let regs = &self.i2s.i2s;
+        // Enable TX FIFO only
+        regs.fifocfg.modify(|_, w| {
+            w.enabletx()
+                .enabled()
+                .enablerx()
+                .disabled()
+                .dmatx()
+                .disabled()
+                .txi2se0()
+                .zero()
+        });
+
+        // Flush
+        regs.fifocfg.modify(|_, w| w.emptytx().set_bit());
+
+        regs.cfg2
+            .modify(|_, w| unsafe { w.position().bits(0).framelen().bits(63) }); // framelen = 64
+
+        let bclk_div = (MCLK_FREQ / SAMPLE_RATE / 64) as u16;
+        regs.div
+            .modify(|_, w| unsafe { w.div().bits(bclk_div - 1) }); // Clock source is MCLK (12.288MHz) / 4 = 3MHz
+
+        // Config
+        regs.cfg1.modify(|_, w| unsafe {
+            w.mstslvcfg()
+                .normal_master()
+                .onechannel()
+                .dual_channel()
+                .datalen()
+                .bits(31)
+                .mainenable()
+                .disabled()
+                .mode()
+                .classic_mode()
+                .datapause()
+                .normal()
+        });
         self.dac.init();
         self.dac.change_rate(SAMPLE_RATE);
     }
-    fn start(&self) {
+    ///Transition -> Stopped:
-        self.running.store(true, Ordering::Relaxed);
+    ///clear queue, mute DAC, mask I2S ISR, stop I2S peripheral, disable & reset feedback and performance queues
-        defmt::info!("playback starting, enabling interrupts");
+    fn stop(&mut self) {
+        self.dac.mute();
+        // Disable level interrupt on I2S
         self.i2s.i2s.fifointenclr.write(|w| w.txlvl().set_bit());
+        // Clear any samples in the FIFO
+        self.i2s.i2s.fifocfg.modify(|_, w| w.emptytx().set_bit());
+        // Disable I2S
+        self.i2s.i2s.cfg1.modify(|_, w| w.mainenable().disabled());
+        // Reset feedback state
+        self.fb.reset();
+        // reset performance counters
+        PERF.reset();
+        // Drain anything left in the queue
+        while let Ok(d) = QUEUE.stream_consumer().read() {
+            let len = d.len();
+            d.release(len)
+        }
+    }
+    ///Transition -> Armed
+    /// Start I2S peripheral. Since we assume we have interrupts disabled at
+    /// this point (as we came from Stopped), and the FIFO is empty, this will
+    /// play out 0s.
+    fn arm(&mut self) {
+        self.i2s.i2s.cfg1.modify(|_, w| w.mainenable().enabled());
+    }
+    ///Transition -> Prefill
+    /// Unmute DAC
+    fn prefill(&mut self) {
+        self.dac.unmute();
+    }
+    ///Transition -> Running
+    ///Unmask I2S ISR, start feedback
+    fn run(&mut self) {
+        self.fb.start();
         // FIFO threshold trigger enable
         self.i2s
             .i2s
@@ -262,36 +470,47 @@ impl<T: BbqHandle, D: Dac<I>, I> Audio<T, D, I> {
             .modify(|_, w| unsafe { w.txlvl().bits(6).txlvlena().enabled() });
         // FIFO level interrupt enable
         self.i2s.i2s.fifointenset.modify(|_, w| w.txlvl().enabled());
-        unsafe { pac::NVIC::unmask(pac::Interrupt::FLEXCOMM7) };
     }
-    fn stop(&self) {
+    ///Transition->NoData
-        self.running.store(true, Ordering::Relaxed);
+    ///store framecount at transition so we can time out recovery
-        defmt::info!("playback stopped: {}", PERF);
+    fn nodata(&mut self) {
-        PERF.reset();
+        self.nodata_timeout_frame.store(
-        pac::NVIC::mask(pac::Interrupt::FLEXCOMM7);
+            PERF.queue_underflows.load(Ordering::Relaxed) + NODATA_TIMEOUT_FRAMES, // we underflow every frame, use it as a timeout counter
+            Ordering::Relaxed,
+        );
     }
 }
 impl<T: BbqHandle, D: Dac<I>, I, B: bus::UsbBus> UsbAudioClass<'_, B> for Audio<T, D, I> {
     fn alternate_setting_changed(&mut self, _terminal: usb_device::UsbDirection, alt_setting: u8) {
-        match alt_setting {
+        let state = self.state.load(Ordering::Relaxed);
-            0 => self.stop(),
+        match (alt_setting, state) {
-            1 => self.start(),
+            (0, AudioState::Armed | AudioState::Prefill | AudioState::NoData) => {
-            _ => defmt::error!("unexpected alt setting {}", alt_setting),
+                self.transition(AudioState::Stopped)
             }
+            (0, AudioState::Running) => {} // noop, we naturally transition through LowData to Stopped
+            (1, AudioState::Stopped) => self.transition(AudioState::Armed),
+            (1, _) => {} // altSetting 1 in any other state is a no-op
+            (_, _) => {
+                defmt::error!("Invalid alt setting {}", alt_setting)
+            }
+        }
+        self.alt_setting = alt_setting;
     }
     fn audio_data_rx(
         &mut self,
         ep: &usb_device::endpoint::Endpoint<'_, B, usb_device::endpoint::Out>,
     ) {
+        let state = self.state.load(Ordering::Relaxed);
         let mut buf = [0; SAMPLE_RATE as usize / 1000 * 64];
         let len = match ep.read(&mut buf) {
             Ok(len) => len,
             Err(e) => {
-                defmt::error!("usb error in rx callback");
+                defmt::error!("usb error in rx callback {:?}", e);
                 return;
             }
         };
         let buf = &buf[..len];
+
         if let Ok(mut wg) = self.producer.grant_exact(buf.len()) {
             wg.copy_from_slice(buf);
             wg.commit(buf.len());
@@ -300,16 +519,47 @@ impl<T: BbqHandle, D: Dac<I>, I, B: bus::UsbBus> UsbAudioClass<'_, B> for Audio<
                 .fetch_add(buf.len() / BYTES_PER_FRAME, Ordering::Relaxed);
         } else {
             PERF.queue_overflows.fetch_add(1, Ordering::Relaxed);
-            defmt::error!("overflowed bbq, asked {}", buf.len());
+            // defmt::error!("overflowed bbq, asked {}", buf.len());
+        }
+
+        // Valid states here are Armed, Prefill, Running, Draining and NoData
+        match state {
+            AudioState::Stopped => {
+                defmt::error!("Received audio data when stopped")
+            }
+            // When armed, data rx goes to prefill
+            AudioState::Armed => self.transition(AudioState::Prefill),
+            // When prefilling, if we have received frames over the up threshold, move to running
+            AudioState::Prefill => {
+                if PERF.received_frames.load(Ordering::Relaxed) >= QUEUE_RUNNING_UP {
+                    self.transition(AudioState::Running);
+                }
+            }
+            // When running, USB RX is a no-op
+            AudioState::Running => {}
+            // If draining, check cur_fill, if it rises above QUEUE_RUNNING_UP, move back to running. If it drops to 0, move to NoData or Stopped
+            AudioState::LowData => {
+                let fill = cur_fill() as usize;
+                // Do we check alt setting here? We shouldn't be receiving data at all if we are not in altSetting 1
+                if fill >= QUEUE_RUNNING_UP {
+                    self.transition(AudioState::Running);
+                } else if fill == 0 && self.alt_setting == 0 {
+                    self.transition(AudioState::Stopped);
+                } else if fill == 0 {
+                    self.transition(AudioState::NoData);
+                }
+            }
+            // Any data in NoData moves us into LowData. But maybe it should be more like prefill?
+            AudioState::NoData => self.transition(AudioState::LowData),
         }
     }
     fn feedback(&mut self, nominal_rate: UsbIsochronousFeedback) -> Option<UsbIsochronousFeedback> {
         let target = FIFO_LENGTH as i32 / 2 - nominal_rate.int as i32;
 
         let fill = cur_fill() as i32;
-        let prev = self.filtered_fill.load(Ordering::Relaxed);
+        let prev = self.fb.filtered_fill.load(Ordering::Relaxed);
         let filtered = prev + ((fill - prev) >> 4); // ~1/16 smoothing
-        self.filtered_fill.store(filtered, Ordering::Relaxed);
+        self.fb.filtered_fill.store(filtered, Ordering::Relaxed);
 
         let error = filtered - target;
 
@@ -318,17 +568,17 @@ impl<T: BbqHandle, D: Dac<I>, I, B: bus::UsbBus> UsbAudioClass<'_, B> for Audio<
 
         // Reset integrator when the error is small
         if error.abs() < 2 {
-            self.integrator.store(0, Ordering::Relaxed);
+            self.fb.integrator.store(0, Ordering::Relaxed);
         }
-        let mut integrator = self.integrator.load(Ordering::Relaxed);
+        let mut integrator = self.fb.integrator.load(Ordering::Relaxed);
         integrator = integrator - (integrator >> 6); // ~1/64 leak, reduce windup
 
         integrator = integrator.clamp(-256, 256);
-        self.integrator.store(integrator, Ordering::Relaxed);
+        self.fb.integrator.store(integrator, Ordering::Relaxed);
 
         // gains
         let p = error << 3;
-        let i = integrator * 0; // disabled for now
+        let i = integrator << 2;
 
         let correction = -((p + i) >> 2);
         let nominal_v = nominal_rate.to_u32_12_13() as i32;
@@ -390,46 +640,9 @@ pub fn init_i2s(mut fc7: pac::FLEXCOMM7, i2s7: pac::I2S7, syscon: &mut Syscon) -
     // Select I2S TX function
     fc7.pselid.write(|w| w.persel().i2s_transmit());
 
+    unsafe { NVIC::unmask(interrupt::FLEXCOMM7) }
+
     let regs = i2s7;
-
-    // Enable TX FIFO only
-    regs.fifocfg.modify(|_, w| {
-        w.enabletx()
-            .enabled()
-            .enablerx()
-            .disabled()
-            .dmatx()
-            .disabled()
-            .txi2se0()
-            .zero()
-    });
-
-    // Flush
-    regs.fifocfg.modify(|_, w| w.emptytx().set_bit());
-
-    regs.cfg2
-        .modify(|_, w| unsafe { w.position().bits(0).framelen().bits(63) }); // framelen = 64
-
-    let bclk_div = (MCLK_FREQ / SAMPLE_RATE / 64) as u16;
-    regs.div
-        .modify(|_, w| unsafe { w.div().bits(bclk_div - 1) }); // Clock source is MCLK (12.288MHz) / 4 = 3MHz
-
-    // Config
-    regs.cfg1.modify(|_, w| unsafe {
-        w.mstslvcfg()
-            .normal_master()
-            .onechannel()
-            .dual_channel()
-            .datalen()
-            .bits(31)
-            .mainenable()
-            .enabled()
-            .mode()
-            .classic_mode()
-            .datapause()
-            .normal()
-    });
-
     I2sTx { i2s: regs }
 }
 
@@ -493,6 +706,7 @@ fn main() -> ! {
         .system_frequency(96.MHz())
         .configure(&mut anactrl, &mut pmc, &mut syscon)
         .unwrap();
+    hw::init_sys_pll1();
     let mut delay_timer = Timer::new(
         hal.ctimer
             .0
@@ -528,12 +742,13 @@ fn main() -> ! {
 
     defmt::info!("audio init");
     let mut audio = Audio {
+        state: Atomic::new(AudioState::Stopped),
         i2s: i2s_peripheral,
         dac: dac_impl,
         producer: QUEUE.stream_producer(),
-        running: AtomicBool::new(false),
+        fb: FeedbackState::default(),
-        integrator: AtomicI32::new(0),
+        alt_setting: 0,
-        filtered_fill: AtomicI32::new(0),
+        nodata_timeout_frame: AtomicUsize::new(0),
         _marker: core::marker::PhantomData,
     };
     audio.init();
@@ -559,6 +774,8 @@ fn main() -> ! {
             None,
         ));
     let mut uac2 = config.build(&usb_bus).unwrap();
+    #[cfg(feature = "hid")]
+    let mut hid = HIDClass::new_ep_in(&usb_bus, AudioTelemetryReport::desc(), HID_INTERVAL_MS);
 
     let mut usb_dev = UsbDeviceBuilder::new(&usb_bus, UsbVidPid(0x1209, 0xcc1d))
         .composite_with_iads()
@@ -574,9 +791,38 @@ fn main() -> ! {
         .device_protocol(0x01)
         .build();
 
+    #[cfg(feature = "hid")]
+    let mut poll_all = {
+        let mut hid_update_timer = Timer::new(
+            hal.ctimer
+                .1
+                .enabled(&mut syscon, clocks.support_1mhz_fro_token().unwrap()),
+        );
+        hid_update_timer.start(Microseconds::new(HID_INTERVAL_MS as u32 * 1000));
+
+        move || {
+            let active = usb_dev.poll(&mut [&mut uac2, &mut hid]);
+            if active && hid_update_timer.wait().is_ok() {
+                let report = PERF.build_report();
+                match hid.push_input(&report) {
+                    Ok(_) => {}
+                    Err(UsbError::WouldBlock) => {}
+                    Err(e) => defmt::error!("Failed to send HID report: {:?}", e),
+                }
+                // lpc55 timer is not Periodic, so restart it
+                hid_update_timer.start(Microseconds::new(HID_INTERVAL_MS as u32 * 1000));
+            }
+        }
+    };
+    #[cfg(not(feature = "hid"))]
+    let poll_all = || {
+        usb_dev.poll(&mut [&mut uac2]);
+    };
+
     defmt::info!("main loop");
 
     loop {
-        usb_dev.poll(&mut [&mut uac2]);
+        poll_all();
+        // usb_dev.poll(&mut [&mut uac2]);
     }
 }

src/traits.rs

@@ -1,8 +1,11 @@
 use crate::CodecPins;
 pub trait Dac<T> {
     fn new(i2c: T, pins: CodecPins) -> Self;
+    /// The DAC should start muted
     fn init(&mut self);
     fn change_rate(&mut self, new_rate: u32);
     #[allow(unused_variables)]
     fn set_volume(&mut self, left: u8, right: u8) {}
+    fn mute(&mut self);
+    fn unmute(&mut self);
 }