Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start

["Mukunda,Vijendar" <vijendar.mukunda@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 5221 bytes --]



On 8/5/26 23:41, Robin Everaars wrote:
> The ACP PDM DMIC emits a full-scale burst at the start of every capture stream.
> It is loud and it begins at the same frame every time. On a cold start it lasts
> long enough that the far end of a call hears a click on join.
>
> Hardware and software
> =====================
>
> ASUS ProArt PX13 HN7306EAC, AMD Strix Halo. Card 1 "amdsoundwire", driver
> amd-soundwire, longname
> ASUSTeKCOMPUTERINC.-ProArtPX13HN7306EAC-1.0-HN7306EAC. The DMIC array is
> /proc/asound/card1 pcm4c, id "acp-dmic-codec dmic-hifi-4", reachable as
> hw:1,4 or hw:amdsoundwire,4. Native format S32_LE, 48000 Hz, 2 channels,
> single rate. Kernel 7.1.5, alsa-lib and alsa-utils 1.2.16, PipeWire 1.6.8,
> NixOS.
>
> What the burst looks like
> =========================
>
> Capturing 3 s of a quiet room at 48 kHz gives 304 to 310 samples railed to
> +32767 or -32768, and every one of them falls inside frames 21 to 177, i.e.
> the first 0.44 to 3.65 ms of the stream. After the first 10 ms there are zero
> clipped samples.
>
> The shape
> rules out an analog transient. Printing the first 40 frames shows an
> exponentially growing alternating pattern that collapses into a rail, that is
> a Nyquist-rate oscillation at 0 dBFS:
>
>    (336, 2529) (311, 2454) ... (4765, 1614) (-14926, -7363) (-32768, -32768)
>    (3598, -3283) (-817, 2296) (-32768, 32767) (32767, -32768) (-32768, 32767)
>    (32767, -32768) (-32768, 32767) ...
>
> Left and right are in antiphase. That reads as an unflushed decimator rather
> than anything acoustic.
>
> Reproducible frame for frame across runs, and across every rate, format and
> channel count the PCM accepts.
>
> The cold start is much worse, and it is the common case
> =======================================================
>
> The 3.6 ms figure above is the WARM case, where the ACP was already awake. On a
> cold start, meaning the ACP had runtime-suspended, a second and far larger
> excursion follows: it rails from about 15 ms to about 50 ms and only reaches the
> noise floor around 250 ms.
>
> That is the c
> ase that matters in practice. Any userspace audio server suspends
> an idle capture node after a few seconds (WirePlumber does it after 5 s), so
> every real call join is a cold start.
>
> The driver produces it
> ======================
>
> Identical through plain arecord with PipeWire out of the picture:
>
>    arecord -D hw:1,4 -f S32_LE -r 48000 -c 2 -d 6 -t wav /tmp/x.wav
>
> gives 304 railed samples in frames 21 to 177, against 305 to 307 through
> PipeWire. Same start frame, same duration, same shape.
>
> It also survives band-limiting. Low-passing to 8 kHz and to 4 kHz still leaves
> an audible click, so a call codec does not remove it.
>
> Where it comes from
> ===================
>
> sound/soc/amd/acp/acp-pdm.c has no settling delay, no mute and no discard of
> the first frames. acp_dmic_dai_trigger() enables the PDM controller and the DMA
> in the same breath, so the decimator's first output words go straight to
> userspace. A PDM microphone needs time to settle after its clock starts, and a
>
> CIC or decimation filter chain needs to flush its state, and neither is waited
> for here.
>
> Other ASoC DMIC paths handle this. snd_soc_dmic has a wakeup_delay module
> parameter whose msleep runs at DAPM POST_PMU for exactly this reason.
>
> What would fix it
> =================
>
> Any of these, in rough order of preference:
>
> 1. Discard or mute the first N frames after PDM_ENABLE inside acp-pdm.c, where N
>     covers the decimator's own settling. That is the cheapest fix and it is
>     invisible to userspace.
> 2. Wait for the microphone to settle between enabling the PDM clock and enabling
>     the DMA, rather than doing both in acp_dmic_dai_trigger().
> 3. Expose a settling delay the way snd_soc_dmic does, so a platform that needs
>     longer can set it.
Could you please try attached patch?
>
> A separate, smaller thing on the same device
> ============================================
>
> There is a standing DC offset of about +0.06 FS on channel 0, roughly -24 dBFS,
> present with no acoustic input. It is passed through to user
> space, so naive
> level meters read about -23 dBFS in a silent room and any AGC treats it as
> signal. After an 80 Hz high-pass the real floor is peak -45 dBFS rms -62 dBFS.
>
> If a DC blocker is expected somewhere in this path, it is not running here.
Since the ACP PDM driver is a DSP-less solution, it does not have the 
capability to apply
audio processing effects to mitigate DC offset. A similar issue was 
reported previously,
and applying certain effects in PipeWire helped reduce the DC offset.
>
> Reproducer
> ==========
>
>    # cold start: let the ACP runtime-suspend first, then capture
>    cat /sys/bus/pci/devices/*/power/runtime_status   # wait for suspended
>    arecord -D hw:1,4 -f S32_LE -r 48000 -c 2 -d 3 -t wav /tmp/cold.wav
>
> Then look at the first 300 ms. Any tool will do; the railed samples are obvious.
> Repeating the capture immediately afterwards gives the shorter warm-start
> version, which is a convenient way to see both.
>
> I have no fix to offer, only the measurements. Happy to test a patch on this
> hardware.

[-- Attachment #2: 0001-ASoC-amd-acp-pdm-fix-decimator-transient-on-stream-s.patch --]
[-- Type: text/x-patch, Size: 4391 bytes --]

From dab1a7161822129fe2a576e7eaba63cfcbc51b0d Mon Sep 17 00:00:00 2001
From: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Date: Mon, 10 Aug 2026 10:34:44 +0530
Subject: [PATCH] ASoC: amd: acp: pdm: fix decimator transient on stream start

The ACP PDM DMIC emits a full-scale Nyquist-rate burst at the beginning
of every capture stream.  The burst lasts ~4 ms on a warm start (ACP
already running) and ~250 ms on a cold start (ACP runtime-suspended),
and is audible to remote call participants as a click on join.

Root cause: acp_dmic_dai_trigger() enables the PDM decimator and the DMA
in the same write sequence, so the CIC filter chain's unflushed initial
state goes directly to userspace.  An exponentially growing alternating
pattern collapses into full-scale saturation on both channels in antiphase
-- the classic signature of a CIC integrator/comb chain running from a
non-zero initial condition.

Fix: enable PDM_ENABLE in acp_dmic_prepare() and wait 300 ms before
starting the DMA.  300 ms is chosen to cover the cold-start case; the
warm-start transient clears in under 5 ms.  After the settling delay,
flush ACP_WOV_PDM_FIFO_FLUSH so the ring buffer the DMA reads from
is clean.  acp_dmic_dai_trigger() then only needs to arm the DMA, not
re-enable PDM.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-pdm.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/acp/acp-pdm.c b/sound/soc/amd/acp/acp-pdm.c
index 1bfc34c2aa53..08995d0502ef 100644
--- a/sound/soc/amd/acp/acp-pdm.c
+++ b/sound/soc/amd/acp/acp-pdm.c
@@ -13,6 +13,7 @@
  * Generic Hardware interface for ACP Audio PDM controller
  */
 
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -25,6 +26,17 @@
 
 #define DRV_NAME "acp-pdm"
 
