2020-03-18 06:32:09

by Dominik Brodowski

[permalink] [raw]
Subject: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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


2020-03-18 09:42:23

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 09:58:33

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 10:19:19

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 10:20:29

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 10:50:12

by Keyon Jie

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1



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

2020-03-18 12:41:06

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 15:29:52

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1



>>> 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?

2020-03-18 16:23:41

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 17:11:28

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1



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.

2020-03-18 17:21:22

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 17:31:55

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1



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

2020-03-18 17:37:28

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1


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

2020-03-18 18:28:50

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 19:15:50

by Ross Zwisler

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

On Wed, Mar 18, 2020 at 10:25:32AM -0700, Curtis Malainey wrote:
> +Ross Zwisler <[email protected]>
> 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.

2020-03-18 19:23:28

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 20:45:00

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 21:53:31

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-18 22:23:11

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-19 13:06:39

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-19 13:33:24

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-19 13:42:18

by Mark Brown

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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.


Attachments:
(No filename) (1.03 kB)
signature.asc (499.00 B)
Download all attachments

2020-03-19 15:49:44

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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,

2020-03-19 16:53:02

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-19 17:22:51

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1



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.

2020-03-19 17:34:50

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-19 17:35:56

by Mark Brown

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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.


Attachments:
(No filename) (895.00 B)
signature.asc (499.00 B)
Download all attachments

2020-03-19 17:43:36

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-19 17:46:11

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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.

2020-03-19 18:25:41

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-19 18:35:44

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-19 19:30:04

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1



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.

2020-03-20 03:22:31

by Keyon Jie

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1



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

>

2020-03-30 10:26:17

by Dominik Brodowski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-30 11:11:14

by Cezary Rojewski

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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

2020-03-30 11:40:55

by Mark Brown

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1

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...


Attachments:
(No filename) (1.14 kB)
signature.asc (499.00 B)
Download all attachments

2020-03-30 16:32:42

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since 5.6-rc1



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.