Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASoC: Intel: sof_sdw: Always register the HDMI dai links #4639

Conversation

ujfalusi
Copy link
Collaborator

@ujfalusi ujfalusi commented Oct 13, 2023

The topology files for SDW devices require HDMI dai links to be present and
this is granted under normal conditions but in case of special use cases
the display (i915) driver might not be enabled due to deny-listing,
booting with nomodeset or just not compiled at all.

This should not block the non HDMI audio to be usable so register the dai
links unconditionally. The code has been prepared for this and in case of
no HDMI audio the link is created with dummy codec.

Closes: #4594
Closes: #4648

@ujfalusi
Copy link
Collaborator Author

Fixes: #4594

bardliao
bardliao previously approved these changes Oct 16, 2023
@@ -1572,8 +1572,8 @@ static int sof_card_dai_links_create(struct snd_soc_card *card)
int num_links, link_index = 0;
char *name, *cpu_dai_name;
char *codec_name, *codec_dai_name;
int codec_index, hdmi_num;
Copy link
Member

Choose a reason for hiding this comment

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

i really don't like two variables without any correlation on the same line. It's legal in C but it looks weird. Why would we group codec_index and hdmi_num?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK

@@ -1611,7 +1610,8 @@ static int sof_card_dai_links_create(struct snd_soc_card *card)
bt_num = 1;

dev_dbg(dev, "sdw %d, ssp %d, dmic %d, hdmi %d, bt: %d\n",
sdw_be_num, ssp_num, dmic_num, hdmi_num, bt_num);
sdw_be_num, ssp_num, dmic_num,
ctx->hdmi.idisp_codec ? hdmi_num : 0, bt_num);
Copy link
Member

Choose a reason for hiding this comment

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

commit message needs a

Closes: #4594

@ujfalusi
Copy link
Collaborator Author

Changes since v1:

  • move the declaration of hdmi_num as separate line
  • add Closes tag to commit message

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks @ujfalusi !

@ujfalusi
Copy link
Collaborator Author

Changes since v2:

  • rebased on topic/sof-dev

The topology files for SDW devices require HDMI dai links to be present and
this is granted under normal conditions but in case of special use cases
the display (i915) driver might not be enabled due to deny-listing,
booting with nomodeset or just not compiled at all.

This should not block the non HDMI audio to be usable so register the dai
links unconditionally. The code has been prepared for this and in case of
no HDMI audio the link is created with dummy codec.

Closes: thesofproject#4594
Closes: thesofproject#4648
Signed-off-by: Peter Ujfalusi <[email protected]>
@ujfalusi ujfalusi force-pushed the peter/sof/pr/sdw_without_idisp_01 branch from 588a533 to b19702e Compare October 24, 2023 07:55
@ujfalusi
Copy link
Collaborator Author

Changes since v3:

@ujfalusi
Copy link
Collaborator Author

@plbossart, @keqiaozhang, internal test runs: 33544 and 33550.
33550 fails with HDMI playback with

sof-audio-pci-intel-mtl 0000:00:1f.3: ASoC: error at snd_soc_dai_hw_params on iDisp1 Pin: -22

So the card forms with this patch but the non existent HDMI is not working. Let me see if I can figure out something for this.

@ujfalusi
Copy link
Collaborator Author

This is the exact behavior with HDA machines also. If the HDMI is disabled (nomodeset or denylisted/not compiled display driver) then attempt to play to HDMI PCM results:

sof-audio-pci-intel-tgl 0000:00:1f.3: ASoC: error at snd_soc_dai_hw_params on iDisp1 Pin: -22

I'm not sure how this can be fixed, dummy dai almost works (iow does not). I can get it probe with that with a hack in sound/soc/sof/topology.c :

diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index a3a3af252259..b123f4ee9b48 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -1089,6 +1091,8 @@ static int sof_connect_dai_widget(struct snd_soc_component *scomp,
 			if (!snd_soc_dai_get_widget(cpu_dai, stream)) {
 				snd_soc_dai_set_widget(cpu_dai, stream, w);
 				break;
+			} else if (!strcmp(cpu_dai->name, "snd-soc-dummy-dai")) {
+				break;
 			}
 		}
 		if (i == rtd->dai_link->num_cpus) {

But then the DAPM route is messed up badly and the playback attempt will fail with different error.

I'm not sure if we want or need to fix that and to how...

FWIW, before b785e24 the codec for HDMI did showed up and that's why the playback did not failed:

sof-audio-pci-intel-mtl 0000:00:1f.3: hda codecs found, mask 4

vs after the patch:

sof-audio-pci-intel-mtl 0000:00:1f.3: no hda codecs found!

@plbossart plbossart merged commit 159b6e0 into thesofproject:topic/sof-dev Oct 26, 2023
9 checks passed
@ujfalusi ujfalusi deleted the peter/sof/pr/sdw_without_idisp_01 branch November 30, 2023 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants