Re: buffer stuff

On Tue, Jul 09, 2002 at 12:19:27PM -0500, frank mori hess wrote:
> 
> Dave,
> 
> I noticed you checked a bunch of further work on the buffer stuff.  I have
> one complaint though: that you only kept comedi_buf_put().

Yes.  Because comedi_buf_put() is the only non-primitive function
that is relatively simple and widely used.

> I did create a
> bit of an explosion of variations, and simplifying it is fine, but if we
> just want one version of comedi_buf_put, it should be the general one,
> with a prototype similar to __comedi_buf_put_array().

There were several drivers that were allocating a temporary array,
copying data into it, and then using comedi_buf_put_array() to
copy that array to the buffer.  That is so wrong I can't begin
to describe.  Not only that, some of those drivers were allocating
the temporary array on the stack.

> But for example, at 20MHz with the pcidas-4020, calling comedi_buf_put()
> in a loop just eats up too much cpu time.

pcidas-4020 uses DMA, correct?  Then it should be allocating chunks
using comedi_buf_write_alloc(), DMAing to them, and then freeing
them using comedi_buf_write_free().  This will get all the
bookkeeping correct and also be zero-copy.  I haven't implemented
this correctly yet on any driver.

Note that DMA-based writing is a bit different from comedi_buf_put().
A DMA driver will have a large write region continuously allocated,
and add or subtract to it as necessary.  comedi_buf_put() allocates
2 bytes, writes to it, and then frees it.  I've converted a few of
the drivers that used to loop through comedi_buf_put() so that they
allocate a chunk, write all of it, and then free all of it.  Since
many drivers also need to munge the data, this is better than a
generic function.

The default buffer size for DMA-based drivers probably needs to be
increased, since 4 pages doesn't leave a lot of clearance for
trading pages between the application and the driver.  16 pages
is probably a reasonable minimum.  (Or make the DMA chunk size
variable, which would have other benefits.)

> Hmm, also all the handling of events seems to have been ripped out, which
> is going to cause problems... there seems to currently be a hack in
> comedi_event that always sets the BLOCK event?

I don't want the buffer code to handle setting of events, except
perhaps the COMEDI_CB_EOBUF event, which isn't all that useful
anyway.

COMEDI_CB_BLOCK has always tried to have two meanings, "convenient
block size", and "every interrupt", which are almost always the
same.  The differences are almost useless, so it might as well
be taken out of the drivers.

COMEDI_CB_ERROR should be split up, into "board error" and "buffer
overflow/underflow".  The drivers are now supposed to recognize
buffer overflows by the failure of comedi_buf_write_alloc() or
comedi_buf_put().

Also, async->cur_chan is for the use of the driver.  The core sets
it to zero at the start of a command, but otherwise won't touch it.
This means that the core has no way of determining when end-of-scan
events occur, so that is up to the drivers.  Setting COMEDI_CB_EOS
is unnecessary unless cmd->flags asked for it, and presumably, the
driver altered its behavior to handle it correctly.



dave...

Received on 2002-07-09Z18:04:40