Audio: Module adapter: Add a new simple copy function #7792

singalsu · 2023-06-13T09:03:40Z

This patch adds a simplified version of simple copy for module adapter clients with one source and one sink. The overhead of discovering multiple source and sink buffers is avoided.

The module_adapter_bind() and module_adapter_unbind() functions are changed to detect multiple sources or sinks condition to control the used simple copy function.

src/audio/module_adapter/module_adapter.c

ranj063 · 2023-06-15T13:12:46Z

SOFCI TEST

src/audio/module_adapter/module_adapter.c

singalsu · 2023-06-20T11:08:12Z

The saving with this rebased pipeline2.0 adapted patch is 3.4 MCPS from sum of 10s playback average numbers

                            Name,        Mtrace id, avg MCPS, max MCPS
            host-copier.0.playback,   comp:0 0x40000,     3.39,    11.42
                          gain.1.1,   comp:0 0x60000,     3.28,     9.11
                         mixin.1.1,   comp:0 0x20000,     3.27,     3.45
                        mixout.2.1,   comp:1 0x30000,     2.59,     3.31
                          gain.2.1,   comp:1 0x60001,     3.37,    16.81
    dai-copier.HDA.Analog.playback,   comp:1 0x40001,     4.04,     4.28
                                  ,                 ,    19.95,    48.39

                              Name,        Mtrace id, avg MCPS, max MCPS
            host-copier.0.playback,   comp:0 0x40000,     3.40,     6.89
                          gain.1.1,   comp:0 0x60000,     2.34,     8.22
                         mixin.1.1,   comp:0 0x20000,     2.54,     2.64
                        mixout.2.1,   comp:1 0x30000,     1.74,     2.33
                          gain.2.1,   comp:1 0x60001,     2.44,     8.48
    dai-copier.HDA.Analog.playback,   comp:1 0x40001,     4.13,     4.51
                                  ,                 ,    16.60,    33.06

marcinszkudlinski

Rather minor - just propositions for new names, no pushing. Just please don't use sink/source :)

marcinszkudlinski · 2023-06-20T11:14:59Z

src/include/sof/audio/module_adapter/module/generic.h

