Hi Sebastian, Thanks for the response. On Thursday 15 September 2016 06:52 PM, Sebastian Andrzej Siewior wrote: > On 2016-09-14 17:27:37 [+0530], Vignesh R wrote: >> 1. 8250_dma DMA sets up DMA RX transfer size to be PAGE_SIZE or 4K >> bytes. 8250_omap RX DMA trigger size is currently 48 bytes. Which means >> whenever there are 48 bytes in UART FIFO, DMA is triggered and data is >> taken out by DMA, this happens until 4K bytes are fetched and then DMA >> completion callback is invoked which pushes that data to tty layer. If >> number of bytes in FIFO <48 bytes then RX timeout is raised and data >> driver pushes the data to tty layer. But if the data received is >> multiple of 48 bytes but <4KB then, neither there is RX timeout(as data >> is taken out by DMA) nor DMA completion callback(as 4K bytes have not >> yet been received). This means the data is stuck in DMA destination >> forever. >> Now, I believe this is a generic problem for all 8250 UARTs >> using core 8250_dma. I am not sure how to workaround this. Any ideas?? > > counting is important here. As far as I remember if you receive one byte > _before_ you programmed the DMA engine then the core will wait for 48 > *additional* bytes. That means one byte in the FIFO, DMA engine > programmed, 47 bytes follow (so 48 in total) will result in an timeout > interrupt. I am sure this was the case with EDMA not sure about SDMA but > I *think* it is the same case. I do *not* see this issue with SDMA. But seems to be specific to EDMA. > This is why the OMAP's DMA code programs the DMA transfer upfront. > > Intel's UART on the hand programs the transfer later for 4KiB and the > transfer completes with at "some point". I am not sure if this is OMAP > that does it different compared to Intel or if everyone else behaves > that way except for Intel. This is something I am yet to understand. I don't understand how DMA can handle the case where transfer size is multiple of burst size but <4KiB. Perhaps DMA IP on Intel platform is UART specific. IMO, we may have to periodically drain data that DMA has picked and put into TTY buffer. Setting up a single DMA transfer for 48bytes (burst size = transfer size) seems to be too much of overhead, especially for high bandwidth applications. > >> 2. TX DMA stalls if UART_MCR register is written to(as part of >> set_termios call) when DMA is in progress. Currently, this is worked >> around by delaying termios change. This results in deviation from TX DMA >> flow. IMO, changing termios when transfer is in progress is not a >> real usecase and not recommended. So, either driver can error out in >> this condition or avoid touching UART_MCR register(which is mainly used >> to switch to Loopback mode and enable access to DMA trigger >> configuration register). > > this workaround was introduced after busybox userland trigger this bug > (I think busybox's vi was the test case). > If you error out you probably break *this* userland because the settings > busybox asked for were not applied. And if I am not wrong, set_termios > does not return an error code. > I did test busybox vi, by opening a new file, adding few lines of text and closing it. And did not see any TX DMA stall. Do you recall the exact test that caused TX DMA stall? As per my testing with other test cases, I have root caused that accessing UART_MCR register during set_termios might cause TX DMA stall sometime. I think we can safely avoid touching UART_MCR register when TX DMA is in progress. >> So I propose to submit patches to allow 8250_omap to use 8250_dma for >> UART DMA support and remove UART DMA support for AM335x and AM437x that >> do not have SDMA. > > Nobody stepped up to get this pause-RX transfer merged. You need this > because if you can receive less than 48 bytes and you need to purge the > FIFO manually. And for this you need to pause the transfer _unless_ you > find a way to deal with this situation in a different way. > So before thinking about enabling SDMA you need to get this fixed first. > Yes, I understand. I do intend to re-submit patch authored by you to support RX DMA pause for SDMA, if you are okay with that. > And then why would you want to get rid of EDMA support? As far as I > remember it is working and you can enable RX path. I left it off because > it was easier to disable it for both than to check if we are using EDMA > or SDMA. > But you have still open issues. One thing is that you receive two > interrupts per complete transfer: one from UART and one from DMA. The > UART interrupt should be suppressed / taken care. (I remember someone > pointed out that the Intel manual pointed out that we use an invalid > interrupt mask / MCR register settings but don't remember the details. I > *think* that problem is also present on the Intel HW.) I don't see multiple interrupts(UART IRQ + SDMA completion IRQ) with SDMA. And I am pretty sure, UART IRQs should never fire in UART is in DMA mode even with EDMA (not verified though). > Right now, on high baud rates it is possible that UART interrupt will be > disabled because it returned often enough with IRQ_NONE while the DMA > engine handled all RX packets. I have tested SDMA at low baudrate of 9600 but still do see UART interrupt in DMA mode. > Your second problem is that the starting / programming of a transfer > (RX) is deferred into softirq context which may be too late. That means > you can receive a TIMEOUT interrupt _before_ you the softirq programmed > the dma_engine and the following dma_pause() will returned an error. This is one more problem specific to EDMA, which seems to require DMA to be programmed before UART data ready interrupt and hence, DMA is programmed in DMA completion callback which is a softirq context. But in case of SDMA using core 8250_dma, DMA is programmed in IRQ context. > > I *strongly* suggest you identify and fix the open issue before you > throw out EDMA support. I am sure that there are people that are using > it on am335x. > DMA support is currently disabled in the driver. Even if enabled, EDMA is being used to transfer 48 bytes of data per DMA transaction. I am not sure how beneficial is it to program DMA for every 48 bytes of data especially when user is doing a high bw data transfer (say via BT). Even if EDMA is setup to read 4KiB data with burst size of 48 bytes, then EDMA does not provide to way to know how many bytes have been transferred when dmaengine_pause() is requested. Also there are spurious interrupts with EDMA (even with acking UART_IIR_NO_INT case) which is specific to am335x SoC. Dropping EDMA support for UART on am335x was in specific to Peter Hurley asking whether TI UART design can support generic 8250 dma style tx/rx flow[1]. AFAIU, the main concern was whether all these workarounds in 8250_omap to support DMA are just being carried to support EDMA on am335x and am437x. And as per my experiments, the answer is yes. [1]https://lkml.org/lkml/2016/4/12/1014 (and other mails in same thread) -- Regards Vignesh -- 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
