Tools: Testbench: Add to IPC4 version UUID based components load, apply initial byte control, etc. #9542

singalsu · 2024-10-03T13:15:35Z

To build and run IPC4 topologies, this needs PRs #9551 (already merged #9536, #9538). #9501 is needed if the topology is not containing blobs for byte controls (usually they do).

lgirdwood · 2024-10-03T14:52:20Z

tools/testbench/include/testbench/utils.h

@@ -52,6 +55,27 @@ struct tb_config {
 	int channels;
 	unsigned long format;
 };
+
+struct tb_ctl {


Can we include the ABI header rather than duplicate ? I guess this comes from ALSA asoc.h ?

I'm not sure, this struct is a direct copy from SOF plugin. If we change, we should change both to be able to merge the sources later to a common library like tplg_parser. What do you think @ranj063 ?

ok, so we can do this in parallel with plugin when we ship it.

lgirdwood · 2024-10-03T14:53:09Z

src/audio/src/src_ipc.h

+
+/* src component private data */
+struct ipc4_config_src {
+	struct ipc4_base_module_cfg base;


Should we have size at the start ?

This is not the struct for config blobs. I only moved this structure to be separate from large src_common.h that caused a build problem.

ok, but this is an IPC header, so this should be for host/module configuration.

It's not clear to me what you are suggesting to do. Should these IPC headers be somewhere else or use some more consistent directory locations and naming?

@lgirdwood Ping, can you help me fix this. I don't think this PR has any other opens left.

ok, we can address this later as I see this structure is used as header for module data in other modules.

singalsu · 2024-10-04T07:52:24Z

src/audio/src/src_ipc.h

+
+/* src component private data */
+struct ipc4_config_src {
+	struct ipc4_base_module_cfg base;


This is not the struct for config blobs. I only moved this structure to be separate from large src_common.h that caused a build problem.

singalsu · 2024-10-04T07:54:08Z

tools/testbench/include/testbench/utils.h

@@ -52,6 +55,27 @@ struct tb_config {
 	int channels;
 	unsigned long format;
 };
+
+struct tb_ctl {


I'm not sure, this struct is a direct copy from SOF plugin. If we change, we should change both to be able to merge the sources later to a common library like tplg_parser. What do you think @ranj063 ?

singalsu · 2024-10-04T07:56:51Z

tools/testbench/topology_ipc4.c

+					   VOLUME_FWL);
+
+	return linear_gain;
+}


@ranj063 These volume tables math functions would be a good start for a common utils library. Maybe there could be a place in tplg_parser?

singalsu · 2024-10-08T08:23:21Z

I added to controls parsing commit capability to split a long bytes control blob to several IPC messages if it is large. It's useful for FIR and TDFB blobs.

tools/testbench/utils_ipc4.c

singalsu · 2024-10-10T09:04:25Z

tools/testbench/topology_ipc4.c

+				 * sof_ipc4_prepare_process_module().
+				 */
+				memcpy(&pin_format->audio_fmt, &base_cfg->audio_fmt,
+				       sizeof(pin_format->audio_fmt));


Note: After talking with @ranj063 I now understand resolving this properly need another prepare() pipelines walk step that does not yet exist in testbench and plugin. I'm proposing to keep this as-is for now and wait for development in plugin where things are more constrained by multiple processes. This workaround works for most components where first pin format for input and output is the same.

singalsu · 2024-10-10T09:05:20Z

SOFCI TEST

singalsu · 2024-10-10T09:12:03Z

tools/tplg_parser/audio_formats.c

@@ -68,6 +68,13 @@ static const struct sof_topology_token ipc4_out_audio_format_tokens[] = {
 	 offsetof(struct sof_ipc4_pin_format, buffer_size)},
 };

+static const struct sof_topology_token ipc4_comp_pin_tokens[] = {


@ranj063 Should I keep this patch here or make a separate PR if you need it? It might take long to get this testbench PR approved.

kv2019i

A handful of style/readability improvements (see inline), but really nothing to stop merging.

kv2019i · 2024-10-10T11:57:56Z

tools/testbench/topology_ipc4.c

@@ -643,18 +646,19 @@ int tb_new_dai_in_out(struct testbench_prm *tp, int dir)
 	struct tplg_context *ctx = &tp->tplg;
 	struct tplg_comp_info *comp_info = ctx->current_comp_info;
 	struct ipc4_file_module_cfg *file;
+	struct sof_uuid file_uuid = {.a = 0xbfc7488c, .b = 0x75aa, .c = 0x4ce8,
+				     .d = { 0x9d, 0xbe, 0xd8, 0xda, 0x08, 0xa6, 0x98, 0xc2 }};


Given this doesn't look super nice and is repeated many times in testbench code, would be nice to at least define this in one place.

Yep, moved to file.h.

tools/testbench/include/testbench/topology_ipc4.h

kv2019i · 2024-10-10T12:15:49Z

tools/testbench/topology_ipc4.c

+	int i, j;
+	int pin_format_offset = 0;
+
+	if (pin_type == 0) {


I find these make it harder to read, can you add a enum for 0/1?

kv2019i · 2024-10-10T12:16:20Z

tools/testbench/topology_ipc4.c

+		base_cfg_ext->nb_input_pins = pins->num_input_pins;
+		base_cfg_ext->nb_output_pins = pins->num_output_pins;
+		tb_set_pin_formats(comp_info, base_cfg, base_cfg_ext, 0);
+		tb_set_pin_formats(comp_info, base_cfg, base_cfg_ext, 1);


Here readable enum/defines for 0/1 would help to understand what this code does easier.

This patch copies similar change from SOF plugin. The load of host copier, DAI copier, PGA, and process components are updated to use UUID that is appended to end of IPC4 base module configuration and other component specific init IPC. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This adds parse of controls and set of byte control to components after initialize. Control types mixer, switch, enum are parsed but not yet applied to components. The code is copied and adapted from SOF plugin. As addition the bytes control is split to several messages if the IPC message size limit is reached. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

If kept in src_common.h the inclusion to tplg_parser would create need to include even more headers from SOF. The simple src_ipc.h contains the only needed SRC IPC4 definition. The __SOF_AUDIO_SRC_SRC_H__ is updated to match header name and the ending #endif is moved file end where it should be. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This replaces for IPC4 testbench the previous non-working version that was a copy of IPC3 version. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

The examples need to convert the testbench out.raw to wav format, not the converted input for testbench. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch adds to the parser the capability to parse the tokens and place the found values to pins_info member in tplg_comp_info struct. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

The input and output pins count and format for each pin is added into the "struct ipc4_base_module_cfg_ext" that is placed after "struct ipc4_base_module_cfg" and before test bench and plugin specific UUID. This allows to load components those use the extended configuration. Since there is no harm to apply it to every process type component it done simply for all. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

singalsu · 2024-10-15T11:03:24Z

I updated commit "Tools: Testbench: Add support for parsing controls from topology " to fix xt-testbench build by moving all struct definitions in tb_kcontrol_cb_new() to begin of function.

lgirdwood · 2024-10-15T12:39:57Z

tools/testbench/include/testbench/utils.h

@@ -52,6 +55,27 @@ struct tb_config {
 	int channels;
 	unsigned long format;
 };
+
+struct tb_ctl {


ok, so we can do this in parallel with plugin when we ship it.

lgirdwood · 2024-10-15T12:48:12Z

src/audio/src/src_ipc.h

+
+/* src component private data */
+struct ipc4_config_src {
+	struct ipc4_base_module_cfg base;


ok, we can address this later as I see this structure is used as header for module data in other modules.

lgirdwood · 2024-10-15T12:51:26Z

@wszypelt @lrudyX good to merge ? Not expecting this is tested yet by Internal CI ?

wszypelt · 2024-10-15T14:04:36Z

@lgirdwood good to merge

singalsu marked this pull request as ready for review October 3, 2024 13:43

singalsu requested review from ranj063, abonislawski, lgirdwood, plbossart, mmaka1, lbetlej, dbaluta and kv2019i as code owners October 3, 2024 13:43

lgirdwood reviewed Oct 3, 2024

View reviewed changes

singalsu commented Oct 4, 2024

View reviewed changes

singalsu force-pushed the testbench_ipc4_uuid_bytecontrol branch from afdf2fa to 7cd25e1 Compare October 8, 2024 08:21

singalsu commented Oct 8, 2024

View reviewed changes

tools/testbench/utils_ipc4.c Show resolved Hide resolved

singalsu force-pushed the testbench_ipc4_uuid_bytecontrol branch from 7cd25e1 to 40280c9 Compare October 9, 2024 16:51

singalsu commented Oct 10, 2024

View reviewed changes

kv2019i approved these changes Oct 10, 2024

View reviewed changes

singalsu force-pushed the testbench_ipc4_uuid_bytecontrol branch from 40280c9 to aa9bc6c Compare October 10, 2024 16:33

singalsu added 8 commits October 15, 2024 13:51

Tools: Testbench: Fix SRC component load

a768eba

This replaces for IPC4 testbench the previous non-working version that was a copy of IPC3 version. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Tools: Testbench: Fix ASRC component load

c6a0b84

This replaces for IPC4 testbench the previous non-working version that was a copy of IPC3 version. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Tools: Testbench: Fix typos in README.md

d9d360e

The examples need to convert the testbench out.raw to wav format, not the converted input for testbench. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Tools: Tplg_parser: Add parse of input and output pins count

14933a4

This patch adds to the parser the capability to parse the tokens and place the found values to pins_info member in tplg_comp_info struct. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

singalsu force-pushed the testbench_ipc4_uuid_bytecontrol branch from aa9bc6c to cb5946e Compare October 15, 2024 11:01

lgirdwood approved these changes Oct 15, 2024

View reviewed changes

lgirdwood merged commit 89c0d2b into thesofproject:main Oct 15, 2024
42 of 47 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tools: Testbench: Add to IPC4 version UUID based components load, apply initial byte control, etc. #9542

Tools: Testbench: Add to IPC4 version UUID based components load, apply initial byte control, etc. #9542

singalsu commented Oct 3, 2024 •

edited

Loading

lgirdwood Oct 3, 2024

singalsu Oct 4, 2024

lgirdwood Oct 15, 2024

lgirdwood Oct 3, 2024

singalsu Oct 4, 2024

lgirdwood Oct 4, 2024

singalsu Oct 10, 2024

singalsu Oct 15, 2024

lgirdwood Oct 15, 2024

singalsu Oct 4, 2024

singalsu Oct 4, 2024

singalsu Oct 4, 2024

singalsu commented Oct 8, 2024

singalsu Oct 10, 2024

singalsu commented Oct 10, 2024

singalsu Oct 10, 2024

kv2019i left a comment

kv2019i Oct 10, 2024

singalsu Oct 10, 2024

kv2019i Oct 10, 2024

singalsu Oct 10, 2024

kv2019i Oct 10, 2024

singalsu commented Oct 15, 2024

lgirdwood Oct 15, 2024

lgirdwood Oct 15, 2024

lgirdwood commented Oct 15, 2024

wszypelt commented Oct 15, 2024

Tools: Testbench: Add to IPC4 version UUID based components load, apply initial byte control, etc. #9542

Tools: Testbench: Add to IPC4 version UUID based components load, apply initial byte control, etc. #9542

Conversation

singalsu commented Oct 3, 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

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

singalsu commented Oct 8, 2024

Choose a reason for hiding this comment

singalsu commented Oct 10, 2024

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

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

singalsu commented Oct 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgirdwood commented Oct 15, 2024

wszypelt commented Oct 15, 2024

singalsu commented Oct 3, 2024 •

edited

Loading