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

Audio: Volume: Clear peak meter channel max values in prepare() #9411

Merged

Conversation

singalsu
Copy link
Collaborator

This patch avoids garbage values to be sent to mailbox by peak_vol_update() as very first peak volume measurement in stream start. The data structures cd->peak_vol and cd->peak_regs.peak_meter were not cleared.

@singalsu singalsu marked this pull request as ready for review August 28, 2024 07:46
@lgirdwood
Copy link
Member

@kv2019i pls review/merge

@@ -147,6 +147,7 @@ int volume_init(struct processing_module *mod)
return -ENOMEM;
}

memset(cd->peak_vol, 0, vol_size);
Copy link
Collaborator

Choose a reason for hiding this comment

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

just rzalloc() above. BTW, is the .peak_vol allocation needed if CONFIG_COMP_PEAK_VOL is undefined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's spread to nearly every volume source file, I'll add it to another commit.

Copy link
Collaborator Author

@singalsu singalsu Sep 16, 2024

Choose a reason for hiding this comment

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

We have always for IPC4 systems CONFIG_COMP_PEAK_VOL=y, I don't think it makes sense to make a quite large patch in rush for 2.11. It would make sense if we would have another volume component for ChromeOS and Linux those do not use the peak values reporting from volume.

@lgirdwood lgirdwood added this to the v2.11 milestone Sep 13, 2024
@lgirdwood
Copy link
Member

@singalsu any chance you can get this in om Monday ? @kv2019i fyi - would be nice to fix in v2.11

@singalsu
Copy link
Collaborator Author

@singalsu any chance you can get this in om Monday ? @kv2019i fyi - would be nice to fix in v2.11

Yep, looks like a simple fix & retest.

This patch avoids garbage values to be sent to mailbox by
peak_vol_update() as very first peak volume measurement in
stream start. The data structures cd->peak_vol and
cd->peak_regs.peak_meter were not cleared.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@singalsu singalsu force-pushed the peakvolume_clear_max_level_values branch from 9cb1ee4 to 09417f5 Compare September 16, 2024 08:47
@singalsu singalsu requested a review from lyakh September 16, 2024 08:47
@kv2019i
Copy link
Collaborator

kv2019i commented Sep 16, 2024

Ack @lgirdwood -- I couldn't get any PRs through on Friday (waiting on 9472) through Intel internal CI, so that's a potential problem for today as well. So in practise it depends on now 9472. We can't tag rc1 before that is in, so if we have other PRs ready before that, we can include.

@lgirdwood
Copy link
Member

@wszypelt @lrudyX good to merge ?

@kv2019i kv2019i merged commit 1b08d6c into thesofproject:main Sep 18, 2024
44 of 47 checks passed
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