kingst-la2016: rework acquisition limits, improve CLI use

Remove the rather arbitrary previous limits of 5MSa at 100MSa/s and 5%
capture ratio. Start with the highest available samplerate (per model)
and without any samples count limit. Use 50% capture ratio which is as
arbitrary a choice as the previous value but matches what scopes do.
Drop the lower samples count limit. (An arbitrary check encoded in the
library refuses to set a value of 0, so starting with 0 is the only way
of starting unlimited and being able to optionally specify a limit.)

This results in improved CLI use out of the box. Either of --samples or
--time works as expected, and both can be used in combination. The GUI
starts from a samples count limit by default that is consistent with
other devices. Users should be happy.

Accept any samplerate spec which is covered by hardware constraints.
Keep user provided specs unmodified for later reference, map these to
hardware register values which get forwarded to the device in a best
effort manner.

Make too high sample count limits non-fatal, just cap at 10GSa length.
The device's hardware compression which is affected by input signal
patterns already kept resulting in potentially shorter captures than
configured. There is no surprise here for users either. Use the model
dependent sample memory capacity to derive maximum pre-trigger memory
sizes, which eliminates another arbitrary magic number in the driver.

diff --git a/src/hardware/kingst-la2016/api.c b/src/hardware/kingst-la2016/api.c
index d89d25618ef81d0af2dd5404d54455ababa8b949..0b672eea7ee14226db9ff9a340154bf1cbdc23f8 100644 (file)
--- a/src/hardware/kingst-la2016/api.c
+++ b/src/hardware/kingst-la2016/api.c
@@ -33,16 +33,6 @@
 #include "libsigrok-internal.h"
 #include "protocol.h"
 
-/*
- * Default device configuration. Must be applicable to any of the
- * supported devices (no model specific default values yet). Specific
- * firmware implementation details unfortunately won't let us detect
- * and keep using previously configured values.
- */
-#define LA2016_DFLT_SAMPLERATE SR_MHZ(100)
-#define LA2016_DFLT_SAMPLEDEPTH        (5 * 1000 * 1000)
-#define LA2016_DFLT_CAPT_RATIO 5 /* Capture ratio, in percent. */
-
 static const uint32_t scanopts[] = {
        SR_CONF_CONN,
 };
@@ -568,9 +558,9 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options)
                }
 
                sr_sw_limits_init(&devc->sw_limits);
-               devc->sw_limits.limit_samples = LA2016_DFLT_SAMPLEDEPTH;
-               devc->capture_ratio = LA2016_DFLT_CAPT_RATIO;
-               devc->cur_samplerate = LA2016_DFLT_SAMPLERATE;
+               devc->sw_limits.limit_samples = 0;
+               devc->capture_ratio = 50;
+               devc->cur_samplerate = devc->model->samplerate;
                devc->threshold_voltage_idx = 0;
                devc->threshold_voltage = logic_threshold_value[devc->threshold_voltage_idx];
 
