Re: [PATCH 0/5] ASoC: fsl_ssi: Fixing various channel slips and bad samples insertions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




[..]

SUCCESS!  So far...
Great :)

I'm preparing a v3 of my patches including the SOR register + rebased on top of v4.4.
I will let you propose the water mark / maxburst patch.
But it looks obvious to me that triggering the DMA when only 2 words are left in the FIFO can lead to DMA xruns at such data rate.

The downside is an increased number of DMA requests.
So I don't know if you should propose a configuration through the device tree, or a static configuration as done in your patch.

Arnaud

With your patches (including the latest SOR register update), plus
setting the watermark & DMA MAXBURST to 8, I don't get any more errors
at 48kHz ...  yet.

Even in single fifo mode, 48kHz, 16-bits+16channels seems to work now.

I'll keep you updated on if this really solves all the issues.
Here's the last patch for updating the watermark.

commit b634014b831b9527df319b404ac50e54a3790742
Author: Caleb Crome <caleb@xxxxxxxxx>
Date:   Wed Jan 13 13:12:37 2016 -0800

     ASoC: fsl_ssi: Increase watermark and maxburst to allow SSI to work
     without slips at high data rates.

     The DMA cannot keep up with the SSI consumpation with the watermark
     set to be fifo_depth-2 when running at 48kHz/16-bits/16-channels
     (12288 words/second).  By increasing the watermark to 8, the DMA can
     keep up with the SSI.

     Signed-off-by: Caleb Crome <caleb@xxxxxxxxx>

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 5cfc540..026df79 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->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;

@@ -1518,6 +1509,42 @@ static int fsl_ssi_probe(struct platform_device *pdev)
                  /* Older 8610 DTs didn't have the fifo-depth property */
          ssi_private->fifo_depth = 8;

+    /*
+     * 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.
+     */
+    switch (ssi_private->fifo_depth) {
+    case 15:
+        /*
+         * 2 samples is not enough when running at high data
+         * rates (like 48kHz @ 16 bits/channel, 16 channels)
+         * 8 seems to split things evenly and leave enough time
+         * for the DMA to fill the FIFO before it's over/under
+         * run.
+         */
+        ssi_private->fifo_watermark = 8;
+        ssi_private->dma_maxburst = 8;
+    case 8:
+    default:
+        /*
+         * maintain old behavior for older chips.
+         * Keeping it the same because I don't have an older
+         * board to test with.
+         * I suspect this could be changed to be something to
+         * leave some more space in the fifo.
+         */
+        ssi_private->fifo_watermark = ssi_private->fifo_depth - 2;
+        ssi_private->dma_maxburst = ssi_private->fifo_depth - 2;
+        break;
+    }
+
      dev_set_drvdata(&pdev->dev, ssi_private);

      if (ssi_private->soc->imx) {

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux