It might be interesting to compare this with the implementation of the ARM/PRU ring buffer in http://beaglelogic.net. Ideally, we'll end up with a standard Linux vring implementation for efficiently communicating with the PRUs. Doing this communications efficiently is big advantage to us looking at moving to remote_proc rather than using uio_pruss where userspace needs to get involved and might slow down the data movement.
On Tue, Oct 21, 2014 at 2:44 PM, Rafael Vega <[email protected]> wrote: > Thanks everyone for the feedback! This is indeed more tricky than I had > thought. > I'll post back later with a revised implementation > :) > > > > > On Tuesday, October 21, 2014 12:44:05 PM UTC-5, Guy Grotke wrote: >> >> I have not done this with the PRU, but I work with all sorts of other >> chips that have to use ring buffers. It is usually easiest to start >> with head = tail = 0 as the starting (buffer empty) case. Then drop >> bytes on the sender side if head++ would == tail. It gets really simple >> if the head and tail indexes (not pointers) are a data size that equals >> the size of your ring buffer, for example a 256 byte buffer with >> unsigned char indexes, because then you just increment the indexes right >> across the 255=>0 wrap point. >> >> On 10/21/2014 8:06 AM, Bas Laarhoven wrote: >> > On 21-10-2014 16:33, Charles Steinkuehler wrote: >> >> On 10/20/2014 4:05 PM, Rafael Vega wrote: >> >>> Thanks for the input Peter, you made me realize I had a mistake: >> >>> >> >>> The idea here is that ONLY the PRU changes the end pointer (write >> >>> position >> >>> pointer) and ONLY the ARM changes the start pointer (read position >> >>> pointer). Also, the pointers are updated AFTER the data is read or >> >>> written >> >>> (thus the memory barrier on the ARM side). >> >>> >> >>> When the ARM is reading/updating the start pointer, the PRU could >> write >> >>> messages to fill the buffer, changing the start pointer and >> >>> corrupting the >> >>> buffer. To avoid this, I have changed the PRU code as follows. Note >> >>> that if >> >>> the buffer is full, new messages will be dropped, you can lower the >> >>> chances >> >>> of this happening by making the buffer larger. >> >>> >> >>> inline void buffer_write(unsigned int message){ >> >>> unsigned int is_full = (*buffer_end == >> >>> (*buffer_start^buffer_size)); // >> >>> ^ is orex >> >>> if(!is_full){ >> >>> shared_ram[*buffer_end & (buffer_size-1)] = message; >> >>> *buffer_end = (*buffer_end+1) & (2*buffer_size - 1); >> >>> } >> >>> } >> >> Careful with the buffer size handling! It's hard to tell the >> difference >> >> between empty and full, since in both cases the read and write >> pointers >> >> are identical. >> >> >> >> It looks like you're trying to use an extra bit of buffer_end to track >> >> empty/full, but I don't think your logic is correct. I don't see how >> >> the MSB (indicating buffer full) ever gets cleared once set. >> >> >> >> I've found it's usually much less hassle to live with a maximum buffer >> >> size of N-1 (ie: full = 127 elements, not 128) rather than try to >> >> properly handle the complexity of properly tracking empty/full. Note >> >> that there are *LOTS * of subtle ways you can mess up the empty/full >> >> logic (it's even harder to do properly than the basic pointer handling >> >> for reads/writes), so if you do try to use all buffer elements, be >> >> *VERY* careful with your code. I recommend reviewing the Linux kernel >> >> code if you need a reference implementation. >> >> >> > >> > Rafael, >> > >> > This is good advice from Charles! I've been running this n-1 scheme >> > with my own >> > PRU code and BeBoPr software for over two years now and it has proven >> > rock stable. >> > No concurrency or cache coherency issues, no locking needed and no >> > data re-ordering >> > problems! >> > >> > You can find the C-code in the following file: >> > https://github.com/modmaker/BeBoPr/blob/master/pruss_stepper.c >> > (start with 'pruss_command') >> > The PRUSS code is not open source, but I think the C-code shows enough >> > detail. >> > >> > Cheers, >> > -- Bas >> > >> >> -- > For more options, visit http://beagleboard.org/discuss > --- > You received this message because you are subscribed to the Google Groups > "BeagleBoard" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- For more options, visit http://beagleboard.org/discuss --- You received this message because you are subscribed to the Google Groups "BeagleBoard" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
