Skip to content

ASoC: SOF: topology: add pm-get/put around topology load #241

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

Closed
wants to merge 1 commit into from
Closed

ASoC: SOF: topology: add pm-get/put around topology load #241

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
12 changes: 12 additions & 0 deletions sound/soc/sof/topology.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2463,6 +2463,13 @@ int snd_sof_load_topology(struct snd_sof_dev *sdev, const char *file)
return ret;
}

ret = pm_runtime_get_sync(sdev->dev);
if (ret < 0) {
dev_err(sdev->dev, "error: topology load pm_runtime_get_sync failed with %d\n",
ret);
return ret;
}

hdr = (struct snd_soc_tplg_hdr *)fw->data;
ret = snd_soc_tplg_component_load(sdev->component,
&sof_tplg_ops, fw,
Expand All @@ -2473,6 +2480,11 @@ int snd_sof_load_topology(struct snd_sof_dev *sdev, const char *file)
ret = -EINVAL;
}

ret = pm_runtime_put(sdev->dev);
Copy link
Collaborator

@ranj063 ranj063 Nov 1, 2018 •
edited by plbossart
Loading

Choose a reason for hiding this comment

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

@plbossart this seems to take care of the error. But if we are using autosuspend, shouldnt we use
pm_runtime_mark_last_busy(sdev->dev);
ret = pm_runtime_put_autosuspend(sdev->dev);

On the other hand, instead of this patch what also works is:S ince we are enabling autosuspend on component->dev, we should use runtime_put_autosuspend instead of runtime_idle.

diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index f2e256d129bf5..0476aed01fa17 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -712,7 +712,10 @@ static int sof_pcm_probe(struct snd_soc_component *component)
                                         SND_SOF_SUSPEND_DELAY);
        pm_runtime_use_autosuspend(component->dev);
        pm_runtime_enable(component->dev);
-       err = pm_runtime_idle(component->dev);
+
+       /* autosuspend sof device */
+       pm_runtime_mark_last_busy(component->dev);
+       err = pm_runtime_put_autosuspend(component->dev);
        if (err < 0)
                dev_err(sdev->dev, "error: failed to enter PM idle %d\n", err);

Copy link
Member Author

@plbossart plbossart Nov 1, 2018

Choose a reason for hiding this comment

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

@ranj063 I edited your comment to format the patch contents with the 4 backquotes thingy

I am not sure I understand the proposal above at all, where is the pm_runtime_get_ done prior to calling snd_sof_topology_load?

Copy link
Collaborator

@ranj063 ranj063 Nov 1, 2018

Choose a reason for hiding this comment

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

@plbossart from my observations, I dont think the sof device enter runtime suspend before topology loading occurs.

in pcm.c after topology loading in completed, we enable runtime_pm_enable and use autosuspend for component->dev. And then we call pm_runtime_idle(). This seems to be what is causing the error.

What I was suggesting was that instead of calling pm_runtime_idle, we should use put_autosuspend instead.

Copy link
Member Author

@plbossart plbossart Nov 1, 2018

Choose a reason for hiding this comment

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

@ranj063 My understanding is that for every pm_runtime_put_xxx you need to have a matching pm_runtime_get_xxx. I don't get how we can use put_autosuspend() without a prior get(). What am I missing?

if (ret < 0)
dev_err(sdev->dev, "error: topology load pm_runtime_put failed with %d\n",
ret);

release_firmware(fw);
return ret;
}
Expand Down