* PM issue with Intel SST Atom driver
@ 2017-04-21 13:42 Takashi Iwai
2017-04-24 5:01 ` Vinod Koul
0 siblings, 1 reply; 16+ messages in thread
From: Takashi Iwai @ 2017-04-21 13:42 UTC (permalink / raw)
To: Vinod Koul; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart
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.
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.
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.
Could you guys check the status of these drivers?
thanks,
Takashi
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: PM issue with Intel SST Atom driver 2017-04-21 13:42 PM issue with Intel SST Atom driver Takashi Iwai @ 2017-04-24 5:01 ` Vinod Koul 2017-04-24 7:15 ` Takashi Iwai 0 siblings, 1 reply; 16+ messages in thread From: Vinod Koul @ 2017-04-24 5:01 UTC (permalink / raw) To: Takashi Iwai; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart 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 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 > > 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.. > Could you guys check the status of these drivers? -- ~Vinod ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 5:01 ` Vinod Koul @ 2017-04-24 7:15 ` Takashi Iwai 2017-04-24 9:00 ` Takashi Iwai 2017-04-24 9:09 ` Vinod Koul 0 siblings, 2 replies; 16+ messages in thread From: Takashi Iwai @ 2017-04-24 7:15 UTC (permalink / raw) To: Vinod Koul; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 7:15 ` Takashi Iwai @ 2017-04-24 9:00 ` Takashi Iwai 2017-04-24 9:12 ` Vinod Koul 2017-04-24 9:09 ` Vinod Koul 1 sibling, 1 reply; 16+ messages in thread From: Takashi Iwai @ 2017-04-24 9:00 UTC (permalink / raw) To: Vinod Koul; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart On Mon, 24 Apr 2017 09:15:04 +0200, Takashi Iwai wrote: > > > > 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? Never mind, I figured out that it's in sst_soc_prepare(). So, it's specific to Atom (and Haswell). > > 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. Removing ignore_suspend makes the PM succeeds. But it hits some other ugly kernel bugs. At suspending: [ 567.913706] WARNING: CPU: 3 PID: 3144 at ../kernel/softirq.c:161 __local_bh_enable_ip+0x71/0x90 [ 567.913842] CPU: 3 PID: 3144 Comm: systemd-sleep Tainted: G C O 4.11.0-rc7-3.g64b92e2-default #1 [ 567.913847] Call Trace: [ 567.913861] dump_stack+0x5c/0x7a [ 567.913869] __warn+0xbe/0xe0 [ 567.913879] __local_bh_enable_ip+0x71/0x90 [ 567.913891] sst_create_block+0x83/0xd0 [snd_intel_sst_core] [ 567.913906] sst_create_block_and_ipc_msg+0x4a/0x70 [snd_intel_sst_core] [ 567.913920] sst_prepare_and_post_msg+0x1a0/0x960 [snd_intel_sst_core] [ 567.913936] sst_pause_stream+0x9b/0x110 [snd_intel_sst_core] [ 567.913952] sst_platform_pcm_trigger+0x123/0x1b0 [snd_soc_sst_atom_hifi2_platform] [ 567.913980] soc_pcm_trigger+0xa0/0x120 [snd_soc_core] [ 567.913996] ? sst_soc_complete+0xa0/0xa0 [snd_soc_sst_atom_hifi2_platform] [ 567.914012] dpcm_fe_dai_do_trigger+0xc8/0x1f0 [snd_soc_core] [ 567.914034] snd_pcm_do_suspend+0x3d/0x40 [snd_pcm] [ 567.914054] snd_pcm_action_single+0x2a/0x70 [snd_pcm] [ 567.914065] snd_pcm_suspend+0x2c/0x40 [snd_pcm] [ 567.914076] snd_pcm_suspend_all+0x32/0x70 [snd_pcm] [ 567.914092] snd_soc_suspend+0x15c/0x3d0 [snd_soc_core] [ 567.914102] sst_soc_prepare+0x23/0xa0 [snd_soc_sst_atom_hifi2_platform] [ 567.914108] dpm_prepare+0x237/0x480 [ 567.914113] dpm_suspend_start+0xd/0x50 [ 567.914117] suspend_devices_and_enter+0xac/0x6f0 [ 567.914123] pm_suspend+0x304/0x380 [ 567.914128] state_store+0x5e/0x90 [ 567.914134] kernfs_fop_write+0xfc/0x190 [ 567.914140] __vfs_write+0x23/0x140 [ 567.914146] ? handle_mm_fault+0xd3/0x240 [ 567.914148] ? security_file_permission+0x36/0xb0 [ 567.914151] vfs_write+0xb0/0x190 [ 567.914156] SyS_write+0x42/0x90 [ 567.914160] entry_SYSCALL_64_fastpath+0x1e/0xad [ 567.914164] ---[ end trace b3703d94611f9a06 ]--- [ 567.914176] BUG: scheduling while atomic: systemd-sleep/3144/0x00000003 [ 567.914260] CPU: 3 PID: 3144 Comm: systemd-sleep Tainted: G WC O 4.11.0-rc7-3.g64b92e2-default #1 [ 567.914262] Call Trace: [ 567.914273] dump_stack+0x5c/0x7a [ 567.914278] __schedule_bug+0x55/0x70 [ 567.914284] __schedule+0x63c/0x8c0 [ 567.914290] schedule+0x3d/0x90 [ 567.914295] schedule_timeout+0x16b/0x320 [ 567.914301] ? del_timer_sync+0x50/0x50 [ 567.914308] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] [ 567.914313] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] [ 567.914316] ? remove_wait_queue+0x60/0x60 [ 567.914321] ? sst_prepare_and_post_msg+0x275/0x960 [snd_intel_sst_core] [ 567.914326] ? sst_pause_stream+0x9b/0x110 [snd_intel_sst_core] [ 567.914333] ? sst_platform_pcm_trigger+0x123/0x1b0 [snd_soc_sst_atom_hifi2_platform] [ 567.914346] ? soc_pcm_trigger+0xa0/0x120 [snd_soc_core] [ 567.914353] ? sst_soc_complete+0xa0/0xa0 [snd_soc_sst_atom_hifi2_platform] [ 567.914365] ? dpcm_fe_dai_do_trigger+0xc8/0x1f0 [snd_soc_core] [ 567.914373] ? snd_pcm_do_suspend+0x3d/0x40 [snd_pcm] [ 567.914381] ? snd_pcm_action_single+0x2a/0x70 [snd_pcm] [ 567.914389] ? snd_pcm_suspend+0x2c/0x40 [snd_pcm] [ 567.914396] ? snd_pcm_suspend_all+0x32/0x70 [snd_pcm] [ 567.914408] ? snd_soc_suspend+0x15c/0x3d0 [snd_soc_core] [ 567.914415] ? sst_soc_prepare+0x23/0xa0 [snd_soc_sst_atom_hifi2_platform] [ 567.914418] ? dpm_prepare+0x237/0x480 [ 567.914421] ? dpm_suspend_start+0xd/0x50 [ 567.914423] ? suspend_devices_and_enter+0xac/0x6f0 [ 567.914426] ? pm_suspend+0x304/0x380 [ 567.914428] ? state_store+0x5e/0x90 [ 567.914430] ? kernfs_fop_write+0xfc/0x190 [ 567.914433] ? __vfs_write+0x23/0x140 [ 567.914437] ? handle_mm_fault+0xd3/0x240 [ 567.914439] ? security_file_permission+0x36/0xb0 [ 567.914442] ? vfs_write+0xb0/0x190 [ 567.914445] ? SyS_write+0x42/0x90 [ 567.914447] ? entry_SYSCALL_64_fastpath+0x1e/0xad [ 567.914857] BUG: scheduling while atomic: systemd-sleep/3144/0x7fffffff [ 567.915222] CPU: 1 PID: 3144 Comm: systemd-sleep Tainted: G WC O 4.11.0-rc7-3.g64b92e2-default #1 [ 567.915231] Call Trace: [ 567.915300] dump_stack+0x5c/0x7a [ 567.915324] __schedule_bug+0x55/0x70 [ 567.915345] __schedule+0x63c/0x8c0 [ 567.915375] schedule+0x3d/0x90 [ 567.915392] async_synchronize_cookie_domain+0x85/0x130 [ 567.915414] ? remove_wait_queue+0x60/0x60 [ 567.915472] dapm_power_widgets+0x3d4/0x9c0 [snd_soc_core] [ 567.915530] ? sst_soc_complete+0xa0/0xa0 [snd_soc_sst_atom_hifi2_platform] [ 567.915579] ? snd_soc_dapm_stream_event+0x87/0xa0 [snd_soc_core] [ 567.915628] ? snd_soc_dapm_stream_event+0x87/0xa0 [snd_soc_core] [ 567.915674] ? snd_soc_suspend+0x39c/0x3d0 [snd_soc_core] [ 567.915699] ? sst_soc_prepare+0x23/0xa0 [snd_soc_sst_atom_hifi2_platform] [ 567.915712] ? dpm_prepare+0x237/0x480 [ 567.915723] ? dpm_suspend_start+0xd/0x50 [ 567.915738] ? suspend_devices_and_enter+0xac/0x6f0 [ 567.915749] ? pm_suspend+0x304/0x380 [ 567.915757] ? state_store+0x5e/0x90 [ 567.915771] ? kernfs_fop_write+0xfc/0x190 [ 567.915785] ? __vfs_write+0x23/0x140 [ 567.915800] ? handle_mm_fault+0xd3/0x240 [ 567.915814] ? security_file_permission+0x36/0xb0 [ 567.915824] ? vfs_write+0xb0/0x190 [ 567.915834] ? SyS_write+0x42/0x90 [ 567.915846] ? entry_SYSCALL_64_fastpath+0x1e/0xad ... and at resume: [ 574.320255] BUG: scheduling while atomic: alsa-source-3/1729/0x00000003 [ 574.320435] CPU: 1 PID: 1729 Comm: alsa-source-3 Tainted: G WC O 4.11.0-rc7-3.g64b92e2-default #1 [ 574.320440] Call Trace: [ 574.320474] dump_stack+0x5c/0x7a [ 574.320484] __schedule_bug+0x55/0x70 [ 574.320493] __schedule+0x63c/0x8c0 [ 574.320503] schedule+0x3d/0x90 [ 574.320509] schedule_timeout+0x16b/0x320 [ 574.320517] ? del_timer_sync+0x50/0x50 [ 574.320529] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] [ 574.320535] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] [ 574.320539] ? remove_wait_queue+0x60/0x60 [ 574.320546] ? sst_prepare_and_post_msg+0x275/0x960 [snd_intel_sst_core] [ 574.320553] ? sst_resume_stream+0x5b/0x100 [snd_intel_sst_core] [ 574.320564] ? sst_platform_pcm_trigger+0x6b/0x1b0 [snd_soc_sst_atom_hifi2_platform] [ 574.320586] ? soc_pcm_trigger+0xa0/0x120 [snd_soc_core] [ 574.320603] ? dpcm_fe_dai_do_trigger+0xc8/0x1f0 [snd_soc_core] [ 574.320618] ? snd_pcm_action_single+0x2a/0x70 [snd_pcm] [ 574.320628] ? snd_pcm_common_ioctl1+0x2e5/0xc60 [snd_pcm] [ 574.320634] ? do_signal+0x23/0x670 [ 574.320644] ? snd_pcm_capture_ioctl1+0x117/0x280 [snd_pcm] [ 574.320648] ? ktime_get_ts64+0x4a/0xf0 [ 574.320659] ? snd_pcm_capture_ioctl+0x2a/0x30 [snd_pcm] [ 574.320663] ? do_vfs_ioctl+0x8f/0x5d0 [ 574.320667] ? __fget+0x70/0xc0 [ 574.320671] ? SyS_ioctl+0x74/0x80 [ 574.320675] ? entry_SYSCALL_64_fastpath+0x1e/0xad [ 574.320799] show_signal_msg: 8 callbacks suppressed [ 574.320805] alsa-source-3[1729]: segfault at 7f28e8052000 ip 00007f28f635eac4 sp 00007f28f0ad4be8 error 6 [ 574.320882] BUG: scheduling while atomic: alsa-source-3/1729/0x7fffffff [ 574.321040] CPU: 1 PID: 1729 Comm: alsa-source-3 Tainted: G WC O 4.11.0-rc7-3.g64b92e2-default #1 [ 574.321043] Call Trace: [ 574.321068] dump_stack+0x5c/0x7a [ 574.321076] __schedule_bug+0x55/0x70 [ 574.321083] __schedule+0x63c/0x8c0 [ 574.321093] ? wakeup_preempt_entity.isra.58+0x3c/0x50 [ 574.321097] schedule+0x3d/0x90 [ 574.321104] schedule_timeout+0x25a/0x320 [ 574.321109] ? check_preempt_curr+0x79/0x90 [ 574.321113] ? select_task_rq_rt+0x57/0xa0 [ 574.321117] ? sched_clock+0x5/0x10 [ 574.321120] ? sched_clock_cpu+0xc/0xc0 [ 574.321124] ? wait_for_completion+0xe6/0x150 [ 574.321128] ? wait_for_completion+0xe6/0x150 [ 574.321131] ? wake_up_q+0x80/0x80 [ 574.321135] ? do_coredump+0x3ab/0xf10 [ 574.321139] ? __wake_up+0x34/0x50 [ 574.321144] ? get_signal+0x33b/0x660 [ 574.321150] ? do_signal+0x23/0x670 [ 574.321154] ? __send_signal+0x213/0x4d0 [ 574.321160] ? force_sig_info_fault+0x88/0xd0 [ 574.321166] ? exit_to_usermode_loop+0x71/0xb0 [ 574.321169] ? prepare_exit_to_usermode+0x2a/0x30 [ 574.321172] ? retint_user+0x8/0x10 There seem many beasts hidden in this jungle... Takashi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 9:00 ` Takashi Iwai @ 2017-04-24 9:12 ` Vinod Koul 2017-04-24 9:43 ` Takashi Iwai 0 siblings, 1 reply; 16+ messages in thread From: Vinod Koul @ 2017-04-24 9:12 UTC (permalink / raw) To: Takashi Iwai; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > Removing ignore_suspend makes the PM succeeds. But it hits some other > ugly kernel bugs. Okay have you marked .nonatomic = true for the machine DAIs? > > At suspending: > > [ 567.913706] WARNING: CPU: 3 PID: 3144 at ../kernel/softirq.c:161 __local_bh_enable_ip+0x71/0x90 > [ 567.913842] CPU: 3 PID: 3144 Comm: systemd-sleep Tainted: G C O 4.11.0-rc7-3.g64b92e2-default #1 > [ 567.913847] Call Trace: > [ 567.913861] dump_stack+0x5c/0x7a > [ 567.913869] __warn+0xbe/0xe0 > [ 567.913879] __local_bh_enable_ip+0x71/0x90 > [ 567.913891] sst_create_block+0x83/0xd0 [snd_intel_sst_core] > [ 567.913906] sst_create_block_and_ipc_msg+0x4a/0x70 [snd_intel_sst_core] > [ 567.913920] sst_prepare_and_post_msg+0x1a0/0x960 [snd_intel_sst_core] > [ 567.913936] sst_pause_stream+0x9b/0x110 [snd_intel_sst_core] > [ 567.913952] sst_platform_pcm_trigger+0x123/0x1b0 [snd_soc_sst_atom_hifi2_platform] > [ 567.913980] soc_pcm_trigger+0xa0/0x120 [snd_soc_core] > [ 567.913996] ? sst_soc_complete+0xa0/0xa0 [snd_soc_sst_atom_hifi2_platform] > [ 567.914012] dpcm_fe_dai_do_trigger+0xc8/0x1f0 [snd_soc_core] > [ 567.914034] snd_pcm_do_suspend+0x3d/0x40 [snd_pcm] > [ 567.914054] snd_pcm_action_single+0x2a/0x70 [snd_pcm] > [ 567.914065] snd_pcm_suspend+0x2c/0x40 [snd_pcm] > [ 567.914076] snd_pcm_suspend_all+0x32/0x70 [snd_pcm] > [ 567.914092] snd_soc_suspend+0x15c/0x3d0 [snd_soc_core] > [ 567.914102] sst_soc_prepare+0x23/0xa0 [snd_soc_sst_atom_hifi2_platform] > [ 567.914108] dpm_prepare+0x237/0x480 > [ 567.914113] dpm_suspend_start+0xd/0x50 > [ 567.914117] suspend_devices_and_enter+0xac/0x6f0 > [ 567.914123] pm_suspend+0x304/0x380 > [ 567.914128] state_store+0x5e/0x90 > [ 567.914134] kernfs_fop_write+0xfc/0x190 > [ 567.914140] __vfs_write+0x23/0x140 > [ 567.914146] ? handle_mm_fault+0xd3/0x240 > [ 567.914148] ? security_file_permission+0x36/0xb0 > [ 567.914151] vfs_write+0xb0/0x190 > [ 567.914156] SyS_write+0x42/0x90 > [ 567.914160] entry_SYSCALL_64_fastpath+0x1e/0xad > [ 567.914164] ---[ end trace b3703d94611f9a06 ]--- > [ 567.914176] BUG: scheduling while atomic: systemd-sleep/3144/0x00000003 > [ 567.914260] CPU: 3 PID: 3144 Comm: systemd-sleep Tainted: G WC O 4.11.0-rc7-3.g64b92e2-default #1 > [ 567.914262] Call Trace: > [ 567.914273] dump_stack+0x5c/0x7a > [ 567.914278] __schedule_bug+0x55/0x70 > [ 567.914284] __schedule+0x63c/0x8c0 > [ 567.914290] schedule+0x3d/0x90 > [ 567.914295] schedule_timeout+0x16b/0x320 > [ 567.914301] ? del_timer_sync+0x50/0x50 > [ 567.914308] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] > [ 567.914313] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] > [ 567.914316] ? remove_wait_queue+0x60/0x60 > [ 567.914321] ? sst_prepare_and_post_msg+0x275/0x960 [snd_intel_sst_core] > [ 567.914326] ? sst_pause_stream+0x9b/0x110 [snd_intel_sst_core] > [ 567.914333] ? sst_platform_pcm_trigger+0x123/0x1b0 [snd_soc_sst_atom_hifi2_platform] > [ 567.914346] ? soc_pcm_trigger+0xa0/0x120 [snd_soc_core] > [ 567.914353] ? sst_soc_complete+0xa0/0xa0 [snd_soc_sst_atom_hifi2_platform] > [ 567.914365] ? dpcm_fe_dai_do_trigger+0xc8/0x1f0 [snd_soc_core] > [ 567.914373] ? snd_pcm_do_suspend+0x3d/0x40 [snd_pcm] > [ 567.914381] ? snd_pcm_action_single+0x2a/0x70 [snd_pcm] > [ 567.914389] ? snd_pcm_suspend+0x2c/0x40 [snd_pcm] > [ 567.914396] ? snd_pcm_suspend_all+0x32/0x70 [snd_pcm] > [ 567.914408] ? snd_soc_suspend+0x15c/0x3d0 [snd_soc_core] > [ 567.914415] ? sst_soc_prepare+0x23/0xa0 [snd_soc_sst_atom_hifi2_platform] > [ 567.914418] ? dpm_prepare+0x237/0x480 > [ 567.914421] ? dpm_suspend_start+0xd/0x50 > [ 567.914423] ? suspend_devices_and_enter+0xac/0x6f0 > [ 567.914426] ? pm_suspend+0x304/0x380 > [ 567.914428] ? state_store+0x5e/0x90 > [ 567.914430] ? kernfs_fop_write+0xfc/0x190 > [ 567.914433] ? __vfs_write+0x23/0x140 > [ 567.914437] ? handle_mm_fault+0xd3/0x240 > [ 567.914439] ? security_file_permission+0x36/0xb0 > [ 567.914442] ? vfs_write+0xb0/0x190 > [ 567.914445] ? SyS_write+0x42/0x90 > [ 567.914447] ? entry_SYSCALL_64_fastpath+0x1e/0xad > [ 567.914857] BUG: scheduling while atomic: systemd-sleep/3144/0x7fffffff > [ 567.915222] CPU: 1 PID: 3144 Comm: systemd-sleep Tainted: G WC O 4.11.0-rc7-3.g64b92e2-default #1 > [ 567.915231] Call Trace: > [ 567.915300] dump_stack+0x5c/0x7a > [ 567.915324] __schedule_bug+0x55/0x70 > [ 567.915345] __schedule+0x63c/0x8c0 > [ 567.915375] schedule+0x3d/0x90 > [ 567.915392] async_synchronize_cookie_domain+0x85/0x130 > [ 567.915414] ? remove_wait_queue+0x60/0x60 > [ 567.915472] dapm_power_widgets+0x3d4/0x9c0 [snd_soc_core] > [ 567.915530] ? sst_soc_complete+0xa0/0xa0 [snd_soc_sst_atom_hifi2_platform] > [ 567.915579] ? snd_soc_dapm_stream_event+0x87/0xa0 [snd_soc_core] > [ 567.915628] ? snd_soc_dapm_stream_event+0x87/0xa0 [snd_soc_core] > [ 567.915674] ? snd_soc_suspend+0x39c/0x3d0 [snd_soc_core] > [ 567.915699] ? sst_soc_prepare+0x23/0xa0 [snd_soc_sst_atom_hifi2_platform] > [ 567.915712] ? dpm_prepare+0x237/0x480 > [ 567.915723] ? dpm_suspend_start+0xd/0x50 > [ 567.915738] ? suspend_devices_and_enter+0xac/0x6f0 > [ 567.915749] ? pm_suspend+0x304/0x380 > [ 567.915757] ? state_store+0x5e/0x90 > [ 567.915771] ? kernfs_fop_write+0xfc/0x190 > [ 567.915785] ? __vfs_write+0x23/0x140 > [ 567.915800] ? handle_mm_fault+0xd3/0x240 > [ 567.915814] ? security_file_permission+0x36/0xb0 > [ 567.915824] ? vfs_write+0xb0/0x190 > [ 567.915834] ? SyS_write+0x42/0x90 > [ 567.915846] ? entry_SYSCALL_64_fastpath+0x1e/0xad > > > ... and at resume: > > [ 574.320255] BUG: scheduling while atomic: alsa-source-3/1729/0x00000003 > [ 574.320435] CPU: 1 PID: 1729 Comm: alsa-source-3 Tainted: G WC O 4.11.0-rc7-3.g64b92e2-default #1 > [ 574.320440] Call Trace: > [ 574.320474] dump_stack+0x5c/0x7a > [ 574.320484] __schedule_bug+0x55/0x70 > [ 574.320493] __schedule+0x63c/0x8c0 > [ 574.320503] schedule+0x3d/0x90 > [ 574.320509] schedule_timeout+0x16b/0x320 > [ 574.320517] ? del_timer_sync+0x50/0x50 > [ 574.320529] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] > [ 574.320535] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] > [ 574.320539] ? remove_wait_queue+0x60/0x60 > [ 574.320546] ? sst_prepare_and_post_msg+0x275/0x960 [snd_intel_sst_core] > [ 574.320553] ? sst_resume_stream+0x5b/0x100 [snd_intel_sst_core] > [ 574.320564] ? sst_platform_pcm_trigger+0x6b/0x1b0 [snd_soc_sst_atom_hifi2_platform] > [ 574.320586] ? soc_pcm_trigger+0xa0/0x120 [snd_soc_core] > [ 574.320603] ? dpcm_fe_dai_do_trigger+0xc8/0x1f0 [snd_soc_core] > [ 574.320618] ? snd_pcm_action_single+0x2a/0x70 [snd_pcm] > [ 574.320628] ? snd_pcm_common_ioctl1+0x2e5/0xc60 [snd_pcm] > [ 574.320634] ? do_signal+0x23/0x670 > [ 574.320644] ? snd_pcm_capture_ioctl1+0x117/0x280 [snd_pcm] > [ 574.320648] ? ktime_get_ts64+0x4a/0xf0 > [ 574.320659] ? snd_pcm_capture_ioctl+0x2a/0x30 [snd_pcm] > [ 574.320663] ? do_vfs_ioctl+0x8f/0x5d0 > [ 574.320667] ? __fget+0x70/0xc0 > [ 574.320671] ? SyS_ioctl+0x74/0x80 > [ 574.320675] ? entry_SYSCALL_64_fastpath+0x1e/0xad > [ 574.320799] show_signal_msg: 8 callbacks suppressed > [ 574.320805] alsa-source-3[1729]: segfault at 7f28e8052000 ip 00007f28f635eac4 sp 00007f28f0ad4be8 error 6 > [ 574.320882] BUG: scheduling while atomic: alsa-source-3/1729/0x7fffffff > [ 574.321040] CPU: 1 PID: 1729 Comm: alsa-source-3 Tainted: G WC O 4.11.0-rc7-3.g64b92e2-default #1 > [ 574.321043] Call Trace: > [ 574.321068] dump_stack+0x5c/0x7a > [ 574.321076] __schedule_bug+0x55/0x70 > [ 574.321083] __schedule+0x63c/0x8c0 > [ 574.321093] ? wakeup_preempt_entity.isra.58+0x3c/0x50 > [ 574.321097] schedule+0x3d/0x90 > [ 574.321104] schedule_timeout+0x25a/0x320 > [ 574.321109] ? check_preempt_curr+0x79/0x90 > [ 574.321113] ? select_task_rq_rt+0x57/0xa0 > [ 574.321117] ? sched_clock+0x5/0x10 > [ 574.321120] ? sched_clock_cpu+0xc/0xc0 > [ 574.321124] ? wait_for_completion+0xe6/0x150 > [ 574.321128] ? wait_for_completion+0xe6/0x150 > [ 574.321131] ? wake_up_q+0x80/0x80 > [ 574.321135] ? do_coredump+0x3ab/0xf10 > [ 574.321139] ? __wake_up+0x34/0x50 > [ 574.321144] ? get_signal+0x33b/0x660 > [ 574.321150] ? do_signal+0x23/0x670 > [ 574.321154] ? __send_signal+0x213/0x4d0 > [ 574.321160] ? force_sig_info_fault+0x88/0xd0 > [ 574.321166] ? exit_to_usermode_loop+0x71/0xb0 > [ 574.321169] ? prepare_exit_to_usermode+0x2a/0x30 > [ 574.321172] ? retint_user+0x8/0x10 > > > There seem many beasts hidden in this jungle... > > > Takashi -- ~Vinod ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 9:12 ` Vinod Koul @ 2017-04-24 9:43 ` Takashi Iwai 2017-04-24 9:52 ` Vinod Koul 0 siblings, 1 reply; 16+ messages in thread From: Takashi Iwai @ 2017-04-24 9:43 UTC (permalink / raw) To: Vinod Koul; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart On Mon, 24 Apr 2017 11:12:14 +0200, Vinod Koul wrote: > > On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > > Removing ignore_suspend makes the PM succeeds. But it hits some other > > ugly kernel bugs. > > Okay have you marked .nonatomic = true for the machine DAIs? Ah that's it. The patch below seems fixing the PM and the nonatomic problems. I'm not sure about the nonatomic flag for the compress stream, though. Also I fiddled only with FE. Do we need the same flags for BE? The others don't look setting like that, so I left so. thanks, Takashi --- sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -646,7 +646,7 @@ static struct snd_soc_dai_link byt_rt564 .codec_dai_name = "snd-soc-dummy-dai", .codec_name = "snd-soc-dummy", .platform_name = "sst-mfld-platform", - .ignore_suspend = 1, + .nonatomic = true, .dynamic = 1, .dpcm_playback = 1, .dpcm_capture = 1, @@ -659,7 +659,6 @@ static struct snd_soc_dai_link byt_rt564 .codec_dai_name = "snd-soc-dummy-dai", .codec_name = "snd-soc-dummy", .platform_name = "sst-mfld-platform", - .ignore_suspend = 1, .nonatomic = true, .dynamic = 1, .dpcm_playback = 1, @@ -672,6 +671,7 @@ static struct snd_soc_dai_link byt_rt564 .codec_dai_name = "snd-soc-dummy-dai", .codec_name = "snd-soc-dummy", .platform_name = "sst-mfld-platform", + .nonatomic = true, }, /* back ends */ { ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 9:43 ` Takashi Iwai @ 2017-04-24 9:52 ` Vinod Koul 2017-04-24 9:54 ` Takashi Iwai 0 siblings, 1 reply; 16+ messages in thread From: Vinod Koul @ 2017-04-24 9:52 UTC (permalink / raw) To: Takashi Iwai; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > On Mon, 24 Apr 2017 11:12:14 +0200, > Vinod Koul wrote: > > > > On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > > > > Removing ignore_suspend makes the PM succeeds. But it hits some other > > > ugly kernel bugs. > > > > Okay have you marked .nonatomic = true for the machine DAIs? > > Ah that's it. The patch below seems fixing the PM and the nonatomic > problems. I'm not sure about the nonatomic flag for the compress > stream, though. Well we dont have upstream decoders so it wont be used in this case. > Also I fiddled only with FE. Do we need the same flags for BE? The > others don't look setting like that, so I left so. I dont remember if BE needs or not FE should suffice. > > > thanks, > > Takashi > > --- > sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > --- a/sound/soc/intel/boards/bytcr_rt5640.c > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > @@ -646,7 +646,7 @@ static struct snd_soc_dai_link byt_rt564 > .codec_dai_name = "snd-soc-dummy-dai", > .codec_name = "snd-soc-dummy", > .platform_name = "sst-mfld-platform", > - .ignore_suspend = 1, > + .nonatomic = true, > .dynamic = 1, > .dpcm_playback = 1, > .dpcm_capture = 1, > @@ -659,7 +659,6 @@ static struct snd_soc_dai_link byt_rt564 > .codec_dai_name = "snd-soc-dummy-dai", > .codec_name = "snd-soc-dummy", > .platform_name = "sst-mfld-platform", > - .ignore_suspend = 1, > .nonatomic = true, > .dynamic = 1, > .dpcm_playback = 1, > @@ -672,6 +671,7 @@ static struct snd_soc_dai_link byt_rt564 > .codec_dai_name = "snd-soc-dummy-dai", > .codec_name = "snd-soc-dummy", > .platform_name = "sst-mfld-platform", > + .nonatomic = true, > }, > /* back ends */ > { -- ~Vinod ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 9:52 ` Vinod Koul @ 2017-04-24 9:54 ` Takashi Iwai 2017-04-24 11:02 ` Vinod Koul 2017-04-24 14:22 ` Pierre-Louis Bossart 0 siblings, 2 replies; 16+ messages in thread From: Takashi Iwai @ 2017-04-24 9:54 UTC (permalink / raw) To: Vinod Koul; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart On Mon, 24 Apr 2017 11:52:44 +0200, Vinod Koul wrote: > > On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > > On Mon, 24 Apr 2017 11:12:14 +0200, > > Vinod Koul wrote: > > > > > > On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > > > > > > Removing ignore_suspend makes the PM succeeds. But it hits some other > > > > ugly kernel bugs. > > > > > > Okay have you marked .nonatomic = true for the machine DAIs? > > > > Ah that's it. The patch below seems fixing the PM and the nonatomic > > problems. I'm not sure about the nonatomic flag for the compress > > stream, though. > > Well we dont have upstream decoders so it wont be used in this case. > > > Also I fiddled only with FE. Do we need the same flags for BE? The > > others don't look setting like that, so I left so. > > I dont remember if BE needs or not FE should suffice. OK then I leave it as is. When I submit the fix, I should put Cc to stable, and wonder which version we assure the nonatomic ops in SST driver. Did the code base support nonatomic ops from the beginning? thanks, Takashi > > > > > > > thanks, > > > > Takashi > > > > --- > > sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > --- a/sound/soc/intel/boards/bytcr_rt5640.c > > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > > @@ -646,7 +646,7 @@ static struct snd_soc_dai_link byt_rt564 > > .codec_dai_name = "snd-soc-dummy-dai", > > .codec_name = "snd-soc-dummy", > > .platform_name = "sst-mfld-platform", > > - .ignore_suspend = 1, > > + .nonatomic = true, > > .dynamic = 1, > > .dpcm_playback = 1, > > .dpcm_capture = 1, > > @@ -659,7 +659,6 @@ static struct snd_soc_dai_link byt_rt564 > > .codec_dai_name = "snd-soc-dummy-dai", > > .codec_name = "snd-soc-dummy", > > .platform_name = "sst-mfld-platform", > > - .ignore_suspend = 1, > > .nonatomic = true, > > .dynamic = 1, > > .dpcm_playback = 1, > > @@ -672,6 +671,7 @@ static struct snd_soc_dai_link byt_rt564 > > .codec_dai_name = "snd-soc-dummy-dai", > > .codec_name = "snd-soc-dummy", > > .platform_name = "sst-mfld-platform", > > + .nonatomic = true, > > }, > > /* back ends */ > > { > > -- > ~Vinod > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 9:54 ` Takashi Iwai @ 2017-04-24 11:02 ` Vinod Koul 2017-04-24 14:22 ` Pierre-Louis Bossart 1 sibling, 0 replies; 16+ messages in thread From: Vinod Koul @ 2017-04-24 11:02 UTC (permalink / raw) To: Takashi Iwai; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart On Mon, Apr 24, 2017 at 11:54:06AM +0200, Takashi Iwai wrote: > On Mon, 24 Apr 2017 11:52:44 +0200, > Vinod Koul wrote: > > > > On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > > > On Mon, 24 Apr 2017 11:12:14 +0200, > > > Vinod Koul wrote: > > > > > > > > On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > > > > > > > > Removing ignore_suspend makes the PM succeeds. But it hits some other > > > > > ugly kernel bugs. > > > > > > > > Okay have you marked .nonatomic = true for the machine DAIs? > > > > > > Ah that's it. The patch below seems fixing the PM and the nonatomic > > > problems. I'm not sure about the nonatomic flag for the compress > > > stream, though. > > > > Well we dont have upstream decoders so it wont be used in this case. > > > > > Also I fiddled only with FE. Do we need the same flags for BE? The > > > others don't look setting like that, so I left so. > > > > I dont remember if BE needs or not FE should suffice. > > OK then I leave it as is. > > When I submit the fix, I should put Cc to stable, and wonder which > version we assure the nonatomic ops in SST driver. Did the code base > support nonatomic ops from the beginning? 4.1 onwards. The PM was supported thru below commit which went into 4.1. The nonatomic was always a requirement for us due to nature of IPC. Non atomic support went into 3.18 so 4.1 onwards would make sense :) commit 4a8448d4289d7210053a43f9f21e42929beb159b Author: Vinod Koul <vinod.koul@intel.com> Date: Tue Feb 24 11:39:44 2015 +0530 ASoC: Intel: add pm support in sst ipc driver This adds support for system pm support. We need to save the dsp memory which gets lost on suspend and restore that on resume Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> Signed-off-by: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Mark Brown <broonie@kernel.org> Thanks -- ~Vinod ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 9:54 ` Takashi Iwai 2017-04-24 11:02 ` Vinod Koul @ 2017-04-24 14:22 ` Pierre-Louis Bossart 2017-04-24 16:27 ` Vinod Koul 1 sibling, 1 reply; 16+ messages in thread From: Pierre-Louis Bossart @ 2017-04-24 14:22 UTC (permalink / raw) To: Takashi Iwai, Vinod Koul; +Cc: Liam Girdwood, alsa-devel On 4/24/17 4:54 AM, Takashi Iwai wrote: > On Mon, 24 Apr 2017 11:52:44 +0200, > Vinod Koul wrote: >> >> On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: >>> On Mon, 24 Apr 2017 11:12:14 +0200, >>> Vinod Koul wrote: >>>> >>>> On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: >>>> >>>>> Removing ignore_suspend makes the PM succeeds. But it hits some other >>>>> ugly kernel bugs. >>>> >>>> Okay have you marked .nonatomic = true for the machine DAIs? >>> >>> Ah that's it. The patch below seems fixing the PM and the nonatomic >>> problems. I'm not sure about the nonatomic flag for the compress >>> stream, though. >> >> Well we dont have upstream decoders so it wont be used in this case. >> >>> Also I fiddled only with FE. Do we need the same flags for BE? The >>> others don't look setting like that, so I left so. >> >> I dont remember if BE needs or not FE should suffice. > > OK then I leave it as is. > > When I submit the fix, I should put Cc to stable, and wonder which > version we assure the nonatomic ops in SST driver. Did the code base > support nonatomic ops from the beginning? can we take this opportunity to align all drivers? The .nonatomic=true is set in all drivers for the BE, except for cht_bsw_max98090_ti.c It's either needed for all or not needed for all... > > > thanks, > > Takashi > >> >>> >>> >>> thanks, >>> >>> Takashi >>> >>> --- >>> sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> --- a/sound/soc/intel/boards/bytcr_rt5640.c >>> +++ b/sound/soc/intel/boards/bytcr_rt5640.c >>> @@ -646,7 +646,7 @@ static struct snd_soc_dai_link byt_rt564 >>> .codec_dai_name = "snd-soc-dummy-dai", >>> .codec_name = "snd-soc-dummy", >>> .platform_name = "sst-mfld-platform", >>> - .ignore_suspend = 1, >>> + .nonatomic = true, >>> .dynamic = 1, >>> .dpcm_playback = 1, >>> .dpcm_capture = 1, >>> @@ -659,7 +659,6 @@ static struct snd_soc_dai_link byt_rt564 >>> .codec_dai_name = "snd-soc-dummy-dai", >>> .codec_name = "snd-soc-dummy", >>> .platform_name = "sst-mfld-platform", >>> - .ignore_suspend = 1, >>> .nonatomic = true, >>> .dynamic = 1, >>> .dpcm_playback = 1, >>> @@ -672,6 +671,7 @@ static struct snd_soc_dai_link byt_rt564 >>> .codec_dai_name = "snd-soc-dummy-dai", >>> .codec_name = "snd-soc-dummy", >>> .platform_name = "sst-mfld-platform", >>> + .nonatomic = true, >>> }, >>> /* back ends */ >>> { >> >> -- >> ~Vinod >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 14:22 ` Pierre-Louis Bossart @ 2017-04-24 16:27 ` Vinod Koul 2017-04-24 18:32 ` Takashi Iwai 2017-04-24 19:01 ` Pierre-Louis Bossart 0 siblings, 2 replies; 16+ messages in thread From: Vinod Koul @ 2017-04-24 16:27 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: Takashi Iwai, Liam Girdwood, alsa-devel On Mon, Apr 24, 2017 at 09:22:38AM -0500, Pierre-Louis Bossart wrote: > On 4/24/17 4:54 AM, Takashi Iwai wrote: > >On Mon, 24 Apr 2017 11:52:44 +0200, > >Vinod Koul wrote: > >> > >>On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > >>>On Mon, 24 Apr 2017 11:12:14 +0200, > >>>Vinod Koul wrote: > >>>> > >>>>On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > >>>> > >>>>>Removing ignore_suspend makes the PM succeeds. But it hits some other > >>>>>ugly kernel bugs. > >>>> > >>>>Okay have you marked .nonatomic = true for the machine DAIs? > >>> > >>>Ah that's it. The patch below seems fixing the PM and the nonatomic > >>>problems. I'm not sure about the nonatomic flag for the compress > >>>stream, though. > >> > >>Well we dont have upstream decoders so it wont be used in this case. > >> > >>>Also I fiddled only with FE. Do we need the same flags for BE? The > >>>others don't look setting like that, so I left so. > >> > >>I dont remember if BE needs or not FE should suffice. > > > >OK then I leave it as is. > > > >When I submit the fix, I should put Cc to stable, and wonder which > >version we assure the nonatomic ops in SST driver. Did the code base > >support nonatomic ops from the beginning? > > can we take this opportunity to align all drivers? > The .nonatomic=true is set in all drivers for the BE, except for > cht_bsw_max98090_ti.c ?? $ grep nonatomic sound/soc/intel/boards/cht_bsw_max98090_ti.c .nonatomic = true, .nonatomic = true, > It's either needed for all or not needed for all... For the record it is must have for all of the drivers :) $ grep -L nonatomic sound/soc/intel/boards/*.c sound/soc/intel/boards/bdw-rt5677.c sound/soc/intel/boards/broadwell.c sound/soc/intel/boards/byt-max98090.c sound/soc/intel/boards/byt-rt5640.c sound/soc/intel/boards/haswell.c sound/soc/intel/boards/mfld_machine.c So we should add the remaining one byt-max98090.c as Takashi fixed byt-rt5640.c one. I will send the patch for this one. -- ~Vinod ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 16:27 ` Vinod Koul @ 2017-04-24 18:32 ` Takashi Iwai 2017-04-25 3:04 ` Vinod Koul 2017-04-24 19:01 ` Pierre-Louis Bossart 1 sibling, 1 reply; 16+ messages in thread From: Takashi Iwai @ 2017-04-24 18:32 UTC (permalink / raw) To: Vinod Koul; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart On Mon, 24 Apr 2017 18:27:38 +0200, Vinod Koul wrote: > > On Mon, Apr 24, 2017 at 09:22:38AM -0500, Pierre-Louis Bossart wrote: > > On 4/24/17 4:54 AM, Takashi Iwai wrote: > > >On Mon, 24 Apr 2017 11:52:44 +0200, > > >Vinod Koul wrote: > > >> > > >>On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > > >>>On Mon, 24 Apr 2017 11:12:14 +0200, > > >>>Vinod Koul wrote: > > >>>> > > >>>>On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > > >>>> > > >>>>>Removing ignore_suspend makes the PM succeeds. But it hits some other > > >>>>>ugly kernel bugs. > > >>>> > > >>>>Okay have you marked .nonatomic = true for the machine DAIs? > > >>> > > >>>Ah that's it. The patch below seems fixing the PM and the nonatomic > > >>>problems. I'm not sure about the nonatomic flag for the compress > > >>>stream, though. > > >> > > >>Well we dont have upstream decoders so it wont be used in this case. > > >> > > >>>Also I fiddled only with FE. Do we need the same flags for BE? The > > >>>others don't look setting like that, so I left so. > > >> > > >>I dont remember if BE needs or not FE should suffice. > > > > > >OK then I leave it as is. > > > > > >When I submit the fix, I should put Cc to stable, and wonder which > > >version we assure the nonatomic ops in SST driver. Did the code base > > >support nonatomic ops from the beginning? > > > > can we take this opportunity to align all drivers? > > The .nonatomic=true is set in all drivers for the BE, except for > > cht_bsw_max98090_ti.c > > ?? > > $ grep nonatomic sound/soc/intel/boards/cht_bsw_max98090_ti.c > .nonatomic = true, > .nonatomic = true, > > > It's either needed for all or not needed for all... > > For the record it is must have for all of the drivers :) > > $ grep -L nonatomic sound/soc/intel/boards/*.c > sound/soc/intel/boards/bdw-rt5677.c > sound/soc/intel/boards/broadwell.c > sound/soc/intel/boards/byt-max98090.c > sound/soc/intel/boards/byt-rt5640.c > sound/soc/intel/boards/haswell.c > sound/soc/intel/boards/mfld_machine.c > > So we should add the remaining one byt-max98090.c as Takashi fixed > byt-rt5640.c one. I will send the patch for this one. Or maybe we should replace these definitions with some macro to expand to the mostly same contents? The difference is just a few callback functions, basically. Takashi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 18:32 ` Takashi Iwai @ 2017-04-25 3:04 ` Vinod Koul 0 siblings, 0 replies; 16+ messages in thread From: Vinod Koul @ 2017-04-25 3:04 UTC (permalink / raw) To: Takashi Iwai; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart On Mon, Apr 24, 2017 at 08:32:14PM +0200, Takashi Iwai wrote: > On Mon, 24 Apr 2017 18:27:38 +0200, > > > > So we should add the remaining one byt-max98090.c as Takashi fixed > > byt-rt5640.c one. I will send the patch for this one. > > Or maybe we should replace these definitions with some macro to expand > to the mostly same contents? The difference is just a few callback > functions, basically. And while at it, I cant help but wonder but if we can do better and mark it in platform driver thus avoiding it replication in machines. Afterall the atomic trigger is a platform property. Thanks -- ~Vinod ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 16:27 ` Vinod Koul 2017-04-24 18:32 ` Takashi Iwai @ 2017-04-24 19:01 ` Pierre-Louis Bossart 2017-04-25 3:06 ` Vinod Koul 1 sibling, 1 reply; 16+ messages in thread From: Pierre-Louis Bossart @ 2017-04-24 19:01 UTC (permalink / raw) To: Vinod Koul; +Cc: Takashi Iwai, Liam Girdwood, alsa-devel On 4/24/17 11:27 AM, Vinod Koul wrote: > On Mon, Apr 24, 2017 at 09:22:38AM -0500, Pierre-Louis Bossart wrote: >> On 4/24/17 4:54 AM, Takashi Iwai wrote: >>> On Mon, 24 Apr 2017 11:52:44 +0200, >>> Vinod Koul wrote: >>>> >>>> On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: >>>>> On Mon, 24 Apr 2017 11:12:14 +0200, >>>>> Vinod Koul wrote: >>>>>> >>>>>> On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: >>>>>> >>>>>>> Removing ignore_suspend makes the PM succeeds. But it hits some other >>>>>>> ugly kernel bugs. >>>>>> >>>>>> Okay have you marked .nonatomic = true for the machine DAIs? >>>>> >>>>> Ah that's it. The patch below seems fixing the PM and the nonatomic >>>>> problems. I'm not sure about the nonatomic flag for the compress >>>>> stream, though. >>>> >>>> Well we dont have upstream decoders so it wont be used in this case. >>>> >>>>> Also I fiddled only with FE. Do we need the same flags for BE? The >>>>> others don't look setting like that, so I left so. >>>> >>>> I dont remember if BE needs or not FE should suffice. >>> >>> OK then I leave it as is. >>> >>> When I submit the fix, I should put Cc to stable, and wonder which >>> version we assure the nonatomic ops in SST driver. Did the code base >>> support nonatomic ops from the beginning? >> >> can we take this opportunity to align all drivers? >> The .nonatomic=true is set in all drivers for the BE, except for >> cht_bsw_max98090_ti.c > > ?? > > $ grep nonatomic sound/soc/intel/boards/cht_bsw_max98090_ti.c > .nonatomic = true, > .nonatomic = true, Yes, the .nonatomic is set for the two PCM frontends but not for the SSP2 backend. > >> It's either needed for all or not needed for all... > > For the record it is must have for all of the drivers :) > > $ grep -L nonatomic sound/soc/intel/boards/*.c > sound/soc/intel/boards/bdw-rt5677.c > sound/soc/intel/boards/broadwell.c > sound/soc/intel/boards/byt-max98090.c > sound/soc/intel/boards/byt-rt5640.c > sound/soc/intel/boards/haswell.c > sound/soc/intel/boards/mfld_machine.c > > So we should add the remaining one byt-max98090.c as Takashi fixed > byt-rt5640.c one. I will send the patch for this one. Takashi fixed the bytcr-rt5640 driver, I am not sure we should touch the old byt- driver since they are not using the same firmware and they are not enabled by default. Same for broadwell, different driver, different problems. I was also also talking about drivers that have .nonatomic field set but not everywhere consistently. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 19:01 ` Pierre-Louis Bossart @ 2017-04-25 3:06 ` Vinod Koul 0 siblings, 0 replies; 16+ messages in thread From: Vinod Koul @ 2017-04-25 3:06 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: Takashi Iwai, Liam Girdwood, alsa-devel On Mon, Apr 24, 2017 at 02:01:44PM -0500, Pierre-Louis Bossart wrote: > On 4/24/17 11:27 AM, Vinod Koul wrote: > >On Mon, Apr 24, 2017 at 09:22:38AM -0500, Pierre-Louis Bossart wrote: > >>On 4/24/17 4:54 AM, Takashi Iwai wrote: > >>>On Mon, 24 Apr 2017 11:52:44 +0200, > >>>Vinod Koul wrote: > >>>> > >>>>On Mon, Apr 24, 2017 at 11:43:47AM +0200, Takashi Iwai wrote: > >>>>>On Mon, 24 Apr 2017 11:12:14 +0200, > >>>>>Vinod Koul wrote: > >>>>>> > >>>>>>On Mon, Apr 24, 2017 at 11:00:45AM +0200, Takashi Iwai wrote: > >>>>>> > >>>>>>>Removing ignore_suspend makes the PM succeeds. But it hits some other > >>>>>>>ugly kernel bugs. > >>>>>> > >>>>>>Okay have you marked .nonatomic = true for the machine DAIs? > >>>>> > >>>>>Ah that's it. The patch below seems fixing the PM and the nonatomic > >>>>>problems. I'm not sure about the nonatomic flag for the compress > >>>>>stream, though. > >>>> > >>>>Well we dont have upstream decoders so it wont be used in this case. > >>>> > >>>>>Also I fiddled only with FE. Do we need the same flags for BE? The > >>>>>others don't look setting like that, so I left so. > >>>> > >>>>I dont remember if BE needs or not FE should suffice. > >>> > >>>OK then I leave it as is. > >>> > >>>When I submit the fix, I should put Cc to stable, and wonder which > >>>version we assure the nonatomic ops in SST driver. Did the code base > >>>support nonatomic ops from the beginning? > >> > >>can we take this opportunity to align all drivers? > >>The .nonatomic=true is set in all drivers for the BE, except for > >>cht_bsw_max98090_ti.c > > > >?? > > > >$ grep nonatomic sound/soc/intel/boards/cht_bsw_max98090_ti.c > > .nonatomic = true, > > .nonatomic = true, > > Yes, the .nonatomic is set for the two PCM frontends but not for the > SSP2 backend. Oh okay, sorry I misunderstood that, I dont think we need this for BE. > > > > >>It's either needed for all or not needed for all... > > > >For the record it is must have for all of the drivers :) > > > >$ grep -L nonatomic sound/soc/intel/boards/*.c > >sound/soc/intel/boards/bdw-rt5677.c > >sound/soc/intel/boards/broadwell.c > >sound/soc/intel/boards/byt-max98090.c > >sound/soc/intel/boards/byt-rt5640.c > >sound/soc/intel/boards/haswell.c > >sound/soc/intel/boards/mfld_machine.c > > > >So we should add the remaining one byt-max98090.c as Takashi fixed > >byt-rt5640.c one. I will send the patch for this one. > > Takashi fixed the bytcr-rt5640 driver, I am not sure we should touch > the old byt- driver since they are not using the same firmware and > they are not enabled by default. Same for broadwell, different > driver, different problems. on that not we should rename file to depict which driver they use older or newer :) > I was also also talking about drivers that have .nonatomic field set > but not everywhere consistently. -- ~Vinod ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PM issue with Intel SST Atom driver 2017-04-24 7:15 ` Takashi Iwai 2017-04-24 9:00 ` Takashi Iwai @ 2017-04-24 9:09 ` Vinod Koul 1 sibling, 0 replies; 16+ messages in thread From: Vinod Koul @ 2017-04-24 9:09 UTC (permalink / raw) To: Takashi Iwai; +Cc: Liam Girdwood, alsa-devel, Pierre-Louis Bossart 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 :) -- ~Vinod ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-04-25 3:04 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-21 13:42 PM issue with Intel SST Atom driver Takashi Iwai 2017-04-24 5:01 ` Vinod Koul 2017-04-24 7:15 ` Takashi Iwai 2017-04-24 9:00 ` Takashi Iwai 2017-04-24 9:12 ` Vinod Koul 2017-04-24 9:43 ` Takashi Iwai 2017-04-24 9:52 ` Vinod Koul 2017-04-24 9:54 ` Takashi Iwai 2017-04-24 11:02 ` Vinod Koul 2017-04-24 14:22 ` Pierre-Louis Bossart 2017-04-24 16:27 ` Vinod Koul 2017-04-24 18:32 ` Takashi Iwai 2017-04-25 3:04 ` Vinod Koul 2017-04-24 19:01 ` Pierre-Louis Bossart 2017-04-25 3:06 ` Vinod Koul 2017-04-24 9:09 ` Vinod Koul
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.