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.

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

Jump to bottom

[BUG] ramp time extended by 0.03s after capture gain curve_duration change #8238

Closed
keqiaozhang opened this issue Sep 21, 2023 · 8 comments · Fixed by #8242
Closed

[BUG] ramp time extended by 0.03s after capture gain curve_duration change #8238

keqiaozhang opened this issue Sep 21, 2023 · 8 comments · Fixed by #8242
Assignees
@singalsu
Labels
bug Something isn't working as expected I2S Applies to I2S bus for codec connection Intel Linux Daily tests This issue can be found in internal Linux daily tests MTL Applies to Meteor Lake platform P1 Blocker bugs or important features TGL Applies to Tiger Lake

Comments

@keqiaozhang
Copy link
Collaborator

keqiaozhang commented Sep 21, 2023 •
edited
Loading

Describe the bug
Observed this issue on all IPC4-nocodec(single core) platforms. it caused alsabat test failures.
The ramp time extended by 0.03s after capture gain curve_duration change(d0d74a4):
image

To Reproduce
check the alsabat wav file

Reproduction Rate
100%

Environment

  1. Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).
  2. Name of the topology file
    • Topology: {nocodec tplg}
  3. Name of the platform(s) on which the bug is observed.
    • Platform: {NOCODEC-IPC4 platforms}
@keqiaozhang keqiaozhang added bug Something isn't working as expected P1 Blocker bugs or important features I2S Applies to I2S bus for codec connection labels Sep 21, 2023
@keqiaozhang keqiaozhang changed the title [BUG] ramp time extended by 0.03s after clock source change for all SSPs [BUG] ramp time extended by 0.03s after capture gain curve_duration change Sep 21, 2023
@marc-hb
Copy link
Collaborator

marc-hb commented Sep 21, 2023

The commit message says "This change increases the ramp duration from 20 ms to 50 ms"

So it's not a bug, it's a feature? :-)

@keqiaozhang
Copy link
Collaborator Author

So it's not a bug, it's a feature? :-)

Not sure about this, but alsabat cannot survive from such long ramp time.

@singalsu
Copy link
Collaborator

singalsu commented Sep 21, 2023 •
edited
Loading

Ouch, but there's justification for this change. 1) capture artefacts mitigation accidentally dropped from tplg2 vs. tplg1 2) we can avoid a big MCPS peak with this lower rate ramp.

Is there a way to ask alsabat to ignore a bit longer audio from beginning?

@singalsu
Copy link
Collaborator

If not possible to change alsabat we can have a test topology with short ramp?

@singalsu singalsu mentioned this issue Sep 21, 2023
Merged
@kv2019i kv2019i mentioned this issue Sep 21, 2023
Merged
@kv2019i kv2019i added TGL Applies to Tiger Lake MTL Applies to Meteor Lake platform labels Sep 21, 2023
singalsu added a commit to singalsu/sof that referenced this issue Sep 21, 2023
@singalsu
Tools: Topology2: Remove 50 ms curve duration set from nocodec
b4dc316 
We need to care about audio user experience and peak MCPS
usage in production topologies.

The alsabat test is disturbed by longer ramp so it can be removed
from nocodec topologies since the codec is never used by end
users and also the peak MCPS mitigation is not relevant for
a test topology, as long as higher MCPS is not triggering
error reports. The curve duration becomes without explicit set
the default 20 ms.

Fixes: thesofproject#8238
Fixes: d0d74a4
       ("Tools: Topology2: Change in capture gain
       curve_duration to 50 m")

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@singalsu singalsu mentioned this issue Sep 21, 2023
Merged
@singalsu
Copy link
Collaborator

I could not find from alsabat command line options to ignore audio from very beginning of test. I don't like at all to change something because of limitation in our test procedure but this might help #8242 .

singalsu added a commit to singalsu/sof that referenced this issue Sep 21, 2023
@singalsu
Tools: Topology2: Remove 50 ms curve duration set from nocodec
2f163ad 
We need to care about audio user experience and peak MCPS
usage in production topologies.

The alsabat test is disturbed by the longer ramp so the
change can be reverted from nocodec topologies. Those
topologies are never used by end users. Also the peak MCPS
mitigation is not relevant for test topologies, as long as
higher MCPS is not triggering error reports. The curve
duration is restored without explicit set to the default 20 ms.

Fixes: thesofproject#8238
Fixes: d0d74a4
       ("Tools: Topology2: Change in capture gain
       curve_duration to 50 m")

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@kv2019i kv2019i mentioned this issue Sep 22, 2023
Merged
@lgirdwood
Copy link
Member

@singalsu ack, the topology used for the test can have its ramp shortened, providing its not a end user topology. i.e. testing only topology.

@marc-hb marc-hb added the Intel Linux Daily tests This issue can be found in internal Linux daily tests label Sep 22, 2023
lgirdwood pushed a commit that referenced this issue Sep 25, 2023
@singalsu @lgirdwood
Tools: Topology2: Remove 50 ms curve duration set from nocodec
2cdd11a 
We need to care about audio user experience and peak MCPS
usage in production topologies.

The alsabat test is disturbed by the longer ramp so the
change can be reverted from nocodec topologies. Those
topologies are never used by end users. Also the peak MCPS
mitigation is not relevant for test topologies, as long as
higher MCPS is not triggering error reports. The curve
duration is restored without explicit set to the default 20 ms.

Fixes: #8238
Fixes: d0d74a4
       ("Tools: Topology2: Change in capture gain
       curve_duration to 50 m")

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@marc-hb
Copy link
Collaborator

marc-hb commented Sep 25, 2023

Let's wait for validation in the next daily test run.

Stupid Github auto close anti-feature: https://github.com/orgs/community/discussions/17308

@marc-hb marc-hb reopened this Sep 25, 2023
@keqiaozhang
Copy link
Collaborator Author

keqiaozhang commented Sep 26, 2023 •
edited
Loading

Confirmed that alsabat test passed after applying #8242 on nocodec platforms. Closing this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@singalsu singalsu

Labels
bug Something isn't working as expected I2S Applies to I2S bus for codec connection Intel Linux Daily tests This issue can be found in internal Linux daily tests MTL Applies to Meteor Lake platform P1 Blocker bugs or important features TGL Applies to Tiger Lake
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Tools: Topology2: Remove 50 ms curve duration set from nocodec singalsu/sof
5 participants
@lgirdwood @keqiaozhang @singalsu @kv2019i @marc-hb