[PATCH 0/3] ASoC: omap: Fix and add pm_qos configuration

[PATCH 0/3] ASoC: omap: Fix and add pm_qos configuration

[Peter Ujfalusi]

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(-)

-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 1/3] ASoC: omap-mcbsp: Fix latency value calculation for pm_qos

[Peter Ujfalusi]

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);
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 2/3] ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE

[Peter Ujfalusi]

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;
 }
 
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 3/3] ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE

[Peter Ujfalusi]

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);
 
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 0/3] ASoC: omap: Fix and add pm_qos configuration

[H. Nikolaus Schaller]

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

Re: [PATCH 0/3] ASoC: omap: Fix and add pm_qos configuration

[H. Nikolaus Schaller]

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

Re: [PATCH 0/3] ASoC: omap: Fix and add pm_qos configuration

[Peter Ujfalusi]

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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/3] ASoC: omap: Fix and add pm_qos configuration

[Peter Ujfalusi]

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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ASoC: omap-mcbsp: Fix latency value calculation for pm_qos

[Jarkko Nikula]

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?

-- 
Jarkko

Re: [PATCH 0/3] ASoC: omap: Fix and add pm_qos configuration

[Jarkko Nikula]

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>

Re: [PATCH 1/3] ASoC: omap-mcbsp: Fix latency value calculation for pm_qos

[Peter Ujfalusi]

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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/3] ASoC: omap: Fix and add pm_qos configuration

[Peter Ujfalusi]

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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Applied "ASoC: omap-dmic: Add pm_qos handling to avoid overruns with CPU_IDLE" to the asoc tree

[Mark Brown]

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);
 
-- 
2.19.1

Applied "ASoC: omap-mcpdm: Add pm_qos handling to avoid under/overruns with CPU_IDLE" to the asoc tree

[Mark Brown]

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;
 }
 
-- 
2.19.1

Applied "ASoC: omap-mcbsp: Fix latency value calculation for pm_qos" to the asoc tree

[Mark Brown]

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);
-- 
2.19.1