+/*
+ * Time in milliseconds to wait after enabling the PDM clock before
+ * starting the DMA.  The PDM microphone and the CIC/decimation filter
+ * chain need this time to reach a stable state; without it the first
+ * frames delivered to userspace contain a saturated Nyquist-rate
+ * transient (unflushed decimator initial state).  300 ms covers both
+ * the warm-start case (~4 ms) and the cold-start case (~250 ms, when
+ * the ACP has been runtime-suspended).
+ */
+#define PDM_SETTLING_DELAY_MS	300
+
 static int acp_dmic_prepare(struct snd_pcm_substream *substream,
 			    struct snd_soc_dai *dai)
 {
@@ -41,6 +53,18 @@ static int acp_dmic_prepare(struct snd_pcm_substream *substream,
 	dmic_ctrl |= PDM_MISC_CTRL_MASK;
 	writel(dmic_ctrl, chip->base + ACP_WOV_MISC_CTRL);
 
+	/*
+	 * Enable the PDM decimator now, before the DMA starts.  The
+	 * CIC filter chain and the microphone itself need time to reach
+	 * a stable state after the clock is applied.  Without this
+	 * settling period the first frames captured by the DMA contain
+	 * a saturated Nyquist-rate transient.  Wait for the decimator
+	 * to flush, then clear the FIFO so the DMA sees a clean buffer.
+	 */
+	writel(PDM_ENABLE, chip->base + ACP_WOV_PDM_ENABLE);
+	msleep(PDM_SETTLING_DELAY_MS);
+	writel(0x01, chip->base + ACP_WOV_PDM_FIFO_FLUSH);
+
 	period_bytes = frames_to_bytes(substream->runtime,
 			substream->runtime->period_size);
 	size_dmic = frames_to_bytes(substream->runtime,
@@ -74,7 +98,11 @@ static int acp_dmic_dai_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		dma_enable = readl(chip->base + ACP_WOV_PDM_DMA_ENABLE);
 		if (!(dma_enable & DMA_EN_MASK)) {
-			writel(PDM_ENABLE, chip->base + ACP_WOV_PDM_ENABLE);
+			/*
+			 * PDM_ENABLE was already written in prepare() to allow
+			 * the decimator to settle before the DMA starts.  Only
+			 * arm the DMA here; do not re-enable PDM.
+			 */
 			writel(PDM_ENABLE, chip->base + ACP_WOV_PDM_DMA_ENABLE);
 		}
 
@@ -89,12 +117,13 @@ static int acp_dmic_dai_trigger(struct snd_pcm_substream *substream,
 		if ((dma_enable & DMA_EN_MASK)) {
 			writel(PDM_DISABLE, chip->base + ACP_WOV_PDM_ENABLE);
 			writel(PDM_DISABLE, chip->base + ACP_WOV_PDM_DMA_ENABLE);
-
 		}
 
 		ret = readl_poll_timeout_atomic(chip->base + ACP_WOV_PDM_DMA_ENABLE,
 						dma_enable, !(dma_enable & DMA_EN_MASK),
 						DELAY_US, PDM_TIMEOUT);
+		if (!ret)
+			writel(0x01, chip->base + ACP_WOV_PDM_FIFO_FLUSH);
 		break;
 	default:
 		ret = -EINVAL;
-- 
2.48.1