[SKIP SOF-TEST] native loadable up-down-mixer take 2 by pjdobrowolski · Pull Request #8210 · thesofproject/sof

pjdobrowolski · 2023-09-18T05:04:37Z

This is second approach to native sof loadable modules. This time are used zephyr's linker scripts to create separate archive for modules and put them into specific for module sections. After that cmake files, using objcopy put code for up-down-mixer to udm.mod file which is being sign by rimage.

Full process of building module is automated and run by python building scripts:
xtensa-build-zephyr.py -u mtl -k ../keys/<used_key_for_project>

marcinszkudlinski · 2023-09-18T08:59:52Z

 };
-
-DECLARE_MODULE_ADAPTER(up_down_mixer_interface, up_down_mixer_comp_uuid, up_down_mixer_comp_tr);
-SOF_MODULE_INIT(up_down_mixer, sys_comp_module_up_down_mixer_interface_init);


I think we need to have ability to use module in 2 ways - as a loadable OR as a built-in
It may be a kConfig option
quick suggestion (may be incorrect syntax, just a rough example):
choice prompt "up down mixer module" config NO_UP_DOWN_MIXER bool "up down mixer not available" config BUILT_IN_UP_DOWN_MIXER bool "up down mixer as a bullt in module" config LOADABLE_IN_UP_DOWN_MIXER bool "up down mixer as a loadable module" endchoice

Linux Kconfig uses a tristate variable for modules
y = builtin
n = not built
m = module
We shoudl do the same to simplify our Kconfigs.

marcinszkudlinski · 2023-09-18T09:09:32Z

 	${SOF_SAMPLES_PATH}/audio/detect_test.c
 )

-if(CONFIG_IPC_MAJOR_3)


Conversion of a specific module to loadable should be in separate commit than the loadable module infrastructure

marcinszkudlinski · 2023-09-18T09:10:12Z

+ .affinity_mask = 1,
+}
+};
+


marcinszkudlinski · 2023-09-18T09:10:29Z

+ * \brief Type definition of the package entry point.
+ * \note internal purpose.
+ */
+typedef int (ModulePackageEntryPoint)(uint32_t costam);


english please ;)

marcinszkudlinski · 2023-09-18T09:10:56Z

+ DEFINE_MODULE_PACKAGE_ENTRY_POINT(MODULE)
+
+/*
+ * #define DECLARE_LOADABLE_MODULE(MODULE, MODULE_FACTORY) \


why commeted out?

during development we decided to drop that header file.

marcinszkudlinski · 2023-09-18T10:58:38Z

+struct sof_module_api_build_info udm_build_info __attribute__((section(".buildinfo"))) = {
+	ADSP_BUILD_INFO_FORMAT,
+	{
+ ((0x3FF & 5) << 20) |


prepare a macro for those shifts

marcinszkudlinski · 2023-09-18T11:09:14Z


+
+# Compile up_down_mixer as loadable module
+sof_loadable_library(udm ${SOF_AUDIO_PATH}/up_down_mixer/up_down_mixer.toml)


more meaningful name (updwmix) instead of udm

marcinszkudlinski · 2023-09-18T11:39:17Z

+	}
+};
+
+extern struct module_interface up_down_mixer_interface;


this forward really needed? Maybe move declaration of sof_module_api_build_info to the end of the file?

marcinszkudlinski · 2023-09-18T11:41:01Z

+ .type = { .load_type = SOF_MAN_MOD_TYPE_MODULE,
+ .domain_ll = 1 },
+ .affinity_mask = 1,
+}


lyakh · 2023-09-18T13:15:20Z


-#include <sof/audio/module_adapter/iadk/adsp_stddef.h>
+#include <sof/audio/module_adapter/iadk/adsp_stddef.h>
+#include <sof/audio/module_adapter/library/module_api_ver.h>


this doesn't look right. You haven't changed anything in the header, so there's no reason why it now need that additional header

marc-hb

Thanks but I'm pretty sure the plan for /lib/firmare/intel/sof/ is to use --use-platform-subdir like this:

sof/scripts/xtensa-build-zephyr.py --use-platform-subdir mtl
build-sof-staging |-- sof | +-- mtl | |-- community | | +-- sof-mtl.ri sha256=401e8360bd1a...
| |-- sof-mtl.ldc sha256=516ad1320ec8...
| |-- loadable_module1.ri
| |-- loadable_module2.ri
| +-- loadable_module3.ri

EDIT: maybe --use-platform-subdir should be ON by default?

plbossart · 2023-09-18T21:33:04Z

The 'base fw' and loadable modules can be signed with different keys, so we absolutely do not want them to land in the same directory - the loadable modules MUST be in a key-specific directory.

marc-hb · 2023-09-18T21:45:56Z

@plbossart you're referring to --key-type-subdir.

I was talking about --use-platform-subdir which avoids the ugly sof-{file}-lib.ri rename hack.

They're not mutually exclusive.

In any case we need a tiny bit of plain english documentation to be reviewed and approved first, we don't want people to have to search and read python or kernel code[*] to try and fail to guess the /lib/firmware/intel/sof/ structure.

This can be as simple as 1-2 pages with the output of a sample "tree" command with a few plain English words around it. Let's stop making the same mistake over and over again:

[*] from @plbossart https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/intel/pci-mtl.c#L39

pjdobrowolski · 2023-09-19T04:55:32Z

I appreciate your engagement but this is still a draft.

marc-hb · 2023-09-20T06:33:07Z

In any case we need a tiny bit of plain english documentation to be reviewed and approved first, we don't want people to have to search and read python or kernel code[*] to try and fail to guess the /lib/firmware/intel/sof/ structure.

Thank you @kv2019i !

intel_debug: add info for Intel firmware and tplg filenames sof-docs#475

So according to that sof-docs PR, the path for loadable modules is: sof-ipc4-lib/PLAT/community/UUID.bin

lgirdwood · 2023-09-20T14:53:47Z

+/*!
+ * \def STR(s)
+ * intermediate macro for Stringification
+ * \note internal purpose only. Consider to use XSTR(s) for Stringification.
+ */
+#define STR(s) #s
+/*!
+ * \def XSTR(s)
+ * Stringification macro
+ */
+#define XSTR(s) STR(s)


