Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start
From: Mukunda,Vijendar
Date: Thu Aug 13 2026 - 01:11:46 EST
On 8/12/26 19:11, Robin Everaars wrote:
Please try attached patches.PAUSE_PUSH should leave the decimator running. That is the safer ALSAI tested the exact attachment against v7.1.7 on the ASUS ProArt PX13
design and avoids both the transient and a 300 ms pause-resume latency.
Try attached patch.
HN7306EAC. The bound module was snd_ps_pdm_dma from the rebuilt
sound/soc/amd/ps/ps-pdm-dma.c.
The normal start and pause paths work:
- 5/5 cold starts, with acp_ps_pdm_dma.0 confirmed runtime-suspended
before each capture: zero clipped or >=99% full-scale samples in the
first 300 ms.
- 5/5 warm starts, with the device confirmed active: the same result.
- PAUSE_PUSH/PAUSE_RELEASE: 3/3 with a 0.5 s pause and 3/3 with a 2 s
pause, no ALSA error and no full-scale samples.
- prepare followed by close without START: 3/3; the device returned to
runtime-suspended after the 2 s autosuspend delay.
- forced XRUN/reprepare recovery: 5/5; each recovered block had zero
clipped or >=99% full-scale samples.
Eac
h prepare took about 304 to 309 ms. A one-second capture took 1.43 to
1.48 seconds wall time, so the expected start latency is visible.
There is a problem on the system-resume path. I kept an ALSA capture
running across one s2idle cycle. The process reached the real ALSA
SUSPENDED state. Immediately after wake:
SUSPENDED boundary=101384
RESUME rc=0 (ok) state=RUNNING
XRUN boundary=101384
error: Broken pipe
No post-resume sample was delivered. A fresh reopen after resume worked
and contained no clipped samples.
That matches the control flow in the patch: SUSPEND calls
acp63_stop_pdm_dma(), which disables PDM, while RESUME enters
acp63_start_pdm_dma(). That function enables PDM and DMA together when
PDM_ENABLE is clear, without the 300 ms wait or FIFO flush. prepare() is
not called between SUSPEND and RESUME.
Could RESUME use a settling helper before arming DMA, or otherwise force
a prepare/recovery path? I have withheld Tested-by because the in-place
resume test fails. I can test a follow-up patch on the same hardware.
From 729d30468c9dcb49aa4625af7d95ffc3c9d569b8 Mon Sep 17 00:00:00 2001
Thanks,
Robin
From: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
Date: Wed, 12 Aug 2026 14:41:42 +0530
Subject: [PATCH 1/2] ASoC: amd: ps: 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: acp63_start_pdm_dma() enables the PDM decimator and the DMA
in the same call, 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: add a prepare() DAI callback that enables PDM_ENABLE and waits
300 ms before the DMA starts. 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 DMA sees a clean
buffer. acp63_start_pdm_dma() then only arms the DMA, skipping the
PDM_ENABLE write when prepare() has already set it.
For the RESUME path where prepare() is not called, PDM_ENABLE is set
conditionally in acp63_start_pdm_dma() if not already active.
PAUSE handling: PAUSE_PUSH previously disabled both the DMA and the PDM
decimator via acp63_stop_pdm_dma(). After moving PDM_ENABLE to
prepare(), ALSA does not call prepare() between PAUSE_PUSH and
PAUSE_RELEASE, so PAUSE_RELEASE would have re-enabled PDM without the
300 ms settling delay, reproducing the transient on every resume from
pause.
Fix the pause path by adding acp63_pause_pdm_dma(), which stops the DMA
only and leaves the PDM decimator running. PAUSE_RELEASE flushes the
FIFO to discard samples accumulated while the DMA was stopped, then
restarts the DMA. Since the CIC filter never stopped, no settling delay
is needed. STOP and SUSPEND continue to use acp63_stop_pdm_dma(), which
disables both DMA and PDM.
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
---
sound/soc/amd/ps/ps-pdm-dma.c | 103 ++++++++++++++++++++++++++++++++--
1 file changed, 97 insertions(+), 6 deletions(-)
diff --git a/sound/soc/amd/ps/ps-pdm-dma.c b/sound/soc/amd/ps/ps-pdm-dma.c
index 04c014349347..6d0e06b50002 100644
--- a/sound/soc/amd/ps/ps-pdm-dma.c
+++ b/sound/soc/amd/ps/ps-pdm-dma.c
@@ -5,6 +5,7 @@
* Copyright 2022, 2025 Advanced Micro Devices, Inc.
*/
+#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/module.h>
#include <linux/bitfield.h>
@@ -19,6 +20,17 @@
#define DRV_NAME "acp_ps_pdm_dma"
+/*
+ * 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 pdm_gain = 3;
module_param(pdm_gain, int, 0644);
MODULE_PARM_DESC(pdm_gain, "Gain control (0-3)");
@@ -107,12 +119,19 @@ static int acp63_start_pdm_dma(void __iomem *acp_base)
u32 pdm_dma_enable;
int timeout;
- pdm_enable = 0x01;
- pdm_dma_enable = 0x01;
-
acp63_enable_pdm_clock(acp_base);
- writel(pdm_enable, acp_base + ACP_WOV_PDM_ENABLE);
- writel(pdm_dma_enable, acp_base + ACP_WOV_PDM_DMA_ENABLE);
+
+ /*
+ * PDM_ENABLE and the clock were already set in prepare() for the
+ * initial stream start to allow the CIC filter to settle. Only
+ * write PDM_ENABLE if it is not already set, to cover the
+ * RESUME and PAUSE_RELEASE paths where prepare() was not called.
+ */
+ pdm_enable = readl(acp_base + ACP_WOV_PDM_ENABLE);
+ if (!(pdm_enable & ACP_PDM_ENABLE))
+ writel(ACP_PDM_ENABLE, acp_base + ACP_WOV_PDM_ENABLE);
+
+ writel(0x01, acp_base + ACP_WOV_PDM_DMA_ENABLE);
timeout = 0;
while (++timeout < ACP_COUNTER) {
pdm_dma_enable = readl(acp_base + ACP_WOV_PDM_DMA_ENABLE);
@@ -154,6 +173,32 @@ static int acp63_stop_pdm_dma(void __iomem *acp_base)
return 0;
}
+static int acp63_pause_pdm_dma(void __iomem *acp_base)
+{
+ u32 pdm_dma_enable;
+ int timeout;
+
+ /*
+ * Stop only the DMA; leave the PDM decimator running so that
+ * PAUSE_RELEASE does not need to wait for the CIC filter to
+ * settle again. The caller is responsible for flushing the FIFO
+ * before restarting the DMA on PAUSE_RELEASE.
+ */
+ pdm_dma_enable = readl(acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ if (!(pdm_dma_enable & 0x01))
+ return 0;
+
+ writel(0x02, acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ timeout = 0;
+ while (++timeout < ACP_COUNTER) {
+ pdm_dma_enable = readl(acp_base + ACP_WOV_PDM_DMA_ENABLE);
+ if ((pdm_dma_enable & 0x02) == 0x00)
+ return 0;
+ udelay(DELAY_US);
+ }
+ return -ETIMEDOUT;
+}
+
static void acp63_config_dma(struct pdm_stream_instance *rtd, int direction)
{
u16 page_idx;
@@ -286,6 +331,29 @@ static int acp63_pdm_dma_close(struct snd_soc_component *component,
return 0;
}
+static int acp63_pdm_dma_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct pdm_stream_instance *rtd = substream->runtime->private_data;
+
+ if (!rtd || substream->stream != SNDRV_PCM_STREAM_CAPTURE)
+ return 0;
+
+ /*
+ * 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.
+ */
+ acp63_enable_pdm_clock(rtd->acp63_base);
+ writel(ACP_PDM_ENABLE, rtd->acp63_base + ACP_WOV_PDM_ENABLE);
+ msleep(PDM_SETTLING_DELAY_MS);
+ writel(0x01, rtd->acp63_base + ACP_WOV_PDM_FIFO_FLUSH);
+ return 0;
+}
+
static int acp63_pdm_dai_trigger(struct snd_pcm_substream *substream,
int cmd, struct snd_soc_dai *dai)
{
@@ -306,21 +374,43 @@ static int acp63_pdm_dai_trigger(struct snd_pcm_substream *substream,
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
+ writel(ch_mask, rtd->acp63_base + ACP_WOV_PDM_NO_OF_CHANNELS);
+ writel(PDM_DECIMATION_FACTOR, rtd->acp63_base + ACP_WOV_PDM_DECIMATION_FACTOR);
+ rtd->bytescount = acp63_pdm_get_byte_count(rtd, substream->stream);
+ pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
+ if (!pdm_status)
+ ret = acp63_start_pdm_dma(rtd->acp63_base);
+ break;
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
writel(ch_mask, rtd->acp63_base + ACP_WOV_PDM_NO_OF_CHANNELS);
writel(PDM_DECIMATION_FACTOR, rtd->acp63_base + ACP_WOV_PDM_DECIMATION_FACTOR);
rtd->bytescount = acp63_pdm_get_byte_count(rtd, substream->stream);
+ /*
+ * The PDM decimator was left running during PAUSE_PUSH so no
+ * 300 ms settling delay is needed. Flush the FIFO to discard
+ * samples accumulated while the DMA was stopped, then restart
+ * the DMA.
+ */
+ writel(0x01, rtd->acp63_base + ACP_WOV_PDM_FIFO_FLUSH);
pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
if (!pdm_status)
ret = acp63_start_pdm_dma(rtd->acp63_base);
break;
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
- case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
if (pdm_status)
ret = acp63_stop_pdm_dma(rtd->acp63_base);
break;
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ /*
+ * Leave the PDM decimator running so that PAUSE_RELEASE does
+ * not need the 300 ms CIC settling delay. Only stop the DMA.
+ */
+ pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
+ if (pdm_status)
+ ret = acp63_pause_pdm_dma(rtd->acp63_base);
+ break;
default:
ret = -EINVAL;
break;
@@ -329,6 +419,7 @@ static int acp63_pdm_dai_trigger(struct snd_pcm_substream *substream,
}
static const struct snd_soc_dai_ops acp63_pdm_dai_ops = {
+ .prepare = acp63_pdm_dma_prepare,
.trigger = acp63_pdm_dai_trigger,
};
--
2.48.1
From 4c141cc958abacc1204f02004e7c484e071c1764 Mon Sep 17 00:00:00 2001
From: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
Date: Thu, 13 Aug 2026 09:33:46 +0530
Subject: [PATCH 2/2] ASoC: amd: ps: pdm: fix decimator transient on system
resume
After a system suspend/resume cycle with an active ALSA capture,
TRIGGER_RESUME fires after TRIGGER_SUSPEND without an intervening
prepare() call. TRIGGER_SUSPEND calls acp63_stop_pdm_dma() which
disables both the DMA and the PDM decimator. TRIGGER_RESUME then
called acp63_start_pdm_dma() which re-enables PDM and DMA together
without the 300 ms CIC settling delay or the FIFO flush, producing the
same Nyquist-rate transient that the prepare() fix was designed to
prevent. On hardware the transient caused an immediate XRUN and
broken-pipe error after wake.
Fix by separating TRIGGER_RESUME from TRIGGER_START and calling the
new acp63_pdm_settle() helper before arming the DMA. The helper
encapsulates the enable-clock / enable-PDM / msleep / flush-FIFO
sequence that was previously open-coded in prepare(); prepare() is
updated to call it through the helper too. TRIGGER_RESUME is invoked
from PM resume process context, so msleep() is safe.
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
---
sound/soc/amd/ps/ps-pdm-dma.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/sound/soc/amd/ps/ps-pdm-dma.c b/sound/soc/amd/ps/ps-pdm-dma.c
index 6d0e06b50002..e665d147ec11 100644
--- a/sound/soc/amd/ps/ps-pdm-dma.c
+++ b/sound/soc/amd/ps/ps-pdm-dma.c
@@ -331,6 +331,20 @@ static int acp63_pdm_dma_close(struct snd_soc_component *component,
return 0;
}
+/*
+ * acp63_pdm_settle - enable the PDM decimator and wait for it to stabilise.
+ *
+ * Called from prepare() and from TRIGGER_RESUME (where prepare() is not
+ * invoked by the ALSA core). Must not be called from atomic context.
+ */
+static void acp63_pdm_settle(void __iomem *acp_base)
+{
+ acp63_enable_pdm_clock(acp_base);
+ writel(ACP_PDM_ENABLE, acp_base + ACP_WOV_PDM_ENABLE);
+ msleep(PDM_SETTLING_DELAY_MS);
+ writel(0x01, acp_base + ACP_WOV_PDM_FIFO_FLUSH);
+}
+
static int acp63_pdm_dma_prepare(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
@@ -347,10 +361,7 @@ static int acp63_pdm_dma_prepare(struct snd_pcm_substream *substream,
* a saturated Nyquist-rate transient. Wait for the decimator
* to flush, then clear the FIFO so the DMA sees a clean buffer.
*/
- acp63_enable_pdm_clock(rtd->acp63_base);
- writel(ACP_PDM_ENABLE, rtd->acp63_base + ACP_WOV_PDM_ENABLE);
- msleep(PDM_SETTLING_DELAY_MS);
- writel(0x01, rtd->acp63_base + ACP_WOV_PDM_FIFO_FLUSH);
+ acp63_pdm_settle(rtd->acp63_base);
return 0;
}
@@ -373,10 +384,25 @@ static int acp63_pdm_dai_trigger(struct snd_pcm_substream *substream,
}
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
+ writel(ch_mask, rtd->acp63_base + ACP_WOV_PDM_NO_OF_CHANNELS);
+ writel(PDM_DECIMATION_FACTOR, rtd->acp63_base + ACP_WOV_PDM_DECIMATION_FACTOR);
+ rtd->bytescount = acp63_pdm_get_byte_count(rtd, substream->stream);
+ pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
+ if (!pdm_status)
+ ret = acp63_start_pdm_dma(rtd->acp63_base);
+ break;
case SNDRV_PCM_TRIGGER_RESUME:
writel(ch_mask, rtd->acp63_base + ACP_WOV_PDM_NO_OF_CHANNELS);
writel(PDM_DECIMATION_FACTOR, rtd->acp63_base + ACP_WOV_PDM_DECIMATION_FACTOR);
rtd->bytescount = acp63_pdm_get_byte_count(rtd, substream->stream);
+ /*
+ * TRIGGER_SUSPEND called acp63_stop_pdm_dma(), which disabled
+ * the PDM decimator. prepare() is not called between SUSPEND
+ * and RESUME, so re-apply the settling sequence here before
+ * arming the DMA. TRIGGER_RESUME is invoked from PM resume
+ * context, so msleep() is safe.
+ */
+ acp63_pdm_settle(rtd->acp63_base);
pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base);
if (!pdm_status)
ret = acp63_start_pdm_dma(rtd->acp63_base);
--
2.48.1