From 00b1ba58f0efca59e1ef2acbeab21849db4faafe Mon Sep 17 00:00:00 2001 From: jacqueline Date: Fri, 14 Jun 2024 14:27:56 +1000 Subject: [PATCH] Improve DAC power+mute management to reduce clicks and pops --- src/drivers/gpios.cpp | 2 +- src/drivers/i2s_dac.cpp | 47 ++++++++++--------------- src/drivers/include/drivers/i2s_dac.hpp | 2 -- src/tangara/audio/i2s_audio_output.cpp | 8 ----- src/tangara/audio/i2s_audio_output.hpp | 1 - 5 files changed, 20 insertions(+), 40 deletions(-) diff --git a/src/drivers/gpios.cpp b/src/drivers/gpios.cpp index 64ba26a7..52b2b874 100644 --- a/src/drivers/gpios.cpp +++ b/src/drivers/gpios.cpp @@ -43,7 +43,7 @@ static const uint8_t kPortADefault = 0b00111110; // 6 - NC // 7 - NC // Default inputs high, amp off. -static const uint8_t kPortBDefault = 0b00001101; +static const uint8_t kPortBDefault = 0b00011111; /* * Convenience mehod for packing the port a and b bytes into a single 16 bit diff --git a/src/drivers/i2s_dac.cpp b/src/drivers/i2s_dac.cpp index b479d572..b1044896 100644 --- a/src/drivers/i2s_dac.cpp +++ b/src/drivers/i2s_dac.cpp @@ -137,54 +137,43 @@ I2SDac::I2SDac(IGpios& gpio, PcmBuffer& buf, i2s_chan_handle_t i2s_handle) I2S_SLOT_MODE_STEREO)) { clock_config_.clk_src = I2S_CLK_SRC_APLL; - // The amplifier's power rails ramp unevenly, with the negative rail coming - // up ~5ms after the positive rail. Ensure that headphone output is muted - // during this to avoid a loud pop during power up. - gpio_.WriteSync(IGpios::Pin::kAmplifierMute, true); - vTaskDelay(pdMS_TO_TICKS(1)); - - gpio_.WriteSync(IGpios::Pin::kAmplifierEnable, true); + // Power up the DAC. + wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b01); // Reset all registers back to their default values. wm8523::WriteRegister(wm8523::Register::kReset, 1); - // Wait for DAC reset + analog rails ramp. + // Wait for DAC to reset. vTaskDelay(pdMS_TO_TICKS(10)); - wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b0); - // Use zero-cross detection for volume changes. - wm8523::WriteRegister(wm8523::Register::kDacCtrl, 0b10000); + // Enable zero-cross detection and ramping for volume changes. + wm8523::WriteRegister(wm8523::Register::kDacCtrl, 0b10011); + + // Ready to play! + wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b10); } I2SDac::~I2SDac() { - Stop(); - i2s_del_channel(i2s_handle_); + if (i2s_active_) { + SetPaused(true); + } - gpio_.WriteSync(IGpios::Pin::kAmplifierMute, true); - vTaskDelay(pdMS_TO_TICKS(1)); - gpio_.WriteSync(IGpios::Pin::kAmplifierEnable, false); -} + // Power down the DAC. + wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b01); + wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b00); -auto I2SDac::Start() -> void { - std::lock_guard lock(configure_mutex_); - wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b11); -} - -auto I2SDac::Stop() -> void { - std::lock_guard lock(configure_mutex_); - set_channel(false); - wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b0); + i2s_del_channel(i2s_handle_); } auto I2SDac::SetPaused(bool paused) -> void { if (paused) { - gpio_.WriteSync(IGpios::Pin::kAmplifierUnmuteLegacy, false); + wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b10); gpio_.WriteSync(IGpios::Pin::kAmplifierMute, true); set_channel(false); } else { set_channel(true); - gpio_.WriteSync(IGpios::Pin::kAmplifierUnmuteLegacy, true); gpio_.WriteSync(IGpios::Pin::kAmplifierMute, false); + wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b11); } } @@ -250,6 +239,8 @@ auto I2SDac::Reconfigure(Channels ch, BitsPerSample bps, SampleRate rate) if (i2s_active_) { i2s_channel_enable(i2s_handle_); wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b11); + } else { + wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b10); } } diff --git a/src/drivers/include/drivers/i2s_dac.hpp b/src/drivers/include/drivers/i2s_dac.hpp index 138a0c03..cf9258c0 100644 --- a/src/drivers/include/drivers/i2s_dac.hpp +++ b/src/drivers/include/drivers/i2s_dac.hpp @@ -45,8 +45,6 @@ class I2SDac { I2SDac(IGpios& gpio, PcmBuffer&, i2s_chan_handle_t i2s_handle); ~I2SDac(); - auto Start() -> void; - auto Stop() -> void; auto SetPaused(bool) -> void; enum Channels { diff --git a/src/tangara/audio/i2s_audio_output.cpp b/src/tangara/audio/i2s_audio_output.cpp index 09fc3e69..8222b8c9 100644 --- a/src/tangara/audio/i2s_audio_output.cpp +++ b/src/tangara/audio/i2s_audio_output.cpp @@ -41,8 +41,6 @@ static constexpr uint16_t kMaxVolumeBeforeClipping = 0x185; static constexpr uint16_t kLineLevelVolume = 0x13d; static constexpr uint16_t kDefaultVolume = 0x100; -static constexpr size_t kDrainBufferSize = 8 * 1024; - I2SAudioOutput::I2SAudioOutput(drivers::IGpios& expander, drivers::PcmBuffer& buffer) : IAudioOutput(), @@ -55,10 +53,6 @@ I2SAudioOutput::I2SAudioOutput(drivers::IGpios& expander, current_volume_(kDefaultVolume), max_volume_(0) {} -I2SAudioOutput::~I2SAudioOutput() { - dac_->Stop(); -} - auto I2SAudioOutput::changeMode(Modes mode) -> void { if (mode == current_mode_) { return; @@ -70,7 +64,6 @@ auto I2SAudioOutput::changeMode(Modes mode) -> void { // Turning off this output. Ensure we clean up the I2SDac instance to // reclaim its valuable DMA buffers. if (dac_) { - dac_->Stop(); dac_.reset(); } return; @@ -87,7 +80,6 @@ auto I2SAudioOutput::changeMode(Modes mode) -> void { } // Set up the new instance properly. SetVolume(GetVolume()); - dac_->Start(); } current_mode_ = mode; diff --git a/src/tangara/audio/i2s_audio_output.hpp b/src/tangara/audio/i2s_audio_output.hpp index 80a8fc36..35d888b9 100644 --- a/src/tangara/audio/i2s_audio_output.hpp +++ b/src/tangara/audio/i2s_audio_output.hpp @@ -22,7 +22,6 @@ namespace audio { class I2SAudioOutput : public IAudioOutput { public: I2SAudioOutput(drivers::IGpios&, drivers::PcmBuffer&); - ~I2SAudioOutput(); auto SetMaxVolume(uint16_t) -> void; auto SetVolumeDb(uint16_t) -> void;