Loadable up down mixer #7939

pjdobrowolski · 2023-07-13T13:22:10Z

lmdk provide methods for building modules out of sof and zephyr environment with minimum data delivered. Purpose of that PR is to show how to port modules on up down mixer example.

To build that module it is required to follow steps:

set proper enviroment variables:

XTENSA_INSTALL_PATH="c:/usr/xtensa"
export ZEPHYR_TOOLCHAIN_VARIANT=xcc
export XTENSA_CORE=ace10_LX7HiFi4_RI_2020_5
export XTENSA_TOOLS_VERSION=RI-2020.5-win32
export [email protected]
export XTENSA_TOOLCHAIN_PATH=$XTENSA_INSTALL_PATH/XtDevTools/install/tools/$XTENSA_TOOLS_VERSION
export XTENSA_TOOLS_DIR=$XTENSA_INSTALL_PATH/install/tools
export XTENSA_TOOLS=$XTENSA_TOOLS_DIR/$XTENSA_TOOLS_VERSION/XtensaTools
export NINJA_PATH='C:/Program Files/ninja'
export CMAKE_GENERATOR=Ninja
export PATH=/C/usr/xtensa/XtDevTools/install/tools/RI-2020.5-win32/XtensaTools/bin:$PATH

build up to date rimage.exe for signing and manifest creation
move to module example directory
cd $home/$repoDir/sof/lmdk/libraries/module_example
build module with commands:
cmake -B build -G Ninja -DRIMAGE_COMMAND=$rimageDir -DSIGNING_KEY=$privKey
cmake --build build

Different approach but same loadable modules:
#7867
#7861

ranj063 · 2023-07-13T16:16:18Z

@pjdobrowolski the change looks OK but do we have the copy/move the existing source code to convert it into a loadable module?

btian1

header year are all not updated, suggest all update to 2023 althrough it copied and not changed inside.

pjdobrowolski · 2023-07-14T05:19:45Z

@ranj063 I've made a few experiments with creating independent loadable modules and that approach with coping only headers gave me clean binary without any extra zephyr or xtos dependencies. I've cleanup these headers to not include things not required but I think that it is possible to create in sof location with portable independent headers only for loadable modules which could be safely used in loadable modules.

Firstly I would narrow how many of them is really required.

lyakh

Please, let's not copy all the headers over. (1) They will run out of sync with the originals. It took me significant time to debug exactly such an issue - the original header turned out to be newer, it contained an additional field, so the main firmware and the module didn't work together. (2) You also need Zephyr headers and you need the automatically generated autoconf.h with configuration macros. Building modules should be done using a complete SOF + Zephyr installation, built for the target configuration, then you can build your module against that. This is also what the Linux kernel does for external modules.

lgirdwood

We cant dupicate existing code or headers.
We need to

Update lmdk Cmake to use global system header directories.
Update lmdk Cmake to invoke src/audio Cmake to build module archives and link to those archives to link with lmdk modules. We have lost of src/audio/module_hifi.c code that can be linked to.

lgirdwood · 2023-07-14T11:11:54Z

lmdk/include/adsp_stddef.h

@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: BSD-3-Clause


We need to set up our cmake to include from the global include directories to avoid the duplication

lgirdwood · 2023-07-14T11:15:36Z

lmdk/modules/up_down_mixer/up_down_mixer_hifi3.c

