From 027ed8870d4bff10280dea5cfbb482f35645fc32 Mon Sep 17 00:00:00 2001 From: Wojciech Jablonski Date: Mon, 15 Dec 2025 17:05:15 +0100 Subject: [PATCH] audio: copier: Refactor deep copy of copier config Code for copying copier_data is ambiguous. A comment suggests that the deep copy (data structure + variable-length array) is not needed but the copy of the variable-length array is performed afterwards. This change rectifies this by placing ipc4_copier_module_cfg at the end of copier_data structure as it contains the variable-length array. This enables single memory allocation and single 1:1 copy for the complete copier_data with the variable-length addition. Signed-off-by: Wojciech Jablonski --- src/audio/copier/copier.c | 49 +++++++++------------------------- src/audio/copier/copier.h | 9 ++----- src/audio/copier/copier_dai.c | 12 ++++----- src/audio/copier/copier_gain.c | 8 ++---- src/ipc/ipc4/dai.c | 8 ++---- zephyr/lib/cpu.c | 3 ++- 6 files changed, 26 insertions(+), 63 deletions(-) diff --git a/src/audio/copier/copier.c b/src/audio/copier/copier.c index b946db5a749d..dc4d23335e01 100644 --- a/src/audio/copier/copier.c +++ b/src/audio/copier/copier.c @@ -136,48 +136,27 @@ __cold static int copier_init(struct processing_module *mod) struct comp_dev *dev = mod->dev; struct module_data *md = &mod->priv; struct ipc4_copier_module_cfg *copier = (struct ipc4_copier_module_cfg *)md->cfg.init_data; - void *gtw_cfg = NULL; - size_t gtw_cfg_size; + size_t cfg_total_size = sizeof(*copier); + size_t gtw_cfg_var_size = 0; int i, ret = 0; assert_can_be_cold(); - cd = mod_zalloc(mod, sizeof(*cd)); + if (copier->gtw_cfg.config_length > 1) { + /* one word already included in gateway_cfg struct hence subtraction */ + gtw_cfg_var_size += (copier->gtw_cfg.config_length - 1) << 2; + cfg_total_size += gtw_cfg_var_size; + } + + cd = mod_zalloc(mod, sizeof(*cd) + gtw_cfg_var_size); if (!cd) return -ENOMEM; md->private = cd; - /* - * Don't copy the config_data[] variable size array, we don't need to - * store it, it's only used during IPC processing, besides we haven't - * allocated space for it, so don't "fix" this! - */ - if (memcpy_s(&cd->config, sizeof(cd->config), copier, sizeof(*copier)) < 0) { - ret = -EINVAL; - goto error_cd; - } - /* Allocate memory and store gateway_cfg in runtime. Gateway cfg has to - * be kept even after copier is created e.g. during SET_PIPELINE_STATE - * IPC when dai_config_dma_channel() is called second time and DMA - * config is used to assign dma_channel_id value. - */ - if (copier->gtw_cfg.config_length) { - gtw_cfg_size = copier->gtw_cfg.config_length << 2; - gtw_cfg = mod_alloc(mod, gtw_cfg_size); - if (!gtw_cfg) { - ret = -ENOMEM; - goto error_cd; - } - - ret = memcpy_s(gtw_cfg, gtw_cfg_size, &copier->gtw_cfg.config_data, - gtw_cfg_size); - if (ret) { - comp_err(dev, "Unable to copy gateway config from copier blob"); - goto error; - } - - cd->gtw_cfg = gtw_cfg; + if (memcpy_s(&cd->config, cfg_total_size, copier, cfg_total_size) < 0) { + ret = -EINVAL; + goto error; } for (i = 0; i < IPC4_COPIER_MODULE_OUTPUT_PINS_COUNT; i++) @@ -257,8 +236,6 @@ __cold static int copier_init(struct processing_module *mod) dev->state = COMP_STATE_READY; return 0; error: - mod_free(mod, gtw_cfg); -error_cd: mod_free(mod, cd); return ret; } @@ -289,8 +266,6 @@ __cold static int copier_free(struct processing_module *mod) break; } - if (cd) - mod_free(mod, cd->gtw_cfg); mod_free(mod, cd); return 0; diff --git a/src/audio/copier/copier.h b/src/audio/copier/copier.h index 4a6091d99dca..4e29e18d33a6 100644 --- a/src/audio/copier/copier.h +++ b/src/audio/copier/copier.h @@ -241,13 +241,6 @@ struct ipc4_data_segment_enabled { } __attribute__((packed, aligned(4))); struct copier_data { - /* - * struct ipc4_copier_module_cfg actually has variable size, but we - * don't need the variable size array at the end, we won't be copying it - * from the IPC data. - */ - struct ipc4_copier_module_cfg config; - void *gtw_cfg; enum ipc4_gateway_type gtw_type; uint32_t endpoint_num; @@ -277,6 +270,8 @@ struct copier_data { #if CONFIG_INTEL_ADSP_MIC_PRIVACY struct mic_privacy_data *mic_priv; #endif + /* Has to be at the end due to variable size array */ + struct ipc4_copier_module_cfg config; }; int apply_attenuation(struct comp_dev *dev, struct copier_data *cd, diff --git a/src/audio/copier/copier_dai.c b/src/audio/copier/copier_dai.c index 430a080ae65e..821aa59d7640 100644 --- a/src/audio/copier/copier_dai.c +++ b/src/audio/copier/copier_dai.c @@ -270,6 +270,8 @@ __cold int copier_dai_create(struct comp_dev *dev, struct copier_data *cd, struct ipc_config_dai dai; int dai_count; int i, ret; + uint8_t *gtw_cfg_data = (uint8_t *)cd->config.gtw_cfg.config_data; + size_t gtw_cfg_szie = cd->config.gtw_cfg.config_length * 4; assert_can_be_cold(); @@ -296,8 +298,7 @@ __cold int copier_dai_create(struct comp_dev *dev, struct copier_data *cd, dai.type = SOF_DAI_INTEL_SSP; dai.is_config_blob = true; cd->gtw_type = ipc4_gtw_ssp; - ret = ipc4_find_dma_config(&dai, (uint8_t *)cd->gtw_cfg, - copier->gtw_cfg.config_length * 4); + ret = ipc4_find_dma_config(&dai, gtw_cfg_data, gtw_cfg_szie); if (ret != 0) { comp_err(dev, "No ssp dma_config found in blob!"); return -EINVAL; @@ -315,8 +316,8 @@ __cold int copier_dai_create(struct comp_dev *dev, struct copier_data *cd, dai.is_config_blob = true; cd->gtw_type = ipc4_gtw_alh; #endif /* ACE_VERSION > ACE_VERSION_1_5 */ - ret = copier_alh_assign_dai_index(dev, cd->gtw_cfg, node_id, - &dai, dai_index, &dai_count); + ret = copier_alh_assign_dai_index(dev, gtw_cfg_data, node_id, &dai, dai_index, + &dai_count); if (ret) return ret; break; @@ -324,8 +325,7 @@ __cold int copier_dai_create(struct comp_dev *dev, struct copier_data *cd, dai.type = SOF_DAI_INTEL_DMIC; dai.is_config_blob = true; cd->gtw_type = ipc4_gtw_dmic; - ret = ipc4_find_dma_config(&dai, (uint8_t *)cd->gtw_cfg, - copier->gtw_cfg.config_length * 4); + ret = ipc4_find_dma_config(&dai, gtw_cfg_data, gtw_cfg_szie); if (ret != 0) { comp_err(dev, "No dmic dma_config found in blob!"); return -EINVAL; diff --git a/src/audio/copier/copier_gain.c b/src/audio/copier/copier_gain.c index 4c077665e7d0..fc9f6664add8 100644 --- a/src/audio/copier/copier_gain.c +++ b/src/audio/copier/copier_gain.c @@ -34,12 +34,8 @@ __cold int copier_gain_set_params(struct comp_dev *dev, struct copier_gain_param switch (dai_type) { case SOF_DAI_INTEL_DMIC: { - struct dmic_config_data *dmic_cfg = cd->gtw_cfg; - - if (!dmic_cfg) { - comp_err(dev, "No dmic config found"); - return -EINVAL; - } + struct dmic_config_data *dmic_cfg = + (void *)cd->config.gtw_cfg.config_data; union dmic_global_cfg *dmic_glb_cfg = &dmic_cfg->dmic_blob.global_cfg; diff --git a/src/ipc/ipc4/dai.c b/src/ipc/ipc4/dai.c index d736014aabf1..e54a4c694a10 100644 --- a/src/ipc/ipc4/dai.c +++ b/src/ipc/ipc4/dai.c @@ -91,13 +91,9 @@ int dai_config_dma_channel(struct dai_data *dd, struct comp_dev *dev, const void struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); - if (!cd->gtw_cfg) { - comp_err(dev, "No gateway config found!"); - return SOF_DMA_CHAN_INVALID; - } - channel = SOF_DMA_CHAN_INVALID; - const struct sof_alh_configuration_blob *alh_blob = cd->gtw_cfg; + const struct sof_alh_configuration_blob *alh_blob = + (void *)cd->config.gtw_cfg.config_data; for (int i = 0; i < alh_blob->alh_cfg.count; i++) { if (dai->host_dma_config[i]->stream_id == dai->dai_index) { diff --git a/zephyr/lib/cpu.c b/zephyr/lib/cpu.c index 1f242893ebba..93f87e32538e 100644 --- a/zephyr/lib/cpu.c +++ b/zephyr/lib/cpu.c @@ -122,7 +122,8 @@ static void resume_dais(void) if (dai_probe(dd->dai->dev) < 0) { tr_err(&zephyr_tr, "DAI resume failed on probe, type %d index %d", dd->dai->type, dd->dai->index); - } else if (dai_set_config(dd->dai, &dd->ipc_config, cd->gtw_cfg, + } else if (dai_set_config(dd->dai, &dd->ipc_config, + cd->config.gtw_cfg.config_data, cd->config.gtw_cfg.config_length) < 0) { tr_err(&zephyr_tr, "DAI resume failed on config, type %d index %d", dd->dai->type, dd->dai->index);