Repository navigation
[FEATURE] Clean-up topology2/cavs to make it user-friendly and maintainable #6576
Description
The topology2 framework allows for code reuse, modularity, macros and arithmetic to determine settings.
Sadly, our existing cavs topologies are 'developer' level and a number of ground rules were forgotten.
- all controls and PCM names SHALL have a meaningful name.
"volume playback 1' is not acceptable, each PGA and control shall be clearly identified with a pipeline information and position in the pipeline.
The use of 'master' SHALL be revisited. There SHALL not be more than one 'master' control on a path to an endpoint.
when possible, all PCM and control names SHALL be backwards compatible with existing UCM files. Don't be creative.
-
topology2/cavs files SHALL have the same level of modularity as topology1. We know e.g. for HDaudio/SoundWire/I2S that we'll have tons of variations (jack, no jack, echo reference, one amp, 2 amps, BT, HDMI with 3/4 streams). We have to track and make sure all variability in topology1 is supported in topology2
-
the indices used for pipelines and components matter. in topology1 we could easily identify when a pipeline was connected to SSP0. This information is lost in topology2. so the rule is that just by using at the topology viewer, anyone SHALL be able to know which PCM device to use to play on a specific DAI.
Example of poor index management can be seen on this topology:
copier.host.1.1 is not related to the PCM index.
copier.SSP.12.1 is not related to the DAI index.
All modules use '.1' at the end except the copier.module.8.2. It's unclear what all these 1s mean, and which we need a different convention for a copier?
- all generated topology2 files SHALL be named with the conventions used in the kernel. A topology file that is not known to the kernel SHALL be moved to a developer directory. (Done)
Examples of bad names: cavs-mtl-sdw-dmic cavs-sdw-src-gain-mixin cavs-mixin-mixout-hda-2ch-tgl cavs-tgl-nocodec cavs-passthrough-hdmi
Examples of good names: sof-mtl-nocodec
That's actually the only good name in the entire set of topology2 files....
Common sense really.
Activity
plbossart commentedon Nov 10, 2022
@lgirdwood @ranj063 @bardliao @marc-hb @yongzhi1 FYI
marc-hb commentedon Nov 10, 2022
I think examples would be great. Pointers to PRs fixing this sort of issues even better.
ranj063 commentedon Nov 14, 2022
@plbossart this is the first clean up to build the right binaries in the right folder #6592
ranj063 commentedon Nov 17, 2022
#6627 addresses the third bullet in this requirement
ranj063 commentedon Nov 17, 2022
#6592 is for the 4th
ranj063 commentedon Dec 19, 2022
@plbossart Is the position in the pipeline really necessary? We do not have a case where we have more than one volume component in the same pipeline. And what should the protocol be for naming. " Volume " where Direction will be playback/capture OK?
plbossart commentedon Dec 19, 2022
What we agreed is that the pipeline ID is not needed for the host/dai, but will be present for all other components - with the pipeline ID + instance ID. This is different to the position in the pipeline as written above, that is clearly not a good idea in hindsight. I insist that we need to have something else than the pipeline ID for other components, some unique ID such as the instance ID is necessary. We could have a gain that prevents saturation before a processing and compensation later on, it's classic for fixed-point processing
marc-hb commentedon Dec 20, 2022
If a name is wrong, validation MUST NOT silently rename it and "paint" the test results green and leave customers stuck with a non-working system. The purpose of validation is to find bugs before customers do, not to hide them.
If a name is wrong, tests must do their job and FAIL in order to report the naming issue until it is fixed. It's just a file name: hard to agree but very easy to fix after agreement.
https://martinfowler.com/bliki/TwoHardThings.html
ranj063 commentedon Dec 20, 2022
OK then what convention do you suggest for the mixer name in this case?
<Direction> Volume <Pipeline ID> <Instance ID>lgirdwood commentedon Dec 20, 2022
@ranj063 @jsarha pls update the status check boxes for what has been merged etc. Thanks
plbossart commentedon Dec 21, 2022
Not following the question @ranj063, sorry
2 remaining items
ranj063 commentedon Dec 21, 2022
hmm I wonder where the "gain.0.1" or "gain.1.1" is coming from. Looks like they are the widget names which have the mixer. Is this something alsatplg appends automatically?
plbossart commentedon Dec 21, 2022
no idea where this comes from @ranj063, but the topology was sof-tgl-rt711-rt1316-rt714.tplg so it's probably faster if you look at the conf file and how the names are created.
kv2019i commentedon Mar 9, 2023
Two days left before branch, I think we need to move this to v2.6? @jsarha @plbossart @lgirdwood ?
jsarha commentedon Mar 10, 2023
There is one merged PR [1] and couple of more PRs [2] ready for review, and after them there is finally all the tools ready to solve the first tic-box "all controls and PCM names SHALL have a meaningful name.", however I do not have the knowledge to come up with these meaningful names.
Then there is couple of more tic-boxes that have not even been started, so yes this wont get solved in two days.
[1] alsa-project/alsa-utils#189
[2] #7227
thesofproject/linux#4226
lgirdwood commentedon May 10, 2023
@jsarha @ranj063 @plbossart on track for v2.6 ?
ranj063 commentedon May 17, 2023
#7631, #7640 and #7670 complete the third bullet
alex-cri commentedon May 30, 2023
@plbossart, with PR #7682 merged is there anything further required before closing this bug?
ranj063 commentedon May 30, 2023
@alex-cri 2 more pending #7694 and #7704. And I think @jsarha has a couple of more coming for the chrome topology and the sdw topologies