converged: initial commit for converged modules development #4214

bkokoszx · 2021-05-19T12:56:32Z

This commit contains three converged sound open firmware module examples
such as: aca_module, amplifier_module and downmixer_module and whole building
infrastructure (i.e. makefiles, source files and API headers) required to
build module examples and loadable libraries

Signed-off-by: Bartosz Kokoszko bartoszx.kokoszko@linux.intel.com

bkokoszx · 2021-05-19T12:56:51Z

@lgirdwood @mwasko
FYI

lgirdwood

@bkokoszx can you split this into a series of smaller per feature PRs. This way it will be far easier to review. We will also need to put the files in the correct places.
Btw, do you have any build able file today that can be used by CI as a build check.

mwasko · 2021-05-19T14:42:04Z

@bkokoszx can you split this into a series of smaller per feature PRs. This way it will be far easier to review. We will also need to put the files in the correct places.
Btw, do you have any build able file today that can be used by CI as a build check.

+1 from me, maybe split the content into three PRs, one per example module?

Any suggestion on place Liam? My understanding after internal discussion was that we agree to keep the converged modules in converged folder until we align SOF FW Module API as they are not yet build along with any SOF base FW target.

dbaluta · 2021-05-19T15:00:26Z

converged_sound_open_fw_modules_development/FW/intel_common/module_binmaps/aca_module.binmap

@bkokoszx this is very interesting. Can you add more information inthe commit message.

What is a module? A separate binary with some functionality?

How is this used by sof:

statically compiled

dynamically loaded at runtime
etc?

There are some headers with 'INTEL confidential' is that OK?

What is a module? A separate binary with some functionality?

How is this used by sof:

statically compiled

dynamically loaded at runtime
etc?

The module here is an equivalent of SOF processing component. The difference is that these modules will not be statically compiled with SOF base FW but dynamically loaded as a separate binary. That will allow to have one base SOF FW for platform generation and depending on specific audio topology configuration choose which 3rd party processing modules to load.

The modules will be usable with SOF once we implement dynamic library load feature and align processing modules API. This will be the next step.

@dbaluta
The Intel Confidential headers were my fault. I've just updated it.

@mwasko This is a great feature. We also have this project for Google Summer of Code.

https://wiki.linuxfoundation.org/gsoc/2021-gsoc-sound-open-firmware

bkokoszx · 2021-05-20T08:25:38Z

@lgirdwood I will try to split it into several PR's

This commit contains three converged sound open firmware module examples such as: aca_module, amplifier_module and downmixer_module and whole building infrastructure (i.e. makefiles, source files and API headers) required to build module examples and loadable libraries Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>

bkokoszx · 2021-05-20T11:27:48Z

@lgirdwood @mwasko
I've split this PR into:

dbaluta · 2021-05-20T11:38:35Z

@bkokoszx I think the usual practice is to have a single PR with multiple commits.

lgirdwood · 2021-05-20T12:37:37Z

@bkokoszx I think the usual practice is to have a single PR with multiple commits.

@dbaluta I've asked for this to be many PRs to lower the burden on reviewers as many topics are covered which may be confusing to discuss on a single PR.

marc-hb

Makefile comment transferred to #4222 (review)

plbossart

I had to pinch myself when I looked at GnuMake files. This is unmaintainable. Most projects only describe 'what' you want and autogenerate makefiles, this is several bridges and a long tunnel too far.

kv2019i

The README.Md is very good but hidden inside the very large PR. Main concerns I have relate to split between Intel specific and generic (in terms of headers and common makefiles) and the huge chunk of plain make rules.

Some nitpicks I couldn't comment inline:

one binary file include (.chm)

kv2019i · 2021-05-25T13:46:23Z

converged_sound_open_fw_modules_development/FW/portable/include/modules.h

+ * and ADSP FW Builder tool.
+ */
+
+#define MASTER_CORE_AFFINITY        1


Some documentation would be helpful here.

kv2019i · 2021-05-25T13:51:55Z

...ed_sound_open_fw_modules_development/FW/src/intel_adsp/include/ipc/module_initial_settings.h

+
+/*! \brief bits field map which helps to describe each channel location a the data stream buffer */
+typedef uint32_t ChannelMap;
+#if 0 //WIP


Something to remove.

kv2019i · 2021-05-25T13:55:48Z

..._open_fw_modules_development/FW/src/intel_adsp/include/processing_module_factory_interface.h

+         *
+         * A module is required to provide some PinEndpoint arrays to allow the ADSP System to drive streams into the module.
+         */
+        PinEndpoint* sources;


Where is "sources_count" defined? I couldn't find it anywhere in the patchset?

kv2019i · 2021-05-25T14:06:34Z

