From 0a14331898ec68a9f62768b7c4c2bf0aea3af405 Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Wed, 2 Oct 2024 16:34:34 -0700 Subject: [PATCH 1/3] component: add macro to toggle shims by IPC type Add helper to switch out init calls to shim in conversion functions if needed. Signed-off-by: Curtis Malainey --- src/include/sof/audio/component.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index 483a32439e56..ff6419fa0d56 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -247,6 +247,13 @@ enum { /** @}*/ +/** \brief Helper macro for IPC3 init shims */ +#ifdef CONFIG_IPC_MAJOR_3 + #define INIT_IPC_SHIM(shim, init) shim +#else + #define INIT_IPC_SHIM(shim, init) init +#endif + /** \brief Type of endpoint this component is connected to in a pipeline */ enum comp_endpoint_type { COMP_ENDPOINT_HOST, /**< Connected to host dma */ From 1cb3aa7eca0033f4c3886e8c0b1576d51267b4e5 Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Thu, 3 Oct 2024 15:04:10 -0700 Subject: [PATCH 2/3] ipc: make config size generally available Right now we are doing a lot of "guess work" based on type enums and context for what size `spec` is. Move towards checking the actual size and trusting that. Signed-off-by: Curtis Malainey --- src/include/sof/audio/component.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index ff6419fa0d56..0b705a7ea79a 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -558,9 +558,7 @@ struct comp_ipc_config { uint32_t periods_source; /**< 0 means variable */ uint32_t frame_fmt; /**< SOF_IPC_FRAME_ */ uint32_t xrun_action; /**< action we should take on XRUN */ -#if CONFIG_IPC_MAJOR_4 uint32_t ipc_config_size; /**< size of a config received by ipc */ -#endif }; struct comp_perf_data { From c19c8c9034693ba84826e0a02aeb7da35c059b5e Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Thu, 3 Oct 2024 15:37:07 -0700 Subject: [PATCH 3/3] WIP: Example code for downstream type conversion NOT COMPILE TESTED Given we cannot trust our enum field in the IPC as communicator may lie about the component on the other side and the module adapter is obscuring the nature of many of the components we need to move the type casting into the components directly. Here is a breakdown of the general steps to achieve this. 1. report ipc config size for all versions 2. remove type casting in the IPC3 layer 3. on non module adapter components cast the config in a compile time selected shim 4. in the module adapter ipc3 layer, remove type casting again 5. in module adapter components realloc init data and cast over in a compile time enabled shim Signed-off-by: Curtis Malainey --- src/audio/asrc/asrc.c | 25 ++- .../module_adapter/module_adapter_ipc3.c | 61 +------- src/audio/tone.c | 26 +++- src/ipc/ipc3/helper.c | 147 +----------------- 4 files changed, 53 insertions(+), 206 deletions(-) diff --git a/src/audio/asrc/asrc.c b/src/audio/asrc/asrc.c index 055fdab8dfa1..74841dacc2fb 100644 --- a/src/audio/asrc/asrc.c +++ b/src/audio/asrc/asrc.c @@ -860,8 +860,31 @@ static int asrc_reset(struct processing_module *mod) return 0; } +#if CONFIG_IPC_MAJOR_3 +static int asrc_init_shim(struct processing_module *mod) +{ + struct ipc_config_asrc *ipc_asrc = rballoc(0, SOF_MEM_CAPS_RAM, sizeof(*ipc_asrc)); + struct module_data *mod_data = &mod->priv; + const struct sof_ipc_comp_asrc *asrc = + (const struct sof_ipc_comp_asrc *)mod_data->cfg.init_data; + + if (!ipc_asrc) + return -ENOMEM; + // TODO size checks + ipc_asrc->source_rate = asrc->source_rate; + ipc_asrc->sink_rate = asrc->sink_rate; + ipc_asrc->asrc_get_asynchronous_mode = asrc->asrc_verify_params; + ipc_asrc->operation_mode = asrc->operation_mode; + + rfree(mod_data->cfg.init_data); + mod_data->cfg.init_data = ipc_asrc; + mod_data->cfg.data = ipc_asrc; + return asrc_init(mod); +} +#endif + static const struct module_interface asrc_interface = { - .init = asrc_init, + .init = INIT_IPC_SHIM(asrc_init_shim, asrc_init), .prepare = asrc_prepare, .process_audio_stream = asrc_process, .trigger = asrc_trigger, diff --git a/src/audio/module_adapter/module_adapter_ipc3.c b/src/audio/module_adapter/module_adapter_ipc3.c index 765c3e2b82fe..d9e19cf20741 100644 --- a/src/audio/module_adapter/module_adapter_ipc3.c +++ b/src/audio/module_adapter/module_adapter_ipc3.c @@ -38,66 +38,9 @@ int module_adapter_init_data(struct comp_dev *dev, const struct comp_ipc_config *config, const void *spec) { - int ret; - - const unsigned char *data = NULL; - uint32_t size = 0; - - switch (config->type) { - case SOF_COMP_VOLUME: - { - const struct ipc_config_volume *ipc_volume = spec; - - size = sizeof(*ipc_volume); - data = spec; - break; - } - case SOF_COMP_SRC: - { - const struct ipc_config_src *ipc_src = spec; - - size = sizeof(*ipc_src); - data = spec; - break; - } - case SOF_COMP_ASRC: - { - const struct ipc_config_asrc *ipc_asrc = spec; - - size = sizeof(*ipc_asrc); - data = spec; - break; - } - case SOF_COMP_MIXER: - break; - case SOF_COMP_EQ_IIR: - case SOF_COMP_EQ_FIR: - case SOF_COMP_KEYWORD_DETECT: - case SOF_COMP_KPB: - case SOF_COMP_SELECTOR: - case SOF_COMP_DEMUX: - case SOF_COMP_MUX: - case SOF_COMP_DCBLOCK: - case SOF_COMP_SMART_AMP: - case SOF_COMP_MODULE_ADAPTER: - case SOF_COMP_FILEREAD: - case SOF_COMP_FILEWRITE: - case SOF_COMP_NONE: - { - const struct ipc_config_process *ipc_module_adapter = spec; - - size = ipc_module_adapter->size; - data = ipc_module_adapter->data; - break; - } - default: - comp_err(dev, "module_adapter_init_data() unsupported comp type %d", config->type); - return -EINVAL; - } - /* Copy initial config */ - if (size) { - ret = module_load_config(dev, data, size); + if (config->ipc_config_size) { + ret = module_load_config(dev, spec, config->ipc_config_size); if (ret < 0) { comp_err(dev, "module_adapter_init_data() error %d: config loading has failed.", ret); diff --git a/src/audio/tone.c b/src/audio/tone.c index d289c12d5f57..735f4fc69c25 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -706,12 +706,36 @@ static int tone_reset(struct comp_dev *dev) return 0; } +#if CONFIG_IPC_MAJOR_3 +static struct comp_dev *tone_new_shim(const struct comp_driver *drv, + const struct comp_ipc_config *config, + const void *spec) +{ + struct sof_ipc_comp_tone *comp = spec; + struct ipc_config_tone tone; + + // TODO check size of comp against tone + + tone.ampl_mult = tone->ampl_mult; + tone.amplitude = tone->amplitude; + tone.freq_mult = tone->freq_mult; + tone.frequency = tone->frequency; + tone.length = tone->length; + tone.period = tone->period; + tone.ramp_step = tone->ramp_step; + tone.repeats = tone->repeats; + tone.sample_rate = tone->sample_rate; + + return tone_new(drv, config, &tone); +} +#endif + static const struct comp_driver comp_tone = { .type = SOF_COMP_TONE, .uid = SOF_RT_UUID(tone_uuid), .tctx = &tone_tr, .ops = { - .create = tone_new, + .create = INTI_IPC_SHIM(tone_new_shim, tone_new), .free = tone_free, .params = tone_params, #if CONFIG_IPC_MAJOR_3 diff --git a/src/ipc/ipc3/helper.c b/src/ipc/ipc3/helper.c index 678d3306e4b1..f44b3ce12f57 100644 --- a/src/ipc/ipc3/helper.c +++ b/src/ipc/ipc3/helper.c @@ -171,6 +171,7 @@ static void comp_common_builder(struct sof_ipc_comp *comp, config->pipeline_id = comp->pipeline_id; config->proc_domain = COMP_PROCESSING_DOMAIN_LL; config->type = comp->type; + config->ipc_config_size = comp->size; /* buffers dont have the following data */ if (comp->type != SOF_COMP_BUFFER) { @@ -182,144 +183,6 @@ static void comp_common_builder(struct sof_ipc_comp *comp, config->xrun_action = ipc_config->xrun_action; } } -/* - * Stores all the "legacy" init IPC data locally. - */ -union ipc_config_specific { - struct ipc_config_host host; - struct ipc_config_dai dai; - struct ipc_config_volume volume; - struct ipc_config_src src; - struct ipc_config_asrc asrc; - struct ipc_config_tone tone; - struct ipc_config_process process; - struct ipc_comp_file file; -} __attribute__((packed, aligned(4))); - -/* build component specific data */ -static int comp_specific_builder(struct sof_ipc_comp *comp, - union ipc_config_specific *config) -{ -#if CONFIG_LIBRARY - struct sof_ipc_comp_file *file = (struct sof_ipc_comp_file *)comp; -#endif - struct sof_ipc_comp_host *host = (struct sof_ipc_comp_host *)comp; - struct sof_ipc_comp_dai *dai = (struct sof_ipc_comp_dai *)comp; - struct sof_ipc_comp_volume *vol = (struct sof_ipc_comp_volume *)comp; - struct sof_ipc_comp_process *proc = (struct sof_ipc_comp_process *)comp; - struct sof_ipc_comp_src *src = (struct sof_ipc_comp_src *)comp; - struct sof_ipc_comp_asrc *asrc = (struct sof_ipc_comp_asrc *)comp; - struct sof_ipc_comp_tone *tone = (struct sof_ipc_comp_tone *)comp; - - memset(config, 0, sizeof(*config)); - - switch (comp->type) { -#if CONFIG_LIBRARY - /* test bench maps host and DAIs to a file */ - case SOF_COMP_FILEREAD: - case SOF_COMP_FILEWRITE: - if (IPC_TAIL_IS_SIZE_INVALID(*file)) - return -EBADMSG; - - config->file.channels = file->channels; - config->file.fn = file->fn; - config->file.frame_fmt = file->frame_fmt; - config->file.mode = file->mode; - config->file.rate = file->rate; - config->file.direction = file->direction; - - /* For module_adapter_init_data() ipc_module_adapter compatibility */ - config->file.module_header.type = proc->type; - config->file.module_header.size = proc->size; - config->file.module_header.data = (uint8_t *)proc->data - - sizeof(struct ipc_config_process); - break; -#endif - case SOF_COMP_HOST: - case SOF_COMP_SG_HOST: - if (IPC_TAIL_IS_SIZE_INVALID(*host)) - return -EBADMSG; - config->host.direction = host->direction; - config->host.no_irq = host->no_irq; - config->host.dmac_config = host->dmac_config; - break; - case SOF_COMP_DAI: - case SOF_COMP_SG_DAI: - if (IPC_TAIL_IS_SIZE_INVALID(*dai)) - return -EBADMSG; - config->dai.dai_index = dai->dai_index; - config->dai.direction = dai->direction; - config->dai.type = dai->type; - break; - case SOF_COMP_VOLUME: - if (IPC_TAIL_IS_SIZE_INVALID(*vol)) - return -EBADMSG; - config->volume.channels = vol->channels; - config->volume.initial_ramp = vol->initial_ramp; - config->volume.max_value = vol->max_value; - config->volume.min_value = vol->min_value; - config->volume.ramp = vol->ramp; - break; - case SOF_COMP_SRC: - if (IPC_TAIL_IS_SIZE_INVALID(*src)) - return -EBADMSG; - config->src.rate_mask = src->rate_mask; - config->src.sink_rate = src->sink_rate; - config->src.source_rate = src->source_rate; - break; - case SOF_COMP_TONE: - if (IPC_TAIL_IS_SIZE_INVALID(*tone)) - return -EBADMSG; - config->tone.ampl_mult = tone->ampl_mult; - config->tone.amplitude = tone->amplitude; - config->tone.freq_mult = tone->freq_mult; - config->tone.frequency = tone->frequency; - config->tone.length = tone->length; - config->tone.period = tone->period; - config->tone.ramp_step = tone->ramp_step; - config->tone.repeats = tone->repeats; - config->tone.sample_rate = tone->sample_rate; - break; - case SOF_COMP_ASRC: - if (IPC_TAIL_IS_SIZE_INVALID(*asrc)) - return -EBADMSG; - config->asrc.source_rate = asrc->source_rate; - config->asrc.sink_rate = asrc->sink_rate; - config->asrc.asynchronous_mode = asrc->asynchronous_mode; - config->asrc.operation_mode = asrc->operation_mode; - break; - case SOF_COMP_EQ_IIR: - case SOF_COMP_EQ_FIR: - case SOF_COMP_KEYWORD_DETECT: - case SOF_COMP_KPB: - case SOF_COMP_SELECTOR: - case SOF_COMP_DEMUX: - case SOF_COMP_MUX: - case SOF_COMP_DCBLOCK: - case SOF_COMP_SMART_AMP: - case SOF_COMP_MODULE_ADAPTER: - case SOF_COMP_NONE: - if (IPC_TAIL_IS_SIZE_INVALID(*proc)) - return -EBADMSG; - - if (proc->comp.hdr.size + proc->size > SOF_IPC_MSG_MAX_SIZE) - return -EBADMSG; - - config->process.type = proc->type; - config->process.size = proc->size; -#if CONFIG_LIBRARY || UNIT_TEST - config->process.data = proc->data + comp->ext_data_length; -#else - config->process.data = proc->data; -#endif - break; - case SOF_COMP_MIXER: - break; - default: - return -EINVAL; - } - return 0; -} struct ipc_comp_dev *ipc_get_comp_by_ppl_id(struct ipc *ipc, uint16_t type, uint32_t ppl_id, @@ -343,7 +206,6 @@ struct ipc_comp_dev *ipc_get_comp_by_ppl_id(struct ipc *ipc, struct comp_dev *comp_new(struct sof_ipc_comp *comp) { struct comp_ipc_config config; - union ipc_config_specific spec; struct comp_dev *cdev; const struct comp_driver *drv; @@ -361,13 +223,8 @@ struct comp_dev *comp_new(struct sof_ipc_comp *comp) tr_info(&comp_tr, "comp new %pU type %d id %d.%d", drv->tctx->uuid_p, comp->type, comp->pipeline_id, comp->id); - /* build the component */ - if (comp_specific_builder(comp, &spec) < 0) { - comp_cl_err(drv, "comp_new(): component type not recognized"); - return NULL; - } comp_common_builder(comp, &config); - cdev = drv->ops.create(drv, &config, &spec); + cdev = drv->ops.create(drv, &config, comp); if (!cdev) { comp_cl_err(drv, "comp_new(): unable to create the new component"); return NULL;