Skip to content
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

Tools: Topology2: Change in capture gain curve_duration to 50 ms #8208

Merged

Conversation

singalsu
Copy link
Collaborator

This change increases the ramp duration from 20 ms to 50 ms. It lowers the peak load of peak volume component due to longer same gain value blocks. The internal update rate for gain becomes 250 us instead of 125 us. The longer fade-in ramp also conceals better possible analog capture start transients.

This changes for 4ch capture for gain.11.1 in sof-hda-generic-4ch.tplg from

CPU_PEAK(MAX) = 21.95
PEAK(MAX)/AVG(AVG) = 7.51

to

CPU_PEAK(MAX) = 9.07
PEAK(MAX)/AVG(AVG) = 3.12

This change increases the ramp duration from 20 ms to 50 ms. It
lowers the peak load of peak volume component due to longer same
gain value blocks. The internal update rate for gain becomes 250 us
instead of 125 us. The longer fade-in ramp also conceals better
possible analog capture start transients.

This changes for 4ch capture for gain.11.1 in sof-hda-generic-4ch.tplg
from

CPU_PEAK(MAX) = 21.95
PEAK(MAX)/AVG(AVG) = 7.51

to

CPU_PEAK(MAX) = 9.07
PEAK(MAX)/AVG(AVG) = 3.12

Signed-off-by: Seppo Ingalsuo <[email protected]>
@singalsu singalsu requested a review from btian1 September 15, 2023 14:31
Copy link
Contributor

@btian1 btian1 left a comment

Choose a reason for hiding this comment

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

change to 40/50/60ms have same result for peak ramp results, better to have a check for the result.

@singalsu singalsu marked this pull request as ready for review September 18, 2023 11:33
@btian1 btian1 closed this Sep 19, 2023
@btian1 btian1 reopened this Sep 19, 2023
@lgirdwood lgirdwood merged commit d0d74a4 into thesofproject:main Sep 20, 2023
80 of 82 checks passed
@lgirdwood
Copy link
Member

@marcinszkudlinski @mwasko fyi - good improvement on MCPS

@lgirdwood
Copy link
Member

@wszypelt @lrudyX fyi - not sure what volume gain curve duration you guys are using in CI today.

@btian1
Copy link
Contributor

btian1 commented Sep 21, 2023

@singalsu @lgirdwood
https://sof-ci.ostc.intel.com/#/result/planresultdetail/32024

seems CI still failed, either change CI pass reference or revert this patch.

@mszleszy
Copy link

@lgirdwood tests uses different curve duration, and according to my knowledge it should not be static as curve length is being defined in InitInstance ipc or LargeConfigSet message of Gain module
example for 0.1s of fade in:
image

@singalsu
Copy link
Collaborator Author

@singalsu @lgirdwood https://sof-ci.ostc.intel.com/#/result/planresultdetail/32024

seems CI still failed, either change CI pass reference or revert this patch.

@btian1 If you mean issue #8238 the bug is in CI, not in topology or gain component. Alsabat or using alsabat from sof-test should have a way to ignore from start the ramp time.

@singalsu
Copy link
Collaborator Author

@lgirdwood tests uses different curve duration, and according to my knowledge it should not be static as curve length is being defined in InitInstance ipc or LargeConfigSet message of Gain module example for 0.1s of fade in: image

@mszleszy Yes, the gain component follows the curve_duration and curve_type request from init() IPC. But currently the curve duration and curve type from successive volume commands is ignored. Also we can't have different curve per channel.

@mszleszy
Copy link

mszleszy commented Sep 22, 2023

@singalsu as far as i know setting up different configs per channel was supported up to cavs2.5 It's not required any longer but the structure stayed the same for back compatibility.
I can speak only form fw QA perspective that PeakVol and Gain configuration worked pretty well so far, so any new fail on those modules looks like a regression. But if necessary we can do a deep dive for this case

@singalsu
Copy link
Collaborator Author

singalsu commented Sep 25, 2023

@singalsu as far as i know setting up different configs per channel was supported up to cavs2.5 It's not required any longer but the structure stayed the same for back compatibility. I can speak only form fw QA perspective that PeakVol and Gain configuration worked pretty well so far, so any new fail on those modules looks like a regression. But if necessary we can do a deep dive for this case

Yep, we should not attempt to tame the peak MCPS with ways those cause regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants