| Message ID | 20170424120955.15519-1-tiwai@suse.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6e4cac23c5a648d50b107d1b53e9c4e1120c7943 |
| Headers | show |
On Mon, Apr 24, 2017 at 02:09:55PM +0200, Takashi Iwai wrote: > The FE setups of Intel SST bytcr_rt5640 and bytcr_rt5651 drivers carry > the ignore_suspend flag, and this prevents the suspend/resume working > properly while the stream is running, since SST core code has the > check of the running streams and returns -EBUSY. Drop these > superfluous flags for fixing the behavior. > > Also, the bytcr_rt5640 driver lacks of nonatomic flag in some FE > definitions, which leads to the kernel Oops at suspend/resume like: > > BUG: scheduling while atomic: systemd-sleep/3144/0x00000003 > Call Trace: > dump_stack+0x5c/0x7a > __schedule_bug+0x55/0x70 > __schedule+0x63c/0x8c0 > schedule+0x3d/0x90 > schedule_timeout+0x16b/0x320 > ? del_timer_sync+0x50/0x50 > ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] > ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] > ? remove_wait_queue+0x60/0x60 > ? sst_prepare_and_post_msg+0x275/0x960 [snd_intel_sst_core] > ? sst_pause_stream+0x9b/0x110 [snd_intel_sst_core] > .... > > This patch addresses these appropriately, too. Acked-by: Vinod Koul <vinod.koul@intel.com>
On 04/24/2017 07:09 AM, Takashi Iwai wrote: > The FE setups of Intel SST bytcr_rt5640 and bytcr_rt5651 drivers carry > the ignore_suspend flag, and this prevents the suspend/resume working > properly while the stream is running, since SST core code has the > check of the running streams and returns -EBUSY. Drop these > superfluous flags for fixing the behavior. > > Also, the bytcr_rt5640 driver lacks of nonatomic flag in some FE > definitions, which leads to the kernel Oops at suspend/resume like: This patch also fixes the known bug that reboot had to be forced on some BYT platforms with a manual restart when the audio driver was enabled. Thanks Takashi! > > BUG: scheduling while atomic: systemd-sleep/3144/0x00000003 > Call Trace: > dump_stack+0x5c/0x7a > __schedule_bug+0x55/0x70 > __schedule+0x63c/0x8c0 > schedule+0x3d/0x90 > schedule_timeout+0x16b/0x320 > ? del_timer_sync+0x50/0x50 > ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] > ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] > ? remove_wait_queue+0x60/0x60 > ? sst_prepare_and_post_msg+0x275/0x960 [snd_intel_sst_core] > ? sst_pause_stream+0x9b/0x110 [snd_intel_sst_core] > .... > > This patch addresses these appropriately, too. > > Cc: <stable@vger.kernel.org> # v4.1+ > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- > sound/soc/intel/boards/bytcr_rt5651.c | 2 -- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c > index 5c7219fb3aa8..9e2a3404a836 100644 > --- a/sound/soc/intel/boards/bytcr_rt5640.c > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > @@ -621,7 +621,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { > .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, > @@ -634,7 +634,6 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { > .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, > @@ -661,6 +660,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { > | SND_SOC_DAIFMT_CBS_CFS, > .be_hw_params_fixup = byt_rt5640_codec_fixup, > .ignore_suspend = 1, > + .nonatomic = true, > .dpcm_playback = 1, > .dpcm_capture = 1, > .init = byt_rt5640_init, > diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c > index 3186f015939f..8164bec63bf1 100644 > --- a/sound/soc/intel/boards/bytcr_rt5651.c > +++ b/sound/soc/intel/boards/bytcr_rt5651.c > @@ -235,7 +235,6 @@ static struct snd_soc_dai_link byt_rt5651_dais[] = { > .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, > @@ -249,7 +248,6 @@ static struct snd_soc_dai_link byt_rt5651_dais[] = { > .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,
On Mon, 24 Apr 2017 23:39:47 +0200, Pierre-Louis Bossart wrote: > > > > On 04/24/2017 07:09 AM, Takashi Iwai wrote: > > The FE setups of Intel SST bytcr_rt5640 and bytcr_rt5651 drivers carry > > the ignore_suspend flag, and this prevents the suspend/resume working > > properly while the stream is running, since SST core code has the > > check of the running streams and returns -EBUSY. Drop these > > superfluous flags for fixing the behavior. > > > > Also, the bytcr_rt5640 driver lacks of nonatomic flag in some FE > > definitions, which leads to the kernel Oops at suspend/resume like: > This patch also fixes the known bug that reboot had to be forced on > some BYT platforms with a manual restart when the audio driver was > enabled. > Thanks Takashi! While we're at it: could you submit the UCM profiles as alsa-lib upstream? At least the ones for the drivers that are in the current kernel should be merged. If there are multiple board-specific UCMs for the same driver, we can use now card's longname (generated from DMI string) as the primary source, while keeping the $DRIVER/$DRIVER.conf as the fallback. thanks, Takashi
On 4/24/17 4:45 PM, Takashi Iwai wrote: > On Mon, 24 Apr 2017 23:39:47 +0200, > Pierre-Louis Bossart wrote: >> >> >> >> On 04/24/2017 07:09 AM, Takashi Iwai wrote: >>> The FE setups of Intel SST bytcr_rt5640 and bytcr_rt5651 drivers carry >>> the ignore_suspend flag, and this prevents the suspend/resume working >>> properly while the stream is running, since SST core code has the >>> check of the running streams and returns -EBUSY. Drop these >>> superfluous flags for fixing the behavior. >>> >>> Also, the bytcr_rt5640 driver lacks of nonatomic flag in some FE >>> definitions, which leads to the kernel Oops at suspend/resume like: >> This patch also fixes the known bug that reboot had to be forced on >> some BYT platforms with a manual restart when the audio driver was >> enabled. >> Thanks Takashi! > > While we're at it: could you submit the UCM profiles as alsa-lib > upstream? At least the ones for the drivers that are in the current > kernel should be merged. > > If there are multiple board-specific UCMs for the same driver, we can > use now card's longname (generated from DMI string) as the primary > source, while keeping the $DRIVER/$DRIVER.conf as the fallback. Well I've lost track of which git repo UCM files should be in, both the license and the update rate of alsa-lib are problematic for configuration files. We also talked about some include capabilities to avoid copy/paste but I can't recall having seen them upstream. I also haven't had time to test the long name on my devices.
On Tue, 25 Apr 2017 00:18:31 +0200, Pierre-Louis Bossart wrote: > > On 4/24/17 4:45 PM, Takashi Iwai wrote: > > On Mon, 24 Apr 2017 23:39:47 +0200, > > Pierre-Louis Bossart wrote: > >> > >> > >> > >> On 04/24/2017 07:09 AM, Takashi Iwai wrote: > >>> The FE setups of Intel SST bytcr_rt5640 and bytcr_rt5651 drivers carry > >>> the ignore_suspend flag, and this prevents the suspend/resume working > >>> properly while the stream is running, since SST core code has the > >>> check of the running streams and returns -EBUSY. Drop these > >>> superfluous flags for fixing the behavior. > >>> > >>> Also, the bytcr_rt5640 driver lacks of nonatomic flag in some FE > >>> definitions, which leads to the kernel Oops at suspend/resume like: > >> This patch also fixes the known bug that reboot had to be forced on > >> some BYT platforms with a manual restart when the audio driver was > >> enabled. > >> Thanks Takashi! > > > > While we're at it: could you submit the UCM profiles as alsa-lib > > upstream? At least the ones for the drivers that are in the current > > kernel should be merged. > > > > If there are multiple board-specific UCMs for the same driver, we can > > use now card's longname (generated from DMI string) as the primary > > source, while keeping the $DRIVER/$DRIVER.conf as the fallback. > > Well I've lost track of which git repo UCM files should be in, both > the license and the update rate of alsa-lib are problematic for > configuration files. The split to another repo is planned but doesn't happen yet. I guess we'll do it after 1.1.4 release. So far, you can submit the existing files just to alsa-lib as is unless you want inevitably another license. > We also talked about some include capabilities to > avoid copy/paste but I can't recall having seen them upstream. Hm, it's basically always possible as an alsa-lib config feasture, but never used. We can work on it later once after gathering the similar profiles. > I also haven't had time to test the long name on my devices. That's OK, we can begin with the basic ones such as bytcr_rt5640.conf. The longname stuff is new and will be available at first in 3.12, so it's a feature in near future. thanks, Takashi
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index 5c7219fb3aa8..9e2a3404a836 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -621,7 +621,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { .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, @@ -634,7 +634,6 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { .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, @@ -661,6 +660,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { | SND_SOC_DAIFMT_CBS_CFS, .be_hw_params_fixup = byt_rt5640_codec_fixup, .ignore_suspend = 1, + .nonatomic = true, .dpcm_playback = 1, .dpcm_capture = 1, .init = byt_rt5640_init, diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index 3186f015939f..8164bec63bf1 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -235,7 +235,6 @@ static struct snd_soc_dai_link byt_rt5651_dais[] = { .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, @@ -249,7 +248,6 @@ static struct snd_soc_dai_link byt_rt5651_dais[] = { .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,
The FE setups of Intel SST bytcr_rt5640 and bytcr_rt5651 drivers carry the ignore_suspend flag, and this prevents the suspend/resume working properly while the stream is running, since SST core code has the check of the running streams and returns -EBUSY. Drop these superfluous flags for fixing the behavior. Also, the bytcr_rt5640 driver lacks of nonatomic flag in some FE definitions, which leads to the kernel Oops at suspend/resume like: BUG: scheduling while atomic: systemd-sleep/3144/0x00000003 Call Trace: dump_stack+0x5c/0x7a __schedule_bug+0x55/0x70 __schedule+0x63c/0x8c0 schedule+0x3d/0x90 schedule_timeout+0x16b/0x320 ? del_timer_sync+0x50/0x50 ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] ? sst_wait_timeout+0xa9/0x170 [snd_intel_sst_core] ? remove_wait_queue+0x60/0x60 ? sst_prepare_and_post_msg+0x275/0x960 [snd_intel_sst_core] ? sst_pause_stream+0x9b/0x110 [snd_intel_sst_core] .... This patch addresses these appropriately, too. Cc: <stable@vger.kernel.org> # v4.1+ Signed-off-by: Takashi Iwai <tiwai@suse.de> --- sound/soc/intel/boards/bytcr_rt5640.c | 4 ++-- sound/soc/intel/boards/bytcr_rt5651.c | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-)