[audio][aml-g12-pdm] Reduce ring buffer size
This is pretty much the same as http://fxrev.dev/449830 but for
PDM input.
Instead of precalculating the ring buffer size according to the
maximum possible parameters to get 1 second of audio, calculate
the size based on the required format and the min_frames parameter.
Bug: 48587
Test: audio_driver_tests
Test: aml-g12-pdm-test
Change-Id: I97a6f86fa9a861ea809dcbd9955ced41cf243015
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/449999
Reviewed-by: Braden Kell <bradenkell@google.com>
Testability-Review: Braden Kell <bradenkell@google.com>
Commit-Queue: Andres Oportus <andresoportus@google.com>
diff --git a/src/devices/lib/amlogic/include/soc/aml-common/aml-pdm-audio.h b/src/devices/lib/amlogic/include/soc/aml-common/aml-pdm-audio.h
index eb49b1e..be9623b 100644
--- a/src/devices/lib/amlogic/include/soc/aml-common/aml-pdm-audio.h
+++ b/src/devices/lib/amlogic/include/soc/aml-common/aml-pdm-audio.h
@@ -31,6 +31,9 @@
// must resize in lower 32-bits of address space
zx_status_t SetBuffer(zx_paddr_t buf, size_t len);
+ // Get HW alignment required in the buffer in bytes.
+ static uint32_t GetBufferAlignment() { return 8; }
+
/*
Returns offset of dma pointer in the ring buffer
*/
diff --git a/src/media/audio/drivers/aml-g12-pdm/audio-stream-in.cc b/src/media/audio/drivers/aml-g12-pdm/audio-stream-in.cc
index dea6105..348e42b 100644
--- a/src/media/audio/drivers/aml-g12-pdm/audio-stream-in.cc
+++ b/src/media/audio/drivers/aml-g12-pdm/audio-stream-in.cc
@@ -7,6 +7,7 @@
#include <lib/zx/clock.h>
#include <math.h>
+#include <numeric>
#include <optional>
#include <utility>
@@ -120,13 +121,18 @@
return ZX_ERR_NO_MEMORY;
}
- // Calculate ring buffer size for 1 second of 16-bit, 48kHz.
- size_t kRingBufferSize = fbl::round_up<size_t, size_t>(
- kMaxSampleRate * 2 * metadata_.number_of_channels, ZX_PAGE_SIZE);
- // Initialize the ring buffer
- InitBuffer(kRingBufferSize);
-
- lib_->SetBuffer(pinned_ring_buffer_.region(0).phys_addr, pinned_ring_buffer_.region(0).size);
+ // Initial setup of one page of buffer, just to be safe.
+ status = InitBuffer(PAGE_SIZE);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "%s failed to init buffer %d", __FILE__, status);
+ return status;
+ }
+ status =
+ lib_->SetBuffer(pinned_ring_buffer_.region(0).phys_addr, pinned_ring_buffer_.region(0).size);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "%s failed to set buffer %d", __FILE__, status);
+ return status;
+ }
InitHw();
@@ -166,23 +172,33 @@
zx_status_t AudioStreamIn::GetBuffer(const audio_proto::RingBufGetBufferReq& req,
uint32_t* out_num_rb_frames, zx::vmo* out_buffer) {
- uint32_t rb_frames = static_cast<uint32_t>(pinned_ring_buffer_.region(0).size) / frame_size_;
-
- if (req.min_ring_buffer_frames > rb_frames) {
- zxlogf(ERROR, "%s out of range min rin buffer frames", __FUNCTION__);
- return ZX_ERR_OUT_OF_RANGE;
+ size_t ring_buffer_size = fbl::round_up<size_t, size_t>(
+ req.min_ring_buffer_frames * frame_size_, std::lcm(frame_size_, lib_->GetBufferAlignment()));
+ size_t out_frames = ring_buffer_size / frame_size_;
+ if (out_frames > std::numeric_limits<uint32_t>::max()) {
+ return ZX_ERR_INVALID_ARGS;
}
- zx_status_t status;
+
+ size_t vmo_size = fbl::round_up<size_t, size_t>(ring_buffer_size, PAGE_SIZE);
+ auto status = InitBuffer(vmo_size);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "%s failed to init buffer %d", __FILE__, status);
+ return status;
+ }
+
constexpr uint32_t rights = ZX_RIGHT_READ | ZX_RIGHT_WRITE | ZX_RIGHT_MAP | ZX_RIGHT_TRANSFER;
status = ring_buffer_vmo_.duplicate(rights, out_buffer);
if (status != ZX_OK) {
zxlogf(ERROR, "%s failed to duplicate vmo", __FUNCTION__);
return status;
}
-
- *out_num_rb_frames = rb_frames;
-
- lib_->SetBuffer(pinned_ring_buffer_.region(0).phys_addr, rb_frames * frame_size_);
+ status = lib_->SetBuffer(pinned_ring_buffer_.region(0).phys_addr, ring_buffer_size);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "%s failed to set buffer %d", __FILE__, status);
+ return status;
+ }
+ // This is safe because of the overflow check we made above.
+ *out_num_rb_frames = static_cast<uint32_t>(out_frames);
return status;
}
@@ -251,13 +267,13 @@
}
zx_status_t AudioStreamIn::InitBuffer(size_t size) {
- zx_status_t status;
- status = zx_vmo_create_contiguous(bti_.get(), size, 0, ring_buffer_vmo_.reset_and_get_address());
+ pinned_ring_buffer_.Unpin();
+ auto status =
+ zx_vmo_create_contiguous(bti_.get(), size, 0, ring_buffer_vmo_.reset_and_get_address());
if (status != ZX_OK) {
zxlogf(ERROR, "%s failed to allocate ring buffer vmo - %d", __func__, status);
return status;
}
-
status = pinned_ring_buffer_.Pin(ring_buffer_vmo_, bti_, ZX_VM_PERM_READ | ZX_VM_PERM_WRITE);
if (status != ZX_OK) {
zxlogf(ERROR, "%s failed to pin ring buffer vmo - %d", __func__, status);
diff --git a/src/media/audio/drivers/aml-g12-pdm/test/test.cc b/src/media/audio/drivers/aml-g12-pdm/test/test.cc
index 05a692f..0f2bd6b 100644
--- a/src/media/audio/drivers/aml-g12-pdm/test/test.cc
+++ b/src/media/audio/drivers/aml-g12-pdm/test/test.cc
@@ -9,6 +9,7 @@
#include <ddktl/protocol/composite.h>
#include <fake-mmio-reg/fake-mmio-reg.h>
#include <mock-mmio-reg/mock-mmio-reg.h>
+#include <mock/ddktl/protocol/gpio.h>
#include <soc/aml-s905d2/s905d2-hw.h>
#include <zxtest/zxtest.h>
@@ -138,6 +139,33 @@
EXPECT_EQ(step, 12);
}
+ void TestRingBufferSize(uint8_t number_of_channels, uint32_t frames_req,
+ uint32_t frames_expected) {
+ auto metadata = GetDefaultMetadata();
+ metadata.number_of_channels = number_of_channels;
+ tester_.SetMetadata(&metadata, sizeof(metadata));
+
+ auto stream = audio::SimpleAudioStream::Create<TestAudioStreamIn>();
+ audio_fidl::Device::SyncClient client_wrap(std::move(tester_.FidlClient()));
+ audio_fidl::Device::ResultOf::GetChannel ch = client_wrap.GetChannel();
+ ASSERT_EQ(ch.status(), ZX_OK);
+ audio_fidl::StreamConfig::SyncClient client(std::move(ch->channel));
+ zx::channel local, remote;
+ ASSERT_OK(zx::channel::create(0, &local, &remote));
+ fidl::aligned<audio_fidl::PcmFormat> pcm_format = GetDefaultPcmFormat();
+ pcm_format.value.number_of_channels = number_of_channels;
+ auto builder = audio_fidl::Format::UnownedBuilder();
+ builder.set_pcm_format(fidl::unowned_ptr(&pcm_format));
+ client.CreateRingBuffer(builder.build(), std::move(remote));
+
+ auto vmo = audio_fidl::RingBuffer::Call::GetVmo(zx::unowned_channel(local), frames_req, 0);
+ ASSERT_OK(vmo.status());
+ ASSERT_EQ(vmo.Unwrap()->result.response().num_frames, frames_expected);
+
+ stream->DdkAsyncRemove();
+ EXPECT_TRUE(tester_.Ok());
+ stream->DdkRelease();
+ }
FakePDev pdev_;
fake_ddk::Bind tester_;
};
@@ -155,6 +183,22 @@
TestMasks(/*channels*/ 2, /*channels_to_use_bitmask*/ 0xff, /*channels_mask*/ 3, /*mute_mask*/ 0);
}
+// With 16 bits samples, frame size is 2 x number of channels bytes.
+// Frames returned are rounded to HW buffer alignment (8 bytes) and frame size.
+TEST_F(AudioStreamInTest, RingBufferSize1) {
+ TestRingBufferSize(2, 1, 2);
+} // Rounded to HW buffer.
+TEST_F(AudioStreamInTest, RingBufferSize2) {
+ TestRingBufferSize(2, 3, 4);
+} // Rounded to HW buffer.
+TEST_F(AudioStreamInTest, RingBufferSize3) { TestRingBufferSize(3, 1, 4); } // Rounded to both.
+TEST_F(AudioStreamInTest, RingBufferSize4) { TestRingBufferSize(3, 3, 4); } // Rounded to both.
+TEST_F(AudioStreamInTest, RingBufferSize5) {
+ TestRingBufferSize(8, 1, 1);
+} // Rounded to frame size.
+TEST_F(AudioStreamInTest, RingBufferSize6) {
+ TestRingBufferSize(8, 3, 3);
+} // Rounded to frame size.
} // namespace audio::aml_g12
// Redefine PDevMakeMmioBufferWeak per the recommendation in pdev.h.
