On Fri, 11 Jan 2019 15:24:30 +0100, Takashi Iwai wrote:
On Fri, 11 Jan 2019 14:34:34 +0100, Takashi Iwai wrote:
On Fri, 11 Jan 2019 06:20:23 +0100, Yang, Libin wrote:
The below patch may have a small confliction that the trigger will be called twice as our SOF has already call snd_pcm_suspend() in card suspend.
It should be no problem, snd_pcm_suspend() can be called multiple times. If it's already suspended, just nothing happens.
Thinking of this problem again, does a patch like below work instead? This looks like a better and more generic solution.
What I'm not quite sure is whether the device suspend order between PCM device and HD-audio codec device is guaranteed. I guess yes, because the PCM device is registered always after the codec. But ASoC might have another weirdness :)
... and further reading revealed that my afraid is right -- ASoC is another beast. Basically PM in ASoC requires *not* to be done by the usual device PM callbacks. snd_soc_suspend() does the whole suspend jobs sequentially. So, what you need for Intel ASoC driver is to do suspend/resume with the component callbacks instead of device PM ops. Currently the plumbing of hdac-hdmi driver is in a half-baked state.
If this patch works, basically we can get rid of all external callers of snd_pcm_suspend() & co. That'll be a nice cleanup.
OTOH, the statement above is still true for non-ASoC drivers. So we'd end up needing an extra flag or such to prevent the PCM device PM ops for ASoC, while using this for the rest.
thanks,
Takashi
thanks,
Takashi
diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 01b9d62eef14..d8d57336b8b8 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -683,6 +683,25 @@ static inline int snd_pcm_substream_proc_done(struct snd_pcm_substream *substrea
static const struct attribute_group *pcm_dev_attr_groups[];
+#ifdef CONFIG_PM_SLEEP +static int do_pcm_suspend(struct device *dev) +{
- struct snd_pcm_str *pstr = container_of(dev, struct snd_pcm_str, dev);
- snd_pcm_suspend_all(pstr->pcm);
- return 0;
+} +#endif
+static const struct dev_pm_ops pcm_dev_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(do_pcm_suspend, NULL)
+};
+static const struct device_type pcm_dev_type = {
- .name = "pcm",
- .pm = &pcm_dev_pm_ops,
+};
/**
- snd_pcm_new_stream - create a new PCM stream
- @pcm: the pcm instance
@@ -713,6 +732,7 @@ int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count)
snd_device_initialize(&pstr->dev, pcm->card); pstr->dev.groups = pcm_dev_attr_groups;
- pstr->dev.type = &pcm_dev_type; dev_set_name(&pstr->dev, "pcmC%iD%i%c", pcm->card->number, pcm->device, stream == SNDRV_PCM_STREAM_PLAYBACK ? 'p' : 'c');