From 2ed73f7d6176fd7bec0efefaac1b16a8f50111d9 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 5 Dec 2023 08:42:28 -0800 Subject: [PATCH] module: Add support for saving and retrieving the base config extension Modules are bound with one another with a buffer in between. Today, this buffer's size is based on the source module's base config obs and sink module's base config ibs. But this is incorrect because this assumes that the connection is always from output pin 0 of the source module and input pin 0 of the sink module. To determine the buffer size correctly, the buffer size should be based on the pin-based obs/ibs of the source/sink module. To allow this, save the base config extension for the module during init so that it can be retrieved during module binding to get the obs/ibs based on the source/sink pin ID. Modify the google rtc and smart amp modules to save the base config extension during their init. Signed-off-by: Ranjani Sridharan --- .../google/google_rtc_audio_processing.c | 20 ++++ .../module_adapter/module_adapter_ipc4.c | 16 +++ src/include/module/module/base.h | 1 + src/include/sof/audio/component.h | 1 + src/include/sof/audio/component_ext.h | 54 +++++++-- .../sof/audio/module_adapter/module/generic.h | 1 + src/ipc/ipc4/helper.c | 104 ++++++++++++++---- src/samples/audio/smart_amp_test_ipc4.c | 13 +++ 8 files changed, 175 insertions(+), 35 deletions(-) diff --git a/src/audio/google/google_rtc_audio_processing.c b/src/audio/google/google_rtc_audio_processing.c index 3834278ba42e..0003e392189e 100644 --- a/src/audio/google/google_rtc_audio_processing.c +++ b/src/audio/google/google_rtc_audio_processing.c @@ -404,6 +404,21 @@ static int google_rtc_audio_processing_init(struct processing_module *mod) cd->config.reference_fmt = reference_fmt.audio_fmt; cd->config.output_fmt = output_fmt.audio_fmt; + + /* save the base config extension */ + size = base_cfg->base_cfg_ext.nb_input_pins * sizeof(struct ipc4_input_pin_format) + + base_cfg->base_cfg_ext.nb_output_pins * sizeof(struct ipc4_output_pin_format); + + md->cfg.basecfg_ext = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, + sizeof(struct ipc4_base_module_cfg_ext) + size); + if (!md->cfg.basecfg_ext) { + ret = -ENOMEM; + goto fail; + } + + memcpy_s(md->cfg.basecfg_ext, size + sizeof(struct ipc4_base_module_cfg_ext), + &base_cfg->base_cfg_ext, + size + sizeof(struct ipc4_base_module_cfg_ext)); #endif cd->tuning_handler = comp_data_blob_handler_new(dev); @@ -510,6 +525,7 @@ static int google_rtc_audio_processing_init(struct processing_module *mod) rfree(cd->memory_buffer); rfree(cd->raw_mic_buffer); comp_data_blob_handler_free(cd->tuning_handler); + rfree(md->cfg.basecfg_ext); rfree(cd); } @@ -519,6 +535,7 @@ static int google_rtc_audio_processing_init(struct processing_module *mod) static int google_rtc_audio_processing_free(struct processing_module *mod) { struct google_rtc_audio_processing_comp_data *cd = module_get_private_data(mod); + struct module_data *md = &mod->priv; comp_dbg(mod->dev, "google_rtc_audio_processing_free()"); @@ -530,6 +547,9 @@ static int google_rtc_audio_processing_free(struct processing_module *mod) rfree(cd->memory_buffer); rfree(cd->raw_mic_buffer); comp_data_blob_handler_free(cd->tuning_handler); +#if CONFIG_IPC_MAJOR_4 + rfree(md->cfg.basecfg_ext); +#endif rfree(cd); return 0; } diff --git a/src/audio/module_adapter/module_adapter_ipc4.c b/src/audio/module_adapter/module_adapter_ipc4.c index 65ca8e806e3f..029afb2600ee 100644 --- a/src/audio/module_adapter/module_adapter_ipc4.c +++ b/src/audio/module_adapter/module_adapter_ipc4.c @@ -148,6 +148,22 @@ int module_adapter_get_attribute(struct comp_dev *dev, uint32_t type, void *valu memcpy_s(value, sizeof(struct ipc4_base_module_cfg), &mod->priv.cfg.base_cfg, sizeof(mod->priv.cfg.base_cfg)); break; + case COMP_ATTR_BASE_CONFIG_EXT: + { + struct ipc4_base_module_cfg_ext *basecfg_ext = mod->priv.cfg.basecfg_ext; + size_t size; + + if (!basecfg_ext) { + comp_err(mod->dev, "No base config extn set for module"); + return -EINVAL; + } + + size = basecfg_ext->nb_input_pins * sizeof(struct ipc4_input_pin_format) + + basecfg_ext->nb_output_pins * sizeof(struct ipc4_output_pin_format); + memcpy_s(value, sizeof(*basecfg_ext) + size, + basecfg_ext, sizeof(*basecfg_ext) + size); + break; + } default: return -EINVAL; } diff --git a/src/include/module/module/base.h b/src/include/module/module/base.h index 56964162467f..975e42efae3b 100644 --- a/src/include/module/module/base.h +++ b/src/include/module/module/base.h @@ -30,6 +30,7 @@ struct module_config { const void *init_data; /**< Initial IPC configuration. */ #if CONFIG_IPC_MAJOR_4 struct ipc4_base_module_cfg base_cfg; + struct ipc4_base_module_cfg_ext *basecfg_ext; #endif }; diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index 59d5867cae0b..7bcc735dde9b 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -117,6 +117,7 @@ enum { #define COMP_ATTR_COPY_DIR 2 /**< Comp copy direction */ #define COMP_ATTR_VDMA_INDEX 3 /**< Comp index of the virtual DMA at the gateway. */ #define COMP_ATTR_BASE_CONFIG 4 /**< Component base config */ +#define COMP_ATTR_BASE_CONFIG_EXT 5 /**< Component base config extension */ /** @}*/ /** \name Trace macros diff --git a/src/include/sof/audio/component_ext.h b/src/include/sof/audio/component_ext.h index a3d04843b3f9..f32ca2382c20 100644 --- a/src/include/sof/audio/component_ext.h +++ b/src/include/sof/audio/component_ext.h @@ -16,6 +16,7 @@ #ifndef __SOF_AUDIO_COMPONENT_INT_H__ #define __SOF_AUDIO_COMPONENT_INT_H__ +#include #include #include @@ -216,31 +217,60 @@ struct get_attribute_remote_payload { static inline int comp_ipc4_get_attribute_remote(struct comp_dev *dev, uint32_t type, void *value) { - struct ipc4_base_module_cfg *base_cfg; + + struct ipc4_base_module_cfg_ext *basecfg_ext = NULL; + struct ipc4_base_module_cfg *base_cfg = NULL; struct get_attribute_remote_payload payload = {}; struct idc_msg msg = { IDC_MSG_GET_ATTRIBUTE, IDC_EXTENSION(dev->ipc_config.id), dev->ipc_config.core, sizeof(payload), &payload}; + size_t size = MODULE_MAX_SINKS * sizeof(struct ipc4_output_pin_format) + + MODULE_MAX_SOURCES * sizeof(struct ipc4_input_pin_format); int ret; - /* Only COMP_ATTR_BASE_CONFIG is supported for remote access */ - if (type != COMP_ATTR_BASE_CONFIG) - return -EINVAL; + payload.type = type; - base_cfg = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(*base_cfg)); - if (!base_cfg) - return -ENOMEM; + /* + * Only COMP_ATTR_BASE_CONFIG and COMP_ATTR_BASE_CONFIG_EXT are supported for + * remote access + */ + switch (type) { + case COMP_ATTR_BASE_CONFIG: + base_cfg = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, + sizeof(*base_cfg)); + if (!base_cfg) + return -ENOMEM; + + payload.value = base_cfg; + break; + case COMP_ATTR_BASE_CONFIG_EXT: + basecfg_ext = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, + sizeof(*basecfg_ext) + size); + if (!basecfg_ext) + return -ENOMEM; - payload.type = type; - payload.value = base_cfg; + payload.value = basecfg_ext; + break; + default: + return -EINVAL; + } ret = idc_send_msg(&msg, IDC_BLOCKING); - if (ret == 0) - memcpy_s(value, sizeof(struct ipc4_base_module_cfg), - base_cfg, sizeof(struct ipc4_base_module_cfg)); + if (!ret) { + switch (type) { + case COMP_ATTR_BASE_CONFIG: + memcpy_s(value, sizeof(struct ipc4_base_module_cfg), + base_cfg, sizeof(struct ipc4_base_module_cfg)); + break; + case COMP_ATTR_BASE_CONFIG_EXT: + memcpy_s(value, size, basecfg_ext, size); + break; + } + } rfree(base_cfg); + rfree(basecfg_ext); return ret; } #endif /* CONFIG_IPC_MAJOR_4 */ diff --git a/src/include/sof/audio/module_adapter/module/generic.h b/src/include/sof/audio/module_adapter/module/generic.h index 1ab2485fe348..264168522912 100644 --- a/src/include/sof/audio/module_adapter/module/generic.h +++ b/src/include/sof/audio/module_adapter/module/generic.h @@ -36,6 +36,7 @@ #define MAX_BLOB_SIZE 8192 #define MODULE_MAX_SOURCES 8 +#define MODULE_MAX_SINKS 8 #define API_CALL(cd, cmd, sub_cmd, value, ret) \ do { \ diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 4006ce82be8d..7993372a0ad7 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -424,6 +425,7 @@ static int ll_wait_finished_on_core(struct comp_dev *dev) int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) { + struct ipc4_base_module_cfg_ext *src_basecfg_ext, *sink_basecfg_ext; struct ipc4_module_bind_unbind *bu; struct comp_buffer *buffer; struct comp_dev *source; @@ -431,8 +433,11 @@ int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) struct ipc4_base_module_cfg source_src_cfg; struct ipc4_base_module_cfg sink_src_cfg; uint32_t flags; + uint32_t ibs, obs; + uint32_t buf_size; int src_id, sink_id; int ret; + size_t size; bu = (struct ipc4_module_bind_unbind *)_connect; src_id = IPC4_COMP_ID(bu->primary.r.module_id, bu->primary.r.instance_id); @@ -453,32 +458,85 @@ int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) if (!cpu_is_me(source->ipc_config.core) && !cross_core_bind) return ipc4_process_on_core(source->ipc_config.core, false); - /* these might call comp_ipc4_get_attribute_remote() if necessary */ - ret = comp_get_attribute(source, COMP_ATTR_BASE_CONFIG, &source_src_cfg); - if (ret < 0) { - tr_err(&ipc_tr, "failed to get base config for module %#x", dev_comp_id(source)); - return IPC4_FAILURE; - } + size = MODULE_MAX_SINKS * sizeof(struct ipc4_output_pin_format) + + MODULE_MAX_SOURCES * sizeof(struct ipc4_input_pin_format); + + /* get obs from the base config extension if the src queue ID is non-zero */ + if (bu->extension.r.src_queue) { + struct ipc4_output_pin_format out_fmt; + size_t output_fmt_offset; + + src_basecfg_ext = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, + sizeof(*src_basecfg_ext) + size); + if (!src_basecfg_ext) + return IPC4_OUT_OF_MEMORY; + + ret = comp_get_attribute(source, COMP_ATTR_BASE_CONFIG_EXT, src_basecfg_ext); + if (ret < 0) { + tr_err(&ipc_tr, "failed to get base config extn for src module %#x", + dev_comp_id(source)); + rfree(src_basecfg_ext); + return IPC4_FAILURE; + } - ret = comp_get_attribute(sink, COMP_ATTR_BASE_CONFIG, &sink_src_cfg); - if (ret < 0) { - tr_err(&ipc_tr, "failed to get base config for module %#x", dev_comp_id(sink)); - return IPC4_FAILURE; + output_fmt_offset = + src_basecfg_ext->nb_input_pins * sizeof(struct ipc4_input_pin_format) + + bu->extension.r.src_queue * sizeof(struct ipc4_output_pin_format); + + memcpy_s(&out_fmt, sizeof(struct ipc4_output_pin_format), + &src_basecfg_ext->pin_formats[output_fmt_offset], + sizeof(struct ipc4_output_pin_format)); + obs = out_fmt.obs; + rfree(src_basecfg_ext); + } else { + /* these might call comp_ipc4_get_attribute_remote() if necessary */ + ret = comp_get_attribute(source, COMP_ATTR_BASE_CONFIG, &source_src_cfg); + + if (ret < 0) { + tr_err(&ipc_tr, "failed to get base config for src module %#x", + dev_comp_id(source)); + return IPC4_FAILURE; + } + obs = source_src_cfg.obs; } - /* create a buffer - * in case of LL -> LL or LL->DP - * size = 2*obs of source module (obs is single buffer size) - * in case of DP -> LL - * size = 2*ibs of destination (LL) module. DP queue will handle obs of DP module - */ - uint32_t buf_size; + /* get ibs from the base config extension if the sink queue ID is non-zero */ + if (bu->extension.r.dst_queue) { + struct ipc4_input_pin_format in_fmt; + size_t input_fmt_offset; + + sink_basecfg_ext = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, + sizeof(*sink_basecfg_ext) + size); + if (!sink_basecfg_ext) + return IPC4_OUT_OF_MEMORY; + + ret = comp_get_attribute(sink, COMP_ATTR_BASE_CONFIG_EXT, sink_basecfg_ext); + if (ret < 0) { + tr_err(&ipc_tr, "failed to get base config extn for sink module %#x", + dev_comp_id(sink)); + rfree(sink_basecfg_ext); + return IPC4_FAILURE; + } + input_fmt_offset = bu->extension.r.dst_queue * sizeof(struct ipc4_input_pin_format); - if (source->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_LL) - buf_size = source_src_cfg.obs * 2; - else - buf_size = sink_src_cfg.ibs * 2; + memcpy_s(&in_fmt, sizeof(struct ipc4_input_pin_format), + &sink_basecfg_ext->pin_formats[input_fmt_offset], + sizeof(struct ipc4_input_pin_format)); + ibs = in_fmt.ibs; + rfree(sink_basecfg_ext); + } else { + ret = comp_get_attribute(sink, COMP_ATTR_BASE_CONFIG, &sink_src_cfg); + if (ret < 0) { + tr_err(&ipc_tr, "failed to get base config for sink module %#x", + dev_comp_id(sink)); + return IPC4_FAILURE; + } + + ibs = sink_src_cfg.ibs; + } + /* allocate buffer with size large enough to fit ibs of the sink or obs of the source */ + buf_size = MAX(ibs * 2, obs * 2); buffer = ipc4_create_buffer(source, cross_core_bind, buf_size, bu->extension.r.src_queue, bu->extension.r.dst_queue); if (!buffer) { @@ -496,8 +554,8 @@ int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) * sink_module needs to set its IBS (input buffer size) * as min_available in buffer's source ifc */ - sink_set_min_free_space(audio_stream_get_sink(&buffer->stream), source_src_cfg.obs); - source_set_min_available(audio_stream_get_source(&buffer->stream), sink_src_cfg.ibs); + sink_set_min_free_space(audio_stream_get_sink(&buffer->stream), obs); + source_set_min_available(audio_stream_get_source(&buffer->stream), ibs); /* * Connect and bind the buffer to both source and sink components with LL processing been diff --git a/src/samples/audio/smart_amp_test_ipc4.c b/src/samples/audio/smart_amp_test_ipc4.c index d09a8f9a9720..159944d3ef1f 100644 --- a/src/samples/audio/smart_amp_test_ipc4.c +++ b/src/samples/audio/smart_amp_test_ipc4.c @@ -48,6 +48,7 @@ static int smart_amp_init(struct processing_module *mod) const size_t out_size = sizeof(struct ipc4_output_pin_format) * SMART_AMP_NUM_OUT_PINS; int ret; const struct ipc4_base_module_extended_cfg *base_cfg = mod_data->cfg.init_data; + size_t size; comp_dbg(dev, "smart_amp_init()"); sad = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, sizeof(*sad)); @@ -78,6 +79,16 @@ static int smart_amp_init(struct processing_module *mod) mod->max_sources = SMART_AMP_NUM_IN_PINS; + /* save the base config extension */ + size = sizeof(struct ipc4_base_module_cfg_ext) + in_size + out_size; + mod_data->cfg.basecfg_ext = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, size); + if (!mod_data->cfg.basecfg_ext) { + ret = -ENOMEM; + goto sad_fail; + } + + memcpy_s(mod_data->cfg.basecfg_ext, size, &base_cfg->base_cfg_ext, size); + return 0; sad_fail: @@ -163,10 +174,12 @@ static inline int smart_amp_get_config(struct processing_module *mod, static int smart_amp_free(struct processing_module *mod) { struct smart_amp_data *sad = module_get_private_data(mod); + struct module_data *mod_data = &mod->priv; struct comp_dev *dev = mod->dev; comp_dbg(dev, "smart_amp_free()"); comp_data_blob_handler_free(sad->model_handler); + rfree(mod_data->cfg.basecfg_ext); rfree(sad); return 0; }