volume-basic-test.sh: switch to .tplg parsing to test topologies v2 #1193

marc-hb · 2024-05-13T21:16:17Z

4 commits

Until now this test was "screen-scraping" amixer output. This was not
compatible with v2 topologies which means the test was always SKIP for
everything developped on the main branch. Switch to parsing topologies
to extract volume kcontrols.

Fixes: #1069, see also discussion in earlier attempt #1068.

No typo left behind. Signed-off-by: Marc Herbert <[email protected]>

marc-hb · 2024-05-14T02:01:10Z

In some test configurations there are many more "volume" kcontrols than found in the topology. Is this expected?

Probably the most extreme example is on stable-v2.2 / topology v1:

jf-cml-hel-rt5682-05:~$ amixer | grep -B1 Capabilities:.*volume

38 "volume" controls!! for instance:

numid=2,iface=MIXER,name='CBJ Boost Volume'
numid=1,iface=MIXER,name='DAC1 Playback Volume'
...

The vast majority of these are NOT in sof-tplg/sof-cml-rt1011-rt5682.tplg which has only PGA1 to PGA7.

Similarly,
https://sof-ci.01.org/softestpr/PR1193/build420/devicetest/index.html?model=LNLM_SDW_AIOC&testcase=volume-basic-test

ba-lnlm-rvp-sdw-03:~$ amixer | grep -B1 Capabilities:.*volume

23 volume controls, among others:

'Pre Mixer Deepbuffer Jack Out', 'rt1316-1 Left I Tag Select', 'rt711 FU15 Gain', 'rt714 FU0C Boost', etc.

Yet sof-ipc4-tplg/sof-lnl-rt711-l0-rt1316-l23-rt714-l1.tplg has only 5 PGAs gain.N.M:

PGA: gain.0.1
Pre Mixer Jack Out Playback Volume
PGA: gain.1.1
Post Mixer Jack Out Playback Volume
PGA: gain.15.1
Pre Mixer Deepbuffer Jack Out Volume
PGA: gain.20.1
Amplifier Volume
PGA: gain.21.1
Main Amplifier Volume

plbossart · 2024-05-14T18:41:29Z

numid=2,iface=MIXER,name='CBJ Boost Volume'
numid=1,iface=MIXER,name='DAC1 Playback Volume'

Those are codec kcontrols.

marc-hb · 2024-05-14T18:45:56Z

Those are codec kcontrols.

Thanks, @ranj063 just told me: no need to test codec kcontrols (at least not in this "basic" test)

marc-hb

New volume tests passing in:

Old tests still passing "the new way" on stable-v2.2

https://sof-ci.01.org/sofpr/PR9113/build4600/devicetest/index.html

All test results looking good!

marc-hb · 2024-05-14T18:47:42Z

tools/topo2_list_vol_kcontrols.py

