Topology2: add smart amp nocodec test topology #6799
Conversation
aiChaoSONG
requested review from
aborisovich,
dbaluta,
lbetlej,
lgirdwood,
marc-hb,
mmaka1 and
plbossart
as code owners
|
You write "test" topology but I don't see any "development" folder like in topology1. This means these will be released in sof-bin, is that intentional? |
|
@marc-hb This topology is just like the nocodec topology, and we do have the smart_amp module in the firmware. I think it is good. I am not sure if we want to move all nocodec to a new folder in the future, if yes, this one can be move with all nocodec topologies |
There was a problem hiding this comment.
NO.
The rule is that only topology files that are known to the kernel can be in the default topology directory.
Whenever we introduce a variant that requires a setting be modified, and a tplg_filename set, then it's clearly a development/test topology that needs to be treated as such.
We are not going to start pushing all kinds of test topology2 outside of intel, please be mindful of what is test and what is intended to be released.
There was a problem hiding this comment.
@plbossart @marc-hb Now I add a new sub-folder for cavs test topologies, and move this topology into it.
We should not have test modules in the base firmware. It's ok as a temporary step while the firmware team works on generating separate libraries, but it cannot be the long term direction.
agreed and sounds good to me, I think finally, we will make the smart amp test module a loadable one.
There was a problem hiding this comment.
Now I add a new sub-folder for cavs test topologies, and move this topology into it
Thanks! Validation can copy / "merge" this test folder into the production folder just before testing if needed. I hope we can keep the filenames different and collision-free.
No moving of files one by one please.
There was a problem hiding this comment.
the topology was not moved in the update, it's in tools/topology/topology2/cavs-smart-amp-nocodec.conf
Difficult to be more confusing.
That is in addition a fundamentally wrong assumption. We should not have test modules in the base firmware. It's ok as a temporary step while the firmware team works on generating separate libraries, but it cannot be the long term direction. |
aiChaoSONG
force-pushed
the
sma_tplg_upstream
branch
from
a080ca9 to
9ce2bae
Compare
plbossart
left a comment
plbossart
left a comment
There was a problem hiding this comment.
this needs a lot of work, not ready at all for merge.
There was a problem hiding this comment.
I don't know what is either but I know this would be much better to avoid double and triple negations:
| no_pm "true" | |
| pm "false" |
(I hope the default value can be either "true" or "false")
There was a problem hiding this comment.
sorry this cannot be changed. no_pm is an existing terminology used by ALSA topology and must be set to true for all widgets
There was a problem hiding this comment.
don't we need to specific which pin is used for what? I vaguely recall there was a special mapping needed
There was a problem hiding this comment.
its added in the third patch at the top-level topology file
There was a problem hiding this comment.
no this is wrong. We should have a different pipeline for host and dai, and the smart-amp should be part of the dai pipeline. Keep in mind we do want to have multiple PCMs for playback, all mixed with the result processed by the smart amp.
There was a problem hiding this comment.
why is this file not in a test directory? It's at the top level of topology2, this is wrong.
There was a problem hiding this comment.
I don't want to see ANY references to passthrough moving forward. I have repeated over and over that we do not do passthrough pipelines any longer. We want mixers on playback and demux on capture. Get on with the program please.
There was a problem hiding this comment.
its added in the third patch at the top-level topology file
There was a problem hiding this comment.
@aiChaoSONG after discussing with @plbossart, I think we have the smartamp included in the wrong pipeline. It needs to go after the mixout in the BE pipeline.
There was a problem hiding this comment.
why do we need a separate test folder for this feature? Why not include this in the nocodec tplg and let it get tested regularly?
There was a problem hiding this comment.
This was discussed above @ranj063. Because we don't want to release it in sof-bin. It can be tested regularly either way.
There was a problem hiding this comment.
can this be moved out to a separate conf file and included here? It is polluting the top-level tplg file
There was a problem hiding this comment.
in fact why can't all of this go into smart_amp.conf and we should only set the name here in the top-level
aiChaoSONG
force-pushed
the
sma_tplg_upstream
branch
from
9ce2bae to
a24e1da
Compare
The output format is not the only optional content in the initialization payload, but the current used boolean token payload_with_output_fmt is specific to output format, and is not extensible. This patch renames the token name to init_payload_format, and the optional output format only takes bit 0, bits 1~31 are reserved for future use. Signed-off-by: Chao Song <chao.song@linux.intel.com>
This patch adds the widget class for smart_amp module Signed-off-by: Chao Song <chao.song@linux.intel.com>
This patch adds mixout-gain-smart-amp-dai-copier-playback pipeline. Signed-off-by: Chao Song <chao.song@linux.intel.com>
This patch adds the kontrol data for SOF IPC4 smart amp test module initialization. Signed-off-by: Chao Song <chao.song@linux.intel.com>
Use mixout-gain-smart-amp-dai-copier-playback pipeline for SSP0 DAI playback. Signed-off-by: Chao Song <chao.song@linux.intel.com>
aiChaoSONG
force-pushed
the
sma_tplg_upstream
branch
from
a24e1da to
d037b3a
Compare
|
replacement PR #6996, close |
4 participants
This patch adds the smart amp nocodec topology, this topology should be used with sof main and thesofproject/linux#4121