-
Notifications
You must be signed in to change notification settings - Fork 327
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
ASRC: move to DRAM #9844
ASRC: move to DRAM #9844
Conversation
Mark non-performance critical code with the __cold attribute. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
memory.h is required for __cold and __cold_rodata, add it. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
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.
LGTM
@@ -533,7 +560,7 @@ static enum asrc_error_code initialise_filter(struct comp_dev *dev, | |||
/* Reset coefficients for possible exit with error. */ | |||
src_obj->filter_length = 0; | |||
src_obj->num_filters = 0; | |||
src_obj->polyphase_filters = NULL; |
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.
Looks like this was never needed for anything else but to potentially find bugs where the filter is accessed uninitialized. For that purpose it could still be valid there.
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.
@jsarha it is now inside the asrc_free_polyphase_filter()
function and I think it can be needed when called from asrc_set_fs_ratio()
because of #9845 I cannot run-time test this, so perhaps it's better to not merge it before it's been tested |
Waiting for thesofproject/linux#5335 to be merged |
@@ -321,6 +321,7 @@ static int asrc_free(struct processing_module *mod) | |||
|
|||
rfree(cd->buf); | |||
asrc_release_buffers(cd->asrc_obj); |
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.
can you explain why is this needed? Is sram much smaller?
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.
Yes, its smaller/faster.
@lyakh can you check CI, looks like valgrind has picked up something in testbench test. |
Free SRAM by moving ASRC coefficients to DRAM and only copying the single required array on demand. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
CI:
|
Move ASRC filter coefficients and some DRC functions to DRAM