StopWatch: recover ES8311 capture after power-on reset

src/M5Unified.cpp

@@ -493,25 +493,61 @@ static constexpr const uint8_t _pin_table_mbus[][31] = {
   }
 #endif
 
-  static void in_i2c_bulk_write(const uint8_t i2c_addr, const uint8_t* bulk_data, const uint32_t i2c_freq = 100000u, const uint8_t retry = 0)
+  /// @return true when every write in the table was acknowledged.
+  static bool in_i2c_bulk_write(const uint8_t i2c_addr, const uint8_t* bulk_data, const uint32_t i2c_freq = 100000u, const uint8_t retry = 0)
   {
     // bulk_data example..
     // const uint8_t bulk_data[] = {
     //   2, 0x00, 0x00,       // <- datalen = 2, reg = 0x00, data = 0x00
     //   3, 0x01, 0x00, 0x02, // <- datalen = 3, reg = 0x01, data = 0x00, 0x02
     //   0 };                 // <- datalen 0 is end of data.
 
+    bool all_ok = true;
     while (*bulk_data) {
       uint8_t len = *bulk_data++;
       uint8_t r = retry + 1;
       while (!M5.In_I2C.writeRegister(i2c_addr, bulk_data[0], &bulk_data[1], len - 1, i2c_freq) && --r) { m5gfx::delay(1); }
+      all_ok &= (r != 0);
       bulk_data += len;
     }
+    return all_ok;
   }
 
   static constexpr uint8_t es7210_i2c_addr = 0x40;
   static constexpr uint8_t es8311_i2c_addr0 = 0x18;
   static constexpr uint8_t es8311_i2c_addr1 = 0x19;
+
+#if defined (CONFIG_IDF_TARGET_ESP32S3)
+  /// The ES8311 arms its capture path only through a SYSTEM(0x0D) write done
+  /// while the I2S clock is running; a pre-clock write is absorbed silently
+  /// (it even reads back). The arming is needed once per codec power-on
+  /// reset and survives I2C power-down/up cycles.
+  static std::atomic<bool> es8311_capture_armed { false };
+
+  /// Post-start callback: arms the capture path once the I2S clock runs.
+  /// The analog stage then warms up on its own (~1 s after power-up); that
+  /// is chip physics, so begin() pays only for the arming here.
+  static bool _microphone_post_start_cb_stopwatch(void* args)
+  {
+    (void)args;
+    if (es8311_capture_armed.load(std::memory_order_acquire)) { return true; }
+    m5gfx::i2c::i2c_temporary_switcher_t backup_i2c_setting(1, GPIO_NUM_47, GPIO_NUM_48);
+    bool ok = false;
+    bool last_ok = false;
+    for (int i = 0; i < 3; ++i)
+    { // writes at ~+30/+60/+90 ms after the clock start; the earliest arming
+      // observed on hardware is ~+25 ms, the later points are the margin
+      m5gfx::delay(30);
+      last_ok = M5.In_I2C.writeRegister8(es8311_i2c_addr0, 0x0D, 0x01, 100000);
+      ok |= last_ok;
+    }
+    backup_i2c_setting.restore();
+    /// an acknowledge proves only the transport, so the latch requires the
+    /// latest (most conservative) write to have been acknowledged
+    if (last_ok) { es8311_capture_armed.store(true, std::memory_order_release); }
+    return ok;
+  }
+#endif
   static constexpr uint8_t es8388_i2c_addr = 0x10;
   static constexpr uint8_t pi4io1_i2c_addr = 0x43;
   static constexpr uint8_t m5pm1_i2c_addr = 0x6E;
@@ -692,10 +728,17 @@ static constexpr const uint8_t _pin_table_mbus[][31] = {
       ioe1.digitalWrite(M5IOE1_Class::gpio10, true); // Enable PA (M5IOE1_G10)
     }
     else
