Skip to content

ALSA: hda/hdmi: fix failure at PCM open - #2223

Closed
kv2019i wants to merge 3 commits into
thesofproject:topic/sof-devfrom
kv2019i:topic/hdmi-nopin-open-fix
Closed

ALSA: hda/hdmi: fix failure at PCM open#2223
kv2019i wants to merge 3 commits into
thesofproject:topic/sof-devfrom
kv2019i:topic/hdmi-nopin-open-fix

Conversation

@kv2019i

@kv2019i kv2019i commented Jun 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator 
ALSA: hda/hdmi: fix failure at PCM open

As legacy of the HDMI codec driver, PCM open does not report errors
if device is opened when no receiver is connected. This was implemented
to keep Pulseaudio happy when it does the initial scan of PCM devices.
On systems using UCM, this is no longer needed as Pulseaudio can get
list of PCM devices from UCM. As this does not cover all systems,
the legacy mode needs to be supported as well.

To implement the behaviour, codec driver uses two code paths for PCM
open -- one when receiver is connected and other for "no pin" case.
Problem occurs in following sequence:

 1) pcm0 connected, pcm1 not connected
        - pcm0 is attached to cvt0
 2) application opens pcm1
        - no monitor connected to pcm1, so use the first free CVT
          cvt0 for pcm1
 3) application opens pcm0
        - cvt0 not available
        - cvt1...N not attached to pcm0, hdmi_choose_cvt() will fail
        - open fails to -EBUSY

Fix the problem by prioritizing the converter with the same index as PCM
(i.e. mimic logic in hdmi_find_pcm_slot()). E.g. in above example, in
step 2, driver would assign cvt1 for pcm1.

Fixes: #2216

kv2019i added 3 commits June 22, 2020 12:33
@kv2019i
ASoC: hdac_hda: fix deadlock after PCM open error
3c4189c 
Commit 202bac2 ("ASoC: soc-dai: revert all changes to DAI
startup/shutdown sequence"), introduced a slight change of semantics
to DAI startup/shutdown. If startup() returns an error, shutdown()
is now called for the DAI.

This causes a deadlock in hdac_hda which issues a call to
snd_hda_codec_pcm_put() in case open fails. Upon error, soc_pcm_open()
will call shutdown(), and pcm_put() ends up getting called twice. Result
is a deadlock on pcm->open_mutex, as snd_device_free() gets called from
within snd_pcm_open(). Typical task backtrace looks like this:

[  334.244627]  snd_pcm_dev_disconnect+0x49/0x340 [snd_pcm]
[  334.244634]  __snd_device_disconnect.part.0+0x2c/0x50 [snd]
[  334.244640]  __snd_device_free+0x7f/0xc0 [snd]
[  334.244650]  snd_hda_codec_pcm_put+0x87/0x120 [snd_hda_codec]
[  334.244660]  soc_pcm_open+0x6a0/0xbe0 [snd_soc_core]
[  334.244676]  ? dpcm_add_paths.isra.0+0x491/0x590 [snd_soc_core]
[  334.244679]  ? kfree+0x9a/0x230
[  334.244686]  dpcm_be_dai_startup+0x255/0x300 [snd_soc_core]
[  334.244695]  dpcm_fe_dai_open+0x20e/0xf30 [snd_soc_core]
[  334.244701]  ? snd_pcm_hw_rule_muldivk+0x110/0x110 [snd_pcm]
[  334.244709]  ? dpcm_be_dai_startup+0x300/0x300 [snd_soc_core]
[  334.244714]  ? snd_pcm_attach_substream+0x3c4/0x540 [snd_pcm]
[  334.244719]  snd_pcm_open_substream+0x69a/0xb60 [snd_pcm]
[  334.244729]  ? snd_pcm_release_substream+0x30/0x30 [snd_pcm]
[  334.244732]  ? __mutex_lock_slowpath+0x10/0x10
[  334.244736]  snd_pcm_open+0x1b3/0x3c0 [snd_pcm]

Fixes: 202bac2 ("ASoC: soc-dai: revert all changes to DAI startup/shutdown sequence")
BugLink: thesofproject#2159
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
ALSA: hda/hdmi: fix failure at PCM open
8a59b42 
As legacy of the HDMI codec driver, PCM open does not report errors
if device is opened when no receiver is connected. This was implemented
to keep Pulseaudio happy when it does the initial scan of PCM devices.
On systems using UCM, this is no longer needed as Pulseaudio can get
list of PCM devices from UCM. As this does not cover all systems,
the legacy mode needs to be supported as well.

To implement the behaviour, codec driver uses two code paths for PCM
open -- one when receiver is connected and other for "no pin" case.
Problem occurs in following sequence:

 1) pcm0 connected, pcm1 not connected
	- pcm0 is attached to cvt0
 2) application opens pcm1
	- no monitor connected to pcm1, so use the first free CVT
          cvt0 for pcm1
 3) application opens pcm0
	- cvt0 not available
	- cvt1...N not attached to pcm0, hdmi_choose_cvt() will fail
	- open fails to -EBUSY

Fix the problem by prioritizing the converter with the same index as PCM
(i.e. mimic logic in hdmi_find_pcm_slot()). E.g. in above example, in
step 2, driver would assign cvt1 for pcm1.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
ALSA: hda/hdmi: improve debug traces
50d6266 
The HDMI codec driver has two debug traces printed from different
functions but with identical message content:

"HDMI: hinfo 000000006a6b84d9 not registered"

Fix this this duplication and also add a bit more context in
addition to raw object pointer, to help analysis of kernel logs.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i mentioned this pull request Jun 22, 2020
Closed
plbossart
plbossart requested changes Jun 22, 2020
View reviewed changes
Comment thread sound/pci/hda/patch_hdmi.c
err = hdmi_choose_cvt(codec, -1, &cvt_idx);
err = hdmi_choose_cvt_nopin(codec, pcm_idx, &cvt_idx);
if (err)
err = hdmi_choose_cvt(codec, -1, &cvt_idx);

@plbossart plbossart Jun 22, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit message nit-pick:
Fix the problem by prioritizing the converter with the same index as PCM

This isn't really prioritizing, you first try with the nopin version then fallback to the existing solution.

This was referenced Jun 23, 2020
Closed
Closed
@kv2019i

kv2019i commented Jun 26, 2020

Copy link
Copy Markdown
Collaborator Author

This did not fix the full problem. I found some further issues and am working on a fix. Closing this one.

@kv2019i kv2019i closed this Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@plbossart plbossart plbossart requested changes
@bardliao bardliao Awaiting requested review from bardliao bardliao is a code owner
@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner
@lgirdwood lgirdwood Awaiting requested review from lgirdwood lgirdwood is a code owner
@ranj063 ranj063 Awaiting requested review from ranj063 ranj063 is a code owner

Assignees

No one assigned

Labels

Draft-for-testing-not-ready-for-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[bug][JSL]Warning "ASoC: error at snd_soc_dai_startup on intel-hdmi-hifi1: -16" shows in dmesg when boot up with DP connected.

2 participants

@kv2019i @plbossart