Skip to content

Conversation

@libinyang
Copy link
Contributor

@libinyang libinyang commented Dec 13, 2022

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
Copy link
Contributor Author

libinyang commented Dec 13, 2022

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 libinyang force-pushed the dummy_wov_tplg_submit branch from b065b0a to ef4fa5e Compare December 13, 2022 11:52
@libinyang
Copy link
Contributor Author

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

@libinyang libinyang force-pushed the dummy_wov_tplg_submit branch from ef4fa5e to 58dcbaa Compare December 13, 2022 12:26
@libinyang libinyang changed the title Dummy wov tplg submit topology2: dummy wov pipeline support Dec 14, 2022
@mengdonglin mengdonglin changed the title topology2: dummy wov pipeline support topology2: add dummy wov pipeline to nocodec Dec 14, 2022
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 need inputs from others here.

@libinyang libinyang force-pushed the dummy_wov_tplg_submit branch from 58dcbaa to e7f0613 Compare December 15, 2022 06:36
@libinyang libinyang force-pushed the dummy_wov_tplg_submit branch 2 times, most recently from ce6edf8 to fc3cf85 Compare December 19, 2022 05:31
@libinyang
Copy link
Contributor Author

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

@libinyang libinyang force-pushed the dummy_wov_tplg_submit branch from fc3cf85 to b5705ce Compare December 19, 2022 07:34
@libinyang
Copy link
Contributor Author

v4 update:

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

Copy link
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
Contributor Author

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
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
Contributor Author

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
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
Contributor Author

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.

@marc-hb
Copy link
Collaborator

marc-hb commented Dec 19, 2022

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
Contributor Author

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
Contributor Author

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
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
Collaborator

@ranj063 ranj063 Dec 20, 2022

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
Contributor Author

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
Contributor Author

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.

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
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?

@libinyang libinyang force-pushed the dummy_wov_tplg_submit branch from 0d75f94 to cd2f990 Compare December 21, 2022 03:00
Copy link
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
Contributor Author

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.

@libinyang libinyang force-pushed the dummy_wov_tplg_submit branch 2 times, most recently from 094a87d to d2b8cdb Compare December 21, 2022 04:07
@libinyang
Copy link
Contributor Author

libinyang commented Dec 21, 2022

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
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
Contributor Author

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

@libinyang libinyang force-pushed the dummy_wov_tplg_submit branch from d2b8cdb to fe5c4f4 Compare December 21, 2022 07:17
@libinyang
Copy link
Contributor Author

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>
@libinyang libinyang force-pushed the dummy_wov_tplg_submit branch from fe5c4f4 to 5dc2a8f Compare December 21, 2022 07:22
Copy link
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
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
Contributor Author

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
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 libinyang force-pushed the dummy_wov_tplg_submit branch from 5dc2a8f to a25f4f6 Compare December 22, 2022 02:05
@libinyang
Copy link
Contributor Author

v12 update:

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

Copy link
Collaborator

@ranj063 ranj063 left a comment

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:main Dec 22, 2022
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.

7 participants