ASoC: fsl_ssi: set fifo watermark to more reliable value

[ Upstream commit 4ee437fbf626b5ad756889d8bc0fcead3d66dde7 ] The fsl_ssi fifo watermark is by default set to 2 free spaces (i.e. activate DMA on FIFO when only 2 spaces are left.) This means the DMA must service the fifo within 2 audio samples, which is just not enough time for many use cases with high data rate. In many configurations the audio channel slips (causing l/r swap in stereo configurations, or channel slipping in multi-channel configurations). This patch gives more breathing room and allows the SSI to operate reliably by changing the fifio refill watermark to 8. There is no change in behavior for older chips (with an 8-deep fifo). Only the newer chips with a 15-deep fifo get the new behavior. I suspect a new fifo depth setting could be optimized on the older chips too, but I have not tested. Signed-off-by: Caleb Crome <caleb@crome.org> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com> Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <alexander.levin@verizon.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

ASoC: fsl_ssi: set fifo watermark to more reliable value
[ Upstream commit 4ee437fbf626b5ad756889d8bc0fcead3d66dde7 ] The fsl_ssi fifo watermark is by default set to 2 free spaces (i.e. activate DMA on FIFO when only 2 spaces are left.) This means the DMA must service the fifo within 2 audio samples, which is just not enough time for many use cases with high data rate. In many configurations the audio channel slips (causing l/r swap in stereo configurations, or channel slipping in multi-channel configurations). This patch gives more breathing room and allows the SSI to operate reliably by changing the fifio refill watermark to 8. There is no change in behavior for older chips (with an 8-deep fifo). Only the newer chips with a 15-deep fifo get the new behavior. I suspect a new fifo depth setting could be optimized on the older chips too, but I have not tested. Signed-off-by: Caleb Crome <caleb@crome.org> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com> Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <alexander.levin@verizon.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Caleb Crome · Greg Kroah-Hartman
1 parent 2548d893a5
Showing 1 changed file with 53 additions and 21 deletions Side-by-side Diff
sound/soc/fsl/fsl_ssi.c
@@ -224,6 +224,12 @@
  * @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;
@@ -263,6 +269,9 @@
  
 	const struct fsl_ssi_soc_data *soc;
 	struct device *dev;
+
+	u32 fifo_watermark;
+	u32 dma_maxburst;
 };
  
 /*
@@ -1051,21 +1060,7 @@
 	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) |
@@ -1373,12 +1368,8 @@
 		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;
  
@@ -1542,6 +1533,47 @@
 	else
                 /* Older 8610 DTs didn't have the fifo-depth property */
 		ssi_private->fifo_depth = 8;
+
+	/*
+	 * Set the watermark for transmit FIFO 0 and receive FIFO 0. We don't
+	 * use FIFO 1 but set the watermark appropriately nontheless.
+	 * We program the transmit water to signal a DMA transfer
+	 * if there are N elements left in the FIFO. For chips with 15-deep
+	 * FIFOs, set watermark to 8.  This allows the SSI to operate at a
+	 * high data rate without channel slipping. Behavior is unchanged
+	 * for the older chips with a fifo depth of only 8.  A value of 4
+	 * might be appropriate for the older chips, but is left at
+	 * fifo_depth-2 until sombody has a chance to test.
+	 *
+	 * 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;
+		break;
+	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);
...	...	@@ -224,6 +224,12 @@
224	224	* @dbg_stats: Debugging statistics
225	225	*
226	226	* @soc: SoC specific data
	227	+ *
	228	+ * @fifo_watermark: the FIFO watermark setting. Notifies DMA when
	229	+ * there are @fifo_watermark or fewer words in TX fifo or
	230	+ * @fifo_watermark or more empty words in RX fifo.
	231	+ * @dma_maxburst: max number of words to transfer in one go. So far,
	232	+ * this is always the same as fifo_watermark.
227	233	*/
228	234	struct fsl_ssi_private {
229	235	struct regmap *regs;
...	...	@@ -263,6 +269,9 @@
263	269
264	270	const struct fsl_ssi_soc_data *soc;
265	271	struct device *dev;
	272	+
	273	+ u32 fifo_watermark;
	274	+ u32 dma_maxburst;
266	275	};
267	276
268	277	/*
...	...	@@ -1051,21 +1060,7 @@
1051	1060	regmap_write(regs, CCSR_SSI_SRCR, srcr);
1052	1061	regmap_write(regs, CCSR_SSI_SCR, scr);
1053	1062
1054		- /*
1055		- * Set the watermark for transmit FIFI 0 and receive FIFO 0. We don't
1056		- * use FIFO 1. We program the transmit water to signal a DMA transfer
1057		- * if there are only two (or fewer) elements left in the FIFO. Two
1058		- * elements equals one frame (left channel, right channel). This value,
1059		- * however, depends on the depth of the transmit buffer.
1060		- *
1061		- * We set the watermark on the same level as the DMA burstsize. For
1062		- * fiq it is probably better to use the biggest possible watermark
1063		- * size.
1064		- */
1065		- if (ssi_private->use_dma)
1066		- wm = ssi_private->fifo_depth - 2;
1067		- else
1068		- wm = ssi_private->fifo_depth;
	1063	+ wm = ssi_private->fifo_watermark;
