From patchwork Fri May 11 02:36:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mark Brown X-Patchwork-Id: 10392903 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 810CA60236 for ; Fri, 11 May 2018 02:38:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C3720286D4 for ; Fri, 11 May 2018 02:38:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B7B22286DD; Fri, 11 May 2018 02:38:08 +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=-2.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 0794E286D4 for ; Fri, 11 May 2018 02:38:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: MIME-Version:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Date:Message-Id:In-Reply-To:Subject:To:From:Reply-To :Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=idYklTPd9dJwJDBp5C+9MJfAsJFv87dC3lIBMZwbJeg=; b=O1WZPezrzsSMxY e1vTCEnI0+w8aoF7Pc9fo1E73b7KeZSkcJlKmp0BoawfTaehCuTgs9FoBlZB5PdHZ1dgIWj1Vd0Z9 KcvimkazGl2o0eItKZkv90dvVLmz3pOChPWpQa0f17OYA5xfla+oBHl8pXaDFrzsIcWoeqAhtSlAD MC6zEhn9C6MGUzfFHJaIau/+imjhORdDYmq72rONITRs//WBQWnicfeWVvGtSeOqoCi3N7rFkUmtW c1tp/yU5AKaGE/koMw5CalbNcUm5WYGLHWF8Os2DqbU8dyNQVQQ4VfWgoPz3YhqR8q1jBuYpjNusA BxAGJXTiv3heB1sirLXA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1fGxwT-0005av-Ag; Fri, 11 May 2018 02:37:57 +0000 Received: from heliosphere.sirena.org.uk ([2a01:7e01::f03c:91ff:fed4:a3b6]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fGxvk-0005Cf-VX for linux-arm-kernel@lists.infradead.org; Fri, 11 May 2018 02:37:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sirena.org.uk; s=20170815-heliosphere; h=Date:Message-Id:In-Reply-To: Subject:Cc:To:From:Sender:Reply-To:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner: List-Archive; bh=Phq/D0CK6+9r4Na/BdaCQm3O1v/tyFSZZa81HU9PBts=; b=f56RQkDP5IDx 6m1LoY1EprWTgZulzAp85sQn06NFChK1k9PANWy7+HsU26hXXDPcWq1fe+TX1SsWdGSvoTfjixmax lGkzRfCRmU/6ltZvqAwxe3At1iogLbVzdIOrNFF5/NtPva3KFejcvUg+EbyO2dD26JDBoNKnHoL3t pJa6A=; Received: from [2001:268:c0cd:b8ca:2aa0:ebb3:99a:5339] (helo=finisterre.ee.mobilebroadband) by heliosphere.sirena.org.uk with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1fGxvM-0002Sj-6u; Fri, 11 May 2018 02:36:50 +0000 Received: by finisterre.ee.mobilebroadband (Postfix, from userid 1000) id 8683844007A; Fri, 11 May 2018 03:36:30 +0100 (BST) From: Mark Brown To: Alexander Sverdlin Subject: Applied "ASoC: cirrus: i2s: IRQ-based stream watchdog" to the asoc tree In-Reply-To: <20180428205142.31921-6-alexander.sverdlin@gmail.com> Message-Id: <20180511023630.8683844007A@finisterre.ee.mobilebroadband> Date: Fri, 11 May 2018 03:36:30 +0100 (BST) X-Bad-Reply: In-Reply-To but no 'Re:' in Subject. X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180510_193713_201926_BE30BA7C X-CRM114-Status: GOOD ( 20.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, Ryan Mallon , Liam Girdwood , Takashi Iwai , Hartley Sweeten , Mark Brown , linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP The patch ASoC: cirrus: i2s: IRQ-based stream watchdog 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 98e1241c357b82e070294f486cef91a1263874fa Mon Sep 17 00:00:00 2001 From: Alexander Sverdlin Date: Sat, 28 Apr 2018 22:51:42 +0200 Subject: [PATCH] ASoC: cirrus: i2s: IRQ-based stream watchdog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I2S controller on EP93xx seems to have undocumented HW issue. According to "EP93xx User’s Guide", controller can handle underflow and either transmit last sample or zeroes in such case until FIFO is filled again. In reality undeflow conditions seem to confuse internal state machine from time to time and the whole stream gets shifted by one byte (as captured by logic analyser on the I2S outputs). One could only hear noise instead of original stream and this continues until the FIFO is disabled and enabled again. Work this around by watching underflow interrupt and resetting I2S TX channel + fill FIFO with zero samples until DMA catches up again. This is a nasty workaround, but it works. Hence, Kconfig option to disable it in case of problems. Signed-off-by: Alexander Sverdlin Signed-off-by: Mark Brown --- sound/soc/cirrus/Kconfig | 17 +++++++++ sound/soc/cirrus/ep93xx-i2s.c | 68 +++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/sound/soc/cirrus/Kconfig b/sound/soc/cirrus/Kconfig index c7cd60f009e9..e09199124c36 100644 --- a/sound/soc/cirrus/Kconfig +++ b/sound/soc/cirrus/Kconfig @@ -9,6 +9,23 @@ config SND_EP93XX_SOC config SND_EP93XX_SOC_I2S tristate +if SND_EP93XX_SOC_I2S + +config SND_EP93XX_SOC_I2S_WATCHDOG + bool "IRQ based underflow watchdog workaround" + default y + help + I2S controller on EP93xx seems to have undocumented HW issue. + Underflow of internal I2S controller FIFO could confuse the + state machine and the whole stream can be shifted by one byte + until I2S is disabled. This option enables IRQ based watchdog + which disables and re-enables I2S in case of underflow and + fills FIFO with zeroes. + + If you are unsure how to answer this question, answer Y. + +endif # if SND_EP93XX_SOC_I2S + config SND_EP93XX_SOC_AC97 tristate select AC97_BUS diff --git a/sound/soc/cirrus/ep93xx-i2s.c b/sound/soc/cirrus/ep93xx-i2s.c index 42cc6d22baac..0918c5da575a 100644 --- a/sound/soc/cirrus/ep93xx-i2s.c +++ b/sound/soc/cirrus/ep93xx-i2s.c @@ -35,8 +35,12 @@ #define EP93XX_I2S_TXCLKCFG 0x00 #define EP93XX_I2S_RXCLKCFG 0x04 +#define EP93XX_I2S_GLSTS 0x08 #define EP93XX_I2S_GLCTRL 0x0C +#define EP93XX_I2S_I2STX0LFT 0x10 +#define EP93XX_I2S_I2STX0RT 0x14 + #define EP93XX_I2S_TXLINCTRLDATA 0x28 #define EP93XX_I2S_TXCTRL 0x2C #define EP93XX_I2S_TXWRDLEN 0x30 @@ -55,12 +59,22 @@ #define EP93XX_I2S_TXLINCTRLDATA_R_JUST BIT(2) /* Right justify */ +/* + * Transmit empty interrupt level select: + * 0 - Generate interrupt when FIFO is half empty + * 1 - Generate interrupt when FIFO is empty + */ +#define EP93XX_I2S_TXCTRL_TXEMPTY_LVL BIT(0) +#define EP93XX_I2S_TXCTRL_TXUFIE BIT(1) /* Transmit interrupt enable */ + #define EP93XX_I2S_CLKCFG_LRS (1 << 0) /* lrclk polarity */ #define EP93XX_I2S_CLKCFG_CKP (1 << 1) /* Bit clock polarity */ #define EP93XX_I2S_CLKCFG_REL (1 << 2) /* First bit transition */ #define EP93XX_I2S_CLKCFG_MASTER (1 << 3) /* Master mode */ #define EP93XX_I2S_CLKCFG_NBCG (1 << 4) /* Not bit clock gating */ +#define EP93XX_I2S_GLSTS_TX0_FIFO_FULL BIT(12) + struct ep93xx_i2s_info { struct clk *mclk; struct clk *sclk; @@ -116,12 +130,24 @@ static void ep93xx_i2s_enable(struct ep93xx_i2s_info *info, int stream) else base_reg = EP93XX_I2S_RX0EN; ep93xx_i2s_write_reg(info, base_reg, 1); + + /* Enable TX IRQs (FIFO empty or underflow) */ + if (IS_ENABLED(CONFIG_SND_EP93XX_SOC_I2S_WATCHDOG) && + stream == SNDRV_PCM_STREAM_PLAYBACK) + ep93xx_i2s_write_reg(info, EP93XX_I2S_TXCTRL, + EP93XX_I2S_TXCTRL_TXEMPTY_LVL | + EP93XX_I2S_TXCTRL_TXUFIE); } static void ep93xx_i2s_disable(struct ep93xx_i2s_info *info, int stream) { unsigned base_reg; + /* Disable IRQs */ + if (IS_ENABLED(CONFIG_SND_EP93XX_SOC_I2S_WATCHDOG) && + stream == SNDRV_PCM_STREAM_PLAYBACK) + ep93xx_i2s_write_reg(info, EP93XX_I2S_TXCTRL, 0); + /* Disable fifo */ if (stream == SNDRV_PCM_STREAM_PLAYBACK) base_reg = EP93XX_I2S_TX0EN; @@ -141,6 +167,37 @@ static void ep93xx_i2s_disable(struct ep93xx_i2s_info *info, int stream) } } +/* + * According to documentation I2S controller can handle underflow conditions + * just fine, but in reality the state machine is sometimes confused so that + * the whole stream is shifted by one byte. The watchdog below disables the TX + * FIFO, fills the buffer with zeroes and re-enables the FIFO. State machine + * is being reset and by filling the buffer we get some time before next + * underflow happens. + */ +static irqreturn_t ep93xx_i2s_interrupt(int irq, void *dev_id) +{ + struct ep93xx_i2s_info *info = dev_id; + + /* Disable FIFO */ + ep93xx_i2s_write_reg(info, EP93XX_I2S_TX0EN, 0); + /* + * Fill TX FIFO with zeroes, this way we can defer next IRQs as much as + * possible and get more time for DMA to catch up. Actually there are + * only 8 samples in this FIFO, so even on 8kHz maximum deferral here is + * 1ms. + */ + while (!(ep93xx_i2s_read_reg(info, EP93XX_I2S_GLSTS) & + EP93XX_I2S_GLSTS_TX0_FIFO_FULL)) { + ep93xx_i2s_write_reg(info, EP93XX_I2S_I2STX0LFT, 0); + ep93xx_i2s_write_reg(info, EP93XX_I2S_I2STX0RT, 0); + } + /* Re-enable FIFO */ + ep93xx_i2s_write_reg(info, EP93XX_I2S_TX0EN, 1); + + return IRQ_HANDLED; +} + static int ep93xx_i2s_dai_probe(struct snd_soc_dai *dai) { struct ep93xx_i2s_info *info = snd_soc_dai_get_drvdata(dai); @@ -390,6 +447,17 @@ static int ep93xx_i2s_probe(struct platform_device *pdev) if (IS_ERR(info->regs)) return PTR_ERR(info->regs); + if (IS_ENABLED(CONFIG_SND_EP93XX_SOC_I2S_WATCHDOG)) { + int irq = platform_get_irq(pdev, 0); + if (irq <= 0) + return irq < 0 ? irq : -ENODEV; + + err = devm_request_irq(&pdev->dev, irq, ep93xx_i2s_interrupt, 0, + pdev->name, info); + if (err) + return err; + } + info->mclk = clk_get(&pdev->dev, "mclk"); if (IS_ERR(info->mclk)) { err = PTR_ERR(info->mclk);