The recent reworks of the fx2lafw made sure that the total buffer size is large
enough hold 500ms of data. This was done to improve performance and stability.
That the timeout value for a transfer was also increased to over 500ms, a side
effect of this is that when sampling is stopped there will be a additional delay
of 500ms. This is because the driver waits for all transfers to be freed
before it sends a SR_DF_END packet. Once sampling has stopped this will only
happen once a transfer times out. This patch cancels all pending transfers when
sampling is stopped, this will cause them to be freed almost immediately and the
additional delay will disappear.
Also make sure, that if we know, that we just have received the last transfer to
not resubmit this transfer again.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Currently timeout and buffer size are hard-coded in the fx2lafw driver which is
non-optimal if we want to get good results at both high and low sample rates.
The timeout is hard-coded to 40ms, which doesn't work well when sampling at
a low sample rate. E.g. at 20kHz filling all available buffer space alone takes
6 seconds. So naturally we'll see a lot of transfers timeout in this case.
The buffer size is hard-coded to 4096 bytes, which does not work well with high
sample rates. E.g. at 24MHz these 4096 bytes are enough space for 0.17ms of
data. The total buffer size is enough for about 5ms of data. Sooner or later the
application won't be able to resubmit a transfer within this time span and the
device will abort data acquisition. Usually this happens within the first few
seconds of sampling.
This patch adds a few new helper functions which calculate the buffer size and
timeout based on the current sample rate.
The buffer size is chosen to be large enough to hold about 10ms of data and it
also must be a multiple of 512 bytes since the firmware will send us the data
in 512 byte chunks.
The timeout is set to the time it would take to fill the whole available buffer
space plus a 25% headroom to accommodate for jitter. This is more than enough,
but there is no need to make the timeout a tight deadline, since it is only
meant as a last resort in case the device stops submitting data. And in this
case data acquisition will be aborted anyway.
The patch also limits the the number of transfers so that the total buffer
space is not much more of 500ms. This will ensure that we do not have to
wait too long when aborting data acquisition.
This patch also significantly reduces the number of context switches when
sampling at a higher sample rate. On my system for example the CPU load of
sigrok-cli when sampling at 24MHz goes down from ~25% to 3-4%.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
While errors are usually already implicitly caught by looking at the packet
length field there is one error status which is worth special handling. If the
device has been removed there is not really a chance to recover from this error
so data acquisition can be stopped immediately.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
In receive_transfer for each completed transfer a new buffer is allocated and
the old one is freed. We can avoid this by simply reusing the buffer for the
next transfer. This is possible if we only resubmit the transfer after all
processing on the data buffer has been done. A new buffer is only allocated if
the size of the old one is not 4096 bytes.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
When freeing a transfer we also have to free the transfer buffer. We also have
to keep track of the number of allocated transfers and if the freed transfer was
the last one stop acquisition. This patch introduces a helper function which
takes care of all of this.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
The header and packet struct are only used in the scope of this function and
they are freed at the end of it. Also these structs are rather small, so they
can safely be allocated on the stack. By doing so memory leaks on the error
paths are avoided.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Commit 88b75eb719 ("fx2lafw: Added device caps and added support for wide
sampling") increased the size of the trigger buffer from 8 to 16 bit, but forgot
to adjust the unitsize logic packet which is used to send the contents of the
trigger buffer. This patch sets the unitsize to sizeof() of the trigger buffer.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Multistage triggers currently do no work, because there is a return statement
in the middle of the trigger detector which will be hit as soon as the first
stage in a multistage trigger matches. This patch removes the return statement
so that the trigger detector can continue to try to match the next stage. In
order for this to work we also make sure that the trigger stage is only reset
if the current sample does not match.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
There are a few memory leaks in the receive_transfer transfer function. The most
serve of them is that a sample buffer is not freed if the triggered has not
matched yet, which causes a sigrok process which is waiting for a trigger to
consume several megabytes of memory within seconds. The other leaks are on the
error paths in that function.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
The glib GTimeVal data type (and some functions using it) will be faded
out from glib sooner or later, so it's not a good idea to use them anyway.
In this specific case GTimeVal.tv_sec was overflowing, leading a check in
libsigrok to fail, and thus to FX2 firmware upload errors, i.e.
non-working fx2lafw devices.
http://thread.gmane.org/gmane.comp.debugging.sigrok.devel/166
The root cause is that GTimeVal.tv_sec is a 'glong' (8 bytes on 64bit
systems, but only 4 on 32bit systems).
We now use an int64_t (and g_get_monotonic_time() instead of the more
problematics g_get_current_time() which uses a GTimeVal).
This has been verified to fix the issue on a 32bit system.
Other uses of GTimeVal in libsigrok will be removed in a later release.
Also, drop unneeded GTV_TO_MSEC.
Start/stop acquisition callbacks: Consistently name the 'void *' parameter
cb_data for now. The per-device-instance device pointer is called
'session_dev_id' everywhere for now, but this should be renamed to
something more clear.
Lars-Peter Clausen
Joel Holdsworth
Bert Vermeulen
Uwe Hermann