* ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start @ 2026-08-05 18:11 Robin Everaars 2026-08-10 5:11 ` Mukunda,Vijendar 0 siblings, 1 reply; 10+ messages in thread From: Robin Everaars @ 2026-08-05 18:11 UTC (permalink / raw) To: Vijendar.Mukunda, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 4682 bytes --] 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. 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. 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 #1.2: publickey - robineveraars@pm.me - 0x8B6BA132.asc --] [-- Type: application/pgp-keys, Size: 889 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 322 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-05 18:11 ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start Robin Everaars @ 2026-08-10 5:11 ` Mukunda,Vijendar 2026-08-11 12:33 ` Robin Everaars 0 siblings, 1 reply; 10+ messages in thread From: Mukunda,Vijendar @ 2026-08-10 5:11 UTC (permalink / raw) To: Robin Everaars, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar [-- 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-10 5:11 ` Mukunda,Vijendar @ 2026-08-11 12:33 ` Robin Everaars 2026-08-11 15:27 ` Mukunda,Vijendar 2026-08-12 9:38 ` Mukunda,Vijendar 0 siblings, 2 replies; 10+ messages in thread From: Robin Everaars @ 2026-08-11 12:33 UTC (permalink / raw) To: Mukunda,Vijendar, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar [-- Attachment #1.1: Type: text/plain, Size: 1531 bytes --] > Could you please try attached patch? Thanks. I checked the bound driver before building it and found an error in my original report: I named acp-pdm.c, but that is not the path used by this machine. The PX13 is bound as follows: /sys/bus/platform/devices/acp_ps_pdm_dma.0/driver/module -> /sys/module/snd_ps_pdm_dma snd_ps_pdm_dma: sound/soc/amd/ps/snd-ps-pdm-dma.ko.xz platform alias acp_ps_pdm_dma The attached patch changes sound/soc/amd/acp/acp-pdm.c, which builds snd-acp-pdm. That module is installed but does not own this PCM, so applying the patch as-is would not exercise the reported hardware. The corresponding start sequence is acp63_start_pdm_dma() in sound/soc/amd/ps/ps-pdm-dma.c. It also enables the PDM clock, decimator and DMA without a settling interval. I can port the 300 ms delay and FIFO flush there and test it, but I would prefer to test the exact shape you want upstream. Would you send an equivalent patch for snd-ps-pdm-dma, or would you like me to prepare the test diff? One detail may also need handling in both versions. PAUSE_PUSH currently takes the stop path and disables PDM. After moving PDM_ENABLE to prepare(), PAUSE_RELEASE arms only the DMA, and ALSA does not necessarily call prepare() between those commands. Should PAUSE_PUSH leave the decimator running, or should PAUSE_RELEASE use a separate non-sleeping re-enable path? Sorry for pointing the original report at the wrong driver. The measurements and PCM are unchanged; only my source attribution was wrong. [-- Attachment #1.2: publickey - robineveraars@pm.me - 0x8B6BA132.asc --] [-- Type: application/pgp-keys, Size: 889 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 322 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-11 12:33 ` Robin Everaars @ 2026-08-11 15:27 ` Mukunda,Vijendar 2026-08-12 9:38 ` Mukunda,Vijendar 1 sibling, 0 replies; 10+ messages in thread From: Mukunda,Vijendar @ 2026-08-11 15:27 UTC (permalink / raw) To: Robin Everaars, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar On 8/11/26 18:03, Robin Everaars wrote: >> Could you please try attached patch? > Thanks. I checked the bound driver before building it and found an error in my > original report: I named acp-pdm.c, but that is not the path used by this > machine. > > The PX13 is bound as follows: > > /sys/bus/platform/devices/acp_ps_pdm_dma.0/driver/module > -> /sys/module/snd_ps_pdm_dma > > snd_ps_pdm_dma: > sound/soc/amd/ps/snd-ps-pdm-dma.ko.xz > platform alias acp_ps_pdm_dma > > The attached patch changes sound/soc/amd/acp/acp-pdm.c, which builds > snd-acp-pdm. That module is installed but does not own this PCM, so applying the > patch as-is would not exercise the reported hardware. > > The corresponding start sequence is acp63_start_pdm_dma() in > sound/soc/amd/ps/ps-pdm-dma.c. It also enables the PDM clock, decimator and DMA > without a settling interval. I can port the 300 ms delay and FIFO flush there > and test it, but I would prefer to test the exact shape you want upstream. Would > you send an equivalent patch for snd-ps-pdm-dma, > or would you like me to prepare > the test diff? Will share the patch. > One detail may also need handling in both versions. PAUSE_PUSH currently takes > the stop path and disables PDM. After moving PDM_ENABLE to prepare(), > PAUSE_RELEASE arms only the DMA, and ALSA does not necessarily call prepare() > between those commands. Should PAUSE_PUSH leave the decimator running, or > should PAUSE_RELEASE use a separate non-sleeping re-enable path? > > Sorry for pointing the original report at the wrong driver. The measurements > and PCM are unchanged; only my source attribution was wrong. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-11 12:33 ` Robin Everaars 2026-08-11 15:27 ` Mukunda,Vijendar @ 2026-08-12 9:38 ` Mukunda,Vijendar 2026-08-12 13:41 ` Robin Everaars 1 sibling, 1 reply; 10+ messages in thread From: Mukunda,Vijendar @ 2026-08-12 9:38 UTC (permalink / raw) To: Robin Everaars, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar, Syed Saba Kareem [-- Attachment #1: Type: text/plain, Size: 1802 bytes --] On 8/11/26 18:03, Robin Everaars wrote: >> Could you please try attached patch? > Thanks. I checked the bound driver before building it and found an error in my > original report: I named acp-pdm.c, but that is not the path used by this > machine. > > The PX13 is bound as follows: > > /sys/bus/platform/devices/acp_ps_pdm_dma.0/driver/module > -> /sys/module/snd_ps_pdm_dma > > snd_ps_pdm_dma: > sound/soc/amd/ps/snd-ps-pdm-dma.ko.xz > platform alias acp_ps_pdm_dma > > The attached patch changes sound/soc/amd/acp/acp-pdm.c, which builds > snd-acp-pdm. That module is installed but does not own this PCM, so applying the > patch as-is would not exercise the reported hardware. > > The corresponding start sequence is acp63_start_pdm_dma() in > sound/soc/amd/ps/ps-pdm-dma.c. It also enables the PDM clock, decimator and DMA > without a settling interval. I can port the 300 ms delay and FIFO flush there > and test it, but I would prefer to test the exact shape you want upstream. Would > you send an equivalent patch for snd-ps-pdm-dma, > or would you like me to prepare > the test diff? > > One detail may also need handling in both versions. PAUSE_PUSH currently takes > the stop path and disables PDM. After moving PDM_ENABLE to prepare(), > PAUSE_RELEASE arms only the DMA, and ALSA does not necessarily call prepare() > between those commands. Should PAUSE_PUSH leave the decimator running, or > should PAUSE_RELEASE use a separate non-sleeping re-enable path? PAUSE_PUSH should leave the decimator running. That is the safer ALSA design and avoids both the transient and a 300 ms pause-resume latency. Try attached patch. > > Sorry for pointing the original report at the wrong driver. The measurements > and PCM are unchanged; only my source attribution was wrong. [-- Attachment #2: 0001-ASoC-amd-ps-pdm-fix-decimator-transient-on-stream-st.patch --] [-- Type: text/x-patch, Size: 8374 bytes --] From e59d4aead850caff2ed0b293ebfcdb19ae5e8721 Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda <Vijendar.Mukunda@amd.com> Date: Wed, 12 Aug 2026 14:41:42 +0530 Subject: [PATCH] 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@amd.com> --- 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-12 9:38 ` Mukunda,Vijendar @ 2026-08-12 13:41 ` Robin Everaars 2026-08-13 5:10 ` Mukunda,Vijendar 0 siblings, 1 reply; 10+ messages in thread From: Robin Everaars @ 2026-08-12 13:41 UTC (permalink / raw) To: Mukunda,Vijendar, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar, Syed Saba Kareem [-- Attachment #1.1: Type: text/plain, Size: 2042 bytes --] > PAUSE_PUSH should leave the decimator running. That is the safer ALSA > design and avoids both the transient and a 300 ms pause-resume latency. > > Try attached patch. I tested the exact attachment against v7.1.7 on the ASUS ProArt PX13 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. Thanks, Robin [-- Attachment #1.2: publickey - robineveraars@pm.me - 0x8B6BA132.asc --] [-- Type: application/pgp-keys, Size: 889 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 322 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-12 13:41 ` Robin Everaars @ 2026-08-13 5:10 ` Mukunda,Vijendar 2026-08-13 12:16 ` Robin Everaars 0 siblings, 1 reply; 10+ messages in thread From: Mukunda,Vijendar @ 2026-08-13 5:10 UTC (permalink / raw) To: Robin Everaars, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar, Syed Saba Kareem [-- Attachment #1: Type: text/plain, Size: 2212 bytes --] On 8/12/26 19:11, Robin Everaars wrote: >> PAUSE_PUSH should leave the decimator running. That is the safer ALSA >> design and avoids both the transient and a 300 ms pause-resume latency. >> >> Try attached patch. > I tested the exact attachment against v7.1.7 on the ASUS ProArt PX13 > 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. Please try attached patches. > > Thanks, > Robin [-- Attachment #2: 0001-ASoC-amd-ps-pdm-fix-decimator-transient-on-stream-st.patch --] [-- Type: text/x-patch, Size: 8379 bytes --] From 729d30468c9dcb49aa4625af7d95ffc3c9d569b8 Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda <Vijendar.Mukunda@amd.com> 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@amd.com> --- 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 [-- Attachment #3: 0002-ASoC-amd-ps-pdm-fix-decimator-transient-on-system-re.patch --] [-- Type: text/x-patch, Size: 3977 bytes --] From 4c141cc958abacc1204f02004e7c484e071c1764 Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda <Vijendar.Mukunda@amd.com> 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@amd.com> --- 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-13 5:10 ` Mukunda,Vijendar @ 2026-08-13 12:16 ` Robin Everaars 2026-08-13 12:26 ` Robin Everaars 0 siblings, 1 reply; 10+ messages in thread From: Robin Everaars @ 2026-08-13 12:16 UTC (permalink / raw) To: Mukunda,Vijendar, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar, Syed Saba Kareem [-- Attachment #1.1: Type: text/plain, Size: 2253 bytes --] > Please try attached patches. I tested the exact two attachments against v7.1.7 on the ASUS ProArt PX13 HN7306EAC. Their SHA-256 values were: patch 1 c7e60e6850e3e61582d00a419f20afba863cee8253a3c5afe79a80de9464d8d8 patch 2 ceda8170c0307415c8d2aabea652b667391801c19a2c34e67eca3216b750f5bd The ordinary paths remain clean: - 5/5 runtime-suspended cold starts and 5/5 warm starts delivered no clipped or >=99% full-scale samples. - PAUSE_PUSH/PAUSE_RELEASE passed 3/3 with a 0.5 s pause and 3/3 with a 2 s pause. All six complete recordings had no clipped or >=99% full-scale samples. - prepare followed by close passed 3/3, with prepare taking 303 to 304 ms. Runtime PM suspended the device afterward. - forced XRUN/reprepare recovery passed 5/5, with clean recovered blocks and prepare times of 304 to 308 ms. One initial cold attempt returned -EIO before delivering data. It did not recur in the next five runtime-suspended starts and ther e was no matching kernel diagnostic, so I have kept that observation separate from the reproducible system-resume result below. Patch 2 cannot sleep in the trigger callback on this path. During a true active-capture s2idle test, ALSA reached SUSPENDED at frame 90128. On resume the kernel reported: BUG: scheduling while atomic: pdm-alsa-lifecy/13258/0x00000002 __schedule_bug.cold msleep acp63_pdm_dai_trigger+0x10d/0x2fc [snd_ps_pdm_dma] soc_dai_trigger.part.0 [snd_soc_core] snd_soc_pcm_dai_trigger [snd_soc_core] soc_pcm_trigger [snd_soc_core] The comment stating that TRIGGER_RESUME runs in a sleepable PM resume context does not hold for this ASoC trigger path. snd_pcm_resume() never returned to the test program, no post-resume sample was delivered and the program later received SIGSEGV while querying the PCM state in libasound. The kernel logged a second scheduling-while-atomic warning during core dump handling. A fresh capture after resume worke d and delivered 48000 frames with no clipped or >=99% full-scale samples. The settling operation therefore needs a sleepable path before DMA is armed, rather than msleep() inside the trigger callback. I have withheld Tested-by and can test another revision on the same hardware. Thanks, Robin [-- Attachment #1.2: publickey - robineveraars@pm.me - 0x8B6BA132.asc --] [-- Type: application/pgp-keys, Size: 889 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 322 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-13 12:16 ` Robin Everaars @ 2026-08-13 12:26 ` Robin Everaars 2026-08-14 8:35 ` Mukunda,Vijendar 0 siblings, 1 reply; 10+ messages in thread From: Robin Everaars @ 2026-08-13 12:26 UTC (permalink / raw) To: Mukunda,Vijendar, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar, Syed Saba Kareem [-- Attachment #1.1: Type: text/plain, Size: 702 bytes --] One clarification about the isolated -EIO mentioned above: I removed patch 2, rebuilt and rebooted into the earlier patch-1-only module. Its SHA-256 was: cd55e119961b80066c66ae7abb5f05ca97fa2747de7887a17ee45c724f19fc9d The first runtime-suspended capture after that reboot again returned -EIO before delivering data, with no matching kernel diagnostic. The next three runtime-suspended captures each delivered all 48000 frames with no clipped or >=99% full-scale samples. I therefore do not attribute the isolated -EIO to patch 2. The reproducible patch-2 regression remains the scheduling-while-atomic failure from msleep() in acp63_pdm_dai_trigger() during active-capture resume. Thanks, Robin [-- Attachment #1.2: publickey - robineveraars@pm.me - 0x8B6BA132.asc --] [-- Type: application/pgp-keys, Size: 889 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 322 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start 2026-08-13 12:26 ` Robin Everaars @ 2026-08-14 8:35 ` Mukunda,Vijendar 0 siblings, 0 replies; 10+ messages in thread From: Mukunda,Vijendar @ 2026-08-14 8:35 UTC (permalink / raw) To: Robin Everaars, linux-sound Cc: venkataprasad.potturu, lgirdwood, broonie, linux-kernel, Mario Limonciello, Dommati, Sunil-kumar, Syed Saba Kareem [-- Attachment #1: Type: text/plain, Size: 3231 bytes --] On 8/13/26 17:56, Robin Everaars wrote: > One clarification about the isolated -EIO mentioned above: I removed patch 2, > rebuilt and rebooted into the earlier patch-1-only module. Its SHA-256 was: > > cd55e119961b80066c66ae7abb5f05ca97fa2747de7887a17ee45c724f19fc9d > > The first runtime-suspended capture after that reboot again returned -EIO > before delivering data, with no matching kernel diagnostic. The next three > runtime-suspended captures each delivered all 48000 frames with no clipped or >> =99% full-scale samples. > I therefore do not attribute the isolated -EIO to patch 2. The reproducible > patch-2 regression remains the scheduling-while-atomic failure from msleep() > in acp63_pdm_dai_trigger() during active-capture resume. Thank you for the detailed testing and for helping isolate both issues. The patch-1-only module result is conclusive: the first cold-start -EIO occurs with patch 1 alone, is not introduced by patch 2, and self-recovers on the next attempt without any corresponding kernel diagnostic. Since the failure does not reproduce across subsequent runtime-suspended starts, it appears to be a pre-existing intermittent condition, most likely related to hardware timing during the first DMA transaction after a cold ACP power-on. We will continue tracking it separately, but it does not affect the correctness of either patch in this series. Regarding the scheduling-while-atomic warning observed with patch 2, your analysis is correct. The original assumption that TRIGGER_RESUME runs in a sleepable PM resume context was incorrect. During resume, snd_pcm_resume() dispatches TRIGGER_RESUME through snd_pcm_action_lock_irq(), which holds the PCM stream spinlock with IRQs disabled before invoking the DAI trigger callback. As a result, calling msleep() from acp63_pdm_dai_trigger() can trigger a scheduling-while-atomic BUG. To address this, the revised series removes acp63_pdm_settle() from the trigger path and moves it into acp63_pdm_resume(), the system-sleep PM resume hook (SYSTEM_SLEEP_PM_OPS). This resume callback executes in a sleepable process context and completes before the PCM layer issues TRIGGER_RESUME, ensuring the CIC filter has fully settled before DMA is armed. The trigger callback is therefore reduced to programming the DMA path without any sleep operations. In addition, the PDM DAI link is explicitly marked nonatomic = 1 in the machine driver. While the existing prepare() path was not responsible for the BUG because snd_pcm_prepare() already uses the nonatomic action path, the DAI operations legitimately require sleeping and the nonatomic declaration aligns the machine-driver configuration with those requirements, helping to avoid future regressions. With these updates, the scheduling-while-atomic warning remains the only reproducible regression attributable to patch 2 and is addressed by the revised implementation. Could you please re-test with the updated patches, particularly the active-capture s2idle resume scenario that originally triggered the BUG? The expected result is a clean resume with no scheduling-while-atomic warning and successful capture continuation after resume without requiring a fresh stream restart. > Thanks, > Robin [-- Attachment #2: 0000-cover-letter.patch --] [-- Type: text/x-patch, Size: 2626 bytes --] From 56300d48776be7f8424ca96b5ca46b74383ba9d1 Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda <Vijendar.Mukunda@amd.com> Date: Fri, 14 Aug 2026 12:42:27 +0530 Subject: [PATCH V2 0/3] ASoC: amd: ps: pdm: fix CIC decimator transients and resume BUG The ACP PDM DMIC on Pink Sardine (ACP6.3/7.0/7.1) emits a full-scale Nyquist-rate burst at the start of every capture stream and after a system suspend/resume cycle. This series fixes both cases and also corrects a scheduling-while-atomic BUG introduced in v1 of the resume fix. Patch 1 adds a prepare() DAI callback that enables the PDM decimator and waits 300 ms for the CIC filter chain to stabilise before the DMA is armed. A new acp63_pause_pdm_dma() helper keeps the decimator running across PAUSE_PUSH/PAUSE_RELEASE so no settling delay is needed on pause resume. Patch 2 extends the fix to the system resume path. After TRIGGER_SUSPEND the decimator is disabled; because prepare() is not called before TRIGGER_RESUME, the settle sequence must run elsewhere. It is placed in acp63_pdm_resume() (SYSTEM_SLEEP_PM_OPS resume hook), which runs in sleepable process context before the PCM layer issues TRIGGER_RESUME. A shared acp63_pdm_settle() helper is factored out so prepare() and acp63_pdm_resume() use identical logic. Patch 3 marks the PDM DAI link nonatomic = 1 in the machine driver to match the sleep requirements of the ops registered on the link. Changes in v2: - Patch 1: no code changes. - Patch 2: fix scheduling-while-atomic BUG on s2idle resume reported by the reviewer. v1 called acp63_pdm_settle() (msleep(300)) inside TRIGGER_RESUME, which runs under the PCM stream spinlock with IRQs disabled via snd_pcm_action_lock_irq(). This triggered: BUG: scheduling while atomic: .../0x00000002 msleep acp63_pdm_dai_trigger+0x10d/0x2fc [snd_ps_pdm_dma] soc_dai_trigger.part.0 [snd_soc_core] Fix: move acp63_pdm_settle() from the trigger callback to acp63_pdm_resume(), which is sleepable. Introduce the acp63_pdm_settle() helper so prepare() and acp63_pdm_resume() share the sequence without duplication. - Patch 3: new in v2. Marks the DAI link nonatomic = 1, to match the sleep requirements of the registered ops. Vijendar Mukunda (3): ASoC: amd: ps: pdm: fix decimator transient on stream start ASoC: amd: ps: pdm: fix decimator transient on system resume ASoC: amd: ps: pdm: set PDM DAI link as non-atomic sound/soc/amd/ps/ps-mach.c | 1 + sound/soc/amd/ps/ps-pdm-dma.c | 136 ++++++++++++++++++++++++++++++++-- 2 files changed, 131 insertions(+), 6 deletions(-) -- 2.48.1 [-- Attachment #3: 0001-ASoC-amd-ps-pdm-fix-decimator-transient-on-stream-st.patch --] [-- Type: text/x-patch, Size: 8366 bytes --] From e0bc5065c65a6b7947a55ae4cd8d67826009f777 Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda <Vijendar.Mukunda@amd.com> Date: Wed, 12 Aug 2026 14:41:42 +0530 Subject: [PATCH V2 1/3] 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@amd.com> --- 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..e8f84d83a886 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 [-- Attachment #4: 0002-ASoC-amd-ps-pdm-fix-decimator-transient-on-system-re.patch --] [-- Type: text/x-patch, Size: 4793 bytes --] From 1f401ac36327baf67554612b01fd2831a1ac59eb Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda <Vijendar.Mukunda@amd.com> Date: Fri, 14 Aug 2026 11:38:45 +0530 Subject: [PATCH V2 2/3] 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 adding the new acp63_pdm_settle() helper that encapsulates the enable-clock / enable-PDM / msleep / flush-FIFO sequence, and calling it from acp63_pdm_resume(), the system-sleep PM resume hook, which runs in sleepable process context before the PCM layer issues TRIGGER_RESUME. prepare() is also updated to call through this helper. TRIGGER_RESUME runs under the PCM stream spinlock (IRQs disabled via snd_pcm_action_lock_irq()), so msleep() is not permitted inside the trigger callback. Placing the settle in acp63_pdm_resume() ensures the CIC filter is stable before TRIGGER_RESUME fires, without sleeping in atomic context. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com> --- sound/soc/amd/ps/ps-pdm-dma.c | 41 +++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/sound/soc/amd/ps/ps-pdm-dma.c b/sound/soc/amd/ps/ps-pdm-dma.c index e8f84d83a886..e48dfaf4908d 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,24 @@ 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); + /* + * acp63_pdm_resume() already called acp63_pdm_settle() in + * sleepable PM resume context before the PCM layer issued + * TRIGGER_RESUME. The trigger callback runs under the PCM + * stream spinlock (irqs disabled), so no msleep() is allowed + * here. Just arm the DMA. + */ pdm_status = acp63_check_pdm_dma_status(rtd->acp63_base); if (!pdm_status) ret = acp63_start_pdm_dma(rtd->acp63_base); @@ -510,6 +535,14 @@ static int acp63_pdm_resume(struct device *dev) acp63_config_dma(rtd, SNDRV_PCM_STREAM_CAPTURE); acp63_init_pdm_ring_buffer(PDM_MEM_WINDOW_START, buffer_len, period_bytes, adata->acp63_base); + /* + * TRIGGER_SUSPEND disabled the PDM decimator. The PCM layer + * will issue TRIGGER_RESUME under the stream spinlock (IRQs + * disabled), so msleep() is not permitted there. Run the + * CIC settling sequence here in sleepable PM resume context + * before the trigger fires. + */ + acp63_pdm_settle(rtd->acp63_base); } acp63_enable_pdm_interrupts(adata); return 0; -- 2.48.1 [-- Attachment #5: 0003-ASoC-amd-ps-pdm-set-PDM-DAI-link-as-non-atomic.patch --] [-- Type: text/x-patch, Size: 1155 bytes --] From 56300d48776be7f8424ca96b5ca46b74383ba9d1 Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda <Vijendar.Mukunda@amd.com> Date: Fri, 14 Aug 2026 11:39:10 +0530 Subject: [PATCH V2 3/3] ASoC: amd: ps: pdm: set PDM DAI link as non-atomic The prepare() DAI callback calls msleep() via acp63_pdm_settle() to allow the CIC filter to settle before DMA starts. Mark the DAI link nonatomic = 1 so the trigger path is also nonatomic, matching the sleep requirements of the ops registered on this link and preventing any future msleep() inside the trigger from hitting a scheduling-while- atomic BUG. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com> --- sound/soc/amd/ps/ps-mach.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/amd/ps/ps-mach.c b/sound/soc/amd/ps/ps-mach.c index 3e49c255c025..a040b5fcb0b7 100644 --- a/sound/soc/amd/ps/ps-mach.c +++ b/sound/soc/amd/ps/ps-mach.c @@ -32,6 +32,7 @@ static struct snd_soc_dai_link acp63_dai_pdm[] = { .name = "acp63-dmic-capture", .stream_name = "DMIC capture", .capture_only = 1, + .nonatomic = 1, SND_SOC_DAILINK_REG(acp63_pdm, dmic_codec, pdm_platform), }, }; -- 2.48.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-14 8:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-05 18:11 ASoC: amd: acp-pdm: full-scale burst on every DMIC capture start Robin Everaars 2026-08-10 5:11 ` Mukunda,Vijendar 2026-08-11 12:33 ` Robin Everaars 2026-08-11 15:27 ` Mukunda,Vijendar 2026-08-12 9:38 ` Mukunda,Vijendar 2026-08-12 13:41 ` Robin Everaars 2026-08-13 5:10 ` Mukunda,Vijendar 2026-08-13 12:16 ` Robin Everaars 2026-08-13 12:26 ` Robin Everaars 2026-08-14 8:35 ` Mukunda,Vijendar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®