Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 150
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
216feda4e33d00d6ec58301ffdb77700d9fab6ff22File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1196,6 +1196,34 @@ static int create_bt_dailinks(struct snd_soc_card *card, | ||
| return 0; | ||
| } | ||
| static int create_echoref_dailink(struct snd_soc_card *card, | ||
| struct snd_soc_dai_link **dai_links, int *be_id) | ||
| { | ||
| struct device *dev = card->dev; | ||
| int ret; | ||
| char *name = devm_kasprintf(dev, GFP_KERNEL, "Loopback_Virtual"); | ||
| if (!name) | ||
| return -ENOMEM; | ||
| /* | ||
| * use dummy DAI names as this won't be connected to an actual DAI but just to establish a | ||
| * fe <-> be connection for loopback capture for echo reference | ||
| */ | ||
| 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); | ||
Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only sdw can have this feature, HDA or SSP machines never? CollaboratorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HDA and SSP not priority. | ||
| if (ret) | ||
| return ret; | ||
| (*dai_links)++; | ||
Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how this will work with existing topologies and with function topology? CollaboratorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It shouldnt matter if we use function topologies right? We just have a single machine even in that case right? Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the function topology parsing will fall back to monolithic when it faces with a link of unknown origin/no fragment. I'm not sure, but this is my feeling. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. best to comment what the assumption will be here regarding topology. | ||
| dev_dbg(dev, "Added echo reference DAI link\n"); | ||
| return 0; | ||
| } | ||
| static int sof_card_dai_links_create(struct snd_soc_card *card) | ||
| { | ||
| struct device *dev = card->dev; | ||
| @@ -1304,8 +1332,12 @@ static int sof_card_dai_links_create(struct snd_soc_card *card) | ||
| goto err_end; | ||
| } | ||
| /* allocate BE dailinks */ | ||
| num_links = sdw_be_num + ssp_num + dmic_num + hdmi_num + bt_num; | ||
| /* | ||
| * allocate BE dailinks, add an extra DAI link for echo reference capture. | ||
| * This should be the last DAI link and it is expected both for monolithic | ||
| * and functional SOF topologies to support echo reference. | ||
| */ | ||
| num_links = sdw_be_num + ssp_num + dmic_num + hdmi_num + bt_num + 1; | ||
ranj063 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this is also a chicken and egg problem... This is purely topology construct, but we don't know if the topology have virtual DAI before we pick and parse the topology and for that we need the links in place if the topology do have this defined, right? CollaboratorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, lets capture this behavior in the comments. | ||
| dai_links = devm_kcalloc(dev, num_links, sizeof(*dai_links), GFP_KERNEL); | ||
| if (!dai_links) { | ||
| ret = -ENOMEM; | ||
| @@ -1354,6 +1386,13 @@ static int sof_card_dai_links_create(struct snd_soc_card *card) | ||
| goto err_end; | ||
| } | ||
| /* dummy echo ref link. keep this as the last DAI link. The DAI link ID does not matter */ | ||
| ret = create_echoref_dailink(card, &dai_links, &be_id); | ||
lgirdwood marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (ret) { | ||
| dev_err(dev, "failed to create echo ref dai link: %d\n", ret); | ||
| goto err_end; | ||
| } | ||
| WARN_ON(codec_conf != card->codec_conf + card->num_configs); | ||
| WARN_ON(dai_links != card->dai_link + card->num_links); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -76,6 +76,10 @@ static const struct sof_topology_token ipc4_sched_tokens[] = { | ||
| offsetof(struct sof_ipc4_pipeline, core_id)}, | ||
| {SOF_TKN_SCHED_PRIORITY, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, | ||
| offsetof(struct sof_ipc4_pipeline, priority)}, | ||
| {SOF_TKN_SCHED_DIRECTION, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, | ||
| offsetof(struct sof_ipc4_pipeline, direction)}, | ||
| {SOF_TKN_SCHED_DIRECTION, SND_SOC_TPLG_TUPLE_TYPE_BOOL, get_token_u16, | ||
| offsetof(struct sof_ipc4_pipeline, direction_valid)}, | ||
| }; | ||
| static const struct sof_topology_token pipeline_tokens[] = { | ||
| @@ -939,6 +943,10 @@ static int sof_ipc4_widget_setup_comp_pipeline(struct snd_sof_widget *swidget) | ||
| swidget->core = pipeline->core_id; | ||
| spipe->core_mask |= BIT(pipeline->core_id); | ||
| if (pipeline->direction_valid) { | ||
| spipe->direction = pipeline->direction; | ||
| spipe->direction_valid = true; | ||
| } | ||
| if (pipeline->use_chain_dma) { | ||
| dev_dbg(scomp->dev, "Set up chain DMA for %s\n", swidget->widget->name); | ||
| @@ -954,9 +962,9 @@ static int sof_ipc4_widget_setup_comp_pipeline(struct snd_sof_widget *swidget) | ||
| goto err; | ||
| } | ||
| dev_dbg(scomp->dev, "pipeline '%s': id %d, pri %d, core_id %u, lp mode %d\n", | ||
| dev_dbg(scomp->dev, "pipeline '%s': id %d, pri %d, core_id %u, lp mode %d direction %d\n", | ||
| swidget->widget->name, swidget->pipeline_id, | ||
| pipeline->priority, pipeline->core_id, pipeline->lp_mode); | ||
| pipeline->priority, pipeline->core_id, pipeline->lp_mode, pipeline->direction); | ||
| swidget->private = pipeline; | ||
| @@ -2745,12 +2753,14 @@ static int sof_ipc4_prepare_process_module(struct snd_sof_widget *swidget, | ||
| int input_fmt_index = 0; | ||
| int ret; | ||
| input_fmt_index = sof_ipc4_init_input_audio_fmt(sdev, swidget, | ||
| &process->base_config, | ||
| pipeline_params, | ||
| available_fmt); | ||
| if (input_fmt_index < 0) | ||
| return input_fmt_index; | ||
| if (available_fmt->num_input_formats) { | ||
| input_fmt_index = sof_ipc4_init_input_audio_fmt(sdev, swidget, | ||
| &process->base_config, | ||
| pipeline_params, | ||
| available_fmt); | ||
| if (input_fmt_index < 0) | ||
| return input_fmt_index; | ||
| } | ||
| /* Configure output audio format only if the module supports output */ | ||
| if (available_fmt->num_output_formats) { | ||
| @@ -2759,12 +2769,28 @@ static int sof_ipc4_prepare_process_module(struct snd_sof_widget *swidget, | ||
| u32 out_ref_rate, out_ref_channels; | ||
| int out_ref_valid_bits, out_ref_type; | ||
| in_fmt = &available_fmt->input_pin_fmts[input_fmt_index].audio_fmt; | ||
| if (available_fmt->num_input_formats) { | ||
| in_fmt = &available_fmt->input_pin_fmts[input_fmt_index].audio_fmt; | ||
| out_ref_rate = in_fmt->sampling_frequency; | ||
| out_ref_channels = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(in_fmt->fmt_cfg); | ||
| out_ref_valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(in_fmt->fmt_cfg); | ||
| out_ref_type = sof_ipc4_fmt_cfg_to_type(in_fmt->fmt_cfg); | ||
| out_ref_rate = in_fmt->sampling_frequency; | ||
| out_ref_channels = | ||
| SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(in_fmt->fmt_cfg); | ||
| out_ref_valid_bits = | ||
ranj063 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(in_fmt->fmt_cfg); | ||
| out_ref_type = sof_ipc4_fmt_cfg_to_type(in_fmt->fmt_cfg); | ||
Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and to add I have this PR for some time which touches the same code and fixes issue when not all parameters are changed by a process module: | ||
| } else { | ||
| /* for modules without input formats, use FE params as reference */ | ||
| out_ref_rate = params_rate(fe_params); | ||
| out_ref_channels = params_channels(fe_params); | ||
| ret = sof_ipc4_get_sample_type(sdev, fe_params); | ||
| if (ret < 0) | ||
ranj063 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return ret; | ||
| out_ref_type = (u32)ret; | ||
| out_ref_valid_bits = sof_ipc4_get_valid_bits(sdev, fe_params); | ||
| if (out_ref_valid_bits < 0) | ||
| return out_ref_valid_bits; | ||
| } | ||
| output_fmt_index = sof_ipc4_init_output_audio_fmt(sdev, swidget, | ||
| &process->base_config, | ||
| @@ -2792,6 +2818,16 @@ static int sof_ipc4_prepare_process_module(struct snd_sof_widget *swidget, | ||
| if (ret) | ||
| return ret; | ||
| } | ||
| /* set base cfg to match the first output format if there are no input formats */ | ||
| if (!available_fmt->num_input_formats) { | ||
lgirdwood marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| struct sof_ipc4_audio_format *out_fmt; | ||
| out_fmt = &available_fmt->output_pin_fmts[0].audio_fmt; | ||
| /* copy output format */ | ||
| memcpy(&process->base_config.audio_fmt, out_fmt, sizeof(*out_fmt)); | ||
Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? CollaboratorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? | ||
| } | ||
| } | ||
| sof_ipc4_dbg_module_audio_format(sdev->dev, swidget, available_fmt, | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -150,6 +150,8 @@ struct sof_ipc4_copier_config_set_sink_format { | ||
| * @use_chain_dma: flag to indicate if the firmware shall use chained DMA | ||
| * @msg: message structure for pipeline | ||
| * @skip_during_fe_trigger: skip triggering this pipeline during the FE DAI trigger | ||
| * @direction_valid: flag indicating if valid direction is set in topology | ||
| * @direction: pipeline direction set in topology if direction_valid is true | ||
| */ | ||
| struct sof_ipc4_pipeline { | ||
| uint32_t priority; | ||
| @@ -160,6 +162,8 @@ struct sof_ipc4_pipeline { | ||
| bool use_chain_dma; | ||
| struct sof_ipc4_msg msg; | ||
| bool skip_during_fe_trigger; | ||
| bool direction_valid; | ||
| u32 direction; | ||
lgirdwood marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should it be documented what values are valid for CollaboratorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ujfalusi these come directly from topology, so are checked for validity there. Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so, the value for kernel does not matter, only they should match? Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can add that context to the comment. Can complain too if out of range. | ||
| }; | ||
| /** | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.