Skip to content

fixup! ASoC: SOF: Add support for dynamic pipelines#2897

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.

Sign up for GitHub

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

Merged
kv2019i merged 1 commit into from
May 12, 2021
Merged

fixup! ASoC: SOF: Add support for dynamic pipelines #2897

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 31 additions & 24 deletions sound/soc/sof/sof-audio.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -340,7 +340,6 @@ int sof_widget_list_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm, in
{
struct snd_soc_dapm_widget_list *list = spcm->stream[dir].list;
struct snd_soc_dapm_widget *widget;
int num_pipe_widgets = 0;
int i, ret, num_widgets;

/* nothing to set up */
Expand All@@ -350,13 +349,36 @@ int sof_widget_list_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm, in
/* set up widgets in the list */
for_each_dapm_widgets(list, num_widgets, widget) {
struct snd_sof_widget *swidget = widget->dobj.private;
struct snd_sof_widget *pipe_widget;

if (!swidget)
continue;

ret = sof_widget_setup(sdev, swidget);
/*
* The scheduler widget for a pipeline is not part of the connected DAPM
* widget list and it needs to be set up before the widgets in the pipeline
* are set up. The use_count for the scheduler widget is incremented for every
* widget in a given pipeline to ensure that it is freed only after the last
* widget in the pipeline is freed.
*/
pipe_widget = swidget->pipe_widget;
if (!pipe_widget) {
dev_err(sdev->dev, "error: no pipeline widget found for %s\n",
swidget->widget->name);
ret = -EINVAL;
goto widget_free;
}

ret = sof_widget_setup(sdev, pipe_widget);
Comment thread
keyonjie marked this conversation as resolved.
Comment thread
plbossart marked this conversation as resolved.
if (ret < 0)
goto widget_free;

/* set up the widget */
ret = sof_widget_setup(sdev, swidget);
Comment thread
plbossart marked this conversation as resolved.
if (ret < 0) {
sof_widget_free(sdev, pipe_widget);
goto widget_free;
}
}

/*
Expand All@@ -367,7 +389,7 @@ int sof_widget_list_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm, in
if (ret < 0)
goto widget_free;

/* setup pipeline widgets and complete pipelines */
/* complete pipelines */
for_each_dapm_widgets(list, i, widget) {
struct snd_sof_widget *swidget = widget->dobj.private;
struct snd_sof_widget *pipe_widget;
Expand All@@ -380,49 +402,34 @@ int sof_widget_list_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm, in
dev_err(sdev->dev, "error: no pipeline widget found for %s\n",
swidget->widget->name);
ret = -EINVAL;
goto pipe_free;
goto widget_free;
}

ret = sof_widget_setup(sdev, pipe_widget);
if (ret < 0)
goto pipe_free;

num_pipe_widgets++;

if (pipe_widget->complete)
continue;

pipe_widget->complete = snd_sof_complete_pipeline(sdev->dev, pipe_widget);
if (pipe_widget->complete < 0) {
ret = pipe_widget->complete;
goto pipe_free;
goto widget_free;
}
}

return 0;

pipe_free:
/* free the pipe widgets that were set up successfully */
widget_free:

@keyonjiekeyonjieMay 8, 2021•
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 a little bit concern about the error path here, we have 5 failure cases and will jump to this same label here, is the flow below can handle all these 5 failures correctly? Does the num_widgets have enough information for us? If no, we might hit swidget->use_count imbalanced issue?

@ranj063ranj063May 11, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keyonjie yes, the same error path is good enough for all failures. num_widgets represents the number of widgets that are set up successfully and in case of ay error, only those set up successfully need to be freed.

/* free all widgets that have been set up successfully */
for_each_dapm_widgets(list, i, widget) {
struct snd_sof_widget *swidget = widget->dobj.private;

if (!swidget)
continue;

if (!num_pipe_widgets--)
break;

sof_widget_free(sdev, swidget->pipe_widget);
}

widget_free:
/* free all widgets that have been set up successfully */
for_each_dapm_widgets(list, i, widget) {
if (!num_widgets--)
break;

if (widget->dobj.private)
sof_widget_free(sdev, widget->dobj.private);
sof_widget_free(sdev, swidget);
sof_widget_free(sdev, swidget->pipe_widget);
Comment thread
keyonjie marked this conversation as resolved.
Comment thread
plbossart marked this conversation as resolved.
}

return ret;
Expand Down