commit f343a21781eef32ddcde4089c331b5b67d1bbe61
author Sam Leffler <sleffler@google.com> Thu Aug 15 16:59:41 2024 -0700
committer Sam Leffler <sleffler@google.com> Fri Aug 16 21:05:04 2024 +0000
tree c8a844d3fd3690afd586603b4e57c6a2898604de
parent 697f3a66c06f2fd54f66113a9cb53f5f04d5b8ab

soundstream: fix i2s_wait_for_rx_watermark

Having multiple waiters on the RxWatermarkInterrupt futex only works
if the waker explicitly asks for multiple threads to be woken (the
expectation is MultiWaiter will be used in this case). This broke with
the tickless scheduler. Fix is to introduce a separate futex to wait
for rx_watermark_seen.

Bypass-Presubmit-Reason:collateral damage

Change-Id: I29194244b120b97a5cdae1581a70768575ce9329

sw/device/cheriot/soundstream/i2s.cc

diff --git a/sw/device/cheriot/soundstream/i2s.cc b/sw/device/cheriot/soundstream/i2s.cc
index e20358f..56d860b 100644
--- a/sw/device/cheriot/soundstream/i2s.cc
+++ b/sw/device/cheriot/soundstream/i2s.cc
@@ -54,8 +54,8 @@
   semaphore_put(&startup);
 }
 
-// NB: could be atomic but not needed for our usage.
 volatile bool rx_watermark_seen = false;
+uint32_t rx_watermark_seen_futex = 0;
 
 void i2s_isr(void) {
   Timeout t = {0, UnlimitedTimeout};
@@ -69,17 +69,18 @@
     Debug::Assert(futex_wait(rxWatermarkFutex, last) == 0, "futex_wait");
     last = *rxWatermarkFutex;
     Debug::log("i2s_isr: i2s {} last {})", i2s.base_addr.base, last);
-    rx_watermark_seen = true;
     // Acknowledge and disable the interrupt; it will be
     // re-enabled after the RX FIFO is drained.
     i2s_irq_acknowledge(kI2sIrqRxWatermark);
     i2s_irq_set_enabled(kI2sIrqRxWatermark, /*enabled=*/false);
     CHECK(interrupt_complete(
         STATIC_SEALED_VALUE(i2sRxWatermarkInterruptCapability)) == 0);
+
+    rx_watermark_seen = true;
+    futex_wake(&rx_watermark_seen_futex, 1);
   }
 }
 bool i2s_rx_watermark_seen(void) {
-  // return atomic_swap(&rx_watermark_seen, false);
   bool was_seen = rx_watermark_seen;
   if (was_seen) {
     rx_watermark_seen = false;
@@ -88,12 +89,13 @@
 }
 
 void i2s_wait_for_rx_watermark(void) {
-  const uint32_t *rxWatermarkFutex = interrupt_futex_get(
-      STATIC_SEALED_VALUE(i2sRxWatermarkInterruptCapability));
-  uint32_t last = *rxWatermarkFutex;
+  // NB: this is conservative; we only have one consumer so could
+  //   just do: if (!i2s_rx_watermark_seen()) futex_wait(...).
+  uint32_t last = rx_watermark_seen_futex;
   while (!i2s_rx_watermark_seen()) {
-    Debug::Assert(futex_wait(rxWatermarkFutex, last) == 0, "futex_wait");
-    last = *rxWatermarkFutex;
+    Debug::Assert(futex_wait(&rx_watermark_seen_futex, last) == 0,
+                  "futex_wait");
+    last = rx_watermark_seen_futex;
   }
 }