From: Gerhard Sittig Date: Sun, 30 Jan 2022 07:42:21 +0000 (+0100) Subject: kingst-la2016: rework acquisition limits, improve CLI use X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=d8fbfcd9d6d66bffcb8607e9de7706ce322d42c7;p=libsigrok.git 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 d89d2561..0b672eea 100644 --- 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 102c5cc4..9d60beb1 100644 --- 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 1feabee3..8b1121ec 100644 --- 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)