Skip to content

module: Revert "loading module: fix native module interface register" #8923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lyakh wants to merge 1 commit into from
Closed

module: Revert "loading module: fix native module interface register" #8923

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 26 additions & 16 deletions src/audio/module_adapter/module/modules.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ static int modules_new(struct processing_module *mod, const void *buildinfo,
(module_entry->segment[SOF_MAN_SEGMENT_TEXT].v_base_addr);
}

void *mod_adp;

byte_array_t mod_cfg = {
.data = (uint8_t *)md->cfg.init_data,
/* Intel modules expects DW size here */
Expand All @@ -91,21 +93,21 @@ static int modules_new(struct processing_module *mod, const void *buildinfo,
/* Check if module is FDK */
if (mod_buildinfo->format == IADK_MODULE_API_BUILD_INFO_FORMAT &&
mod_buildinfo->api_version_number.full == IADK_MODULE_API_CURRENT_VERSION) {
md->module_adapter = (void *)system_agent_start(module_entry_point,
module_id, instance_id,
0, log_handle, &mod_cfg);
mod_adp = system_agent_start(module_entry_point, module_id,
instance_id, 0, log_handle, &mod_cfg);
} else if (mod_buildinfo->format == SOF_MODULE_API_BUILD_INFO_FORMAT &&
mod_buildinfo->api_version_number.full == SOF_MODULE_API_CURRENT_VERSION) {
/* The module is native: start agent for sof loadable */
mod->is_native_sof = true;
md->ops = native_system_agent_start(mod->sys_service, module_entry_point,
module_id, instance_id,
0, log_handle, &mod_cfg);
mod_adp = native_system_agent_start(mod->sys_service, module_entry_point,
module_id, instance_id, 0, log_handle,
&mod_cfg);
} else {
return -ENOEXEC;
}

md->module_entry_point = module_entry_point;
md->module_adapter = mod_adp;
md->private = mod;

return 0;
Expand Down Expand Up @@ -161,7 +163,8 @@ static int modules_init(struct processing_module *mod)

/* Call module specific init function if exists. */
if (mod->is_native_sof) {
const struct module_interface *mod_in = md->ops;
struct module_interface *mod_in =
(struct module_interface *)md->module_adapter;

Copy link
Contributor

@pjdobrowolski pjdobrowolski Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That changes break loadable modules #8667 which works on main. Please do not introduce changes breaking working flow.

Copy link
Collaborator Author

@lyakh lyakh Mar 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pjdobrowolski I don't see how this revert can break anything. As you see from the hunk above around lines 90-110 the version before this commit stored a pointer to native loadable module interface, returned by native_system_agent_start() in md->ops and after this commit it's in md->module_adapter. The effect of this is that before this commit all framework calls to interface functions went directly to the loaded module functions, whereas after this commit they first call wrappers from module.c, which then call respective methods from the loaded module.
The difference is particularly visible for .free() and .init(). For the .free() case you have to call the wrapper, otherwise you miss calling lib_manager_free_module(). As for .init() this change probably has the effect, that if you don't reload the loaded module, then the second call to .init() would go directly to the loaded module, instead of modules_init(). Is this what is breaking your tests?

/* The order of preference */
if (mod_in->process)
Expand Down Expand Up @@ -204,7 +207,8 @@ static int modules_prepare(struct processing_module *mod,

/* Call module specific prepare function if exists. */
if (mod->is_native_sof) {
const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in =
(struct module_interface *)mod->priv.module_adapter;

ret = mod_in->prepare(mod, sources, num_of_sources, sinks, num_of_sinks);
} else {
Expand Down Expand Up @@ -235,7 +239,7 @@ static int modules_process(struct processing_module *mod,
return iadk_wrapper_process(mod->priv.module_adapter, sources,
num_of_sources, sinks, num_of_sinks);

const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in = (struct module_interface *)mod->priv.module_adapter;

return mod_in->process(mod, sources, num_of_sources, sinks, num_of_sinks);
}
Expand All @@ -249,7 +253,7 @@ static int modules_process_audio_stream(struct processing_module *mod,
if (!mod->is_native_sof)
return -EOPNOTSUPP;

const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in = (struct module_interface *)mod->priv.module_adapter;

return mod_in->process_audio_stream(mod, input_buffers, num_input_buffers,
output_buffers, num_output_buffers);
Expand Down Expand Up @@ -277,7 +281,8 @@ static int modules_process_raw(struct processing_module *mod,
modules_init_process(mod);

/* Call module specific process function. */
const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in =
(struct module_interface *)mod->priv.module_adapter;

return mod_in->process_raw_data(mod, input_buffers, num_input_buffers,
output_buffers, num_output_buffers);
Expand All @@ -299,7 +304,8 @@ static int modules_free(struct processing_module *mod)

comp_info(dev, "modules_free()");
if (mod->is_native_sof) {
const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in =
(struct module_interface *)mod->priv.module_adapter;

ret = mod_in->free(mod);
} else {
Expand Down Expand Up @@ -344,7 +350,8 @@ static int modules_set_configuration(struct processing_module *mod, uint32_t con
size_t response_size)
{
if (mod->is_native_sof) {
const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in =
(struct module_interface *)mod->priv.module_adapter;

return mod_in->set_configuration(mod, config_id, pos, data_offset_size, fragment,
fragment_size, response, response_size);
Expand All @@ -371,7 +378,8 @@ static int modules_get_configuration(struct processing_module *mod, uint32_t con
size_t fragment_size)
{
if (mod->is_native_sof) {
const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in =
(struct module_interface *)mod->priv.module_adapter;

return mod_in->get_configuration(mod, config_id, data_offset_size,
fragment, fragment_size);
Expand All @@ -392,7 +400,8 @@ static int modules_set_processing_mode(struct processing_module *mod,
enum module_processing_mode mode)
{
if (mod->is_native_sof) {
const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in =
(struct module_interface *)mod->priv.module_adapter;

return mod_in->set_processing_mode(mod, mode);
}
Expand Down Expand Up @@ -420,7 +429,8 @@ static enum module_processing_mode modules_get_processing_mode(struct processing
static int modules_reset(struct processing_module *mod)
{
if (mod->is_native_sof) {
const struct module_interface *mod_in = mod->priv.ops;
struct module_interface *mod_in =
(struct module_interface *)mod->priv.module_adapter;

return mod_in->reset(mod);
}
Expand Down