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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