1069	1064
1070	1065	regmap_write(regs, CCSR_SSI_SFCSR,
1071	1066	CCSR_SSI_SFCSR_TFWM0(wm) \| CCSR_SSI_SFCSR_RFWM0(wm) \|
...	...	@@ -1373,12 +1368,8 @@
1373	1368	dev_dbg(&pdev->dev, "could not get baud clock: %ld\n",
1374	1369	PTR_ERR(ssi_private->baudclk));
1375	1370
1376		- /*
1377		- * We have burstsize be "fifo_depth - 2" to match the SSI
1378		- * watermark setting in fsl_ssi_startup().
1379		- */
1380		- ssi_private->dma_params_tx.maxburst = ssi_private->fifo_depth - 2;
1381		- ssi_private->dma_params_rx.maxburst = ssi_private->fifo_depth - 2;
	1371	+ ssi_private->dma_params_tx.maxburst = ssi_private->dma_maxburst;
	1372	+ ssi_private->dma_params_rx.maxburst = ssi_private->dma_maxburst;
1382	1373	ssi_private->dma_params_tx.addr = ssi_private->ssi_phys + CCSR_SSI_STX0;
1383	1374	ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + CCSR_SSI_SRX0;
1384	1375
...	...	@@ -1542,6 +1533,47 @@
1542	1533	else
1543	1534	/* Older 8610 DTs didn't have the fifo-depth property */
1544	1535	ssi_private->fifo_depth = 8;
	1536	+
	1537	+ /*
	1538	+ * Set the watermark for transmit FIFO 0 and receive FIFO 0. We don't
	1539	+ * use FIFO 1 but set the watermark appropriately nontheless.
	1540	+ * We program the transmit water to signal a DMA transfer
	1541	+ * if there are N elements left in the FIFO. For chips with 15-deep
	1542	+ * FIFOs, set watermark to 8. This allows the SSI to operate at a
	1543	+ * high data rate without channel slipping. Behavior is unchanged
	1544	+ * for the older chips with a fifo depth of only 8. A value of 4
	1545	+ * might be appropriate for the older chips, but is left at
	1546	+ * fifo_depth-2 until sombody has a chance to test.
	1547	+ *
	1548	+ * We set the watermark on the same level as the DMA burstsize. For
	1549	+ * fiq it is probably better to use the biggest possible watermark
	1550	+ * size.
	1551	+ */
	1552	+ switch (ssi_private->fifo_depth) {
	1553	+ case 15:
	1554	+ /*
	1555	+ * 2 samples is not enough when running at high data
	1556	+ * rates (like 48kHz @ 16 bits/channel, 16 channels)
	1557	+ * 8 seems to split things evenly and leave enough time
	1558	+ * for the DMA to fill the FIFO before it's over/under
	1559	+ * run.
	1560	+ */
	1561	+ ssi_private->fifo_watermark = 8;
	1562	+ ssi_private->dma_maxburst = 8;
	1563	+ break;
	1564	+ case 8:
	1565	+ default:
	1566	+ /*
	1567	+ * maintain old behavior for older chips.
	1568	+ * Keeping it the same because I don't have an older
	1569	+ * board to test with.
	1570	+ * I suspect this could be changed to be something to
	1571	+ * leave some more space in the fifo.
	1572	+ */
	1573	+ ssi_private->fifo_watermark = ssi_private->fifo_depth - 2;
	1574	+ ssi_private->dma_maxburst = ssi_private->fifo_depth - 2;
	1575	+ break;
	1576	+ }
1545	1577
1546	1578	dev_set_drvdata(&pdev->dev, ssi_private);
1547	1579