topology2: Mod cfg ibs obs cpc addition

[jsarha]

I am feeling quite clueless here, so even if this does not work, I push this out in hope someone can spot the problem.

For some reason the widget tuple array only contains the first instance of mod_cfg objects in the topology2 conf file. I am trying to read the array out with this code: https://github.com/jsarha/linux-sof/tree/peter-cpc-test

[jsarha]

@ranj063 can help me out here?

[ranj063]

I am feeling quite clueless here, so even if this does not work, I push this out in hope someone can spot the problem.

For some reason the widget tuple array only contains the first instance of mod_cfg objects in the topology2 conf file. I am trying to read the array out with this code: https://github.com/jsarha/linux-sof/tree/peter-cpc-test

@jsarha the tplg is fine. but what you are missing is the parsing bits in the kernel. Take a look at how we parset multiple audio formats here: https://github.com/thesofproject/linux/blob/bef9852fe3228ce83717dada3ebcb7ad07c6099b/sound/soc/sof/topology.c#L1238

You need to allocate memory for "n" number of token sets first. And then parse the number of sets you're expecting like https://github.com/thesofproject/linux/blob/bef9852fe3228ce83717dada3ebcb7ad07c6099b/sound/soc/sof/ipc4-topology.c#L221 and then finally parse the tokens like https://github.com/thesofproject/linux/blob/bef9852fe3228ce83717dada3ebcb7ad07c6099b/sound/soc/sof/ipc4-topology.c#L257. Hope this helps

[jsarha]

This should now be good. This introduces the mod_cfg class for all relevant widgets. However, this pr does not set any mod_cfg:s as such.

[ujfalusi]

@lgirdwood, I have my own variant of this (looks similar and I think @jsarha based this on my wip). fwiw, this is how I set it for one module instance for testing:

diff --git a/tools/topology/topology2/include/pipelines/cavs/mixout-gain-dai-copier-playback.conf b/tools/topology/topology2/include/pipelines/cavs/mixout-gain-dai-copier-playback.conf
index 48a9d62938c2..ec2e31a72d76 100644
--- a/tools/topology/topology2/include/pipelines/cavs/mixout-gain-dai-copier-playback.conf
+++ b/tools/topology/topology2/include/pipelines/cavs/mixout-gain-dai-copier-playback.conf
@@ -17,6 +17,7 @@
 #
 
 <include/common/audio_format.conf>
+<include/common/mod_cfg.conf>
 <include/components/copier.conf>
 <include/components/gain.conf>
 <include/components/mixout.conf>
