On Mon, 24 Apr 2017 07:01:44 +0200,
Vinod Koul wrote:
>
> On Fri, Apr 21, 2017 at 03:42:43PM +0200, Takashi Iwai wrote:
> > Hi,
> >
> > I noticed that the unstable PM behavior on my Cherrytrail laptop
> > actually comes from the sound driver. First off, the atom/sst/sst.c
> > has the following suspend code:
> >
> > static int intel_sst_suspend(struct device *dev)
> > {
> > ....
> > /*
> > * check if any stream is active and running
> > * they should already by suspend by soc_suspend
> > */
> > for (i = 1; i <= ctx->info.max_streams; i++) {
> > struct stream_info *stream = &ctx->streams[i];
> >
> > if (stream->status == STREAM_RUNNING) {
> > dev_err(dev, "stream %d is running, can't suspend, abort\n", i);
> > return -EBUSY;
> > }
> > }
> >
> > This doesn't look good, and I actually hit this when I tried to
> > suspend while playing something. In general, the driver shouldn't
> > reject at this point, because this is the PM suspend callback,
> > i.e. the user wants to suspend the device inevitably. The driver
> > should return an error only when it faces to a fatal error.
>
> Mea culpa
>
> And you are now second person to complain about this so I wonder if we
> should rework this
Well, something is definitely wrong :)
> So from hardware POV, we need to first suspend all running streams and then
> save the context from DSP (in order to restore them later) and then we can
> shutdown the DSP.
>
> The problem is driver being split into platform (which knows streams) and
> sst dsp driver. So we devised a careful sequence where platform suspend is
> invoked first (calls using asoc pm ops) and then DSP
>
> This results is ASoC doing stream suspend for us and when we hit
> intel_sst_suspend() we are supposed to have stream suspended, so save and
> shut off DSP.
>
> Now for you issue you see can you check why platform suspend is not getting
> called?
>
> >
> > But I wondered why this happened at all, and noticed that the machine
> > driver (in my case bytcr_rt5640) has no its own PM ops. But hooking
> > the snd_soc_pm_ops there seems causing a hang up at suspend by some
> > reason.
>
> O yes, thats due to double suspend
>
> See 3639ac1cd5177685a5c8abb7230096b680e1d497
I haven't followed the code deeply enough. Who is calling to trigger
double-suspend?
> > Maybe this wasn't a big problem until now since the BYT/CHT didn't
> > support the suspend/resume properly in the past. But now PM suspend
> > is supported on these devices, so the problem surfaced more often.
>
> The Chromebooks shipped on BSW use this method so..
Interestingly, when I checked another CHT machine with cx2072x codec,
the PM works (although the playback doesn't restart at resume
properly).
Wait... Now closely looking at the code, I noticed the
"ignore_suspend" marks in many places in bytcr_rt5640.c. Why is this
needed?
Other two machine drivers I've tested (cht_bsw_rt5672 and Pierre's
cht_cx2072x) have no such a flag set, thus they work. With the
ignore_suspend, the PCM suspend calls are prevented, and it shall hit
the problem above.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
