Skip to content

Conversation

@kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Jun 29, 2020

ALSA: hda/hdmi: fix failures at PCM open on Intel ICL and later

Bugfix and one trace improvement patch.

Fixes: #1978
Fixes: #2216
Fixes: #2217

ranj063
ranj063 previously approved these changes Jul 1, 2020
plbossart
plbossart previously approved these changes Jul 2, 2020
@plbossart
Copy link
Member

plbossart commented Jul 2, 2020

@kv2019i I'll let you merge this, i don't recall if this is the final version or still WIP.

@libinyang
Copy link

@kv2019i Good to catch the initialization issue.

If we go through the code deeply, it seems we should not set intel_hsw_fixup flag for ICL as the latest platforms are not hsw like platform. Without the flag, the connection list can be initialized correctly. We are wrongly use this flag now. And I can also see some potential issues if we set intel_hsw_fixup flag on ICL.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Jul 3, 2020

@libinyang wrote:

If we go through the code deeply, it seems we should not set intel_hsw_fixup flag for ICL as the latest platforms are not hsw
like platform. Without the flag, the connection list can be initialized correctly. We are wrongly use this flag now. And I can also > see some potential issues if we set intel_hsw_fixup flag on ICL.

That's a good point, thanks. Let me do some tests on this. The hsw_fixup flag is used for multiple things and I don't think all have been fixed for ICL. E.g. it's used for hdmi_add_pin(). But if the snd_hda_get_raw_connections() works correctly on ICL, we could indeed remote the exception for ICL and newer.

@plbossart @ranj063 Let's not merge this yet.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Jul 3, 2020

@libinyang OK, I tried without the hsw_fixup, but it still fails. On pins with monitor connected, snd_hda_get_raw_connections() works ok and full list of cvt nids is returned, but on pins with no monitor connected at time of probe, no connections are returned.

So maybe best to go with current patch still.

UPDATE: @libinyang I tested with ICL and CFL systems without the fixup and the results are similar, so I'll keep the fixup.

@kv2019i kv2019i added the upstream Patch has been sent to upstream (e.g. alsa-devel) label Jul 3, 2020
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jul 3, 2020

I sent this upstream now, let's see if we need to adjust more before merging to sof-dev.

lyakh
lyakh previously approved these changes Jul 6, 2020
kv2019i added 2 commits July 7, 2020 12:10
When HDMI PCM devices are opened in a specific order, with at least one
HDMI/DP receiver connected, ALSA PCM open fails to -EBUSY on the
connected monitor, on recent Intel platforms (ICL/JSL and newer). While
this is not a typical sequence, at least Pulseaudio does this every time
when it is started, to discover the available PCMs.

The rootcause is an invalid assumption in hdmi_add_pin(), where the
total number of converters is assumed to be known at the time the
function is called. On older Intel platforms this held true, but after
ICL/JSL, the order how pins and converters are in the subnode list as
returned by snd_hda_get_sub_nodes(), was changed. As a result,
information for some converters was not stored to per_pin->mux_nids.
And this means some pins cannot be connected to all converters, and
application instead gets -EBUSY instead at open.

The assumption that converters are always before pins in the subnode
list, is not really a valid one. Fix the problem in hdmi_parse_codec()
by introducing separate loops for discovering converters and pins.

BugLink: thesofproject#1978
BugLink: thesofproject#2216
BugLink: thesofproject#2217
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Link: https://lore.kernel.org/r/20200703153818.2808592-1-kai.vehmanen@linux.intel.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
(cherry picked from commit 5627503)
The HDMI codec driver has two debug traces printed from different
functions but with identical message content:

"HDMI: hinfo 000000006a6b84d9 not registered"

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

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Link: https://lore.kernel.org/r/20200703153818.2808592-2-kai.vehmanen@linux.intel.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
(cherry picked from commit 90670fd)
@kv2019i kv2019i dismissed stale reviews from lyakh, plbossart, and ranj063 via 6bdaac1 July 7, 2020 09:12
@kv2019i kv2019i force-pushed the topic/hdmi-icljsl-pcm-open-bugfix branch from 3aa695f to 6bdaac1 Compare July 7, 2020 09:12
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jul 7, 2020

Merged upstream. As this fixes multiple SOF issues, I'll go ahead and merge the upstream version to sof-dev right away.

@kv2019i kv2019i merged commit 7b70f8b into thesofproject:topic/sof-dev Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

upstream Patch has been sent to upstream (e.g. alsa-devel)

Projects

None yet

5 participants