On 09/20/16 18:00, Vignesh R wrote: >>> Given the HW, I think it is wrong to have RX FIFO set in a way we do atm. > > > I think RX trigger was set to 48 bytes as that is 3/4th the size of UART > FIFO size. That may not a wise choice for console usecase. > > But there is one thing to verify: with .src_maxburst = 1 and .rx_size = > 4K, there won't be any timeout as every byte that arrives at RX FIFO is > immediately taken out by DMA and if <4K bytes of data is received, DMA > does not raise completion interrupt and data is stuck in destination > buffer forever. It is never pushed to higher layer. (I have not verified > whether this is the case when RX_TRIGGER is 1 or we will get a TIMEOUT > at the end of transfer irrespective of whether data is taken out from > FIFO or not with RX_TRIGGER as 1) I got it. Basically you need to have the transfer_size == DMA-burst == FIFO rx threshold in order to this logic to work. But if you have RX_TRIGGER as 1, you will keep the UART FIFO empty and the data is moved to memory as it arrives, right? With dmaengine_prep_slave_single() you tell the DMA that you want it to move dma->rx_size amount of bytes from UART to the memory (with a given dma_slave_config). In this case DMA - rightfully - only going to let you know when it is done moving the dma->rx_size amount. It does not matter if you have half an hour gap between receiving the first 10 bytes and the rest. It is the driver using the DMAengine who needs to care about expected timeout and handle that. >From the DMA point of view it is perfectly OK to ask it to transfer 4K. It is perfectly OK that after each K transfer you have seconds, minutes, whatever delay, it will wait for the IP to tell the DMA that now you can move RX_THRESHOLD amount, please. And when some entity decides that we have waited enough for data, we should first disable the DMA request generation in UART, check the DMA pointer, stop the UART reception. Not sure, but disabling the DMA request in UART is a good replacement of the missing pause support in sDMA and for the resume as well. If you stop the DMA requests the DMA will stop as well when finished the ongoing transfer of RX_THRESHOLD amount. > >>> >>> FWIW the following change works on omap5-uevm (sDMA) with DMA enabled serial >>> console: >>> >>> diff --git a/drivers/tty/serial/8250/8250_omap.c >>> b/drivers/tty/serial/8250/8250_omap.c >>> index 61ad6c3b20a0..fa1b1b6fa83b 100644 >>> --- a/drivers/tty/serial/8250/8250_omap.c >>> +++ b/drivers/tty/serial/8250/8250_omap.c >>> @@ -80,7 +80,7 @@ >>> #define OMAP_UART_TX_WAKEUP_EN (1 << 7) >>> >>> #define TX_TRIGGER 1 >>> -#define RX_TRIGGER 48 >>> +#define RX_TRIGGER 1 >>> >>> #define OMAP_UART_TCR_RESTORE(x) ((x / 4) << 4) >>> #define OMAP_UART_TCR_HALT(x) ((x / 4) << 0) >>> @@ -1215,7 +1215,6 @@ static int omap8250_probe(struct platform_device *pdev) >>> priv->omap8250_dma.fn = the_no_dma_filter_fn; >>> priv->omap8250_dma.tx_dma = omap_8250_tx_dma; >>> priv->omap8250_dma.rx_dma = omap_8250_rx_dma; >>> - priv->omap8250_dma.rx_size = RX_TRIGGER; >>> priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER; >>> priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER; > > > Did you test after removing the line: > priv->rx_dma_broken = true; > > W/o this, DMA is still disabled. Oh, yes. The RX DMA is disabled so we read out with PIO (FIFO Interrupt mode). This is because of the issues around the DMA pause mostly? >> and this fixes the annoying lag on the serial console as well I have since the >> DMA was enabled for the UART as typing one character would be not enough to >> trigger the DMA, resulting wait for the timeout and flushing the data out from >> the FIFO. >> >> I really see no point of having DMA for the serial console... not with the >> current setup. Probably for TX, but for sure not for the RX. >> >> So I give my: >> Tested-by/Reviewed-by/Recommended-by to this change. >> >> I know that this might affect performance as we constantly run the DMA from >> UART to memory, but it makes the serial console usable. >> > > So, I guess we can use TX_TRIGGER = RX_TRIGGER = 48 bytes if we ignore > DMA for console use case The TX_TRIGGER is a bit tricky. What if the 'dma->tx_size - skip_byte' is less then 48 bytes? You ask DMA to move 48 bytes, but you don't have 48 bytes valid data to transfer... The TX_TRIGGER should be decided for each transfer. And I'm still not convinced that it is good to have 48 bytes for RX_THRESHOLD given the fact that RX DMA is disabled ;) -- Péter -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
