| Message ID | Z1_rRXqdhxhL6wBw@mva-rohm (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] iio: gts: Simplify available scale table build | expand |
On Mon, 16 Dec 2024 10:56:37 +0200 Matti Vaittinen <mazziesaccount@gmail.com> wrote: > Make available scale building more clear. This hurts the performance > quite a bit by looping throgh the scales many times instead of doing > everything in one loop. It however simplifies logic by: > - decoupling the gain and scale allocations & computations > - keeping the temporary 'per_time_gains' table inside the > per_time_scales computation function. > - separating building the 'all scales' table in own function and doing > it based on the already computed per-time scales. > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> Looks good to me, but I want to leave it on list a while before applying. Ideal if it gets some tested-by or other tags before I pick it up. As always, this is fiddly code, so the more eyes the better! Jonathan > --- > Revision history: > v2: > - Add a few comments > - Use more descriptive variable name > > This is still only tested using the kunit tests. All further testing is > still highly appreciated! > --- > drivers/iio/industrialio-gts-helper.c | 272 ++++++++++++++++---------- > 1 file changed, 174 insertions(+), 98 deletions(-) > > diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c > index 291c0fc332c9..65697be58a10 100644 > --- a/drivers/iio/industrialio-gts-helper.c > +++ b/drivers/iio/industrialio-gts-helper.c > @@ -160,16 +160,123 @@ static void iio_gts_purge_avail_scale_table(struct iio_gts *gts) > gts->num_avail_all_scales = 0; > } > > +static int scale_eq(int *sc1, int *sc2) > +{ > + return sc1[0] == sc2[0] && sc1[1] == sc2[1]; > +} > + > +static int scale_smaller(int *sc1, int *sc2) > +{ > + if (sc1[0] != sc2[0]) > + return sc1[0] < sc2[0]; > + > + /* If integer parts are equal, fixp parts */ > + return sc1[1] < sc2[1]; > +} > + > +/* > + * Do a single table listing all the unique scales that any combination of > + * supported gains and times can provide. > + */ > +static int do_combined_scaletable(struct iio_gts *gts, > + size_t all_scales_tbl_bytes) > +{ > + int t_idx, i, new_idx; > + int **scales = gts->per_time_avail_scale_tables; > + int *all_scales = kcalloc(gts->num_itime, all_scales_tbl_bytes, > + GFP_KERNEL); > + > + if (!all_scales) > + return -ENOMEM; > + /* > + * Create table containing all of the supported scales by looping > + * through all of the per-time scales and copying the unique scales > + * into one sorted table. > + * > + * We assume all the gains for same integration time were unique. > + * It is likely the first time table had greatest time multiplier as > + * the times are in the order of preference and greater times are > + * usually preferred. Hence we start from the last table which is likely > + * to have the smallest total gains. > + */ > + t_idx = gts->num_itime - 1; > + memcpy(all_scales, scales[t_idx], all_scales_tbl_bytes); > + new_idx = gts->num_hwgain * 2; > + > + while (t_idx-- > 0) { > + for (i = 0; i < gts->num_hwgain ; i++) { > + int *candidate = &scales[t_idx][i * 2]; > + int chk; > + > + if (scale_smaller(candidate, &all_scales[new_idx - 2])) { > + all_scales[new_idx] = candidate[0]; > + all_scales[new_idx + 1] = candidate[1]; > + new_idx += 2; > + > + continue; > + } > + for (chk = 0; chk < new_idx; chk += 2) > + if (!scale_smaller(candidate, &all_scales[chk])) > + break; > + > + if (scale_eq(candidate, &all_scales[chk])) > + continue; > + > + memmove(&all_scales[chk + 2], &all_scales[chk], > + (new_idx - chk) * sizeof(int)); > + all_scales[chk] = candidate[0]; > + all_scales[chk + 1] = candidate[1]; > + new_idx += 2; > + } > + } > + > + gts->num_avail_all_scales = new_idx / 2; > + gts->avail_all_scales_table = all_scales; > + > + return 0; > +} > + > +static void iio_gts_free_int_table_array(int **arr, int num_tables) > +{ > + int i; > + > + for (i = 0; i < num_tables; i++) > + kfree(arr[i]); > + > + kfree(arr); > +} > + > +static int iio_gts_alloc_int_table_array(int ***arr, int num_tables, int num_table_items) > +{ > + int i, **tmp; > + > + tmp = kcalloc(num_tables, sizeof(**arr), GFP_KERNEL); > + if (!tmp) > + return -ENOMEM; > + > + for (i = 0; i < num_tables; i++) { > + tmp[i] = kcalloc(num_table_items, sizeof(int), GFP_KERNEL); > + if (!tmp[i]) > + goto err_free; > + } > + > + *arr = tmp; > + > + return 0; > +err_free: > + iio_gts_free_int_table_array(tmp, i); > + > + return -ENOMEM; > +} > + > static int iio_gts_gain_cmp(const void *a, const void *b) > { > return *(int *)a - *(int *)b; > } > > -static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) > +static int fill_and_sort_scaletables(struct iio_gts *gts, int **gains, int **scales) > { > - int i, j, new_idx, time_idx, ret = 0; > - int *all_gains; > - size_t gain_bytes; > + int i, j, ret; > > for (i = 0; i < gts->num_itime; i++) { > /* > @@ -189,71 +296,69 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) > } > } > > - gain_bytes = array_size(gts->num_hwgain, sizeof(int)); > - all_gains = kcalloc(gts->num_itime, gain_bytes, GFP_KERNEL); > - if (!all_gains) > - return -ENOMEM; > + return 0; > +} > + > +static void compute_per_time_gains(struct iio_gts *gts, int **gains) > +{ > + int i, j; > + > + for (i = 0; i < gts->num_itime; i++) { > + for (j = 0; j < gts->num_hwgain; j++) > + gains[i][j] = gts->hwgain_table[j].gain * > + gts->itime_table[i].mul; > + } > +} > + > +static int compute_per_time_tables(struct iio_gts *gts, int **scales) > +{ > + int **per_time_gains; > + int ret; > > /* > - * We assume all the gains for same integration time were unique. > - * It is likely the first time table had greatest time multiplier as > - * the times are in the order of preference and greater times are > - * usually preferred. Hence we start from the last table which is likely > - * to have the smallest total gains. > + * Create a temporary array of the 'total gains' for each integration > + * time. > */ > - time_idx = gts->num_itime - 1; > - memcpy(all_gains, gains[time_idx], gain_bytes); > - new_idx = gts->num_hwgain; > + ret = iio_gts_alloc_int_table_array(&per_time_gains, gts->num_itime, > + gts->num_hwgain); > + if (ret) > + return ret; > > - while (time_idx-- > 0) { > - for (j = 0; j < gts->num_hwgain; j++) { > - int candidate = gains[time_idx][j]; > - int chk; > + compute_per_time_gains(gts, per_time_gains); > > - if (candidate > all_gains[new_idx - 1]) { > - all_gains[new_idx] = candidate; > - new_idx++; > + /* Convert the gains to scales and populate the scale tables */ > + ret = fill_and_sort_scaletables(gts, per_time_gains, scales); > > - continue; > - } > - for (chk = 0; chk < new_idx; chk++) > - if (candidate <= all_gains[chk]) > - break; > + iio_gts_free_int_table_array(per_time_gains, gts->num_itime); > > - if (candidate == all_gains[chk]) > - continue; > + return ret; > +} > > - memmove(&all_gains[chk + 1], &all_gains[chk], > - (new_idx - chk) * sizeof(int)); > - all_gains[chk] = candidate; > - new_idx++; > - } > - } > +/* > + * Create a table of supported scales for each supported integration time. > + * This can be used as available_scales by drivers which don't allow scale > + * setting to change the integration time to display correct set of scales > + * depending on the used integration time. > + */ > +static int **create_per_time_scales(struct iio_gts *gts) > +{ > + int **per_time_scales, ret; > > - gts->avail_all_scales_table = kcalloc(new_idx, 2 * sizeof(int), > - GFP_KERNEL); > - if (!gts->avail_all_scales_table) { > - ret = -ENOMEM; > - goto free_out; > - } > - gts->num_avail_all_scales = new_idx; > + ret = iio_gts_alloc_int_table_array(&per_time_scales, gts->num_itime, > + gts->num_hwgain * 2); > + if (ret) > + return ERR_PTR(ret); > > - for (i = 0; i < gts->num_avail_all_scales; i++) { > - ret = iio_gts_total_gain_to_scale(gts, all_gains[i], > - >s->avail_all_scales_table[i * 2], > - >s->avail_all_scales_table[i * 2 + 1]); > + ret = compute_per_time_tables(gts, per_time_scales); > + if (ret) > + goto err_out; > > - if (ret) { > - kfree(gts->avail_all_scales_table); > - gts->num_avail_all_scales = 0; > - goto free_out; > - } > - } > + return per_time_scales; > > -free_out: > - kfree(all_gains); > +err_out: > + iio_gts_free_int_table_array(per_time_scales, gts->num_itime); > > - return ret; > + return ERR_PTR(ret); > } > > /** > @@ -275,55 +380,26 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) > */ > static int iio_gts_build_avail_scale_table(struct iio_gts *gts) > { > - int **per_time_gains, **per_time_scales, i, j, ret = -ENOMEM; > - > - per_time_gains = kcalloc(gts->num_itime, sizeof(*per_time_gains), GFP_KERNEL); > - if (!per_time_gains) > - return ret; > - > - per_time_scales = kcalloc(gts->num_itime, sizeof(*per_time_scales), GFP_KERNEL); > - if (!per_time_scales) > - goto free_gains; > + int ret, all_scales_tbl_bytes; > + int **per_time_scales; > > - for (i = 0; i < gts->num_itime; i++) { > - per_time_scales[i] = kcalloc(gts->num_hwgain, 2 * sizeof(int), > - GFP_KERNEL); > - if (!per_time_scales[i]) > - goto err_free_out; > - > - per_time_gains[i] = kcalloc(gts->num_hwgain, sizeof(int), > - GFP_KERNEL); > - if (!per_time_gains[i]) { > - kfree(per_time_scales[i]); > - goto err_free_out; > - } > - > - for (j = 0; j < gts->num_hwgain; j++) > - per_time_gains[i][j] = gts->hwgain_table[j].gain * > - gts->itime_table[i].mul; > - } > + if (unlikely(check_mul_overflow(gts->num_hwgain, 2 * sizeof(int), > + &all_scales_tbl_bytes))) > + return -EOVERFLOW; > > - ret = gain_to_scaletables(gts, per_time_gains, per_time_scales); > - if (ret) > - goto err_free_out; > + per_time_scales = create_per_time_scales(gts); > + if (IS_ERR(per_time_scales)) > + return PTR_ERR(per_time_scales); > > - for (i = 0; i < gts->num_itime; i++) > - kfree(per_time_gains[i]); > - kfree(per_time_gains); > gts->per_time_avail_scale_tables = per_time_scales; > > - return 0; > - > -err_free_out: > - for (i--; i >= 0; i--) { > - kfree(per_time_scales[i]); > - kfree(per_time_gains[i]); > + ret = do_combined_scaletable(gts, all_scales_tbl_bytes); > + if (ret) { > + iio_gts_free_int_table_array(per_time_scales, gts->num_itime); > + return ret; > } > - kfree(per_time_scales); > -free_gains: > - kfree(per_time_gains); > > - return ret; > + return 0; > } > > static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times,
diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c index 291c0fc332c9..65697be58a10 100644 --- a/drivers/iio/industrialio-gts-helper.c +++ b/drivers/iio/industrialio-gts-helper.c @@ -160,16 +160,123 @@ static void iio_gts_purge_avail_scale_table(struct iio_gts *gts) gts->num_avail_all_scales = 0; } +static int scale_eq(int *sc1, int *sc2) +{ + return sc1[0] == sc2[0] && sc1[1] == sc2[1]; +} + +static int scale_smaller(int *sc1, int *sc2) +{ + if (sc1[0] != sc2[0]) + return sc1[0] < sc2[0]; + + /* If integer parts are equal, fixp parts */ + return sc1[1] < sc2[1]; +} + +/* + * Do a single table listing all the unique scales that any combination of + * supported gains and times can provide. + */ +static int do_combined_scaletable(struct iio_gts *gts, + size_t all_scales_tbl_bytes) +{ + int t_idx, i, new_idx; + int **scales = gts->per_time_avail_scale_tables; + int *all_scales = kcalloc(gts->num_itime, all_scales_tbl_bytes, + GFP_KERNEL); + + if (!all_scales) + return -ENOMEM; + /* + * Create table containing all of the supported scales by looping + * through all of the per-time scales and copying the unique scales + * into one sorted table. + * + * We assume all the gains for same integration time were unique. + * It is likely the first time table had greatest time multiplier as + * the times are in the order of preference and greater times are + * usually preferred. Hence we start from the last table which is likely + * to have the smallest total gains. + */ + t_idx = gts->num_itime - 1; + memcpy(all_scales, scales[t_idx], all_scales_tbl_bytes); + new_idx = gts->num_hwgain * 2; + + while (t_idx-- > 0) { + for (i = 0; i < gts->num_hwgain ; i++) { + int *candidate = &scales[t_idx][i * 2]; + int chk; + + if (scale_smaller(candidate, &all_scales[new_idx - 2])) { + all_scales[new_idx] = candidate[0]; + all_scales[new_idx + 1] = candidate[1]; + new_idx += 2; + + continue; + } + for (chk = 0; chk < new_idx; chk += 2) + if (!scale_smaller(candidate, &all_scales[chk])) + break; + + if (scale_eq(candidate, &all_scales[chk])) + continue; + + memmove(&all_scales[chk + 2], &all_scales[chk], + (new_idx - chk) * sizeof(int)); + all_scales[chk] = candidate[0]; + all_scales[chk + 1] = candidate[1]; + new_idx += 2; + } + } + + gts->num_avail_all_scales = new_idx / 2; + gts->avail_all_scales_table = all_scales; + + return 0; +} + +static void iio_gts_free_int_table_array(int **arr, int num_tables) +{ + int i; + + for (i = 0; i < num_tables; i++) + kfree(arr[i]); + + kfree(arr); +} + +static int iio_gts_alloc_int_table_array(int ***arr, int num_tables, int num_table_items) +{ + int i, **tmp; + + tmp = kcalloc(num_tables, sizeof(**arr), GFP_KERNEL); + if (!tmp) + return -ENOMEM; + + for (i = 0; i < num_tables; i++) { + tmp[i] = kcalloc(num_table_items, sizeof(int), GFP_KERNEL); + if (!tmp[i]) + goto err_free; + } + + *arr = tmp; + + return 0; +err_free: + iio_gts_free_int_table_array(tmp, i); + + return -ENOMEM; +} + static int iio_gts_gain_cmp(const void *a, const void *b) { return *(int *)a - *(int *)b; } -static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) +static int fill_and_sort_scaletables(struct iio_gts *gts, int **gains, int **scales) { - int i, j, new_idx, time_idx, ret = 0; - int *all_gains; - size_t gain_bytes; + int i, j, ret; for (i = 0; i < gts->num_itime; i++) { /* @@ -189,71 +296,69 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) } } - gain_bytes = array_size(gts->num_hwgain, sizeof(int)); - all_gains = kcalloc(gts->num_itime, gain_bytes, GFP_KERNEL); - if (!all_gains) - return -ENOMEM; + return 0; +} + +static void compute_per_time_gains(struct iio_gts *gts, int **gains) +{ + int i, j; + + for (i = 0; i < gts->num_itime; i++) { + for (j = 0; j < gts->num_hwgain; j++) + gains[i][j] = gts->hwgain_table[j].gain * + gts->itime_table[i].mul; + } +} + +static int compute_per_time_tables(struct iio_gts *gts, int **scales) +{ + int **per_time_gains; + int ret; /* - * We assume all the gains for same integration time were unique. - * It is likely the first time table had greatest time multiplier as - * the times are in the order of preference and greater times are - * usually preferred. Hence we start from the last table which is likely - * to have the smallest total gains. + * Create a temporary array of the 'total gains' for each integration + * time. */ - time_idx = gts->num_itime - 1; - memcpy(all_gains, gains[time_idx], gain_bytes); - new_idx = gts->num_hwgain; + ret = iio_gts_alloc_int_table_array(&per_time_gains, gts->num_itime, + gts->num_hwgain); + if (ret) + return ret; - while (time_idx-- > 0) { - for (j = 0; j < gts->num_hwgain; j++) { - int candidate = gains[time_idx][j]; - int chk; + compute_per_time_gains(gts, per_time_gains); - if (candidate > all_gains[new_idx - 1]) { - all_gains[new_idx] = candidate; - new_idx++; + /* Convert the gains to scales and populate the scale tables */ + ret = fill_and_sort_scaletables(gts, per_time_gains, scales); - continue; - } - for (chk = 0; chk < new_idx; chk++) - if (candidate <= all_gains[chk]) - break; + iio_gts_free_int_table_array(per_time_gains, gts->num_itime); - if (candidate == all_gains[chk]) - continue; + return ret; +} - memmove(&all_gains[chk + 1], &all_gains[chk], - (new_idx - chk) * sizeof(int)); - all_gains[chk] = candidate; - new_idx++; - } - } +/* + * Create a table of supported scales for each supported integration time. + * This can be used as available_scales by drivers which don't allow scale + * setting to change the integration time to display correct set of scales + * depending on the used integration time. + */ +static int **create_per_time_scales(struct iio_gts *gts) +{ + int **per_time_scales, ret; - gts->avail_all_scales_table = kcalloc(new_idx, 2 * sizeof(int), - GFP_KERNEL); - if (!gts->avail_all_scales_table) { - ret = -ENOMEM; - goto free_out; - } - gts->num_avail_all_scales = new_idx; + ret = iio_gts_alloc_int_table_array(&per_time_scales, gts->num_itime, + gts->num_hwgain * 2); + if (ret) + return ERR_PTR(ret); - for (i = 0; i < gts->num_avail_all_scales; i++) { - ret = iio_gts_total_gain_to_scale(gts, all_gains[i], - >s->avail_all_scales_table[i * 2], - >s->avail_all_scales_table[i * 2 + 1]); + ret = compute_per_time_tables(gts, per_time_scales); + if (ret) + goto err_out; - if (ret) { - kfree(gts->avail_all_scales_table); - gts->num_avail_all_scales = 0; - goto free_out; - } - } + return per_time_scales; -free_out: - kfree(all_gains); +err_out: + iio_gts_free_int_table_array(per_time_scales, gts->num_itime); - return ret; + return ERR_PTR(ret); } /** @@ -275,55 +380,26 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) */ static int iio_gts_build_avail_scale_table(struct iio_gts *gts) { - int **per_time_gains, **per_time_scales, i, j, ret = -ENOMEM; - - per_time_gains = kcalloc(gts->num_itime, sizeof(*per_time_gains), GFP_KERNEL); - if (!per_time_gains) - return ret; - - per_time_scales = kcalloc(gts->num_itime, sizeof(*per_time_scales), GFP_KERNEL); - if (!per_time_scales) - goto free_gains; + int ret, all_scales_tbl_bytes; + int **per_time_scales; - for (i = 0; i < gts->num_itime; i++) { - per_time_scales[i] = kcalloc(gts->num_hwgain, 2 * sizeof(int), - GFP_KERNEL); - if (!per_time_scales[i]) - goto err_free_out; - - per_time_gains[i] = kcalloc(gts->num_hwgain, sizeof(int), - GFP_KERNEL); - if (!per_time_gains[i]) { - kfree(per_time_scales[i]); - goto err_free_out; - } - - for (j = 0; j < gts->num_hwgain; j++) - per_time_gains[i][j] = gts->hwgain_table[j].gain * - gts->itime_table[i].mul; - } + if (unlikely(check_mul_overflow(gts->num_hwgain, 2 * sizeof(int), + &all_scales_tbl_bytes))) + return -EOVERFLOW; - ret = gain_to_scaletables(gts, per_time_gains, per_time_scales); - if (ret) - goto err_free_out; + per_time_scales = create_per_time_scales(gts); + if (IS_ERR(per_time_scales)) + return PTR_ERR(per_time_scales); - for (i = 0; i < gts->num_itime; i++) - kfree(per_time_gains[i]); - kfree(per_time_gains); gts->per_time_avail_scale_tables = per_time_scales; - return 0; - -err_free_out: - for (i--; i >= 0; i--) { - kfree(per_time_scales[i]); - kfree(per_time_gains[i]); + ret = do_combined_scaletable(gts, all_scales_tbl_bytes); + if (ret) { + iio_gts_free_int_table_array(per_time_scales, gts->num_itime); + return ret; } - kfree(per_time_scales); -free_gains: - kfree(per_time_gains); - return ret; + return 0; } static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times,
Make available scale building more clear. This hurts the performance quite a bit by looping throgh the scales many times instead of doing everything in one loop. It however simplifies logic by: - decoupling the gain and scale allocations & computations - keeping the temporary 'per_time_gains' table inside the per_time_scales computation function. - separating building the 'all scales' table in own function and doing it based on the already computed per-time scales. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- Revision history: v2: - Add a few comments - Use more descriptive variable name This is still only tested using the kunit tests. All further testing is still highly appreciated! --- drivers/iio/industrialio-gts-helper.c | 272 ++++++++++++++++---------- 1 file changed, 174 insertions(+), 98 deletions(-)