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

From: Cezary Rojewski <cezary.rojewski@intel.com>
To: Hans de Goede <hdegoede@redhat.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.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: Tue, 31 Mar 2020 12:28:23 +0200	[thread overview]
Message-ID: <9f1884b4-e356-9a8f-6912-0d55c4bcbf06@intel.com> (raw)
In-Reply-To: <259cbb60-9e59-1132-8371-83f837577f86@redhat.com>

On 2020-03-30 23:39, Hans de Goede wrote:
> Hi,
> 
>> On 3/19/20 11:14 PM, Pierre-Louis Bossart wrote:
>>
>> 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...
> 
> Regards,
> 
> Hans
> 

Thanks for your report Hans!

As all streams (whether that's playback routed through Headphones or 
Speaker, or capture) were connected to SSP0 dai link, there is a 
question to be raised: Is Capture path needed to be up during suspend 
for broadwell solutions?

Guess these two lines you mentioned above have the exact same impact on 
playback as complete .ignore_suspend flag removal from SSP0 dai link.

Don't believe WoV for WPT has been added for SST linux so 
.ignore_suspend=1 achieves nothing. The offload part is probably just 
limited to bigger buffer size compared to system pin than anything else. 
So, nothing prevents from removing .ignore_suspend for SST solutions 
until WoV is verified. Don't know about SOF side.
Pierre, what's your take on this?

Czarek