@@ -738,8 +728,7 @@ static int config_list(uint32_t key, GVariant **data,
                        *data = std_gvar_samplerates(ARRAY_AND_SIZE(rates_100mhz));
                break;
        case SR_CONF_LIMIT_SAMPLES:
-               *data = std_gvar_tuple_u64(LA2016_NUM_SAMPLES_MIN,
-                       LA2016_NUM_SAMPLES_MAX);
+               *data = std_gvar_tuple_u64(0, LA2016_NUM_SAMPLES_MAX);
                break;
        case SR_CONF_VOLTAGE_THRESHOLD:
                *data = std_gvar_min_max_step_thresholds(

diff --git a/src/hardware/kingst-la2016/protocol.c b/src/hardware/kingst-la2016/protocol.c
index 102c5cc4b5da87ff16e44c69d13819d9453730fe..9d60beb105ec2e475230be8e6c913200df7d9b6d 100644 (file)
--- a/src/hardware/kingst-la2016/protocol.c
+++ b/src/hardware/kingst-la2016/protocol.c
@@ -615,7 +615,7 @@ static int set_trigger_config(const struct sr_dev_inst *sdi)
 static int set_sample_config(const struct sr_dev_inst *sdi)
 {
        struct dev_context *devc;
-       double clock_divisor;
+       uint64_t min_samplerate, eff_samplerate;
        uint16_t divider_u16;
        uint64_t limit_samples;
        uint64_t pre_trigger_samples;
@@ -631,17 +631,15 @@ static int set_sample_config(const struct sr_dev_inst *sdi)
                        devc->cur_samplerate);
                return SR_ERR_ARG;
        }
-       if (devc->cur_samplerate < MIN_SAMPLE_RATE_LA2016) {
+       min_samplerate = devc->model->samplerate;
+       min_samplerate /= 65536;
+       if (devc->cur_samplerate < min_samplerate) {
                sr_err("Too low a sample rate: %" PRIu64 ".",
                        devc->cur_samplerate);
                return SR_ERR_ARG;
        }
-
-       clock_divisor = devc->model->samplerate / (double)devc->cur_samplerate;
-       if (clock_divisor > 65535)
-               return SR_ERR_ARG;
-       divider_u16 = (uint16_t)(clock_divisor + 0.5);
-       devc->cur_samplerate = devc->model->samplerate / divider_u16;
+       divider_u16 = devc->model->samplerate / devc->cur_samplerate;
+       eff_samplerate = devc->model->samplerate / divider_u16;
 
        ret = sr_sw_limits_get_remain(&devc->sw_limits,
                &limit_samples, NULL, NULL, NULL);
@@ -650,12 +648,14 @@ static int set_sample_config(const struct sr_dev_inst *sdi)
                return ret;
        }
        if (limit_samples > LA2016_NUM_SAMPLES_MAX) {
-               sr_err("Too high a sample depth: %" PRIu64 ".", limit_samples);
-               return SR_ERR_ARG;
+               sr_warn("Too high a sample depth: %" PRIu64 ", capping.",
+                       limit_samples);
+               limit_samples = LA2016_NUM_SAMPLES_MAX;
        }
-       if (limit_samples < LA2016_NUM_SAMPLES_MIN) {
-               sr_err("Too low a sample depth: %" PRIu64 ".", limit_samples);
-               return SR_ERR_ARG;
+       if (limit_samples == 0) {
+               limit_samples = LA2016_NUM_SAMPLES_MAX;
+               sr_dbg("Passing %" PRIu64 " to HW for unlimited samples.",
+                       limit_samples);
        }
 
        /*
@@ -670,23 +670,36 @@ static int set_sample_config(const struct sr_dev_inst *sdi)
         * limit the amount of sample memory to use for pre-trigger
         * data. Only the upper 24 bits of that memory size spec get
         * communicated to the device (written to its FPGA register).
+        *
+        * TODO Determine whether the pre-trigger memory size gets
+        * specified in samples or in bytes. A previous implementation
+        * suggests bytes but this is suspicious when every other spec
+        * is in terms of samples.
         */
-       pre_trigger_samples = limit_samples * devc->capture_ratio / 100;
-       pre_trigger_memory = LA2016_PRE_MEM_LIMIT_BASE;
-       pre_trigger_memory *= devc->capture_ratio;
-       pre_trigger_memory /= 100;
+       if (devc->trigger_involved) {
+               pre_trigger_samples = limit_samples;
+               pre_trigger_samples *= devc->capture_ratio;
+               pre_trigger_samples /= 100;
+               pre_trigger_memory = devc->model->memory_bits;
+               pre_trigger_memory *= UINT64_C(1024 * 1024 * 1024);
+               pre_trigger_memory /= 8; /* devc->model->channel_count ? */
+               pre_trigger_memory *= devc->capture_ratio;
+               pre_trigger_memory /= 100;
+       } else {
+               sr_dbg("No trigger setup, skipping pre-trigger config.");
+               pre_trigger_samples = 1;
+               pre_trigger_memory = 0;
+       }
+       /* Ensure non-zero value after LSB shift out in HW reg. */
+       if (pre_trigger_memory < 0x100) {
+               pre_trigger_memory = 0x100;
+       }
 
        sr_dbg("Set sample config: %" PRIu64 "kHz, %" PRIu64 " samples.",
-               devc->cur_samplerate / 1000, limit_samples);
+               eff_samplerate / SR_KHZ(1), limit_samples);
        sr_dbg("Capture ratio %" PRIu64 "%%, count %" PRIu64 ", mem %" PRIu64 ".",
                devc->capture_ratio, pre_trigger_samples, pre_trigger_memory);
 
-       if (!devc->trigger_involved) {
-               sr_dbg("Voiding pre-trigger setup. No trigger involved.");
-               pre_trigger_samples = 1;
-               pre_trigger_memory = 0x100;
-       }
-
        /*
         * The acquisition configuration occupies a total of 16 bytes:
         * - A 34bit total samples count limit (up to 10 billions) that

diff --git a/src/hardware/kingst-la2016/protocol.h b/src/hardware/kingst-la2016/protocol.h
index 1feabee37693d2cf4fbebb4e646fd6bf8886077e..8b1121ec726eea19f2b86ab1ee3c718aaf0150f9 100644 (file)
--- a/src/hardware/kingst-la2016/protocol.h
+++ b/src/hardware/kingst-la2016/protocol.h
@@ -69,24 +69,12 @@
 #define LA2016_THR_VOLTAGE_MIN 0.40
 #define LA2016_THR_VOLTAGE_MAX 4.00
 
-#define LA2016_NUM_SAMPLES_MIN (UINT64_C(256))
 #define LA2016_NUM_SAMPLES_MAX (UINT64_C(10 * 1000 * 1000 * 1000))
 
 /* Maximum device capabilities. May differ between models. */
-#define MAX_SAMPLE_RATE_LA2016 SR_MHZ(200)
-#define MAX_SAMPLE_RATE_LA1016 SR_MHZ(100)
-#define MIN_SAMPLE_RATE_LA2016 SR_KHZ(10)
 #define MAX_PWM_FREQ           SR_MHZ(20)
 #define PWM_CLOCK              SR_MHZ(200)     /* 200MHz for both LA2016 and LA1016 */
 
-/* TODO
- * What is the origin and motivation of that 128Mi literal? What is its
- * unit? How does it relate to a device's hardware capabilities? How to
- * map the 1GiB of RAM of an LA2016 (at 16 channels) to the 128Mi value?
- * It cannot be sample count. Is it memory size in bytes perhaps?
- */
-#define LA2016_PRE_MEM_LIMIT_BASE      (128 * 1024 * 1024)
-
 #define LA2016_NUM_PWMCH_MAX   2
 
 #define LA2016_CONVBUFFER_SIZE (4 * 1024 * 1024)