@@ -76,6 +77,7 @@ Class.Pipeline."mixout-gain-dai-copier-playback" {
 			num_audio_formats 2
 			num_input_audio_formats 2
 			num_output_audio_formats 2
+			num_mod_cfgs 3
 
 			# 32-bit 48KHz 2ch
 			Object.Base.audio_format.1 {
@@ -91,6 +93,24 @@ Class.Pipeline."mixout-gain-dai-copier-playback" {
 				out_bit_depth		32
 				out_valid_bit_depth	32
 			}
+
+			Object.Base.mod_cfg.1 {
+				ibs 192
+				obs 256
+				cpc 1456
+			}
+
+			Object.Base.mod_cfg.2 {
+				ibs 384
+				obs 384
+				cpc 4242
+			}
+
+			Object.Base.mod_cfg.3 {
+				ibs 432
+				obs 256
+				cpc 2566
+			}
 		}
 
 		pipeline."1" {

[lgirdwood]

@jsarha @ujfalusi @ranj063 we need to have a per HW target per module file for each module i.e.

cavs/tgl/copier.conf
ace/mtl/copier.conf

and so on that we would include in teh high level generic pipeline files.

[jsarha]

@jsarha @ujfalusi @ranj063 we need to have a per HW target per module file for each module i.e.

cavs/tgl/copier.conf
ace/mtl/copier.conf

and so on that we would include in teh high level generic pipeline files.

I still have some trouble in understanding the best ways to implement things in topology2, but I would like much more to have one place were we can define ibs/obs/cpc configurations for different platforms, than duplicating the module definitions for it. Could we have something like this:

Class.Widget."copier" { # # Pipeline ID for the copier object # DefineAttribute."index" {}

    IncludeByKey.PLATFORM {
     "mtl"	"platform/intel/mtl-copier-mod-cfgs.conf"
     "tgl"	"platform/intel/tgl-copier-mod-cfgs.conf"
    }

---

I have no idea if this would work, but it would be nice if it did 😅. @ranj063 can say if this approach could be made to work?

[sys-pt1s]

Can one of the admins verify this patch?

[kv2019i]

@jsarha Status? Please mark as draft if this is not intended for merging...

[jsarha]

@jsarha Status? Please mark as draft if this is not intended for merging...

This PR is good as such, but it does not contain the structures requested here: https://github.com/thesofproject/sof/pull/7348#issuecomment-1497387922

They could be implemented as separate PR. But maybe I'll mark this as draft as you requested and try to put all the pieces together. Then again I have no idea of any mod_cfg settings for any platform, so in the end there will not be any settings in the later PR either, just places for different platforms to put them.

[kv2019i]

@jsarha So ack, so maybe we need to push this forward then. https://github.com/thesofproject/linux/pull/4315 is close to merging on driver side, so adding @ujfalusi and @ranj063 to review.

[jsarha]

Actually, I am not so sure about the second commit in this series. Maybe its better to include the mod_cfg.conf already in the component files (e.g. in include/component/*.conf), and not in the pipeline definitions. I already have an alternative commit: https://github.com/thesofproject/sof/commit/ab8538cb37559fb477d1e5c8878d634b71cbf68f
@ranj063 , this is better, isn't it?

[ranj063]

Actually, I am not so sure about the second commit in this series. Maybe its better to include the mod_cfg.conf already in the component files (e.g. in include/component/*.conf), and not in the pipeline definitions. I already have an alternative commit: ab8538c @ranj063 , this is better, isn't it?

@jsarha yes adding it to the widget class definitions where they will be instantaited and used is probably better

[jsarha]

New version pushed. This version should have what was requested in https://github.com/thesofproject/sof/pull/7348#issuecomment-1497387922 . Please check that I have added the placeholder and included it in actual component widgets. I already removed pipeline from them, but for many of them I have no idea what those entities are.

[jsarha]

New version pushed. This version should have what was requested in #7348 (comment) . Please check that I have added the placeholder and included it in actual component widgets. I already removed pipeline from them, but for many of them I have no idea what those entities are.

Hmm. The last commit does not work. I do not know if there is a way to instantiate mod_cfg objects already in widget class definition.

[jsarha]

New version pushed. This version should have what was requested in #7348 (comment) . Please check that I have added the placeholder and included it in actual component widgets. I already removed pipeline from them, but for many of them I have no idea what those entities are.

Hmm. The last commit does not work. I do not know if there is a way to instantiate mod_cfg objects already in widget class definition.

Nope, this indeed does work. But only for targets that have PLATFROM variable set at compiler line. Generic targets, like sof-hda-generic.tplg, do not have any mod_cfg settings, but how could they, if the platform is not know at compile time. So, AFAICT this PR should be good.

[ranj063]

@jsarha conflicts?

[jsarha]

@jsarha conflicts?

Solved now.

[jsarha]

Pushed forwards, and changes requested in mail, so turning to draft again.

Girdwood, Liam R:

Jyri, a topology override must copy the manifest CPC schema exactly. Even the unused fields must be copied and populated with 0 today.

[lgirdwood]

@jsarha now that the manifest and kernel CPC parts are working we should add the overide option for v2.7. @ujfalusi @plbossart fyi.

[plbossart]

@jsarha now that the manifest and kernel CPC parts are working we should add the overide option for v2.7. @ujfalusi @plbossart fyi.

We need to agree on the definition of 'working'. I still routinely see messages telling me that CPC is not set ?

[lgirdwood]

@jsarha now that the manifest and kernel CPC parts are working we should add the overide option for v2.7. @ujfalusi @plbossart fyi.

We need to agree on the definition of 'working'. I still routinely see messages telling me that CPC is not set ?

There are still a lot of holes in the manifest CPC data for each module and (format, rate, channels) configuration. @aiChaoSONG will be working on populating this prior to v2.7. This should fix the kernel warning messages for all tested configs.

The toplogy part will use the same CPC schema and kernel logic/IPC, but will allow

1. Adding a missing CPC data element if one is missing in manifest for a new configuration.
2. Overide an existing manifest CPC data element if needed.

[plbossart]

I'd like to reach the point where all CPC data is valid in the manifest and used for something meaningful, before we plan work on overriding stuff.

[lgirdwood]

I'd like to reach the point where all CPC data is valid in the manifest and used for something meaningful, before we plan work on overriding stuff.

Need to do both in parallel, to be ready for mistakes in manifest CPC.

[plbossart]

I'd like to reach the point where all CPC data is valid in the manifest and used for something meaningful, before we plan work on overriding stuff.

Need to do both in parallel, to be ready for mistakes in manifest CPC.

we can't afford to hedge our bets for every single feature. The firmware team is responsible for providing correct CPC values, if they don't it's a bug and they fix it. We have lots of other things to do really with SoundWire-related enablement.

[lgirdwood]

I'd like to reach the point where all CPC data is valid in the manifest and used for something meaningful, before we plan work on overriding stuff.

Need to do both in parallel, to be ready for mistakes in manifest CPC.

we can't afford to hedge our bets for every single feature. The firmware team is responsible for providing correct CPC values, if they don't it's a bug and they fix it. We have lots of other things to do really with SoundWire-related enablement.

CPC architecture is based on a single module unit test with no upstream/downstream module I$/D$ impact taken into consideration for module CPC performance delta when different pipeline topologies are used. This is not a big item and will be painful to modify/fix in manifest.

[kv2019i]

In draft status, pushing to v2.9.

[kv2019i]

Stable-v2.9 branched, this didn't make the cut, bumping forward. @jsarha is this still valid (given bumps over multiple releases)?

[kv2019i]

No activity in 2.7...2.9, clearing the milestone.