[Sound-open-firmware] [PATCH 1/5] IPC: Add new enum commands and an index into control data struct
This patch adds to struct sof_ipc_ctrl_data three new commands SOF_CTRL_CMD_ENUM, SOF_CTRL_CMD_SWITCH, and SOF_CTRL_CMD_BINARY. In addition an index parameter is added to address various features or registers. Commands SOF_CTRL_CMD_ROUTE, SOF_CTRL_CMD_SRC, SOF_CTRL_CMD_LOOPBACK, SOF_CTRL_CMD_EQ_SWITCH, SOF_CTRL_CMD_EQ_CONFIG, SOF_CTRL_CMD_MUTE, and SOF_CTRL_CMD_UNMUTE are removed. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/include/uapi/ipc.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index 281e77a..5658b9a 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -437,25 +437,23 @@ struct sof_ipc_stream_posn { /* control data type and direction */ enum sof_ipc_ctrl_type { + /* per channel data - uses struct sof_ipc_ctrl_value_chan */ SOF_CTRL_TYPE_VALUE_CHAN_GET = 0, SOF_CTRL_TYPE_VALUE_CHAN_SET, + /* per component data - uses struct sof_ipc_ctrl_value_comp */ SOF_CTRL_TYPE_VALUE_COMP_GET, SOF_CTRL_TYPE_VALUE_COMP_SET, + /* bespoke data - struct struct sof_abi_hdr */ SOF_CTRL_TYPE_DATA_GET, SOF_CTRL_TYPE_DATA_SET, }; /* control command type */ enum sof_ipc_ctrl_cmd { - SOF_CTRL_CMD_VOLUME = 0, - SOF_CTRL_CMD_ROUTE, - SOF_CTRL_CMD_SRC, - SOF_CTRL_CMD_LOOPBACK, - SOF_CTRL_CMD_EQ_SWITCH, - SOF_CTRL_CMD_EQ_CONFIG, - /* Mute is similar to volume, but maps better onto ALSA switch controls */ - SOF_CTRL_CMD_MUTE, - SOF_CTRL_CMD_UNMUTE, + SOF_CTRL_CMD_VOLUME = 0, /* maps to ALSA volume style controls */ + SOF_CTRL_CMD_ENUM, /* maps to ALSA enum style controls */ + SOF_CTRL_CMD_SWITCH, /* maps to ALSA switch style controls */ + SOF_CTRL_CMD_BINARY, /* maps to ALSA binary style controls */ }; /* generic channel mapped value data */ @@ -481,6 +479,7 @@ struct sof_ipc_ctrl_data { /* control access and data type */ enum sof_ipc_ctrl_type type; enum sof_ipc_ctrl_cmd cmd; + uint32_t index; /* control index for comps > 1 control */ /* control data - can either be appended or DMAed from host */ struct sof_ipc_host_buffer buffer; @@ -497,7 +496,6 @@ struct sof_ipc_ctrl_data { }; } __attribute__((packed)); - /* * Component */ -- 2.11.0
Due to previous patch the volume mute is updated to be controlled with SOF_CTRL_CMD_SWITCH that maps to ALSA switch style control. Mute for a channel is set with an (unsigned) value larger than zero. Zero value unmutes the channel. In both mute and volume set code the if statement to compare component channel map is changed to compare to chanv[j].channel instead of used chanv[j].value that looks incorrect. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/audio/volume.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/audio/volume.c b/src/audio/volume.c index 137be3c..28d3225 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -449,36 +449,35 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c switch (cdata->cmd) { case SOF_CTRL_CMD_VOLUME: - + trace_volume("vst"); for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) + tracev_value(cdata->chanv[j].channel); + tracev_value(cdata->chanv[j].value); + if (cdata->chanv[j].channel == cd->chan[i]) volume_set_chan(dev, i, cdata->chanv[j].value); } } - work_schedule_default(&cd->volwork, VOL_RAMP_US); break; - case SOF_CTRL_CMD_MUTE: + case SOF_CTRL_CMD_SWITCH: + trace_volume("mst"); for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) - volume_set_chan_mute(dev, i); + tracev_value(cdata->chanv[j].channel); + tracev_value(cdata->chanv[j].value); + if (cdata->chanv[j].channel == cd->chan[i]) { + if (cdata->chanv[j].value > 0) + volume_set_chan_mute(dev, i); + else + volume_set_chan_unmute(dev, i); + } } } work_schedule_default(&cd->volwork, VOL_RAMP_US); break; - case SOF_CTRL_CMD_UNMUTE: - for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { - for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) - volume_set_chan_unmute(dev, i); - } - } - work_schedule_default(&cd->volwork, VOL_RAMP_US); - break; default: trace_volume_error("gs1"); return -EINVAL; -- 2.11.0
On 10/20/17 12:15 PM, Seppo Ingalsuo wrote:
Due to previous patch the volume mute is updated to be controlled with SOF_CTRL_CMD_SWITCH that maps to ALSA switch style control. Mute for a channel is set with an (unsigned) value larger than zero. Zero value unmutes the channel.
Mute and switch are not synonyms - the results are inverted. ALSA switches set to 1 means enable audio (unmute), 0 means mute. we've had this confusion in the past with inverted logic for the SST driver. We should also treat switch as a boolean, it's zero or one so we can test with switch or !switch.
In both mute and volume set code the if statement to compare component channel map is changed to compare to chanv[j].channel instead of used chanv[j].value that looks incorrect.
Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/audio/volume.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/audio/volume.c b/src/audio/volume.c index 137be3c..28d3225 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -449,36 +449,35 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c
switch (cdata->cmd) { case SOF_CTRL_CMD_VOLUME: - + trace_volume("vst"); for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) + tracev_value(cdata->chanv[j].channel); + tracev_value(cdata->chanv[j].value); + if (cdata->chanv[j].channel == cd->chan[i]) volume_set_chan(dev, i, cdata->chanv[j].value); } } - work_schedule_default(&cd->volwork, VOL_RAMP_US); break; - case SOF_CTRL_CMD_MUTE:
+ case SOF_CTRL_CMD_SWITCH: + trace_volume("mst"); for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) - volume_set_chan_mute(dev, i); + tracev_value(cdata->chanv[j].channel); + tracev_value(cdata->chanv[j].value); + if (cdata->chanv[j].channel == cd->chan[i]) { + if (cdata->chanv[j].value > 0) + volume_set_chan_mute(dev, i); + else + volume_set_chan_unmute(dev, i); + } } } work_schedule_default(&cd->volwork, VOL_RAMP_US); break; - case SOF_CTRL_CMD_UNMUTE:
- for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { - for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) - volume_set_chan_unmute(dev, i); - } - } - work_schedule_default(&cd->volwork, VOL_RAMP_US); - break; default: trace_volume_error("gs1"); return -EINVAL;
On 21.10.2017 00:38, Pierre-Louis Bossart wrote:
On 10/20/17 12:15 PM, Seppo Ingalsuo wrote:
Due to previous patch the volume mute is updated to be controlled with SOF_CTRL_CMD_SWITCH that maps to ALSA switch style control. Mute for a channel is set with an (unsigned) value larger than zero. Zero value unmutes the channel.
Mute and switch are not synonyms - the results are inverted. ALSA switches set to 1 means enable audio (unmute), 0 means mute. we've had this confusion in the past with inverted logic for the SST driver. We should also treat switch as a boolean, it's zero or one so we can test with switch or !switch.
Oops, I'll correct this. The same issue is also in patch 4 and 5 for EQs. Thanks, Seppo
In both mute and volume set code the if statement to compare component channel map is changed to compare to chanv[j].channel instead of used chanv[j].value that looks incorrect.
Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/audio/volume.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/audio/volume.c b/src/audio/volume.c index 137be3c..28d3225 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -449,36 +449,35 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c switch (cdata->cmd) { case SOF_CTRL_CMD_VOLUME: - + trace_volume("vst"); for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) + tracev_value(cdata->chanv[j].channel); + tracev_value(cdata->chanv[j].value); + if (cdata->chanv[j].channel == cd->chan[i]) volume_set_chan(dev, i, cdata->chanv[j].value); } } - work_schedule_default(&cd->volwork, VOL_RAMP_US); break; - case SOF_CTRL_CMD_MUTE: + case SOF_CTRL_CMD_SWITCH: + trace_volume("mst"); for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) - volume_set_chan_mute(dev, i); + tracev_value(cdata->chanv[j].channel); + tracev_value(cdata->chanv[j].value); + if (cdata->chanv[j].channel == cd->chan[i]) { + if (cdata->chanv[j].value > 0) + volume_set_chan_mute(dev, i); + else + volume_set_chan_unmute(dev, i); + } } } work_schedule_default(&cd->volwork, VOL_RAMP_US); break; - case SOF_CTRL_CMD_UNMUTE: - for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { - for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) - volume_set_chan_unmute(dev, i); - } - } - work_schedule_default(&cd->volwork, VOL_RAMP_US); - break; default: trace_volume_error("gs1"); return -EINVAL;
_______________________________________________ Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
The control interface is fixed to follow the component ABI. Separate tone generator instances are used to feed individual channels. It allows simultaneous testing of all channels by use of different tone frequencies. To simplify features there is no mixer so e.g. DTMF telephony UI sounds generation would need a channel mixer component after a two channel tone output in the pipeline. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/audio/tone.c | 307 ++++++++++++++++++++++++------------- src/{audio => include/uapi}/tone.h | 44 ++---- 2 files changed, 221 insertions(+), 130 deletions(-) rename src/{audio => include/uapi}/tone.h (58%) diff --git a/src/audio/tone.c b/src/audio/tone.c index 1fde455..d565c17 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -45,7 +45,7 @@ #include <reef/audio/pipeline.h> #include <reef/math/trig.h> #include <uapi/ipc.h> -#include "tone.h" +#include <uapi/tone.h> #ifdef MODULE_TEST #include <stdio.h> @@ -55,14 +55,17 @@ #define tracev_tone(__e) tracev_event(TRACE_CLASS_TONE, __e) #define trace_tone_error(__e) trace_error(TRACE_CLASS_TONE, __e) -#define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ -#define TONE_AMPLITUDE_DEFAULT MINUS_60DB_Q1_31 /* -60 dB */ -#define TONE_FREQUENCY_DEFAULT TONE_FREQ(997.0) /* 997 Hz */ +/* Convert float frequency in Hz to Q16.16 fractional format */ +#define TONE_FREQ(f) Q_CONVERT_FLOAT(f, 16) -static int32_t tonegen(struct tone_state *sg); -static void tonegen_control(struct tone_state *sg); -static void tonegen_update_f(struct tone_state *sg, int32_t f); +/* Convert float gain to Q1.31 fractional format */ +#define TONE_GAIN(v) Q_CONVERT_FLOAT(v, 31) +/* Set default tone amplitude and frequency */ +#define TONE_AMPLITUDE_DEFAULT TONE_GAIN(0.5) /* -6 dB */ +#define TONE_FREQUENCY_DEFAULT TONE_FREQ(82.41) /* E2 note */ + +#define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ /* 2*pi/Fs lookup tables in Q1.31 for each Fs */ static const int32_t tone_fs_list[TONE_NUM_FS] = { @@ -76,70 +79,78 @@ static const int32_t tone_pi2_div_fs[TONE_NUM_FS] = { /* tone component private data */ -/* TODO: Remove *source when internal endpoint is possible */ +struct tone_state { + int mute; + int32_t a; /* Current amplitude Q1.31 */ + int32_t a_target; /* Target amplitude Q1.31 */ + int32_t ampl_coef; /* Amplitude multiplier Q2.30 */ + int32_t c; /* Coefficient 2*pi/Fs Q1.31 */ + int32_t f; /* Frequency Q16.16 */ + int32_t freq_coef; /* Frequency multiplier Q2.30 */ + int32_t fs; /* Sample rate in Hertz Q32.0 */ + int32_t ramp_step; /* Amplitude ramp step Q1.31 */ + int32_t w; /* Angle radians Q4.28 */ + int32_t w_step; /* Angle step Q4.28 */ + uint32_t block_count; + uint32_t repeat_count; + uint32_t repeats; /* Number of repeats for tone (sweep steps) */ + uint32_t sample_count; + uint32_t samples_in_block; /* Samples in 125 us block */ + uint32_t tone_length; /* Active length in 125 us blocks */ + uint32_t tone_period; /* Active + idle time in 125 us blocks */ +}; + struct comp_data { uint32_t period_bytes; uint32_t channels; uint32_t frame_bytes; uint32_t rate; - struct tone_state sg; + struct tone_state sg[PLATFORM_MAX_CHANNELS]; void (*tone_func)(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames); + uint32_t frames); }; +static int32_t tonegen(struct tone_state *sg); +static void tonegen_control(struct tone_state *sg); +static void tonegen_update_f(struct tone_state *sg, int32_t f); + /* * Tone generator algorithm code */ +static inline void tone_circ_inc_wrap(int32_t **ptr, int32_t *end, size_t size) +{ + if (*ptr >= end) + *ptr = (int32_t *) ((size_t) * ptr - size); +} + static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames) + uint32_t frames) { struct comp_data *cd = comp_get_drvdata(dev); - int32_t sine_sample; int32_t *dest = (int32_t*) sink->w_ptr; int i; int n; int n_wrap_dest; + int n_min; int nch = cd->channels; n = frames * nch; while (n > 0) { n_wrap_dest = (int32_t *) sink->end_addr - dest; - if (n < n_wrap_dest) { - /* No circular wrap need */ - while (n > 0) { - /* Update period count for sweeps, etc. */ - tonegen_control(&cd->sg); - /* Calculate mono sine wave sample and then - * duplicate to channels. - */ - sine_sample = tonegen(&cd->sg); - n -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } + n_min = (n < n_wrap_dest) ? n : n_wrap_dest; + /* Process until wrap or completed n */ + while (n_min > 0) { + n -= nch; + n_min -= nch; + for (i = 0; i < nch; i++) { + tonegen_control(&cd->sg[i]); + *dest = tonegen(&cd->sg[i]); + dest++; } - } else { - /* Process until wrap */ - while (n_wrap_dest > 0) { - tonegen_control(&cd->sg); - sine_sample = tonegen(&cd->sg); - n -= nch; - n_wrap_dest -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } - } - /* No need to check if past end_addr, - * it is so just subtract buffer size. - */ - dest = (int32_t *) ((size_t) dest - - sink->ipc_buffer.size); } + tone_circ_inc_wrap(&dest, sink->end_addr, sink->size); } - sink->w_ptr = dest; } static int32_t tonegen(struct tone_state *sg) @@ -226,39 +237,46 @@ static void tonegen_control(struct tone_state *sg) } } +/* Set sine amplitude */ static inline void tonegen_set_a(struct tone_state *sg, int32_t a) { sg->a_target = a; } -static inline void tonegen_set_f(struct tone_state *sg, int32_t f) +/* Repeated number of beeps */ +static void tonegen_set_repeats(struct tone_state *sg, uint32_t r) { - sg->f = f; + sg->repeats = r; } -#if 0 -/* Tone sweep parameters description: - * fc - Multiplication factor for frequency as Q2.30 for logarithmic change - * ac - Multiplication factor for amplitude as Q2.30 for logarithmic change - * l - Tone length in samples, this is the active length of tone - * p - Tone period in samples, this is the length including the pause after beep - * r - Repeated number of beeps +/* The next functions support zero as shortcut for defaults to get + * make a nicer API without need to remember the neutral steady + * non-swept tone settings. */ -static void tonegen_set_sweep(struct tone_state *sg, int32_t fc, int32_t ac, - uint32_t l, uint32_t p, uint32_t r) +/* Multiplication factor for frequency as Q2.30 for logarithmic change */ +static void tonegen_set_freq_mult(struct tone_state *sg, int32_t fm) { - sg->repeats = r; + sg->freq_coef = (fm > 0) ? fm : ONE_Q2_30; /* Set freq mult to 1.0 */ +} - /* Zeros as defaults make a nicer API without need to remember - * the neutral settings for sweep and repeat parameters. - */ - sg->freq_coef = (fc > 0) ? fc : ONE_Q2_30; /* Set freq mult to 1.0 */ - sg->ampl_coef = (ac > 0) ? ac : ONE_Q2_30; /* Set ampl mult to 1.0 */ - sg->tone_length = (l > 0) ? l : INT32_MAXVALUE; /* Count rate 125 us */ - sg->tone_period = (p > 0) ? p : INT32_MAXVALUE; /* Count rate 125 us */ +/* Multiplication factor for amplitude as Q2.30 for logarithmic change */ +static void tonegen_set_ampl_mult(struct tone_state *sg, int32_t am) +{ + sg->ampl_coef = (am > 0) ? am : ONE_Q2_30; /* Set ampl mult to 1.0 */ +} + +/* Tone length in samples, this is the active length of tone */ +static void tonegen_set_length(struct tone_state *sg, uint32_t tl) +{ + sg->tone_length = (tl > 0) ? tl : INT32_MAXVALUE; /* Count rate 125 us */ +} + +/* Tone period in samples, this is the length including the pause after beep */ +static void tonegen_set_period(struct tone_state *sg, uint32_t tp) +{ + sg->tone_period = (tp > 0) ? tp : INT32_MAXVALUE; /* Count rate 125 us */ } -#endif /* Tone ramp parameters: * step - Value that is added or subtracted to amplitude. A zero or negative @@ -382,6 +400,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) struct sof_ipc_comp_tone *tone; struct sof_ipc_comp_tone *ipc_tone = (struct sof_ipc_comp_tone *) comp; struct comp_data *cd; + int i; trace_tone("new"); @@ -403,7 +422,8 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) cd->tone_func = tone_s32_default; /* Reset tone generator and set channels volumes to default */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]); dev->state = COMP_STATE_READY; return dev; @@ -441,34 +461,104 @@ static int tone_params(struct comp_dev *dev) return 0; } -static int tone_ctrl_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +static int tone_cmd_set_value(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev); + int j; + uint32_t ch; + uint32_t val; + + if (cdata->cmd == SOF_CTRL_CMD_SWITCH) { + trace_tone("mst"); + for (j = 0; j < cdata->num_elems; j++) { + ch = cdata->chanv[j].channel; + val = cdata->chanv[j].value; + tracev_value(ch); + tracev_value(val); + if (ch >= PLATFORM_MAX_CHANNELS) { + trace_tone_error("che"); + return -EINVAL; + } + if (val > 0) + tonegen_mute(&cd->sg[ch]); + else + tonegen_unmute(&cd->sg[ch]); + + } + } else { + trace_tone_error("ste"); + return -EINVAL; + } + + return 0; +} + +static int tone_cmd_set_data(struct comp_dev *dev, + struct sof_ipc_ctrl_data *cdata) +{ + struct comp_data *cd = comp_get_drvdata(dev); + struct sof_ipc_ctrl_value_comp *compv; + int i; + uint32_t ch; + uint32_t val; trace_tone("tri"); + /* Check version from ABI header */ + if (cdata->data->comp_abi != SOF_TONE_ABI_VERSION) { + trace_tone_error("abi"); + return -EINVAL; + } + switch (cdata->cmd) { - case SOF_CTRL_CMD_MUTE: - trace_tone("TMu"); - tonegen_mute(&cd->sg); - break; - case SOF_CTRL_CMD_UNMUTE: - trace_tone("TUm"); - tonegen_unmute(&cd->sg); - break; -/* TODO: use comp value list array to set this */ -#if 0 - case SOF_CTRL_TYPE_VALUE_COMP_SET: - trace_tone("Tto"); - ct = (struct sof_ipc_comp_tone *) data; - /* Ignore channels while tone implementation is mono */ - tonegen_set_f(&cd->sg, ct->frequency); - tonegen_set_a(&cd->sg, ct->amplitude); - tonegen_set_sweep(&cd->sg, ct->freq_mult, ct->ampl_mult, - ct->length, ct->period, ct->repeats); - tonegen_set_linramp(&cd->sg, ct->ramp_step); + case SOF_CTRL_CMD_ENUM: + trace_tone("ten"); + trace_value(cdata->index); + compv = (struct sof_ipc_ctrl_value_comp *) cdata->data->data; + for (i = 0; i < (int) cdata->num_elems; i++) { + ch = compv[i].index; + val = compv[i].svalue; + tracev_value(ch); + tracev_value(val); + switch (cdata->index) { + case SOF_TONE_IDX_FREQUENCY: + trace_tone("tfr"); + tonegen_update_f(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_AMPLITUDE: + trace_tone("tam"); + tonegen_set_a(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_FREQ_MULT: + trace_tone("tfx"); + tonegen_set_freq_mult(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_AMPL_MULT: + trace_tone("tax"); + tonegen_set_ampl_mult(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_LENGTH: + trace_tone("tle"); + tonegen_set_length(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_PERIOD: + trace_tone("tpe"); + tonegen_set_period(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_REPEATS: + trace_tone("trp"); + tonegen_set_repeats(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_LIN_RAMP_STEP: + trace_tone("trs"); + tonegen_set_linramp(&cd->sg[ch], val); + break; + default: + trace_tone_error("ier"); + return -EINVAL; + } + } break; -#endif default: trace_tone_error("ec1"); return -EINVAL; @@ -489,17 +579,22 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data) if (ret < 0) return ret; - if (cmd == COMP_CMD_SET_VALUE) - ret = tone_ctrl_cmd(dev, cdata); + switch (cmd) { + case COMP_CMD_SET_DATA: + ret = tone_cmd_set_data(dev, cdata); + break; + case COMP_CMD_SET_VALUE: + ret = tone_cmd_set_value(dev, cdata); + break; + } return ret; } /* copy and process stream data from source to sink buffers */ -static int tone_copy(struct comp_dev *dev) +static int tone_copy(struct comp_dev * dev) { struct comp_buffer *sink; - struct comp_buffer *source = NULL; struct comp_data *cd = comp_get_drvdata(dev); trace_comp("cpy"); @@ -513,19 +608,22 @@ static int tone_copy(struct comp_dev *dev) */ if (sink->free >= cd->period_bytes) { /* create tone */ - cd->tone_func(dev, sink, source, dev->frames); - comp_update_buffer_produce(sink, 0); + cd->tone_func(dev, sink, dev->frames); + + /* calc new free and available */ + comp_update_buffer_produce(sink, cd->period_bytes); } return dev->frames; } -static int tone_prepare(struct comp_dev *dev) +static int tone_prepare(struct comp_dev * dev) { struct comp_data *cd = comp_get_drvdata(dev); int32_t f; int32_t a; int ret; + int i; trace_tone("TPp"); @@ -538,30 +636,34 @@ static int tone_prepare(struct comp_dev *dev) tracev_value(cd->channels); tracev_value(cd->rate); - f = tonegen_get_f(&cd->sg); - a = tonegen_get_a(&cd->sg); - if (tonegen_init(&cd->sg, cd->rate, f, a) < 0) { - comp_set_state(dev, COMP_CMD_RESET); - return -EINVAL; + for (i = 0; i < cd->channels; i++) { + f = tonegen_get_f(&cd->sg[i]); + a = tonegen_get_a(&cd->sg[i]); + if (tonegen_init(&cd->sg[i], cd->rate, f, a) < 0) { + comp_set_state(dev, COMP_CMD_RESET); + return -EINVAL; + } } return 0; } -static int tone_preload(struct comp_dev *dev) +static int tone_preload(struct comp_dev * dev) { return tone_copy(dev); } -static int tone_reset(struct comp_dev *dev) +static int tone_reset(struct comp_dev * dev) { struct comp_data *cd = comp_get_drvdata(dev); + int i; trace_tone("TRe"); /* Initialize with the defaults */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]); comp_set_state(dev, COMP_CMD_RESET); @@ -570,8 +672,7 @@ static int tone_reset(struct comp_dev *dev) struct comp_driver comp_tone = { .type = SOF_COMP_TONE, - .ops = - { + .ops = { .new = tone_new, .free = tone_free, .params = tone_params, diff --git a/src/audio/tone.h b/src/include/uapi/tone.h similarity index 58% rename from src/audio/tone.h rename to src/include/uapi/tone.h index fcaf404..b04e7dc 100644 --- a/src/audio/tone.h +++ b/src/include/uapi/tone.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, Intel Corporation + * Copyright (c) 2017, Intel Corporation * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -26,33 +26,23 @@ * POSSIBILITY OF SUCH DAMAGE. * * Author: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> - * Liam Girdwood <liam.r.girdwood@linux.intel.com> - * Keyon Jie <yang.jie@linux.intel.com> */ -/* Convert float frequency in Hz to Q16.16 fractional format */ -#define TONE_FREQ(f) Q_CONVERT_FLOAT(f, 16) +#ifndef TONE_H +#define TONE_H -/* Convert float gain to Q1.31 fractional format */ -#define TONE_GAIN(v) Q_CONVERT_FLOAT(v, 31) +/* Component will reject non-matching configuration. The version number need + * to be incremented with any ABI changes in function fir_cmd(). + */ +#define SOF_TONE_ABI_VERSION 1 + +#define SOF_TONE_IDX_FREQUENCY 0 +#define SOF_TONE_IDX_AMPLITUDE 1 +#define SOF_TONE_IDX_FREQ_MULT 2 +#define SOF_TONE_IDX_AMPL_MULT 3 +#define SOF_TONE_IDX_LENGTH 4 +#define SOF_TONE_IDX_PERIOD 5 +#define SOF_TONE_IDX_REPEATS 6 +#define SOF_TONE_IDX_LIN_RAMP_STEP 7 -struct tone_state { - int mute; - int32_t a; /* Current amplitude Q1.31 */ - int32_t a_target; /* Target amplitude Q1.31 */ - int32_t ampl_coef; /* Amplitude multiplier Q2.30 */ - int32_t c; /* Coefficient 2*pi/Fs Q1.31 */ - int32_t f; /* Frequency Q16.16 */ - int32_t freq_coef; /* Frequency multiplier Q2.30 */ - int32_t fs; /* Sample rate in Hertz Q32.0 */ - int32_t ramp_step; /* Amplitude ramp step Q1.31 */ - int32_t w; /* Angle radians Q4.28 */ - int32_t w_step; /* Angle step Q4.28 */ - uint32_t block_count; - uint32_t repeat_count; - uint32_t repeats; /* Number of repeats for tone (sweep steps) */ - uint32_t sample_count; - uint32_t samples_in_block; /* Samples in 125 us block */ - uint32_t tone_length; /* Active length in 125 us blocks */ - uint32_t tone_period; /* Active + idle time in 125 us blocks */ -}; +#endif /* TONE_ABI_H */ -- 2.11.0
On 10/20/17 12:15 PM, Seppo Ingalsuo wrote:
The control interface is fixed to follow the component ABI. Separate tone generator instances are used to feed individual channels. It allows simultaneous testing of all channels by use of different tone frequencies. To simplify features there is no mixer so e.g. DTMF telephony UI sounds generation would need a channel mixer component after a two channel tone output in the pipeline.
[...]
struct comp_data { uint32_t period_bytes; uint32_t channels; uint32_t frame_bytes; uint32_t rate; - struct tone_state sg; + struct tone_state sg[PLATFORM_MAX_CHANNELS]; void (*tone_func)(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames); + uint32_t frames);
should this patch be split in two, one that deals with control IPC and one with internal implementation changes? it'd make the review easier, no?
};
+static int32_t tonegen(struct tone_state *sg); +static void tonegen_control(struct tone_state *sg); +static void tonegen_update_f(struct tone_state *sg, int32_t f); + /* * Tone generator algorithm code */
+static inline void tone_circ_inc_wrap(int32_t **ptr, int32_t *end, size_t size) +{ + if (*ptr >= end) + *ptr = (int32_t *) ((size_t) * ptr - size);
This is using pointer and integer arithmetic, not great. The rest of the patch is hard to review with tons of changes...
+} + static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames) + uint32_t frames) { struct comp_data *cd = comp_get_drvdata(dev); - int32_t sine_sample; int32_t *dest = (int32_t*) sink->w_ptr; int i; int n; int n_wrap_dest; + int n_min; int nch = cd->channels;
n = frames * nch; while (n > 0) { n_wrap_dest = (int32_t *) sink->end_addr - dest; - if (n < n_wrap_dest) { - /* No circular wrap need */ - while (n > 0) { - /* Update period count for sweeps, etc. */ - tonegen_control(&cd->sg); - /* Calculate mono sine wave sample and then - * duplicate to channels. - */ - sine_sample = tonegen(&cd->sg); - n -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } + n_min = (n < n_wrap_dest) ? n : n_wrap_dest; + /* Process until wrap or completed n */ + while (n_min > 0) { + n -= nch; + n_min -= nch; + for (i = 0; i < nch; i++) { + tonegen_control(&cd->sg[i]); + *dest = tonegen(&cd->sg[i]); + dest++; } - } else { - /* Process until wrap */ - while (n_wrap_dest > 0) { - tonegen_control(&cd->sg); - sine_sample = tonegen(&cd->sg); - n -= nch; - n_wrap_dest -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } - } - /* No need to check if past end_addr, - * it is so just subtract buffer size. - */ - dest = (int32_t *) ((size_t) dest - - sink->ipc_buffer.size); } + tone_circ_inc_wrap(&dest, sink->end_addr, sink->size); } - sink->w_ptr = dest; }
static int32_t tonegen(struct tone_state *sg) @@ -226,39 +237,46 @@ static void tonegen_control(struct tone_state *sg) } }
+/* Set sine amplitude */ static inline void tonegen_set_a(struct tone_state *sg, int32_t a) { sg->a_target = a; }
-static inline void tonegen_set_f(struct tone_state *sg, int32_t f) +/* Repeated number of beeps */ +static void tonegen_set_repeats(struct tone_state *sg, uint32_t r) { - sg->f = f; + sg->repeats = r; }
-#if 0 -/* Tone sweep parameters description: - * fc - Multiplication factor for frequency as Q2.30 for logarithmic change - * ac - Multiplication factor for amplitude as Q2.30 for logarithmic change - * l - Tone length in samples, this is the active length of tone - * p - Tone period in samples, this is the length including the pause after beep - * r - Repeated number of beeps +/* The next functions support zero as shortcut for defaults to get + * make a nicer API without need to remember the neutral steady + * non-swept tone settings. */
-static void tonegen_set_sweep(struct tone_state *sg, int32_t fc, int32_t ac, - uint32_t l, uint32_t p, uint32_t r) +/* Multiplication factor for frequency as Q2.30 for logarithmic change */ +static void tonegen_set_freq_mult(struct tone_state *sg, int32_t fm) { - sg->repeats = r; + sg->freq_coef = (fm > 0) ? fm : ONE_Q2_30; /* Set freq mult to 1.0 */ +}
- /* Zeros as defaults make a nicer API without need to remember - * the neutral settings for sweep and repeat parameters. - */ - sg->freq_coef = (fc > 0) ? fc : ONE_Q2_30; /* Set freq mult to 1.0 */ - sg->ampl_coef = (ac > 0) ? ac : ONE_Q2_30; /* Set ampl mult to 1.0 */ - sg->tone_length = (l > 0) ? l : INT32_MAXVALUE; /* Count rate 125 us */ - sg->tone_period = (p > 0) ? p : INT32_MAXVALUE; /* Count rate 125 us */ +/* Multiplication factor for amplitude as Q2.30 for logarithmic change */ +static void tonegen_set_ampl_mult(struct tone_state *sg, int32_t am) +{ + sg->ampl_coef = (am > 0) ? am : ONE_Q2_30; /* Set ampl mult to 1.0 */ +} + +/* Tone length in samples, this is the active length of tone */ +static void tonegen_set_length(struct tone_state *sg, uint32_t tl) +{ + sg->tone_length = (tl > 0) ? tl : INT32_MAXVALUE; /* Count rate 125 us */ +} + +/* Tone period in samples, this is the length including the pause after beep */ +static void tonegen_set_period(struct tone_state *sg, uint32_t tp) +{ + sg->tone_period = (tp > 0) ? tp : INT32_MAXVALUE; /* Count rate 125 us */ } -#endif
/* Tone ramp parameters: * step - Value that is added or subtracted to amplitude. A zero or negative @@ -382,6 +400,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) struct sof_ipc_comp_tone *tone; struct sof_ipc_comp_tone *ipc_tone = (struct sof_ipc_comp_tone *) comp; struct comp_data *cd; + int i;
trace_tone("new");
@@ -403,7 +422,8 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) cd->tone_func = tone_s32_default;
/* Reset tone generator and set channels volumes to default */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]);
dev->state = COMP_STATE_READY; return dev; @@ -441,34 +461,104 @@ static int tone_params(struct comp_dev *dev) return 0; }
-static int tone_ctrl_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +static int tone_cmd_set_value(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev); + int j; + uint32_t ch; + uint32_t val; + + if (cdata->cmd == SOF_CTRL_CMD_SWITCH) { + trace_tone("mst"); + for (j = 0; j < cdata->num_elems; j++) { + ch = cdata->chanv[j].channel; + val = cdata->chanv[j].value; + tracev_value(ch); + tracev_value(val); + if (ch >= PLATFORM_MAX_CHANNELS) { + trace_tone_error("che"); + return -EINVAL; + } + if (val > 0) + tonegen_mute(&cd->sg[ch]); + else + tonegen_unmute(&cd->sg[ch]); + + } + } else { + trace_tone_error("ste"); + return -EINVAL; + } + + return 0; +} + +static int tone_cmd_set_data(struct comp_dev *dev, + struct sof_ipc_ctrl_data *cdata) +{ + struct comp_data *cd = comp_get_drvdata(dev); + struct sof_ipc_ctrl_value_comp *compv; + int i; + uint32_t ch; + uint32_t val;
trace_tone("tri");
+ /* Check version from ABI header */ + if (cdata->data->comp_abi != SOF_TONE_ABI_VERSION) { + trace_tone_error("abi"); + return -EINVAL; + } + switch (cdata->cmd) { - case SOF_CTRL_CMD_MUTE: - trace_tone("TMu"); - tonegen_mute(&cd->sg); - break; - case SOF_CTRL_CMD_UNMUTE: - trace_tone("TUm"); - tonegen_unmute(&cd->sg); - break; -/* TODO: use comp value list array to set this */ -#if 0 - case SOF_CTRL_TYPE_VALUE_COMP_SET: - trace_tone("Tto"); - ct = (struct sof_ipc_comp_tone *) data; - /* Ignore channels while tone implementation is mono */ - tonegen_set_f(&cd->sg, ct->frequency); - tonegen_set_a(&cd->sg, ct->amplitude); - tonegen_set_sweep(&cd->sg, ct->freq_mult, ct->ampl_mult, - ct->length, ct->period, ct->repeats); - tonegen_set_linramp(&cd->sg, ct->ramp_step); + case SOF_CTRL_CMD_ENUM: + trace_tone("ten"); + trace_value(cdata->index); + compv = (struct sof_ipc_ctrl_value_comp *) cdata->data->data; + for (i = 0; i < (int) cdata->num_elems; i++) { + ch = compv[i].index; + val = compv[i].svalue; + tracev_value(ch); + tracev_value(val); + switch (cdata->index) { + case SOF_TONE_IDX_FREQUENCY: + trace_tone("tfr"); + tonegen_update_f(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_AMPLITUDE: + trace_tone("tam"); + tonegen_set_a(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_FREQ_MULT: + trace_tone("tfx"); + tonegen_set_freq_mult(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_AMPL_MULT: + trace_tone("tax"); + tonegen_set_ampl_mult(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_LENGTH: + trace_tone("tle"); + tonegen_set_length(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_PERIOD: + trace_tone("tpe"); + tonegen_set_period(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_REPEATS: + trace_tone("trp"); + tonegen_set_repeats(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_LIN_RAMP_STEP: + trace_tone("trs"); + tonegen_set_linramp(&cd->sg[ch], val); + break; + default: + trace_tone_error("ier"); + return -EINVAL; + } + } break; -#endif default: trace_tone_error("ec1"); return -EINVAL; @@ -489,17 +579,22 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data) if (ret < 0) return ret;
- if (cmd == COMP_CMD_SET_VALUE) - ret = tone_ctrl_cmd(dev, cdata); + switch (cmd) { + case COMP_CMD_SET_DATA: + ret = tone_cmd_set_data(dev, cdata); + break; + case COMP_CMD_SET_VALUE: + ret = tone_cmd_set_value(dev, cdata); + break; + }
return ret; }
/* copy and process stream data from source to sink buffers */ -static int tone_copy(struct comp_dev *dev) +static int tone_copy(struct comp_dev * dev) { struct comp_buffer *sink; - struct comp_buffer *source = NULL; struct comp_data *cd = comp_get_drvdata(dev);
trace_comp("cpy"); @@ -513,19 +608,22 @@ static int tone_copy(struct comp_dev *dev) */ if (sink->free >= cd->period_bytes) { /* create tone */ - cd->tone_func(dev, sink, source, dev->frames); - comp_update_buffer_produce(sink, 0); + cd->tone_func(dev, sink, dev->frames); + + /* calc new free and available */ + comp_update_buffer_produce(sink, cd->period_bytes); }
return dev->frames; }
-static int tone_prepare(struct comp_dev *dev) +static int tone_prepare(struct comp_dev * dev) { struct comp_data *cd = comp_get_drvdata(dev); int32_t f; int32_t a; int ret; + int i;
trace_tone("TPp");
@@ -538,30 +636,34 @@ static int tone_prepare(struct comp_dev *dev) tracev_value(cd->channels); tracev_value(cd->rate);
- f = tonegen_get_f(&cd->sg); - a = tonegen_get_a(&cd->sg); - if (tonegen_init(&cd->sg, cd->rate, f, a) < 0) { - comp_set_state(dev, COMP_CMD_RESET); - return -EINVAL; + for (i = 0; i < cd->channels; i++) { + f = tonegen_get_f(&cd->sg[i]); + a = tonegen_get_a(&cd->sg[i]); + if (tonegen_init(&cd->sg[i], cd->rate, f, a) < 0) { + comp_set_state(dev, COMP_CMD_RESET); + return -EINVAL; + } }
return 0; }
-static int tone_preload(struct comp_dev *dev) +static int tone_preload(struct comp_dev * dev) { return tone_copy(dev); }
-static int tone_reset(struct comp_dev *dev) +static int tone_reset(struct comp_dev * dev) {
struct comp_data *cd = comp_get_drvdata(dev); + int i;
trace_tone("TRe");
/* Initialize with the defaults */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]);
comp_set_state(dev, COMP_CMD_RESET);
@@ -570,8 +672,7 @@ static int tone_reset(struct comp_dev *dev)
struct comp_driver comp_tone = { .type = SOF_COMP_TONE, - .ops = - { + .ops = { .new = tone_new, .free = tone_free, .params = tone_params, diff --git a/src/audio/tone.h b/src/include/uapi/tone.h similarity index 58% rename from src/audio/tone.h rename to src/include/uapi/tone.h index fcaf404..b04e7dc 100644 --- a/src/audio/tone.h +++ b/src/include/uapi/tone.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, Intel Corporation + * Copyright (c) 2017, Intel Corporation * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -26,33 +26,23 @@ * POSSIBILITY OF SUCH DAMAGE. * * Author: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> - * Liam Girdwood <liam.r.girdwood@linux.intel.com> - * Keyon Jie <yang.jie@linux.intel.com> */
-/* Convert float frequency in Hz to Q16.16 fractional format */ -#define TONE_FREQ(f) Q_CONVERT_FLOAT(f, 16) +#ifndef TONE_H +#define TONE_H
-/* Convert float gain to Q1.31 fractional format */ -#define TONE_GAIN(v) Q_CONVERT_FLOAT(v, 31) +/* Component will reject non-matching configuration. The version number need + * to be incremented with any ABI changes in function fir_cmd(). + */ +#define SOF_TONE_ABI_VERSION 1 + +#define SOF_TONE_IDX_FREQUENCY 0 +#define SOF_TONE_IDX_AMPLITUDE 1 +#define SOF_TONE_IDX_FREQ_MULT 2 +#define SOF_TONE_IDX_AMPL_MULT 3 +#define SOF_TONE_IDX_LENGTH 4 +#define SOF_TONE_IDX_PERIOD 5 +#define SOF_TONE_IDX_REPEATS 6 +#define SOF_TONE_IDX_LIN_RAMP_STEP 7
-struct tone_state { - int mute; - int32_t a; /* Current amplitude Q1.31 */ - int32_t a_target; /* Target amplitude Q1.31 */ - int32_t ampl_coef; /* Amplitude multiplier Q2.30 */ - int32_t c; /* Coefficient 2*pi/Fs Q1.31 */ - int32_t f; /* Frequency Q16.16 */ - int32_t freq_coef; /* Frequency multiplier Q2.30 */ - int32_t fs; /* Sample rate in Hertz Q32.0 */ - int32_t ramp_step; /* Amplitude ramp step Q1.31 */ - int32_t w; /* Angle radians Q4.28 */ - int32_t w_step; /* Angle step Q4.28 */ - uint32_t block_count; - uint32_t repeat_count; - uint32_t repeats; /* Number of repeats for tone (sweep steps) */ - uint32_t sample_count; - uint32_t samples_in_block; /* Samples in 125 us block */ - uint32_t tone_length; /* Active length in 125 us blocks */ - uint32_t tone_period; /* Active + idle time in 125 us blocks */ -}; +#endif /* TONE_ABI_H */
On 21.10.2017 00:44, Pierre-Louis Bossart wrote:
On 10/20/17 12:15 PM, Seppo Ingalsuo wrote:
The control interface is fixed to follow the component ABI. Separate tone generator instances are used to feed individual channels. It allows simultaneous testing of all channels by use of different tone frequencies. To simplify features there is no mixer so e.g. DTMF telephony UI sounds generation would need a channel mixer component after a two channel tone output in the pipeline.
[...]
struct comp_data { uint32_t period_bytes; uint32_t channels; uint32_t frame_bytes; uint32_t rate; - struct tone_state sg; + struct tone_state sg[PLATFORM_MAX_CHANNELS]; void (*tone_func)(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames); + uint32_t frames);
should this patch be split in two, one that deals with control IPC and one with internal implementation changes? it'd make the review easier, no?
Yep, it's a fair request, will do.
}; +static int32_t tonegen(struct tone_state *sg); +static void tonegen_control(struct tone_state *sg); +static void tonegen_update_f(struct tone_state *sg, int32_t f); + /* * Tone generator algorithm code */ +static inline void tone_circ_inc_wrap(int32_t **ptr, int32_t *end, size_t size) +{ + if (*ptr >= end) + *ptr = (int32_t *) ((size_t) * ptr - size);
This is using pointer and integer arithmetic, not great.
I'd prefer in audio processing code to use indexes and arrays, count samples and not size_t bytes, etc. The interface from SOF buffers with pointers to C arrays needs some nice way so good ideas are welcome.
The rest of the patch is hard to review with tons of changes...
The component was with obsolete control interface non-functional for a long time so a lot changes need was accumulated. But I'll try 1st if patch can be split (= possible to compile code between patches) without need to work backwards with the code.
+} + static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames) + uint32_t frames) { struct comp_data *cd = comp_get_drvdata(dev); - int32_t sine_sample; int32_t *dest = (int32_t*) sink->w_ptr; int i; int n; int n_wrap_dest; + int n_min; int nch = cd->channels; n = frames * nch; while (n > 0) { n_wrap_dest = (int32_t *) sink->end_addr - dest; - if (n < n_wrap_dest) { - /* No circular wrap need */ - while (n > 0) { - /* Update period count for sweeps, etc. */ - tonegen_control(&cd->sg); - /* Calculate mono sine wave sample and then - * duplicate to channels. - */ - sine_sample = tonegen(&cd->sg); - n -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } + n_min = (n < n_wrap_dest) ? n : n_wrap_dest; + /* Process until wrap or completed n */ + while (n_min > 0) { + n -= nch; + n_min -= nch; + for (i = 0; i < nch; i++) { + tonegen_control(&cd->sg[i]); + *dest = tonegen(&cd->sg[i]); + dest++; } - } else { - /* Process until wrap */ - while (n_wrap_dest > 0) { - tonegen_control(&cd->sg); - sine_sample = tonegen(&cd->sg); - n -= nch; - n_wrap_dest -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } - } - /* No need to check if past end_addr, - * it is so just subtract buffer size. - */ - dest = (int32_t *) ((size_t) dest - - sink->ipc_buffer.size); } + tone_circ_inc_wrap(&dest, sink->end_addr, sink->size); } - sink->w_ptr = dest; } static int32_t tonegen(struct tone_state *sg) @@ -226,39 +237,46 @@ static void tonegen_control(struct tone_state *sg) } } +/* Set sine amplitude */ static inline void tonegen_set_a(struct tone_state *sg, int32_t a) { sg->a_target = a; } -static inline void tonegen_set_f(struct tone_state *sg, int32_t f) +/* Repeated number of beeps */ +static void tonegen_set_repeats(struct tone_state *sg, uint32_t r) { - sg->f = f; + sg->repeats = r; } -#if 0 -/* Tone sweep parameters description: - * fc - Multiplication factor for frequency as Q2.30 for logarithmic change - * ac - Multiplication factor for amplitude as Q2.30 for logarithmic change - * l - Tone length in samples, this is the active length of tone - * p - Tone period in samples, this is the length including the pause after beep - * r - Repeated number of beeps +/* The next functions support zero as shortcut for defaults to get + * make a nicer API without need to remember the neutral steady + * non-swept tone settings. */ -static void tonegen_set_sweep(struct tone_state *sg, int32_t fc, int32_t ac, - uint32_t l, uint32_t p, uint32_t r) +/* Multiplication factor for frequency as Q2.30 for logarithmic change */ +static void tonegen_set_freq_mult(struct tone_state *sg, int32_t fm) { - sg->repeats = r; + sg->freq_coef = (fm > 0) ? fm : ONE_Q2_30; /* Set freq mult to 1.0 */ +} - /* Zeros as defaults make a nicer API without need to remember - * the neutral settings for sweep and repeat parameters. - */ - sg->freq_coef = (fc > 0) ? fc : ONE_Q2_30; /* Set freq mult to 1.0 */ - sg->ampl_coef = (ac > 0) ? ac : ONE_Q2_30; /* Set ampl mult to 1.0 */ - sg->tone_length = (l > 0) ? l : INT32_MAXVALUE; /* Count rate 125 us */ - sg->tone_period = (p > 0) ? p : INT32_MAXVALUE; /* Count rate 125 us */ +/* Multiplication factor for amplitude as Q2.30 for logarithmic change */ +static void tonegen_set_ampl_mult(struct tone_state *sg, int32_t am) +{ + sg->ampl_coef = (am > 0) ? am : ONE_Q2_30; /* Set ampl mult to 1.0 */ +} + +/* Tone length in samples, this is the active length of tone */ +static void tonegen_set_length(struct tone_state *sg, uint32_t tl) +{ + sg->tone_length = (tl > 0) ? tl : INT32_MAXVALUE; /* Count rate 125 us */ +} + +/* Tone period in samples, this is the length including the pause after beep */ +static void tonegen_set_period(struct tone_state *sg, uint32_t tp) +{ + sg->tone_period = (tp > 0) ? tp : INT32_MAXVALUE; /* Count rate 125 us */ } -#endif /* Tone ramp parameters: * step - Value that is added or subtracted to amplitude. A zero or negative @@ -382,6 +400,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) struct sof_ipc_comp_tone *tone; struct sof_ipc_comp_tone *ipc_tone = (struct sof_ipc_comp_tone *) comp; struct comp_data *cd; + int i; trace_tone("new"); @@ -403,7 +422,8 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) cd->tone_func = tone_s32_default; /* Reset tone generator and set channels volumes to default */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]); dev->state = COMP_STATE_READY; return dev; @@ -441,34 +461,104 @@ static int tone_params(struct comp_dev *dev) return 0; } -static int tone_ctrl_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +static int tone_cmd_set_value(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev); + int j; + uint32_t ch; + uint32_t val; + + if (cdata->cmd == SOF_CTRL_CMD_SWITCH) { + trace_tone("mst"); + for (j = 0; j < cdata->num_elems; j++) { + ch = cdata->chanv[j].channel; + val = cdata->chanv[j].value; + tracev_value(ch); + tracev_value(val); + if (ch >= PLATFORM_MAX_CHANNELS) { + trace_tone_error("che"); + return -EINVAL; + } + if (val > 0) + tonegen_mute(&cd->sg[ch]); + else + tonegen_unmute(&cd->sg[ch]); + + } + } else { + trace_tone_error("ste"); + return -EINVAL; + } + + return 0; +} + +static int tone_cmd_set_data(struct comp_dev *dev, + struct sof_ipc_ctrl_data *cdata) +{ + struct comp_data *cd = comp_get_drvdata(dev); + struct sof_ipc_ctrl_value_comp *compv; + int i; + uint32_t ch; + uint32_t val; trace_tone("tri"); + /* Check version from ABI header */ + if (cdata->data->comp_abi != SOF_TONE_ABI_VERSION) { + trace_tone_error("abi"); + return -EINVAL; + } + switch (cdata->cmd) { - case SOF_CTRL_CMD_MUTE: - trace_tone("TMu"); - tonegen_mute(&cd->sg); - break; - case SOF_CTRL_CMD_UNMUTE: - trace_tone("TUm"); - tonegen_unmute(&cd->sg); - break; -/* TODO: use comp value list array to set this */ -#if 0 - case SOF_CTRL_TYPE_VALUE_COMP_SET: - trace_tone("Tto"); - ct = (struct sof_ipc_comp_tone *) data; - /* Ignore channels while tone implementation is mono */ - tonegen_set_f(&cd->sg, ct->frequency); - tonegen_set_a(&cd->sg, ct->amplitude); - tonegen_set_sweep(&cd->sg, ct->freq_mult, ct->ampl_mult, - ct->length, ct->period, ct->repeats); - tonegen_set_linramp(&cd->sg, ct->ramp_step); + case SOF_CTRL_CMD_ENUM: + trace_tone("ten"); + trace_value(cdata->index); + compv = (struct sof_ipc_ctrl_value_comp *) cdata->data->data; + for (i = 0; i < (int) cdata->num_elems; i++) { + ch = compv[i].index; + val = compv[i].svalue; + tracev_value(ch); + tracev_value(val); + switch (cdata->index) { + case SOF_TONE_IDX_FREQUENCY: + trace_tone("tfr"); + tonegen_update_f(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_AMPLITUDE: + trace_tone("tam"); + tonegen_set_a(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_FREQ_MULT: + trace_tone("tfx"); + tonegen_set_freq_mult(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_AMPL_MULT: + trace_tone("tax"); + tonegen_set_ampl_mult(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_LENGTH: + trace_tone("tle"); + tonegen_set_length(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_PERIOD: + trace_tone("tpe"); + tonegen_set_period(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_REPEATS: + trace_tone("trp"); + tonegen_set_repeats(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_LIN_RAMP_STEP: + trace_tone("trs"); + tonegen_set_linramp(&cd->sg[ch], val); + break; + default: + trace_tone_error("ier"); + return -EINVAL; + } + } break; -#endif default: trace_tone_error("ec1"); return -EINVAL; @@ -489,17 +579,22 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data) if (ret < 0) return ret; - if (cmd == COMP_CMD_SET_VALUE) - ret = tone_ctrl_cmd(dev, cdata); + switch (cmd) { + case COMP_CMD_SET_DATA: + ret = tone_cmd_set_data(dev, cdata); + break; + case COMP_CMD_SET_VALUE: + ret = tone_cmd_set_value(dev, cdata); + break; + } return ret; } /* copy and process stream data from source to sink buffers */ -static int tone_copy(struct comp_dev *dev) +static int tone_copy(struct comp_dev * dev) { struct comp_buffer *sink; - struct comp_buffer *source = NULL; struct comp_data *cd = comp_get_drvdata(dev); trace_comp("cpy"); @@ -513,19 +608,22 @@ static int tone_copy(struct comp_dev *dev) */ if (sink->free >= cd->period_bytes) { /* create tone */ - cd->tone_func(dev, sink, source, dev->frames); - comp_update_buffer_produce(sink, 0); + cd->tone_func(dev, sink, dev->frames); + + /* calc new free and available */ + comp_update_buffer_produce(sink, cd->period_bytes); } return dev->frames; } -static int tone_prepare(struct comp_dev *dev) +static int tone_prepare(struct comp_dev * dev) { struct comp_data *cd = comp_get_drvdata(dev); int32_t f; int32_t a; int ret; + int i; trace_tone("TPp"); @@ -538,30 +636,34 @@ static int tone_prepare(struct comp_dev *dev) tracev_value(cd->channels); tracev_value(cd->rate); - f = tonegen_get_f(&cd->sg); - a = tonegen_get_a(&cd->sg); - if (tonegen_init(&cd->sg, cd->rate, f, a) < 0) { - comp_set_state(dev, COMP_CMD_RESET); - return -EINVAL; + for (i = 0; i < cd->channels; i++) { + f = tonegen_get_f(&cd->sg[i]); + a = tonegen_get_a(&cd->sg[i]); + if (tonegen_init(&cd->sg[i], cd->rate, f, a) < 0) { + comp_set_state(dev, COMP_CMD_RESET); + return -EINVAL; + } } return 0; } -static int tone_preload(struct comp_dev *dev) +static int tone_preload(struct comp_dev * dev) { return tone_copy(dev); } -static int tone_reset(struct comp_dev *dev) +static int tone_reset(struct comp_dev * dev) { struct comp_data *cd = comp_get_drvdata(dev); + int i; trace_tone("TRe"); /* Initialize with the defaults */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]); comp_set_state(dev, COMP_CMD_RESET); @@ -570,8 +672,7 @@ static int tone_reset(struct comp_dev *dev) struct comp_driver comp_tone = { .type = SOF_COMP_TONE, - .ops = - { + .ops = { .new = tone_new, .free = tone_free, .params = tone_params, diff --git a/src/audio/tone.h b/src/include/uapi/tone.h similarity index 58% rename from src/audio/tone.h rename to src/include/uapi/tone.h index fcaf404..b04e7dc 100644 --- a/src/audio/tone.h +++ b/src/include/uapi/tone.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, Intel Corporation + * Copyright (c) 2017, Intel Corporation * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -26,33 +26,23 @@ * POSSIBILITY OF SUCH DAMAGE. * * Author: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> - * Liam Girdwood <liam.r.girdwood@linux.intel.com> - * Keyon Jie <yang.jie@linux.intel.com> */ -/* Convert float frequency in Hz to Q16.16 fractional format */ -#define TONE_FREQ(f) Q_CONVERT_FLOAT(f, 16) +#ifndef TONE_H +#define TONE_H -/* Convert float gain to Q1.31 fractional format */ -#define TONE_GAIN(v) Q_CONVERT_FLOAT(v, 31) +/* Component will reject non-matching configuration. The version number need + * to be incremented with any ABI changes in function fir_cmd(). + */ +#define SOF_TONE_ABI_VERSION 1 + +#define SOF_TONE_IDX_FREQUENCY 0 +#define SOF_TONE_IDX_AMPLITUDE 1 +#define SOF_TONE_IDX_FREQ_MULT 2 +#define SOF_TONE_IDX_AMPL_MULT 3 +#define SOF_TONE_IDX_LENGTH 4 +#define SOF_TONE_IDX_PERIOD 5 +#define SOF_TONE_IDX_REPEATS 6 +#define SOF_TONE_IDX_LIN_RAMP_STEP 7 -struct tone_state { - int mute; - int32_t a; /* Current amplitude Q1.31 */ - int32_t a_target; /* Target amplitude Q1.31 */ - int32_t ampl_coef; /* Amplitude multiplier Q2.30 */ - int32_t c; /* Coefficient 2*pi/Fs Q1.31 */ - int32_t f; /* Frequency Q16.16 */ - int32_t freq_coef; /* Frequency multiplier Q2.30 */ - int32_t fs; /* Sample rate in Hertz Q32.0 */ - int32_t ramp_step; /* Amplitude ramp step Q1.31 */ - int32_t w; /* Angle radians Q4.28 */ - int32_t w_step; /* Angle step Q4.28 */ - uint32_t block_count; - uint32_t repeat_count; - uint32_t repeats; /* Number of repeats for tone (sweep steps) */ - uint32_t sample_count; - uint32_t samples_in_block; /* Samples in 125 us block */ - uint32_t tone_length; /* Active length in 125 us blocks */ - uint32_t tone_period; /* Active + idle time in 125 us blocks */ -}; +#endif /* TONE_ABI_H */
On 23.10.2017 11:48, Seppo Ingalsuo wrote:
should this patch be split in two, one that deals with control IPC and one with internal implementation changes? it'd make the review easier, no?
Yep, it's a fair request, will do.
From ea85a69b759144a214f34ce943a118375a66cf2e Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> Date: Tue, 24 Oct 2017 11:28:21 +0300 Subject: [PATCH 1/2] Tone: Independent tones for each audio channel (DO NOT APPLY THIS) The single large tone patch has been applied already. This split patch only shows changes per feature to ease the patch review. This patch contains the multi-channel tone generation and code cleanup such as remove unused function argument such as source buffer struct for tone copy. Also the samples copy loop is simplified and buffer write pointer circular wrap is done with an inline function for nicer looking code. In the end of tone copy() the call of comp_update_buffer_produce() is done with actual value instead of zero since all the sink buffer updates are now done in the update produce function. Calling this function in buffer.h with zero as parameter is not allowed usage (may corrupt the buffer). White space changes include some edits for Linux kernel style. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/audio/tone.c | 76 ++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 44 deletions(-) diff --git a/src/audio/tone.c b/src/audio/tone.c index 1fde455..24ea901 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -82,64 +82,48 @@ struct comp_data { uint32_t channels; uint32_t frame_bytes; uint32_t rate; - struct tone_state sg; + struct tone_state sg[PLATFORM_MAX_CHANNELS]; void (*tone_func)(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames); + uint32_t frames); }; /* * Tone generator algorithm code */ +static inline void tone_circ_inc_wrap(int32_t **ptr, int32_t *end, size_t size) +{ + if (*ptr >= end) + *ptr = (int32_t *) ((size_t) * ptr - size); +} + static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames) + uint32_t frames) { struct comp_data *cd = comp_get_drvdata(dev); - int32_t sine_sample; int32_t *dest = (int32_t*) sink->w_ptr; int i; int n; int n_wrap_dest; + int n_min; int nch = cd->channels; n = frames * nch; while (n > 0) { n_wrap_dest = (int32_t *) sink->end_addr - dest; - if (n < n_wrap_dest) { - /* No circular wrap need */ - while (n > 0) { - /* Update period count for sweeps, etc. */ - tonegen_control(&cd->sg); - /* Calculate mono sine wave sample and then - * duplicate to channels. - */ - sine_sample = tonegen(&cd->sg); - n -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } - } - } else { - /* Process until wrap */ - while (n_wrap_dest > 0) { - tonegen_control(&cd->sg); - sine_sample = tonegen(&cd->sg); - n -= nch; - n_wrap_dest -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } + n_min = (n < n_wrap_dest) ? n : n_wrap_dest; + /* Process until wrap or completed n */ + while (n_min > 0) { + n -= nch; + n_min -= nch; + for (i = 0; i < nch; i++) { + tonegen_control(&cd->sg[i]); + *dest = tonegen(&cd->sg[i]); + dest++; } - /* No need to check if past end_addr, - * it is so just subtract buffer size. - */ - dest = (int32_t *) ((size_t) dest - - sink->ipc_buffer.size); } + tone_circ_inc_wrap(&dest, sink->end_addr, sink->size); } - sink->w_ptr = dest; } static int32_t tonegen(struct tone_state *sg) @@ -382,6 +366,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) struct sof_ipc_comp_tone *tone; struct sof_ipc_comp_tone *ipc_tone = (struct sof_ipc_comp_tone *) comp; struct comp_data *cd; + int i; trace_tone("new"); @@ -403,7 +388,8 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) cd->tone_func = tone_s32_default; /* Reset tone generator and set channels volumes to default */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]); dev->state = COMP_STATE_READY; return dev; @@ -496,10 +482,9 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data) } /* copy and process stream data from source to sink buffers */ -static int tone_copy(struct comp_dev *dev) +static int tone_copy(struct comp_dev * dev) { struct comp_buffer *sink; - struct comp_buffer *source = NULL; struct comp_data *cd = comp_get_drvdata(dev); trace_comp("cpy"); @@ -513,8 +498,10 @@ static int tone_copy(struct comp_dev *dev) */ if (sink->free >= cd->period_bytes) { /* create tone */ - cd->tone_func(dev, sink, source, dev->frames); - comp_update_buffer_produce(sink, 0); + cd->tone_func(dev, sink, dev->frames); + + /* calc new free and available */ + comp_update_buffer_produce(sink, cd->period_bytes); } return dev->frames; @@ -557,11 +544,13 @@ static int tone_reset(struct comp_dev *dev) { struct comp_data *cd = comp_get_drvdata(dev); + int i; trace_tone("TRe"); /* Initialize with the defaults */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]); comp_set_state(dev, COMP_CMD_RESET); @@ -570,8 +559,7 @@ static int tone_reset(struct comp_dev *dev) struct comp_driver comp_tone = { .type = SOF_COMP_TONE, - .ops = - { + .ops = { .new = tone_new, .free = tone_free, .params = tone_params, -- 2.11.0
On 23.10.2017 11:48, Seppo Ingalsuo wrote:
should this patch be split in two, one that deals with control IPC and one with internal implementation changes? it'd make the review easier, no?
Yep, it's a fair request, will do.
From a4bb19d45290b69852c211077a3a28892449e034 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> Date: Tue, 24 Oct 2017 11:37:24 +0300 Subject: [PATCH 2/2] Tone: Fix control IPC (DO NOT APPLY THIS) The single large tone patch has been applied already. This split patch only shows changes per feature to ease the patch review. This patch fixes the control interface to follow the component ABI. The new ABI compliant header file include/uapi/tone.h replaces the old src/audio/tone.h that is deleted. The definitions from there are moved to new tone.h and head of module tone.c. Other changes are add of some comments to functions to set sine wave tone parameters. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/audio/tone.c | 225 ++++++++++++++++++++++++++++--------- src/{audio => include/uapi}/tone.h | 44 +++----- 2 files changed, 186 insertions(+), 83 deletions(-) rename src/{audio => include/uapi}/tone.h (58%) diff --git a/src/audio/tone.c b/src/audio/tone.c index 24ea901..fefecd5 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -45,7 +45,7 @@ #include <reef/audio/pipeline.h> #include <reef/math/trig.h> #include <uapi/ipc.h> -#include "tone.h" +#include <uapi/tone.h> #ifdef MODULE_TEST #include <stdio.h> @@ -55,14 +55,17 @@ #define tracev_tone(__e) tracev_event(TRACE_CLASS_TONE, __e) #define trace_tone_error(__e) trace_error(TRACE_CLASS_TONE, __e) -#define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ -#define TONE_AMPLITUDE_DEFAULT MINUS_60DB_Q1_31 /* -60 dB */ -#define TONE_FREQUENCY_DEFAULT TONE_FREQ(997.0) /* 997 Hz */ +/* Convert float frequency in Hz to Q16.16 fractional format */ +#define TONE_FREQ(f) Q_CONVERT_FLOAT(f, 16) -static int32_t tonegen(struct tone_state *sg); -static void tonegen_control(struct tone_state *sg); -static void tonegen_update_f(struct tone_state *sg, int32_t f); +/* Convert float gain to Q1.31 fractional format */ +#define TONE_GAIN(v) Q_CONVERT_FLOAT(v, 31) +/* Set default tone amplitude and frequency */ +#define TONE_AMPLITUDE_DEFAULT TONE_GAIN(0.5) /* -6 dB */ +#define TONE_FREQUENCY_DEFAULT TONE_FREQ(82.41) /* E2 note */ + +#define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ /* 2*pi/Fs lookup tables in Q1.31 for each Fs */ static const int32_t tone_fs_list[TONE_NUM_FS] = { @@ -76,7 +79,27 @@ static const int32_t tone_pi2_div_fs[TONE_NUM_FS] = { /* tone component private data */ -/* TODO: Remove *source when internal endpoint is possible */ +struct tone_state { + int mute; + int32_t a; /* Current amplitude Q1.31 */ + int32_t a_target; /* Target amplitude Q1.31 */ + int32_t ampl_coef; /* Amplitude multiplier Q2.30 */ + int32_t c; /* Coefficient 2*pi/Fs Q1.31 */ + int32_t f; /* Frequency Q16.16 */ + int32_t freq_coef; /* Frequency multiplier Q2.30 */ + int32_t fs; /* Sample rate in Hertz Q32.0 */ + int32_t ramp_step; /* Amplitude ramp step Q1.31 */ + int32_t w; /* Angle radians Q4.28 */ + int32_t w_step; /* Angle step Q4.28 */ + uint32_t block_count; + uint32_t repeat_count; + uint32_t repeats; /* Number of repeats for tone (sweep steps) */ + uint32_t sample_count; + uint32_t samples_in_block; /* Samples in 125 us block */ + uint32_t tone_length; /* Active length in 125 us blocks */ + uint32_t tone_period; /* Active + idle time in 125 us blocks */ +}; + struct comp_data { uint32_t period_bytes; uint32_t channels; @@ -87,6 +110,10 @@ struct comp_data { uint32_t frames); }; +static int32_t tonegen(struct tone_state *sg); +static void tonegen_control(struct tone_state *sg); +static void tonegen_update_f(struct tone_state *sg, int32_t f); + /* * Tone generator algorithm code */ @@ -210,39 +237,46 @@ static void tonegen_control(struct tone_state *sg) } } +/* Set sine amplitude */ static inline void tonegen_set_a(struct tone_state *sg, int32_t a) { sg->a_target = a; } -static inline void tonegen_set_f(struct tone_state *sg, int32_t f) +/* Repeated number of beeps */ +static void tonegen_set_repeats(struct tone_state *sg, uint32_t r) { - sg->f = f; + sg->repeats = r; } -#if 0 -/* Tone sweep parameters description: - * fc - Multiplication factor for frequency as Q2.30 for logarithmic change - * ac - Multiplication factor for amplitude as Q2.30 for logarithmic change - * l - Tone length in samples, this is the active length of tone - * p - Tone period in samples, this is the length including the pause after beep - * r - Repeated number of beeps +/* The next functions support zero as shortcut for defaults to get + * make a nicer API without need to remember the neutral steady + * non-swept tone settings. */ -static void tonegen_set_sweep(struct tone_state *sg, int32_t fc, int32_t ac, - uint32_t l, uint32_t p, uint32_t r) +/* Multiplication factor for frequency as Q2.30 for logarithmic change */ +static void tonegen_set_freq_mult(struct tone_state *sg, int32_t fm) { - sg->repeats = r; + sg->freq_coef = (fm > 0) ? fm : ONE_Q2_30; /* Set freq mult to 1.0 */ +} - /* Zeros as defaults make a nicer API without need to remember - * the neutral settings for sweep and repeat parameters. - */ - sg->freq_coef = (fc > 0) ? fc : ONE_Q2_30; /* Set freq mult to 1.0 */ - sg->ampl_coef = (ac > 0) ? ac : ONE_Q2_30; /* Set ampl mult to 1.0 */ - sg->tone_length = (l > 0) ? l : INT32_MAXVALUE; /* Count rate 125 us */ - sg->tone_period = (p > 0) ? p : INT32_MAXVALUE; /* Count rate 125 us */ +/* Multiplication factor for amplitude as Q2.30 for logarithmic change */ +static void tonegen_set_ampl_mult(struct tone_state *sg, int32_t am) +{ + sg->ampl_coef = (am > 0) ? am : ONE_Q2_30; /* Set ampl mult to 1.0 */ +} + +/* Tone length in samples, this is the active length of tone */ +static void tonegen_set_length(struct tone_state *sg, uint32_t tl) +{ + sg->tone_length = (tl > 0) ? tl : INT32_MAXVALUE; /* Count rate 125 us */ +} + +/* Tone period in samples, this is the length including the pause after beep */ +static void tonegen_set_period(struct tone_state *sg, uint32_t tp) +{ + sg->tone_period = (tp > 0) ? tp : INT32_MAXVALUE; /* Count rate 125 us */ } -#endif /* Tone ramp parameters: * step - Value that is added or subtracted to amplitude. A zero or negative @@ -427,34 +461,104 @@ static int tone_params(struct comp_dev *dev) return 0; } -static int tone_ctrl_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +static int tone_cmd_set_value(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +{ + struct comp_data *cd = comp_get_drvdata(dev); + int j; + uint32_t ch; + uint32_t val; + + if (cdata->cmd == SOF_CTRL_CMD_SWITCH) { + trace_tone("mst"); + for (j = 0; j < cdata->num_elems; j++) { + ch = cdata->chanv[j].channel; + val = cdata->chanv[j].value; + tracev_value(ch); + tracev_value(val); + if (ch >= PLATFORM_MAX_CHANNELS) { + trace_tone_error("che"); + return -EINVAL; + } + if (val > 0) + tonegen_mute(&cd->sg[ch]); + else + tonegen_unmute(&cd->sg[ch]); + + } + } else { + trace_tone_error("ste"); + return -EINVAL; + } + + return 0; +} + +static int tone_cmd_set_data(struct comp_dev *dev, + struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev); + struct sof_ipc_ctrl_value_comp *compv; + int i; + uint32_t ch; + uint32_t val; trace_tone("tri"); + /* Check version from ABI header */ + if (cdata->data->comp_abi != SOF_TONE_ABI_VERSION) { + trace_tone_error("abi"); + return -EINVAL; + } + switch (cdata->cmd) { - case SOF_CTRL_CMD_MUTE: - trace_tone("TMu"); - tonegen_mute(&cd->sg); - break; - case SOF_CTRL_CMD_UNMUTE: - trace_tone("TUm"); - tonegen_unmute(&cd->sg); - break; -/* TODO: use comp value list array to set this */ -#if 0 - case SOF_CTRL_TYPE_VALUE_COMP_SET: - trace_tone("Tto"); - ct = (struct sof_ipc_comp_tone *) data; - /* Ignore channels while tone implementation is mono */ - tonegen_set_f(&cd->sg, ct->frequency); - tonegen_set_a(&cd->sg, ct->amplitude); - tonegen_set_sweep(&cd->sg, ct->freq_mult, ct->ampl_mult, - ct->length, ct->period, ct->repeats); - tonegen_set_linramp(&cd->sg, ct->ramp_step); + case SOF_CTRL_CMD_ENUM: + trace_tone("ten"); + trace_value(cdata->index); + compv = (struct sof_ipc_ctrl_value_comp *) cdata->data->data; + for (i = 0; i < (int) cdata->num_elems; i++) { + ch = compv[i].index; + val = compv[i].svalue; + tracev_value(ch); + tracev_value(val); + switch (cdata->index) { + case SOF_TONE_IDX_FREQUENCY: + trace_tone("tfr"); + tonegen_update_f(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_AMPLITUDE: + trace_tone("tam"); + tonegen_set_a(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_FREQ_MULT: + trace_tone("tfx"); + tonegen_set_freq_mult(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_AMPL_MULT: + trace_tone("tax"); + tonegen_set_ampl_mult(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_LENGTH: + trace_tone("tle"); + tonegen_set_length(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_PERIOD: + trace_tone("tpe"); + tonegen_set_period(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_REPEATS: + trace_tone("trp"); + tonegen_set_repeats(&cd->sg[ch], val); + break; + case SOF_TONE_IDX_LIN_RAMP_STEP: + trace_tone("trs"); + tonegen_set_linramp(&cd->sg[ch], val); + break; + default: + trace_tone_error("ier"); + return -EINVAL; + } + } break; -#endif default: trace_tone_error("ec1"); return -EINVAL; @@ -475,8 +579,14 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data) if (ret < 0) return ret; - if (cmd == COMP_CMD_SET_VALUE) - ret = tone_ctrl_cmd(dev, cdata); + switch (cmd) { + case COMP_CMD_SET_DATA: + ret = tone_cmd_set_data(dev, cdata); + break; + case COMP_CMD_SET_VALUE: + ret = tone_cmd_set_value(dev, cdata); + break; + } return ret; } @@ -513,6 +623,7 @@ static int tone_prepare(struct comp_dev *dev) int32_t f; int32_t a; int ret; + int i; trace_tone("TPp"); @@ -525,11 +636,13 @@ static int tone_prepare(struct comp_dev *dev) tracev_value(cd->channels); tracev_value(cd->rate); - f = tonegen_get_f(&cd->sg); - a = tonegen_get_a(&cd->sg); - if (tonegen_init(&cd->sg, cd->rate, f, a) < 0) { - comp_set_state(dev, COMP_CMD_RESET); - return -EINVAL; + for (i = 0; i < cd->channels; i++) { + f = tonegen_get_f(&cd->sg[i]); + a = tonegen_get_a(&cd->sg[i]); + if (tonegen_init(&cd->sg[i], cd->rate, f, a) < 0) { + comp_set_state(dev, COMP_CMD_RESET); + return -EINVAL; + } } return 0; diff --git a/src/audio/tone.h b/src/include/uapi/tone.h similarity index 58% rename from src/audio/tone.h rename to src/include/uapi/tone.h index fcaf404..b04e7dc 100644 --- a/src/audio/tone.h +++ b/src/include/uapi/tone.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, Intel Corporation + * Copyright (c) 2017, Intel Corporation * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -26,33 +26,23 @@ * POSSIBILITY OF SUCH DAMAGE. * * Author: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> - * Liam Girdwood <liam.r.girdwood@linux.intel.com> - * Keyon Jie <yang.jie@linux.intel.com> */ -/* Convert float frequency in Hz to Q16.16 fractional format */ -#define TONE_FREQ(f) Q_CONVERT_FLOAT(f, 16) +#ifndef TONE_H +#define TONE_H -/* Convert float gain to Q1.31 fractional format */ -#define TONE_GAIN(v) Q_CONVERT_FLOAT(v, 31) +/* Component will reject non-matching configuration. The version number need + * to be incremented with any ABI changes in function fir_cmd(). + */ +#define SOF_TONE_ABI_VERSION 1 + +#define SOF_TONE_IDX_FREQUENCY 0 +#define SOF_TONE_IDX_AMPLITUDE 1 +#define SOF_TONE_IDX_FREQ_MULT 2 +#define SOF_TONE_IDX_AMPL_MULT 3 +#define SOF_TONE_IDX_LENGTH 4 +#define SOF_TONE_IDX_PERIOD 5 +#define SOF_TONE_IDX_REPEATS 6 +#define SOF_TONE_IDX_LIN_RAMP_STEP 7 -struct tone_state { - int mute; - int32_t a; /* Current amplitude Q1.31 */ - int32_t a_target; /* Target amplitude Q1.31 */ - int32_t ampl_coef; /* Amplitude multiplier Q2.30 */ - int32_t c; /* Coefficient 2*pi/Fs Q1.31 */ - int32_t f; /* Frequency Q16.16 */ - int32_t freq_coef; /* Frequency multiplier Q2.30 */ - int32_t fs; /* Sample rate in Hertz Q32.0 */ - int32_t ramp_step; /* Amplitude ramp step Q1.31 */ - int32_t w; /* Angle radians Q4.28 */ - int32_t w_step; /* Angle step Q4.28 */ - uint32_t block_count; - uint32_t repeat_count; - uint32_t repeats; /* Number of repeats for tone (sweep steps) */ - uint32_t sample_count; - uint32_t samples_in_block; /* Samples in 125 us block */ - uint32_t tone_length; /* Active length in 125 us blocks */ - uint32_t tone_period; /* Active + idle time in 125 us blocks */ -}; +#endif /* TONE_ABI_H */ -- 2.11.0
The previous patch for SOF updated component IPC commands and provided an ABI for configuration and control. This patch updates the FIR equalizer to use such controls. Also the missing COMP_STATE_READY from method new() is added. Lack of it caused pipeline prepare fail. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/audio/eq_fir.c | 101 +++++++++++++++++++++++++++++--------------------- src/include/uapi/eq.h | 4 +- 2 files changed, 62 insertions(+), 43 deletions(-) diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c index d1f5072..9056808 100644 --- a/src/audio/eq_fir.c +++ b/src/audio/eq_fir.c @@ -224,24 +224,16 @@ static int eq_fir_setup(struct fir_state_32x16 fir[], } static int eq_fir_switch_response(struct fir_state_32x16 fir[], - struct sof_eq_fir_config *config, - struct sof_ipc_ctrl_value_comp compv[], uint32_t num_elemens, int nch) + struct sof_eq_fir_config *config, uint32_t ch, int32_t response) { - int i; - int j; int ret; /* Copy assign response from update and re-initilize EQ */ - if (config == NULL) + if ((config == NULL) || (ch >= PLATFORM_MAX_CHANNELS)) return -EINVAL; - for (i = 0; i < num_elemens; i++) { - j = compv[i].index; - if (j < config->channels_in_config) - config->data[i] = compv[i].svalue; - } - - ret = eq_fir_setup(fir, config, nch); + config->data[ch] = response; + ret = eq_fir_setup(fir, config, PLATFORM_MAX_CHANNELS); return ret; } @@ -282,6 +274,7 @@ static struct comp_dev *eq_fir_new(struct sof_ipc_comp *comp) for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) fir_reset(&cd->fir[i]); + dev->state = COMP_STATE_READY; return dev; } @@ -332,7 +325,36 @@ static int eq_fir_params(struct comp_dev *dev) return 0; } -static int fir_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +static int fir_cmd_set_value(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +{ + struct comp_data *cd = comp_get_drvdata(dev); + int j; + uint32_t ch; + + if (cdata->cmd == SOF_CTRL_CMD_SWITCH) { + trace_eq("mst"); + for (j = 0; j < cdata->num_elems; j++) { + ch = cdata->chanv[j].channel; + tracev_value(ch); + tracev_value(cdata->chanv[j].value); + if (ch >= PLATFORM_MAX_CHANNELS) { + trace_eq_error("che"); + return -EINVAL; + } + if (cdata->chanv[j].value > 0) + fir_mute(&cd->fir[ch]); + else + fir_unmute(&cd->fir[ch]); + } + } else { + trace_eq_error("ste"); + return -EINVAL; + } + + return 0; +} + +static int fir_cmd_set_data(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev); struct sof_ipc_ctrl_value_comp *compv; @@ -347,22 +369,28 @@ static int fir_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) return -EINVAL; switch (cdata->cmd) { - case SOF_CTRL_CMD_EQ_SWITCH: - trace_eq("EFx"); - compv = (struct sof_ipc_ctrl_value_comp *) cdata->data->data; - ret = eq_fir_switch_response(cd->fir, cd->config, - compv, cdata->num_elems, PLATFORM_MAX_CHANNELS); - if (ret < 0) { - trace_eq_error("ec1"); - return ret; + case SOF_CTRL_CMD_ENUM: + trace_eq("EFe"); + if (cdata->index == SOF_EQ_FIR_IDX_SWITCH) { + trace_eq("EFs"); + compv = (struct sof_ipc_ctrl_value_comp *) cdata->data->data; + for (i = 0; i < (int) cdata->num_elems; i++) { + tracev_value(compv[i].index); + tracev_value(compv[i].svalue); + ret = eq_fir_switch_response(cd->fir, cd->config, + compv[i].index, compv[i].svalue); + if (ret < 0) { + trace_eq_error("swe"); + return -EINVAL; + } + } + } else { + trace_eq_error("une"); + trace_value(cdata->index); + return -EINVAL; } - - /* Print trace information */ - for (i = 0; i < cd->config->channels_in_config; i++) - tracev_value(cd->config->data[i]); - break; - case SOF_CTRL_CMD_EQ_CONFIG: + case SOF_CTRL_CMD_BINARY: trace_eq("EFc"); /* Check and free old config */ @@ -380,18 +408,6 @@ static int fir_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) memcpy(cd->config, cdata->data->data, bs); ret = eq_fir_setup(cd->fir, cd->config, PLATFORM_MAX_CHANNELS); break; - case SOF_CTRL_CMD_MUTE: - trace_eq("EFm"); - for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) - fir_mute(&cd->fir[i]); - - break; - case SOF_CTRL_CMD_UNMUTE: - trace_eq("EFu"); - for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) - fir_unmute(&cd->fir[i]); - - break; default: trace_eq_error("ec1"); ret = -EINVAL; @@ -414,14 +430,15 @@ static int eq_fir_cmd(struct comp_dev *dev, int cmd, void *data) return ret; switch (cmd) { + case COMP_CMD_SET_VALUE: + ret = fir_cmd_set_value(dev, cdata); + break; case COMP_CMD_SET_DATA: - ret = fir_cmd(dev, cdata); + ret = fir_cmd_set_data(dev, cdata); break; case COMP_CMD_STOP: comp_buffer_reset(dev); break; - default: - break; } return ret; diff --git a/src/include/uapi/eq.h b/src/include/uapi/eq.h index 281df3b..c512f29 100644 --- a/src/include/uapi/eq.h +++ b/src/include/uapi/eq.h @@ -36,7 +36,9 @@ /* Component will reject non-matching configuration. The version number need * to be incremented with any ABI changes in function fir_cmd(). */ -#define SOF_EQ_FIR_ABI_VERSION 1 +#define SOF_EQ_FIR_ABI_VERSION 1 + +#define SOF_EQ_FIR_IDX_SWITCH 0 #define SOF_EQ_FIR_MAX_SIZE 4096 /* Max size allowed for coef data in bytes */ -- 2.11.0
The previous patch for SOF updated component IPC commands and provided an ABI for configuration and control. This patch updates the IIR equalizer to use such controls. Also the missing COMP_STATE_READY from method new() is added. Lack of it caused pipeline prepare fail. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> --- src/audio/eq_iir.c | 104 ++++++++++++++++++++++++++++++-------------------- src/include/uapi/eq.h | 4 +- 2 files changed, 66 insertions(+), 42 deletions(-) diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c index 723e5b3..d3a8fa3 100644 --- a/src/audio/eq_iir.c +++ b/src/audio/eq_iir.c @@ -229,24 +229,16 @@ static int eq_iir_setup(struct iir_state_df2t iir[], } static int eq_iir_switch_response(struct iir_state_df2t iir[], - struct sof_eq_iir_config *config, - struct sof_ipc_ctrl_value_comp compv[], uint32_t num_elemens, int nch) + struct sof_eq_iir_config *config, uint32_t ch, int32_t response) { - int i; - int j; int ret; /* Copy assign response from update and re-initilize EQ */ - if (config == NULL) + if ((config == NULL) || (ch >= PLATFORM_MAX_CHANNELS)) return -EINVAL; - for (i = 0; i < num_elemens; i++) { - j = compv[i].index; - if (j < config->channels_in_config) - config->data[i] = compv[i].svalue; - } - - ret = eq_iir_setup(iir, config, nch); + config->data[ch] = response; + ret = eq_iir_setup(iir, config, PLATFORM_MAX_CHANNELS); return ret; } @@ -283,6 +275,7 @@ static struct comp_dev *eq_iir_new(struct sof_ipc_comp *comp) for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) iir_reset_df2t(&cd->iir[i]); + dev->state = COMP_STATE_READY; return dev; } @@ -333,7 +326,38 @@ static int eq_iir_params(struct comp_dev *dev) return 0; } -static int iir_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +static int iir_cmd_set_value(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) +{ + struct comp_data *cd = comp_get_drvdata(dev); + int j; + uint32_t ch; + uint32_t val; + + if (cdata->cmd == SOF_CTRL_CMD_SWITCH) { + trace_eq_iir("mst"); + for (j = 0; j < cdata->num_elems; j++) { + ch = cdata->chanv[j].channel; + val = cdata->chanv[j].value; + tracev_value(ch); + tracev_value(val); + if (ch >= PLATFORM_MAX_CHANNELS) { + trace_eq_iir_error("che"); + return -EINVAL; + } + if (val > 0) + iir_mute_df2t(&cd->iir[ch]); + else + iir_unmute_df2t(&cd->iir[ch]); + } + } else { + trace_eq_iir_error("ste"); + return -EINVAL; + } + + return 0; +} + +static int iir_cmd_set_data(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev); struct sof_ipc_ctrl_value_comp *compv; @@ -342,20 +366,29 @@ static int iir_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) size_t bs; switch (cdata->cmd) { - case SOF_CTRL_CMD_EQ_SWITCH: - trace_eq_iir("EFx"); - compv = (struct sof_ipc_ctrl_value_comp *) cdata->data->data; - ret = eq_iir_switch_response(cd->iir, cd->config, - compv, cdata->num_elems, PLATFORM_MAX_CHANNELS); - - /* Print trace information */ - tracev_value(cd->config->channels_in_config); - for (i = 0; i < cd->config->channels_in_config; i++) - tracev_value(cd->config->data[i]); - + case SOF_CTRL_CMD_ENUM: + trace_eq_iir("EIe"); + if (cdata->index == SOF_EQ_IIR_IDX_SWITCH) { + trace_eq_iir("EIs"); + compv = (struct sof_ipc_ctrl_value_comp *) cdata->data->data; + for (i = 0; i < (int) cdata->num_elems; i++) { + tracev_value(compv[i].index); + tracev_value(compv[i].svalue); + ret = eq_iir_switch_response(cd->iir, cd->config, + compv[i].index, compv[i].svalue); + if (ret < 0) { + trace_eq_iir_error("swe"); + return -EINVAL; + } + } + } else { + trace_eq_iir_error("une"); + trace_value(cdata->index); + return -EINVAL; + } break; - case SOF_CTRL_CMD_EQ_CONFIG: - trace_eq_iir("EFc"); + case SOF_CTRL_CMD_BINARY: + trace_eq_iir("EIb"); /* Check and free old config */ eq_iir_free_parameters(&cd->config); @@ -375,18 +408,6 @@ static int iir_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) */ ret = eq_iir_setup(cd->iir, cd->config, PLATFORM_MAX_CHANNELS); break; - case SOF_CTRL_CMD_MUTE: - trace_eq_iir("EFm"); - for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) - iir_mute_df2t(&cd->iir[i]); - - break; - case SOF_CTRL_CMD_UNMUTE: - trace_eq_iir("EFu"); - for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) - iir_unmute_df2t(&cd->iir[i]); - - break; default: trace_eq_iir_error("ec1"); ret = -EINVAL; @@ -409,14 +430,15 @@ static int eq_iir_cmd(struct comp_dev *dev, int cmd, void *data) return ret; switch (cmd) { + case COMP_CMD_SET_VALUE: + ret = iir_cmd_set_value(dev, cdata); + break; case COMP_CMD_SET_DATA: - ret = iir_cmd(dev, cdata); + ret = iir_cmd_set_data(dev, cdata); break; case COMP_CMD_STOP: comp_buffer_reset(dev); break; - default: - break; } return ret; diff --git a/src/include/uapi/eq.h b/src/include/uapi/eq.h index c512f29..99b2770 100644 --- a/src/include/uapi/eq.h +++ b/src/include/uapi/eq.h @@ -72,7 +72,9 @@ struct sof_eq_fir_config { /* Component will reject non-matching configuration. The version number need * to be incremented with any ABI changes in function fir_cmd(). */ -#define SOF_EQ_FIR_ABI_VERSION 1 +#define SOF_EQ_FIR_ABI_VERSION 1 + +#define SOF_EQ_IIR_IDX_SWITCH 0 #define SOF_EQ_IIR_MAX_SIZE 1024 /* Max size allowed for coef data in bytes */ -- 2.11.0
participants (2)
-
Pierre-Louis Bossart -
Seppo Ingalsuo