Revert "module_adapter: dp: Decrease default heap size from 20k to 16k"#10689
Conversation
This reverts commit a32d983. It looks like this causes more trouble than it solves. 16k is not enough heap for 11025Hz to 48000Hz conversion. The inability to have two SRCs active at the same time is not as critical issue as failing some conversions completely. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
There was a problem hiding this comment.
Pull request overview
Reverts a prior change that reduced the module adapter DP heap allocation from 20 KiB to 16 KiB, restoring the larger default to avoid failures in certain sample-rate conversions (e.g., 11025 Hz → 48000 Hz) where 16 KiB is insufficient.
Changes:
- Restore
buf_sizedefault from 16 KiB back to 20 KiB in the module adapter DP heap allocation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
It's a shame jenkins CI is not available now, but custom tests show this does solve the 11025Hz conversion case. #10684 could also help further with being able to use DSP topologies with more concurrent SRCs, and in longer term, ability to pass the heap size from topology (per module) will fix this in more maintainable way (thesofproject/linux#5537 ).
|
@jsarha I think we do test cases with two SRC instances at the same time - one for capture and one for playback, as configured in nocodec topologies. But I think those tests passed initially before the commit, that you're reverting here was added. Unfortunately I don't have any sof-test test results for this revert. |
|
@kv2019i , Its ok by me. But now that I browsed the JIRA history, there was a test-case failing also due to multiple SRCs not able to configure at the same time. The case was: TPLG=/lib/firmware/intel/development/sof-ptl-nocodec.tplg MODEL=PTLH_RVP_NOCODEC SOF_TEST_INTERVAL=5 ~/sof-test/test-case/check-capture.sh -d 1 -l 1 -r 50 But still I think the failure to do some conversions completely is worse than not able to pass some test due to limited resources. |
|
Ack @jsarha , let's proceed with merging this and bump priority of thesofproject/linux#5537 so we can get away from a one-size-fits-all module heap size. |
This reverts commit a32d983.
It looks like this causes more trouble than it solves. 16k is not enough heap for 11025Hz to 48000Hz conversion. The inability to have two SRCs active at the same time is not as critical issue as failing some conversions completely.