Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loading module: fix native module interface register #8750

Merged

Conversation

pjdobrowolski
Copy link
Contributor

In SOF we have a two type loadable modules, IADK and native whose are sharing the same registering mechanism but for C++/C needs IADK is using adapter which translates methods for SOF needs.
Native loadable module is not using any additional adapters and returns pointer for API used in built-in modules and loadable.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pjdobrowolski looks like an optimization only in this PR ? Sorry, not following what the change is from current code in the commit message,

@pjdobrowolski
Copy link
Contributor Author

That is not an optimalization but bugfix. Currently native loadable modules don't register theirs API.

@lgirdwood
Copy link
Member

That is not an optimalization but bugfix. Currently native loadable modules don't register theirs API.

oh, ok - a bug fix then. Best to spell that out in the commit as it was not clear for me this was fixing an issue. Thanks !

@marc-hb marc-hb requested a review from lyakh January 17, 2024 21:38
@marc-hb

This comment was marked as resolved.

@pjdobrowolski pjdobrowolski changed the title loading module: module interface register refactor loading module: fix native module interface register Jan 18, 2024
@pjdobrowolski
Copy link
Contributor Author

Ok, you're right. Changed title and commit name.

@lgirdwood
Copy link
Member

@pjdobrowolski not sure if CI build error is related ?

ccache /home/cwd_user/zephyr-sdk-0.16.4/xtensa-intel_ace15_mtpm_zephyr-elf/bin/xtensa-intel_ace15_mtpm_zephyr-elf-gcc -DCC_OPTIMIZE_FLAGS=\"-O2\" -DKERNEL -DK_HEAP_MEM_POOL_SIZE=8192 -DXCC_TOOLS_VERSION=\"zephyr\" -D__ZEPHYR__=1 -DRELATIVE_FILE=\"../sof/zephyr/lib/cpu.c\" -I/zep_workspace/zephyr/include -I/zep_workspace/build-mtl/zephyr/include/generated -I/zep_workspace/zephyr/soc/xtensa/intel_adsp/ace -I/zep_workspace/zephyr/soc/xtensa/intel_adsp/common/include -I/zep_workspace/zephyr/soc/xtensa/intel_adsp/ace/include -I/zep_workspace/zephyr/soc/xtensa/intel_adsp/ace/include/intel_ace15_mtpm -I/zep_workspace/zephyr/drivers -I/zep_workspace/sof/zephyr/include -I/zep_workspace/sof/src/platform/intel/ace/include -I/zep_workspace/sof/src/platform/meteorlake/include -I/zep_workspace/sof/src/include/sof/audio/module_adapter/iadk -I/zep_workspace/sof/src/include/sof/audio/module_adapter/library -I/zep_workspace/modules/hal/xtensa/include -I/zep_workspace/modules/hal/xtensa/XTENSA_HAL -I/zep_workspace/modules/hal/xtensa/zephyr/soc/intel_ace15_mtpm -I/zep_workspace/sof/tools/rimage/src/include -I/zep_workspace/sof/src/include -I/zep_workspace/sof/src/arch/xtensa/include -I/zep_workspace/sof/third_party/include -I/zep_workspace/sof/xtos/include -isystem /zep_workspace/zephyr/lib/libc/minimal/include -isystem /zep_workspace/zephyr/lib/libc/common/include -isystem /opt/toolchains/zephyr-sdk-0.16.4/xtensa-intel_ace15_mtpm_zephyr-elf/bin/../lib/gcc/xtensa-intel_ace15_mtpm_zephyr-elf/12.2.0/include -isystem /opt/toolchains/zephyr-sdk-0.16.4/xtensa-intel_ace15_mtpm_zephyr-elf/bin/../lib/gcc/xtensa-intel_ace15_mtpm_zephyr-elf/12.2.0/include-fixed -fno-strict-aliasing -O2 -imacros /zep_workspace/build-mtl/zephyr/include/generated/autoconf.h -ffreestanding -fno-common -g -gdwarf-4 -fdiagnostics-color=always --sysroot=/home/cwd_user/zephyr-sdk-0.16.4/xtensa-intel_ace15_mtpm_zephyr-elf/xtensa-intel_ace15_mtpm_zephyr-elf -imacros /zep_workspace/zephyr/include/zephyr/toolchain/zephyr_stdint.h -Wall -Wformat -Wformat-security -Wno-format-zero-length -Wno-pointer-sign -Wpointer-arith -Wexpansion-to-defined -Wno-unused-but-set-variable -Werror=implicit-int -fno-pic -fno-pie -Werror -fno-asynchronous-unwind-tables -fstrict-overflow -fno-reorder-functions --param=min-pagesize=0 -fno-defer-pop -fmacro-prefix-map=/zep_workspace/sof/app=CMAKE_SOURCE_DIR -fmacro-prefix-map=/zep_workspace/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/zep_workspace=WEST_TOPDIR -ffunction-sections -fdata-sections -mlongcalls -DXTENSA_TOOLCHAIN_VARIANT=1 -std=c99 -nostdinc -std=gnu99 -MD -MT modules/sof/CMakeFiles/modules_sof.dir/lib/cpu.c.obj -MF modules/sof/CMakeFiles/modules_sof.dir/lib/cpu.c.obj.d -o modules/sof/CMakeFiles/modules_sof.dir/lib/cpu.c.obj -c /zep_workspace/sof/zephyr/lib/cpu.c
/zep_workspace/sof/zephyr/lib/cpu.c: In function 'secondary_init':
/zep_workspace/sof/zephyr/lib/cpu.c:47:9: error: implicit declaration of function 'z_smp_thread_init'; did you mean 'smp_timer_init'? [-Werror=implicit-function-declaration]
   47 |         z_smp_thread_init(arg, &dummy_thread);
      |         ^~~~~~~~~~~~~~~~~
      |         smp_timer_init
