-
Notifications
You must be signed in to change notification settings - Fork 349
module: prepare sink & source in bind & unbind function #8594
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.
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
Conversation
marcinszkudlinski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't duplicate identical functionality - pointers to sink and sources are set twice, using identical code
remove module_adapter_sink_src_prepare from module adapter - consider applying d0bf41b together with this patch
2382968 to
f2dbad7
Compare
@marcinszkudlinski Thanks, integrate your suggestion. |
f2dbad7 to
2cb6e3f
Compare
|
Looks like CI reports problems with commits from this PR cherry-picked here #8576. On previous versions of this fix there were no such CI failures. |
|
@RanderWang we are getting a failure loading topology then a leak (which could be in testbench and NOT in pipeline). @andrula-song and @singalsu can you help here. |
|
This looks good to go otherwise, but the testbench failure needs to be fixed. If it's a problem in testbench, we can have a fix in separate PR, but we need a rootcause before we can merge this. |
|
@RanderWang Does the SRC work in a normal IPC4 device with this PR? I'm not sure we have in CI test that runs a SRC pipeline. Looks that mod->num_of_sources and mod->num_of_sinks() are set after calling module_prepare() in module_adapter_dp_queue_prepare(). For testbench in module_adapter_prepare() the first if true is not taken because mod->dev->ipc_config.proc_domain is not COMP_PROCESSING_DOMAIN_DP. Then in the second else if the num_of_ arguments are zeros. |
2cb6e3f to
fec1eb1
Compare
I tested SRC. but it is working LL mode not in DP mode |
|
I also tested SRC in DP mode, but no issue found |
fec1eb1 to
87837d9
Compare
87837d9 to
58a5bfe
Compare
Module will update source & sink information when bind & unbind event happen. Signed-off-by: Rander Wang <rander.wang@intel.com>
58a5bfe to
bcedb1b
Compare
Thanks, then it looks like this is only issue for IPC3 or testbench. Can you check if adding for IPC3 build some of the removed num_of calculations fixes the test issue? In the above code snippet, for the else if call before calling module_prepare(). |
ipc4 They are done in bind & unbind(). But we need to keep it for ipc3. Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com> Signed-off-by: Rander Wang <rander.wang@intel.com>
bcedb1b to
47f4c21
Compare
thanks, I got the same idea. IPC3 doesnt has bind & unbind |
|
Looks better now, let's wait for the mandatory CI runs to complete and if all good, this is ready to be merged. |
|
CI almost clean. I do see one fail here: Looks like a case of thesofproject/linux#4832 but triggered in a different test case. |
|
@RanderWang Can you check the above fail? I browsed through history of PR tests and I don't see this failing for any other PR. |
@kv2019i it was a old bug hard to be reproduced. Let run it again. [ 53.980498] <err> dai_comp: dai_common_new: comp:1 0x10004 dai_new(): dai_get() failed to create DAI.
[ 53.980518] <err> copier: copier_dai_create: comp:1 0x10004 failed to create dai |
|
SOFCI TEST |
|
@kv2019i pass. It is unlucky that it can't be reproduced again. |
After thesofproject#8594 been merged, sources and sinks are now setup on .bind() and .unbind(). Previously they were setup in .prepare(). However, there were code left in module_adapter_reset() which clears sources and sinks arrays. That broke some tests: modules which use source/sink API stopped working correctly after pipeline reset. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
After #8594 been merged, sources and sinks are now setup on .bind() and .unbind(). Previously they were setup in .prepare(). However, there were code left in module_adapter_reset() which clears sources and sinks arrays. That broke some tests: modules which use source/sink API stopped working correctly after pipeline reset. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
After #8594 been merged, sources and sinks are now setup on .bind() and .unbind(). Previously they were setup in .prepare(). However, there were code left in module_adapter_reset() which clears sources and sinks arrays. That broke some tests: modules which use source/sink API stopped working correctly after pipeline reset. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>

Module will update source & sink information when bind & unbind event happen.