From patchwork Wed Aug 31 16:59:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Lindgren X-Patchwork-Id: 9307663 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 02178607D2 for ; Wed, 31 Aug 2016 18:16:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id ED61D28FAD for ; Wed, 31 Aug 2016 18:16:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E1E0429062; Wed, 31 Aug 2016 18:16:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=unavailable version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CD9ED28FD0 for ; Wed, 31 Aug 2016 18:16:22 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id 386442675A4; Wed, 31 Aug 2016 20:10:32 +0200 (CEST) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id 8AB172675F6; Wed, 31 Aug 2016 19:37:31 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id A818A2675F2; Wed, 31 Aug 2016 19:37:29 +0200 (CEST) Received: from muru.com (muru.com [72.249.23.125]) by alsa0.perex.cz (Postfix) with ESMTP id 06F99267232 for ; Wed, 31 Aug 2016 19:00:04 +0200 (CEST) Received: from atomide.com (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTPS id 5299B8081; Wed, 31 Aug 2016 17:00:50 +0000 (UTC) Date: Wed, 31 Aug 2016 09:59:59 -0700 From: Tony Lindgren To: Peter Ujfalusi Message-ID: <20160831165959.3sldcjxaci3kdx6z@atomide.com> References: <20160829182746.53vrgbxadqodzzug@atomide.com> <925ded57-1e91-3db6-9e82-3a3ff5775412@ti.com> <20160831141340.h24szdqjnicmt2ew@atomide.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160831141340.h24szdqjnicmt2ew@atomide.com> User-Agent: Mutt/1.6.2-neo (2016-07-23) Cc: alsa-devel@alsa-project.org, Ivaylo Dimitrov , Aaro Koskinen , Liam Girdwood , Sebastian Reichel , Mark Brown , Jarkko Nikula , Pavel Machel , Pali =?utf-8?B?Um9ow6Fy?= , linux-omap@vger.kernel.org Subject: Re: [alsa-devel] [PATCH] ASoC: omap-mcbsp: Add PM QoS support for McBSP to prevent glitches X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" 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 X-Virus-Scanned: ClamAV using ClamSMTP * Tony Lindgren [160831 07:14]: > * Peter Ujfalusi [160831 04:38]: > > If we have McBSP w/o FIFO there is no need for the qos AFAIK. > > I'm fine with the 30ms, if you have done the testing on OMAP3.McBSP2, probably > > setting 30ms for McBSP with 1280 FIFO, 3ms for 128 FIFO and no qos for McBSP > > w/o FIFO might be better. Or: > > > > latency = mcbsp->pdata->buffer_size * 23; /* 29.44ms on omap3's mcbsp2 */ > > > > if (latency) > > pm_qos_add_request(&mcbsp->pm_qos_req, PM_QOS_CPU_DMA_LATENCY, > > latency); > > OK let me run some tests with that and mcbsp2 fifo set to 128. My guess > is that things already work for that case with no patches with the existing > fifo based snd_pcm_hw_constraint_step() setting limits low enough so > we never enter deeper idle states, but let's see :) Well not quite so :) The fixed limit of 30 ms also works with fifo set to 128, but we still have audio fail without any patches. So basically anything blocking off idle for cpuidle is enough. Retention idle is OK to allow. With your calculation retention idle never happens when FIFO size is 128. It seems the latency value should eventually be SoC and cpuidle policy specific that we can probably do with dev_pm_qos once we have set_latency_tolerance implemented. I've also confirmed that we're hitting retention idle while playing a wav or mp3 file on omap3 even with FIFO limited to 128 and updated the patch description accordingly. Updated version of the patch below. Regards, Tony 8< ----------------- From: Tony Lindgren Date: Mon, 29 Aug 2016 11:27:46 -0700 Subject: [PATCHv3] ASoC: omap-mcbsp: Add PM QoS support for McBSP to prevent glitches 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. Let's fix the issue by adding PM QoS latency requirement for McBSP. Based on my experiments a working latency value is somewhere around 35 ms with 40 ms breaking. So let's use a safer value of 30 ms. Note that this is different from snd_pcm_hw_constraint_step() as that can configure things based on the buffer and period size. However, that does not help to block idle between the fifo fills. The value for the latency should eventually come from SoC specific configured value for dev_pm_qos, but we can use the measured value for now. I've confirmed that we are hitting core retention on omap3 while playing a wav and mp3 file even with mcbsp2 FIFO set to 128 bytes. If somebody needs to hit off idle while playing and gets it somehow working, we can set the latency requirements accordingly. Signed-off-by: Tony Lindgren --- Changes since v1 & 2: - Configure the latency during init in case we need to set SoC and cpuidle specific values later on - Update description and comments for latency requirements sound/soc/omap/mcbsp.c | 21 +++++++++++++++++++++ sound/soc/omap/mcbsp.h | 3 +++ 2 files changed, 24 insertions(+) diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c --- a/sound/soc/omap/mcbsp.c +++ b/sound/soc/omap/mcbsp.c @@ -25,6 +25,7 @@ #include #include #include +#include #include @@ -643,6 +644,11 @@ void omap_mcbsp_start(struct omap_mcbsp *mcbsp, int tx, int rx) int enable_srg = 0; u16 w; + /* Prevent omap hardware from hitting off between fifo fills */ + if (mcbsp->latency) + pm_qos_add_request(&mcbsp->pm_qos_req, PM_QOS_CPU_DMA_LATENCY, + mcbsp->latency); + if (mcbsp->st_data) omap_st_start(mcbsp); @@ -731,6 +737,8 @@ void omap_mcbsp_stop(struct omap_mcbsp *mcbsp, int tx, int rx) if (mcbsp->st_data) omap_st_stop(mcbsp); + + pm_qos_remove_request(&mcbsp->pm_qos_req); } int omap2_mcbsp_set_clks_src(struct omap_mcbsp *mcbsp, u8 fck_src_id) @@ -1064,6 +1072,16 @@ int omap_mcbsp_init(struct platform_device *pdev) mcbsp->max_tx_thres = max_thres(mcbsp) - 0x10; mcbsp->max_rx_thres = max_thres(mcbsp) - 0x10; + /* + * Prevent device from hitting deeper idle states between + * DMA fifo loads. On omap3 mcbsp2 we have a larger FIFO of + * 1280 bytes instead of the usual 128 bytes, and the measured + * max latency we can tolerate is somewhere below 40 ms. To be + * safe, use a fixed value of 30 ms to block off idle on omap3 + * while still allowing retention idle. + */ + mcbsp->latency = 30 * 1000; + ret = sysfs_create_group(&mcbsp->dev->kobj, &additional_attr_group); if (ret) { @@ -1098,6 +1116,9 @@ err_thres: 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 --- a/sound/soc/omap/mcbsp.h +++ b/sound/soc/omap/mcbsp.h @@ -325,6 +325,9 @@ struct omap_mcbsp { unsigned int in_freq; int clk_div; int wlen; + + s32 latency; + struct pm_qos_request pm_qos_req; }; void omap_mcbsp_config(struct omap_mcbsp *mcbsp,