topology2 : add DTS topology and changes for MTL #8378

joechengxperi · 2023-10-24T06:32:54Z

Add DTS topology and related changes for MTL with rt5682. Will enable DTS later until rt5650 is ready.

lgirdwood

LGTM, just a question about index.

lgirdwood · 2023-10-25T15:25:02Z

tools/topology/topology2/cavs-mixin-mixout-eqiir-dts-ssp.conf

+	# playback pipelines
+	host-copier-gain-mixin-playback [
+		{
+			index	1


@jsarha @bardliao @plbossart do we still need index, IIRC we were (planning?) removing index from topology to be automatic.

@lgirdwood yes it is still needed. There's no plan to make it automatic right now because widgets are identified with the index for top-level routes.

ranj063 · 2023-10-25T16:34:50Z

tools/topology/topology2/cavs-mixin-mixout-ssp.conf

+		}
+	]
+
+        mixout-gain-dai-copier-playback [


this seems a bit too much duplication. Can we simplify this by moving the mixout-gain to a separate pipeline and connecting to the DAI copier at the top-level first? And then add a new pipeline with just the eq-iir->dts and insert it only when needed along with the new routes at the top-level?

I have one question. For "insert" you mentioned, do you mean using IncludeByKey to choose different pipeline or route at the top-level?

Ack, I think @ranj063 was referring to IncludeByKey here.

kv2019i

Ranjani and Jyri are the experts with a change like this, but a few comments from me as well. Mostly looks good, one smaller thing and then the question of how much existing definitions can be reused.

kv2019i · 2023-10-26T12:58:17Z

tools/topology/topology2/cavs-mixin-mixout-ssp.conf

+		}
+	]
+
+        mixout-gain-dai-copier-playback [


Ack, I think @ranj063 was referring to IncludeByKey here.

tools/topology/topology2/cavs-rt5682.conf

joechengxperi · 2023-10-27T13:28:57Z

Ranjani and Jyri are the experts with a change like this, but a few comments from me as well. Mostly looks good, one smaller thing and then the question of how much existing definitions can be reused.

host-copier-gain-mixin-playback is duplicated. For mixout-gain-dai-copier-playback and mixout-gain-eqiir-dts-dai-copier-playback, dai copier and gain are duplicated. This is the question Ranjani asked.

I roughly have an idea to use IncludeByKey to implement Ranjani's suggestion. The difficult part is to just insert eq iir -> dts only. But the structure might become a little bit complicate comparing to current patch having two mixout-gain pipeline.

And I'm curious about the importance to avoid duplication here. I had two versions of top-level topology at the beginning. The first one is more likely to follow cavs-nocodec.conf. And the second is to follow sof-hda-generic.conf. Comparing these two version, I think sof-hda-generic.conf is more easy to read and could add more playback pipelines easily if necessary. But the drawback is too much duplication.

tools/topology/topology2/include/components/dts.conf

btian1 · 2023-10-30T03:58:05Z

tools/topology/topology2/include/components/dts/dts_spk.conf

@@ -0,0 +1,148 @@
+Object.Base.data."dts_spk" {


add comments for what's table used for?

This table is the initial parameters to DTS library.

This looks like a full-spec config, which should be set from external rather than by default.
Why don't we use an empty config for default similar to topology1?

@johnylin76 Yes, this is the full config with default parameters for DTS library. To use empty config or not set the parameters lead to some problems here. Instead, to put the default parameters here works fine. We'll still need to set the parameters by UCM afterward since it varies by project.

The empty config I meant is the ABI-header-only blob, e.g. https://github.com/thesofproject/sof/blob/cavs2.5-001-drop-stable/tools/topology/topology1/m4/dts_codec_adapter.m4#L5

bytes " 0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,0x00,0xa0,0x01,0x03, 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00"

Have you tried this config and still led to problems?

I think we can set the full config here for developing purposes, but should not push this to upstream. Instead, a minimal and least effective config as default is preferred.

p.s. if the config above doesn't work, maybe try this instead:

bytes " 0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00, 0x0c,0x00,0x00,0x00,0x00,0xa0,0x01,0x03, 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,0x0c,0x00,0x00,0x00, 0x00,0x00,0x00,0x00"

which contains a 12-byte module config {paramId=0, paramSize=0xc, paramData=[0x0]}

@johnylin76
I tried both of them before. They didn't work. But I notice a minor difference here now that makes the second one work. I'll test it further and update it.

bytes " 0x53,0x4f,0x46,0x34,0x00,0x00,0x00,0x00, 0x0c,0x00,0x00,0x00,0x00,0xa0,0x01,0x03, 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, 0x01,0x00,0x00,0x00,0x0c,0x00,0x00,0x00, 0x00,0x00,0x00,0x00" '''

@johnylin76 Updated the table now.

The updated bytes will pass a param entry {paramId=1, paramSize=0xc, paramData=[0x0]} to DTS library. Given that paramId=1 is used in the full config, which should be the effective param ID for DTS library.

My previous idea is to pass {paramId=0, paramSize=0xc, paramData=[0x0]} which avoids the effective ID for DTS. However it sounds that errors are produced by paramId=0.

As long as DTS library has no side effect by setting {paramId=1, paramSize=0xc, paramData=[0x0]}, that looks good to me.

This commit adds DTS componenet and pipeline for topology2. 1. DTS component 2. A new pipeline to support EQ IIR + DTS processing dts_spk.conf includes the parameters for DTS SDK V1.1.3 Co-developed-by: Mac Chiang <[email protected]> Signed-off-by: Mac Chiang <[email protected]> Signed-off-by: Joe Cheng <[email protected]>

In order to adopt more different playback pipelines, refer to sof-hda-generic.conf and move playback pipeline in cavs-rt5682.conf to cavs-mixin-mixout-ssp.conf. Co-developed-by: Mac Chiang <[email protected]> Signed-off-by: Mac Chiang <[email protected]> Signed-off-by: Joe Cheng <[email protected]>

Add cavs-mixin-mixin-mixout-eqiir-dts-ssp.conf to support EQ IIR + DTS processing on rt5682 Co-developed-by: Mac Chiang <[email protected]> Signed-off-by: Mac Chiang <[email protected]> Signed-off-by: Joe Cheng <[email protected]>

johnylin76 · 2023-11-03T04:42:43Z

tools/topology/topology2/include/components/dts/dts_spk.conf

@@ -0,0 +1,148 @@
+Object.Base.data."dts_spk" {


The updated bytes will pass a param entry {paramId=1, paramSize=0xc, paramData=[0x0]} to DTS library. Given that paramId=1 is used in the full config, which should be the effective param ID for DTS library.

My previous idea is to pass {paramId=0, paramSize=0xc, paramData=[0x0]} which avoids the effective ID for DTS. However it sounds that errors are produced by paramId=0.

As long as DTS library has no side effect by setting {paramId=1, paramSize=0xc, paramData=[0x0]}, that looks good to me.

kv2019i · 2023-11-03T16:32:57Z

The ipc3-stub build failure is unrelated, proceeding with merge.

joechengxperi requested review from ranj063 and jsarha as code owners October 24, 2023 06:32

joechengxperi requested review from macchian, markbartonxperi, johnylin76 and cujomalainey October 24, 2023 06:33

joechengxperi force-pushed the main-dts-mtl-tplg-split-2-1 branch from e235b4f to f7bf6c4 Compare October 24, 2023 08:36

lgirdwood reviewed Oct 25, 2023

View reviewed changes

lgirdwood added this to the v2.8 milestone Oct 25, 2023

lgirdwood approved these changes Oct 25, 2023

View reviewed changes

ranj063 reviewed Oct 25, 2023

View reviewed changes

kv2019i reviewed Oct 26, 2023

View reviewed changes

btian1 reviewed Oct 30, 2023

View reviewed changes

joechengxperi force-pushed the main-dts-mtl-tplg-split-2-1 branch from f7bf6c4 to ab9b233 Compare November 3, 2023 02:15

joechengxperi added 2 commits November 3, 2023 11:16

topology2: dts: add DTS playback pipeline

bd7ddd6

Add cavs-mixin-mixin-mixout-eqiir-dts-ssp.conf to support EQ IIR + DTS processing on rt5682 Co-developed-by: Mac Chiang <[email protected]> Signed-off-by: Mac Chiang <[email protected]> Signed-off-by: Joe Cheng <[email protected]>

joechengxperi force-pushed the main-dts-mtl-tplg-split-2-1 branch from ab9b233 to bd7ddd6 Compare November 3, 2023 03:18

johnylin76 approved these changes Nov 3, 2023

View reviewed changes

kv2019i merged commit 244df9f into thesofproject:main Nov 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

topology2 : add DTS topology and changes for MTL #8378

topology2 : add DTS topology and changes for MTL #8378

joechengxperi commented Oct 24, 2023

lgirdwood left a comment

lgirdwood Oct 25, 2023

ranj063 Oct 25, 2023

ranj063 Oct 25, 2023 •

edited

Loading

joechengxperi Oct 26, 2023

kv2019i Oct 26, 2023

kv2019i left a comment

kv2019i Oct 26, 2023

joechengxperi commented Oct 27, 2023

btian1 Oct 30, 2023

joechengxperi Oct 30, 2023

johnylin76 Oct 31, 2023

joechengxperi Nov 1, 2023

johnylin76 Nov 1, 2023

joechengxperi Nov 1, 2023

joechengxperi Nov 3, 2023

johnylin76 Nov 3, 2023

johnylin76 Nov 3, 2023

kv2019i commented Nov 3, 2023

topology2 : add DTS topology and changes for MTL #8378

topology2 : add DTS topology and changes for MTL #8378

Conversation

joechengxperi commented Oct 24, 2023

lgirdwood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranj063 Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kv2019i left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joechengxperi commented Oct 27, 2023

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

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

kv2019i commented Nov 3, 2023

ranj063 Oct 25, 2023 •

edited

Loading