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

topology2: add support for MTL Acer Swift Go 14 #9041

Closed
wants to merge 1 commit into from

Conversation

plbossart
Copy link
Member

This device has RT712 on link0 2 PCH-attached DMICs.

Add 3 configurations with no DMIC, 4 DMIC and 2 DMIC. For now no support for -pdm1.

Link: thesofproject/linux#4923

@plbossart plbossart requested review from bardliao, ranj063 and jsarha and removed request for jsarha and ranj063 April 12, 2024 21:35
"cavs-sdw\;sof-mtl-rt712-l0\;PLATFORM=mtl,NUM_SDW_AMP_LINKS=1,\
SDW_AMP_FEEDBACK=false,SDW_SPK_STREAM=Playback-SmartAmp,\
SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack,\
HDMI1_ID=4,HDMI2_ID=5,HDMI3_ID=6"
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no amp feedback on rt712. So rt712 link ids will end at 2.

 snd_soc_sof_sdw:init_dai_link: sof_sdw sof_sdw: create dai link SDW0-Playback-SimpleJack, id 0
 snd_soc_sof_sdw:init_dai_link: sof_sdw sof_sdw: create dai link SDW0-Capture-SimpleJack, id 1
 snd_soc_sof_sdw:init_dai_link: sof_sdw sof_sdw: create dai link SDW0-Playback-SmartAmp, id 2

So, HDMI1_ID should start with 3.

Suggested change is

diff --git a/tools/topology/topology2/production/tplg-targets-ace1.cmake b/tools/topology/topology2/production/tplg-targets-ace1.cmake
index d2d9817dba97..aa20c3fcb20c 100644
--- a/tools/topology/topology2/production/tplg-targets-ace1.cmake
+++ b/tools/topology/topology2/production/tplg-targets-ace1.cmake
@@ -16,21 +16,21 @@ SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack"
 "cavs-sdw\;sof-mtl-rt712-l0\;PLATFORM=mtl,NUM_SDW_AMP_LINKS=1,\
 SDW_AMP_FEEDBACK=false,SDW_SPK_STREAM=Playback-SmartAmp,\
 SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack,\
-HDMI1_ID=4,HDMI2_ID=5,HDMI3_ID=6"
+HDMI1_ID=3,HDMI2_ID=4,HDMI3_ID=5"

 "cavs-sdw\;sof-mtl-rt712-l0-4ch\;PLATFORM=mtl,NUM_SDW_AMP_LINKS=1,\
 SDW_AMP_FEEDBACK=false,SDW_SPK_STREAM=Playback-SmartAmp,\
 SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack,\
-NUM_DMICS=4,PDM1_MIC_A_ENABLE=0,PDM1_MIC_B_ENABLE=0,\
+NUM_DMICS=4,PDM1_MIC_A_ENABLE=0,PDM1_MIC_B_ENABLE=0,DMIC0_ID=3,DMIC1_ID=4,\
 PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-sof-mtl-rt712-l0-4ch.bin,\
-HDMI1_ID=6,HDMI2_ID=7,HDMI3_ID=8"
+HDMI1_ID=5,HDMI2_ID=6,HDMI3_ID=7"

 "cavs-sdw\;sof-mtl-rt712-l0-2ch\;PLATFORM=mtl,NUM_SDW_AMP_LINKS=1,\
 SDW_AMP_FEEDBACK=false,SDW_SPK_STREAM=Playback-SmartAmp,\
 SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack,\
-NUM_DMICS=2,PDM1_MIC_A_ENABLE=0,PDM1_MIC_B_ENABLE=0,\
+NUM_DMICS=2,PDM1_MIC_A_ENABLE=0,PDM1_MIC_B_ENABLE=0,DMIC0_ID=3,\
 PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-sof-mtl-rt712-l0-2ch.bin,\
-HDMI1_ID=6,HDMI2_ID=7,HDMI3_ID=8"
+HDMI1_ID=5,HDMI2_ID=6,HDMI3_ID=7"

Note that sof_sdw create dmic01 and dmic16k no matter 1 or 2 dmics are present. So, HDMI link ids are the same for -2ch and -4ch topologies.

        /* enable dmic01 & dmic16k */
        if (sof_sdw_quirk & SOF_SDW_PCH_DMIC || mach_params->dmic_num)
                dmic_num = 2;

@plbossart plbossart force-pushed the sdw/acer-swift-go-14 branch from 1ce8ed3 to 82948bc Compare April 15, 2024 14:16
@plbossart plbossart requested a review from bardliao April 15, 2024 14:16
@plbossart
Copy link
Member Author

v2 updated with suggested changes from @bardliao. Thanks Bard.

"cavs-sdw\;sof-mtl-rt712-l0-4ch\;PLATFORM=mtl,NUM_SDW_AMP_LINKS=1,\
SDW_AMP_FEEDBACK=false,SDW_SPK_STREAM=Playback-SmartAmp,\
SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack,\
NUM_DMICS=4,PDM1_MIC_A_ENABLE=0,PDM1_MIC_B_ENABLE=0,DMIC0_ID=3,DMIC1_ID=4,\
Copy link
Member Author

Choose a reason for hiding this comment

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

What is DMIC1_ID @bardliao ? If indeed the 2-ch version creates dmic01 and dmic16k, should we not have an ID for the BE ID for the dmic16k dailink?

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is DMIC1_ID @bardliao ? If indeed the 2-ch version creates dmic01 and dmic16k, should we not have an ID for the BE ID for the dmic16k dailink?

Hmm, DMIC1_ID is for the second DMIC DAI id. I thought the second DMIC DAI will not be created if NUM_DMICS=2. But apparently, it is always created. So, we need to set DMIC1_ID=4 in the -2ch topology, too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, PDM1_MIC_A_ENABLE and PDM1_MIC_B_ENABLE should be 1 for -4ch topology.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @bardliao, updated both parts.

This device has RT712 on link0 2 PCH-attached DMICs.

Add 3 configurations with no DMIC, 4 DMIC and 2 DMIC. For now no
support for -pdm1.

Link: thesofproject/linux#4923
Signed-off-by: Pierre-Louis Bossart <[email protected]>
@plbossart plbossart force-pushed the sdw/acer-swift-go-14 branch from 82948bc to 020ca78 Compare April 16, 2024 13:27
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

LGTM, but will wait for @bardliao to approve.

Copy link
Collaborator

@bardliao bardliao left a comment

Choose a reason for hiding this comment

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

LGTM, but better to wait for the test result of thesofproject/linux#4923

@lgirdwood
Copy link
Member

LGTM, but better to wait for the test result of thesofproject/linux#4923

Ok, @bardliao pls ping when the tests pass. Thanks !

@bardliao
Copy link
Collaborator

LGTM, but better to wait for the test result of thesofproject/linux#4923

Ok, @bardliao pls ping when the tests pass. Thanks !

@lgirdwood From thesofproject/linux#4970, this PR should be good. The DSP panic looks like a different issue.

Copy link
Contributor

@ujfalusi ujfalusi left a comment

Choose a reason for hiding this comment

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

The HDMI PCM device id should be 5, 6 and 7 for sof-soundwire cards, we must not use the PCM ids which is used by sof-hda-dsp card.

"cavs-sdw\;sof-mtl-rt712-l0\;PLATFORM=mtl,NUM_SDW_AMP_LINKS=1,\
SDW_AMP_FEEDBACK=false,SDW_SPK_STREAM=Playback-SmartAmp,\
SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack,\
HDMI1_ID=3,HDMI2_ID=4,HDMI3_ID=5"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct, sof-soundwire is expected to have HDMI PCM deviecs of 5, 6 and 7. UCM will break if this is not true: https://github.com/alsa-project/alsa-ucm-conf/blob/master/ucm2/sof-soundwire/Hdmi.conf

Also to note that the support for HDMI passthrough also relies on this rule: alsa-project/alsa-ucm-conf@0397a38

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is correct, sof-soundwire is expected to have HDMI PCM deviecs of 5, 6 and 7. UCM will break if this is not true: https://github.com/alsa-project/alsa-ucm-conf/blob/master/ucm2/sof-soundwire/Hdmi.conf

HDMI1_ID is BE id, not PCM id. The default value HDMI PCM IDs are HDMI1_PCM_ID 5, HDMI2_PCM_ID 6, and HDMI3_PCM_ID 7, and the PR doesn't change the HDMI PCM IDs.

Also to note that the support for HDMI passthrough also relies on this rule: alsa-project/alsa-ucm-conf@0397a38

Copy link
Contributor

Choose a reason for hiding this comment

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

@bardliao, I se, but the user reports that no HDMI audio and speaker is not working, so we are still missing something.

@thedanhoffman
Copy link

What's the current status on this? Thanks

@plbossart
Copy link
Member Author

What's the current status on this? Thanks

still being tested, see thesofproject/linux#4970

@bardliao
Copy link
Collaborator

bardliao commented Aug 2, 2024

@plbossart Finally Acer Swift Go 14 works with UCM thesofproject/linux#4923 (comment). My only open for this PR is that we have sof-mtl-rt712-l0 in the existing topology. The sof-mtl-rt712-l0 doesn't have either SDW DMIC or PCH DMIC support and use the default HDMI_IDs. This doesn't look right.

@bardliao
Copy link
Collaborator

bardliao commented Aug 2, 2024

Maybe split the PR into 2 commits, one to edit the HDMI_IDs ie. HDMI1_ID=3,HDMI2_ID=4,HDMI3_ID=5 for sof-mtl-rt712-l0 and the other add sof-mtl-rt712-l0-2ch and sof-mtl-rt712-l0-4ch

@plbossart
Copy link
Member Author

@bardliao do you mind submitting your changes directly in a new PR?
I don't really know what you sent for testing so likely I'll screw-up...

@bardliao
Copy link
Collaborator

bardliao commented Aug 2, 2024

@bardliao do you mind submitting your changes directly in a new PR? I don't really know what you sent for testing so likely I'll screw-up...

Sure, will do.

@plbossart
Copy link
Member Author

ok, thanks @bardliao I'll close this PR then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants