From de5bfbf01b9a8cdfc79ebfbc4b2335d2572916bf Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Tue, 4 Jun 2024 12:54:48 -0700 Subject: [PATCH 1/5] module_adapter: Remove modules_shim_new() This is dead code: just a wrapper around module_adapter_new(), which is a public API already in use. Presumably a forgotten relic. Remove. Signed-off-by: Andy Ross --- src/audio/module_adapter/module/modules.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/audio/module_adapter/module/modules.c b/src/audio/module_adapter/module/modules.c index c25f8f7d9cb4..a848ea2fa1c6 100644 --- a/src/audio/module_adapter/module/modules.c +++ b/src/audio/module_adapter/module/modules.c @@ -245,23 +245,3 @@ const struct module_interface processing_module_adapter_interface = { .reset = modules_reset, .free = modules_free, }; - -/** - * \brief Create a module adapter component. - * \param[in] drv - component driver pointer. - * \param[in] config - component ipc descriptor pointer. - * \param[in] spec - pointer to module configuration data - * - * \return: a pointer to newly created module adapter component on success. NULL on error. - * - * \note: For dynamically loaded module the spec size is not known by base FW, since this is - * loaded module specific information. Therefore configuration size is required here. - * New module details are discovered during its loading, therefore comp_driver initialisation - * happens at this point. - */ -struct comp_dev *modules_shim_new(const struct comp_driver *drv, - const struct comp_ipc_config *config, - const void *spec) -{ - return module_adapter_new(drv, config, spec); -} From 25c620693293e8ad845da03cd53b7fc16aceab19 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Wed, 5 Jun 2024 05:18:58 -0700 Subject: [PATCH 2/5] comp_dev: Eliminate use of drvdata for module backpointer The current SOF architecture splits a "module" across two structs, the legacy comp_dev and a newer struct processing_module. The latter contains a pointer to the former, but in many places existing code has needed a backpointer to recover the module from a component. So far this has abused the drvdata mechanism to store this as a void*, but that's fragile as many components are already (!) using drvdata for other purposes and will clobber the setting. The fact that it worked is seeming just by luck. That pointer is for the exclusive use of the comp_driver code associated with a component to store its own data. We can't be touching it from the global module code. Just give the pointer a properly-typed field of its own and make sure the two are initialized in tandem. Longer term, SOF really needs to fix this bifurcation and unify the two structs. Signed-off-by: Andy Ross --- src/audio/asrc/asrc_ipc4.c | 8 ++-- src/audio/copier/copier.c | 20 ++++---- src/audio/copier/copier_dai.c | 8 ++-- src/audio/copier/copier_host.c | 6 +-- src/audio/eq_fir/eq_fir.c | 2 +- src/audio/mixin_mixout/mixin_mixout.c | 2 +- src/audio/module_adapter/module/generic.c | 2 +- src/audio/module_adapter/module_adapter.c | 47 ++++++++++--------- .../module_adapter/module_adapter_ipc3.c | 8 ++-- .../module_adapter/module_adapter_ipc4.c | 16 +++---- src/audio/pipeline/pipeline-schedule.c | 2 +- src/audio/pipeline/pipeline-stream.c | 4 +- src/audio/src/src_ipc3.c | 2 +- src/audio/src/src_ipc4.c | 2 +- src/audio/volume/volume.h | 2 +- src/include/sof/audio/component.h | 11 +++++ src/ipc/ipc4/dai.c | 2 +- src/ipc/ipc4/helper.c | 4 +- src/library_manager/lib_manager.c | 4 +- test/cmocka/src/audio/eq_fir/eq_fir_process.c | 18 +++---- test/cmocka/src/audio/eq_iir/eq_iir_process.c | 18 +++---- test/cmocka/src/audio/module_adapter_test.c | 2 +- test/cmocka/src/audio/mux/demux_copy.c | 2 +- test/cmocka/src/audio/mux/mux_copy.c | 2 +- .../audio/mux/mux_get_processing_function.c | 2 +- 25 files changed, 104 insertions(+), 92 deletions(-) diff --git a/src/audio/asrc/asrc_ipc4.c b/src/audio/asrc/asrc_ipc4.c index b0630865614f..4f645a322e67 100644 --- a/src/audio/asrc/asrc_ipc4.c +++ b/src/audio/asrc/asrc_ipc4.c @@ -27,7 +27,7 @@ int asrc_dai_configure_timestamp(struct comp_data *cd) if (!cd->dai_dev) return -ENODEV; - struct processing_module *mod = comp_get_drvdata(cd->dai_dev); + struct processing_module *mod = comp_mod(cd->dai_dev); const struct module_interface *const ops = mod->dev->drv->adapter_ops; return ops->endpoint_ops->dai_ts_config(cd->dai_dev); @@ -38,7 +38,7 @@ int asrc_dai_start_timestamp(struct comp_data *cd) if (!cd->dai_dev) return -ENODEV; - struct processing_module *mod = comp_get_drvdata(cd->dai_dev); + struct processing_module *mod = comp_mod(cd->dai_dev); const struct module_interface *const ops = mod->dev->drv->adapter_ops; return ops->endpoint_ops->dai_ts_start(cd->dai_dev); @@ -49,7 +49,7 @@ int asrc_dai_stop_timestamp(struct comp_data *cd) if (!cd->dai_dev) return -ENODEV; - struct processing_module *mod = comp_get_drvdata(cd->dai_dev); + struct processing_module *mod = comp_mod(cd->dai_dev); const struct module_interface *const ops = mod->dev->drv->adapter_ops; return ops->endpoint_ops->dai_ts_stop(cd->dai_dev); @@ -64,7 +64,7 @@ int asrc_dai_get_timestamp(struct comp_data *cd, struct timestamp_data *tsd) if (!cd->dai_dev) return -ENODEV; - struct processing_module *mod = comp_get_drvdata(cd->dai_dev); + struct processing_module *mod = comp_mod(cd->dai_dev); const struct module_interface *const ops = mod->dev->drv->adapter_ops; return ops->endpoint_ops->dai_ts_get(cd->dai_dev, tsd); diff --git a/src/audio/copier/copier.c b/src/audio/copier/copier.c index 0d1f87de55c4..2c564ed57f38 100644 --- a/src/audio/copier/copier.c +++ b/src/audio/copier/copier.c @@ -293,7 +293,7 @@ static int copier_reset(struct processing_module *mod) static int copier_comp_trigger(struct comp_dev *dev, int cmd) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct sof_ipc_stream_posn posn; struct comp_dev *dai_copier; @@ -660,7 +660,7 @@ static int copier_set_sink_fmt(struct comp_dev *dev, const void *data, int max_data_size) { const struct ipc4_copier_config_set_sink_format *sink_fmt = data; - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct list_item *sink_list; struct comp_buffer *sink; @@ -711,7 +711,7 @@ static int copier_set_sink_fmt(struct comp_dev *dev, const void *data, static int set_attenuation(struct comp_dev *dev, uint32_t data_offset, const char *data) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); uint32_t attenuation; enum sof_ipc_frame valid_fmt, frame_fmt; @@ -860,7 +860,7 @@ static int copier_get_configuration(struct processing_module *mod, static uint64_t copier_get_processed_data(struct comp_dev *dev, uint32_t stream_no, bool input) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); uint64_t ret = 0; bool source; @@ -905,7 +905,7 @@ static uint64_t copier_get_processed_data(struct comp_dev *dev, uint32_t stream_ static int copier_position(struct comp_dev *dev, struct sof_ipc_stream_posn *posn) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); int ret = 0; @@ -933,7 +933,7 @@ static int copier_position(struct comp_dev *dev, struct sof_ipc_stream_posn *pos static int copier_dai_ts_config_op(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct dai_data *dd = cd->dd[0]; @@ -942,7 +942,7 @@ static int copier_dai_ts_config_op(struct comp_dev *dev) static int copier_dai_ts_start_op(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct dai_data *dd = cd->dd[0]; @@ -957,7 +957,7 @@ static int copier_dai_ts_get_op(struct comp_dev *dev, struct dai_ts_data *tsd) static int copier_dai_ts_get_op(struct comp_dev *dev, struct timestamp_data *tsd) #endif { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct dai_data *dd = cd->dd[0]; @@ -968,7 +968,7 @@ static int copier_dai_ts_get_op(struct comp_dev *dev, struct timestamp_data *tsd static int copier_dai_ts_stop_op(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct dai_data *dd = cd->dd[0]; @@ -980,7 +980,7 @@ static int copier_dai_ts_stop_op(struct comp_dev *dev) static int copier_get_hw_params(struct comp_dev *dev, struct sof_ipc_stream_params *params, int dir) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct dai_data *dd = cd->dd[0]; diff --git a/src/audio/copier/copier_dai.c b/src/audio/copier/copier_dai.c index 74c6d62a4d6b..e2f03573f721 100644 --- a/src/audio/copier/copier_dai.c +++ b/src/audio/copier/copier_dai.c @@ -35,7 +35,7 @@ static int copier_set_alh_multi_gtw_channel_map(struct comp_dev *dev, const struct ipc4_copier_module_cfg *copier_cfg, int index) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); const struct sof_alh_configuration_blob *alh_blob; uint8_t chan_bitmask; @@ -71,7 +71,7 @@ static int copier_alh_assign_dai_index(struct comp_dev *dev, int *dai_index, int *dai_count) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); const struct sof_alh_configuration_blob *alh_blob = gtw_cfg_data; uint8_t *dma_config; @@ -162,7 +162,7 @@ static int copier_dai_init(struct comp_dev *dev, enum ipc4_gateway_type type, int index, int dai_count) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct dai_data *dd; int ret; @@ -230,7 +230,7 @@ int copier_dai_create(struct comp_dev *dev, struct copier_data *cd, const struct ipc4_copier_module_cfg *copier, struct pipeline *pipeline) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct comp_ipc_config *config = &dev->ipc_config; int dai_index[IPC4_ALH_MAX_NUMBER_OF_GTW]; union ipc4_connector_node_id node_id; diff --git a/src/audio/copier/copier_host.c b/src/audio/copier/copier_host.c index abce0f0557ed..a7eafac04eaa 100644 --- a/src/audio/copier/copier_host.c +++ b/src/audio/copier/copier_host.c @@ -94,7 +94,7 @@ void delete_from_fpi_sync_group(struct host_data *hd) /* Playback only */ static int init_pipeline_reg(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); struct ipc4_pipeline_registers pipe_reg; uint32_t gateway_id; @@ -126,7 +126,7 @@ int copier_host_create(struct comp_dev *dev, struct copier_data *cd, const struct ipc4_copier_module_cfg *copier_cfg, struct pipeline *pipeline) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct comp_ipc_config *config = &dev->ipc_config; struct ipc_config_host ipc_host; struct host_data *hd; @@ -244,7 +244,7 @@ void copier_host_free(struct copier_data *cd) */ void copier_host_dma_cb(struct comp_dev *dev, size_t bytes) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); int ret, frames; diff --git a/src/audio/eq_fir/eq_fir.c b/src/audio/eq_fir/eq_fir.c index b7402dba2207..3ff65910fc2b 100644 --- a/src/audio/eq_fir/eq_fir.c +++ b/src/audio/eq_fir/eq_fir.c @@ -96,7 +96,7 @@ static int eq_fir_init_coef(struct comp_dev *dev, struct sof_eq_fir_config *conf /* Called from validate(), we shall find nch and assign it accordingly, * as the parameter is not valid */ - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct comp_data *cd = module_get_private_data(mod); nch = cd->nch; diff --git a/src/audio/mixin_mixout/mixin_mixout.c b/src/audio/mixin_mixout/mixin_mixout.c index e89a5a15185b..5171cbda22e0 100644 --- a/src/audio/mixin_mixout/mixin_mixout.c +++ b/src/audio/mixin_mixout/mixin_mixout.c @@ -318,7 +318,7 @@ static int mixin_process(struct processing_module *mod, continue; } - mixout_mod = comp_get_drvdata(mixout); + mixout_mod = comp_mod(mixout); active_mixouts[i] = mixout_mod; mixout_sink = mixout_mod->sinks[0]; diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index 85e6f4cadff8..e4448922b9f9 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -26,7 +26,7 @@ int module_load_config(struct comp_dev *dev, const void *cfg, size_t size) { int ret; struct module_config *dst; - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct module_data *md = &mod->priv; comp_dbg(dev, "module_load_config() start"); diff --git a/src/audio/module_adapter/module_adapter.c b/src/audio/module_adapter/module_adapter.c index 18cb5dc0dfff..8c1e825451c6 100644 --- a/src/audio/module_adapter/module_adapter.c +++ b/src/audio/module_adapter/module_adapter.c @@ -78,9 +78,10 @@ struct comp_dev *module_adapter_new(const struct comp_driver *drv, } dst = &mod->priv.cfg; + mod->dev = dev; + dev->mod = mod; - comp_set_drvdata(dev, mod); list_init(&mod->sink_buffer_list); ret = module_adapter_init_data(dev, dst, config, spec); @@ -134,7 +135,7 @@ EXPORT_SYMBOL(module_adapter_new); #if CONFIG_ZEPHYR_DP_SCHEDULER static void module_adapter_calculate_dp_period(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); unsigned int period = UINT32_MAX; for (int i = 0; i < mod->num_of_sinks; i++) { @@ -163,7 +164,7 @@ static void module_adapter_calculate_dp_period(struct comp_dev *dev) int module_adapter_prepare(struct comp_dev *dev) { int ret; - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct module_data *md = &mod->priv; struct comp_buffer *sink; struct list_item *blist, *_blist; @@ -440,7 +441,7 @@ EXPORT_SYMBOL(module_adapter_prepare); int module_adapter_params(struct comp_dev *dev, struct sof_ipc_stream_params *params) { int ret; - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); module_adapter_set_params(mod, params); @@ -560,7 +561,7 @@ static void generate_zeroes(struct comp_buffer *sink, uint32_t bytes) static void module_copy_samples(struct comp_dev *dev, struct comp_buffer *src_buffer, struct comp_buffer *sink_buffer, uint32_t produced) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct comp_copy_limits cl; uint32_t copy_bytes; @@ -597,7 +598,7 @@ static void module_copy_samples(struct comp_dev *dev, struct comp_buffer *src_bu static void module_adapter_process_output(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct comp_buffer *sink; struct list_item *blist; int i = 0; @@ -651,7 +652,7 @@ module_single_sink_setup(struct comp_dev *dev, struct comp_buffer **source, struct comp_buffer **sinks) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct list_item *blist; uint32_t num_input_buffers; uint32_t frames; @@ -692,7 +693,7 @@ module_single_source_setup(struct comp_dev *dev, struct comp_buffer **source, struct comp_buffer **sinks) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct list_item *blist; uint32_t min_frames = UINT32_MAX; uint32_t num_output_buffers; @@ -732,7 +733,7 @@ module_single_source_setup(struct comp_dev *dev, static int module_adapter_audio_stream_copy_1to1(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); uint32_t num_output_buffers = 0; uint32_t frames; int ret; @@ -783,7 +784,7 @@ static int module_adapter_audio_stream_type_copy(struct comp_dev *dev) { struct comp_buffer *sources[PLATFORM_MAX_STREAMS]; struct comp_buffer *sinks[PLATFORM_MAX_STREAMS]; - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct list_item *blist; uint32_t num_input_buffers, num_output_buffers; int ret, i = 0; @@ -909,7 +910,7 @@ static int module_adapter_copy_dp_queues(struct comp_dev *dev) * DP module processing itself will take place in DP thread * This is an adapter, to be removed when pipeline2.0 is ready */ - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct list_item *blist; int err; @@ -968,7 +969,7 @@ static inline int module_adapter_copy_dp_queues(struct comp_dev *dev) static int module_adapter_sink_source_copy(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); int ret; int i = 0; @@ -1003,7 +1004,7 @@ static int module_adapter_sink_source_copy(struct comp_dev *dev) static int module_adapter_raw_data_type_copy(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct module_data *md = &mod->priv; struct comp_buffer *source, *sink; struct list_item *blist; @@ -1099,7 +1100,7 @@ int module_adapter_copy(struct comp_dev *dev) { comp_dbg(dev, "module_adapter_copy(): start"); - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); if (IS_PROCESSING_MODE_AUDIO_STREAM(mod)) return module_adapter_audio_stream_type_copy(dev); @@ -1122,7 +1123,7 @@ EXPORT_SYMBOL(module_adapter_copy); int module_adapter_trigger(struct comp_dev *dev, int cmd) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; comp_dbg(dev, "module_adapter_trigger(): cmd %d", cmd); @@ -1149,7 +1150,7 @@ EXPORT_SYMBOL(module_adapter_trigger); int module_adapter_reset(struct comp_dev *dev) { int ret, i; - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct list_item *blist; comp_dbg(dev, "module_adapter_reset(): resetting"); @@ -1197,7 +1198,7 @@ EXPORT_SYMBOL(module_adapter_reset); void module_adapter_free(struct comp_dev *dev) { int ret; - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct list_item *blist, *_blist; comp_dbg(dev, "module_adapter_free(): start"); @@ -1239,7 +1240,7 @@ EXPORT_SYMBOL(module_adapter_free); int module_adapter_get_hw_params(struct comp_dev *dev, struct sof_ipc_stream_params *params, int dir) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; if (interface->endpoint_ops && interface->endpoint_ops->dai_get_hw_params) @@ -1260,7 +1261,7 @@ EXPORT_SYMBOL(module_adapter_get_hw_params); */ int module_adapter_position(struct comp_dev *dev, struct sof_ipc_stream_posn *posn) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; if (interface->endpoint_ops && interface->endpoint_ops->position) @@ -1280,7 +1281,7 @@ EXPORT_SYMBOL(module_adapter_position); */ int module_adapter_ts_config_op(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; if (interface->endpoint_ops && interface->endpoint_ops->dai_ts_config) @@ -1300,7 +1301,7 @@ EXPORT_SYMBOL(module_adapter_ts_config_op); */ int module_adapter_ts_start_op(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; if (interface->endpoint_ops && interface->endpoint_ops->dai_ts_start) @@ -1320,7 +1321,7 @@ EXPORT_SYMBOL(module_adapter_ts_start_op); */ int module_adapter_ts_stop_op(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; if (interface->endpoint_ops && interface->endpoint_ops->dai_ts_stop) @@ -1345,7 +1346,7 @@ int module_adapter_ts_get_op(struct comp_dev *dev, struct dai_ts_data *tsd) int module_adapter_ts_get_op(struct comp_dev *dev, struct timestamp_data *tsd) #endif { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; if (interface->endpoint_ops && interface->endpoint_ops->dai_ts_get) diff --git a/src/audio/module_adapter/module_adapter_ipc3.c b/src/audio/module_adapter/module_adapter_ipc3.c index 5f976ac36313..3df30b7e0873 100644 --- a/src/audio/module_adapter/module_adapter_ipc3.c +++ b/src/audio/module_adapter/module_adapter_ipc3.c @@ -168,7 +168,7 @@ int module_adapter_set_state(struct processing_module *mod, struct comp_dev *dev static int module_adapter_get_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata, bool set) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; enum module_cfg_fragment_position pos; uint32_t data_offset_size; @@ -222,7 +222,7 @@ static int module_adapter_ctrl_get_set_data(struct comp_dev *dev, struct sof_ipc bool set) { int ret; - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); comp_dbg(dev, "module_adapter_ctrl_set_data() start, state %d, cmd %d", mod->priv.state, cdata->cmd); @@ -254,7 +254,7 @@ static int module_adapter_ctrl_get_set_data(struct comp_dev *dev, struct sof_ipc int module_adapter_cmd(struct comp_dev *dev, int cmd, void *data, int max_data_size) { struct sof_ipc_ctrl_data *cdata = ASSUME_ALIGNED(data, 4); - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; int ret = 0; @@ -306,7 +306,7 @@ int module_adapter_cmd(struct comp_dev *dev, int cmd, void *data, int max_data_s int module_adapter_sink_src_prepare(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct list_item *blist; int ret; int i; diff --git a/src/audio/module_adapter/module_adapter_ipc4.c b/src/audio/module_adapter/module_adapter_ipc4.c index 884a57b419f5..3c23411d7fd1 100644 --- a/src/audio/module_adapter/module_adapter_ipc4.c +++ b/src/audio/module_adapter/module_adapter_ipc4.c @@ -99,7 +99,7 @@ int module_adapter_set_state(struct processing_module *mod, struct comp_dev *dev int module_set_large_config(struct comp_dev *dev, uint32_t param_id, bool first_block, bool last_block, uint32_t data_offset_size, const char *data) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; struct module_data *md = &mod->priv; enum module_cfg_fragment_position pos; @@ -138,7 +138,7 @@ EXPORT_SYMBOL(module_set_large_config); int module_get_large_config(struct comp_dev *dev, uint32_t param_id, bool first_block, bool last_block, uint32_t *data_offset_size, char *data) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; struct module_data *md = &mod->priv; size_t fragment_size; @@ -169,7 +169,7 @@ EXPORT_SYMBOL(module_get_large_config); int module_adapter_get_attribute(struct comp_dev *dev, uint32_t type, void *value) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); switch (type) { case COMP_ATTR_BASE_CONFIG: @@ -186,7 +186,7 @@ EXPORT_SYMBOL(module_adapter_get_attribute); static bool module_adapter_multi_sink_source_prepare(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct list_item *blist; int i; @@ -225,7 +225,7 @@ static bool module_adapter_multi_sink_source_prepare(struct comp_dev *dev) int module_adapter_bind(struct comp_dev *dev, void *data) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); int ret; ret = module_bind(mod, data); @@ -240,7 +240,7 @@ EXPORT_SYMBOL(module_adapter_bind); int module_adapter_unbind(struct comp_dev *dev, void *data) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); int ret; ret = module_unbind(mod, data); @@ -256,7 +256,7 @@ EXPORT_SYMBOL(module_adapter_unbind); uint64_t module_adapter_get_total_data_processed(struct comp_dev *dev, uint32_t stream_no, bool input) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; if (interface->endpoint_ops && interface->endpoint_ops->get_total_data_processed) @@ -271,7 +271,7 @@ EXPORT_SYMBOL(module_adapter_get_total_data_processed); int module_adapter_sink_src_prepare(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); /* Prepare module */ return module_prepare(mod, mod->sources, mod->num_of_sources, diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index e354ff0f79dc..3ff48bce8d9d 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -390,7 +390,7 @@ static enum task_state dp_task_run(void *data) int pipeline_comp_dp_task_init(struct comp_dev *comp) { int ret; - struct processing_module *mod = comp_get_drvdata(comp); + struct processing_module *mod = comp_mod(comp); struct task_ops ops = { .run = dp_task_run, .get_deadline = NULL, diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index 51ddbd965ec1..7077f5e30251 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -336,7 +336,7 @@ static int pipeline_calc_cps_consumption(struct comp_dev *current, if (current->drv->type != SOF_COMP_MODULE_ADAPTER) { cd = comp_get_drvdata(current); } else { - struct processing_module *mod = comp_get_drvdata(current); + struct processing_module *mod = comp_mod(current); struct module_data *md = &mod->priv; cd = &md->cfg.base_cfg; @@ -472,7 +472,7 @@ static int pipeline_comp_trigger(struct comp_dev *current, #if CONFIG_IPC_MAJOR_3 dd = comp_get_drvdata(current); #elif CONFIG_IPC_MAJOR_4 - struct processing_module *mod = comp_get_drvdata(current); + struct processing_module *mod = comp_mod(current); struct copier_data *cd = module_get_private_data(mod); dd = cd->dd[0]; diff --git a/src/audio/src/src_ipc3.c b/src/audio/src/src_ipc3.c index ad1f1ed05a36..bef9b728670f 100644 --- a/src/audio/src/src_ipc3.c +++ b/src/audio/src/src_ipc3.c @@ -87,7 +87,7 @@ int src_set_params(struct processing_module *mod, struct sof_sink *sink) void src_get_source_sink_params(struct comp_dev *dev, struct sof_source *source, struct sof_sink *sink) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct comp_data *cd = module_get_private_data(mod); /* Set source/sink_rate/frames */ diff --git a/src/audio/src/src_ipc4.c b/src/audio/src/src_ipc4.c index 74e1cd03fd7b..b53ed757b5c9 100644 --- a/src/audio/src/src_ipc4.c +++ b/src/audio/src/src_ipc4.c @@ -122,7 +122,7 @@ int src_set_params(struct processing_module *mod, struct sof_sink *sink) void src_get_source_sink_params(struct comp_dev *dev, struct sof_source *source, struct sof_sink *sink) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct comp_data *cd = module_get_private_data(mod); enum sof_ipc_frame frame_fmt, valid_fmt; diff --git a/src/audio/volume/volume.h b/src/audio/volume/volume.h index 2076412549b6..d4ec1c8a3272 100644 --- a/src/audio/volume/volume.h +++ b/src/audio/volume/volume.h @@ -221,7 +221,7 @@ static inline vol_scale_func vol_get_processing_function(struct comp_dev *dev, static inline vol_scale_func vol_get_processing_function(struct comp_dev *dev, struct vol_data *cd) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); if (cd->is_passthrough) { switch (mod->priv.cfg.base_cfg.audio_fmt.valid_bit_depth) { diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index 264672ba2791..9c23e43f740c 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -590,6 +590,8 @@ struct comp_dev { const struct comp_driver *drv; /**< driver */ + struct processing_module *mod; /**< self->mod->dev == self, always */ + /* lists */ struct list_item bsource_list; /**< list of source buffers */ struct list_item bsink_list; /**< list of sink buffers */ @@ -678,6 +680,15 @@ static inline struct comp_dev *comp_alloc(const struct comp_driver *drv, return dev; } +/** + * \brief Module adapter associated with a component + * @param dev Component device + */ +static inline struct processing_module *comp_mod(const struct comp_dev *dev) +{ + return dev->mod; +} + /** * \brief Assigns private data to component device. * @param c Component device. diff --git a/src/ipc/ipc4/dai.c b/src/ipc/ipc4/dai.c index 1c96650b13e3..10db13e687a0 100644 --- a/src/ipc/ipc4/dai.c +++ b/src/ipc/ipc4/dai.c @@ -87,7 +87,7 @@ int dai_config_dma_channel(struct dai_data *dd, struct comp_dev *dev, const void #if defined(CONFIG_ACE_VERSION_2_0) if (copier_cfg->gtw_cfg.node_id.f.dma_type == ipc4_alh_link_output_class || copier_cfg->gtw_cfg.node_id.f.dma_type == ipc4_alh_link_input_class) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct copier_data *cd = module_get_private_data(mod); if (!cd->gtw_cfg) { diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 3774b4adca84..63425c9a6147 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -468,8 +468,8 @@ 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); - struct processing_module *srcmod = comp_get_drvdata(source); - struct processing_module *dstmod = comp_get_drvdata(sink); + struct processing_module *srcmod = comp_mod(source); + struct processing_module *dstmod = comp_mod(sink); struct module_config *dstcfg = &dstmod->priv.cfg; struct module_config *srccfg = &srcmod->priv.cfg; diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index e7007517cff6..0311edb7797f 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -556,7 +556,7 @@ static struct comp_dev *lib_manager_module_create(const struct comp_driver *drv, dev = module_adapter_new(drv, config, spec); if (dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); mod->priv.llext = tmp_proc.priv.llext; } else { @@ -567,7 +567,7 @@ static struct comp_dev *lib_manager_module_create(const struct comp_driver *drv, static void lib_manager_module_free(struct comp_dev *dev) { - struct processing_module *mod = comp_get_drvdata(dev); + struct processing_module *mod = comp_mod(dev); struct llext *llext = mod->priv.llext; const struct comp_ipc_config *const config = &mod->dev->ipc_config; const uint32_t module_id = config->id; diff --git a/test/cmocka/src/audio/eq_fir/eq_fir_process.c b/test/cmocka/src/audio/eq_fir/eq_fir_process.c index f13746052f3d..a847a4d34fb6 100644 --- a/test/cmocka/src/audio/eq_fir/eq_fir_process.c +++ b/test/cmocka/src/audio/eq_fir/eq_fir_process.c @@ -157,7 +157,7 @@ static int setup(void **state) td->dev = dev; dev->frames = params->frames; - mod = comp_get_drvdata(dev); + mod = comp_mod(dev); prepare_sink(td, mod); prepare_source(td, mod); @@ -184,7 +184,7 @@ static int setup(void **state) static int teardown(void **state) { struct test_data *td = *state; - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); test_free(mod->input_buffers); test_free(mod->output_buffers); @@ -200,7 +200,7 @@ static int teardown(void **state) #if CONFIG_FORMAT_S16LE static void fill_source_s16(struct test_data *td, int frames_max) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -235,7 +235,7 @@ static void fill_source_s16(struct test_data *td, int frames_max) static void verify_sink_s16(struct test_data *td) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -266,7 +266,7 @@ static void verify_sink_s16(struct test_data *td) #if CONFIG_FORMAT_S24LE static void fill_source_s24(struct test_data *td, int frames_max) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -301,7 +301,7 @@ static void fill_source_s24(struct test_data *td, int frames_max) static void verify_sink_s24(struct test_data *td) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -332,7 +332,7 @@ static void verify_sink_s24(struct test_data *td) #if CONFIG_FORMAT_S32LE static void fill_source_s32(struct test_data *td, int frames_max) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -367,7 +367,7 @@ static void fill_source_s32(struct test_data *td, int frames_max) static void verify_sink_s32(struct test_data *td) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -410,7 +410,7 @@ static int frames_jitter(int frames) static void test_audio_eq_fir(void **state) { struct test_data *td = *state; - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_buffer *source = td->source; struct comp_buffer *sink = td->sink; diff --git a/test/cmocka/src/audio/eq_iir/eq_iir_process.c b/test/cmocka/src/audio/eq_iir/eq_iir_process.c index 529d4328f580..fa18cf5b0c3f 100644 --- a/test/cmocka/src/audio/eq_iir/eq_iir_process.c +++ b/test/cmocka/src/audio/eq_iir/eq_iir_process.c @@ -156,7 +156,7 @@ static int setup(void **state) td->dev = dev; dev->frames = params->frames; - mod = comp_get_drvdata(dev); + mod = comp_mod(dev); prepare_sink(td, mod); prepare_source(td, mod); @@ -183,7 +183,7 @@ static int setup(void **state) static int teardown(void **state) { struct test_data *td = *state; - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); test_free(mod->input_buffers); test_free(mod->output_buffers); @@ -199,7 +199,7 @@ static int teardown(void **state) #if CONFIG_FORMAT_S16LE static void fill_source_s16(struct test_data *td, int frames_max) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -234,7 +234,7 @@ static void fill_source_s16(struct test_data *td, int frames_max) static void verify_sink_s16(struct test_data *td) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -262,7 +262,7 @@ static void verify_sink_s16(struct test_data *td) #if CONFIG_FORMAT_S24LE static void fill_source_s24(struct test_data *td, int frames_max) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -297,7 +297,7 @@ static void fill_source_s24(struct test_data *td, int frames_max) static void verify_sink_s24(struct test_data *td) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -325,7 +325,7 @@ static void verify_sink_s24(struct test_data *td) #if CONFIG_FORMAT_S32LE static void fill_source_s32(struct test_data *td, int frames_max) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -360,7 +360,7 @@ static void fill_source_s32(struct test_data *td, int frames_max) static void verify_sink_s32(struct test_data *td) { - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_dev *dev = td->dev; struct comp_buffer *sb; struct audio_stream *ss; @@ -400,7 +400,7 @@ static int frames_jitter(int frames) static void test_audio_eq_iir(void **state) { struct test_data *td = *state; - struct processing_module *mod = comp_get_drvdata(td->dev); + struct processing_module *mod = comp_mod(td->dev); struct comp_buffer *source = td->source; struct comp_buffer *sink = td->sink; diff --git a/test/cmocka/src/audio/module_adapter_test.c b/test/cmocka/src/audio/module_adapter_test.c index 560c57a6158b..29140e4581e9 100644 --- a/test/cmocka/src/audio/module_adapter_test.c +++ b/test/cmocka/src/audio/module_adapter_test.c @@ -28,7 +28,7 @@ int module_adapter_test_setup(struct processing_module_test_data *test_data) dev = test_malloc(sizeof(struct comp_dev)); dev->frames = parameters->frames; mod->dev = dev; - comp_set_drvdata(dev, mod); + dev->mod = mod; test_data->sinks = test_calloc(test_data->num_sinks, sizeof(struct comp_buffer *)); test_data->sources = test_calloc(test_data->num_sources, sizeof(struct comp_buffer *)); diff --git a/test/cmocka/src/audio/mux/demux_copy.c b/test/cmocka/src/audio/mux/demux_copy.c index 857fe6a0b3d1..9ed45924dec0 100644 --- a/test/cmocka/src/audio/mux/demux_copy.c +++ b/test/cmocka/src/audio/mux/demux_copy.c @@ -167,7 +167,7 @@ static int setup_test_case(void **state) if (!dev) return -EINVAL; - mod = comp_get_drvdata(dev); + mod = comp_mod(dev); td->dev = dev; td->mod = mod; td->cd = module_get_private_data(mod); diff --git a/test/cmocka/src/audio/mux/mux_copy.c b/test/cmocka/src/audio/mux/mux_copy.c index 3d41cb176881..0f65030f0513 100644 --- a/test/cmocka/src/audio/mux/mux_copy.c +++ b/test/cmocka/src/audio/mux/mux_copy.c @@ -189,7 +189,7 @@ static int setup_test_case(void **state) if (!dev) return -EINVAL; - mod = comp_get_drvdata(dev); + mod = comp_mod(dev); td->dev = dev; td->mod = mod; td->cd = module_get_private_data(mod); diff --git a/test/cmocka/src/audio/mux/mux_get_processing_function.c b/test/cmocka/src/audio/mux/mux_get_processing_function.c index 6f1528839eb1..3296de3a21bb 100644 --- a/test/cmocka/src/audio/mux/mux_get_processing_function.c +++ b/test/cmocka/src/audio/mux/mux_get_processing_function.c @@ -83,7 +83,7 @@ static int setup_test_case(void **state) if (!dev) return -EINVAL; - mod = comp_get_drvdata(dev); + mod = comp_mod(dev); td->dev = dev; td->mod = mod; td->cd = module_get_private_data(mod); From c4f568907db343831de78ebcd335fda442773510 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Tue, 4 Jun 2024 18:51:46 -0700 Subject: [PATCH 3/5] buffer: Move pipeline_id down into stream params The pipeline_id has historically been part of the comp_buffer struct, but that is being deprecated as a public API. So move it down into the contained sof_audio_stream_params struct where it can be found by new style sink/source code. Note that the actual value of the pipeline ID is a little ambiguous: on IPC3, the buffer is defined by the user in the .tplg file as part of a specific pipeline with a known ID. With IPC4 topology, the buffers are implicitly created and will be assigned the ID of their source (!) component. It is legal to define a connection across two pipelines, and there's no ability here to recover both pipeline IDs. Signed-off-by: Andy Ross --- src/audio/crossover/crossover.c | 2 +- src/audio/crossover/crossover_ipc3.c | 2 +- src/audio/mux/mux.c | 6 +++--- src/audio/mux/mux_ipc4.c | 2 +- src/include/module/audio/audio_stream.h | 1 + src/include/sof/audio/buffer.h | 25 +++++++++++++------------ src/ipc/ipc-helper.c | 4 ++-- tools/testbench/testbench.c | 2 +- 8 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/audio/crossover/crossover.c b/src/audio/crossover/crossover.c index 69135368671a..3cf6e6744709 100644 --- a/src/audio/crossover/crossover.c +++ b/src/audio/crossover/crossover.c @@ -111,7 +111,7 @@ static int crossover_assign_sinks(struct processing_module *mod, unsigned int sink_id, state; sink = container_of(sink_list, struct comp_buffer, source_list); - sink_id = crossover_get_sink_id(cd, sink->pipeline_id, j); + sink_id = crossover_get_sink_id(cd, buffer_pipeline_id(sink), j); state = sink->sink->state; if (state != dev->state) { j++; diff --git a/src/audio/crossover/crossover_ipc3.c b/src/audio/crossover/crossover_ipc3.c index ea8fd32d02ef..c715f1710460 100644 --- a/src/audio/crossover/crossover_ipc3.c +++ b/src/audio/crossover/crossover_ipc3.c @@ -46,7 +46,7 @@ int crossover_check_sink_assign(struct processing_module *mod, unsigned int pipeline_id; sink = container_of(sink_list, struct comp_buffer, source_list); - pipeline_id = sink->pipeline_id; + pipeline_id = buffer_pipeline_id(sink); i = crossover_get_stream_index(mod, config, pipeline_id); if (i < 0) { diff --git a/src/audio/mux/mux.c b/src/audio/mux/mux.c index a9e89afb9644..92fb98a93faa 100644 --- a/src/audio/mux/mux.c +++ b/src/audio/mux/mux.c @@ -248,13 +248,13 @@ static int demux_process(struct processing_module *mod, list_for_item(clist, &dev->bsink_list) { sink = container_of(clist, struct comp_buffer, source_list); if (sink->sink->state == dev->state) { - i = get_stream_index(dev, cd, sink->pipeline_id); + i = get_stream_index(dev, cd, buffer_pipeline_id(sink)); /* return if index wrong */ if (i < 0) { return i; } - look_ups[i] = get_lookup_table(dev, cd, sink->pipeline_id); + look_ups[i] = get_lookup_table(dev, cd, buffer_pipeline_id(sink)); sinks_stream[i] = &sink->stream; } } @@ -310,7 +310,7 @@ static int mux_process(struct processing_module *mod, else frames = input_buffers[j].size; - i = get_stream_index(dev, cd, source->pipeline_id); + i = get_stream_index(dev, cd, buffer_pipeline_id(source)); /* return if index wrong */ if (i < 0) { return i; diff --git a/src/audio/mux/mux_ipc4.c b/src/audio/mux/mux_ipc4.c index bf6e591665bc..a4edc218ab5d 100644 --- a/src/audio/mux/mux_ipc4.c +++ b/src/audio/mux/mux_ipc4.c @@ -114,7 +114,7 @@ static void set_mux_params(struct processing_module *mod) { source = container_of(source_list, struct comp_buffer, sink_list); j = buf_get_id(source); - cd->config.streams[j].pipeline_id = source->pipeline_id; + cd->config.streams[j].pipeline_id = buffer_pipeline_id(source); if (j == BASE_CFG_QUEUED_ID) audio_fmt = &cd->md.base_cfg.audio_fmt; else diff --git a/src/include/module/audio/audio_stream.h b/src/include/module/audio/audio_stream.h index 0941047a6395..5e30c2aab4a9 100644 --- a/src/include/module/audio/audio_stream.h +++ b/src/include/module/audio/audio_stream.h @@ -20,6 +20,7 @@ */ struct sof_audio_stream_params { uint32_t id; + uint32_t pipeline_id; enum sof_ipc_frame frame_fmt; /**< Sample data format */ enum sof_ipc_frame valid_sample_fmt; diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index 662e329c1027..a0172570bd30 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -43,9 +43,6 @@ extern struct tr_ctx buffer_tr; /** \brief Retrieves trace context from the buffer */ #define trace_buf_get_tr_ctx(buf_ptr) (&(buf_ptr)->tctx) -/** \brief Retrieves id (pipe id) from the buffer */ -#define trace_buf_get_id(buf_ptr) ((buf_ptr)->pipeline_id) - /** \brief Retrieves subid (comp id) from the buffer */ #define buf_get_id(buf_ptr) ((buf_ptr)->stream.runtime_stream_params.id) @@ -57,36 +54,36 @@ extern struct tr_ctx buffer_tr; #define __BUF_FMT "buf:%u.%u " #endif -#define buf_err(buf_ptr, __e, ...) LOG_ERR(__BUF_FMT __e, trace_buf_get_id(buf_ptr), \ +#define buf_err(buf_ptr, __e, ...) LOG_ERR(__BUF_FMT __e, buffer_pipeline_id(buf_ptr), \ buf_get_id(buf_ptr), ##__VA_ARGS__) -#define buf_warn(buf_ptr, __e, ...) LOG_WRN(__BUF_FMT __e, trace_buf_get_id(buf_ptr), \ +#define buf_warn(buf_ptr, __e, ...) LOG_WRN(__BUF_FMT __e, buffer_pipeline_id(buf_ptr), \ buf_get_id(buf_ptr), ##__VA_ARGS__) -#define buf_info(buf_ptr, __e, ...) LOG_INF(__BUF_FMT __e, trace_buf_get_id(buf_ptr), \ +#define buf_info(buf_ptr, __e, ...) LOG_INF(__BUF_FMT __e, buffer_pipeline_id(buf_ptr), \ buf_get_id(buf_ptr), ##__VA_ARGS__) -#define buf_dbg(buf_ptr, __e, ...) LOG_DBG(__BUF_FMT __e, trace_buf_get_id(buf_ptr), \ +#define buf_dbg(buf_ptr, __e, ...) LOG_DBG(__BUF_FMT __e, buffer_pipeline_id(buf_ptr), \ buf_get_id(buf_ptr), ##__VA_ARGS__) #else /** \brief Trace error message from buffer */ #define buf_err(buf_ptr, __e, ...) \ - trace_dev_err(trace_buf_get_tr_ctx, trace_buf_get_id, \ + trace_dev_err(trace_buf_get_tr_ctx, buffer_pipeline_id, \ buf_get_id, \ (__sparse_force const struct comp_buffer *)buf_ptr, \ __e, ##__VA_ARGS__) /** \brief Trace warning message from buffer */ #define buf_warn(buf_ptr, __e, ...) \ - trace_dev_warn(trace_buf_get_tr_ctx, trace_buf_get_id, \ + trace_dev_warn(trace_buf_get_tr_ctx, buffer_pipeline_id, \ buf_get_id, \ (__sparse_force const struct comp_buffer *)buf_ptr, \ __e, ##__VA_ARGS__) /** \brief Trace info message from buffer */ #define buf_info(buf_ptr, __e, ...) \ - trace_dev_info(trace_buf_get_tr_ctx, trace_buf_get_id, \ + trace_dev_info(trace_buf_get_tr_ctx, buffer_pipeline_id, \ buf_get_id, \ (__sparse_force const struct comp_buffer *)buf_ptr, \ __e, ##__VA_ARGS__) @@ -96,7 +93,7 @@ extern struct tr_ctx buffer_tr; #define buf_dbg(buf_ptr, __e, ...) #else #define buf_dbg(buf_ptr, __e, ...) \ - trace_dev_dbg(trace_buf_get_tr_ctx, trace_buf_get_id, \ + trace_dev_dbg(trace_buf_get_tr_ctx, buffer_pipeline_id, \ buf_get_id, \ (__sparse_force const struct comp_buffer *)buf_ptr, \ __e, ##__VA_ARGS__) @@ -139,7 +136,6 @@ struct comp_buffer { struct audio_stream stream; /* configuration */ - uint32_t pipeline_id; uint32_t caps; uint32_t core; struct tr_ctx tctx; /* trace settings */ @@ -276,6 +272,11 @@ static inline struct comp_dev *buffer_get_comp(struct comp_buffer *buffer, int d return comp; } +static inline uint32_t buffer_pipeline_id(const struct comp_buffer *buffer) +{ + return buffer->stream.runtime_stream_params.pipeline_id; +} + static inline void buffer_reset_pos(struct comp_buffer *buffer, void *data) { /* reset rw pointers and avail/free bytes counters */ diff --git a/src/ipc/ipc-helper.c b/src/ipc/ipc-helper.c index 305c3623d3b6..23a14fade8e4 100644 --- a/src/ipc/ipc-helper.c +++ b/src/ipc/ipc-helper.c @@ -64,7 +64,7 @@ struct comp_buffer *buffer_new(const struct sof_ipc_buffer *desc, bool is_shared is_shared); if (buffer) { buffer->stream.runtime_stream_params.id = desc->comp.id; - buffer->pipeline_id = desc->comp.pipeline_id; + buffer->stream.runtime_stream_params.pipeline_id = desc->comp.pipeline_id; buffer->core = desc->comp.core; memcpy_s(&buffer->tctx, sizeof(struct tr_ctx), @@ -80,7 +80,7 @@ int32_t ipc_comp_pipe_id(const struct ipc_comp_dev *icd) case COMP_TYPE_COMPONENT: return dev_comp_pipe_id(icd->cd); case COMP_TYPE_BUFFER: - return icd->cb->pipeline_id; + return buffer_pipeline_id(icd->cb); case COMP_TYPE_PIPELINE: return icd->pipeline->pipeline_id; default: diff --git a/tools/testbench/testbench.c b/tools/testbench/testbench.c index 72d8ac4417a5..d4b03538792c 100644 --- a/tools/testbench/testbench.c +++ b/tools/testbench/testbench.c @@ -163,7 +163,7 @@ static void test_pipeline_free_comps(int pipeline_id) icd->id); break; case COMP_TYPE_BUFFER: - if (icd->cb->pipeline_id != pipeline_id) + if (buffer_pipeline_id(icd->cb) != pipeline_id) break; err = ipc_buffer_free(sof_get()->ipc, icd->id); if (err) From 713dfddf6ce2e8dd84320a6fb95c09528bbef0ee Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Tue, 11 Jun 2024 11:42:06 -0700 Subject: [PATCH 4/5] sink/source: Add pipeline_id accessor API Expose the new pipeline_id field in sof_audio_stream_params to a cleaner sink/source API for use by module code. Longer term this may want to be indirected by newer backends. Signed-off-by: Andy Ross --- src/include/module/audio/sink_api.h | 5 +++++ src/include/module/audio/source_api.h | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/include/module/audio/sink_api.h b/src/include/module/audio/sink_api.h index d5fd0118b3bf..98129b229f17 100644 --- a/src/include/module/audio/sink_api.h +++ b/src/include/module/audio/sink_api.h @@ -248,4 +248,9 @@ static inline uint32_t sink_get_id(struct sof_sink *sink) return sink->audio_stream_params->id; } +static inline uint32_t sink_get_pipeline_id(struct sof_sink *sink) +{ + return sink->audio_stream_params->pipeline_id; +} + #endif /* __MODULE_AUDIO_SINK_API_H__ */ diff --git a/src/include/module/audio/source_api.h b/src/include/module/audio/source_api.h index e4b522bdb7f0..0925fb251bd2 100644 --- a/src/include/module/audio/source_api.h +++ b/src/include/module/audio/source_api.h @@ -226,4 +226,9 @@ static inline uint32_t source_get_id(struct sof_source *source) return source->audio_stream_params->id; } +static inline uint32_t source_get_pipeline_id(struct sof_source *source) +{ + return source->audio_stream_params->pipeline_id; +} + #endif /* __MODULE_AUDIO_SOURCE_API_H__ */ From d38f9cd7688acbb055020f965143ecf2c983c48d Mon Sep 17 00:00:00 2001 From: Marcin Szkudlinski Date: Fri, 14 Jun 2024 11:01:01 +0200 Subject: [PATCH 5/5] fix: some code does not care for non-module adapter components Legacy API that is not using module_adapter is now depreciated, but there are still some modules that use it. So all common code must work properly with both types of modules. This commit is fixing crash in bind operation when binding a legacy module. There also some comments added in potentially similar places, but where legacy modules cannot be used. Signed-off-by: Marcin Szkudlinski --- src/audio/module_adapter/module/generic.c | 1 + src/audio/pipeline/pipeline-schedule.c | 1 + src/include/sof/audio/component.h | 4 ++- src/ipc/ipc4/helper.c | 31 ++++++++++++----------- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index e4448922b9f9..3436293a9cf8 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -26,6 +26,7 @@ int module_load_config(struct comp_dev *dev, const void *cfg, size_t size) { int ret; struct module_config *dst; + /* loadable module must use module adapter */ struct processing_module *mod = comp_mod(dev); struct module_data *md = &mod->priv; diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index 3ff48bce8d9d..76b3f1fe1f28 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -390,6 +390,7 @@ static enum task_state dp_task_run(void *data) int pipeline_comp_dp_task_init(struct comp_dev *comp) { int ret; + /* DP tasks are guaranteed to have a module_adapter */ struct processing_module *mod = comp_mod(comp); struct task_ops ops = { .run = dp_task_run, diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index 9c23e43f740c..4dfa2e66a26a 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -590,7 +590,9 @@ struct comp_dev { const struct comp_driver *drv; /**< driver */ - struct processing_module *mod; /**< self->mod->dev == self, always */ + struct processing_module *mod; /**< self->mod->dev == self, NULL if component is not using + * module adapter + */ /* lists */ struct list_item bsource_list; /**< list of source buffers */ diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 63425c9a6147..08f6298c88f1 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -468,14 +468,14 @@ 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); - struct processing_module *srcmod = comp_mod(source); - struct processing_module *dstmod = comp_mod(sink); - struct module_config *dstcfg = &dstmod->priv.cfg; - struct module_config *srccfg = &srcmod->priv.cfg; + if (source->drv->type == SOF_COMP_MODULE_ADAPTER) { + struct processing_module *srcmod = comp_mod(source); + struct module_config *srccfg = &srcmod->priv.cfg; - /* get obs from the base config extension if the src queue ID is non-zero */ - if (bu->extension.r.src_queue && bu->extension.r.src_queue < srccfg->nb_output_pins) - obs = srccfg->output_pins[bu->extension.r.src_queue].obs; + /* get obs from the base config extension if the src queue ID is non-zero */ + if (bu->extension.r.src_queue && bu->extension.r.src_queue < srccfg->nb_output_pins) + obs = srccfg->output_pins[bu->extension.r.src_queue].obs; + } /* get obs from base config if src queue ID is 0 or if base config extn is missing */ if (!obs) { @@ -490,10 +490,14 @@ int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) obs = source_src_cfg.obs; } - /* get ibs from the base config extension if the sink queue ID is non-zero */ - if (bu->extension.r.dst_queue && bu->extension.r.dst_queue < dstcfg->nb_input_pins) - ibs = dstcfg->input_pins[bu->extension.r.dst_queue].ibs; + if (sink->drv->type == SOF_COMP_MODULE_ADAPTER) { + struct processing_module *dstmod = comp_mod(sink); + struct module_config *dstcfg = &dstmod->priv.cfg; + /* get ibs from the base config extension if the sink queue ID is non-zero */ + if (bu->extension.r.dst_queue && bu->extension.r.dst_queue < dstcfg->nb_input_pins) + ibs = dstcfg->input_pins[bu->extension.r.dst_queue].ibs; + } /* get ibs from base config if sink queue ID is 0 or if base config extn is missing */ if (!ibs) { ret = comp_get_attribute(sink, COMP_ATTR_BASE_CONFIG, &sink_src_cfg); @@ -538,13 +542,10 @@ int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) source_set_min_available(audio_stream_get_source(&buffer->stream), ibs); #if CONFIG_ZEPHYR_DP_SCHEDULER - /* mod->dev may be null in case of a module not using module adapter */ - if (dstmod->dev && - dstmod->dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP) + if (sink->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP) /* data destination module needs to use dp_queue */ buffer_create_shadow_dp_queue(buffer, false /* at_input = false */); - else if (srcmod->dev && - srcmod->dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP) + else if (source->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP) /* data source module needs to use dp_queue */ buffer_create_shadow_dp_queue(buffer, true /* at_input = true */); #endif /* CONFIG_ZEPHYR_DP_SCHEDULER */