-    {
+    { /// Keep Audio Power (M5IOE1_G3) on: cutting it forces another codec
+      /// reset, re-arming and re-warm-up on the next capture (and a quickly
+      /// cycled rail misfires). Only the DAC is powered down; Mic disable
+      /// does the deeper I2C power-down.
       auto& ioe1 = self->getIOExpander(0);
       ioe1.digitalWrite(M5IOE1_Class::gpio10, false); // Disable PA (M5IOE1_G10)
-      ioe1.digitalWrite(M5IOE1_Class::gpio3, false); // Disable Audio Power (M5IOE1_G3)
+      static constexpr const uint8_t disabled_bulk_data[] = {
+        2, 0x12, 0x02,  // 0x12 SYSTEM/ power-down DAC
+        0
+      };
+      in_i2c_bulk_write(es8311_i2c_addr0, disabled_bulk_data, 100000, 3);
     }
 #endif
     return true;
@@ -1263,8 +1306,19 @@ static constexpr const uint8_t _pin_table_mbus[][31] = {
       self->delay(5);
     }
     m5gfx::i2c::i2c_temporary_switcher_t backup_i2c_setting(1, GPIO_NUM_47, GPIO_NUM_48);
-    in_i2c_bulk_write(es8311_i2c_addr0, enabled ? enabled_bulk_data : disabled_bulk_data, 100000, 3);
+    if (enabled)
+    { /// 0x17 loses the value a previous setup wrote on any codec reset,
+      /// so it witnesses a reset done outside this library: re-arm then.
+      uint8_t v = 0;
+      if (!M5.In_I2C.readRegister(es8311_i2c_addr0, 0x17, &v, 1, 100000) || v != 0xFF)
+      {
+        es8311_capture_armed.store(false, std::memory_order_release);
+      }
+    }
+    bool setup_ok = in_i2c_bulk_write(es8311_i2c_addr0, enabled ? enabled_bulk_data : disabled_bulk_data, 100000, 3);
     backup_i2c_setting.restore();
+    /// a codec with an incomplete setup must not be published as working
+    if (enabled && !setup_ok) { return false; }
 #endif
     return true;
   }
@@ -2531,6 +2585,7 @@ static constexpr const uint8_t _pin_table_mbus[][31] = {
   void M5Unified::_begin_audio(config_t& cfg)
   {
     bool(*mic_enable_cb)(void*, bool) = nullptr;
+    bool(*mic_post_start_cb)(void*) = nullptr;
     auto mic_cfg = Mic.config();
 
     bool(*spk_enable_cb)(void*, bool) = nullptr;
@@ -2637,6 +2692,7 @@ static constexpr const uint8_t _pin_table_mbus[][31] = {
           mic_cfg.pin_data_in = GPIO_NUM_16;
           mic_cfg.i2s_port = I2S_NUM_1;
           mic_enable_cb = _microphone_enabled_cb_stopwatch;
+          mic_post_start_cb = _microphone_post_start_cb_stopwatch;
         }
       break;
 
@@ -3226,6 +3282,7 @@ static constexpr const uint8_t _pin_table_mbus[][31] = {
     if (mic_cfg.pin_data_in >= 0)
     {
       Mic.setCallback(this, mic_enable_cb);
+      Mic.setPostStartCallback(this, mic_post_start_cb);
       Mic.config(mic_cfg);
     }
     if (spk_cfg.pin_data_out >= 0)

src/utility/Mic_Class.cpp

@@ -591,7 +591,10 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) {
 
 #endif
 
-    _i2s_start(self->_cfg.i2s_port);
+    if (ESP_OK == _i2s_start(self->_cfg.i2s_port))
+    {
+      self->_i2s_active.store(true, std::memory_order_release);
+    }
 
     int32_t gain = self->_cfg.magnification;
     const float f_gain = (float)gain / (oversampling << 1);
@@ -764,27 +767,30 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) {
       }
     }
     _i2s_stop(self->_cfg.i2s_port);
+    self->_i2s_active.store(false, std::memory_order_release);
 
     self->_task_handle = nullptr;
     vTaskDelete(nullptr);
   }
 
   bool Mic_Class::begin(void)
   {
-    // _rec_sample_rate was written before _begun was released, so the
-    // acquire load makes this pair of reads safe without the lock.
-    if (_begun.load(std::memory_order_acquire) && _rec_sample_rate == _calc_rec_rate()) { return true; }
-
     // record() calls begin() lazily from whichever task gets there first,
     // and both the setup and the sample-rate change tear the port down: two
     // of these racing rip the live channel out from under the running task.
     // One caller goes through at a time; the others wait for its outcome.
+    // The already-running check also lives under the lock (vs. end()).
     bool zero = false;
     while (!_begin_lock.compare_exchange_strong(zero, true))
     {
       zero = false;
       vTaskDelay(1);
     }
+    if (_begun.load(std::memory_order_acquire) && _rec_sample_rate == _calc_rec_rate())
+    {
+      _begin_lock.store(false);
+      return true;
+    }
 
     bool res = true;
     if (_task_running)
@@ -793,7 +799,7 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) {
       if (_rec_sample_rate != rate)
       {
         do { vTaskDelay(1); } while (isRecording());
-        end();
+        _end_locked();
         _rec_sample_rate = rate;
       }
     }
