Skip to content

topology2: add dummy wov pipeline to nocodec - #6782

Merged
lgirdwood merged 8 commits into
thesofproject:mainfrom
libinyang:dummy_wov_tplg_submit
Dec 22, 2022
Merged

topology2: add dummy wov pipeline to nocodec#6782
lgirdwood merged 8 commits into
thesofproject:mainfrom
libinyang:dummy_wov_tplg_submit

Conversation

@libinyang

@libinyanglibinyang commented Dec 13, 2022

Copy link
Copy Markdown
Contributor

Add dummy WoV support to nocodec topology on MTL platform.
sof-mtl-nocodec

The topology of dummy WoV is:
DMIC ------> KPB ------------------------------> host_copier
| ^
| |
|----> micsel ----> dummy wov ----> virtual_widget

The connection between virtual_widget and host_copier is a virtual connection, and the virtual widget doesn't exist in the FW.

PS: there seems to be an issue that DMIC0 and DMIC1 can't co-work in the topology2. If I enabled both DMICs, the nhlt blob will complain doesn't support the audio format which is used by dummy WoV. After this issue is fixed, the cavs-nocodec-wov.conf is not necessary and it can reuse cavs-nocodec.conf.

@libinyang

libinyang commented Dec 13, 2022

Copy link
Copy Markdown
ContributorAuthor

Thank @juimonen 's reminder, @singalsu had a PR #6668 which can fix the dmic issue. I will refine this PR to reuse cavs-nocodec.conf.

@libinyang

Copy link
Copy Markdown
ContributorAuthor

Patches are updated to reuse cavs-nocodec based on 8938c31

@libinyanglibinyang changed the title Dummy wov tplg submittopology2: dummy wov pipeline supportDec 14, 2022
@mengdonglinmengdonglin changed the title topology2: dummy wov pipeline supporttopology2: add dummy wov pipeline to nocodecDec 14, 2022

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but need inputs from others here.

Comment threadtools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-generic.conf Outdated
Comment threadtools/topology/topology2/sof-ace-tplg/CMakeLists.txt Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-default.conf Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-default.conf Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-generic.conf Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-wov.conf Outdated
@libinyang
libinyangforce-pushed the dummy_wov_tplg_submit branch 2 times, most recently from ce6edf8 to fc3cf85CompareDecember 19, 2022 05:31
Comment threadtools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf Outdated
@libinyang

Copy link
Copy Markdown
ContributorAuthor

V3 updated: remove enabling WoV on sof-mtl-nocodec. This is because WoV is waiting for Chao's patch on hw_params updating.

@libinyang

Copy link
Copy Markdown
ContributorAuthor

v4 update:

  1. add the blob's struct description of ipc4 header

Comment threadtools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf Outdated
Comment threadtools/topology/topology2/include/components/micsel.conf Outdated
Comment threadtools/topology/topology2/include/components/micsel.conf Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

how is this determined?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Copied from CoE team.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@libinyang when you give someone an answer like this, you should know what the next question is going to be right? Please go back to them and find out how this was derived and add a comment. Thanks!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

cps = cpc * sampling frequency // sample group in ms * frame size, which can be found in the platform config file. So you can calculate the cpc value.
ps: I won't add the cpc calculation description in the patch

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sorry what is cps?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

sorry what is cps?

I think CPS is "Cycles Per Second" after grep the sof fw src code.

Cited more from CoE team
"frame_size - number of milliseconds that one frame consists of.
cpc - number of cycles per chunk, chunk is a data processed during one frame. "

And cps value can be found in the file platform config file in the rimage.

Comment threadtools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-wov.conf Outdated
Comment threadtools/topology/topology2/cavs-nocodec.conf Outdated
Comment threadtools/topology/topology2/cavs-nocodec.conf Outdated
@marc-hb

Copy link
Copy Markdown
Collaborator

sof-test has a https://github.com/thesofproject/sof-test/blob/main/test-case/check-keyword-detection.sh test that we could never use so far (see thesofproject/sof-test#859 and links from there).

Will this test be finally usable? If not, how is this PR being tested?

Disclaimer: I have NOT looked at this PR.

@libinyang

Copy link
Copy Markdown
ContributorAuthor

sof-test has a https://github.com/thesofproject/sof-test/blob/main/test-case/check-keyword-detection.sh test that we could never use so far (see thesofproject/sof-test#859 and links from there).

Will this test be finally usable? If not, how is this PR being tested?

Disclaimer: I have NOT looked at this PR.

@marc-hb I didn't really enable WoV in CMakefile. We still need @aiChaoSONG PR of updating the hw_params. Without @aiChaoSONG ' patch, there will be some failure in test WoV. After Chao's patch is merged, I will enable WoV in CMakefile.

@libinyang

Copy link
Copy Markdown
ContributorAuthor

v9 update:

  1. use wov-detect as a general wov detection pipeline
  2. use conf file as the input of wov kcontrol data
  3. some other small changes as @ranj063 suggested.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sorry what is cps?

Comment threadtools/topology/topology2/include/components/wov.conf Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-default.conf Outdated
Comment threadtools/topology/topology2/cavs-nocodec.conf Outdated
Comment threadtools/topology/topology2/cavs-nocodec.conf Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-generic.conf Outdated

