linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] serial: imx: adapt rx buffer and dma periods
@ 2019-09-19 10:26 Philipp Puschmann
  2019-09-19 11:22 ` Uwe Kleine-König
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Puschmann @ 2019-09-19 10:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: festevam, fugang.duan, linux-serial, gregkh, s.hauer, linux-imx,
	kernel, jslaby, Philipp Puschmann, yibin.gong, shawnguo,
	linux-arm-kernel, l.stach

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] serial: imx: adapt rx buffer and dma periods
  2019-09-19 10:26 [PATCH v2] serial: imx: adapt rx buffer and dma periods Philipp Puschmann
@ 2019-09-19 11:22 ` Uwe Kleine-König
  2019-09-19 11:40   ` Philipp Puschmann
  0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2019-09-19 11:22 UTC (permalink / raw)
  To: Philipp Puschmann
  Cc: festevam, fugang.duan, linux-serial, gregkh, s.hauer,
	linux-kernel, linux-imx, kernel, jslaby, yibin.gong, shawnguo,
	linux-arm-kernel, l.stach

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/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] serial: imx: adapt rx buffer and dma periods
  2019-09-19 11:22 ` Uwe Kleine-König
@ 2019-09-19 11:40   ` Philipp Puschmann
  2019-09-19 12:00     ` Uwe Kleine-König
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Puschmann @ 2019-09-19 11:40 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: festevam, fugang.duan, linux-serial, gregkh, s.hauer,
	linux-kernel, linux-imx, kernel, jslaby, yibin.gong, shawnguo,
	linux-arm-kernel, l.stach

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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] serial: imx: adapt rx buffer and dma periods
  2019-09-19 11:40   ` Philipp Puschmann
@ 2019-09-19 12:00     ` Uwe Kleine-König
  0 siblings, 0 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2019-09-19 12:00 UTC (permalink / raw)
  To: Philipp Puschmann
  Cc: fugang.duan, kernel, gregkh, s.hauer, linux-kernel, linux-imx,
	linux-serial, jslaby, shawnguo, yibin.gong, festevam,
	linux-arm-kernel, l.stach

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/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-09-19 12:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19 10:26 [PATCH v2] serial: imx: adapt rx buffer and dma periods Philipp Puschmann
2019-09-19 11:22 ` Uwe Kleine-König
2019-09-19 11:40   ` Philipp Puschmann
2019-09-19 12:00     ` Uwe Kleine-König

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).