-
Notifications
You must be signed in to change notification settings - Fork 140
Add support for hostless and DAIless pipelines #5609
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
base: topic/sof-dev
Are you sure you want to change the base?
Conversation
|
I tested this PR with other echo ref capture related PRs (thesofproject/sof#10329 and thesofproject/sof#10387), it works perfectly. |
44a47e3 to
6c97700
Compare
6c97700 to
640894b
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.
Some minor opens
sound/soc/intel/boards/sof_sdw.c
Outdated
| { | ||
| struct device *dev = card->dev; | ||
| int ret; | ||
| char *name = devm_kasprintf(dev, GFP_KERNEL, "EchoRef_virtual_DAI"); |
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.
Will this always be echo ref or can it also be a capture device for recording playback ?
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 set up as as capture device for recording playback by default f playback is active. If playback is not active, it produces silence
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.
ok, so could we make a more generic/meaningful name so that users will know what to do with 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.
This looks too fragile to me.
It just happens that we use a tone/signal generator (which generates 0) to feed to an EchoRef capture PCM, but the concept is defined in topology.
Do you have topology PR to see how this looks like?
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.
Here you go @ujfalusi. This is what an echo ref topology looks like

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.
ok, so could we make a more generic/meaningful name so that users will know what to do with it.
@lgirdwood how about "LoopbackCapture_virtual_DAI"?
sound/soc/sof/sof-audio.c
Outdated
| goto sink_prepare; | ||
|
|
||
| /* skip aggregated DAIs */ | ||
| if(is_aggregated_dai(swidget)) |
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.
ditto
ujfalusi
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.
@ranj063, I don't think I understand all the details and how this changes the whole walking logic, I trust that the logic is solid.
The last patch revealed (to commit message) how the direction comes to be used...
I suppose it could be possible to add a simple tone generator now:
virtual DAI -> tone generator -> Host (tone capture)
but it is not, isn't it? We would need to change the machine driver and other things to match with the topology change?
I'm not sure if there is a more dynamic, topology driven way to achieve the virtual DAI concept, which would give really nice user feature at the end.
sound/soc/intel/boards/sof_sdw.c
Outdated
| { | ||
| struct device *dev = card->dev; | ||
| int ret; | ||
| char *name = devm_kasprintf(dev, GFP_KERNEL, "EchoRef_virtual_DAI"); |
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 looks too fragile to me.
It just happens that we use a tone/signal generator (which generates 0) to feed to an EchoRef capture PCM, but the concept is defined in topology.
Do you have topology PR to see how this looks like?
| ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, name, | ||
| 0, 1, "snd-soc-dummy-dai", "dummy", | ||
| snd_soc_dummy_dlc.name, snd_soc_dummy_dlc.dai_name, | ||
| 1, NULL, NULL); |
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.
Only sdw can have this feature, HDA or SSP machines never?
We cannot add to topology a tone generator for example at will, but we need to change the kernel as well?
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.
In theory the same logic would work for all machines and yes you would need the machine driver change in order to use it. But in the case of SSP, we dont need this. We can use the Loopback mode like we've always used. So maybe just the HDA machine change would be enough
| out_fmt = &available_fmt->output_pin_fmts[0].audio_fmt; | ||
|
|
||
| /* copy output format */ | ||
| memcpy(&process->base_config.audio_fmt, out_fmt, sizeof(*out_fmt)); |
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.
what will happen if the fe_parmas and the first output_fmt of the module does not match?
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.
You'll end up with an error that out format match wasnt found on line 2802 isnt it?
| #include "sof-audio.h" | ||
| #include "ops.h" | ||
|
|
||
| /* |
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 does one 'start' such a tone generator playback? There is no PCM device involved, so there must be a kcontrol switch which forces the system to motion.
How?
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.
There is a PCM with the tone generator pipeline. It serves the purpose of propragating the hw_params and triggers to the list of connected DAPM widgets. A kcontrol is a possibility but without a PCM, it wont be easy to know which widgets to set up or even how to the hw_params for the FE DAI and BE DAI and the codec. The PCM is a shortcut in this case to achieve that functionality
| { | ||
| return (WIDGET_IS_DAI(swidget->id) && | ||
| isdigit(swidget->widget->name[strlen(swidget->widget->name) - 1]) && | ||
| swidget->widget->name[strlen(swidget->widget->name) - 1] != '0'); |
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 did this worked then currently if we should not be dealing with the .[1..9] DAIs?
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.
today it works because we stop traversing the path when we see a virtual widget. So if you look at the aggregated DAI alh-copier-SDW1.Playback.1 in the topology I shared in another comment, it is preceded by a virtual widget. So we see that and ignore everything below it. But now for tone and echo ref we want to go down the path from either an aggregated DAI or a virtual widget to the sink components in the path.
@ujfalusi yes this is all you need in topology and it will work. No need to change anything for tone playback. |
640894b to
ebdfe34
Compare
Align with the firmware and add the missing token for pipeline kcps. Signed-off-by: Ranjani Sridharan <[email protected]>
In preparation for supporting hostless pipelines, invoke the host_config op conditionally only for AIF type widgets. Signed-off-by: Ranjani Sridharan <[email protected]>
Add a DAI link for loopback capture as the last link to make sure the other DAI link ID's remain unaffected. It serves as a dummy DAI link to enable echo reference capture in the SDW topologies which do not have an actual backend capture DAI. Signed-off-by: Ranjani Sridharan <[email protected]>
Parse the pipeline direction from topology. The direction_valid token is required for backward-compatibility with older topologies that may not have the direction set for pipelines. This will be used when setting up pipelines to check if a pipeline is in the same direction as the requested params and skip those in the opposite direction like in the case of echo reference capture pipelines during playback. Signed-off-by: Ranjani Sridharan <[email protected]>
…ut pins A tone generator module can be a type of processing module with no input pins. Adjust the logic to set the reference params for selecting output format and the basecfg format based on the output format. Signed-off-by: Ranjani Sridharan <[email protected]>
Today, we use virtual widgets in topology to represent aggregated DAI's in the graph. So, when walking the list of DAPM widgets, when we encounter a virtual widget, we end the walk there. Modify the logic to allow walking past the virtual widgets without handling them during prepare/unprepare/setup/free. This will be useful for the hostless pipelines that use the virtual widget as the start of the playback pipeline. For example, in the case of a tone generator playback pipeline, we could have virtual widget (type input) -> tone generator -> gain -> BE DAI The virtual widget exists only to establish a FE <-> BE connection for DPCM. So while walking the list of widgets, skip the virtual widget and move on so that the tone generator and all of the other widgets downstream can be handled. But in order for this to work with the current use of virtual widgets to represent aggregated DAI's, we also need to skip the aggregated DAI's from being handled during prepare/unprepare/free/setup. Add a new helper function to determine if a DAI widget is an aggregated DAI and skip past the widget when walking the list of connected DAPM widgets. Signed-off-by: Ranjani Sridharan <[email protected]>
ebdfe34 to
43616af
Compare
singalsu
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.
This patch version worked OK with firmware and topology patches to create echo reference capture PCMs.
This patch add supports for hostless/dailess pipelines in the SOF topology. An example of hostless pipeline is a pipeline that's completely internal to the DSP with the tone generator as the source. In order to support this with DPCM, we make use of a virtual widget of type snd_soc_dapm_input as the source widget with the pipeline as follows: virtual widget (snd_soc_dapm_input) -> tone generator (effect) -> gain -> DAI As far as the SOF driver is concerned, any widget of type snd_soc_dapm_input is not handled and is treated as a virtual widget but the previous patch allows traversing the list of widgets past this widget to set up the rest of the widgets in the pipeline. This patch also amends the logic for finding the source widget for this pipeline to include the type snd_soc_dapm_input in order to walk from the source to the sink DAI widget when the pipeline is started. An example of a DAI-less pipeline would be the echo reference capture in the speaker playback path. This pipeline is set up as follows: Host(Playback) -> mixin -> mixout -> gain -> module-copier -> DAI | V Host(Capture) <- Process module <- virtual DAI In the above example, the virtual DAI exploits the concept of an aggregated DAI (one with a non-zero DAI ID) in topology to enable this pipeline to work with DPCM. A virtual DAI is a DAI widget with a non-zero DAI ID and hence is skipped when traversing the list of DAPM widgets during widget prepare/set/up/free/unprepare. The process module in the above pipeline generates 0's that are captured by the echo reference PCM. When the playback path is active, the process module acts as a passthrough module to allow the playback samples to be passthrough to the capture host. In order for these pipelines to work properly, the logic for setting/preparing/freeing/unpreparing the widgets needs to be amended to make sure that only the widgets that are in the pipeline in the same direction as the PCM being started are set up. For example, when the playback PCM is started, the capture pipeline widgets also show up in the list of connected DAPM widgets but they shouldn't be set up yet because the echo reference capture PCM hasn't been started yet. Alternatively, when the echo reference capture PCM is started, the playback pipeline widgets should not be setup. Finally, the last step needed to put this all together is the set the routes for widgets connecting the playback and the capture pipelines when both are active. Signed-off-by: Ranjani Sridharan <[email protected]>
43616af to
779773c
Compare
No description provided.