[PATCH v2] serial: imx: adapt rx buffer and dma periods

[PATCH v2] serial: imx: adapt rx buffer and dma periods

[Philipp Puschmann]

Using only 4 DMA periods for UART RX is very few if we have a high
frequency of small transfers - like in our case using Bluetooth with
many small packets via UART - causing many dma transfers but in each
only filling a fraction of a single buffer. Such a case may lead to
the situation that DMA RX transfer is triggered but no free buffer is
available. While we have addressed the dma handling already with
"dmaengine: imx-sdma: fix dma freezes" we still want to avoid
UART RX FIFO overrun. So we decrease the size of the buffers and
increase their number and the total buffer size.

Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---

Changelog v2:
 - split this patch from series "Fix UART DMA freezes for iMX6"
 - add Reviewed-by tag

 drivers/tty/serial/imx.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 87c58f9f6390..51dc19833eab 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1034,8 +1034,6 @@ static void imx_uart_timeout(struct timer_list *t)
 	}
 }
 
-#define RX_BUF_SIZE	(PAGE_SIZE)
-
 /*
  * There are two kinds of RX DMA interrupts(such as in the MX6Q):
  *   [1] the RX DMA buffer is full.
@@ -1118,7 +1116,8 @@ static void imx_uart_dma_rx_callback(void *data)
 }
 
 /* RX DMA buffer periods */
-#define RX_DMA_PERIODS 4
+#define RX_DMA_PERIODS	16
+#define RX_BUF_SIZE	(PAGE_SIZE / 4)
 
 static int imx_uart_start_rx_dma(struct imx_port *sport)
 {
-- 
2.23.0

Re: [PATCH v2] serial: imx: adapt rx buffer and dma periods

[Uwe Kleine-König]

On Thu, Sep 19, 2019 at 12:26:28PM +0200, Philipp Puschmann wrote:
> Using only 4 DMA periods for UART RX is very few if we have a high
> frequency of small transfers - like in our case using Bluetooth with
> many small packets via UART - causing many dma transfers but in each
> only filling a fraction of a single buffer. Such a case may lead to
> the situation that DMA RX transfer is triggered but no free buffer is
> available. While we have addressed the dma handling already with
> "dmaengine: imx-sdma: fix dma freezes" we still want to avoid

Is this statement still true now that you split this patch out of your
bigger series?

> UART RX FIFO overrun. So we decrease the size of the buffers and
> increase their number and the total buffer size.

What happens when such an RX FIFO overrun happens? Are characters lost?
Or only time? Does your change have an influence if I do fewer but
bigger transfers?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2] serial: imx: adapt rx buffer and dma periods

[Philipp Puschmann]

Hi Uwe

Am 19.09.19 um 13:22 schrieb Uwe Kleine-König:
> On Thu, Sep 19, 2019 at 12:26:28PM +0200, Philipp Puschmann wrote:
>> Using only 4 DMA periods for UART RX is very few if we have a high
>> frequency of small transfers - like in our case using Bluetooth with
>> many small packets via UART - causing many dma transfers but in each
>> only filling a fraction of a single buffer. Such a case may lead to
>> the situation that DMA RX transfer is triggered but no free buffer is
>> available. While we have addressed the dma handling already with
>> "dmaengine: imx-sdma: fix dma freezes" we still want to avoid
> 
> Is this statement still true now that you split this patch out of your
> bigger series?
Yes. The dma patches care about stopping DMA channel. This patch tries to
avoid that the channel runs out of usable buffers (aka dma periods).

> 
>> UART RX FIFO overrun. So we decrease the size of the buffers and
>> increase their number and the total buffer size.
> 
> What happens when such an RX FIFO overrun happens? Are characters lost?
> Or only time?
Good question. In explanation i have missed an important point:
When using HW flowcontrol via RTS/CTS and the buffer is full CTS is used to
tell the remote device - here the Bluetooth chip - to stop sending data.
For a while this prevents losing of characters. But then the remote device
comes into trouble as its internal TX buffers runs over. Depends on the
device how it handles this case and if it recovers if data flow is enabled
again.

In case without HW flow control characters would be lost. Depends on the upper
layer what happens then.

> Does your change have an influence if I do fewer but
> bigger transfers?
Don't think so. The dma periods are raw data buffers. If one is full the next one
is being used. For the performance i don't see a significant difference between
using 1 kB buffers or 4 kB buffers.

Regards,
Philipp

> 
> Best regards
> Uwe
>

Re: [PATCH v2] serial: imx: adapt rx buffer and dma periods

[Uwe Kleine-König]

On Thu, Sep 19, 2019 at 01:40:03PM +0200, Philipp Puschmann wrote:
> Hi Uwe
> 
> Am 19.09.19 um 13:22 schrieb Uwe Kleine-König:
> > On Thu, Sep 19, 2019 at 12:26:28PM +0200, Philipp Puschmann wrote:
> >> Using only 4 DMA periods for UART RX is very few if we have a high
> >> frequency of small transfers - like in our case using Bluetooth with
> >> many small packets via UART - causing many dma transfers but in each
> >> only filling a fraction of a single buffer. Such a case may lead to
> >> the situation that DMA RX transfer is triggered but no free buffer is
> >> available. While we have addressed the dma handling already with
> >> "dmaengine: imx-sdma: fix dma freezes" we still want to avoid
> > 
> > Is this statement still true now that you split this patch out of your
> > bigger series?
> Yes. The dma patches care about stopping DMA channel. This patch tries to
> avoid that the channel runs out of usable buffers (aka dma periods).
> 
> > 
> >> UART RX FIFO overrun. So we decrease the size of the buffers and
> >> increase their number and the total buffer size.
> > 
> > What happens when such an RX FIFO overrun happens? Are characters lost?
> > Or only time?
> Good question. In explanation i have missed an important point:
> When using HW flowcontrol via RTS/CTS and the buffer is full CTS is used to
> tell the remote device - here the Bluetooth chip - to stop sending data.
> For a while this prevents losing of characters. But then the remote device
> comes into trouble as its internal TX buffers runs over. Depends on the
> device how it handles this case and if it recovers if data flow is enabled
> again.
> 
> In case without HW flow control characters would be lost. Depends on the upper
> layer what happens then.
> 
> > Does your change have an influence if I do fewer but
> > bigger transfers?
> Don't think so. The dma periods are raw data buffers. If one is full the next one
> is being used. For the performance i don't see a significant difference between
> using 1 kB buffers or 4 kB buffers.

Would be great to add these infos to the commit log.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |