On 08/10/2015 06:07 AM, Jiada Wang wrote:
> Hi Lucas
>
>
> On 08/06/2015 01:03 AM, Lucas Stach wrote:
>> Triggering the DMA engine for every byte is horribly inefficient.
>> Also it doesn't allow to use the aging timer for the RX FIFO as this
>> requires the DMA engine to leave one byte remaining in the FIFO when
>> doing a normal burst transfer.
>>
>> Adjust watermark levels so that the DMA engine can do at least 8 byte
>> burst transfers. This is a conservative value, as the both TX and RX
>> FIFOs are able to contain 32 bytes.
>>
>> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
>> ---
>> drivers/tty/serial/imx.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 7bb683264013..363c82dcfbb7 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -971,6 +971,8 @@ static int start_rx_dma(struct imx_port *sport)
>>
>> #define TXTL_DEFAULT 2 /* reset default */
>> #define RXTL_DEFAULT 1 /* reset default */
>> +#define TXTL_DMA 8 /* DMA burst setting */
>> +#define RXTL_DMA 9 /* DMA burst setting */
>>
>> static void imx_setup_ufcr(struct imx_port *sport,
>> unsigned char txwl, unsigned char rxwl)
>> @@ -1018,7 +1020,8 @@ static int imx_uart_dma_init(struct imx_port *sport)
>> slave_config.direction = DMA_DEV_TO_MEM;
>> slave_config.src_addr = sport->port.mapbase + URXD0;
>> slave_config.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
>> - slave_config.src_maxburst = RXTL_DEFAULT;
>> + /* one byte less than the watermark level to enable the aging timer */
>> + slave_config.src_maxburst = RXTL_DMA - 1;
>> ret = dmaengine_slave_config(sport->dma_chan_rx, &slave_config);
>> if (ret) {
>> dev_err(dev, "error in RX dma configuration.\n");
>> @@ -1042,7 +1045,7 @@ static int imx_uart_dma_init(struct imx_port *sport)
>> slave_config.direction = DMA_MEM_TO_DEV;
>> slave_config.dst_addr = sport->port.mapbase + URTX0;
>> slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
>> - slave_config.dst_maxburst = TXTL_DEFAULT;
>> + slave_config.dst_maxburst = TXTL_DMA;
>> ret = dmaengine_slave_config(sport->dma_chan_tx, &slave_config);
>> if (ret) {
>> dev_err(dev, "error in TX dma configuration.");
>> @@ -1079,6 +1082,8 @@ static void imx_enable_dma(struct imx_port *sport)
>> temp |= UCR4_IDDMAEN;
>> writel(temp, sport->port.membase + UCR4);
>>
>> + imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
>> +
>> sport->dma_is_enabled = 1;
>> }
>>
>> @@ -1101,6 +1106,8 @@ static void imx_disable_dma(struct imx_port *sport)
>> temp &= ~UCR4_IDDMAEN;
>> writel(temp, sport->port.membase + UCR4);
>>
>> + imx_setup_ufcr(sport, TXTL_DEFAULT, RXTL_DEFAULT);
>> +
>> sport->dma_is_enabled = 0;
>> }
>>
>>
> How about make watermark value configurable via DTS?
Not via devicetree, please. sysfs is the suitable interface for adding
this type of configuration.
Regards,
Peter Hurley
> For some specific user case & HW, user may want to have higer rx
> watermark value to improve performance more,
> for example in case HW flow control is available, then it's safe to have
> higher rx watermark value (by set CTS trigger level properly)
> if there is no specific setting in DTS, then we can use default value
> RXTL_DMA/TXTL_DMA
>
> What do you think?
--
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
