Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

From: Hans de Goede <hdegoede@redhat.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Cezary Rojewski <cezary.rojewski@intel.com>,
	alsa-devel@alsa-project.org
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Curtis Malainey <cujomalainey@google.com>,
	tiwai@suse.com, Dominik Brodowski <linux@dominikbrodowski.net>,
	lgirdwood@gmail.com, vkoul@kernel.org, broonie@kernel.org,
	Ben Zhang <benzh@google.com>
Subject: Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link
Date: Sun, 5 Apr 2020 20:10:04 +0200	[thread overview]
Message-ID: <fdb1f5f5-53a2-f1ae-c193-145c8a7f4f42@redhat.com> (raw)
In-Reply-To: <259cbb60-9e59-1132-8371-83f837577f86@redhat.com>

Hi,

On 3/30/20 11:39 PM, Hans de Goede wrote:
> Hi,
> 
> On 3/19/20 11:14 PM, Pierre-Louis Bossart wrote:
>>
>>
>> On 3/19/20 3:49 PM, Cezary Rojewski wrote:
>>> As of commit:
>>> ASoC: soc-core: care .ignore_suspend for Component suspend
>>>
>>> function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend'
>>> flag for dai links. While BE dai link for System Pin is
>>> supposed to follow standard suspend-resume flow, appended
>>> 'ignore_suspend' flag disturbs that flow and causes audio to break
>>> right after resume. Remove the flag to address this.
>>>
>>> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
>>> Cc: Mark Brown <broonie@kernel.org>
>>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
>>
>> we should ask Ben and Curtis @ Google if the changes related to suspend interfere with the wake-on-voice support?
>>
>> Btw the .ignore_suspend is also set in bytcr_rt5640/51 drivers, so wondering if additional devices are broken, or if there's something off about Broadwell in general. Hans, have you heard of any regressions on Baytrail devices?
> 
> I've just tested 5.6.0 on Bay Trail + a rt5651 codec,
> using the bytcr_rt5651 machine driver which sets
> ignore_suspend, as well as on a Cherry Trail + rt5645
> device using the chtrt5645 machine driver which does
> _not_ set ignore suspend.
> 
> Suspend/resume work fine on both and music playing
> before suspend continues playing after suspend.
> 
> Note that the bytcr_rt5651 machine driver also does:
> 
>          snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone");
>          snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
> 
> Which the bdw-rt5677 seems to not do...

I just noticed something which is somewhat related to this
discussion (and also somewhat off topic).

I just noticed on a Bay Trail device with a RT5672 codec
and on a Cherry Trail device with a RT5645 codec that
if an input / recording audio stream is active while
suspending the tablet, then after resume audio is broken,
playback seems to work (audio samples get consumed at normal
speed) but it is silent.  Recording also is broken, not
sure if it is broken, or just silent too.

When this happens, closing all apps which consume audio
and waiting 5 seconds for a runtime-suspend to kick in
fixes things. After this both record and playback
works again.

Any idea what the cause for this problem might be?

I can reproduce this in 2 ways:

1. Have the sound capplet of GNOME's control-panel
open, this shows a VU meter for the default audio
input, this VU meter stops working after a suspend
resume and playback also stops working if a suspend/resume
is done with the VU meter active when suspending.

2. Start a sound recording in gnome-sound-recorder
and then suspend + resume.

Regards,

Hans