converged_sound_open_fw_modules_development/README.md

+infrastructure (i.e. makefiles, source files and API headers) required to
+build module examples and loadable libraries. Loadable library is composed by
+a set of N precompiled loadable modules.
+## Modules


Thanks @bkokoszx , this is a very informational file and should get more attention. Now it's piled under the 119 files changed.

kv2019i · 2021-05-25T14:14:10Z

converged_sound_open_fw_modules_development/FW/src/intel_adsp/include/adsp_error_code.h

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


The module interface should be common for all platforms, so FW/SRC/intel_adsp seems like the wrong path for the interface headers.

kv2019i · 2021-05-25T14:16:19Z

..._fw_modules_development/FW/src/intel_adsp/build_framework/build_core/ace_extlib_meu_conf.xml

+    <Name value="ADSP" help_text="Name to use in the output binary's directory. Maximum length is 4 characters. " />
+    <Length value="0x0" help_text="Length of output binary, extra space will be filled with 0xFF's. If length is smaller than required, an error will be reported. If set to 0, the length will be computed as needed by the tool." />
+    <Usage value="audioImage0Manifest" value_list="CseBupManifest,,CseMainManifest,,PmcManifest,,UsbTypeCIOMManifest,,UsbTypeCNphyManifest,,UsbTypeCTBTManifest,,WcodManifest,,LoclManifest,,IntelUtokManifest,,SPHYManifest,,PchcManifest,,SamfManifest,,PphyManifest,,GbstManifest,,BootPolicyManifest,,iUnitBootLoaderManifest,,iUnitMainFwManifest,,audioImage0Manifest,,audioImage1Manifest,,IfwiManifest,,OsBootLoaderManifest,,OsKernelManifest,,OemSmipManifest,,IshManifest,,OemDebugManifest,,OemKeyManifest,,SilentLakeVmmManifest,,OemDnxIfwiManifest" help_text="Indicates the type of data contained in this binary. This value is used during signature verification to validate the public key." />
+    <VendorId value="0x8086" />


The value_list should probably be omitted here. We just need to specify the value. @mwasko please check

kv2019i · 2021-05-25T14:18:27Z

...fw_modules_development/FW/src/intel_adsp/build_framework/build_core/custom_firmware_rules.mk

@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: BSD-3-Clause


This is one huge body of plain makefile rules. We'd need some common make rules how to build modules. Maybe the common rules are done in Cmake and these remain as intel_adsp specific reference?

sys-pt1s · 2022-04-20T07:04:46Z

Can one of the admins verify this patch?

kv2019i · 2022-04-20T15:41:00Z

Almost a year without activity, closing.

bkokoszx requested review from dbaluta, lbetlej, lgirdwood, mmaka1 and plbossart as code owners May 19, 2021 12:56

bkokoszx requested a review from mwasko May 19, 2021 12:57

lgirdwood reviewed May 19, 2021

View reviewed changes

lgirdwood added this to the v1.9 milestone May 19, 2021

dbaluta reviewed May 19, 2021

View reviewed changes

bkokoszx force-pushed the converged-sound-open-fw-modules-development branch from 2b779ea to aa9cc2c Compare May 20, 2021 08:21

bkokoszx force-pushed the converged-sound-open-fw-modules-development branch from aa9cc2c to 792c63b Compare May 20, 2021 08:36

bkokoszx force-pushed the converged-sound-open-fw-modules-development branch from 792c63b to ebe9fb2 Compare May 20, 2021 08:44

bkokoszx mentioned this pull request May 21, 2021

converged: add adsp_fw_builder binaries thesofproject/sof-bin#42

Closed

marc-hb requested changes May 24, 2021

View reviewed changes

plbossart requested changes May 24, 2021

View reviewed changes

kv2019i requested changes May 25, 2021

View reviewed changes

kv2019i closed this Apr 20, 2022

converged: initial commit for converged modules development #4214

converged: initial commit for converged modules development #4214

Uh oh!

Conversation

bkokoszx commented May 19, 2021

Uh oh!

bkokoszx commented May 19, 2021

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

mwasko commented May 19, 2021

Uh oh!

dbaluta May 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mwasko May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkokoszx commented May 20, 2021

Uh oh!

bkokoszx commented May 20, 2021

Uh oh!

dbaluta commented May 20, 2021

Uh oh!

lgirdwood commented May 20, 2021

Uh oh!

marc-hb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sys-pt1s commented Apr 20, 2022

Uh oh!

kv2019i commented Apr 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

dbaluta May 19, 2021 •

edited

Loading

mwasko May 20, 2021 •

edited

Loading

marc-hb left a comment •

edited

Loading