Ipc4 : add google rtc AEC module support by RanderWang · Pull Request #125 · thesofproject/rimage

RanderWang · 2023-01-11T02:45:40Z

No description provided.

config/tgl-cavs.toml

RanderWang · 2023-01-11T03:20:28Z

FW PR: thesofproject/sof#6927

plbossart · 2023-01-11T03:48:10Z

FW PR: thesofproject/sof#6927

this is about the Google RTC AEC, what;s the link with the Windows reference you made above @RanderWang ?

RanderWang · 2023-01-11T09:07:55Z

FW PR: thesofproject/sof#6927

this is about the Google RTC AEC, what;s the link with the Windows reference you made above @RanderWang ?

Thanks, I got it. I may misunderstand that google AEC is sof AEC. I will add entry for google AEC

lgirdwood · 2023-01-11T13:56:33Z

@johnylin76 @cujomalainey pls review

config/mtl.toml

config/tgl-cavs.toml

RanderWang · 2023-02-03T05:39:31Z

rebase to latest code

lgirdwood · 2023-02-03T14:43:33Z

@plbossart update good for you ?

lgirdwood · 2023-02-14T16:23:49Z

@RanderWang conflict.

RanderWang · 2023-02-15T07:28:14Z

@RanderWang conflict.

Thanks, updated

cujomalainey

whoops forgot to publish these on the previous version

cujomalainey · 2023-02-14T20:25:45Z

config/mtl.toml

+        [[module.entry]]
+        name = "RTC_AEC"
+        uuid = "B780A0A6-269F-466F-B477-23DFA05AF758"
+        affinity_mask = "0x3"


curious why this is not 0xF?

It is a duplication of general AEC. Let's use 0xF for RTC AEC

cujomalainey · 2023-02-14T20:26:08Z

config/mtl.toml

+        name = "RTC_AEC"
+        uuid = "B780A0A6-269F-466F-B477-23DFA05AF758"
+        affinity_mask = "0x3"
+        instance_count = "1"


why 1? What if we want to have one on HS and DMIC?

ok, let's use 2

@mwasko could you clarify here, if the ext manifest says only 1 module instance but topology says 2 what should happen ?

@mwasko could you clarify here, if the ext manifest says only 1 module instance but topology says 2 what should happen ?

In such case FW should report an error on attempt to create second module instance.

ok, wont this mean generating differnt baseFWs for different topologies ?

@mwasko @lgirdwood can I ask a somewhat unrelated question related to the affinity_mask? Is there a good reason why some modules are restricted to core0, some to core0/1, and most do not have any restrictions.

I am specifically concerned about the core0 restriction, which I view as incompatible with the direction to allow low-latency pipelines to run on all cores. if a mixin is always on core0, then something's not adding up.

@plbossart agreed, this seems to put component level restrictions on topology configuration without context. I would expect more restrictions like "MIPS estimates cannot violate this amount per core in worst case scenarios" and the stack check for this platform.

@mwasko @lgirdwood can I ask a somewhat unrelated question related to the affinity_mask? Is there a good reason why some modules are restricted to core0, some to core0/1, and most do not have any restrictions.

This field is used by Windows driver that has more flexible approach on how pipelines and modules are instantiated. The pipeline/component is assigned to specific core based on current CPU resource availability (MCPS).

I am specifically concerned about the core0 restriction, which I view as incompatible with the direction to allow low-latency pipelines to run on all cores. if a mixin is always on core0, then something's not adding up.

Originally Windows was grouping LL modules on Core 0 and I believe that is why Rander defined it like that. However, @plbossart is right here and with SOF we allow low-latency pipelines to run on all cores so all the built-in modules should not have affinity restrictions. The 3rd party modules affinity is up to integrator but the default option should be 'no restrictions'.

Thanks @mwasko, your explanations match the little understanding I have of the modules.

@RanderWang can we have a follow-up patch to clean-up the affinity definitions for built-in modules, i.e. remove the restriction to core0 and core0-1 for all built-in modules? Thanks!

sure, I will follow up it.

config/mtl.toml

Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang · 2023-03-06T03:07:02Z

@cujomalainey sorry, update late.

cujomalainey

Also this spec is impossible to read at face value without the docs with you. Would be great to make the parser smarter to handle enums

config/tgl-cavs.toml

RanderWang requested a review from lgirdwood January 11, 2023 02:45

plbossart requested changes Jan 11, 2023

View reviewed changes

config/tgl-cavs.toml Outdated Show resolved Hide resolved

RanderWang force-pushed the ipc4-eac branch 2 times, most recently from 0ed6676 to 7bdf996 Compare January 11, 2023 09:14

RanderWang changed the title ~~Ipc4 : add AEC module support~~ Ipc4 : add google rtc AEC module support Jan 11, 2023

cujomalainey requested review from cujomalainey and perahgren January 11, 2023 21:01

cujomalainey reviewed Jan 11, 2023

View reviewed changes

config/mtl.toml Show resolved Hide resolved

config/tgl-cavs.toml Show resolved Hide resolved

RanderWang mentioned this pull request Jan 31, 2023

rimage: add ipc4 extended module config introduction thesofproject/sof-docs#451

Open

RanderWang force-pushed the ipc4-eac branch 2 times, most recently from 0406efa to 049b209 Compare February 3, 2023 05:38

lgirdwood previously approved these changes Feb 8, 2023

View reviewed changes

RanderWang dismissed lgirdwood’s stale review via 5b75326 February 15, 2023 07:27

RanderWang force-pushed the ipc4-eac branch from 049b209 to 5b75326 Compare February 15, 2023 07:27

lgirdwood previously approved these changes Feb 20, 2023

View reviewed changes

cujomalainey reviewed Feb 21, 2023

View reviewed changes

RanderWang added 2 commits March 6, 2023 11:03

ipc4: add google rtc AEC support for mtl

9d1d256

Signed-off-by: Rander Wang <rander.wang@intel.com>

ipc4: add google rtc AEC support in config

083f295

Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang dismissed lgirdwood’s stale review via 083f295 March 6, 2023 03:06

RanderWang force-pushed the ipc4-eac branch from 5b75326 to 083f295 Compare March 6, 2023 03:06

cujomalainey suggested changes Mar 7, 2023

View reviewed changes

config/tgl-cavs.toml Show resolved Hide resolved

RanderWang closed this Jul 19, 2023

RanderWang deleted the ipc4-eac branch July 19, 2023 06:33

RanderWang mentioned this pull request Aug 23, 2023

[BUG] [MTL] set pipeline state returned error when running RTC AEC on secondary core thesofproject/sof#8024

Closed

Conversation

RanderWang commented Jan 11, 2023

Uh oh!

Uh oh!

RanderWang commented Jan 11, 2023

Uh oh!

plbossart commented Jan 11, 2023

Uh oh!

RanderWang commented Jan 11, 2023

Uh oh!

lgirdwood commented Jan 11, 2023

Uh oh!

Uh oh!

Uh oh!

RanderWang commented Feb 3, 2023

Uh oh!

lgirdwood commented Feb 3, 2023

Uh oh!

lgirdwood commented Feb 14, 2023

Uh oh!

RanderWang commented Feb 15, 2023

Uh oh!

cujomalainey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

RanderWang commented Mar 6, 2023

Uh oh!

cujomalainey left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cujomalainey left a comment •

edited

Loading