DMA for RPi

[Pavel Pisa]

Hello Mudit,

I have put some comments inline on GitHub.

On Sunday 31 of July 2016 13:14:46 Mudit Jain wrote:
> Hi all,
>
> I have added code for DMA support for RPi. Here is the link for the same :
>
> GitHub Link :
> https://github.com/spark1729/rtems/commit/f328c4cd89b02977ccc5d7154b4af5ee9
>c424ba3
>
> Kindly review the code and provide comments to improve the same.
>
> Questions :
> If someone could specifically look at the portion in the rpi_dma_init
> function, marked by FIXME. I require some guidance to ascertain the logic
> implemented is right.

I am not sure why 

error = bus_dmamap_load_buffer( dm_segments, cb_virt,
      sizeof( struct bcm_dma_cb ), 0x00, &lastaddr, &nsegs, 1 );

is used allocated during initialization at all. It looks like preparation of 
some preallocated buffers for future DMA operations, but as I understand, you 
have only one cb per channel allocated and leave its setup to the user.

As you state, that you have used BSD code, please keep copyright
(plus add yours) if substantial part is copied or if that is
not case then put there standard RTEMS header and write comment
that code is inspired, modeled etc. by (precise source specification).

> Notes :
>
> 1. The basic idea is to control a particular channel using a control block
> structure. The structure is populated and the address of the same in
> written into a particular register corresponding to that channel.

As I understand, one DMA control structure per one channel for now
no scatter-gather. Thats OK but then bus_dmamap_load_buffer
is used, required for now. Am I not right?

> 2. The software context is primarily a array of structures [ channel
> structures ]. The control block is a member of the channel struct and is
> used to control that particular channel.
>
> 3. Important thing to note is that the DMA controller is directly connected
> to the peripherals, so it deals with real address spaces. Thus cache
> coherency is required when you consider changing addresses between
> different address spaces.

You use  rtems_cache_flush_multiple_data_lines on data area
before transfer. RTEMS API doe not distinguish between
flush with invalidate and clean only and I think that flush
causes only pushing data to main memory but not invalidate
on RAM as it is implemented now. This means that flush
is enough for transfers from cached memory to device
but not enough for other direction.

I am not sure if user data cache synchronization belongs
to DMA API functions, user knows if used area is some
device, special memory or main cached memory.
So if it is documented, I would incline to left
cache synchronization on DMA API user.

On the other hand synchronization of control blocks content
is duty of DMA API.
 

> 4. As mentioned above, the logic is taken from the FreeBSD DMA controller
> for BCM2835.
>
> 5.Instead of adding the entire dma_tag API, I have added relevant sections
> which offer the same utility. For mutex, I have used, the struct Atomic
> flag in RTEMS.

It can be used to flag concurrent use as error, it is appropriate
for concuretent access to single channel but disqualifies use of independent
DMA channels for different drivers. See my comments on GitHub.

> 6. I have run uncrustify on the two files to account for the code
> conventions and added comments to increase readability. The files are added
> to the build process and builds without any errors.
OK

> 7. The PAGE_SIZE is taken as 4096.

No problem but I think that function which uses it is out of scope
of actual API.

> 8. *rtems_cache_flush_multiple_data_lines,

OK for memory to device.

> rtems_cache_coherent_allocate *- 
> These functions in RTEMS are used to maintain cache coherency.

Cache coherent heap is not implemented for RaspberryPi BSP (yet). I have found 
support only for powerpc/qoriq, arm/altera-cyclone-v, arm/atsam/startup and 
arm/xilinx-zynq till now.

So the call rtems_cache_coherent_allocate returns regular cached memory.

One option is to implement coherent heap, other easier for now, is to allocate 
space for bcm_dma_cb with cache_aligned_malloc and ensure push of data to DMA 
engine visible memory before transfer by 
rtems_cache_flush_multiple_data_lines

Best wishes,

              Pavel