On Mon, Nov 07, 2022 at 12:21:26PM +0200, Ilpo Järvinen wrote:
> When DMA Rx completes, the current behavior is to just exit the DMA
> completion handler without future actions. If the transfer is still
> on-going, UART will trigger an interrupt and that eventually rearms the
> DMA Rx. The extra interrupt round-trip has an inherent latency cost
> that increases the risk of FIFO overrun. In such situations, the
> latency margin tends to already be less due to FIFO not being empty.
>
> Add check into DMA Rx completion handler to detect if LSR has DR (Data
> Ready) still set. DR indicates there will be more characters pending
> and DMA Rx can be rearmed right away to handle them.
Yep, I used to have something like draft of the below change locally.
Thanks for putting it in shape and upstreamimg!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Gilles BULOZ <gilles.buloz@xxxxxxxxxxx>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_dma.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
> index b85c82616e8c..37d6af2ec427 100644
> --- a/drivers/tty/serial/8250/8250_dma.c
> +++ b/drivers/tty/serial/8250/8250_dma.c
> @@ -38,9 +38,8 @@ static void __dma_tx_complete(void *param)
> spin_unlock_irqrestore(&p->port.lock, flags);
> }
>
> -static void __dma_rx_complete(void *param)
> +static void __dma_rx_complete(struct uart_8250_port *p)
> {
> - struct uart_8250_port *p = param;
> struct uart_8250_dma *dma = p->dma;
> struct tty_port *tty_port = &p->port.state->port;
> struct dma_tx_state state;
> @@ -57,6 +56,20 @@ static void __dma_rx_complete(void *param)
> tty_flip_buffer_push(tty_port);
> }
>
> +static void dma_rx_complete(void *param)
> +{
> + struct uart_8250_port *p = param;
> + struct uart_8250_dma *dma = p->dma;
> + unsigned long flags;
> +
> + __dma_rx_complete(p);
> +
> + spin_lock_irqsave(&p->port.lock, flags);
> + if (!dma->rx_running && (serial_lsr_in(p) & UART_LSR_DR))
> + p->dma->rx_dma(p);
> + spin_unlock_irqrestore(&p->port.lock, flags);
> +}
> +
> int serial8250_tx_dma(struct uart_8250_port *p)
> {
> struct uart_8250_dma *dma = p->dma;
> @@ -130,7 +143,7 @@ int serial8250_rx_dma(struct uart_8250_port *p)
> return -EBUSY;
>
> dma->rx_running = 1;
> - desc->callback = __dma_rx_complete;
> + desc->callback = dma_rx_complete;
> desc->callback_param = p;
>
> dma->rx_cookie = dmaengine_submit(desc);
> --
> 2.30.2
>
--
With Best Regards,
Andy Shevchenko
