Hi,
The defconfig for OMAP2+ now have the CPU_IDLE enabled which can cause audio artifacts because we try to enter too low power state from where the wakeup takes longer than the FIFO can tolerate on the dai side.
While adding pm_qos to McPDM and DMIC I have noticed that the McBSP calculation was not correct as we need usec for the latency value.
Regards, Peter --- Peter Ujfalusi (3): ASoC: omap-mcbsp: Fix latency value calculation for pm_qos ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE
sound/soc/omap/omap-dmic.c | 9 ++++++++ sound/soc/omap/omap-mcbsp.c | 6 +++--- sound/soc/omap/omap-mcpdm.c | 43 ++++++++++++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-)
The latency number is in usec for the pm_qos. Correct the calculation to give us the time in usec
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-mcbsp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index e0ef4774c710..a395598f1f20 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -999,9 +999,9 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, pkt_size = channels; }
- latency = ((((buffer_size - pkt_size) / channels) * 1000) - / (params->rate_num / params->rate_den)); - + latency = (buffer_size - pkt_size) / channels; + latency = latency * USEC_PER_SEC / + (params->rate_num / params->rate_den); mcbsp->latency[substream->stream] = latency;
omap_mcbsp_set_threshold(substream, pkt_size);
On 11/14/18 1:06 PM, Peter Ujfalusi wrote:
The latency number is in usec for the pm_qos. Correct the calculation to give us the time in usec
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/omap-mcbsp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index e0ef4774c710..a395598f1f20 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -999,9 +999,9 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, pkt_size = channels; }
latency = ((((buffer_size - pkt_size) / channels) * 1000)/ (params->rate_num / params->rate_den));
latency = (buffer_size - pkt_size) / channels;latency = latency * USEC_PER_SEC /(params->rate_num / params->rate_den);
Did I understand correctly this doesn't fix any audio issue but allows CPU sleep longer or reach deeper sleep?
On 2018-11-18 19:04, Jarkko Nikula wrote:
On 11/14/18 1:06 PM, Peter Ujfalusi wrote:
The latency number is in usec for the pm_qos. Correct the calculation to give us the time in usec
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/omap-mcbsp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index e0ef4774c710..a395598f1f20 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -999,9 +999,9 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, pkt_size = channels; }
latency = ((((buffer_size - pkt_size) / channels) * 1000)/ (params->rate_num / params->rate_den));
latency = (buffer_size - pkt_size) / channels;latency = latency * USEC_PER_SEC /(params->rate_num / params->rate_den);Did I understand correctly this doesn't fix any audio issue but allows CPU sleep longer or reach deeper sleep?
PM core will block any power state from where the wake-up would take longer than the latency value.
The latency is in usec, this patch just fixing the calculation to get the time in usec.
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
The patch
ASoC: omap-mcbsp: Fix latency value calculation for pm_qos
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From dd2f52d8991af9fe0928d59ec502ba52be7bc38d Mon Sep 17 00:00:00 2001
From: Peter Ujfalusi peter.ujfalusi@ti.com Date: Wed, 14 Nov 2018 13:06:21 +0200 Subject: [PATCH] ASoC: omap-mcbsp: Fix latency value calculation for pm_qos
The latency number is in usec for the pm_qos. Correct the calculation to give us the time in usec
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Acked-by: Jarkko Nikula jarkko.nikula@bitmer.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/omap/omap-mcbsp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index d0ebb6b9bfac..2d6decbfc99e 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -308,9 +308,9 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, pkt_size = channels; }
- latency = ((((buffer_size - pkt_size) / channels) * 1000) - / (params->rate_num / params->rate_den)); - + latency = (buffer_size - pkt_size) / channels; + latency = latency * USEC_PER_SEC / + (params->rate_num / params->rate_den); mcbsp->latency[substream->stream] = latency;
omap_mcbsp_set_threshold(substream, pkt_size);
We need to block sleep states which would require longer time to leave than the time the DMA must react to the DMA request in order to keep the FIFO serviced without under of overrun.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-mcpdm.c | 43 ++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c index 4c1be36c2207..7d5bdc5a2890 100644 --- a/sound/soc/omap/omap-mcpdm.c +++ b/sound/soc/omap/omap-mcpdm.c @@ -54,6 +54,8 @@ struct omap_mcpdm { unsigned long phys_base; void __iomem *io_base; int irq; + struct pm_qos_request pm_qos_req; + int latency[2];
struct mutex mutex;
@@ -277,6 +279,9 @@ static void omap_mcpdm_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + int tx = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); + int stream1 = tx ? SNDRV_PCM_STREAM_PLAYBACK : SNDRV_PCM_STREAM_CAPTURE; + int stream2 = tx ? SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
mutex_lock(&mcpdm->mutex);
@@ -289,6 +294,14 @@ static void omap_mcpdm_dai_shutdown(struct snd_pcm_substream *substream, } }
+ if (mcpdm->latency[stream2]) + pm_qos_update_request(&mcpdm->pm_qos_req, + mcpdm->latency[stream2]); + else if (mcpdm->latency[stream1]) + pm_qos_remove_request(&mcpdm->pm_qos_req); + + mcpdm->latency[stream1] = 0; + mutex_unlock(&mcpdm->mutex); }
@@ -300,7 +313,7 @@ static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream, int stream = substream->stream; struct snd_dmaengine_dai_dma_data *dma_data; u32 threshold; - int channels; + int channels, latency; int link_mask = 0;
channels = params_channels(params); @@ -344,14 +357,25 @@ static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream,
dma_data->maxburst = (MCPDM_DN_THRES_MAX - threshold) * channels; + latency = threshold; } else { /* If playback is not running assume a stereo stream to come */ if (!mcpdm->config[!stream].link_mask) mcpdm->config[!stream].link_mask = (0x3 << 3);
dma_data->maxburst = threshold * channels; + latency = (MCPDM_DN_THRES_MAX - threshold); }
+ /* + * The DMA must act to a DMA request within latency time (usec) to avoid + * under/overflow + */ + mcpdm->latency[stream] = latency * USEC_PER_SEC / params_rate(params); + + if (!mcpdm->latency[stream]) + mcpdm->latency[stream] = 10; + /* Check if we need to restart McPDM with this stream */ if (mcpdm->config[stream].link_mask && mcpdm->config[stream].link_mask != link_mask) @@ -366,6 +390,20 @@ static int omap_mcpdm_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + struct pm_qos_request *pm_qos_req = &mcpdm->pm_qos_req; + int tx = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); + int stream1 = tx ? SNDRV_PCM_STREAM_PLAYBACK : SNDRV_PCM_STREAM_CAPTURE; + int stream2 = tx ? SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK; + int latency = mcpdm->latency[stream2]; + + /* Prevent omap hardware from hitting off between FIFO fills */ + if (!latency || mcpdm->latency[stream1] < latency) + latency = mcpdm->latency[stream1]; + + if (pm_qos_request_active(pm_qos_req)) + pm_qos_update_request(pm_qos_req, latency); + else if (latency) + pm_qos_add_request(pm_qos_req, PM_QOS_CPU_DMA_LATENCY, latency);
if (!omap_mcpdm_active(mcpdm)) { omap_mcpdm_start(mcpdm); @@ -427,6 +465,9 @@ static int omap_mcpdm_remove(struct snd_soc_dai *dai) free_irq(mcpdm->irq, (void *)mcpdm); pm_runtime_disable(mcpdm->dev);
+ if (pm_qos_request_active(&mcpdm->pm_qos_req)) + pm_qos_remove_request(&mcpdm->pm_qos_req); + return 0; }
The patch
ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 373a500e34aea97971c9d71e45edad458d3da98f Mon Sep 17 00:00:00 2001
From: Peter Ujfalusi peter.ujfalusi@ti.com Date: Wed, 14 Nov 2018 13:06:22 +0200 Subject: [PATCH] ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE
We need to block sleep states which would require longer time to leave than the time the DMA must react to the DMA request in order to keep the FIFO serviced without under of overrun.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Acked-by: Jarkko Nikula jarkko.nikula@bitmer.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/omap/omap-mcpdm.c | 43 ++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c index 4c1be36c2207..7d5bdc5a2890 100644 --- a/sound/soc/omap/omap-mcpdm.c +++ b/sound/soc/omap/omap-mcpdm.c @@ -54,6 +54,8 @@ struct omap_mcpdm { unsigned long phys_base; void __iomem *io_base; int irq; + struct pm_qos_request pm_qos_req; + int latency[2];
struct mutex mutex;
@@ -277,6 +279,9 @@ static void omap_mcpdm_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + int tx = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); + int stream1 = tx ? SNDRV_PCM_STREAM_PLAYBACK : SNDRV_PCM_STREAM_CAPTURE; + int stream2 = tx ? SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
mutex_lock(&mcpdm->mutex);
@@ -289,6 +294,14 @@ static void omap_mcpdm_dai_shutdown(struct snd_pcm_substream *substream, } }
+ if (mcpdm->latency[stream2]) + pm_qos_update_request(&mcpdm->pm_qos_req, + mcpdm->latency[stream2]); + else if (mcpdm->latency[stream1]) + pm_qos_remove_request(&mcpdm->pm_qos_req); + + mcpdm->latency[stream1] = 0; + mutex_unlock(&mcpdm->mutex); }
@@ -300,7 +313,7 @@ static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream, int stream = substream->stream; struct snd_dmaengine_dai_dma_data *dma_data; u32 threshold; - int channels; + int channels, latency; int link_mask = 0;
channels = params_channels(params); @@ -344,14 +357,25 @@ static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream,
dma_data->maxburst = (MCPDM_DN_THRES_MAX - threshold) * channels; + latency = threshold; } else { /* If playback is not running assume a stereo stream to come */ if (!mcpdm->config[!stream].link_mask) mcpdm->config[!stream].link_mask = (0x3 << 3);
dma_data->maxburst = threshold * channels; + latency = (MCPDM_DN_THRES_MAX - threshold); }
+ /* + * The DMA must act to a DMA request within latency time (usec) to avoid + * under/overflow + */ + mcpdm->latency[stream] = latency * USEC_PER_SEC / params_rate(params); + + if (!mcpdm->latency[stream]) + mcpdm->latency[stream] = 10; + /* Check if we need to restart McPDM with this stream */ if (mcpdm->config[stream].link_mask && mcpdm->config[stream].link_mask != link_mask) @@ -366,6 +390,20 @@ static int omap_mcpdm_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + struct pm_qos_request *pm_qos_req = &mcpdm->pm_qos_req; + int tx = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); + int stream1 = tx ? SNDRV_PCM_STREAM_PLAYBACK : SNDRV_PCM_STREAM_CAPTURE; + int stream2 = tx ? SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK; + int latency = mcpdm->latency[stream2]; + + /* Prevent omap hardware from hitting off between FIFO fills */ + if (!latency || mcpdm->latency[stream1] < latency) + latency = mcpdm->latency[stream1]; + + if (pm_qos_request_active(pm_qos_req)) + pm_qos_update_request(pm_qos_req, latency); + else if (latency) + pm_qos_add_request(pm_qos_req, PM_QOS_CPU_DMA_LATENCY, latency);
if (!omap_mcpdm_active(mcpdm)) { omap_mcpdm_start(mcpdm); @@ -427,6 +465,9 @@ static int omap_mcpdm_remove(struct snd_soc_dai *dai) free_irq(mcpdm->irq, (void *)mcpdm); pm_runtime_disable(mcpdm->dev);
+ if (pm_qos_request_active(&mcpdm->pm_qos_req)) + pm_qos_remove_request(&mcpdm->pm_qos_req); + return 0; }
We need to block sleep states which would require longer time to leave than the time the DMA must react to the DMA request in order to keep the FIFO serviced without overrun.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-dmic.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c index fe966272bd0c..cba9645b6487 100644 --- a/sound/soc/omap/omap-dmic.c +++ b/sound/soc/omap/omap-dmic.c @@ -48,6 +48,8 @@ struct omap_dmic { struct device *dev; void __iomem *io_base; struct clk *fclk; + struct pm_qos_request pm_qos_req; + int latency; int fclk_freq; int out_freq; int clk_div; @@ -124,6 +126,8 @@ static void omap_dmic_dai_shutdown(struct snd_pcm_substream *substream,
mutex_lock(&dmic->mutex);
+ pm_qos_remove_request(&dmic->pm_qos_req); + if (!dai->active) dmic->active = 0;
@@ -228,6 +232,8 @@ static int omap_dmic_dai_hw_params(struct snd_pcm_substream *substream, /* packet size is threshold * channels */ dma_data = snd_soc_dai_get_dma_data(dai, substream); dma_data->maxburst = dmic->threshold * channels; + dmic->latency = (OMAP_DMIC_THRES_MAX - dmic->threshold) * USEC_PER_SEC / + params_rate(params);
return 0; } @@ -238,6 +244,9 @@ static int omap_dmic_dai_prepare(struct snd_pcm_substream *substream, struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai); u32 ctrl;
+ if (pm_qos_request_active(&dmic->pm_qos_req)) + pm_qos_update_request(&dmic->pm_qos_req, dmic->latency); + /* Configure uplink threshold */ omap_dmic_write(dmic, OMAP_DMIC_FIFO_CTRL_REG, dmic->threshold);
The patch
ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From ffdcc3638c58d55a6fa68b6e5dfd4fb4109652eb Mon Sep 17 00:00:00 2001
From: Peter Ujfalusi peter.ujfalusi@ti.com Date: Wed, 14 Nov 2018 13:06:23 +0200 Subject: [PATCH] ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE
We need to block sleep states which would require longer time to leave than the time the DMA must react to the DMA request in order to keep the FIFO serviced without overrun.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Acked-by: Jarkko Nikula jarkko.nikula@bitmer.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/omap/omap-dmic.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c index fe966272bd0c..cba9645b6487 100644 --- a/sound/soc/omap/omap-dmic.c +++ b/sound/soc/omap/omap-dmic.c @@ -48,6 +48,8 @@ struct omap_dmic { struct device *dev; void __iomem *io_base; struct clk *fclk; + struct pm_qos_request pm_qos_req; + int latency; int fclk_freq; int out_freq; int clk_div; @@ -124,6 +126,8 @@ static void omap_dmic_dai_shutdown(struct snd_pcm_substream *substream,
mutex_lock(&dmic->mutex);
+ pm_qos_remove_request(&dmic->pm_qos_req); + if (!dai->active) dmic->active = 0;
@@ -228,6 +232,8 @@ static int omap_dmic_dai_hw_params(struct snd_pcm_substream *substream, /* packet size is threshold * channels */ dma_data = snd_soc_dai_get_dma_data(dai, substream); dma_data->maxburst = dmic->threshold * channels; + dmic->latency = (OMAP_DMIC_THRES_MAX - dmic->threshold) * USEC_PER_SEC / + params_rate(params);
return 0; } @@ -238,6 +244,9 @@ static int omap_dmic_dai_prepare(struct snd_pcm_substream *substream, struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai); u32 ctrl;
+ if (pm_qos_request_active(&dmic->pm_qos_req)) + pm_qos_update_request(&dmic->pm_qos_req, dmic->latency); + /* Configure uplink threshold */ omap_dmic_write(dmic, OMAP_DMIC_FIFO_CTRL_REG, dmic->threshold);
Hi Peter,
Am 14.11.2018 um 12:06 schrieb Peter Ujfalusi peter.ujfalusi@ti.com:
Hi,
The defconfig for OMAP2+ now have the CPU_IDLE enabled which can cause audio artifacts because we try to enter too low power state from where the wakeup takes longer than the FIFO can tolerate on the dai side.
While adding pm_qos to McPDM and DMIC I have noticed that the McBSP calculation was not correct as we need usec for the latency value.
Regards, Peter
Peter Ujfalusi (3): ASoC: omap-mcbsp: Fix latency value calculation for pm_qos ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE
sound/soc/omap/omap-dmic.c | 9 ++++++++ sound/soc/omap/omap-mcbsp.c | 6 +++--- sound/soc/omap/omap-mcpdm.c | 43 ++++++++++++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-)
I will test asap.
BR and thanks, Nikolaus
Hi,
Am 14.11.2018 um 13:45 schrieb H. Nikolaus Schaller hns@goldelico.com:
Hi Peter,
Am 14.11.2018 um 12:06 schrieb Peter Ujfalusi peter.ujfalusi@ti.com:
Hi,
The defconfig for OMAP2+ now have the CPU_IDLE enabled which can cause audio artifacts because we try to enter too low power state from where the wakeup takes longer than the FIFO can tolerate on the dai side.
While adding pm_qos to McPDM and DMIC I have noticed that the McBSP calculation was not correct as we need usec for the latency value.
Regards, Peter
Peter Ujfalusi (3): ASoC: omap-mcbsp: Fix latency value calculation for pm_qos ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE
sound/soc/omap/omap-dmic.c | 9 ++++++++ sound/soc/omap/omap-mcbsp.c | 6 +++--- sound/soc/omap/omap-mcpdm.c | 43 ++++++++++++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-)
I will test asap.
I have now:
* v4.20-rc2 * plus your new patches * plus letux-4.20-rc2 patches * including our private AESS patch set (mostly inactive, because it fails to load firmware) * CONFIG_CPU_IDLE=y
and it works. I can use aplay and play to get handsfree audio as with CONFIG_CPU_IDLE=n
Sometimes, there is a scratchy tenth of a second (in handsfree and headset), but the basic rhythm does not get interrupted any more and the play command does not get stuck.
I have played an mp3 of 4 minutes and the play process did succeed and didn't report buffer underrun issues.
So your patches seem to fix the issue. At least the basic problems. This scratchy thing needs further study, if it is a spurious thing on my Pyra protoype device. Or if it is still there if we disable CPU_IDLE again (I haven't tried that yet).
BR and thanks for quick help, Nikolaus
Hi,
On 2018-11-15 14:26, H. Nikolaus Schaller wrote:
Hi,
Am 14.11.2018 um 13:45 schrieb H. Nikolaus Schaller hns@goldelico.com:
Hi Peter,
Am 14.11.2018 um 12:06 schrieb Peter Ujfalusi peter.ujfalusi@ti.com:
Hi,
The defconfig for OMAP2+ now have the CPU_IDLE enabled which can cause audio artifacts because we try to enter too low power state from where the wakeup takes longer than the FIFO can tolerate on the dai side.
While adding pm_qos to McPDM and DMIC I have noticed that the McBSP calculation was not correct as we need usec for the latency value.
Regards, Peter
Peter Ujfalusi (3): ASoC: omap-mcbsp: Fix latency value calculation for pm_qos ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE
sound/soc/omap/omap-dmic.c | 9 ++++++++ sound/soc/omap/omap-mcbsp.c | 6 +++--- sound/soc/omap/omap-mcpdm.c | 43 ++++++++++++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-)
I will test asap.
I have now:
- v4.20-rc2
- plus your new patches
- plus letux-4.20-rc2 patches
- including our private AESS patch set (mostly inactive, because it fails to load firmware)
- CONFIG_CPU_IDLE=y
and it works. I can use aplay and play to get handsfree audio as with CONFIG_CPU_IDLE=n
Thanks for testing!
Sometimes, there is a scratchy tenth of a second (in handsfree and headset),
If you set CONFIG_CPU_IDLE=n, do you still have the scratchy sound? Or with 4.19 kernel if you can cross check it?
but the basic rhythm does not get interrupted any more and the play command does not get stuck.
I have played an mp3 of 4 minutes and the play process did succeed and didn't report buffer underrun issues.
So your patches seem to fix the issue. At least the basic problems. This scratchy thing needs further study, if it is a spurious thing on my Pyra protoype device. Or if it is still there if we disable CPU_IDLE again (I haven't tried that yet).
BR and thanks for quick help, Nikolaus
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 2018-11-15 16:29, Peter Ujfalusi wrote:
Sometimes, there is a scratchy tenth of a second (in handsfree and headset),
If you set CONFIG_CPU_IDLE=n, do you still have the scratchy sound? Or with 4.19 kernel if you can cross check it?
If it is not there w/o CPU_IDLE then can you try one thing: print out the latency usec the patch is calculating and, by guess divide it by 10 and pass that to pm_qos API. My calculation of the usec is not correct or I'm calculating it in a wrong way?
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 11/15/18 2:26 PM, H. Nikolaus Schaller wrote:
Peter Ujfalusi (3): ASoC: omap-mcbsp: Fix latency value calculation for pm_qos ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE
sound/soc/omap/omap-dmic.c | 9 ++++++++ sound/soc/omap/omap-mcbsp.c | 6 +++--- sound/soc/omap/omap-mcpdm.c | 43 ++++++++++++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-)
I will test asap.
I have now:
- v4.20-rc2
- plus your new patches
- plus letux-4.20-rc2 patches
- including our private AESS patch set (mostly inactive, because it fails to load firmware)
- CONFIG_CPU_IDLE=y
and it works. I can use aplay and play to get handsfree audio as with CONFIG_CPU_IDLE=n
Sometimes, there is a scratchy tenth of a second (in handsfree and headset), but the basic rhythm does not get interrupted any more and the play command does not get stuck.
I have played an mp3 of 4 minutes and the play process did succeed and didn't report buffer underrun issues.
So your patches seem to fix the issue. At least the basic problems. This scratchy thing needs further study, if it is a spurious thing on my Pyra protoype device. Or if it is still there if we disable CPU_IDLE again (I haven't tried that yet).
Peter: do you have some simple test case for N810 or N900? I tried to play with a few different aplay --buffer-size and --period-size combinations to see can I hit this. N810 most probably wasn't able to hit deep enough idle as the display is on due there is no driver for it. On N900 I have display blanked.
I have one educational question on 1/3 but no any show stoppers. For all three:
Acked-by: Jarkko Nikula jarkko.nikula@bitmer.com
On 2018-11-18 19:04, Jarkko Nikula wrote:
On 11/15/18 2:26 PM, H. Nikolaus Schaller wrote:
Peter Ujfalusi (3): ASoC: omap-mcbsp: Fix latency value calculation for pm_qos ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE
sound/soc/omap/omap-dmic.c | 9 ++++++++ sound/soc/omap/omap-mcbsp.c | 6 +++--- sound/soc/omap/omap-mcpdm.c | 43 ++++++++++++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-)
I will test asap.
I have now:
- v4.20-rc2
- plus your new patches
- plus letux-4.20-rc2 patches
- including our private AESS patch set (mostly inactive, because it fails to load firmware)
- CONFIG_CPU_IDLE=y
and it works. I can use aplay and play to get handsfree audio as with CONFIG_CPU_IDLE=n
Sometimes, there is a scratchy tenth of a second (in handsfree and headset), but the basic rhythm does not get interrupted any more and the play command does not get stuck.
I have played an mp3 of 4 minutes and the play process did succeed and didn't report buffer underrun issues.
So your patches seem to fix the issue. At least the basic problems. This scratchy thing needs further study, if it is a spurious thing on my Pyra protoype device. Or if it is still there if we disable CPU_IDLE again (I haven't tried that yet).
Peter: do you have some simple test case for N810 or N900? I tried to play with a few different aplay --buffer-size and --period-size combinations to see can I hit this. N810 most probably wasn't able to hit deep enough idle as the display is on due there is no driver for it. On N900 I have display blanked.
If you have CPU_IDLE enabled (it should be by default) and you don't experience any crackling than it is working OK.
Tony: or is there something else to look for in OMAP2/3 ?
I have one educational question on 1/3 but no any show stoppers. For all three:
Acked-by: Jarkko Nikula jarkko.nikula@bitmer.com
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
-
H. Nikolaus Schaller -
Jarkko Nikula -
Mark Brown -
Peter Ujfalusi