[alsa-devel] PM issue with Intel SST Atom driver
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
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?
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
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
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
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 */ {
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,@@ -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,@@ -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,
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,@@ -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,@@ -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,-- ~Vinod
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
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,@@ -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,@@ -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,-- ~Vinod
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/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.
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
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
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.
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.
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 :)
participants (3)
-
Pierre-Louis Bossart -
Takashi Iwai -
Vinod Koul