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

board: intel_adsp_ace15_mtpm: Enable AEC mock module by default #8496

Closed
wants to merge 1 commit into from
Closed

board: intel_adsp_ace15_mtpm: Enable AEC mock module by default #8496

wants to merge 1 commit into from

Conversation

yongzhi1
Copy link
Contributor

@yongzhi1 yongzhi1 commented Nov 18, 2023 •
edited
Loading

This patch enables Google RTC mock module by default for CI build tests, etc.

Also avoid manually applying the changes each time to build the FW for AEC sanity check.

kv2019i
kv2019i reviewed Nov 22, 2023
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Hmm, not sure about this, whatever is in this conf gets shipped to upstream as-is.

@yongzhi1
Copy link
Contributor Author

yongzhi1 commented Nov 22, 2023 •
edited
Loading

Hmm, not sure about this, whatever is in this conf gets shipped to upstream as-is.

We have RTC_AEC defined in mtl.toml, so AEC enabled tplg will load successfully and exposed to user space, but we cannot use "DMIC0 RTC AEC" without a mock module.

kv2019i
kv2019i approved these changes Nov 23, 2023
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

If it's in the toml, then the cat is already out of the bag. No harm in this I guess, but if we hit binary size issues, this will be high on the list of modules to drop.

kv2019i
kv2019i requested changes Nov 24, 2023
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Hmm, I see build errors in CI, please take a look @yongzhi1

@lgirdwood
Copy link
Member

@marcinszkudlinski @ranj063 good for you ?

lgirdwood
lgirdwood approved these changes Nov 24, 2023
View reviewed changes
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

LGTM, but lets see if Marcin/Ranjani also ready or do we have other dependencies to merge 1st for topology or DP.

@yongzhi1
Copy link
Contributor Author

SOFCI TEST

@yongzhi1
Copy link
Contributor Author

Hmm, I see build errors in CI, please take a look @yongzhi1

The build errors should be fixed in current main now, thanks,

@lgirdwood
Copy link
Member

@yongzhi1 there was an issue last week, if CI results dont improve can you rebase and force-push. Thanks.

@yongzhi1 yongzhi1 force-pushed the build-aec branch 2 times, most recently from f3281b1 to a872a43 Compare November 27, 2023 17:55
@yongzhi1
Copy link
Contributor Author

yongzhi1 commented Nov 27, 2023 •
edited
Loading

@yongzhi1 there was an issue last week, if CI results dont improve can you rebase and force-push. Thanks.

Oh, the code is broken again due to a62b7cf

EDIT:
Fixup submitted #8536

@lgirdwood
Copy link
Member

@yongzhi1 there was an issue last week, if CI results dont improve can you rebase and force-push. Thanks.

Oh, the code is broken again due to a62b7cf

EDIT: Fixup submitted #8536

Fix now merged, can you try force push now ?

@yongzhi1
board: intel_adsp_ace15_mtpm: Enable AEC mock module by default
e010d0b 
This patch enables Google RTC mock module by default for CI build tests, etc.

Signed-off-by: Yong Zhi <[email protected]>
@yongzhi1 yongzhi1 marked this pull request as draft November 30, 2023 17:00
@yongzhi1
Copy link
Contributor Author

Maybe we can use the following one already done by @marcinszkudlinski
c90eb8b

@lgirdwood
Copy link
Member

Maybe we can use the following one already done by @marcinszkudlinski c90eb8b

Do you mean close this PR ? if so , pls close. Thanks !

@marcinszkudlinski
Copy link
Contributor

yes. please close this.
I'm still waiting for feedback from 007 before proceeding with AEC upstream.

@yongzhi1 yongzhi1 closed this Dec 6, 2023
@yongzhi1 yongzhi1 deleted the build-aec branch December 6, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kv2019i kv2019i kv2019i requested changes

@lyakh lyakh lyakh approved these changes

@lgirdwood lgirdwood lgirdwood approved these changes

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@lbetlej lbetlej Awaiting requested review from lbetlej lbetlej is a code owner

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yongzhi1 @lgirdwood @marcinszkudlinski @lyakh @kv2019i