On Sat, 12 Jan 2019 02:46:33 +0100, Yang, Libin wrote: > > Hi Takashi, > > >> > > >> > 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 :) > > The suspend order is always a quite hard issue for me. I have to spend > a lot time checking the parent-child relationship. And if they don't have > such relationship, I don't know how to find the order. > > My initial idea to get rid of such dependency is: do the pcm suspend before > suspend(), for example put it in prepare(). And set device clk off and power > off in suspend(). This can make sure pcm is always properly suspended. > But, I'm not sure prepare() is a right place because it seems prepare() is not > designed to do such things and prepare() may impact the suspend/resume > sequence based on its return value. If your below patch works, I think it will > be a best solution which can handle such things in ALSA level. I think we > may need to do a lot of test because the different cards drivers are > extremely different. As mentioned below, ASoC is another thing. Its PM sequence is found in snd_soc_suspend(). Takashi > > Regards, > Libin > > > > >... 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'); > >> > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel