-
Notifications
You must be signed in to change notification settings - Fork 349
topology2: add dummy wov pipeline to nocodec #6782
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
topology2: add dummy wov pipeline to nocodec #6782
Conversation
b065b0a to
ef4fa5e
Compare
|
Patches are updated to reuse cavs-nocodec based on 8938c31 |
ef4fa5e to
58dcbaa
Compare
lgirdwood
left a comment
There was a problem hiding this 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.
tools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf
Outdated
Show resolved
Hide resolved
58dcbaa to
e7f0613
Compare
ce6edf8 to
fc3cf85
Compare
tools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf
Outdated
Show resolved
Hide resolved
|
V3 updated: remove enabling WoV on sof-mtl-nocodec. This is because WoV is waiting for Chao's patch on hw_params updating. |
fc3cf85 to
b5705ce
Compare
|
v4 update:
|
tools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf
Outdated
Show resolved
Hide resolved
tools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is this determined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from CoE team.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry what is cps?
There was a problem hiding this comment.
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.
tools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf
Outdated
Show resolved
Hide resolved
tools/topology/topology2/include/pipelines/cavs/passthrough-wov.conf
Outdated
Show resolved
Hide resolved
|
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. |
|
v9 update:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry what is cps?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
0d75f94 to
cd2f990
Compare
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
094a87d to
d2b8cdb
Compare
|
v10 update:
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
d2b8cdb to
fe5c4f4
Compare
|
v11 update: |
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>
fe5c4f4 to
5dc2a8f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why CAVS?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
5dc2a8f to
a25f4f6
Compare
|
v12 update:
|
ranj063
left a comment
There was a problem hiding this 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
Add dummy WoV support to nocodec topology on MTL platform.

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.