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

rtos: abstraction: partition RTOS logic into separate directories #6161

Merged
merged 4 commits into from
Aug 31, 2022

Conversation

lgirdwood
Copy link
Member

@lgirdwood lgirdwood commented Aug 18, 2022

In preparation for further RTOS abstraction, move the xtos and zephyr specific headers to new top level rtos directories to enable:

  1. Partitioning of code by RTOS type e.g. xtos, Zephyr.
  2. Partitioning of RTOS and non RTOS logic and APIs

This PR reuses the existing zephyr top level directory and creates a new xtos top level directory to store RTOS specific files, logic and wrappers. Other RTOSes like FreeRTOS or CMSIS support would be added in the same way.

This PR makes no runtime functional change, but changes some RTOS specific headers to be included with a new directory. i.e.

#include <sof/spinlock.h>

now becomes

#include <rtos/spinlock.h>

As this now maps to the correct RTOS and is no longer using the sof prefix which is for common code.

This PR also adds a high level Kconfig for developers (disabled by default) so that they can easily switch to fully native Zephyr headers for development testing.

Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com

@lgirdwood lgirdwood force-pushed the lrg/topic/headers branch 2 times, most recently from f8b7f5e to a55a08b Compare August 18, 2022 20:05
@@ -88,6 +88,10 @@ target_include_directories(SOF INTERFACE ../zephyr/include)
target_include_directories(SOF INTERFACE ${SOF_SRC_PATH}/include)
target_include_directories(SOF INTERFACE ${SOF_SRC_PATH}/arch/${ARCH}/include)

# TODO: The xtos-wrapper include PATH will eventually become optional based on the
# RTOS selection. It's manadatory atm until the native Zephyr integration is ready.
target_include_directories(SOF INTERFACE ${SOF_SRC_PATH}/arch/xtos-wrapper/include)
Copy link
Collaborator

Choose a reason for hiding this comment

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

hm, is this needed even if CONFIG_ZEPHYR_NATIVE_DRIVERS is used? If not, maybe make it conditional on this, if yes - maybe keep the headers, that Zephyr builds still need at their current locations and only move them over one by one as we stop using them in Zephyr?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, that the plan and why it's a TODO. Its still WIP atm.

@lgirdwood lgirdwood force-pushed the lrg/topic/headers branch 3 times, most recently from e5f67c4 to bf296d6 Compare August 19, 2022 20:10
@lgirdwood lgirdwood marked this pull request as ready for review August 20, 2022 10:20
@lgirdwood lgirdwood changed the title xtos: include: move xtos related headers from generic lib folder. rtos: abstraction: partition RTOS logic into separate directories Aug 20, 2022
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

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

This all makes sense to me. Unfortunately it looks likely to collide badly with some terrible (but thematically similar) hackery I've been doing to get SOF to build against the native_posix architecture for an internal fuzzing requirement. There the glitch is "xtensa assumptions in headers" and not "xthal/zephyr assumptions", but the theme is similar.

@lgirdwood
Copy link
Member Author

This all makes sense to me. Unfortunately it looks likely to collide badly with some terrible (but thematically similar) hackery I've been doing to get SOF to build against the native_posix architecture for an internal fuzzing requirement. There the glitch is "xtensa assumptions in headers" and not "xthal/zephyr assumptions", but the theme is similar.

Ah, we are thinking the same hackery then. Intention is to put all common (i.e. non RTOS logic) in include <sof/blah.h> and all the RTOS features in include <rtos/blah.h> as contamination between xtos and zephyr is super easy to do and difficult to unpick atm.

Fwiw, pls see my lrg/topic/plugin branch - I have generic FW running in POSIX environment as an ALSA plugin. i.e. the generic FW exposes a POSIX message queue for IPC which can be fuzzed. This also allows reverse fuzzing of the clients too as we have a message queue for each direction.

@lgirdwood lgirdwood force-pushed the lrg/topic/headers branch 6 times, most recently from 0edd69f to 3e37e5a Compare August 25, 2022 05:39
@lgirdwood
Copy link
Member Author

Target core power on on ALD nocodec, seems unrelated. Lets try again.

[    24794289.223097] (       13670.936523) c0 ipc                  src/ipc/ipc3/handler.c:1605 INFO ipc: new cmd 0x40070000
[    24794310.004346] (          20.781250) c0 ipc                  src/ipc/ipc3/handler.c:716  INFO ipc: pm core mask 0x9 -> enable
[    24814476.253545] (       20166.250000) c0 idc                           src/idc/idc.c:113  ERROR idc_wait_in_blocking_mode() error: timeout, target_core 3
[    24814498.180627] (          21.927082) c0 idc                ......./intel/cavs/idc.c:160  ERROR idc_send_msg(), power up core 3 failed, reason 0x0
[    24814519.274376] (          21.093750) c0 ipc                  src/ipc/ipc3/handler.c:723  ERROR Failed to enable core 3
[    24815109.274353] (         590.000000) c0 ipc                  src/ipc/ipc3/handler.c:1605 INFO ipc: new cmd 0x60010000
[    24815138.232685] (          28.958332) c0 ipc                  src/ipc/ipc3/handler.c:207  ERROR ipc: comp 41 not found
[    26621389.150494] (     1806250.875000) c0 ll-schedule        ./schedule/ll_schedule.c:186  INFO task 0x9e19ec18 agent-work  avg 110, max 127

@lgirdwood
Copy link
Member Author

SOFCI TEST

@lgirdwood lgirdwood force-pushed the lrg/topic/headers branch 4 times, most recently from f6efd11 to 7a038f5 Compare August 29, 2022 19:42
No functional runtime change, but changes to rtos partitioning and the
layout of headers .

This patch creates RTOS specifc header paths and updates spinlock.h
and kernel.h to show the new usage. Other headers will incrementally follow.
It reuses the current zephyr topleve directory and creates a new
toplevel xtos directory for xtos specific files.

Due to the mixing of RTOS, driver and library headers at the top level include
directory it was necessary to create rtos specific header directories i.e.

src/include/rtos-xtos
src/include/rtos-zephyr

These RTOS include directories will eventually contain RTOS specific headers
whilst common logic and structures will be placed in non RTOS directories.

This will also mean

"#include <sof/spinlock.h>"

will become

"#include <rtos/spinlock.h>"

and will allow easier visualisation of where and why RTOS headers are being used.
This will help to eliminate cross usage of headers between RTOSes.

Subsequqnt patches will move more headers and rtos specific wrppaer
source files into rtos specific locations.

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
No runtime functional change. Code can now include <rtos/atomic.h>

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
No runtime functional change. Code can now include <rtos/bit.h>

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
No runtime semantic change. Use C library when RTOS uses
C library otherwise use own C library calls.

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
@lgirdwood
Copy link
Member Author

License server issues on retired platforms. No need to rebuild.

@lgirdwood lgirdwood merged commit c90055f into main Aug 31, 2022
@lgirdwood lgirdwood deleted the lrg/topic/headers branch August 31, 2022 11:01
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.

6 participants