need to be careful here as I would imagine these common macros should come from Zephyr.

lgirdwood · 2023-09-20T14:54:42Z

+ * \note internal purpose.
+ */
+#define DEFINE_MODULE_PACKAGE_ENTRY_POINT(MODULE) \
+ volatile sof_module_api_build_info MODULE_PACKAGE_ENTRY_BUILD_INFO_NAME(MODULE) BUILD_INFO_SECTION = \


why does this need to be volatile ?

lgirdwood · 2023-09-20T14:56:45Z

@@ -0,0 +1,86 @@
+version = [3, 0]
+
+[adsp]


fwiw, @aiChaoSONG is looking at having an #include <platform.toml> for all the non module stuff. This will help simplify

lgirdwood · 2023-09-20T15:39:08Z

 };
-
-DECLARE_MODULE_ADAPTER(up_down_mixer_interface, up_down_mixer_comp_uuid, up_down_mixer_comp_tr);
-SOF_MODULE_INIT(up_down_mixer, sys_comp_module_up_down_mixer_interface_init);


Linux Kconfig uses a tristate variable for modules
y = builtin
n = not built
m = module
We shoudl do the same to simplify our Kconfigs.

Added toml file to enable building up down mixer as loadable module. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Updated zephyr revision to the top of 006 branch Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

updated zephyr to mtl-006 updated rimage to mtl-006

Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

pjdobrowolski requested review from abonislawski, dbaluta, kv2019i, lbetlej, lgirdwood, marc-hb, mmaka1 and plbossart as code owners September 18, 2023 05:04

pjdobrowolski requested review from RDharageswari, abonislawski, kv2019i, lgirdwood, lyakh and softwarecki and removed request for abonislawski, dbaluta, kv2019i, lbetlej, lgirdwood, marc-hb, mmaka1 and plbossart September 18, 2023 05:04

pjdobrowolski changed the title ~~Sof pawelxd loadable udm~~native loadable up-down-mixer take 2Sep 18, 2023

pjdobrowolski closed this Sep 18, 2023

pjdobrowolski reopened this Sep 18, 2023

pjdobrowolski marked this pull request as draft September 18, 2023 07:27

marcinszkudlinski reviewed Sep 18, 2023
View reviewed changes

lyakh reviewed Sep 18, 2023
View reviewed changes

marc-hb requested changes Sep 18, 2023
View reviewed changes

marc-hb mentioned this pull request Sep 18, 2023
ASoC: SOF: Intel: pci-tgl: Change the default paths and firmware names thesofproject/linux#4382
Merged

kv2019i mentioned this pull request Sep 19, 2023
intel_debug: add info for Intel firmware and tplg filenames thesofproject/sof-docs#475
Merged

lgirdwood reviewed Sep 20, 2023
View reviewed changes

softwareckiand others added 4 commits September 25, 2023 09:13

Add support for building sof native loadable modules
527b5ac
Added toml file to enable building up down mixer as loadable module. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

west: Update Zephyr revision
4493f0c
Updated zephyr revision to the top of 006 branch Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

scripts: xtensa-build_zephyr: Copy loadable modules to staging directory
a5d536e
Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

west: update zephyr version
8663ea4
updated zephyr to mtl-006 updated rimage to mtl-006

pjdobrowolski force-pushed the sof_pawelxd_loadable_udm branch 4 times, most recently from c662f6d to 58c0bdfCompare September 28, 2023 06:44

marc-hb changed the title ~~native loadable up-down-mixer take 2~~[SKIP SOF-TEST] native loadable up-down-mixer take 2Sep 28, 2023

pjdobrowolski force-pushed the sof_pawelxd_loadable_udm branch from 58c0bdf to 223cb55Compare September 28, 2023 06:59

Example of loadable up_down_mixer
69925ce
Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

pjdobrowolski force-pushed the sof_pawelxd_loadable_udm branch from 223cb55 to 69925ceCompare September 28, 2023 07:16

pjdobrowolski closed this Oct 25, 2023



		# Compile up_down_mixer as loadable module
		sof_loadable_library(udm ${SOF_AUDIO_PATH}/up_down_mixer/up_down_mixer.toml)

Uh oh!

Conversation

pjdobrowolski commented Sep 18, 2023

Uh oh!

marcinszkudlinskiSep 18, 2023• 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!

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!

Choose a reason for hiding this comment

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 commented Sep 18, 2023

Uh oh!

marc-hb commented Sep 18, 2023

Uh oh!

pjdobrowolski commented Sep 19, 2023

Uh oh!

marc-hb commented Sep 20, 2023

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

marcinszkudlinskiSep 18, 2023•
edited
Loading

marc-hb left a comment •
edited
Loading