On Mon, Apr 24, 2017 at 09:15:04AM +0200, Takashi Iwai wrote:
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?
the machine and the platform see sst_soc_prepare()
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).
okay which machine driver was for cx2072x and which one are you using. I can take a look at the code
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.
So ignore_suspend is used to keep PM on those devices even when platform is suspended. It is quite used in keeping BIAS on when suspend, or doing modem-codec interactions when SoC is in suspend.
I don't think we need this for a generic PC/laptop case, so removing them should do the trick.
Use the working machine as ref :)