+
+            # TODO: find how the kernel makes this "topology v1 versus v2"
+            # decision and mimic it here.
+            pga_prefix = (


Clue from @ranj063 (thx!) no_wname_in_kcontrol_name

https://github.com/thesofproject/sof/blob/fdb67ee782808f6d5488a53441cdecd75f458677/tools/topology/topology2/include/components/widget-common.conf#L121

marc-hb · 2024-05-14T20:19:07Z

Those are codec kcontrols.

Wait, is numid=9,iface=MIXER,name='Master Playback Volume' really a codec kcontrol? The name really does not sound like it (and it's not in sof-ipc4-tplg/sof-hda-generic-ace1-4ch.tplg)

https://sof-ci.01.org/softestpr/PR1193/build420/devicetest/index.html?model=LNLM_RVP_HDA&testcase=volume-basic-test

plbossart · 2024-05-14T20:32:20Z

You could skip the SOF driver with 'options snd-intel-dspcfg dsp_driver=1' and you would see which controls are from the codec. 'Master Playback' is rather typical of HDaudio codecs.

That said, we should indeed have added an SOF prefix to make all firmware-controls self-explanatory.

Synchronize with Linux commit 246b5b77fc74 ("ASoC: SOF: topology: Add a token for dropping widget name in kcontrol name") Signed-off-by: Marc Herbert <[email protected]>

Will be used in next commit to fix thesofproject#1068. See also discussion in earlier attempt thesofproject#1068.

Until now this test was "screen-scraping" amixer output. This was not compatible with v2 topologies which means the test was always SKIP for everything developped on the main branch. Switch to parsing topologies to extract volume kcontrols. Fixes: thesofproject#1069, see also discussion in earlier attempt thesofproject#1068. Signed-off-by: Marc Herbert <[email protected]>

marc-hb · 2024-05-15T23:57:28Z

@ranj063 , @plbossart , I flipped the NO_WNAME setting so one .tplg file goes past the 44 characters limit in order to test the kcontrol truncation issue spotted in:

volume-basic-test.sh: don't skip "playback volume" controls #1068 (comment)

jf-adlp-rvp-nocodec-8:~/$ amixer controls

numid=14,iface=MIXER,name='gain.17.1 Post Demux Port0 2nd Capture Volu'
numid=15,iface=MIXER,name='gain.18.1 Post Demux DMIC SFX1 Capture Volu'

Then I ran the new test in this PR. It passes without any issue because either amixer or the kernel truncates on BOTH output AND input. In other words, this command works just fine:

$ amixer cget name='gain.17.1 Post Demux Port0 2nd Capture VoluTRUNCATED_DOES_NOT_MATTER'
numid=14,iface=MIXER,name='gain.17.1 Post Demux Port0 2nd Capture Volu'
  ; type=INTEGER,access=rw---R--,values=2,min=0,max=45,step=0
  : values=45,45
  | dBscale-min=-90.00dB,step=2.00dB,mute=1

Note the name inside the .tplg file is NOT truncated.

marc-hb · 2024-05-15T23:57:43Z

volume-basic test results look good, this is ready.

plbossart

way beyond my comfort zone....

plbossart · 2024-05-17T16:51:20Z

tools/topo_vol_kcontrols.py

+    volume_kcontrols = [
+        kc
+        for kc in gain_block.kcontrols
+        if kc.hdr.type == DapmType.MIXER.name and kc.body.max != 1


indentation doesn't look like usual python, is this intentional?

python list comprehension @plbossart

python list comprehension

Yes.

This new script was formatted with black; recommended. Warning: black formats in place, make a backup first if needed.

plbossart · 2024-05-17T16:52:21Z

tools/topo_vol_kcontrols.py

+        prv.elems
+        for prv in widget.priv
+        if prv.elems[0].token
+        == SofVendorToken.SOF_TKN_COMP_NO_WNAME_IN_KCONTROL_NAME.name


same here, indentation is probably something intentional but someone like me will never figure out what this does.

List comprehension here too. This is just building a list from another, filtered list.

plbossart · 2024-05-17T16:54:05Z

tools/tplgtool2.py

@@ -1330,6 +1330,7 @@ def main():
        errors = 0
        for f in files:
            tplg = GroupedTplg(tplgFormat.parse_file(f))
+            assert set(dump_types) <= set(supported_dump), f"unsupported type in {dump_types}"


no idea what this does.

Before this commit, a typo in the command-line argument was silently producing no output.

plbossart · 2024-05-17T16:57:18Z

jf-lnlm-rvp-sdw-1 seems to be in bad shape @fredoh9, was the BIOS revert applied?

[  233.238872] kernel: soundwire sdw-master-0-2: Controller Timed out on bank switch
[  233.238996] kernel: soundwire sdw-master-0-2: multi link bank switch failed: -110

tools/topo_vol_kcontrols.py

fredoh9 · 2024-05-17T18:10:20Z

jf-lnlm-rvp-sdw-1 seems to be in bad shape @fredoh9, was the BIOS revert applied?
[  233.238872] kernel: soundwire sdw-master-0-2: Controller Timed out on bank switch
[  233.238996] kernel: soundwire sdw-master-0-2: multi link bank switch failed: -110

I did and run-all-tests.sh tested. I will move it to maintenance mode and double check again.

marc-hb · 2024-05-17T18:20:20Z

SOFCI TEST

marc-hb

way beyond my comfort zone....

Thanks for taking a look anyway! In theory, you shouldn't need to be proficient in Python to at least follow most of what is going on. If you cannot, then the functions and variable names are probably not good enough.

jf-lnlm-rvp-sdw-1 seems to be in bad shape @fredoh9, was the BIOS revert applied?

https://sof-ci.01.org/softestpr/PR1193/build431/devicetest/index.html was an OLD run!

I'm re-running tests now (I was waiting for daily runs to finish)

tools/topo_vol_kcontrols.py

marc-hb · 2024-05-17T18:15:53Z

tools/topo_vol_kcontrols.py

+    volume_kcontrols = [
+        kc
+        for kc in gain_block.kcontrols
+        if kc.hdr.type == DapmType.MIXER.name and kc.body.max != 1


python list comprehension

Yes.

This new script was formatted with black; recommended. Warning: black formats in place, make a backup first if needed.

marc-hb · 2024-05-17T18:16:48Z

tools/topo_vol_kcontrols.py

+        prv.elems
+        for prv in widget.priv
+        if prv.elems[0].token
+        == SofVendorToken.SOF_TKN_COMP_NO_WNAME_IN_KCONTROL_NAME.name


List comprehension here too. This is just building a list from another, filtered list.

marc-hb · 2024-05-17T18:19:21Z

tools/tplgtool2.py

@@ -1330,6 +1330,7 @@ def main():
        errors = 0
        for f in files:
            tplg = GroupedTplg(tplgFormat.parse_file(f))
+            assert set(dump_types) <= set(supported_dump), f"unsupported type in {dump_types}"


Before this commit, a typo in the command-line argument was silently producing no output.

tplgtool2.py: do not silently ignore unsupported "dump type" args

3eaebc6

No typo left behind. Signed-off-by: Marc Herbert <[email protected]>

marc-hb mentioned this pull request May 13, 2024

volume-basic-test.sh: don't skip "playback volume" controls #1068

Closed

marc-hb force-pushed the tplg-vol branch 2 times, most recently from b1af0ab to 5a4cf38 Compare May 14, 2024 00:53

marc-hb marked this pull request as ready for review May 14, 2024 02:13

marc-hb requested a review from a team as a code owner May 14, 2024 02:13

marc-hb marked this pull request as draft May 14, 2024 02:13

marc-hb commented May 14, 2024

View reviewed changes

marc-hb added 3 commits May 14, 2024 23:17

tplgtool2.py: add missing SOF_TKN_COMP_NO_WNAME_IN_KCONTROL_NAME

ac8d62a

Synchronize with Linux commit 246b5b77fc74 ("ASoC: SOF: topology: Add a token for dropping widget name in kcontrol name") Signed-off-by: Marc Herbert <[email protected]>

Add new tools/topo_vol_kcontrols.py

09df371

Will be used in next commit to fix thesofproject#1068. See also discussion in earlier attempt thesofproject#1068.

marc-hb force-pushed the tplg-vol branch from 5a4cf38 to 1dce3d7 Compare May 14, 2024 23:38

marc-hb marked this pull request as ready for review May 15, 2024 00:39

marc-hb requested a review from kv2019i May 16, 2024 14:14

plbossart reviewed May 17, 2024

View reviewed changes

ranj063 reviewed May 17, 2024

View reviewed changes

tools/topo_vol_kcontrols.py Show resolved Hide resolved

marc-hb commented May 17, 2024

View reviewed changes

marc-hb requested a review from jsarha May 17, 2024 18:31

ranj063 approved these changes May 18, 2024

View reviewed changes

marc-hb merged commit 82b4831 into thesofproject:main May 20, 2024
5 of 7 checks passed

marc-hb deleted the tplg-vol branch May 20, 2024 14:50

marc-hb mentioned this pull request May 29, 2024

[WIP][RFC] Test-case: Set PGAs to unity gain in script check-alsabat.sh #437

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

volume-basic-test.sh: switch to .tplg parsing to test topologies v2 #1193

volume-basic-test.sh: switch to .tplg parsing to test topologies v2 #1193

marc-hb commented May 13, 2024 •

edited

Loading

marc-hb commented May 14, 2024

plbossart commented May 14, 2024

marc-hb commented May 14, 2024 •

edited

Loading

marc-hb left a comment

marc-hb May 14, 2024

marc-hb commented May 14, 2024

plbossart commented May 14, 2024

marc-hb commented May 15, 2024 •

edited

Loading

marc-hb commented May 15, 2024 •

edited

Loading

plbossart left a comment

plbossart May 17, 2024

ranj063 May 17, 2024

marc-hb May 17, 2024 •

edited

Loading

plbossart May 17, 2024

marc-hb May 17, 2024

plbossart May 17, 2024

marc-hb May 17, 2024

plbossart commented May 17, 2024

fredoh9 commented May 17, 2024

marc-hb commented May 17, 2024

marc-hb left a comment

marc-hb May 17, 2024 •

edited

Loading

marc-hb May 17, 2024

marc-hb May 17, 2024

volume-basic-test.sh: switch to .tplg parsing to test topologies v2 #1193

volume-basic-test.sh: switch to .tplg parsing to test topologies v2 #1193

Conversation

marc-hb commented May 13, 2024 • edited Loading

marc-hb commented May 14, 2024

plbossart commented May 14, 2024

marc-hb commented May 14, 2024 • edited Loading

marc-hb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb commented May 14, 2024

plbossart commented May 14, 2024

marc-hb commented May 15, 2024 • edited Loading

marc-hb commented May 15, 2024 • edited Loading

plbossart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plbossart commented May 17, 2024

fredoh9 commented May 17, 2024

marc-hb commented May 17, 2024

marc-hb left a comment

Choose a reason for hiding this comment

marc-hb May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb commented May 13, 2024 •

edited

Loading

marc-hb commented May 14, 2024 •

edited

Loading

marc-hb commented May 15, 2024 •

edited

Loading

marc-hb commented May 15, 2024 •

edited

Loading

marc-hb May 17, 2024 •

edited

Loading

marc-hb May 17, 2024 •

edited

Loading