Samsung / TizenRT

TizenRT is a lightweight RTOS-based platform to support low-end IoT devices
Apache License 2.0
566 stars 571 forks source link

os/board/rtl8730e&rtl8721csm: Support CMSIS #6196

Closed ZhenBei-Sin closed 3 months ago

ZhenBei-Sin commented 4 months ago

external/cmsis: Add CMSIS in the external Change Notes:

apps/examples/board_specific_demo: Add CMSIS example Change Notes:

build/configs/rtl8730e&rtl8721csm: Adjust defconfig for CMSIS rtl8730e: Support CMSIS-NN rtl8721csm: Support CMSIS-NN and CMSIS-DSP

Change Notes:

Taejun-Kwon commented 4 months ago

In our side, we checked performance became better. How about your side?

Dane-Kang commented 4 months ago

In our side, we checked performance became better. How about your side?

Hi @Taejun-Kwon , May i know if you tested on loadable app config also ? In our side, There are link issue like over during build. https://github.com/Samsung/TizenRT/pull/6196#discussion_r1619915158

So, Zhenbei asked how to resolve it .

vibhor-m commented 4 months ago

May i know if you tested on loadable app config also ?

We tested loadable configuration with cmsis_nn, it was working fine. Now I see @sunghan-chang has suggested not to change os/*.mk files. So we will check the compilation again and share result with you.

aadotverma commented 3 months ago

In our side, we checked performance became better. How about your side?

Hi @Taejun-Kwon , May i know if you tested on loadable app config also ? In our side, There are link issue like over during build. #6196 (comment)

So, Zhenbei asked how to resolve it .

Hello @ZhenBei-Sin, Please refer the following commit for resolving cmsis library linking issue. https://github.com/aadotverma/TizenRT/commit/5e5239027f328d62be540b30ec7eb1bcf4b145cb Compilation is success for cmsis_nn for both rtl8721csm/hello & rtl8721csm/loadable_apps configuration. but for cmsis_dsp, multiple definition error is coming for rtl8721csm/loadable_apps. I checked, some symbols are defined two times in cmsis_dsp library.

ZhenBei-Sin commented 3 months ago

In our side, we checked performance became better. How about your side?

Hi @Taejun-Kwon , May i know if you tested on loadable app config also ? In our side, There are link issue like over during build. #6196 (comment) So, Zhenbei asked how to resolve it .

Hello @ZhenBei-Sin, Please refer the following commit for resolving cmsis library linking issue. aadotverma@5e52390 Compilation is success for cmsis_nn for both rtl8721csm/hello & rtl8721csm/loadable_apps configuration. but for cmsis_dsp, multiple definition error is coming for rtl8721csm/loadable_apps. I checked, some symbols are defined two times in cmsis_dsp library.

Hi @aadotverma ,

Thank you for assist to archive cmsis lib into libexternal.a for loadable_apps.

I updated the latest lib_cmsis_dsp.a, which solved the multiple definition error. Root Cause: Include the .c multiple time cause multiple definition (example: ComplexMathFunctions.c) https://github.com/ARM-software/CMSIS-DSP/blob/v1.15.0/Source/ComplexMathFunctions/ComplexMathFunctions.c

ZhenBei-Sin commented 3 months ago

Let's split the commit as 3 commits

  1. add cmsis in the external (including removing from os)
  2. add example in the apps
  3. adjust defconfig for cmsis

Solved the link lib for loadable_apps. Will proceed to resolve the conflicts and separate to 3 commits.

ZhenBei-Sin commented 3 months ago

Hi @sunghan-chang , @Taejun-Kwon , @vibhor-m , @aadotverma

Please help to review the final changes. Thank You.