From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Ujfalusi Subject: Re: [PATCH v5] ASoC: omap-mcbsp: Add PM QoS support for McBSP to prevent glitches Date: Mon, 9 Jan 2017 15:12:01 +0200 Message-ID: <1372efd2-5319-e6c4-629e-6ed5f2e2fe29@ti.com> References: <20170105015918.21621-1-matt@ranostay.consulting> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from fllnx209.ext.ti.com (fllnx209.ext.ti.com [198.47.19.16]) by alsa0.perex.cz (Postfix) with ESMTP id AD2E9266D02 for ; Mon, 9 Jan 2017 14:12:02 +0100 (CET) In-Reply-To: <20170105015918.21621-1-matt@ranostay.consulting> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Matt Ranostay , alsa-devel@alsa-project.org, linux-omap@vger.kernel.org Cc: Tony Lindgren List-Id: alsa-devel@alsa-project.org On 01/05/2017 03:59 AM, Matt Ranostay wrote: > We can get audio errors if hitting deeper idle states on omaps: > = > [alsa.c:230] error: Fatal problem with alsa output, error -5. > [audio.c:614] error: Error in writing audio (Input/output error?)! > = > This seems to happen with off mode idle enabled as power for the > whole SoC may get cut off between filling the McBSP fifo using DMA. > While active DMA blocks deeper idle states in hardware, McBSP > activity does not seem to do so. > = > Basing the QoS latency calculation on the FIFO size, threshold, > sample rate, and channels. Looks good to me, thank you! Acked-by: Peter Ujfalusi > Based on the original patch by Tony Lindgren > Link: https://patchwork.kernel.org/patch/9305867/ > = > Cc: Tony Lindgren > Cc: Peter Ujfalusi > Signed-off-by: Matt Ranostay > --- > Changes from v1: > * add calculations for latency per number of FIFO locations > = > Changes from v2: > * add missing mcbsp.h header change > = > Changes from v3: > * base the latency calculations on threshold, buffer size, sample > rate, and channels > = > Changes from v4: > * using Peter Ujfalusi's suggestions for restoring a higher latency on > audio stream completion, or if not applicable remove the QoS request > = > sound/soc/omap/mcbsp.c | 37 +++++++++++++++++++++++++++++++++++++ > sound/soc/omap/mcbsp.h | 3 +++ > sound/soc/omap/omap-mcbsp.c | 11 ++++++++++- > 3 files changed, 50 insertions(+), 1 deletion(-) > = > diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c > index 06fec5699cc8..f079650b71e3 100644 > --- a/sound/soc/omap/mcbsp.c > +++ b/sound/soc/omap/mcbsp.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > = > #include > = > @@ -640,9 +641,28 @@ void omap_mcbsp_free(struct omap_mcbsp *mcbsp) > */ > void omap_mcbsp_start(struct omap_mcbsp *mcbsp, int tx, int rx) > { > + struct pm_qos_request *pm_qos_req =3D &mcbsp->pm_qos_req; > + int stream1 =3D tx ? SNDRV_PCM_STREAM_PLAYBACK : SNDRV_PCM_STREAM_CAPTU= RE; > + int stream2 =3D tx ? SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBA= CK; > int enable_srg =3D 0; > + int latency; > u16 w; > = > + /* Prevent omap hardware from hitting off between FIFO fills */ > + if (!mcbsp->latency[stream2] || > + mcbsp->latency[stream1] < mcbsp->latency[stream2]) > + latency =3D mcbsp->latency[stream1]; > + else > + latency =3D mcbsp->latency[stream2]; > + > + if (latency) { > + if (pm_qos_request_active(pm_qos_req)) > + pm_qos_update_request(pm_qos_req, latency); > + else > + pm_qos_add_request(pm_qos_req, PM_QOS_CPU_DMA_LATENCY, > + latency); > + } > + > if (mcbsp->st_data) > omap_st_start(mcbsp); > = > @@ -731,6 +751,20 @@ void omap_mcbsp_stop(struct omap_mcbsp *mcbsp, int t= x, int rx) > = > if (mcbsp->st_data) > omap_st_stop(mcbsp); > + > + if (pm_qos_request_active(&mcbsp->pm_qos_req)) { > + int stream1 =3D tx ? SNDRV_PCM_STREAM_PLAYBACK : > + SNDRV_PCM_STREAM_CAPTURE; > + int stream2 =3D tx ? SNDRV_PCM_STREAM_CAPTURE : > + SNDRV_PCM_STREAM_PLAYBACK; > + > + mcbsp->latency[stream1] =3D 0; > + if (mcbsp->latency[stream2]) > + pm_qos_update_request(&mcbsp->pm_qos_req, > + mcbsp->latency[stream2]); > + else > + pm_qos_remove_request(&mcbsp->pm_qos_req); > + } > } > = > int omap2_mcbsp_set_clks_src(struct omap_mcbsp *mcbsp, u8 fck_src_id) > @@ -1098,6 +1132,9 @@ int omap_mcbsp_init(struct platform_device *pdev) > = > void omap_mcbsp_cleanup(struct omap_mcbsp *mcbsp) > { > + if (pm_qos_request_active(&mcbsp->pm_qos_req)) > + pm_qos_remove_request(&mcbsp->pm_qos_req); > + > if (mcbsp->pdata->buffer_size) > sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group); > = > diff --git a/sound/soc/omap/mcbsp.h b/sound/soc/omap/mcbsp.h > index 61e93b1c185d..46ae1269a698 100644 > --- a/sound/soc/omap/mcbsp.h > +++ b/sound/soc/omap/mcbsp.h > @@ -323,8 +323,11 @@ struct omap_mcbsp { > = > unsigned int fmt; > unsigned int in_freq; > + unsigned int latency[2]; > int clk_div; > int wlen; > + > + struct pm_qos_request pm_qos_req; > }; > = > void omap_mcbsp_config(struct omap_mcbsp *mcbsp, > diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c > index d018e966e533..7e00938271c6 100644 > --- a/sound/soc/omap/omap-mcbsp.c > +++ b/sound/soc/omap/omap-mcbsp.c > @@ -226,6 +226,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_su= bstream *substream, > int wlen, channels, wpf; > int pkt_size =3D 0; > unsigned int format, div, framesize, master; > + unsigned int buffer_size =3D mcbsp->pdata->buffer_size; > = > dma_data =3D snd_soc_dai_get_dma_data(cpu_dai, substream); > channels =3D params_channels(params); > @@ -240,7 +241,9 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_su= bstream *substream, > default: > return -EINVAL; > } > - if (mcbsp->pdata->buffer_size) { > + if (buffer_size) { > + int latency; > + > if (mcbsp->dma_op_mode =3D=3D MCBSP_DMA_MODE_THRESHOLD) { > int period_words, max_thrsh; > int divider =3D 0; > @@ -271,6 +274,12 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_s= ubstream *substream, > /* Use packet mode for non mono streams */ > pkt_size =3D channels; > } > + > + latency =3D ((((buffer_size - pkt_size) / channels) * 1000) > + / (params->rate_num / params->rate_den)); > + > + mcbsp->latency[substream->stream] =3D latency; > + > omap_mcbsp_set_threshold(substream, pkt_size); > } > = > = -- = P=E9ter