+#include <stdint.h>
+#include <stdbool.h>
+
+void upmix32bit_1_to_5_1(struct up_down_mixer_data *cd, const uint8_t * const in_data,


We should reuse existing hifi3 up/down mixer code and link as an archive. i.e. we link lmdk up_down_mixer.c with src/audio/up_down_mixer.a

pjdobrowolski · 2023-07-14T11:28:15Z

It is not copying, it is extracting avoiding xtos and zephyr dependencies.

lgirdwood · 2023-07-14T11:48:00Z

It is not copying, it is extracting avoiding xtos and zephyr dependencies.

If we need to remove RTOS deps then we should move the rtos logic out of the headers. This may need some headers being splitt up.

What problems are the RTOS headers giving here ?

pjdobrowolski · 2023-07-14T11:59:58Z

It is not copying, it is extracting avoiding xtos and zephyr dependencies.

If we need to remove RTOS deps then we should move the rtos logic out of the headers. This may need some headers being splitt up.

What problems are the RTOS headers giving here ?

Yest that is one of an idea to be independent from RTOS and SOF changes. For customer it is convenient to work in fixed environment without risk that some unexpected changes will affect working theirs closed source module.

Splitting up headers is one of solutions and I think that it is goal to create portable include files which stay untouchable. That is how in legacy fw is done.

lgirdwood · 2023-07-14T12:08:45Z

It is not copying, it is extracting avoiding xtos and zephyr dependencies.

If we need to remove RTOS deps then we should move the rtos logic out of the headers. This may need some headers being splitt up.
What problems are the RTOS headers giving here ?

Yest that is one of an idea to be independent from RTOS and SOF changes. For customer it is convenient to work in fixed environment without risk that some unexpected changes will affect working theirs closed source module.

Splitting up headers is one of solutions and I think that it is goal to create portable include files which stay untouchable. That is how in legacy fw is done.

It would be a lot easier to change the deployment model for 3rd party module SDK going forward since all infrastructure is open today. i.e. we should be providing a simple (for user) wrapper around west that

Downloads all SW/FW code using correct versions/tags for product.
Configures user environment with PATHs, Kconfig configuration files for product etc to build any dependecies and modules.

At this point developer is ready to go and we dont need to create a seperate SDK environment for loadable modules. Its saves everyone effort.

pjdobrowolski · 2023-07-18T06:11:47Z

It might be true if changes in sof or zephyr wouldn't be so dynamical. For client it is unnecessary hassle to keep up with changes and remember each time to do west update. We must put ourself in clients shoes and think more product wise.

However, I agree with @lgirdwood and @lyakh that using zephyr/sof dependencies is more convenient and we can do 2 versions of loadable modules, ones with separate SDK and another more advance with build in west building etc.

lyakh · 2023-07-18T08:17:28Z

It might be true if changes in sof or zephyr wouldn't be so dynamical. For client it is unnecessary hassle to keep up with changes and remember each time to do west update. We must put ourself in clients shoes and think more product wise.

However, I agree with @lgirdwood and @lyakh that using zephyr/sof dependencies is more convenient and we can do 2 versions of loadable modules, ones with separate SDK and another more advance with build in west building etc.

@pjdobrowolski the biggest problem I see with an SDK is the need for stable APIs. If we create an SDK we have to guarantee to our users that the exported APIs don't change "too often" and when they do change, we provide a reasonable update path. Doing that would (1) create too much maintenance overhead on us when updating APIs and (2) make development more difficult.

aiChaoSONG · 2023-08-22T07:46:55Z

@pjdobrowolski I am not able to compile it with latest main, can you make sure this patch compiles with main?

FAILED: CMakeFiles/up_down_mixer.dir/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer.c.obj 
/home/chao/work/xtensa/XtDevTools/install/tools/RI-2020.5-linux/XtensaTools/bin/xt-xcc -DCONFIG_IPC_MAJOR_4=1 -DCONFIG_XTENSA=1 -DMAJOR_IADSP_API_VERSION=5 -DMIDDLE_IADSP_API_VERSION=0 -DMINOR_IADSP_API_VERSION=0 -D__ZEPHYR__=1 -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/coefficients -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/ipc -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/ipc4 -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/math -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/lib -I/home/chao/work2/sofdev/zsof/sof/lmdk/include -I/home/chao/work2/sofdev/zsof/sof/rimage/src/include  -MD -MT CMakeFiles/up_down_mixer.dir/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer.c.obj -MF CMakeFiles/up_down_mixer.dir/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer.c.obj.d -o CMakeFiles/up_down_mixer.dir/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer.c.obj -c /home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer.c
In file included from /home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer.c:13:
/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/../include/module_adapter/module/generic.h:15:33: error: sof/audio/component.h: No such file or directory
/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/../include/module_adapter/module/generic.h:16:20: error: sof/ut.h: No such file or directory
/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/../include/module_adapter/module/generic.h:17:28: error: sof/lib/memory.h: No such file or directory
In file included from /home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/../include/module_adapter/module/generic.h:18,
                 from /home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer.c:13:
/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/../include/module_adapter/module/module_interface.h:16:37: error: sof/compiler_attributes.h: No such file or directory
/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/../include/module_adapter/module/module_interface.h:17:32: error: sof/audio/sink_api.h: No such file or directory
/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/../include/module_adapter/module/module_interface.h:18:34: error: sof/audio/source_api.h: No such file or directory
/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer.c:14:69: error: ../include/module_adapter/system_service/system_service.h: No such file or directory
[2/4] /home/chao/work/xtensa/XtDevTools/install/tools/RI-2020.5-linux/XtensaTools/bin/xt-xcc -DCONFIG_IPC_MAJOR_4=1 -DCONFIG_XTENSA=1 -DMAJOR_IADSP_API_VERSION=5 -DMIDDLE_IADSP_API_VERSION=0 -DMINOR_IADSP_API_VERSION=0 -D__ZEPHYR__=1 -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/coefficients -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/ipc -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/ipc4 -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/math -I/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/../../include/lib -I/home/chao/work2/sofdev/zsof/sof/lmdk/include -I/home/chao/work2/sofdev/zsof/sof/rimage/src/include  -MD -MT CMakeFiles/up_down_mixer.dir/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer_hifi3.c.obj -MF CMakeFiles/up_down_mixer.dir/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer_hifi3.c.obj.d -o CMakeFiles/up_down_mixer.dir/home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer_hifi3.c.obj -c /home/chao/work2/sofdev/zsof/sof/lmdk/modules/up_down_mixer/up_down_mixer_hifi3.c

marc-hb · 2023-09-05T21:34:28Z

For customer it is convenient to work in fixed environment without risk that some unexpected changes will affect working theirs closed source module.

A branch is the simpler and better answer to this problem. That's why version control branches exist: to avoid unwanted changes and keep them under control. Copy/paste is a much older and more dangerous way to achieve the same thing.

If using git is too demanding for some customers then (a subset of) headers can be released from a branch.

No copy/paste required.

If we create an SDK we have to guarantee to our users that the exported APIs don't change "too often" and when they do change, we provide a reasonable update path. Doing that would (1) create too much maintenance overhead on us when updating APIs and (2) make development more difficult.

Yes: a branch is only the beginning. Then you need a lot of work to maintain it. That's true with any solution.

Headers represent only data structures which are neccessary for proper working pipelines

Up to date module adapter and system services

All files required for building up_down_mixer

lgirdwood

@pjdobrowolski whats the plan for reusing existing headers ? Do you have an ETA ?

lgirdwood · 2023-09-29T15:22:07Z

lmdk/modules/up_down_mixer/up_down_mixer.c

+enum mem_zone {
+	SOF_MEM_ZONE_SYS = 0,		/**< System zone */
+	SOF_MEM_ZONE_SYS_RUNTIME,	/**< System-runtime zone */
+	SOF_MEM_ZONE_RUNTIME,		/**< Runtime zone */
+	SOF_MEM_ZONE_BUFFER,		/**< Buffer zone */
+	SOF_MEM_ZONE_RUNTIME_SHARED,	/**< Runtime shared zone */
+	SOF_MEM_ZONE_SYS_SHARED,	/**< System shared zone */
+};


Need to get these from the header.

lgirdwood · 2023-09-29T15:22:47Z

lmdk/modules/up_down_mixer/up_down_mixer.c

+		cd->downmix_coefficients = k_scaled_lo_ro_downmix32bit;
+		break;
+	default:
+//		comp_err(dev, "set_downmix_coefficients(): invalid channel config.");


Would it make it easier to use the Zephyr native LOG() API here ?

btian1 reviewed Jul 14, 2023

View reviewed changes

lyakh requested changes Jul 14, 2023

View reviewed changes

lgirdwood requested changes Jul 14, 2023

View reviewed changes

pjdobrowolski force-pushed the loadable_up_down_mixer branch from dd48553 to d311845 Compare July 26, 2023 10:23

pjdobrowolski added 5 commits September 29, 2023 07:53

lmdk: up_down_mixer loadable body

0411fea

lmdk: data structures headers required for modules

d3fa041

Headers represent only data structures which are neccessary for proper working pipelines

lmdk: module adapter and module interface

012afeb

Up to date module adapter and system services

lmdk: loadable up_down_mixer source code

97933bc

All files required for building up_down_mixer

lmdk: update cmake, UPDWMIX, module_interface

ee17d3f

pjdobrowolski force-pushed the loadable_up_down_mixer branch from d311845 to ee17d3f Compare September 29, 2023 07:25

lgirdwood reviewed Sep 29, 2023

View reviewed changes

pjdobrowolski closed this Oct 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Loadable up down mixer #7939

Loadable up down mixer #7939

pjdobrowolski commented Jul 13, 2023 •

edited

Loading

ranj063 commented Jul 13, 2023

btian1 left a comment

pjdobrowolski commented Jul 14, 2023

lyakh left a comment

lgirdwood left a comment

lgirdwood Jul 14, 2023

lgirdwood Jul 14, 2023

pjdobrowolski commented Jul 14, 2023

lgirdwood commented Jul 14, 2023

pjdobrowolski commented Jul 14, 2023

lgirdwood commented Jul 14, 2023

pjdobrowolski commented Jul 18, 2023

lyakh commented Jul 18, 2023

aiChaoSONG commented Aug 22, 2023

marc-hb commented Sep 5, 2023

lgirdwood left a comment

lgirdwood Sep 29, 2023

lgirdwood Sep 29, 2023

Loadable up down mixer #7939

Loadable up down mixer #7939

Conversation

pjdobrowolski commented Jul 13, 2023 • edited Loading

ranj063 commented Jul 13, 2023

btian1 left a comment

Choose a reason for hiding this comment

pjdobrowolski commented Jul 14, 2023

lyakh left a comment

Choose a reason for hiding this comment

lgirdwood left a comment

Choose a reason for hiding this comment

lgirdwood Jul 14, 2023

Choose a reason for hiding this comment

lgirdwood Jul 14, 2023

Choose a reason for hiding this comment

pjdobrowolski commented Jul 14, 2023

lgirdwood commented Jul 14, 2023

pjdobrowolski commented Jul 14, 2023

lgirdwood commented Jul 14, 2023

pjdobrowolski commented Jul 18, 2023

lyakh commented Jul 18, 2023

aiChaoSONG commented Aug 22, 2023

marc-hb commented Sep 5, 2023

lgirdwood left a comment

Choose a reason for hiding this comment

lgirdwood Sep 29, 2023

Choose a reason for hiding this comment

lgirdwood Sep 29, 2023

Choose a reason for hiding this comment

pjdobrowolski commented Jul 13, 2023 •

edited

Loading