@@ -184,6 +184,8 @@ struct processing_module {
 	struct output_stream_buffer *output_buffers;
 	uint32_t num_input_buffers; /**< number of input buffers */
 	uint32_t num_output_buffers; /**< number of output buffers */
+	struct comp_buffer *source; /**< single source buffer for multi_source_or_sink = false */


Please don't call it sink/source - may be confusions with struct sof_source / struct sof_sink interface

struct comp_buffer *source_comp_buffer;

or simiar

marcinszkudlinski · 2023-06-20T11:16:23Z

src/include/sof/audio/module_adapter/module/generic.h

+	/* False for module with one sink and one source to reduce module processing
+	 * overhead. True if multiple source or sink buffers.
+	 */
+	bool multi_source_or_sink;


bool simple_1_1_copy ?
to be consistent with module_adapter_audio_stream_copy_1to1 that is called when true

singalsu · 2023-06-21T11:10:38Z

The PR is updated according to @marcinszkudlinski 's proposals. The boolean is inverted with new name mod->stream_copy_single_to_single .

lyakh

just a couple of very minor comments

lyakh · 2023-06-22T06:22:46Z

src/audio/module_adapter/module_adapter.c

+	} else {
+		mod->source_comp_buffer = NULL;
+		mod->sink_comp_buffer = NULL;
+		mod->stream_copy_single_to_single = false;


.stream_copy_1to1?

I was thinking 1to1 looks too much like 1:1 that would give wrong impression about doing just memcpy() from buffer to buffer.

lyakh · 2023-06-22T06:27:05Z

src/audio/module_adapter/module_adapter.c

+	if (mod->input_buffers[0].consumed)
+		audio_stream_consume(&source_c->stream, mod->input_buffers[0].consumed);
+
+	/* produce data into all active output buffers */


there's just one output buffer, right?

singalsu · 2023-06-22T15:05:47Z

Previous CI round 43 successful and 2 failing checks, failed Internal Intel CI System/merge/build with West, failed
sof-ci/jenkins/pr-device-test/main-cavs2.5 with multiple suspend resume. New try...

btian1

seems Ranjani trying other method with your PR for #7850 #7858
This one should be fine for merge.

Just one concern, all our optimization is trying to figure out special case and use specific function to handle it, this have benefit for this special case, however, it does increase performance for worst case and code size, I am not clear why COE team must get those module performance low for this, if there is no low power(38.4mhz）mode, it is not must for handle module with multiple routines.

singalsu · 2023-06-26T08:25:36Z

seems Ranjani trying other method with your PR for #7850 #7858 This one should be fine for merge.

Just one concern, all our optimization is trying to figure out special case and use specific function to handle it, this have benefit for this special case, however, it does increase performance for worst case and code size, I am not clear why COE team must get those module performance low for this, if there is no low power(38.4mhz）mode, it is not must for handle module with multiple routines.

The previous version did not not enable the optimization, now it's using fix by @ranj063 to get the bind/unbind code to work correctly.

Yes, this is a best case optimization. But it's quite common that components have only one source and and one sink so I think this is worth it. It will be even better when pipeline 2.x let's components have linear shared buffers. Since this is small add there is not much code size overhead and it can be cleaned up when the shared buffers work is ready.

This patch adds a simplified version of audio stream copy for module adapter clients with one source and one sink. The overhead of discovering multiple source and sink buffers is avoided. The module_adapter_bind() and module_adapter_unbind() functions are changed to detect multiple sources or sinks condition to control the used simple copy function. The mod->source and mod->sink pointers to buffers are updated when there's only one source and sink. Signed-off-by: Ranjani Sridharan <[email protected]> Signed-off-by: Seppo Ingalsuo <[email protected]>

singalsu · 2023-06-27T10:06:30Z

42 successful and 3 failing checks

Zephyr / build-linux (zmain, imx8 imx8x imx8m) (pull_request)
sof-ci/jenkins/pr-device-test/main-ace --> verify-kernel-boot-log.sh, check-pause-resume-capture-100.sh
sof-ci/jenkins/pr-device-test/main-cavs2.5 --> check-suspend-resume-5.sh

singalsu · 2023-06-27T10:06:56Z

SOFCI TEST

singalsu · 2023-06-27T12:48:23Z

Now 43 successful and 2 failing checks

Zephyr / build-linux (zmain, imx8 imx8x imx8m)
sof-ci/jenkins/pr-device-test/main-cavs2.5 --> check-suspend-resume-with-playback.sh timeout

singalsu · 2023-06-27T12:49:36Z

SOFCI TEST

ranj063 · 2023-06-27T14:58:31Z

SOFCI TEST

singalsu · 2023-06-28T07:32:32Z

SOFCI TEST

kv2019i

Thanks @singalsu , looks good now!

Marcin's comments have been addressed and he's not available for re-review this week

kv2019i · 2023-06-28T10:58:46Z

https://sof-ci.01.org/sofpr/PR7792/build10174/devicetest/index.html looks a bit worrisome. I went though all the failures (verify-kernel-bootlog, check-runtime-pm-status , multiple-pipeline-capture ) and all are failures of #7482 , IOW with MOD_SET_DX failing. I'm going to proceed and merge this, but we need to raise attention to #7482 .

singalsu force-pushed the main_module_adapter_very_simple_copy branch from 92020c5 to 6ab77b6 Compare June 13, 2023 09:19

lyakh reviewed Jun 14, 2023

View reviewed changes

singalsu requested review from abonislawski and removed request for abonislawski June 14, 2023 15:35

singalsu force-pushed the main_module_adapter_very_simple_copy branch from 6ab77b6 to 571e207 Compare June 14, 2023 16:02

singalsu requested a review from lyakh June 14, 2023 16:09

ranj063 reviewed Jun 14, 2023

View reviewed changes

src/audio/module_adapter/module_adapter.c Outdated Show resolved Hide resolved

src/audio/module_adapter/module_adapter.c Outdated Show resolved Hide resolved

src/audio/module_adapter/module_adapter.c Outdated Show resolved Hide resolved

singalsu force-pushed the main_module_adapter_very_simple_copy branch 2 times, most recently from da4ef1f to 510d4a7 Compare June 15, 2023 11:31

singalsu marked this pull request as ready for review June 15, 2023 14:34

singalsu requested review from jxstelter, lgirdwood, plbossart, mmaka1, lbetlej and dbaluta as code owners June 15, 2023 14:34

ranj063 mentioned this pull request Jun 16, 2023

Mixin/Mixout Optimization for simple pipelines #7824

Draft

btian1 reviewed Jun 19, 2023

View reviewed changes

src/audio/module_adapter/module_adapter.c Show resolved Hide resolved

singalsu force-pushed the main_module_adapter_very_simple_copy branch from 510d4a7 to 5193101 Compare June 20, 2023 10:57

singalsu requested a review from marcinszkudlinski June 20, 2023 10:58

marcinszkudlinski previously requested changes Jun 20, 2023

View reviewed changes

singalsu force-pushed the main_module_adapter_very_simple_copy branch from 5193101 to e5bf8ca Compare June 21, 2023 11:07

singalsu requested review from marcinszkudlinski, btian1 and ranj063 June 21, 2023 11:09

lyakh reviewed Jun 22, 2023

View reviewed changes

singalsu force-pushed the main_module_adapter_very_simple_copy branch from e5bf8ca to 7b1ed0a Compare June 22, 2023 09:50

singalsu force-pushed the main_module_adapter_very_simple_copy branch 2 times, most recently from 2f88a2f to c23b13b Compare June 22, 2023 16:56

btian1 approved these changes Jun 26, 2023

View reviewed changes

singalsu force-pushed the main_module_adapter_very_simple_copy branch from c23b13b to 97ad7fd Compare June 26, 2023 08:21

singalsu force-pushed the main_module_adapter_very_simple_copy branch from 97ad7fd to fd7c5ad Compare June 26, 2023 13:09

singalsu force-pushed the main_module_adapter_very_simple_copy branch from fd7c5ad to d9a6491 Compare June 26, 2023 15:26

ranj063 approved these changes Jun 26, 2023

View reviewed changes

kv2019i approved these changes Jun 28, 2023

View reviewed changes

kv2019i merged commit ee6eb1b into thesofproject:main Jun 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Audio: Module adapter: Add a new simple copy function #7792

Audio: Module adapter: Add a new simple copy function #7792

singalsu commented Jun 13, 2023

ranj063 commented Jun 15, 2023

singalsu commented Jun 20, 2023

marcinszkudlinski left a comment

marcinszkudlinski Jun 20, 2023 •

edited

Loading

marcinszkudlinski Jun 20, 2023 •

edited

Loading

singalsu commented Jun 21, 2023

lyakh left a comment

lyakh Jun 22, 2023

singalsu Jun 22, 2023

lyakh Jun 22, 2023

singalsu commented Jun 22, 2023

btian1 left a comment

singalsu commented Jun 26, 2023

singalsu commented Jun 27, 2023

singalsu commented Jun 27, 2023

singalsu commented Jun 27, 2023

singalsu commented Jun 27, 2023

ranj063 commented Jun 27, 2023

singalsu commented Jun 28, 2023

kv2019i left a comment

kv2019i commented Jun 28, 2023

Audio: Module adapter: Add a new simple copy function #7792

Audio: Module adapter: Add a new simple copy function #7792

Conversation

singalsu commented Jun 13, 2023

ranj063 commented Jun 15, 2023

singalsu commented Jun 20, 2023

marcinszkudlinski left a comment

Choose a reason for hiding this comment

marcinszkudlinski Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

marcinszkudlinski Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

singalsu commented Jun 21, 2023

lyakh left a comment

Choose a reason for hiding this comment

lyakh Jun 22, 2023

Choose a reason for hiding this comment

singalsu Jun 22, 2023

Choose a reason for hiding this comment

lyakh Jun 22, 2023

Choose a reason for hiding this comment

singalsu commented Jun 22, 2023

btian1 left a comment

Choose a reason for hiding this comment

singalsu commented Jun 26, 2023

singalsu commented Jun 27, 2023

singalsu commented Jun 27, 2023

singalsu commented Jun 27, 2023

singalsu commented Jun 27, 2023

ranj063 commented Jun 27, 2023

singalsu commented Jun 28, 2023

kv2019i left a comment

Choose a reason for hiding this comment

kv2019i commented Jun 28, 2023

marcinszkudlinski Jun 20, 2023 •

edited

Loading

marcinszkudlinski Jun 20, 2023 •

edited

Loading