Skip to content

module_adapter: add request for prepare() after reset by module#6216

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
johnylin76 wants to merge 2 commits into from
Closed

module_adapter: add request for prepare() after reset by module #6216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92d357a
module_adapter: add request for prepare() after reset by module
johnylin76 Aug 30, 2022
d025fb1
module_adapter: volume: set state to INITIALIZED after reset
johnylin76 Aug 30, 2022
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
20 changes: 15 additions & 5 deletions src/audio/module_adapter/module/generic.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -264,13 +264,26 @@ int module_reset(struct processing_module *mod)
{
int ret;
struct module_data *md = &mod->priv;
enum module_state new_state;

/* the default behavior of module reset is back to the initial condition after prepare()
* so change its state to IDLE to reflect this.
*/
new_state = MODULE_IDLE;

/* if the module was never prepared, no need to reset */
if (md->state < MODULE_IDLE)
return 0;

ret = md->ops->reset(mod);
if (ret) {
if (ret == MODULE_PREPARE_REQUESTED) {
/* module resets itself to the state before prepare() so change its state to
* INITIALIZED to reflect this.
*/
comp_dbg(mod->dev, "module_reset(): module requests prepare() for comp %d",
dev_comp_id(mod->dev));
new_state = MODULE_INITIALIZED;
} else if (ret) {
comp_err(mod->dev, "module_reset() error %d: module specific reset() failed for comp %d",
ret, dev_comp_id(mod->dev));
return ret;
Expand All@@ -280,10 +293,7 @@ int module_reset(struct processing_module *mod)
md->cfg.size = 0;
rfree(md->cfg.data);

/* module resets itself to the initial condition after prepare()
* so let's change its state to reflect that.
*/
md->state = MODULE_IDLE;
md->state = new_state;

return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/audio/module_adapter/module/volume/volume.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1285,7 +1285,7 @@ static int volume_reset(struct processing_module *mod)

comp_dbg(mod->dev, "volume_reset()");
reset_state(cd);
return 0;
return MODULE_PREPARE_REQUESTED;
}

#if CONFIG_COMP_LEGACY_INTERFACE
Expand Down
9 changes: 9 additions & 0 deletions src/include/sof/audio/module_adapter/module/generic.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -82,6 +82,15 @@ enum module_state {
MODULE_PROCESSING, /**< Module is processing samples now. */
};

/**
* \enum module_retcode
* \brief Module-specific return codes - 0 is preserved for function success and
* all negative values are preserved for errors.
*/
enum module_retcode {
MODULE_PREPARE_REQUESTED = 1, /**< Module requests prepare() call after reset. */

@ranj063ranj063Aug 31, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@johnylin76 I want to remove the MODULE_* states and replace them with the existing COMP_STATE_* in the near future.

@johnylin76johnylin76Aug 31, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@ranj063 do you mean the alignment for module state and module_adapter state (combine them into one)? It sounds fine to me as well.
Could you also take a look of #6166 and provide comments? Would it be covered by the state alignment?
Thanks

@ranj063ranj063Aug 31, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@johnylin76 I think the fix in #6184 is probably the right thing to do with the changes to remove the prepare() call inside the cadence codec's reset and addition of memory free in all other modules' reset callbacks. This should handle the issue with volume in #6034 and also fix the alsa conformance test failures that you are seeing. Once this is done, I will modify the module adapter to remove the MODULE_IDLE/INITIALIZED states and use the comp states. Let me open a new PR Hope that is OK

};

/**
* \struct module_param
* \brief Module TLV parameters container - used for both config types.
Expand Down