From 79284083caecbc512ce2192f0bec842da4fb4be1 Mon Sep 17 00:00:00 2001 From: Daniel Hammer Date: Tue, 19 May 2026 18:13:37 +0200 Subject: [PATCH] feat: audio stack performance improvements --- kernel/src/Drivers/Audio/IntelHda.cpp | 16 ++++++++++ kernel/src/Drivers/Audio/IntelHda.hpp | 7 +++++ kernel/src/Drivers/Audio/Mixer.cpp | 44 ++++++++++++++++++++------- programs/src/audio/main.cpp | 34 ++++++++++++++++++--- 4 files changed, 86 insertions(+), 15 deletions(-) diff --git a/kernel/src/Drivers/Audio/IntelHda.cpp b/kernel/src/Drivers/Audio/IntelHda.cpp index 241a2f1..d3968a8 100644 --- a/kernel/src/Drivers/Audio/IntelHda.cpp +++ b/kernel/src/Drivers/Audio/IntelHda.cpp @@ -1074,6 +1074,22 @@ namespace Drivers::Audio::IntelHda { KernelLogStream(OK, "HDA") << "Stream closed"; } + uint32_t GetWriteSpace(int handle) { + if (handle != 0 || !g_stream.Active) return 0; + + uint32_t hwPos = g_dmaPos[g_stream.StreamIndex * 2]; + uint32_t writePos = g_stream.WritePos; + uint32_t available; + if (writePos >= hwPos) { + available = TOTAL_BUFFER_SIZE - (writePos - hwPos); + } else { + available = hwPos - writePos; + } + if (available > 64) available -= 64; + else available = 0; + return available; + } + int Write(int handle, const uint8_t* data, uint32_t size) { if (handle != 0 || !g_stream.Active || !data || size == 0) return -1; diff --git a/kernel/src/Drivers/Audio/IntelHda.hpp b/kernel/src/Drivers/Audio/IntelHda.hpp index 5bcecc0..05f2496 100644 --- a/kernel/src/Drivers/Audio/IntelHda.hpp +++ b/kernel/src/Drivers/Audio/IntelHda.hpp @@ -274,6 +274,13 @@ namespace Drivers::Audio::IntelHda { // Returns number of bytes written (may be less than requested if buffer full). int Write(int handle, const uint8_t* data, uint32_t size); + // Free bytes in the DMA ring that a caller may write right now. + // Returns 0 if the stream is not active. The mixer uses this to size + // each pump so it never produces more than HDA can accept (otherwise + // surplus mixed data would be silently dropped and the stream readers + // would still advance, scrambling playback). + uint32_t GetWriteSpace(int handle); + // Control commands constexpr int AUDIO_CTL_SET_VOLUME = 0; // value: 0-100 constexpr int AUDIO_CTL_GET_VOLUME = 1; diff --git a/kernel/src/Drivers/Audio/Mixer.cpp b/kernel/src/Drivers/Audio/Mixer.cpp index 1d8fa43..10a4851 100644 --- a/kernel/src/Drivers/Audio/Mixer.cpp +++ b/kernel/src/Drivers/Audio/Mixer.cpp @@ -164,9 +164,16 @@ namespace Drivers::Audio::Mixer { static void Pump() { if (!g_hdaOpened) return; - // Ask IntelHda how much room there is. The driver clamps writes to free - // space, so we feed it a generous buffer and let it consume what it can. - uint32_t frames = MAX_PUMP_FRAMES; + // Produce only as many frames as the HDA DMA ring has room for right + // now. Producing more would mean the surplus is silently dropped by + // IntelHda::Write while the per-stream resamplers had already + // advanced — that's what scrambles speech into a sequence of + // unrelated chunks. Clamp to MAX_PUMP_FRAMES so the scratch buffers + // are bounded. + uint32_t freeBytes = IntelHda::GetWriteSpace(g_hdaHandle); + uint32_t frames = freeBytes / 4; + if (frames == 0) return; + if (frames > MAX_PUMP_FRAMES) frames = MAX_PUMP_FRAMES; // Always write to HDA, even when no streams are active, so the // hardware ring stays filled with silence instead of looping the @@ -448,15 +455,26 @@ namespace Drivers::Audio::Mixer { void SetMasterVolume(int percent) { if (percent < 0) percent = 0; if (percent > 100) percent = 100; + + // Take the mixer lock just long enough to update software state and + // capture a snapshot for the HW write. The IntelHda codec command + // path busy-waits on the RIRB for a few hundred microseconds, so + // doing it inside the lock would freeze interrupts (BCIS, scheduler, + // input) for the whole duration of every drag tick — visible as + // slider lag. g_lock.Acquire(); bool changed = (g_masterVolume != percent); g_masterVolume = percent; - if (g_hdaOpened) { - IntelHda::Control(g_hdaHandle, IntelHda::AUDIO_CTL_SET_VOLUME, - g_masterMute ? 0 : g_masterVolume); - } + int curVol = g_masterVolume; + bool curMute = g_masterMute; + bool hdaOpen = g_hdaOpened; if (changed) BumpSerialLocked(); g_lock.Release(); + + if (changed && hdaOpen) { + IntelHda::Control(g_hdaHandle, IntelHda::AUDIO_CTL_SET_VOLUME, + curMute ? 0 : curVol); + } if (changed) Sched::WakeObjectWaiters((void*)&g_serial); } @@ -468,12 +486,16 @@ namespace Drivers::Audio::Mixer { g_lock.Acquire(); bool changed = (g_masterMute != muted); g_masterMute = muted; - if (g_hdaOpened) { - IntelHda::Control(g_hdaHandle, IntelHda::AUDIO_CTL_SET_VOLUME, - g_masterMute ? 0 : g_masterVolume); - } + int curVol = g_masterVolume; + bool curMute = g_masterMute; + bool hdaOpen = g_hdaOpened; if (changed) BumpSerialLocked(); g_lock.Release(); + + if (changed && hdaOpen) { + IntelHda::Control(g_hdaHandle, IntelHda::AUDIO_CTL_SET_VOLUME, + curMute ? 0 : curVol); + } if (changed) Sched::WakeObjectWaiters((void*)&g_serial); } diff --git a/programs/src/audio/main.cpp b/programs/src/audio/main.cpp index 7eb7ddf..d047fae 100644 --- a/programs/src/audio/main.cpp +++ b/programs/src/audio/main.cpp @@ -180,6 +180,7 @@ static void refresh_master() { static void apply_master_volume(int v) { if (v < 0) v = 0; if (v > 100) v = 100; + if (v == g_master_vol) return; // No change → skip syscall + HW codec hit. g_master_vol = v; montauk::audio_set_master_volume(v); } @@ -198,6 +199,7 @@ static void apply_stream_volume(int idx, int v) { if (idx < 0 || idx >= g_stream_count) return; if (v < 0) v = 0; if (v > 100) v = 100; + if ((uint8_t)v == g_streams[idx].volume) return; g_streams[idx].volume = (uint8_t)v; montauk::audio_set_volume(g_streams[idx].handle, v); } @@ -433,13 +435,16 @@ extern "C" void _start() { // Event-driven sync: the kernel bumps a mixer serial on every state // change. Reading it is one cheap syscall; we only re-snapshot the - // full mixer state when the serial has actually moved. The master - // value is left alone while the user is dragging the master slider - // so a mid-drag wakeup can't snap the knob to the kernel value. + // full mixer state when the serial has actually moved. Only redraw + // when the snapshot differs from what we already show, otherwise + // self-driven bumps (e.g. the user dragging our own slider) would + // cause a redraw on every drag tick. uint64_t now_serial = montauk::audio_wait(g_mixer_serial, 0); if (now_serial != g_mixer_serial) { g_mixer_serial = now_serial; + // Master: only refresh when we're not dragging master ourselves, + // otherwise a wake mid-drag could snap our knob to the kernel. if (g_drag_target != -1) { int prev_vol = g_master_vol; bool prev_muted = g_master_muted; @@ -448,8 +453,29 @@ extern "C" void _start() { redraw = true; } + // Per-stream: diff before redrawing so dragging the master slider + // (which bumps the serial) doesn't trigger redundant repaints. + int prev_count = g_stream_count; + uint8_t prev_vols[8] = {0}; + uint8_t prev_mutes[8] = {0}; + int prev_handles[8] = {0}; + for (int i = 0; i < prev_count && i < 8; i++) { + prev_vols[i] = g_streams[i].volume; + prev_mutes[i] = g_streams[i].muted; + prev_handles[i] = g_streams[i].handle; + } refresh_streams(); - redraw = true; + if (g_stream_count != prev_count) { + redraw = true; + } else { + for (int i = 0; i < g_stream_count; i++) { + if (g_streams[i].handle != prev_handles[i] || + g_streams[i].volume != prev_vols[i] || + g_streams[i].muted != prev_mutes[i]) { + redraw = true; break; + } + } + } } if (r < 0) break;