@ranj063ranj063Dec 20, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you please remind me what would happen if we dont set the cpc?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Can you please remind me what would happen if we dont set the cpc?

This is a good question! I tried not to set cpc before when enabling wov SOF fw. It seemed to work even without cpc setting. Maybe we need CoE expert help to answer this question. @iganakov

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

And maybe we can enhance the fw to set the cpc in FW itself and don't need tplg setting it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

was also wondering if the cpc is needed in every widget, we should probably add it to base class etc. I guess it is originally used to count the pipeline cycles so it has "room" to run, not sure what is the use in practice.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if we dont need cpc for wov, is it needed for kpb and micsel?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Capture 18? Can we call this "WoV Capture" instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

DMIC1 is a general usage, it can be used for other purposes.

Comment threadtools/topology/topology2/platform/intel/dmic-default.conf Outdated
Comment threadtools/topology/topology2/cavs-nocodec.conf Outdated
@libinyang
libinyangforce-pushed the dummy_wov_tplg_submit branch 2 times, most recently from 094a87d to d2b8cdbCompareDecember 21, 2022 04:07
@libinyang

libinyang commented Dec 21, 2022

Copy link
Copy Markdown
ContributorAuthor

v10 update:

  1. refine payload_with_output_fmt token based on thesofproject/linux@831a267
  2. update kpb cpc
  3. some other tiny changes based on @ranj063 's comments.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Passthrough Capture 18 makes it even worse. Please call it something more indicative of what it is. DMIC1 capture?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This is defined in dmic-default.conf, cavs-nocodec.conf will re-define it as 'Capture 18'. I will change it to 'DMIC1 WOV capture" in cavs-nocodec.conf

Comment threadtools/topology/topology2/platform/intel/dmic-wov.conf Outdated
Comment threadtools/topology/topology2/platform/intel/dmic-wov.conf Outdated
@libinyang

Copy link
Copy Markdown
ContributorAuthor

v11 update:
Some tiny changes based on @ranj063 's comments: remove the 'stream_name' define; rename the DMIC1_PCM_CAPS; refine the commit message.

The token payload_with_output_fmt is process module specific,
so set it as a process module token.
Signed-off-by: Chao Song <chao.song@linux.intel.com>
Signed-off-by: Libin Yang <libin.yang@intel.com>
Add the audio channel selection component support.
In case 1 output channel is selected, the component provides
the selected channel on output. In case 2 or 4 channels are selected on
output, the component works in a passthrough mode.
Signed-off-by: Libin Yang <libin.yang@intel.com>
Add the kpb component support. kpb component is used to transfer
the data to the proper pipeline based on the WOV component events.
Signed-off-by: Libin Yang <libin.yang@intel.com>
Add the wov module support. The keyword of the wov is the sound
of clap.
Signed-off-by: Libin Yang <libin.yang@intel.com>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why CAVS?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@libinyang even though this works. This is incorrect. What you need is a new pipeline for kpb-capture that has the DAI copier and the KPB connected

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@libinyang even though this works. This is incorrect. What you need is a new pipeline for kpb-capture that has the DAI copier and the KPB connected

Why do we need a different pipeline for kbp and DAI? The dmic-wov is only used for wov and I prefer it not to be re-used by other scenarios.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if we dont need cpc for wov, is it needed for kpb and micsel?

wov-detect pipeline contains a micsel component, a wov component
and a virtual component. micsel component accepts 2 channels audio data and
output 1 channel audio data to the wov component. wov's keyword is a clap.
The virtual component is only used by the driver, and the FW will not
know that there is a virtual component in the pipeline. The virtual
component is used to connect the pipeline to a host copier pipeline
as per drivers requirement.
Signed-off-by: Libin Yang <libin.yang@intel.com>
dai-kpb-be pipeline contains a copier DAI component and a kpb component.
Signed-off-by: Libin Yang <libin.yang@intel.com>
The dmic-wov.conf file implements the WOV feature. The connection is
DMIC ------> KPB ------------------------------> host_copier
| ^
| |
|----> micsel ----> wov ------> virtual_widget
The DMIC and KPB is on DAI ppl.
The host_copier is on Host ppl.
The micsel, wov and virtual is on wov ppl.
The connection between host_copier and virtual_widget is a
virtual connection. The virtual_widget doesn't exist in the FW.
Signed-off-by: Libin Yang <libin.yang@intel.com>
Add the definitions of wov in cavs-nocodec.conf.
Signed-off-by: Libin Yang <libin.yang@intel.com>
@libinyang

Copy link
Copy Markdown
ContributorAuthor

v12 update:

  1. add a dai-kpb-be pipeline instead of using passthrough-be.

@ranj063ranj063 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good @libinyang. The only open I have is the cpc. I think we need to do a broader cleanup of cpc's for all topologies. Lets plan for that next

@lgirdwood
lgirdwood merged commit 1447444 into thesofproject:mainDec 22, 2022
Sign up for freeto 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.

7 participants

@libinyang@marc-hb@juimonen@ranj063@bardliao@lgirdwood@singalsu