/zep_workspace/sof/zephyr/lib/cpu.c:58:9: error: implicit declaration of function 'z_smp_thread_swap'; did you mean 'z_impl_k_thread_start'? [-Werror=implicit-function-declaration]
   58 |         z_smp_thread_swap();
      |         ^~~~~~~~~~~~~~~~~
      |         z_impl_k_thread_start
/zep_workspace/sof/zephyr/lib/cpu.c: In function 'cpu_enable_core':
/zep_workspace/sof/zephyr/lib/cpu.c:152:17: error: implicit declaration of function 'z_init_cpu' [-Werror=implicit-function-declaration]
  152 |                 z_init_cpu(id);
      |                 ^~~~~~~~~~
cc1: all warnings being treated as errors
[232/400] Building C object modules/sof/CMakeFiles/modules_sof.dir/zep_workspace/sof/src/audio/eq_fir/eq_fir_hifi2ep.c.obj

@marc-hb
Copy link
Collaborator

marc-hb commented Jan 18, 2024

Failures to compile with the Zephyr main branch are known and unrelated:
https://github.com/thesofproject/sof/actions/runs/7562861040/job/20594168710?pr=8754

implicit declaration of function 'z_smp_thread_init';

They're caused by backwards-incompatible change:

src/audio/module_adapter/module/modules.c Outdated Show resolved Hide resolved
@pjdobrowolski pjdobrowolski force-pushed the dynamic_module_adapter_refactor branch 4 times, most recently from cb75b1e to af9b0df Compare January 19, 2024 11:23
@pjdobrowolski pjdobrowolski force-pushed the dynamic_module_adapter_refactor branch 2 times, most recently from 2bacbbd to 5c662d9 Compare January 22, 2024 12:33
In SOF we have a two type loadable modules, IADK and native whose are
sharing the same registering mechanism but for C++/C needs IADK is using
adapter which translates methods for SOF needs.
Native loadable module is not using any additional adapters and returns
pointer for API used in built-in modules and loadable.

Signed-off-by: Dobrowolski, PawelX <pawelx.dobrowolski@intel.com>
@mwasko mwasko merged commit a082752 into thesofproject:main Jan 24, 2024
40 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants