snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

Hi!

While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
some sound-related trouble: after boot, the sound works fine -- but once I
suspend and resume my broadwell-based XPS13, I need to switch to headphone
and back to speaker to hear something. But what I hear isn't music but
garbled output.

A few dmesg snippets from v5.6-rc6-9-gac309e7744be which might be of
interest. I've highlighted the lines differing from v.5.5.x which might be
of special interest:

	...
	snd_hda_intel 0000:00:03.0: enabling device (0000 -> 0002)
	usbcore: registered new interface driver snd-usb-audio
	snd_hda_intel 0000:00:03.0: bound 0000:00:02.0 (ops i915_audio_component_bind_ops)
	input: HDA Intel HDMI HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:03.0/sound/card0/input13
	input: HDA Intel HDMI HDMI/DP,pcm=7 as /devices/pci0000:00/0000:00:03.0/sound/card0/input14
	input: HDA Intel HDMI HDMI/DP,pcm=8 as /devices/pci0000:00/0000:00:03.0/sound/card0/input15
	input: HDA Intel HDMI HDMI/DP,pcm=9 as /devices/pci0000:00/0000:00:03.0/sound/card0/input16
	input: HDA Intel HDMI HDMI/DP,pcm=10 as /devices/pci0000:00/0000:00:03.0/sound/card0/input17
	Console: switching to colour frame buffer device 240x67
!!!	sst-acpi INT3438:00: WARN: Device release is not defined so it is not safe to unbind this driver while in use
	i915 0000:00:02.0: fb0: i915drmfb frame buffer device
	sst-acpi INT3438:00: DesignWare DMA Controller, 8 channels
	psmouse serio1: synaptics: Unable to query device: -5
	haswell-pcm-audio haswell-pcm-audio: Direct firmware load for intel/IntcPP01.bin failed with error -2
	haswell-pcm-audio haswell-pcm-audio: fw image intel/IntcPP01.bin not available(-2)
	haswell-pcm-audio haswell-pcm-audio: FW loaded, mailbox readback FW info: type 01, - version: 00.00, build 77, source commit id: 876ac6906f31a43b6772b23c7c983ce9dcb18a19
	rt286 i2c-INT343A:00: ASoC: sink widget DMIC1 overwritten
	rt286 i2c-INT343A:00: ASoC: source widget DMIC1 overwritten
	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> System Pin mapping ok
	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Offload0 Pin mapping ok
	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Offload1 Pin mapping ok
	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Loopback Pin mapping ok
	broadwell-audio broadwell-audio: rt286-aif1 <-> snd-soc-dummy-dai mapping ok
	input: broadwell-rt286 Headset as /devices/pci0000:00/INT3438:00/broadwell-audio/sound/card1/input18
	...
	ALSA device list:
	  #0: HDA Intel HDMI at 0xf7218000 irq 48
	  #1: DellInc.-XPS139343--0TM99H
	...
!!!	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to reset, ignore it.
!!!	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to free, ignore it.

(these last two messages already are printed a couple of time after boot, and then
again during a suspend/resume cycle. On v.5.5.y, there are similar messages
"no context buffer need to restore!"). Everything is built-in, no modules
are loaded.

Unfortunately, I cannot bisect this issue easily -- i915 was broken for
quite some time on this system[*], prohibiting boot...

Thanks for taking a look at this issue!

	Dominik


[*] https://gitlab.freedesktop.org/drm/intel/issues/1151

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-18 07:30, Dominik Brodowski wrote:
> Hi!
> 
> While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
> some sound-related trouble: after boot, the sound works fine -- but once I
> suspend and resume my broadwell-based XPS13, I need to switch to headphone
> and back to speaker to hear something. But what I hear isn't music but
> garbled output.
> 
> A few dmesg snippets from v5.6-rc6-9-gac309e7744be which might be of
> interest. I've highlighted the lines differing from v.5.5.x which might be
> of special interest:
> 

Thank you for the report, Dominik. You definitely got our attention.

I've checked the market: Dell XPS 13 9343, yes? Once you confirm model 
id, I'll order a piece immediately to our site.

In regard to logs, thanks for highlighting important lines. Build is of 
'rc' so bugs can still be in plenty - any reason for switching to 
cutting-edge kernel on production stuff? Our CI didn't detect any 
anomalies yet as it is running on 5.5.

I'll direct your ticket on todays meeting. On the first look, issue 
seems to be connected with recent changes to /drivers/dma/dmaengine.c. 
DesignWare DMA controller drv - which HSW/BDW makes use of - might not 
have been updated accordingly. Will dig further on that.

One more, just to make it clear for the rest of the viewers:

 > 	haswell-pcm-audio haswell-pcm-audio: Direct firmware load for 
intel/IntcPP01.bin failed with error -2
 > 	haswell-pcm-audio haswell-pcm-audio: fw image intel/IntcPP01.bin not 
available(-2)

Back in the ancient days of DSP (HSW/BDW are actually the very first 
audio DSP hws for Intel) topology was part of FW - SW could not 
configure it and probably that's why library IntcPP01 is attempted to be 
loaded on every boot, even if it's not part of configuration for given 
hw. Maybe we could make it quieter though..

> 
> (these last two messages already are printed a couple of time after boot, and then
> again during a suspend/resume cycle. On v.5.5.y, there are similar messages
> "no context buffer need to restore!"). Everything is built-in, no modules
> are loaded.
> 
> Unfortunately, I cannot bisect this issue easily -- i915 was broken for
> quite some time on this system[*], prohibiting boot...

Hmm, sounds like that issue is quite old. DSP for Haswell and Broadwell 
is available for I2S devices only, so this relates directly to legacy 
HDA driver. Compared to Skylake+, HDAudio controller for older platforms 
is found within GPU. My advice is to notify the DRM guys about this issue.

Takashi, are you aware of problems with HDMI on HSW/ BDW or should I 
just loop Jani and other DRM peps here?

Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Wed, Mar 18, 2020 at 10:41:42AM +0100, Cezary Rojewski wrote:
> On 2020-03-18 07:30, Dominik Brodowski wrote:
> > Hi!
> > 
> > While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
> > some sound-related trouble: after boot, the sound works fine -- but once I
> > suspend and resume my broadwell-based XPS13, I need to switch to headphone
> > and back to speaker to hear something. But what I hear isn't music but
> > garbled output.
> > 
> > A few dmesg snippets from v5.6-rc6-9-gac309e7744be which might be of
> > interest. I've highlighted the lines differing from v.5.5.x which might be
> > of special interest:
> > 
> 
> Thank you for the report, Dominik. You definitely got our attention.
> 
> I've checked the market: Dell XPS 13 9343, yes? Once you confirm model id,
> I'll order a piece immediately to our site.

Thanks, for taking a look at this issue, Czarek. Indeed, it's a

	Dell Inc. XPS 13 9343/0TM99H, BIOS A19 12/24/2018

and IIRC there should be at least one of those already at the Linux sound
team at Intel (you may want to check with Yang Jie and Han Lu; I was in
contact with them relating to a different sound-related issue in 2015).

> In regard to logs, thanks for highlighting important lines. Build is of 'rc'
> so bugs can still be in plenty - any reason for switching to cutting-edge
> kernel on production stuff? Our CI didn't detect any anomalies yet as it is
> running on 5.5.

Well, one has to test things to find bugs ;-) -- and as I sometimes
contribute to the kernel, it is better to keep current on things.

> I'll direct your ticket on todays meeting. On the first look, issue seems to
> be connected with recent changes to /drivers/dma/dmaengine.c. DesignWare DMA
> controller drv - which HSW/BDW makes use of - might not have been updated
> accordingly. Will dig further on that.
> 
> One more, just to make it clear for the rest of the viewers:
> 
> > 	haswell-pcm-audio haswell-pcm-audio: Direct firmware load for intel/IntcPP01.bin failed with error -2
> > 	haswell-pcm-audio haswell-pcm-audio: fw image intel/IntcPP01.bin not available(-2)
> 
> Back in the ancient days of DSP (HSW/BDW are actually the very first audio
> DSP hws for Intel) topology was part of FW - SW could not configure it and
> probably that's why library IntcPP01 is attempted to be loaded on every
> boot, even if it's not part of configuration for given hw. Maybe we could
> make it quieter though..
> 
> > 
> > (these last two messages already are printed a couple of time after boot, and then
> > again during a suspend/resume cycle. On v.5.5.y, there are similar messages
> > "no context buffer need to restore!"). Everything is built-in, no modules
> > are loaded.
> > 
> > Unfortunately, I cannot bisect this issue easily -- i915 was broken for
> > quite some time on this system[*], prohibiting boot...
> 
> Hmm, sounds like that issue is quite old. DSP for Haswell and Broadwell is
> available for I2S devices only, so this relates directly to legacy HDA
> driver. Compared to Skylake+, HDAudio controller for older platforms is
> found within GPU. My advice is to notify the DRM guys about this issue.
> 
> Takashi, are you aware of problems with HDMI on HSW/ BDW or should I just
> loop Jani and other DRM peps here?

Well, it works on v5.5, so this issue is not really "quite old" (the "no
context buffer need to restore!" message seen there seems harmless).

Thanks again, and best wishes,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-18 10:57, Dominik Brodowski wrote:
>>>
>>> Unfortunately, I cannot bisect this issue easily -- i915 was broken for
>>> quite some time on this system[*], prohibiting boot...
>>
>> Hmm, sounds like that issue is quite old. DSP for Haswell and Broadwell is
>> available for I2S devices only, so this relates directly to legacy HDA
>> driver. Compared to Skylake+, HDAudio controller for older platforms is
>> found within GPU. My advice is to notify the DRM guys about this issue.
>>
>> Takashi, are you aware of problems with HDMI on HSW/ BDW or should I just
>> loop Jani and other DRM peps here?
> 
> Well, it works on v5.5, so this issue is not really "quite old" (the "no
> context buffer need to restore!" message seen there seems harmless).
> 
> Thanks again, and best wishes,
> 	Dominik
> 

Was commenting the "i915 was broken for quite some time on this 
system[*], prohibiting boot...". Unless I misunderstood you, this ain't 
a DSP driver issue but HDAudio/iDisp one. Essentially, these are two 
issues you mentioned here.

Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Wed, Mar 18, 2020 at 11:05:21AM +0100, Cezary Rojewski wrote:
> On 2020-03-18 10:57, Dominik Brodowski wrote:
> > > > 
> > > > Unfortunately, I cannot bisect this issue easily -- i915 was broken for
> > > > quite some time on this system[*], prohibiting boot...
> > > 
> > > Hmm, sounds like that issue is quite old. DSP for Haswell and Broadwell is
> > > available for I2S devices only, so this relates directly to legacy HDA
> > > driver. Compared to Skylake+, HDAudio controller for older platforms is
> > > found within GPU. My advice is to notify the DRM guys about this issue.
> > > 
> > > Takashi, are you aware of problems with HDMI on HSW/ BDW or should I just
> > > loop Jani and other DRM peps here?
> > 
> > Well, it works on v5.5, so this issue is not really "quite old" (the "no
> > context buffer need to restore!" message seen there seems harmless).
> > 
> > Thanks again, and best wishes,
> > 	Dominik
> > 
> 
> Was commenting the "i915 was broken for quite some time on this system[*],
> prohibiting boot...". Unless I misunderstood you, this ain't a DSP driver
> issue but HDAudio/iDisp one. Essentially, these are two issues you mentioned
> here.

Ah, sorry for the confusion. That issue was introduced post v5.5 as well,
but fixed for v5.6-rc4 -- meaning that it works now, but that bisecting
between v5.5 and v5.6-rc4 is not working as such.

Thanks,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Keyon Jie]

On 3/18/20 2:30 PM, Dominik Brodowski wrote:
> Hi!
> 
> While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
> some sound-related trouble: after boot, the sound works fine -- but once I
> suspend and resume my broadwell-based XPS13, I need to switch to headphone
> and back to speaker to hear something. But what I hear isn't music but
> garbled output.
> 
> A few dmesg snippets from v5.6-rc6-9-gac309e7744be which might be of
> interest. I've highlighted the lines differing from v.5.5.x which might be
> of special interest:
> 
> 	...
> 	snd_hda_intel 0000:00:03.0: enabling device (0000 -> 0002)
> 	usbcore: registered new interface driver snd-usb-audio
> 	snd_hda_intel 0000:00:03.0: bound 0000:00:02.0 (ops i915_audio_component_bind_ops)
> 	input: HDA Intel HDMI HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:03.0/sound/card0/input13
> 	input: HDA Intel HDMI HDMI/DP,pcm=7 as /devices/pci0000:00/0000:00:03.0/sound/card0/input14
> 	input: HDA Intel HDMI HDMI/DP,pcm=8 as /devices/pci0000:00/0000:00:03.0/sound/card0/input15
> 	input: HDA Intel HDMI HDMI/DP,pcm=9 as /devices/pci0000:00/0000:00:03.0/sound/card0/input16
> 	input: HDA Intel HDMI HDMI/DP,pcm=10 as /devices/pci0000:00/0000:00:03.0/sound/card0/input17
> 	Console: switching to colour frame buffer device 240x67
> !!!	sst-acpi INT3438:00: WARN: Device release is not defined so it is not safe to unbind this driver while in use
> 	i915 0000:00:02.0: fb0: i915drmfb frame buffer device
> 	sst-acpi INT3438:00: DesignWare DMA Controller, 8 channels
> 	psmouse serio1: synaptics: Unable to query device: -5
> 	haswell-pcm-audio haswell-pcm-audio: Direct firmware load for intel/IntcPP01.bin failed with error -2
> 	haswell-pcm-audio haswell-pcm-audio: fw image intel/IntcPP01.bin not available(-2)
> 	haswell-pcm-audio haswell-pcm-audio: FW loaded, mailbox readback FW info: type 01, - version: 00.00, build 77, source commit id: 876ac6906f31a43b6772b23c7c983ce9dcb18a19
> 	rt286 i2c-INT343A:00: ASoC: sink widget DMIC1 overwritten
> 	rt286 i2c-INT343A:00: ASoC: source widget DMIC1 overwritten
> 	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> System Pin mapping ok
> 	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Offload0 Pin mapping ok
> 	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Offload1 Pin mapping ok
> 	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Loopback Pin mapping ok
> 	broadwell-audio broadwell-audio: rt286-aif1 <-> snd-soc-dummy-dai mapping ok
> 	input: broadwell-rt286 Headset as /devices/pci0000:00/INT3438:00/broadwell-audio/sound/card1/input18
> 	...
> 	ALSA device list:
> 	  #0: HDA Intel HDMI at 0xf7218000 irq 48
> 	  #1: DellInc.-XPS139343--0TM99H
> 	...
> !!!	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to reset, ignore it.
> !!!	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to free, ignore it.
> 
> (these last two messages already are printed a couple of time after boot, and then
> again during a suspend/resume cycle. On v.5.5.y, there are similar messages
> "no context buffer need to restore!"). Everything is built-in, no modules
> are loaded.
> 
> Unfortunately, I cannot bisect this issue easily -- i915 was broken for
> quite some time on this system[*], prohibiting boot...
> 
> Thanks for taking a look at this issue!
> 
> 	Dominik
> 
> 
> [*] https://gitlab.freedesktop.org/drm/intel/issues/1151
> 

Hi Dominik,

Thanks for reporting, yes, this is Keyon(Yang Jie) here, we do have 
several other Broadwell comercial platforms in the market, adding Curtis 
from Google in case they may hit similar issue once upgrading to the 
latest kernel.

 From your description above, it looks the ucm and pulseaudio is 
involved, can you help do a quick try if using aplay directly will hit 
the same issue?

Thanks,
~Keyon

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Wed, Mar 18, 2020 at 06:49:56PM +0800, Keyon Jie wrote:
> On 3/18/20 2:30 PM, Dominik Brodowski wrote:
> > Hi!
> > 
> > While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
> > some sound-related trouble: after boot, the sound works fine -- but once I
> > suspend and resume my broadwell-based XPS13, I need to switch to headphone
> > and back to speaker to hear something. But what I hear isn't music but
> > garbled output.
> > 
> > A few dmesg snippets from v5.6-rc6-9-gac309e7744be which might be of
> > interest. I've highlighted the lines differing from v.5.5.x which might be
> > of special interest:
> > 
> > 	...
> > 	snd_hda_intel 0000:00:03.0: enabling device (0000 -> 0002)
> > 	usbcore: registered new interface driver snd-usb-audio
> > 	snd_hda_intel 0000:00:03.0: bound 0000:00:02.0 (ops i915_audio_component_bind_ops)
> > 	input: HDA Intel HDMI HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:03.0/sound/card0/input13
> > 	input: HDA Intel HDMI HDMI/DP,pcm=7 as /devices/pci0000:00/0000:00:03.0/sound/card0/input14
> > 	input: HDA Intel HDMI HDMI/DP,pcm=8 as /devices/pci0000:00/0000:00:03.0/sound/card0/input15
> > 	input: HDA Intel HDMI HDMI/DP,pcm=9 as /devices/pci0000:00/0000:00:03.0/sound/card0/input16
> > 	input: HDA Intel HDMI HDMI/DP,pcm=10 as /devices/pci0000:00/0000:00:03.0/sound/card0/input17
> > 	Console: switching to colour frame buffer device 240x67
> > !!!	sst-acpi INT3438:00: WARN: Device release is not defined so it is not safe to unbind this driver while in use
> > 	i915 0000:00:02.0: fb0: i915drmfb frame buffer device
> > 	sst-acpi INT3438:00: DesignWare DMA Controller, 8 channels
> > 	psmouse serio1: synaptics: Unable to query device: -5
> > 	haswell-pcm-audio haswell-pcm-audio: Direct firmware load for intel/IntcPP01.bin failed with error -2
> > 	haswell-pcm-audio haswell-pcm-audio: fw image intel/IntcPP01.bin not available(-2)
> > 	haswell-pcm-audio haswell-pcm-audio: FW loaded, mailbox readback FW info: type 01, - version: 00.00, build 77, source commit id: 876ac6906f31a43b6772b23c7c983ce9dcb18a19
> > 	rt286 i2c-INT343A:00: ASoC: sink widget DMIC1 overwritten
> > 	rt286 i2c-INT343A:00: ASoC: source widget DMIC1 overwritten
> > 	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> System Pin mapping ok
> > 	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Offload0 Pin mapping ok
> > 	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Offload1 Pin mapping ok
> > 	broadwell-audio broadwell-audio: snd-soc-dummy-dai <-> Loopback Pin mapping ok
> > 	broadwell-audio broadwell-audio: rt286-aif1 <-> snd-soc-dummy-dai mapping ok
> > 	input: broadwell-rt286 Headset as /devices/pci0000:00/INT3438:00/broadwell-audio/sound/card1/input18
> > 	...
> > 	ALSA device list:
> > 	  #0: HDA Intel HDMI at 0xf7218000 irq 48
> > 	  #1: DellInc.-XPS139343--0TM99H
> > 	...
> > !!!	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to reset, ignore it.
> > !!!	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to free, ignore it.
> > 
> > (these last two messages already are printed a couple of time after boot, and then
> > again during a suspend/resume cycle. On v.5.5.y, there are similar messages
> > "no context buffer need to restore!"). Everything is built-in, no modules
> > are loaded.
> > 
> > Unfortunately, I cannot bisect this issue easily -- i915 was broken for
> > quite some time on this system[*], prohibiting boot...
> > 
> > Thanks for taking a look at this issue!
> > 
> > 	Dominik
> > 
> > 
> > [*] https://gitlab.freedesktop.org/drm/intel/issues/1151
> > 
> 
> Hi Dominik,
> 
> Thanks for reporting, yes, this is Keyon(Yang Jie) here, we do have several
> other Broadwell comercial platforms in the market, adding Curtis from Google
> in case they may hit similar issue once upgrading to the latest kernel.

Thanks!

> From your description above, it looks the ucm and pulseaudio is involved,
> can you help do a quick try if using aplay directly will hit the same issue?

$ cat nice-music.wav | aplay -f S16_LE -r 44100 -c 2 --device="sysdefault:CARD=broadwellrt286"

runs nicely until I suspend. Upon resume, I hear nothing, with a number of

	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to reset, ignore it.

messages appearing in dmesg. Strangely, pavucontrol then (after resume)
cannot find any suitable audio device any more.

Hope this helps,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Pierre-Louis Bossart]

>>> While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
>>> some sound-related trouble: after boot, the sound works fine -- but once I
>>> suspend and resume my broadwell-based XPS13, I need to switch to headphone
>>> and back to speaker to hear something. But what I hear isn't music but
>>> garbled output.

It's my understanding that the use of the haswell driver is opt-in for 
Dell XPS13 9343. When we run the SOF driver on this device, we have to 
explicitly bypass an ACPI quirk that forces HDAudio to be used:

https://github.com/thesofproject/linux/commit/944b6a2d620a556424ed4195c8428485fcb6c2bd

Have you tried to run in plain vanilla HDAudio mode?

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Wed, Mar 18, 2020 at 10:13:54AM -0500, Pierre-Louis Bossart wrote:
> 
> 
> > > > While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
> > > > some sound-related trouble: after boot, the sound works fine -- but once I
> > > > suspend and resume my broadwell-based XPS13, I need to switch to headphone
> > > > and back to speaker to hear something. But what I hear isn't music but
> > > > garbled output.
> 
> It's my understanding that the use of the haswell driver is opt-in for Dell
> XPS13 9343. When we run the SOF driver on this device, we have to explicitly
> bypass an ACPI quirk that forces HDAudio to be used:
> 
> https://github.com/thesofproject/linux/commit/944b6a2d620a556424ed4195c8428485fcb6c2bd
> 
> Have you tried to run in plain vanilla HDAudio mode?

I had (see 18d78b64fddc), but not any more in years (and I'd like to keep
using I2S, which has worked flawlessly in these years).

Thanks,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Pierre-Louis Bossart]

On 3/18/20 11:20 AM, Dominik Brodowski wrote:
> On Wed, Mar 18, 2020 at 10:13:54AM -0500, Pierre-Louis Bossart wrote:
>>
>>
>>>>> While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
>>>>> some sound-related trouble: after boot, the sound works fine -- but once I
>>>>> suspend and resume my broadwell-based XPS13, I need to switch to headphone
>>>>> and back to speaker to hear something. But what I hear isn't music but
>>>>> garbled output.
>>
>> It's my understanding that the use of the haswell driver is opt-in for Dell
>> XPS13 9343. When we run the SOF driver on this device, we have to explicitly
>> bypass an ACPI quirk that forces HDAudio to be used:
>>
>> https://github.com/thesofproject/linux/commit/944b6a2d620a556424ed4195c8428485fcb6c2bd
>>
>> Have you tried to run in plain vanilla HDAudio mode?
> 
> I had (see 18d78b64fddc), but not any more in years (and I'd like to keep
> using I2S, which has worked flawlessly in these years).

ok. I don't think Intel folks have this device available, or it's used 
for other things, but if you want to bisect on you may want to use [1] 
to solve DRM issues. I used it to make Broadwell/Samus work again with SOF.

[1] 
https://gitlab.freedesktop.org/drm/intel/uploads/ef10c6c27fdc53d114f827bb72b078aa/0001-drm-i915-psr-Force-PSR-probe-only-after-full-initial.patch.txt

An alternate path would be to switch to SOF. It's still viewed as a 
developer option but Broadwell/Samus work reliably for me and we have a 
Broadwell-rt286 platform used for CI.

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Wed, Mar 18, 2020 at 12:08:24PM -0500, Pierre-Louis Bossart wrote:
> On 3/18/20 11:20 AM, Dominik Brodowski wrote:
> > On Wed, Mar 18, 2020 at 10:13:54AM -0500, Pierre-Louis Bossart wrote:
> > > 
> > > 
> > > > > > While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
> > > > > > some sound-related trouble: after boot, the sound works fine -- but once I
> > > > > > suspend and resume my broadwell-based XPS13, I need to switch to headphone
> > > > > > and back to speaker to hear something. But what I hear isn't music but
> > > > > > garbled output.
> > > 
> > > It's my understanding that the use of the haswell driver is opt-in for Dell
> > > XPS13 9343. When we run the SOF driver on this device, we have to explicitly
> > > bypass an ACPI quirk that forces HDAudio to be used:
> > > 
> > > https://github.com/thesofproject/linux/commit/944b6a2d620a556424ed4195c8428485fcb6c2bd
> > > 
> > > Have you tried to run in plain vanilla HDAudio mode?
> > 
> > I had (see 18d78b64fddc), but not any more in years (and I'd like to keep
> > using I2S, which has worked flawlessly in these years).
> 
> ok. I don't think Intel folks have this device available, or it's used for
> other things, but if you want to bisect on you may want to use [1] to solve
> DRM issues. I used it to make Broadwell/Samus work again with SOF.
> 
> [1] https://gitlab.freedesktop.org/drm/intel/uploads/ef10c6c27fdc53d114f827bb72b078aa/0001-drm-i915-psr-Force-PSR-probe-only-after-full-initial.patch.txt
> 
> An alternate path would be to switch to SOF. It's still viewed as a
> developer option but Broadwell/Samus work reliably for me and we have a
> Broadwell-rt286 platform used for CI.

What do you mean with SOF? And no other ideas on the root cause than a
tedious bisect?

Thanks,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Pierre-Louis Bossart]

On 3/18/20 12:19 PM, Dominik Brodowski wrote:
> On Wed, Mar 18, 2020 at 12:08:24PM -0500, Pierre-Louis Bossart wrote:
>> On 3/18/20 11:20 AM, Dominik Brodowski wrote:
>>> On Wed, Mar 18, 2020 at 10:13:54AM -0500, Pierre-Louis Bossart wrote:
>>>>
>>>>
>>>>>>> While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
>>>>>>> some sound-related trouble: after boot, the sound works fine -- but once I
>>>>>>> suspend and resume my broadwell-based XPS13, I need to switch to headphone
>>>>>>> and back to speaker to hear something. But what I hear isn't music but
>>>>>>> garbled output.
>>>>
>>>> It's my understanding that the use of the haswell driver is opt-in for Dell
>>>> XPS13 9343. When we run the SOF driver on this device, we have to explicitly
>>>> bypass an ACPI quirk that forces HDAudio to be used:
>>>>
>>>> https://github.com/thesofproject/linux/commit/944b6a2d620a556424ed4195c8428485fcb6c2bd
>>>>
>>>> Have you tried to run in plain vanilla HDAudio mode?
>>>
>>> I had (see 18d78b64fddc), but not any more in years (and I'd like to keep
>>> using I2S, which has worked flawlessly in these years).
>>
>> ok. I don't think Intel folks have this device available, or it's used for
>> other things, but if you want to bisect on you may want to use [1] to solve
>> DRM issues. I used it to make Broadwell/Samus work again with SOF.
>>
>> [1] https://gitlab.freedesktop.org/drm/intel/uploads/ef10c6c27fdc53d114f827bb72b078aa/0001-drm-i915-psr-Force-PSR-probe-only-after-full-initial.patch.txt
>>
>> An alternate path would be to switch to SOF. It's still viewed as a
>> developer option but Broadwell/Samus work reliably for me and we have a
>> Broadwell-rt286 platform used for CI.
> 
> What do you mean with SOF? And no other ideas on the root cause than a
> tedious bisect?

Sound Open Firmware (SOF) [1]. You can build your own audio firmware for 
Broadwell and the driver is supported in the mainline (you'd need to 
disable the legacy driver [2]).

I can't think of any changes to that haswell/broadwell legacy driver, 
apart from Takashi's buffer management changes 3f93b1ed4ac1e2.

But there were multiple changes to the ASoC core, so it's possible that 
they impact suspend/resume. Just a blind guess.

[1] https://thesofproject.github.io/latest/index.html
[2] 
https://github.com/thesofproject/kconfig/blob/023cc25cd0b26a9757592d0bbbbe719825dd728d/sof-dev-defconfig#L20

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-18 18:29, Pierre-Louis Bossart wrote:
> 
> 
> On 3/18/20 12:19 PM, Dominik Brodowski wrote:
>> On Wed, Mar 18, 2020 at 12:08:24PM -0500, Pierre-Louis Bossart wrote:
>>> On 3/18/20 11:20 AM, Dominik Brodowski wrote:
>>>> On Wed, Mar 18, 2020 at 10:13:54AM -0500, Pierre-Louis Bossart wrote:
>>>>>
>>>>>
>>>>>>>> While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) 
>>>>>>>> causes me
>>>>>>>> some sound-related trouble: after boot, the sound works fine -- 
>>>>>>>> but once I
>>>>>>>> suspend and resume my broadwell-based XPS13, I need to switch to 
>>>>>>>> headphone
>>>>>>>> and back to speaker to hear something. But what I hear isn't 
>>>>>>>> music but
>>>>>>>> garbled output.
>>>>>
>>>>> It's my understanding that the use of the haswell driver is opt-in 
>>>>> for Dell
>>>>> XPS13 9343. When we run the SOF driver on this device, we have to 
>>>>> explicitly
>>>>> bypass an ACPI quirk that forces HDAudio to be used:
>>>>>
>>>>> https://github.com/thesofproject/linux/commit/944b6a2d620a556424ed4195c8428485fcb6c2bd 
>>>>>
>>>>>
>>>>> Have you tried to run in plain vanilla HDAudio mode?
>>>>
>>>> I had (see 18d78b64fddc), but not any more in years (and I'd like to 
>>>> keep
>>>> using I2S, which has worked flawlessly in these years).
>>>
>>> ok. I don't think Intel folks have this device available, or it's 
>>> used for
>>> other things, but if you want to bisect on you may want to use [1] to 
>>> solve
>>> DRM issues. I used it to make Broadwell/Samus work again with SOF.
>>>
>>> [1] 
>>> https://gitlab.freedesktop.org/drm/intel/uploads/ef10c6c27fdc53d114f827bb72b078aa/0001-drm-i915-psr-Force-PSR-probe-only-after-full-initial.patch.txt 
>>>
>>>
>>> An alternate path would be to switch to SOF. It's still viewed as a
>>> developer option but Broadwell/Samus work reliably for me and we have a
>>> Broadwell-rt286 platform used for CI.
>>
>> What do you mean with SOF? And no other ideas on the root cause than a
>> tedious bisect?
> 
> Sound Open Firmware (SOF) [1]. You can build your own audio firmware for 
> Broadwell and the driver is supported in the mainline (you'd need to 
> disable the legacy driver [2]).
> 
> I can't think of any changes to that haswell/broadwell legacy driver, 
> apart from Takashi's buffer management changes 3f93b1ed4ac1e2.
> 
> But there were multiple changes to the ASoC core, so it's possible that 
> they impact suspend/resume. Just a blind guess.
> 
> [1] https://thesofproject.github.io/latest/index.html
> [2] 
> https://github.com/thesofproject/kconfig/blob/023cc25cd0b26a9757592d0bbbbe719825dd728d/sof-dev-defconfig#L20 
> 
> 

Please refrain from advertising SOF as a solution for this problem.

There were plenty changes done to dmaengine and ASoC during that time. 
Right now we are digging through the code.

While we might not have Dell XPS 13 available currently on our site, 
issue reproduces on our RVPs.

Regards,
Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-18 17:20, Dominik Brodowski wrote:
> On Wed, Mar 18, 2020 at 10:13:54AM -0500, Pierre-Louis Bossart wrote:
>>>>> While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
>>>>> some sound-related trouble: after boot, the sound works fine -- but once I
>>>>> suspend and resume my broadwell-based XPS13, I need to switch to headphone
>>>>> and back to speaker to hear something. But what I hear isn't music but
>>>>> garbled output.

> 
> I had (see 18d78b64fddc), but not any more in years (and I'd like to keep
> using I2S, which has worked flawlessly in these years).
> 

Due to pandemic I'm working remotely and right now won't be able to test 
audio quality so focusing on the stream==NULL issue. And thus we got to 
help each other out : )

Could you verify issue reproduces on 5.6.0-rc1 on your machine? On my 
RVPs looks like it does. There is one more thing that worries me. After 
enabling dbg logs I see some IPCs queried but not delivered (dsp busy):

[  170.330009] snd_soc_core:dpcm_fe_dai_prepare:  System PCM: ASoC: 
prepare FE System PCM
[  170.330019] snd_soc_core:dpcm_be_dai_prepare:  Codec: ASoC: prepare 
BE Codec
[  170.347068] snd_soc_core:dpcm_dapm_stream_event:  Codec: ASoC: BE 
Codec event 1 dir 0
[  170.348814] snd_soc_core:dpcm_do_trigger:  Codec: ASoC: trigger BE 
Codec cmd 1
[  170.348826] snd_soc_core:dpcm_dai_trigger_fe_be:  System PCM: ASoC: 
post trigger FE System PCM cmd 1
[  170.348839] snd_soc_sst_ipc:ipc_tx_msgs: haswell-pcm-audio 
haswell-pcm-audio: ipc_tx_msgs dsp busy
[  182.583710]  System PCM: ASoC: trigger FE cmd: 7 failed: -22
[  182.583811] snd_soc_core:dpcm_dai_trigger_fe_be:  System PCM: ASoC: 
pre trigger FE System PCM cmd 0
[  182.583839] snd_soc_core:dpcm_do_trigger:  Codec: ASoC: trigger BE 
Codec cmd 0
[  182.583862] snd_soc_core:dpcm_fe_dai_hw_free:  System PCM: ASoC: 
hw_free FE System PCM
[  182.583872] snd_soc_core:dpcm_be_dai_hw_free:  Codec: ASoC: hw_free 
BE Codec
[  182.584127] snd_soc_core:dpcm_fe_dai_hw_free:  System PCM: ASoC: 
hw_free FE System PCM
[  182.584144] snd_soc_core:dpcm_be_dai_hw_free:  Codec: ASoC: hw_free 
BE Codec
[  182.584161] snd_soc_core:dpcm_be_dai_shutdown:  Codec: ASoC: close BE 
Codec
[  182.584211] snd_soc_sst_ipc:ipc_tx_msgs: haswell-pcm-audio 
haswell-pcm-audio: ipc_tx_msgs dsp busy
[  182.587411] snd_soc_core:dpcm_fe_dai_shutdown:  System PCM: ASoC: 
close FE System PCM
[  182.587427] haswell-pcm-audio haswell-pcm-audio: warning: stream is 
NULL, no stream to reset, ignore it.
[  182.587435] haswell-pcm-audio haswell-pcm-audio: warning: stream is 
NULL, no stream to free, ignore it.
[  182.587451] snd_soc_core:dpcm_be_disconnect:  System PCM: ASoC: BE 
playback disconnect check for Codec
[  182.587460] snd_soc_core:dpcm_be_disconnect:  System PCM: freed DSP 
playback path System PCM -> Codec
[  187.626116] snd_soc_core:snd_soc_close_delayed_work:  System PCM: 
ASoC: pop wq checking: Playback status: inactive waiting: yes

Will be scanning IPCs now. Seems like regression has been introduced 
immediately in 5.6.0-rc1 as linux-stable 5.5.7 works just fine for me.

Regards,
Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Ross Zwisler]

On Wed, Mar 18, 2020 at 10:25:32AM -0700, Curtis Malainey wrote:
> +Ross Zwisler <zwisler@google.com>
> Do we have any BDW boards that use the rt286 codec? I can't recall any.
> Also I never saw this issue, did you?

No, I'm not aware of any Chrome OS BDW boards that use rt286.
Samus uses rt5677.

I haven't seen this issue, no.

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Wed, Mar 18, 2020 at 07:27:58PM +0100, Cezary Rojewski wrote:
> On 2020-03-18 17:20, Dominik Brodowski wrote:
> > On Wed, Mar 18, 2020 at 10:13:54AM -0500, Pierre-Louis Bossart wrote:
> > > > > > While 5.5.x works fine, mainline as of ac309e7744be (v5.6-rc6+) causes me
> > > > > > some sound-related trouble: after boot, the sound works fine -- but once I
> > > > > > suspend and resume my broadwell-based XPS13, I need to switch to headphone
> > > > > > and back to speaker to hear something. But what I hear isn't music but
> > > > > > garbled output.
> 
> > 
> > I had (see 18d78b64fddc), but not any more in years (and I'd like to keep
> > using I2S, which has worked flawlessly in these years).
> > 
> 
> Due to pandemic I'm working remotely and right now won't be able to test
> audio quality so focusing on the stream==NULL issue. And thus we got to help
> each other out : )

Sure, and thanks for taking a look at this!

> Could you verify issue reproduces on 5.6.0-rc1 on your machine?

It reproduces on 5.6.0-rc1 + i915-bugfix. I'm trying to bisect it further in
the background, but that may take quite some time.

Thanks,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-18 20:22, Dominik Brodowski wrote:
> On Wed, Mar 18, 2020 at 07:27:58PM +0100, Cezary Rojewski wrote:

>>
>> Due to pandemic I'm working remotely and right now won't be able to test
>> audio quality so focusing on the stream==NULL issue. And thus we got to help
>> each other out : )
> 
> Sure, and thanks for taking a look at this!
> 
>> Could you verify issue reproduces on 5.6.0-rc1 on your machine?
> 
> It reproduces on 5.6.0-rc1 + i915-bugfix. I'm trying to bisect it further in
> the background, but that may take quite some time.
> 

Could you checkout v5.6-rc1 with following commit reverted:
	ASoC: Intel: broadwell: change cpu_dai and platform components for SOF

For my working v5.6-rc1 commit id is: 
64df6afa0dab5eda95cc4cc2269e3d4e83b6b6ce.

Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Wed, Mar 18, 2020 at 09:43:54PM +0100, Cezary Rojewski wrote:
> On 2020-03-18 20:22, Dominik Brodowski wrote:
> > On Wed, Mar 18, 2020 at 07:27:58PM +0100, Cezary Rojewski wrote:
> 
> > > 
> > > Due to pandemic I'm working remotely and right now won't be able to test
> > > audio quality so focusing on the stream==NULL issue. And thus we got to help
> > > each other out : )
> > 
> > Sure, and thanks for taking a look at this!
> > 
> > > Could you verify issue reproduces on 5.6.0-rc1 on your machine?
> > 
> > It reproduces on 5.6.0-rc1 + i915-bugfix. I'm trying to bisect it further in
> > the background, but that may take quite some time.
> > 
> 
> Could you checkout v5.6-rc1 with following commit reverted:
> 	ASoC: Intel: broadwell: change cpu_dai and platform components for SOF
> 
> For my working v5.6-rc1 commit id is:
> 64df6afa0dab5eda95cc4cc2269e3d4e83b6b6ce.

Hm, no joy -- after suspend/resume, no sound at first, and if I twiggle some
options with pulseaudio, I get garbled output (even when using

	aplay -f S16_LE -r 44100 -c 2 --device="sysdefault:CARD=broadwellrt286"

). Will try to bisect further the next days.

Thanks,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-18 22:52, Dominik Brodowski wrote:
> On Wed, Mar 18, 2020 at 09:43:54PM +0100, Cezary Rojewski wrote:
>> On 2020-03-18 20:22, Dominik Brodowski wrote:
>>> On Wed, Mar 18, 2020 at 07:27:58PM +0100, Cezary Rojewski wrote:
>>
>>>>
>>>> Due to pandemic I'm working remotely and right now won't be able to test
>>>> audio quality so focusing on the stream==NULL issue. And thus we got to help
>>>> each other out : )
>>>
>>> Sure, and thanks for taking a look at this!
>>>
>>>> Could you verify issue reproduces on 5.6.0-rc1 on your machine?
>>>
>>> It reproduces on 5.6.0-rc1 + i915-bugfix. I'm trying to bisect it further in
>>> the background, but that may take quite some time.
>>>
>>
>> Could you checkout v5.6-rc1 with following commit reverted:
>> 	ASoC: Intel: broadwell: change cpu_dai and platform components for SOF
>>
>> For my working v5.6-rc1 commit id is:
>> 64df6afa0dab5eda95cc4cc2269e3d4e83b6b6ce.
> 
> Hm, no joy -- after suspend/resume, no sound at first, and if I twiggle some
> options with pulseaudio, I get garbled output (even when using
> 
> 	aplay -f S16_LE -r 44100 -c 2 --device="sysdefault:CARD=broadwellrt286"
> 
> ). Will try to bisect further the next days.
> 

Thanks for quick reply. Revert of said commit fixes stream==NULL issue 
for me. See if there were any changes in dmesg.
Will ask technicians to assist me on site tomorrow.

Regards,
Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Wed, Mar 18, 2020 at 11:20:55PM +0100, Cezary Rojewski wrote:
> On 2020-03-18 22:52, Dominik Brodowski wrote:
> > On Wed, Mar 18, 2020 at 09:43:54PM +0100, Cezary Rojewski wrote:
> > > On 2020-03-18 20:22, Dominik Brodowski wrote:
> > > > On Wed, Mar 18, 2020 at 07:27:58PM +0100, Cezary Rojewski wrote:
> > > 
> > > > > 
> > > > > Due to pandemic I'm working remotely and right now won't be able to test
> > > > > audio quality so focusing on the stream==NULL issue. And thus we got to help
> > > > > each other out : )
> > > > 
> > > > Sure, and thanks for taking a look at this!
> > > > 
> > > > > Could you verify issue reproduces on 5.6.0-rc1 on your machine?
> > > > 
> > > > It reproduces on 5.6.0-rc1 + i915-bugfix. I'm trying to bisect it further in
> > > > the background, but that may take quite some time.
> > > > 
> > > 
> > > Could you checkout v5.6-rc1 with following commit reverted:
> > > 	ASoC: Intel: broadwell: change cpu_dai and platform components for SOF
> > > 
> > > For my working v5.6-rc1 commit id is:
> > > 64df6afa0dab5eda95cc4cc2269e3d4e83b6b6ce.
> > 
> > Hm, no joy -- after suspend/resume, no sound at first, and if I twiggle some
> > options with pulseaudio, I get garbled output (even when using
> > 
> > 	aplay -f S16_LE -r 44100 -c 2 --device="sysdefault:CARD=broadwellrt286"
> > 
> > ). Will try to bisect further the next days.
> > 
> 
> Thanks for quick reply. Revert of said commit fixes stream==NULL issue for
> me. See if there were any changes in dmesg.
> Will ask technicians to assist me on site tomorrow.

Have some good news now, namely that a bisect is complete: That pointed to
1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
therefore I've added Kuninori Morimoto to this e-mail thread.

Additionally, I have tested mainline (v5.6-rc6+ as of 5076190daded) with
*both* 64df6afa0dab (which you suggested yesterday) and 1272063a7ee4
reverted. And that works like a charm as well.

Hope this helps!

Thanks,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-19 14:00, Dominik Brodowski wrote:
> On Wed, Mar 18, 2020 at 11:20:55PM +0100, Cezary Rojewski wrote:

>>
>> Thanks for quick reply. Revert of said commit fixes stream==NULL issue for
>> me. See if there were any changes in dmesg.
>> Will ask technicians to assist me on site tomorrow.
> 
> Have some good news now, namely that a bisect is complete: That pointed to
> 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
> therefore I've added Kuninori Morimoto to this e-mail thread.
> 
> Additionally, I have tested mainline (v5.6-rc6+ as of 5076190daded) with
> *both* 64df6afa0dab (which you suggested yesterday) and 1272063a7ee4
> reverted. And that works like a charm as well.
> 
> Hope this helps!
> 
> Thanks,
> 	Dominik
> 

To make everyone not miss a bit - I believe we had 2 issues here, even 
though that one may seem harmless from user perspective:

 From IPC logs indeed it looks like a redundant (additional) stream 
initialization has occurred - said redundant stream is destroyed right 
after it has been created, and only to be recreated yet again.. Can 
share the logs if required.

While hw_params() handled doubled init nicely, _reset and _free
did not (during on pcm_close()) -> secondary invokes attempted to RESET
and FREE stream despite it being destroyed long ago. With revert of
patch I had mentioned, no lines:

!!!	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no
stream to reset, ignore it.
!!!	haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no
stream to free, ignore it.

should appear.

I'll focus now on the commits you found offending during your bisect. 
Thank you Dominik!

Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

On Thu, Mar 19, 2020 at 02:00:49PM +0100, Dominik Brodowski wrote:

> Have some good news now, namely that a bisect is complete: That pointed to
> 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
> therefore I've added Kuninori Morimoto to this e-mail thread.

If that's an issue it feels more like a driver bug in that if the driver
asked for ignore_suspend then it should expect not to have the suspend
callback called.

> Additionally, I have tested mainline (v5.6-rc6+ as of 5076190daded) with
> *both* 64df6afa0dab (which you suggested yesterday) and 1272063a7ee4
> reverted. And that works like a charm as well.

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-19 14:41, Mark Brown wrote:
> On Thu, Mar 19, 2020 at 02:00:49PM +0100, Dominik Brodowski wrote:
> 
>> Have some good news now, namely that a bisect is complete: That pointed to
>> 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
>> therefore I've added Kuninori Morimoto to this e-mail thread.
> 
> If that's an issue it feels more like a driver bug in that if the driver
> asked for ignore_suspend then it should expect not to have the suspend
> callback called.
> 

Requested for tests with following diff applied:

diff --git a/sound/soc/intel/boards/broadwell.c 
b/sound/soc/intel/boards/broadwell.c
index db7e1e87156d..6ed4c1b0a515 100644
--- a/sound/soc/intel/boards/broadwell.c
+++ b/sound/soc/intel/boards/broadwell.c
@@ -212,7 +212,6 @@ static struct snd_soc_dai_link 
broadwell_rt286_dais[] = {
                 .init = broadwell_rt286_codec_init,
                 .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
                         SND_SOC_DAIFMT_CBS_CFS,
-               .ignore_suspend = 1,
                 .ignore_pmdown_time = 1,
                 .be_hw_params_fixup = broadwell_ssp0_fixup,
                 .ops = &broadwell_rt286_ops,

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Thu, Mar 19, 2020 at 04:48:03PM +0100, Cezary Rojewski wrote:
> On 2020-03-19 14:41, Mark Brown wrote:
> > On Thu, Mar 19, 2020 at 02:00:49PM +0100, Dominik Brodowski wrote:
> > 
> > > Have some good news now, namely that a bisect is complete: That pointed to
> > > 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
> > > therefore I've added Kuninori Morimoto to this e-mail thread.
> > 
> > If that's an issue it feels more like a driver bug in that if the driver
> > asked for ignore_suspend then it should expect not to have the suspend
> > callback called.
> > 
> 
> Requested for tests with following diff applied:
> 
> diff --git a/sound/soc/intel/boards/broadwell.c
> b/sound/soc/intel/boards/broadwell.c
> index db7e1e87156d..6ed4c1b0a515 100644
> --- a/sound/soc/intel/boards/broadwell.c
> +++ b/sound/soc/intel/boards/broadwell.c
> @@ -212,7 +212,6 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] =
> {
>                 .init = broadwell_rt286_codec_init,
>                 .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
>                         SND_SOC_DAIFMT_CBS_CFS,
> -               .ignore_suspend = 1,
>                 .ignore_pmdown_time = 1,
>                 .be_hw_params_fixup = broadwell_ssp0_fixup,
>                 .ops = &broadwell_rt286_ops,

That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
("ASoC: Intel: broadwell: change cpu_dai and platform components for SOF")
on top of that. But you can assess better whether that patch needs care for
other reasons; for me, this one-liner you have suggested is perfect.

Many thanks -- it's been a pleasure to work with you on tracking this issue
down.

	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Pierre-Louis Bossart]

