BTW, while checking these things, I noticed that there are three exceptional drivers: sound/soc/intel/atom/sst-mfld-platform-pcm.c, sound/soc/intel/haswell/sst-haswell-pcm.c, and sound/soc/samsung/tm2_wm5110.c. (You can simply grep the external snd_soc_suspend call, and only these match.)
The former two drivers look really weird: they do handle the PM only with PM prepare and complete callbacks, while snd_soc_suspend() and co are called internally from there. The prepare and complete callbacks aren't designed for the complete suspend/resume tasks, so I'd say it's a quite abuse.
For the Atom/SST driver, I remember there was a need to set/restore the DSP state with a specific command that wasn't handled with regular controls - largely a work-around due to the firmware design.
For the Haswell driver, there was also a need to preserve/restore state and pause/stop pipelines (a recurring issue with the "Made for Windows" firmware).
These drivers are quite old now and it's not clear to me if they are broken or if we are talking of an improvement. Could you clarify what you view as "abuse"?
a) is this the fact that there are prepare/complete callback for those drivers, instead of others such as freeze, thaw, etc.
b) the fact they they call snd_soc_suspend/resume directly?
c) the fact that they suspend the PCM streams?
d) all of the above (which is entirely possible).
The purpose of PM prepare and complete devops aren't for actually do suspend and resume devices there, while these drivers call snd_soc_suspend() and snd_soc_resume() to perform the complete suspend / resume procedure. That's not the way these callbacks are supposed to be used.
The prepare callback is called before the suspend callback of *all* devices on the system. Ditto for complete, it's called after the resume of all devices.
I guess they use prepare/callback to assure some tasks to be performed always suspend and resume. But it's still puzzling, e.g. sst_soc_prepare() has
static int sst_soc_prepare(struct device *dev) { struct sst_data *drv = dev_get_drvdata(dev); struct snd_soc_pcm_runtime *rtd;
if (!drv->soc_card) return 0;
/* suspend all pcms first */ snd_soc_suspend(drv->soc_card->dev); snd_soc_poweroff(drv->soc_card->dev);
/* set the SSPs to idle */ for_each_card_rtds(drv->soc_card, rtd) { struct snd_soc_dai *dai = rtd->cpu_dai;
if (dai->active) { send_ssp_cmd(dai, dai->name, 0); sst_handle_vb_timer(dai, false); }}
return 0; }
... and it calls snd_soc_poweroff() for suspend. That's odd and likely superfluous.
And, the last part ("set the SSPs to idle") can be moved to card->suspend_post hook, and then we can simply call snd_soc_suspend(). Or it can be moved to PM devops suspend_late.
Similarly for sst_soc_complete(), the task "restart SSPs" can be moved to card->resume_pre hook or PM devops resume_pre.
The rest is to make sure the device PM ops order, and that's the hardest part.
Further looking at the code, we can see that several Intel ASoC drivers have device PM ops.
sound/soc/intel/atom/sst/sst.c sound/soc/intel/haswell/sst-haswell-pcm.c sound/soc/intel/skylake/skl.c sound/soc/intel/atom/sst-mfld-platform-pcm.c
... and there are codecs. We need to list up and define the suspend / resume call order.
I had an offline discussion with Vinod and here are the key points for the Atom/SST driver
- the DSP isn't completely modeled with DPCM, there are some pipeline management and commands that need to be send manually. This isn't necessarily a perfect design but the one that was defined in 2013
- the choice of the .prepare is intentional. The tasks are split between the SST device (ACPI or PCI) and the platform device it creates. The ACPI/PCI layer handles DSP boot, config, shutdown, fw load, and the platform driver handles PCM/pipelines. The PM starts with the .prepare done in the child before the .suspend done at a higher level.
For Haswell I have no idea, and I wonder if there are actually any devices using this driver. Even from Broadwell we only know of the Pixel 2015 Chromebook and the Dell XPS 13 where the I2S mode is activated (and the latter is deactivated with an ACPI quirk), most devices use HDaudio. Even if we found someone at Intel with bandwidth, changing this part is going to be very difficult between lack of devices and initial teams/individual contributors who have moved on.
SOF will support all these platforms, it might be a better idea to spend time making sure we do the right thing with the newer drivers than try to fix things but actually introduce regressions in legacy code.
-Pierre