From: Caleb Crome <caleb@crome.org>
To: Timur Tabi <timur@tabi.org>,
Nicolin Chen <nicoleotsuka@gmail.com>,
Xiubo Li <Xiubo.Lee@gmail.com>,
Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
Takashi Iwai <tiwai@suse.com>, Caleb Crome <caleb@crome.org>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
alsa-devel@alsa-project.org
Subject: [PATCH RFC 1/1] ASoC: fsl_ssi: Make fifo watermark and maxburst settings device tree options
Date: Thu, 14 Jan 2016 08:29:42 -0800 [thread overview]
Message-ID: <1452788982-11583-1-git-send-email-caleb@crome.org> (raw)
Tuning the SSI fifo watermark & maxburst settings needs to be
optimized differently depending on the demands on the system. The
current default of 2 is too low for high data-rate systems. This
patch maintains exactly the same behavior by default (i.e defaults to
2), but adds device tree options to set maxburst & fifo depth from the
device tree. This is necessary because a setting of 2 simply doesn't
work at higher data rates.
Signed-off-by: Caleb Crome <caleb@crome.org>
---
.../devicetree/bindings/sound/fsl,ssi.txt | 10 +++
sound/soc/fsl/fsl_ssi.c | 87 ++++++++++++++--------
2 files changed, 68 insertions(+), 29 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl,ssi.txt b/Documentation/devicetree/bindings/sound/fsl,ssi.txt
index 5b76be4..7af62b9 100644
--- a/Documentation/devicetree/bindings/sound/fsl,ssi.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,ssi.txt
@@ -61,6 +61,16 @@ Optional properties:
- fsl,mode: The operating mode for the AC97 interface only.
"ac97-slave" - AC97 mode, SSI is clock slave
"ac97-master" - AC97 mode, SSI is clock master
+- fsl,fifo-watermark: Sets the fifo watermark. The default is
+ fifo_depth-2 words, meaning 'initiate dma transfer
+ when 2 words are left in the fifo'. At higher
+ data rates (48kHz, 16-channels for example), this
+ causes silent but deadly DMA xruns and channel
+ slips. For 15 word FIFOs (like on MX5, MX6) 8 is
+ a good value when running at high data rates
+- fsl,dma-maxburst: sets the max number of words to transfer in DMA.
+ This defaults to the same value as
+ fsl,fifo-watermark.
Child 'codec' node required properties:
- compatible: Compatible list, contains the name of the codec
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 5cfc540..94d8b5d7 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -221,6 +221,12 @@ struct fsl_ssi_soc_data {
* @dbg_stats: Debugging statistics
*
* @soc: SoC specific data
+ *
+ * @fifo_watermark: the FIFO watermark setting. Notifies DMA when
+ * there are @fifo_watermark or fewer words in TX fifo or
+ * @fifo_watermark or more empty words in RX fifo.
+ * @dma_maxburst: max number of words to transfer in one go. So far,
+ * this is always the same as fifo_watermark.
*/
struct fsl_ssi_private {
struct regmap *regs;
@@ -259,6 +265,9 @@ struct fsl_ssi_private {
const struct fsl_ssi_soc_data *soc;
struct device *dev;
+
+ u32 fifo_watermark;
+ u32 dma_maxburst;
};
/*
@@ -1037,21 +1046,7 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
regmap_write(regs, CCSR_SSI_SRCR, srcr);
regmap_write(regs, CCSR_SSI_SCR, scr);
- /*
- * Set the watermark for transmit FIFI 0 and receive FIFO 0. We don't
- * use FIFO 1. We program the transmit water to signal a DMA transfer
- * if there are only two (or fewer) elements left in the FIFO. Two
- * elements equals one frame (left channel, right channel). This value,
- * however, depends on the depth of the transmit buffer.
- *
- * We set the watermark on the same level as the DMA burstsize. For
- * fiq it is probably better to use the biggest possible watermark
- * size.
- */
- if (ssi_private->use_dma)
- wm = ssi_private->fifo_depth - 2;
- else
- wm = ssi_private->fifo_depth;
+ wm = ssi_private->fifo_watermark;
regmap_write(regs, CCSR_SSI_SFCSR,
CCSR_SSI_SFCSR_TFWM0(wm) | CCSR_SSI_SFCSR_RFWM0(wm) |
@@ -1359,12 +1354,8 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
dev_dbg(&pdev->dev, "could not get baud clock: %ld\n",
PTR_ERR(ssi_private->baudclk));
- /*
- * We have burstsize be "fifo_depth - 2" to match the SSI
- * watermark setting in fsl_ssi_startup().
- */
- ssi_private->dma_params_tx.maxburst = ssi_private->fifo_depth - 2;
- ssi_private->dma_params_rx.maxburst = ssi_private->fifo_depth - 2;
+ ssi_private->dma_params_tx.maxburst = ssi_private->dma_maxburst;
+ ssi_private->dma_params_rx.maxburst = ssi_private->dma_maxburst;
ssi_private->dma_params_tx.addr = ssi_private->ssi_phys + CCSR_SSI_STX0;
ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + CCSR_SSI_SRX0;
@@ -1421,6 +1412,51 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev,
clk_disable_unprepare(ssi_private->clk);
}
+static void fsl_ssi_set_fifo_settings(
+ struct device_node *np,
+ struct fsl_ssi_private *ssi_private)
+{
+ const uint32_t *iprop;
+ /*
+ * Determine the FIFO depth. & watermark
+ *
+ * Set the watermark for transmit FIFO 0/1 and receive FIFO
+ * 0/1. We program the transmit water to signal a DMA transfer
+ * when this many words are left to be transmitted (or
+ * received). Previous setting was 2 elements, which was
+ * assumed to be one frame. Such a small value causes silent
+ * DMA xruns at high data rates. 8 seems to be a good
+ * tradeoff between xruns & number of DMA transfers.
+ *
+ * We set the watermark on the same level as the DMA burstsize. For
+ * fiq it is probably better to use the biggest possible watermark
+ * size.
+ */
+ iprop = of_get_property(np, "fsl,fifo-depth", NULL);
+ if (iprop)
+ ssi_private->fifo_depth = be32_to_cpup(iprop);
+ else
+ /* Older 8610 DTs didn't have the fifo-depth property */
+ ssi_private->fifo_depth = 8;
+
+ iprop = of_get_property(np, "fsl,fifo-watermark", NULL);
+ if (iprop)
+ ssi_private->fifo_watermark = be32_to_cpup(iprop);
+ else
+ /* Default to old value of 2, which is too
+ * small at high data rates, but it's the
+ * default that's been there for years.
+ */
+ ssi_private->fifo_watermark = ssi_private->fifo_depth - 2;
+
+ iprop = of_get_property(np, "fsl,dma-maxburst", NULL);
+
+ if (iprop)
+ ssi_private->dma_maxburst = be32_to_cpup(iprop);
+ else
+ ssi_private->dma_maxburst = ssi_private->fifo_watermark;
+}
+
static int fsl_ssi_probe(struct platform_device *pdev)
{
struct fsl_ssi_private *ssi_private;
@@ -1428,7 +1464,6 @@ static int fsl_ssi_probe(struct platform_device *pdev)
struct device_node *np = pdev->dev.of_node;
const struct of_device_id *of_id;
const char *p, *sprop;
- const uint32_t *iprop;
struct resource *res;
void __iomem *iomem;
char name[64];
@@ -1510,13 +1545,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
ssi_private->cpu_dai_drv.symmetric_samplebits = 1;
}
- /* Determine the FIFO depth. */
- iprop = of_get_property(np, "fsl,fifo-depth", NULL);
- if (iprop)
- ssi_private->fifo_depth = be32_to_cpup(iprop);
- else
- /* Older 8610 DTs didn't have the fifo-depth property */
- ssi_private->fifo_depth = 8;
+ fsl_ssi_set_fifo_settings(np, ssi_private);
dev_set_drvdata(&pdev->dev, ssi_private);
--
2.1.4
next reply other threads:[~2016-01-14 16:29 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-14 16:29 Caleb Crome [this message]
[not found] ` <1452788982-11583-1-git-send-email-caleb-EebDDntmC0DYtjvyW6yDsg@public.gmane.org>
2016-01-14 20:18 ` [PATCH RFC 1/1] ASoC: fsl_ssi: Make fifo watermark and maxburst settings device tree options Nicolin Chen
2016-01-14 21:26 ` Caleb Crome
2016-01-15 2:45 ` Nicolin Chen
2016-01-15 4:56 ` Caleb Crome
2016-01-15 18:12 ` Nicolin Chen
2016-01-15 1:31 ` Timur Tabi
2016-01-15 2:33 ` Nicolin Chen
2016-01-15 3:25 ` Rob Herring
[not found] ` <56984BE7.2050303-N01EOCouUvQ@public.gmane.org>
2016-01-15 13:13 ` Mark Brown
[not found] ` <20160115131325.GW6588-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2016-01-15 13:46 ` Timur Tabi
2016-01-15 17:03 ` Caleb Crome
2016-01-15 18:38 ` Nicolin Chen
2016-01-15 18:49 ` Caleb Crome
2016-01-15 18:57 ` Nicolin Chen
2016-01-15 19:10 ` Caleb Crome
2016-01-15 19:23 ` Nicolin Chen
2016-01-15 19:49 ` Timur Tabi
[not found] ` <CAG5mAdyUfSuCqA4_342MS2AyshvuyeiYsjQyRqifmqVhn4T2xA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-01-16 14:15 ` Timur Tabi
2016-01-17 23:34 ` Caleb Crome
2016-01-15 19:51 ` Timur Tabi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1452788982-11583-1-git-send-email-caleb@crome.org \
--to=caleb@crome.org \
--cc=Xiubo.Lee@gmail.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nicoleotsuka@gmail.com \
--cc=perex@perex.cz \
--cc=timur@tabi.org \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).