On 3/19/20 11:51 AM, Dominik Brodowski wrote:
> On Thu, Mar 19, 2020 at 04:48:03PM +0100, Cezary Rojewski wrote:
>> On 2020-03-19 14:41, Mark Brown wrote:
>>> On Thu, Mar 19, 2020 at 02:00:49PM +0100, Dominik Brodowski wrote:
>>>
>>>> Have some good news now, namely that a bisect is complete: That pointed to
>>>> 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
>>>> therefore I've added Kuninori Morimoto to this e-mail thread.
>>>
>>> If that's an issue it feels more like a driver bug in that if the driver
>>> asked for ignore_suspend then it should expect not to have the suspend
>>> callback called.
>>>
>>
>> Requested for tests with following diff applied:
>>
>> diff --git a/sound/soc/intel/boards/broadwell.c
>> b/sound/soc/intel/boards/broadwell.c
>> index db7e1e87156d..6ed4c1b0a515 100644
>> --- a/sound/soc/intel/boards/broadwell.c
>> +++ b/sound/soc/intel/boards/broadwell.c
>> @@ -212,7 +212,6 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] =
>> {
>>                  .init = broadwell_rt286_codec_init,
>>                  .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
>>                          SND_SOC_DAIFMT_CBS_CFS,
>> -               .ignore_suspend = 1,
>>                  .ignore_pmdown_time = 1,
>>                  .be_hw_params_fixup = broadwell_ssp0_fixup,
>>                  .ops = &broadwell_rt286_ops,
> 
> That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
> ("ASoC: Intel: broadwell: change cpu_dai and platform components for SOF")
> on top of that. But you can assess better whether that patch needs care for
> other reasons; for me, this one-liner you have suggested is perfect.

.ignore_suspend is set for bdw-rt5677.c and bdw-rt5650.c as well. I 
don't know if that was intentional.

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-19 17:51, Dominik Brodowski wrote:
> On Thu, Mar 19, 2020 at 04:48:03PM +0100, Cezary Rojewski wrote:
>> On 2020-03-19 14:41, Mark Brown wrote:
>>> On Thu, Mar 19, 2020 at 02:00:49PM +0100, Dominik Brodowski wrote:
>>>
>>>> Have some good news now, namely that a bisect is complete: That pointed to
>>>> 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
>>>> therefore I've added Kuninori Morimoto to this e-mail thread.
>>>
>>> If that's an issue it feels more like a driver bug in that if the driver
>>> asked for ignore_suspend then it should expect not to have the suspend
>>> callback called.
>>>
>>
>> Requested for tests with following diff applied:
>>
>> diff --git a/sound/soc/intel/boards/broadwell.c
>> b/sound/soc/intel/boards/broadwell.c
>> index db7e1e87156d..6ed4c1b0a515 100644
>> --- a/sound/soc/intel/boards/broadwell.c
>> +++ b/sound/soc/intel/boards/broadwell.c
>> @@ -212,7 +212,6 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] =
>> {
>>                  .init = broadwell_rt286_codec_init,
>>                  .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
>>                          SND_SOC_DAIFMT_CBS_CFS,
>> -               .ignore_suspend = 1,
>>                  .ignore_pmdown_time = 1,
>>                  .be_hw_params_fixup = broadwell_ssp0_fixup,
>>                  .ops = &broadwell_rt286_ops,
> 
> That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
> ("ASoC: Intel: broadwell: change cpu_dai and platform components for SOF")
> on top of that. But you can assess better whether that patch needs care for
> other reasons; for me, this one-liner you have suggested is perfect.
> 
> Many thanks -- it's been a pleasure to work with you on tracking this issue
> down.
> 
> 	Dominik
> 

Thank you for being so cooperative during this 2day debug session.

The patch I mentioned earlier unintentionally (?) changed 'platform' 
component param for ssp0_port from 'dummy' to 'platform' for non-SOF 
solution:

diff --git a/sound/soc/intel/boards/broadwell.c 
b/sound/soc/intel/boards/broadwell.c
index b9c12e24c70b..db7e1e87156d 100644
--- a/sound/soc/intel/boards/broadwell.c
+++ b/sound/soc/intel/boards/broadwell.c
@@ -164,14 +164,6 @@ SND_SOC_DAILINK_DEF(platform,
  SND_SOC_DAILINK_DEF(codec,
         DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));

-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL)
-SND_SOC_DAILINK_DEF(ssp0_port,
-           DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
-#else
-SND_SOC_DAILINK_DEF(ssp0_port,
-           DAILINK_COMP_ARRAY(COMP_DUMMY()));
-#endif
-
  /* broadwell digital audio interface glue - connects codec <--> CPU */
  static struct snd_soc_dai_link broadwell_rt286_dais[] = {
         /* Front End DAI links */
@@ -226,7 +218,7 @@ static struct snd_soc_dai_link 
broadwell_rt286_dais[] = {
                 .ops = &broadwell_rt286_ops,
                 .dpcm_playback = 1,
                 .dpcm_capture = 1,
-               SND_SOC_DAILINK_REG(ssp0_port, codec, platform),
+               SND_SOC_DAILINK_REG(dummy, codec, dummy),
         },


Said change causes following to occur:


(stream start)
[  113.251950] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000003000000 size: 77
[  113.252090] haswell-pcm-audio haswell-pcm-audio: > rx: 
0x0000000043000000 size: 48
[  113.252097] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006301000 size: 20
[  113.252147] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006301000 size: 20
[  113.252179] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006100000 size: 0
[  113.252219] snd_soc_core:dpcm_fe_dai_hw_params:  System PCM: ASoC: 
hw_params FE System PCM rate 48000 chan 2 fmt 2
[  113.252229] snd_soc_core:dapm_update_dai_unlocked: haswell-pcm-audio 
haswell-pcm-audio: Update DAI routes for System Pin playback
[  113.252236] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006000000 size: 0
[  113.252304] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000004000000 size: 4
[  113.252425] snd_soc_sst_haswell_pcm:create_adsp_page_table:  System 
PCM: generating page table for 00000000a8c2b8a6 size 0x17700 pages 24


(In essence these tx'es denote sequence for stream initialization while 
the last two for stream RESET (0x6000000) and FREE (0x4000000))

and that is only to recreate the stream once again:


[  113.252673] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000003000000 size: 77
[  113.252803] haswell-pcm-audio haswell-pcm-audio: > rx: 
0x0000000043000000 size: 48
[  113.252810] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006301000 size: 20
[  113.252864] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006301000 size: 20
[  113.252900] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006100000 size: 0
[  113.252987] snd_soc_core:dpcm_fe_dai_prepare:  System PCM: ASoC: 
prepare FE System PCM
[  113.252993] snd_soc_core:dpcm_be_dai_prepare:  Codec: ASoC: prepare 
BE Codec
[  113.253028] snd_soc_core:dpcm_dapm_stream_event:  Codec: ASoC: BE 
Codec event 1 dir 0
[  113.254962] snd_soc_core:dpcm_do_trigger:  Codec: ASoC: trigger BE 
Codec cmd 1


Because of that we ended up in _reset and _free being called twice:


[  113.254969] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006200000 size: 0
[  113.254980] snd_soc_core:dpcm_dai_trigger_fe_be:  System PCM: ASoC: 
post trigger FE System PCM cmd 1
[  113.254983] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006200000 size: 0
[  113.254996] snd_soc_sst_ipc:ipc_tx_msgs: haswell-pcm-audio 
haswell-pcm-audio: ipc_tx_msgs dsp busy
[  118.486291]  System PCM: ASoC: trigger FE cmd: 7 failed: -22
[  118.486431] snd_soc_core:dpcm_dai_trigger_fe_be:  System PCM: ASoC: 
pre trigger FE System PCM cmd 0
[  118.486464] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006100000 size: 0
[  118.486495] snd_soc_core:dpcm_do_trigger:  Codec: ASoC: trigger BE 
Codec cmd 0
[  118.486514] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006100000 size: 0
[  118.486550] snd_soc_core:dpcm_fe_dai_hw_free:  System PCM: ASoC: 
hw_free FE System PCM
[  118.486569] snd_soc_core:dpcm_be_dai_hw_free:  Codec: ASoC: hw_free 
BE Codec
[  118.486719] snd_soc_core:dpcm_fe_dai_hw_free:  System PCM: ASoC: 
hw_free FE System PCM
[  118.486734] snd_soc_core:dpcm_be_dai_hw_free:  Codec: ASoC: hw_free 
BE Codec
[  118.486751] snd_soc_core:dpcm_be_dai_shutdown:  Codec: ASoC: close BE 
Codec
[  118.486801] snd_soc_sst_ipc:ipc_tx_msgs: haswell-pcm-audio 
haswell-pcm-audio: ipc_tx_msgs dsp busy
[  118.489279] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006000000 size: 0
[  118.489382] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000004000000 size: 4
[  118.489535] snd_soc_core:dpcm_fe_dai_shutdown:  System PCM: ASoC: 
close FE System PCM
[  118.489547] haswell-pcm-audio haswell-pcm-audio: warning: stream is 
NULL, no stream to reset, ignore it.
[  118.489553] haswell-pcm-audio haswell-pcm-audio: warning: stream is 
NULL, no stream to free, ignore it.
[  118.489571] snd_soc_core:dpcm_be_disconnect:  System PCM: ASoC: BE 
playback disconnect check for Codec
[  118.489580] snd_soc_core:dpcm_be_disconnect:  System PCM: freed DSP 
playback path System PCM -> Codec


Could you confirm the same happens on your machine when revert of 
mentioned patch is not applied ("stream is NULL" messages occur)? Issue 
may be harmless but explained sequence does not look right.

Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

On Thu, Mar 19, 2020 at 12:21:47PM -0500, Pierre-Louis Bossart wrote:
> On 3/19/20 11:51 AM, Dominik Brodowski wrote:

> > That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
> > ("ASoC: Intel: broadwell: change cpu_dai and platform components for SOF")
> > on top of that. But you can assess better whether that patch needs care for
> > other reasons; for me, this one-liner you have suggested is perfect.

Good news!

> .ignore_suspend is set for bdw-rt5677.c and bdw-rt5650.c as well. I don't
> know if that was intentional.

The intended use case is for applications doing audio during suspend
like telephony audio between the modem and CODEC on a phone or
compressed audio playback.  I guess the compressed audio playback case
could possibly apply with these systems though x86 suspend/resume is
usually sufficiently heavyweight that it's surprising.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-19 18:21, Pierre-Louis Bossart wrote:
> On 3/19/20 11:51 AM, Dominik Brodowski wrote:
>> On Thu, Mar 19, 2020 at 04:48:03PM +0100, Cezary Rojewski wrote:

>>>
>>> Requested for tests with following diff applied:
>>>
>>> diff --git a/sound/soc/intel/boards/broadwell.c
>>> b/sound/soc/intel/boards/broadwell.c
>>> index db7e1e87156d..6ed4c1b0a515 100644
>>> --- a/sound/soc/intel/boards/broadwell.c
>>> +++ b/sound/soc/intel/boards/broadwell.c
>>> @@ -212,7 +212,6 @@ static struct snd_soc_dai_link 
>>> broadwell_rt286_dais[] =
>>> {
>>>                  .init = broadwell_rt286_codec_init,
>>>                  .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
>>>                          SND_SOC_DAIFMT_CBS_CFS,
>>> -               .ignore_suspend = 1,
>>>                  .ignore_pmdown_time = 1,
>>>                  .be_hw_params_fixup = broadwell_ssp0_fixup,
>>>                  .ops = &broadwell_rt286_ops,
>>
>> That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
>> ("ASoC: Intel: broadwell: change cpu_dai and platform components for 
>> SOF")
>> on top of that. But you can assess better whether that patch needs 
>> care for
>> other reasons; for me, this one-liner you have suggested is perfect.
> 
> .ignore_suspend is set for bdw-rt5677.c and bdw-rt5650.c as well. I 
> don't know if that was intentional.

haswell has it too.

My guess is that it's supposed to mimic offload behaviour on Windows: 
offload pin playback allows for non-interrupted playback during sleep 
while system pin follows standard path: breaks on sleep and resumes once 
sleep concludes. This of course also involves cooperation from 
application side.

However, one pin cannot serve two masters. Either it's offload or it's not.

This is just a guess of course, and my vision might be clouded becuase 
of Windows background.
Other SSP0 examples: rt286 (SKL/ KBL) rt298 (APL) and rt274 (CNL) do not 
have .ignore_suspend enabled for their links, except when DMIC is 
involved. So it might be just a bug that has been covered by another bug 
present in ASoC core, which Morimoto' San fixed during his cleanup series.

Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-19 18:33, Cezary Rojewski wrote:
> 
> Thank you for being so cooperative during this 2day debug session.
> 
> The patch I mentioned earlier unintentionally (?) changed 'platform' 
> component param for ssp0_port from 'dummy' to 'platform' for non-SOF 
> solution:
> 
> diff --git a/sound/soc/intel/boards/broadwell.c 
> b/sound/soc/intel/boards/broadwell.c
> index b9c12e24c70b..db7e1e87156d 100644
> --- a/sound/soc/intel/boards/broadwell.c
> +++ b/sound/soc/intel/boards/broadwell.c
> @@ -164,14 +164,6 @@ SND_SOC_DAILINK_DEF(platform,
>   SND_SOC_DAILINK_DEF(codec,
>          DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));
> 
> -#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL)
> -SND_SOC_DAILINK_DEF(ssp0_port,
> -           DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
> -#else
> -SND_SOC_DAILINK_DEF(ssp0_port,
> -           DAILINK_COMP_ARRAY(COMP_DUMMY()));
> -#endif
> -
>   /* broadwell digital audio interface glue - connects codec <--> CPU */
>   static struct snd_soc_dai_link broadwell_rt286_dais[] = {
>          /* Front End DAI links */
> @@ -226,7 +218,7 @@ static struct snd_soc_dai_link 
> broadwell_rt286_dais[] = {
>                  .ops = &broadwell_rt286_ops,
>                  .dpcm_playback = 1,
>                  .dpcm_capture = 1,
> -               SND_SOC_DAILINK_REG(ssp0_port, codec, platform),
> +               SND_SOC_DAILINK_REG(dummy, codec, dummy),
>          },
> 

Of course I messed up the mail - above is a revert of said change.

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Thu, Mar 19, 2020 at 06:33:50PM +0100, Cezary Rojewski wrote:
> On 2020-03-19 17:51, Dominik Brodowski wrote:
> > On Thu, Mar 19, 2020 at 04:48:03PM +0100, Cezary Rojewski wrote:
> > > On 2020-03-19 14:41, Mark Brown wrote:
> > > > On Thu, Mar 19, 2020 at 02:00:49PM +0100, Dominik Brodowski wrote:
> > > > 
> > > > > Have some good news now, namely that a bisect is complete: That pointed to
> > > > > 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
> > > > > therefore I've added Kuninori Morimoto to this e-mail thread.
> > > > 
> > > > If that's an issue it feels more like a driver bug in that if the driver
> > > > asked for ignore_suspend then it should expect not to have the suspend
> > > > callback called.
> > > > 
> > > 
> > > Requested for tests with following diff applied:
> > > 
> > > diff --git a/sound/soc/intel/boards/broadwell.c
> > > b/sound/soc/intel/boards/broadwell.c
> > > index db7e1e87156d..6ed4c1b0a515 100644
> > > --- a/sound/soc/intel/boards/broadwell.c
> > > +++ b/sound/soc/intel/boards/broadwell.c
> > > @@ -212,7 +212,6 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] =
> > > {
> > >                  .init = broadwell_rt286_codec_init,
> > >                  .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> > >                          SND_SOC_DAIFMT_CBS_CFS,
> > > -               .ignore_suspend = 1,
> > >                  .ignore_pmdown_time = 1,
> > >                  .be_hw_params_fixup = broadwell_ssp0_fixup,
> > >                  .ops = &broadwell_rt286_ops,
> > 
> > That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
> > ("ASoC: Intel: broadwell: change cpu_dai and platform components for SOF")
> > on top of that. But you can assess better whether that patch needs care for
> > other reasons; for me, this one-liner you have suggested is perfect.
> > 
> > Many thanks -- it's been a pleasure to work with you on tracking this issue
> > down.
> > 
> > 	Dominik
> > 
> 
> Thank you for being so cooperative during this 2day debug session.
> 
> The patch I mentioned earlier unintentionally (?) changed 'platform'
> component param for ssp0_port from 'dummy' to 'platform' for non-SOF
> solution:
> 
> diff --git a/sound/soc/intel/boards/broadwell.c
> b/sound/soc/intel/boards/broadwell.c
> index b9c12e24c70b..db7e1e87156d 100644
> --- a/sound/soc/intel/boards/broadwell.c
> +++ b/sound/soc/intel/boards/broadwell.c
> @@ -164,14 +164,6 @@ SND_SOC_DAILINK_DEF(platform,
>  SND_SOC_DAILINK_DEF(codec,
>         DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));
> 
> -#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL)
> -SND_SOC_DAILINK_DEF(ssp0_port,
> -           DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
> -#else
> -SND_SOC_DAILINK_DEF(ssp0_port,
> -           DAILINK_COMP_ARRAY(COMP_DUMMY()));
> -#endif
> -
>  /* broadwell digital audio interface glue - connects codec <--> CPU */
>  static struct snd_soc_dai_link broadwell_rt286_dais[] = {
>         /* Front End DAI links */
> @@ -226,7 +218,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] =
> {
>                 .ops = &broadwell_rt286_ops,
>                 .dpcm_playback = 1,
>                 .dpcm_capture = 1,
> -               SND_SOC_DAILINK_REG(ssp0_port, codec, platform),
> +               SND_SOC_DAILINK_REG(dummy, codec, dummy),
>         },
> 
> 
> Said change causes following to occur:
> 
> 
> (stream start)
> [  113.251950] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000003000000
> size: 77
> [  113.252090] haswell-pcm-audio haswell-pcm-audio: > rx: 0x0000000043000000
> size: 48
> [  113.252097] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006301000
> size: 20
> [  113.252147] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006301000
> size: 20
> [  113.252179] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006100000
> size: 0
> [  113.252219] snd_soc_core:dpcm_fe_dai_hw_params:  System PCM: ASoC:
> hw_params FE System PCM rate 48000 chan 2 fmt 2
> [  113.252229] snd_soc_core:dapm_update_dai_unlocked: haswell-pcm-audio
> haswell-pcm-audio: Update DAI routes for System Pin playback
> [  113.252236] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006000000
> size: 0
> [  113.252304] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000004000000
> size: 4
> [  113.252425] snd_soc_sst_haswell_pcm:create_adsp_page_table:  System PCM:
> generating page table for 00000000a8c2b8a6 size 0x17700 pages 24
> 
> 
> (In essence these tx'es denote sequence for stream initialization while the
> last two for stream RESET (0x6000000) and FREE (0x4000000))
> 
> and that is only to recreate the stream once again:
> 
> 
> [  113.252673] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000003000000
> size: 77
> [  113.252803] haswell-pcm-audio haswell-pcm-audio: > rx: 0x0000000043000000
> size: 48
> [  113.252810] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006301000
> size: 20
> [  113.252864] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006301000
> size: 20
> [  113.252900] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006100000
> size: 0
> [  113.252987] snd_soc_core:dpcm_fe_dai_prepare:  System PCM: ASoC: prepare
> FE System PCM
> [  113.252993] snd_soc_core:dpcm_be_dai_prepare:  Codec: ASoC: prepare BE
> Codec
> [  113.253028] snd_soc_core:dpcm_dapm_stream_event:  Codec: ASoC: BE Codec
> event 1 dir 0
> [  113.254962] snd_soc_core:dpcm_do_trigger:  Codec: ASoC: trigger BE Codec
> cmd 1
> 
> 
> Because of that we ended up in _reset and _free being called twice:
> 
> 
> [  113.254969] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006200000
> size: 0
> [  113.254980] snd_soc_core:dpcm_dai_trigger_fe_be:  System PCM: ASoC: post
> trigger FE System PCM cmd 1
> [  113.254983] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006200000
> size: 0
> [  113.254996] snd_soc_sst_ipc:ipc_tx_msgs: haswell-pcm-audio
> haswell-pcm-audio: ipc_tx_msgs dsp busy
> [  118.486291]  System PCM: ASoC: trigger FE cmd: 7 failed: -22
> [  118.486431] snd_soc_core:dpcm_dai_trigger_fe_be:  System PCM: ASoC: pre
> trigger FE System PCM cmd 0
> [  118.486464] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006100000
> size: 0
> [  118.486495] snd_soc_core:dpcm_do_trigger:  Codec: ASoC: trigger BE Codec
> cmd 0
> [  118.486514] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006100000
> size: 0
> [  118.486550] snd_soc_core:dpcm_fe_dai_hw_free:  System PCM: ASoC: hw_free
> FE System PCM
> [  118.486569] snd_soc_core:dpcm_be_dai_hw_free:  Codec: ASoC: hw_free BE
> Codec
> [  118.486719] snd_soc_core:dpcm_fe_dai_hw_free:  System PCM: ASoC: hw_free
> FE System PCM
> [  118.486734] snd_soc_core:dpcm_be_dai_hw_free:  Codec: ASoC: hw_free BE
> Codec
> [  118.486751] snd_soc_core:dpcm_be_dai_shutdown:  Codec: ASoC: close BE
> Codec
> [  118.486801] snd_soc_sst_ipc:ipc_tx_msgs: haswell-pcm-audio
> haswell-pcm-audio: ipc_tx_msgs dsp busy
> [  118.489279] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000006000000
> size: 0
> [  118.489382] haswell-pcm-audio haswell-pcm-audio: tx: 0x0000000004000000
> size: 4
> [  118.489535] snd_soc_core:dpcm_fe_dai_shutdown:  System PCM: ASoC: close
> FE System PCM
> [  118.489547] haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL,
> no stream to reset, ignore it.
> [  118.489553] haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL,
> no stream to free, ignore it.
> [  118.489571] snd_soc_core:dpcm_be_disconnect:  System PCM: ASoC: BE
> playback disconnect check for Codec
> [  118.489580] snd_soc_core:dpcm_be_disconnect:  System PCM: freed DSP
> playback path System PCM -> Codec
> 
> 
> Could you confirm the same happens on your machine when revert of mentioned
> patch is not applied ("stream is NULL" messages occur)? Issue may be
> harmless but explained sequence does not look right.

Indeed, I still see

haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to reset, ignore it.
haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to free, ignore it.
haswell-pcm-audio haswell-pcm-audio: FW loaded, mailbox readback FW info: type 01, - version: 00.00, build 77, source commit id: 876ac6906f31a43b6772b23c7c983ce9dcb18a19
haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to reset, ignore it.
haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to free, ignore it.

though sounds continues to work.

Thanks again,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-19 19:24, Dominik Brodowski wrote:
> On Thu, Mar 19, 2020 at 06:33:50PM +0100, Cezary Rojewski wrote:
>>
>> Could you confirm the same happens on your machine when revert of mentioned
>> patch is not applied ("stream is NULL" messages occur)? Issue may be
>> harmless but explained sequence does not look right.
> 
> Indeed, I still see
> 
> haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to reset, ignore it.
> haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to free, ignore it.
> haswell-pcm-audio haswell-pcm-audio: FW loaded, mailbox readback FW info: type 01, - version: 00.00, build 77, source commit id: 876ac6906f31a43b6772b23c7c983ce9dcb18a19
> haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to reset, ignore it.
> haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no stream to free, ignore it.
> 
> though sounds continues to work.
> 

Thanks once again for your input and time!

I'll prepare patches for both issues. My guess is haswell-pcm could be 
updated to handle 'platform' component param just fine, but it is 
probably a change of more than few lines. I'd rather revert non-SOF 
broadwell to its previous behavior and start a separate task from there.

Regards,
Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Pierre-Louis Bossart]

On 3/19/20 1:35 PM, Cezary Rojewski wrote:
> On 2020-03-19 19:24, Dominik Brodowski wrote:
>> On Thu, Mar 19, 2020 at 06:33:50PM +0100, Cezary Rojewski wrote:
>>>
>>> Could you confirm the same happens on your machine when revert of 
>>> mentioned
>>> patch is not applied ("stream is NULL" messages occur)? Issue may be
>>> harmless but explained sequence does not look right.
>>
>> Indeed, I still see
>>
>> haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no 
>> stream to reset, ignore it.
>> haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no 
>> stream to free, ignore it.
>> haswell-pcm-audio haswell-pcm-audio: FW loaded, mailbox readback FW 
>> info: type 01, - version: 00.00, build 77, source commit id: 
>> 876ac6906f31a43b6772b23c7c983ce9dcb18a19
>> haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no 
>> stream to reset, ignore it.
>> haswell-pcm-audio haswell-pcm-audio: warning: stream is NULL, no 
>> stream to free, ignore it.
>>
>> though sounds continues to work.
>>
> 
> Thanks once again for your input and time!
> 
> I'll prepare patches for both issues. My guess is haswell-pcm could be 
> updated to handle 'platform' component param just fine, but it is 
> probably a change of more than few lines. I'd rather revert non-SOF 
> broadwell to its previous behavior and start a separate task from there.

It'd be good to know why a dummy platform component is required though.

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Keyon Jie]

On 3/20/20 1:35 AM, Mark Brown wrote:
> On Thu, Mar 19, 2020 at 12:21:47PM -0500, Pierre-Louis Bossart wrote:
>> On 3/19/20 11:51 AM, Dominik Brodowski wrote:
> 
>>> That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
>>> ("ASoC: Intel: broadwell: change cpu_dai and platform components for SOF")
>>> on top of that. But you can assess better whether that patch needs care for
>>> other reasons; for me, this one-liner you have suggested is perfect.
> 
> Good news!
> 
>> .ignore_suspend is set for bdw-rt5677.c and bdw-rt5650.c as well. I don't
>> know if that was intentional.
> 
> The intended use case is for applications doing audio during suspend
> like telephony audio between the modem and CODEC on a phone or
> compressed audio playback.  I guess the compressed audio playback case
> could possibly apply with these systems though x86 suspend/resume is
> usually sufficiently heavyweight that it's surprising.

I think that's true, on many of SKL- intel platforms(byt, hsw, bdw), we 
are seeing this .ignore_suspend set with offload or deep buffer FE 
dai_links configured together.

So it looks we can't ignore calling codec's suspend/resume callbacks 
during the power cycle for rt286 codec(on the Dell XPS here), which is 
actually supported on Chromebook SAMUS(rt5677)?

Thanks,
~Keyon

> 

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Dominik Brodowski]

On Thu, Mar 19, 2020 at 05:51:58PM +0100, Dominik Brodowski wrote:
> On Thu, Mar 19, 2020 at 04:48:03PM +0100, Cezary Rojewski wrote:
> > On 2020-03-19 14:41, Mark Brown wrote:
> > > On Thu, Mar 19, 2020 at 02:00:49PM +0100, Dominik Brodowski wrote:
> > > 
> > > > Have some good news now, namely that a bisect is complete: That pointed to
> > > > 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
> > > > therefore I've added Kuninori Morimoto to this e-mail thread.
> > > 
> > > If that's an issue it feels more like a driver bug in that if the driver
> > > asked for ignore_suspend then it should expect not to have the suspend
> > > callback called.
> > > 
> > 
> > Requested for tests with following diff applied:
> > 
> > diff --git a/sound/soc/intel/boards/broadwell.c
> > b/sound/soc/intel/boards/broadwell.c
> > index db7e1e87156d..6ed4c1b0a515 100644
> > --- a/sound/soc/intel/boards/broadwell.c
> > +++ b/sound/soc/intel/boards/broadwell.c
> > @@ -212,7 +212,6 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] =
> > {
> >                 .init = broadwell_rt286_codec_init,
> >                 .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> >                         SND_SOC_DAIFMT_CBS_CFS,
> > -               .ignore_suspend = 1,
> >                 .ignore_pmdown_time = 1,
> >                 .be_hw_params_fixup = broadwell_ssp0_fixup,
> >                 .ops = &broadwell_rt286_ops,
> 
> That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
> ("ASoC: Intel: broadwell: change cpu_dai and platform components for SOF")
> on top of that. But you can assess better whether that patch needs care for
> other reasons; for me, this one-liner you have suggested is perfect.

Seems this patch didn't make it into v5.6 (and neither did the other ones
you sent relating to the "dummy" components). Can these patches therefore be
marked for stable, please?

Thanks,
	Dominik

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Cezary Rojewski]

On 2020-03-30 12:23, Dominik Brodowski wrote:
> 
> Seems this patch didn't make it into v5.6 (and neither did the other ones
> you sent relating to the "dummy" components). Can these patches therefore be
> marked for stable, please?
> 
> Thanks,
> 	Dominik
> 

While one of the series was accepted and merged, there is a delay caused 
by Google/ SOF folks in merging the second one.

Idk why rt286 aka "broadwell" machine board patch has not been merged 
yet. It's not like we have to merge all (rt5650 + rt5650 + rt286) 
patches at once. Google guys can keep verifying Buddy or whatnot while 
guys with Dell XPS can enjoy smooth audio experience.

Sneak peak Dominik: there will be more suspend/ resume patches coming 
soon ^)^ reducing power consumption in low-power state which honestly 
you might not even notice, but hey why not :P