@@ -816,7 +822,15 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) {
     bool res = true;
     if (_cb_set_enabled) { res = _cb_set_enabled(_cb_set_enabled_args, true); }
 
-    res = (ESP_OK == _setup_i2s()) && res;
+    if (res) { res = (ESP_OK == _setup_i2s()); }
+    if (!res)
+    { // no task to tear down yet, but whatever the enable callback powered
+      // up still has to come back down. No driver uninstall here: this
+      // attempt installed nothing (_setup_i2s cleans up its own failures),
+      // and on the legacy driver the port could belong to someone else.
+      if (_cb_set_enabled) { _cb_set_enabled(_cb_set_enabled_args, false); }
+      return false;
+    }
     if (res)
     {
       size_t stack_size = 2048 + (_cfg.dma_buf_len * sizeof(uint32_t));
@@ -831,16 +845,46 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) {
       {
         res = (pdPASS == xTaskCreate(mic_task, "mic_task", stack_size, this, _cfg.task_priority, &_task_handle));
       }
-      // end() takes the driver and the callback back down; it still sees the
-      // class as running, which is what lets it do that.
-      if (!res) { end(); }
-      else { _begun.store(true, std::memory_order_release); }
+      // _end_locked() takes the driver and the callback back down; it still
+      // sees the class as running, which is what lets it do that.
+      if (!res) { _end_locked(); }
+      else
+      {
+        if (_cb_post_start)
+        { // the callback needs the bus clock running: wait for the task to
+          // enable the channel, and fail the begin when that never happens
+          const uint32_t start_tick = xTaskGetTickCount();
+          while (!_i2s_active.load(std::memory_order_acquire)
+              && (uint32_t)(xTaskGetTickCount() - start_tick) < pdMS_TO_TICKS(1000)) { vTaskDelay(1); }
+          if (!_i2s_active.load(std::memory_order_acquire)
+           || !_cb_post_start(_cb_post_start_args))
+          {
+            _end_locked();
+            res = false;
+          }
+        }
+        if (res) { _begun.store(true, std::memory_order_release); }
+      }
     }
 
     return res;
   }
 
   void Mic_Class::end(void)
+  {
+    // taking _begin_lock keeps end() from tearing the port down while a
+    // begin() is still bringing it up
+    bool zero = false;
+    while (!_begin_lock.compare_exchange_strong(zero, true))
+    {
+      zero = false;
+      vTaskDelay(1);
+    }
+    _end_locked();
+    _begin_lock.store(false);
+  }
+
+  void Mic_Class::_end_locked(void)
   {
     _begun.store(false, std::memory_order_release);
     if (!_task_running) { return; }

src/utility/Mic_Class.hpp

@@ -105,8 +105,13 @@ namespace m5
     mic_config_t config(void) const { return _cfg; }
     void config(const mic_config_t& cfg) { _cfg = cfg; }
 
+    /// start the capture port. serialized with end().
+    /// Success means the port runs and the codec is configured; some codecs
+    /// (ES8311) additionally warm up for about a second after power-up,
+    /// during which captured samples can be all zero.
     bool begin(void);
 
+    /// stop the capture port. serialized with begin().
     void end(void);
 
     bool isRunning(void) const { return _task_running; }
@@ -159,6 +164,8 @@ namespace m5
 
   protected:
 
+    /// The callbacks run under the begin()/end() lock and must not call
+    /// begin() or end() themselves.
     void setCallback(void* args, bool(*func)(void*, bool)) { _cb_set_enabled = func; _cb_set_enabled_args = args; }
 
     struct recording_info_t
@@ -200,15 +207,30 @@ namespace m5
     /// begin() runs from whichever task records first, and setup starts by
     /// tearing the port down - so only one call may go through.
     std::atomic<bool> _begin_lock { false };
-    /// True only once begin() has fully finished; the lock-free early return
-    /// keys on this, so a caller can never see a half-built port as ready.
+    /// true only once begin() has fully finished.
     std::atomic<bool> _begun { false };
 #if defined (SDL_h_)
     SDL_Thread* _task_handle = nullptr;
 #else
     TaskHandle_t _task_handle = nullptr;
     volatile SemaphoreHandle_t _task_semaphore = nullptr;
 #endif
+
+  private:
+
+    /// set a callback that begin() invokes once the capture task has brought
+    /// the I2S clock up, for codecs that accept part of their setup only
+    /// while the bus clock runs (ES8311). A false return (or no clock within
+    /// one second) fails begin() and tears the port back down.
+    void setPostStartCallback(void* args, bool(*func)(void*)) { _cb_post_start = func; _cb_post_start_args = args; }
+
+    void _end_locked(void);
+
+    bool (*_cb_post_start)(void* args) = nullptr;
+    void* _cb_post_start_args = nullptr;
+    /// set by the task once the I2S channel is enabled; begin() waits on it
+    /// before invoking the post-start callback.
+    std::atomic<bool> _i2s_active { false };
   };
 }