Skip to content

[DNM][RFC] RPMsg-Lite and OpenAMP side-by-side #6153

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

Closed
wants to merge 9 commits into from
Closed

[DNM][RFC] RPMsg-Lite and OpenAMP side-by-side #6153

wants to merge 9 commits into from

Conversation

skordal
Copy link

@skordal skordal commented Feb 12, 2018

This pull request adds an example application using OpenAMP to do multiprocessor communication. It is adapted from the recently posted RPMsg-Lite example application, from PR #5960. This application is intended to provide a starting-point for comparison and benchmarking of RPMsg-Lite and OpenAMP, see issue #3065.

The relevant commit in this PR is the most recent commit. However, since the code is dependent on changes included in PR #5960, the commits from that PR is also included here.

I have added build instructions in a README.md file, located in samples/subsys/ipc/openamp/README.rst.

Some details:

  • The OpenAMP example uses the SRAMX RAM block as shared memory, to provide better separation between shared RAM and the RAM that is being used by the application. This also simplified the linking of the application.
  • Heap size had to be overridden in the default board configuration for both the cores in the LPC54114. This is because the heap size is needed (at compile-time) for libmetal, and when compiling libraries outside of the zephyr application folders, only the default board configuration is used.
  • Due to the use of static libraries, the order that the libraries are provided to the linker in matters. The libkernel.a must be linked last, and the CMakeLists.txt files contains code to fix this.

Additional assembly startup code needed when both cores are used.

Origin: NXP MCUXpresso SDK 2.2.1
URL: mcux.nxp.com
Maintained-by: External

Signed-off-by: Stanislav Poboril <[email protected]>
Add soc configuration support and dts files for nxp_lpc54xxx_m0.

Adjusted nxp_lpc54xxx soc linker, configuration and dts files
for the presence of slave core.

Added startup hook support in ARM Cortext-M arch to allow
the initialization of slave core from startup and reset.

Added hw remap of vector table for ARM Cortext-M0+ if particular soc
supports it.

Origin: Original

Signed-off-by: Stanislav Poboril <[email protected]>
Add lpcxpresso54114_m0 board dts, yaml, fixup, docs, configuration,
pinmux and header files.

Origin: Original

Signed-off-by: Stanislav Poboril <[email protected]>
Add driver for MCUX mailbox which can be used for lpcxpresso54114
and other lpc, kinetis and iMX socs.

Origin: Original

Signed-off-by: Stanislav Poboril <[email protected]>
Add ext code to be able to include image of slave core in primary
core image.

Origin: NXP MCUXpresso SDK 2.2.1
URL: mcux.nxp.com
Maintained-by: External

Signed-off-by: Stanislav Poboril <[email protected]>
Add MCUX IPM sample application. It can be run on lpcxpresso54114
board at the moment.

First, the slave core image has to be built from
samples/subsys/ipc/ipm_mcux/remote/.
Then, buid primary core image from samples/subsys/ipc/ipm_mcux/master/.

Origin: Original

Signed-off-by: Stanislav Poboril <[email protected]>
Add RPMsg-Lite version 1.2.0 to ext.

Origin: RPMsg-Lite
License: BSD 3-Clause
URL: https://github.com/codeauroraforum/rpmsg-lite
commit: 3ed2d34376a2d1ec04f59e4a89e201d820b5d5e5
Purpose: Introduction of lightweight implementation of the
         Remote Processor Messaging (RPMsg) protocol to Zephyr.
Maintained-by: External

Signed-off-by: Stanislav Poboril <[email protected]>
Add RPMsg-Lite sample application. It can be run on lpcxpresso54114
board at the moment. The application sends messages from slave
to primary core.

First, the slave core image has to be built from
samples/subsys/ipc/rpmsg_lite/remote/.
Then, buid primary core image from
samples/subsys/ipc/rpmsg_lite/master/.

Origin: Original

Signed-off-by: Stanislav Poboril <[email protected]>
@carlescufi
Copy link
Member

carlescufi commented Feb 12, 2018

@carlescufi carlescufi changed the title [DNM][RFC] OpenAMP example application [DNM][RFC] RPMsg-Lite and OpenAMP side-by-side Feb 12, 2018
Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

Doc looks OK, but there are branch conflicts.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

.rst file for our Zephyr docs

@@ -0,0 +1,72 @@
OpenAMP Example Application
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should be called README.rst (with an .rst extension)

Add a line before this with a document label to make it easier to reference externaly:

.. _openAMP_example:

@@ -0,0 +1,72 @@
OpenAMP Example Application
===========================
Copy link
Contributor

Choose a reason for hiding this comment

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

Our Zephyr doc standards use ### underlines for H1 headings, *** for H2, === for H3, and --- for H4.
So use ### underlines for this heading

===========================

Overview
--------
Copy link
Contributor

Choose a reason for hiding this comment

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

use *** underline for this heading

chip, to be used for comparison and benchmarking of the two RPC libraries.

Building this application
-------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

use *** underlines here

Building this application
-------------------------

Building and running this application requires that libmetal and OpenAMP is
Copy link
Contributor

Choose a reason for hiding this comment

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

change that last is to are


Compile and build libmetal for both cores:

$ cd libmetal
Copy link
Contributor

Choose a reason for hiding this comment

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

use a code-block here too:

.. code-block:: bash

   mkdir build-master build-remote
   cd build-master
   cmake -DWITH_ZEPHYR=ON -DBOARD=lpcxpresso54114 ..
   make
   cd ../build-remote
   cmake -DWITH_ZEPHYR=ON -DBOARD=lpcxpresso54114_m0 ..
   make


Compile and build OpenAMP for both cores:

$ cd open-amp
Copy link
Contributor

Choose a reason for hiding this comment

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

use code-block here too

Compile the remote application, by going to the toplevel folder of the example
(where this file is located) and running the following commands:

$ cd remote
Copy link
Contributor

Choose a reason for hiding this comment

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

code-block


Compile the master application, by going to the toplevel folder of the example
and running the following commands:
$ cd master
Copy link
Contributor

Choose a reason for hiding this comment

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

you know what to do (code-block)

$ make

Flash the project to the board from the master build directory:
$ make flash
Copy link
Contributor

Choose a reason for hiding this comment

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

one more code-block

This commit adds a sample application using OpenAMP for remote procedure
calls on the LPCXpresso54114. It is adapted from the RPMsg-Lite sample
application added in PR #5960, and uses the IPM driver to provide
interprocessor interrupts.

Instructions for how to compile the application, including OpenAMP and
libmetal is provided in the README.md file in the application's
toplevel directory.

Signed-off-by: Kristian Klomsten Skordal <[email protected]>
Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

Takk! (But you still have merge conflicts that need to be resolved)

@skordal
Copy link
Author

skordal commented Feb 15, 2018

Statistics

Tenative comparison of executable sizes based on a build with default settings and GCC 7.2.1. The size of the image for the second core has been subtracted from the sizes of the .text section for the master applications.

Executable .text .data .bss Mapfile
RPMsg-Lite master 16782 328 9600* rpmsg-master.map
RPMsg-Lite remote 11040 128 8544 rpmsg-remote.map
OpenAMP master 24000 664 9512 openamp-master.map
OpenAMP remote 18106 464 8456 openamp-remote.map

* The size of the shared memory section, 6144 bytes, has been subtracted from this number.

Messages received by the master application per second. Measured by removing the limit from the while loop in the examples, making the cores continually send messages between each other. Shared memory was placed either in the SRAMX block on the LPC54114 or at the end of SRAM2 (starting at 0x20026800).

Payload Size RPMsg-Lite RPMsg-Lite (SRAMX) OpenAMP OpenAMP (SRAMX)
2 11215 11145 10847 10845
4 11334 11223 10881 10870
8 11218 11110 10818 10758
16 11013 10899 10789 10640
32 10722 10501 10388 10281
64 9928 9787 9928 9815
128 8774 8615 9118 8996
256 7118 6967 7801 7716

openamp-rpmsglite 2

@MarekNovakNXP
Copy link

Cannot we have both implementations in /ext? Is it not what is /ext intended for?
Why there are for example two crypto libraries (tinycrypt and mbedtls) supported? I can see a similar situation here...
Can we discuss on this?

@PetrLukas
Copy link

@skordal could you please post also map files, you have used for code size comparison?

Did RPMSG-lite use static or dynamic allocation option? Default settings uses dynamic allocation option. There could be also used static to decrease codesize quite a bit.

Thanks, Petr

@skordal
Copy link
Author

skordal commented Feb 20, 2018

@PetrLukas I added the mapfiles to my comment above. I will get some numbers for RPMsg-Lite using the static API and add them to the table.

@PetrLukas
Copy link

@skordal Thank you for analysis and map files.

I was trying to find what is reason for big difference in master BSS size. It seems that in case of RPMSG Lite shared memory buffers are included in statistics (rpmsg_sh_mem), while in case of openAMP implementation it is not. If it would be included it would add another 30kB.

#define SHM_START_ADDRESS 0x04000400
#define SHM_SIZE 0x7c00

Speed comparison - I think it more or less depends on placement of shared buffers. In case of OpenAMP they are placed in SRAMX. It may impact performance for big buffer transfers. We should pace shared buffers to the same SRAM for both implementations.

@skordal
Copy link
Author

skordal commented Feb 23, 2018

Thanks, I did not notice that the shared memory was included in the .bss section for the RPMsg-Lite code. I have updated the numbers to account for this.

I also did an experiment with the shared memory placement. Overall, there seems to be an average performance drop of around 1 % when using SRAMX. I added the numbers to the table in the comment above for comparison.

@MarekNovakNXP
Copy link

Hello,
I reviewed the figures concerning the code size provided by @skordal and I tried to remove the zephyr code from the comparison and to keep only RPMsg-Lite / OpenAMP RPMsg code.
The difference in code size is about 60%.

I attach the excell sheet, where the computation is done. Let me know if you find any error in this benchmark, which reflects more the reality about code size comparison between OpenAMP RPMsg and RPMsg-Lite.

B/CODE remote master   remote master
lite 3106 3256   -60.8224 -58.9304
openamp 8174 7928   3.102926 REF

comparison.xlsx

@nashif nashif added the DNM This PR should not be merged (Do Not Merge) label Mar 28, 2018
@galak galak added the EXT Has change or related to ext/ (obsolete) label Apr 12, 2018
@carlescufi
Copy link
Member

@skordal @MaureenHelm should we close this now?

@skordal
Copy link
Author

skordal commented Apr 25, 2018

That's fine by me, there should be a new PR with a new example when OpenAMP is ready to be included in Zephyr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM This PR should not be merged (Do Not Merge) EXT Has change or related to ext/ (obsolete)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants