From ddc8a04218670f0acb202f1fd8415d81f3d1dce9 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 6 Jan 2022 09:57:52 -0800 Subject: [PATCH 1/5] codec_adapter: drop the type parameter in codec_adapter_set_params() It can be accessed via cdata in the function. Signed-off-by: Ranjani Sridharan --- src/audio/codec_adapter/codec_adapter.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/audio/codec_adapter/codec_adapter.c b/src/audio/codec_adapter/codec_adapter.c index 333f18598e59..504bfe17ebe2 100644 --- a/src/audio/codec_adapter/codec_adapter.c +++ b/src/audio/codec_adapter/codec_adapter.c @@ -461,8 +461,7 @@ int codec_adapter_copy(struct comp_dev *dev) return ret; } -static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata, - enum module_cfg_type type) +static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { int ret; char *dst, *src; @@ -470,6 +469,7 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da uint32_t offset; struct processing_module *mod = comp_get_drvdata(dev); struct module_data *md = &mod->priv; + enum module_cfg_type type = cdata->data->type; comp_dbg(dev, "codec_adapter_set_params(): start: num_of_elem %d, elem remain %d msg_index %u", cdata->num_elems, cdata->elems_remaining, cdata->msg_index); @@ -588,7 +588,7 @@ static int ca_set_binary_data(struct comp_dev *dev, switch (cdata->data->type) { case MODULE_CFG_SETUP: case MODULE_CFG_RUNTIME: - ret = codec_adapter_set_params(dev, cdata, cdata->data->type); + ret = codec_adapter_set_params(dev, cdata); break; default: comp_err(dev, "ca_set_binary_data() error: unknown binary data type"); From 92d6d7aafdb7242b0c6284bf6fe06410d75f8320 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 6 Jan 2022 10:30:06 -0800 Subject: [PATCH 2/5] codec_adapter: remove unnecessary goto's Remove the goto's and return the appropriate values to simplify the codec_adapter_set_params() function. Signed-off-by: Ranjani Sridharan --- src/audio/codec_adapter/codec_adapter.c | 44 ++++++++++++------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/audio/codec_adapter/codec_adapter.c b/src/audio/codec_adapter/codec_adapter.c index 504bfe17ebe2..17f721c64181 100644 --- a/src/audio/codec_adapter/codec_adapter.c +++ b/src/audio/codec_adapter/codec_adapter.c @@ -480,38 +480,38 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da /* Check that there is no work-in-progress on previous request */ if (md->runtime_params) { comp_err(dev, "codec_adapter_set_params() error: busy with previous request"); - ret = -EBUSY; - goto end; - } else if (!size) { + return -EBUSY; + } + + if (!size) { comp_err(dev, "codec_adapter_set_params() error: no configuration size %d", size); /* TODO: return -EINVAL. This is temporary until driver fixes its issue */ - ret = 0; - goto end; - } else if (size > MAX_BLOB_SIZE) { + return 0; + } + + if (size > MAX_BLOB_SIZE) { comp_err(dev, "codec_adapter_set_params() error: blob size is too big cfg size %d, allowed %d", size, MAX_BLOB_SIZE); - ret = -EINVAL; - goto end; - } else if (type != MODULE_CFG_SETUP && type != MODULE_CFG_RUNTIME) { + return -EINVAL; + } + + if (type != MODULE_CFG_SETUP && type != MODULE_CFG_RUNTIME) { comp_err(dev, "codec_adapter_set_params() error: unknown config type"); - ret = -EINVAL; - goto end; + return -EINVAL; } /* Allocate buffer for new params */ md->runtime_params = rballoc(0, SOF_MEM_CAPS_RAM, size); if (!md->runtime_params) { comp_err(dev, "codec_adapter_set_params(): space allocation for new params failed"); - ret = -ENOMEM; - goto end; + return -ENOMEM; } memset(md->runtime_params, 0, size); } else if (!md->runtime_params) { comp_err(dev, "codec_adapter_set_params() error: no memory available for runtime params in consecutive load"); - ret = -EIO; - goto end; + return -EIO; } offset = size - (cdata->num_elems + cdata->elems_remaining); @@ -541,11 +541,11 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da if (ret) { comp_err(dev, "codec_adapter_set_params() error %d: load of runtime config failed.", ret); - goto done; - } else { - comp_dbg(dev, "codec_adapter_set_params() load of runtime config done."); + break; } + comp_dbg(dev, "codec_adapter_set_params() load of runtime config done."); + if (md->state >= MODULE_INITIALIZED) { /* We are already prepared so we can apply runtime * config right away. @@ -566,16 +566,14 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da comp_err(dev, "codec_adapter_set_params(): error: unknown config type."); break; } - } else - goto end; + } else { + return 0; + } -done: if (md->runtime_params) rfree(md->runtime_params); md->runtime_params = NULL; return ret; -end: - return ret; } static int ca_set_binary_data(struct comp_dev *dev, From 879e8fafa00f58c82d04cc73a37ac61923c07772 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 6 Jan 2022 10:31:43 -0800 Subject: [PATCH 3/5] codec_adapter: remove unnecessary braces Remove the unnecessary braces in codec_adapter_set_params(). Signed-off-by: Ranjani Sridharan --- src/audio/codec_adapter/codec_adapter.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/audio/codec_adapter/codec_adapter.c b/src/audio/codec_adapter/codec_adapter.c index 17f721c64181..d1c49e177573 100644 --- a/src/audio/codec_adapter/codec_adapter.c +++ b/src/audio/codec_adapter/codec_adapter.c @@ -528,12 +528,11 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da switch (type) { case MODULE_CFG_SETUP: ret = load_setup_config(dev, md->runtime_params, size); - if (ret) { + if (ret) comp_err(dev, "codec_adapter_set_params(): error %d: load of setup config failed.", ret); - } else { + else comp_dbg(dev, "codec_adapter_set_params() load of setup config done."); - } break; case MODULE_CFG_RUNTIME: @@ -551,12 +550,11 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da * config right away. */ ret = module_apply_runtime_config(dev); - if (ret) { + if (ret) comp_err(dev, "codec_adapter_set_params() error %x: codec runtime config apply failed", ret); - } else { + else comp_dbg(dev, "codec_adapter_set_params() apply of runtime config done."); - } } else { mod->priv.r_cfg.avail = true; } From 91bf06454c3313cc57f23c76f3f4f3a59aa9f038 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 6 Jan 2022 10:21:28 -0800 Subject: [PATCH 4/5] codec_adapter: check if more fragments are expected first. Check if more fragments of config data are expected first and return immediately to make the code easier to follow. Signed-off-by: Ranjani Sridharan --- src/audio/codec_adapter/codec_adapter.c | 76 ++++++++++++------------- 1 file changed, 35 insertions(+), 41 deletions(-) diff --git a/src/audio/codec_adapter/codec_adapter.c b/src/audio/codec_adapter/codec_adapter.c index d1c49e177573..811b4b75b739 100644 --- a/src/audio/codec_adapter/codec_adapter.c +++ b/src/audio/codec_adapter/codec_adapter.c @@ -521,51 +521,45 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da ret = memcpy_s(dst, size - offset, src, cdata->num_elems); assert(!ret); - /* Config has been copied now we can load & apply it depending on - * codec state. - */ - if (!cdata->elems_remaining) { - switch (type) { - case MODULE_CFG_SETUP: - ret = load_setup_config(dev, md->runtime_params, size); - if (ret) - comp_err(dev, "codec_adapter_set_params(): error %d: load of setup config failed.", - ret); - else - comp_dbg(dev, "codec_adapter_set_params() load of setup config done."); + /* return as more fragments of config data expected */ + if (cdata->elems_remaining) + return 0; + /* config fully copied, now load it */ + switch (type) { + case MODULE_CFG_SETUP: + ret = load_setup_config(dev, md->runtime_params, size); + if (ret) + comp_err(dev, "codec_adapter_set_params(): error %d: load of setup config failed.", + ret); + else + comp_dbg(dev, "codec_adapter_set_params() load of setup config done."); + break; + case MODULE_CFG_RUNTIME: + ret = module_load_config(dev, md->runtime_params, size, MODULE_CFG_RUNTIME); + if (ret) { + comp_err(dev, "codec_adapter_set_params() error %d: load of runtime config failed.", + ret); break; - case MODULE_CFG_RUNTIME: - ret = module_load_config(dev, md->runtime_params, size, MODULE_CFG_RUNTIME); - if (ret) { - comp_err(dev, "codec_adapter_set_params() error %d: load of runtime config failed.", - ret); - break; - } - - comp_dbg(dev, "codec_adapter_set_params() load of runtime config done."); - - if (md->state >= MODULE_INITIALIZED) { - /* We are already prepared so we can apply runtime - * config right away. - */ - ret = module_apply_runtime_config(dev); - if (ret) - comp_err(dev, "codec_adapter_set_params() error %x: codec runtime config apply failed", - ret); - else - comp_dbg(dev, "codec_adapter_set_params() apply of runtime config done."); - } else { - mod->priv.r_cfg.avail = true; - } + } - break; - default: - comp_err(dev, "codec_adapter_set_params(): error: unknown config type."); - break; + comp_dbg(dev, "codec_adapter_set_params() load of runtime config done."); + + /* And apply it right away if codec is already prepared */ + if (md->state >= MODULE_INITIALIZED) { + ret = module_apply_runtime_config(dev); + if (ret) + comp_err(dev, "codec_adapter_set_params() error %x: codec runtime config apply failed", + ret); + else + comp_dbg(dev, "codec_adapter_set_params() apply of runtime config done."); + } else { + mod->priv.r_cfg.avail = true; } - } else { - return 0; + break; + default: + comp_err(dev, "codec_adapter_set_params(): error: unknown config type."); + break; } if (md->runtime_params) From 2d39351863ea19057c57ed9321f27267cb437286 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 10 Jan 2022 08:58:25 -0800 Subject: [PATCH 5/5] codec_adapter: remove load_setup_config() Modify module_load_config() to handle both setup and runtime configs and remove load_setup_config(). Signed-off-by: Ranjani Sridharan --- src/audio/codec_adapter/codec/generic.c | 18 ++++ src/audio/codec_adapter/codec_adapter.c | 93 ++----------------- .../sof/audio/codec_adapter/codec_adapter.h | 4 - 3 files changed, 27 insertions(+), 88 deletions(-) diff --git a/src/audio/codec_adapter/codec/generic.c b/src/audio/codec_adapter/codec/generic.c index aa5cc81c2a4c..30ca8af69a8a 100644 --- a/src/audio/codec_adapter/codec/generic.c +++ b/src/audio/codec_adapter/codec/generic.c @@ -33,6 +33,24 @@ int module_load_config(struct comp_dev *dev, void *cfg, size_t size, enum module return -EINVAL; } + /* + * Setup config comprises of two parts: 1. Essential data needed for initialization of + * codec_adapter and follows struct ca_config. 2: Module specific data needed to setup the + * module. Copy the codec_adapter data first. + */ + if (type == MODULE_CFG_SETUP) { + ret = memcpy_s(&mod->ca_config, sizeof(mod->ca_config), cfg, + sizeof(struct ca_config)); + assert(!ret); + + /* And then copy the module-specific data */ + cfg = (char *)cfg + sizeof(struct ca_config); + size -= sizeof(struct ca_config); + + if (!size) + return 0; + } + dst = (type == MODULE_CFG_SETUP) ? &md->s_cfg : &md->r_cfg; diff --git a/src/audio/codec_adapter/codec_adapter.c b/src/audio/codec_adapter/codec_adapter.c index 811b4b75b739..57c87b74f71e 100644 --- a/src/audio/codec_adapter/codec_adapter.c +++ b/src/audio/codec_adapter/codec_adapter.c @@ -23,7 +23,6 @@ #include #include -int load_setup_config(struct comp_dev *dev, void *cfg, uint32_t size); int validate_setup_config(struct ca_config *cfg); /** @@ -69,7 +68,8 @@ struct comp_dev *codec_adapter_new(const struct comp_driver *drv, comp_set_drvdata(dev, mod); /* Copy setup config */ - ret = load_setup_config(dev, ipc_codec_adapter->data, ipc_codec_adapter->size); + ret = module_load_config(dev, ipc_codec_adapter->data, ipc_codec_adapter->size, + MODULE_CFG_SETUP); if (ret) { comp_err(dev, "codec_adapter_new() error %d: config loading has failed.", ret); @@ -99,66 +99,6 @@ int validate_setup_config(struct ca_config *cfg) return 0; } -/** - * \brief Load setup config for both codec adapter and codec library. - * \param[in] dev - codec adapter component device pointer. - * \param[in] cfg - pointer to the configuration data. - * \param[in] size - size of config. - * - * The setup config comprises of two parts - one contains essential data - * for the initialization of codec_adapter and follows struct ca_config. - * Second contains codec specific data needed to setup the codec itself. - * The latter is send in a TLV format organized by struct module_param. - * - * \return integer representing either: - * 0 -> success - * negative value -> failure. - */ -int load_setup_config(struct comp_dev *dev, void *cfg, uint32_t size) -{ - int ret; - void *lib_cfg; - size_t lib_cfg_size; - struct processing_module *mod = comp_get_drvdata(dev); - - comp_dbg(dev, "load_setup_config() start."); - - if (!cfg || !size) { - comp_err(dev, "load_setup_config(): no config available cfg: %x, size: %d", - (uintptr_t)cfg, size); - ret = -EINVAL; - goto end; - } else if (size < sizeof(struct ca_config)) { - comp_err(dev, "load_setup_config(): no codec config available, size %d", size); - ret = -EIO; - goto end; - } - /* Copy codec_adapter part */ - ret = memcpy_s(&mod->ca_config, sizeof(mod->ca_config), cfg, - sizeof(struct ca_config)); - assert(!ret); - ret = validate_setup_config(&mod->ca_config); - if (ret) { - comp_err(dev, "load_setup_config(): validation of setup config for codec_adapter failed."); - goto end; - } - /* Copy codec specific part */ - lib_cfg_size = size - sizeof(struct ca_config); - if (lib_cfg_size) { - lib_cfg = (char *)cfg + sizeof(struct ca_config); - ret = module_load_config(dev, lib_cfg, lib_cfg_size, MODULE_CFG_SETUP); - if (ret) { - comp_err(dev, "load_setup_config(): %d: failed to load setup config for codec id %x", - ret, mod->ca_config.module_id); - goto end; - } - } - - comp_dbg(dev, "load_setup_config() done."); -end: - return ret; -} - /* * \brief Prepare a codec adapter component. * \param[in] dev - component device pointer. @@ -526,26 +466,14 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da return 0; /* config fully copied, now load it */ - switch (type) { - case MODULE_CFG_SETUP: - ret = load_setup_config(dev, md->runtime_params, size); - if (ret) - comp_err(dev, "codec_adapter_set_params(): error %d: load of setup config failed.", - ret); - else - comp_dbg(dev, "codec_adapter_set_params() load of setup config done."); - break; - case MODULE_CFG_RUNTIME: - ret = module_load_config(dev, md->runtime_params, size, MODULE_CFG_RUNTIME); - if (ret) { - comp_err(dev, "codec_adapter_set_params() error %d: load of runtime config failed.", - ret); - break; - } + ret = module_load_config(dev, md->runtime_params, size, type); + if (ret) + goto end; - comp_dbg(dev, "codec_adapter_set_params() load of runtime config done."); + comp_dbg(dev, "codec_adapter_set_params(): config load successful"); - /* And apply it right away if codec is already prepared */ + /* And apply runtime config right away if codec is already prepared */ + if (type == MODULE_CFG_RUNTIME) { if (md->state >= MODULE_INITIALIZED) { ret = module_apply_runtime_config(dev); if (ret) @@ -556,12 +484,9 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da } else { mod->priv.r_cfg.avail = true; } - break; - default: - comp_err(dev, "codec_adapter_set_params(): error: unknown config type."); - break; } +end: if (md->runtime_params) rfree(md->runtime_params); md->runtime_params = NULL; diff --git a/src/include/sof/audio/codec_adapter/codec_adapter.h b/src/include/sof/audio/codec_adapter/codec_adapter.h index f648c1818858..7385ae3422e3 100644 --- a/src/include/sof/audio/codec_adapter/codec_adapter.h +++ b/src/include/sof/audio/codec_adapter/codec_adapter.h @@ -11,9 +11,5 @@ #include #define MAX_BLOB_SIZE 8192 -/*****************************************************************************/ -/* Codec adapter internal functions */ -/*****************************************************************************/ -static int load_setup_config(struct comp_dev *dev, void *cfg, uint32_t size); #endif /* __SOF_AUDIO_CODEC_ADAPTER__ */