Re: [PATCH V3 3/3] i2c: tegra: Add DMA Support

[Dmitry Osipenko]

29.01.2019 5:02, Sowjanya Komatineni ÐÐÑÐÑ:
> 
> 
>>>> This patch adds DMA support for Tegra I2C.
>>>>
>>>> Tegra I2C TX and RX FIFO depth is 8 words. PIO mode is used for 
>>>> transfer size of the max FIFO depth and DMA mode is used for 
>>>> transfer size higher than max FIFO depth to save CPU overhead.
>>>>
>>>> PIO mode needs full intervention of CPU to fill or empty FIFO's and 
>>>> also need to service multiple data requests interrupt for the same 
>>>> transaction adding overhead on CPU for large transfers.
>>>>
>>>> DMA mode is helpful for Large transfers during downloading or 
>>>> uploading FW over I2C to some external devices.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
>>>>
>>>> ---
>>>
>>> Seems there are good news, APB DMA could access IRAM. Quote from TRM: "The source may be DRAM or IRAM, and the destination location could be devices placed on > APB Bus; or vice versa". Hence it shall be much more efficient to use IRAM for the DMA buffer, please consider that variant.
>>>
>>
>> Regarding your request for Write Combined memory, it doesnât guarantee the order.

The memory-access order doesn't matter at all in your case, all you need to care about is that actual data is read from dma-buffer and that caches are flushed after writing is done.

>> Main reason for adding DMA mode support to Tegra I2C driver is not for actual transfer performance but for not having delayed I2C transfer mainly in case of CPU fully loaded and I2C transfer during that time (FW upload/download, Sensor programming..) using PIO > mode introduces delay b/w bytes of transfer as it depends on CPU scheduling and interrupt priority which impacts on serving interrupts and filling FIFO for transfer to happen.
>>
>> Below are the cases where we see the need for DMA mode for I2C transfers (mainly during the use cases where CPU is fully loaded)
>> - Some devices need FW update over I2C and DMA mode helps in such large transfers on I2C reducing CPU intervention mainly when CPU is fully loaded.
>> - Some devices have internal timeout for no data activity on I2C bus after the start command (time restriction b/w data to data within single transaction) to avoid bus hang and they force to stop when timeout happens. with PIO mode in those cases, depending on CPU load, there could be more delay in serving interrupt for data requests and sending the data causing the device to trigger its timeout and transaction stops.
>> - Helps for reading data from I2C sensors with better BW.

Okay, since the efficiency between transfers is not the concern in your case, it doesn't really matter much where dma-buffer is located.

> Found from documentation default attribute is DMA_ATTR_WRITE_BARRIER which forces all pending writes to complete preventing race b/w completion notification and data DMA.
> Will use write-combined memory allocation for DMA buffer in revised patch..

Actually mapping is write-combined on ARM for the coherent DMA allocation with CONFIG_ARM_DMA_MEM_BUFFERABLE=y (enabled by default). I just falsely assumed that it could be writeback. Hence please keep the coherent allocation as-is. And you don't need to care about DMA_ATTR_WRITE_BARRIER, apparently it is only relevant for PCI on IA64.