Czarek

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]

On Mon, Mar 30, 2020 at 01:10:34PM +0200, Cezary Rojewski wrote:
> On 2020-03-30 12:23, Dominik Brodowski wrote:

> > Seems this patch didn't make it into v5.6 (and neither did the other ones
> > you sent relating to the "dummy" components). Can these patches therefore be
> > marked for stable, please?

I sent my pull request already sorry - once it hits Linus' tree I'd send
a request to stable.

> While one of the series was accepted and merged, there is a delay caused by
> Google/ SOF folks in merging the second one.

> Idk why rt286 aka "broadwell" machine board patch has not been merged yet.
> It's not like we have to merge all (rt5650 + rt5650 + rt286) patches at
> once. Google guys can keep verifying Buddy or whatnot while guys with Dell
> XPS can enjoy smooth audio experience.

My scripting is set up to merge things sent to me as a patch series and
we didn't get positive review from Pierre on any of it with the review
on that one patch seeming to suggest it might also be waiting go go
through a test farm.  TBH I also wasn't expecting it to take quite so
long to get reviewed when it came in, it's been over 2 weeks now...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

[Pierre-Louis Bossart]

On 3/30/20 6:39 AM, Mark Brown wrote:
> On Mon, Mar 30, 2020 at 01:10:34PM +0200, Cezary Rojewski wrote:
>> On 2020-03-30 12:23, Dominik Brodowski wrote:
> 
>>> Seems this patch didn't make it into v5.6 (and neither did the other ones
>>> you sent relating to the "dummy" components). Can these patches therefore be
>>> marked for stable, please?
> 
> I sent my pull request already sorry - once it hits Linus' tree I'd send
> a request to stable.
> 
>> While one of the series was accepted and merged, there is a delay caused by
>> Google/ SOF folks in merging the second one.
> 
>> Idk why rt286 aka "broadwell" machine board patch has not been merged yet.
>> It's not like we have to merge all (rt5650 + rt5650 + rt286) patches at
>> once. Google guys can keep verifying Buddy or whatnot while guys with Dell
>> XPS can enjoy smooth audio experience.
> 
> My scripting is set up to merge things sent to me as a patch series and
> we didn't get positive review from Pierre on any of it with the review
> on that one patch seeming to suggest it might also be waiting go go
> through a test farm.  TBH I also wasn't expecting it to take quite so
> long to get reviewed when it came in, it's been over 2 weeks now...

There are multiple problems with Broadwell and device-specific issues on 
suspend-resume - in which Cezary is involved. The tests are not 
automated so depend on people availability.

I tested this series last Friday and I didn't find any new problem on my 
side, so we should probably merge this series.

Everyone should be aware though that suspend-resume is far from stable 
on Broadwell, and if it works on Dell XPS 13 it doesn't work reliably on 
Chrome devices.