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

[BUG] make CONFIG_SOF_ZEPHYR_STRICT_HEADERS the default #9015

Closed
39 tasks done
kv2019i opened this issue Apr 9, 2024 · 10 comments
Closed
39 tasks done

[BUG] make CONFIG_SOF_ZEPHYR_STRICT_HEADERS the default #9015

kv2019i opened this issue Apr 9, 2024 · 10 comments
Assignees
Labels
enhancement New feature or request Zephyr Issues only observed with Zephyr integrated
Milestone

Comments

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 9, 2024

Describe the bug/enhancement
When rtos include layer was added in #6161 , CONFIG_SOF_ZEPHYR_STRICT_HEADERS was added as a transitory tool.

This is still in place and can cause confusing issues as both XTOS and Zephyr headers are added to the include path by default.

To Reproduce
Build target, check .config

Reproduction Rate
100%

Expected behavior
OS include paths not mixed in build.

Impact
Slows down development of new features.

Environment
SOF main as of 2024-04-09

Work breakdown

Following subtasks identified (breakdown started in v2.11 cycle):

Screenshots or console output

@kv2019i kv2019i added bug Something isn't working as expected Zephyr Issues only observed with Zephyr integrated labels Apr 9, 2024
@lgirdwood lgirdwood added this to the v2.10 milestone Apr 16, 2024
@kv2019i kv2019i self-assigned this May 28, 2024
@kv2019i
Copy link
Collaborator Author

kv2019i commented May 28, 2024

Not sure I can tackle this in time for v2.10, but assigning for myself, as nobody else is assigned.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Jun 5, 2024

Update for v2.10: spent some time on this trying to make one Intel target build in strict headers mode, but could not complete the task. There is still quite a lot of use of e..g sof/lib/mailbox.h and sof/lib/memory.h in surprising places in audio code that needs to be cleaned up first. Also we have some SOF side functions list SOF list.h and perf_cnt -- for these, move to some common header might be easier. For now moving to v2.11 -- might be able to complete some of this work in June, but won't backport to 2.10 release (as no functional benefit for the release).

@kv2019i kv2019i modified the milestones: v2.10, v2.11 Jun 5, 2024
@kv2019i kv2019i added enhancement New feature or request and removed bug Something isn't working as expected labels Sep 10, 2024
@kv2019i
Copy link
Collaborator Author

kv2019i commented Sep 10, 2024

Work breakdown added to the issue description with already done worked marked as done.

kv2019i added a commit to kv2019i/sof that referenced this issue Sep 10, 2024
Implement sof/lib/dai.h for Zephyr build and do not rely o
the xtos version for Zephyr builds. Add a warning to catch
invalid build configurations.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
dbaluta pushed a commit that referenced this issue Sep 11, 2024
Implement sof/lib/dai.h for Zephyr build and do not rely o
the xtos version for Zephyr builds. Add a warning to catch
invalid build configurations.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
pillo79 pushed a commit to pillo79/sof that referenced this issue Sep 11, 2024
Implement sof/lib/dai.h for Zephyr build and do not rely o
the xtos version for Zephyr builds. Add a warning to catch
invalid build configurations.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Sep 11, 2024
Remove the shim.h interface from RTOS layer as there is no use
of this interface anymore in SOF codebase.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Sep 12, 2024
sof/list.h is a software interface used by the audio pipeline
framework and should not be in the RTOS abstraction layer.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Sep 13, 2024

2.11 cutoff day. Some progress with this. Work breakdown has been done and shared as a task list in bug description. At end of 2.11 cycle, we have 23 out of 35 identified tasks completed. Pushing the remaining work to v2.12.

@kv2019i kv2019i modified the milestones: v2.11, v2.12 Sep 13, 2024
kv2019i added a commit that referenced this issue Sep 16, 2024
sof/list.h is a software interface used by the audio pipeline
framework and should not be in the RTOS abstraction layer.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit that referenced this issue Sep 16, 2024
Remove the shim.h interface from RTOS layer as there is no use
of this interface anymore in SOF codebase.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Sep 18, 2024
Introduce a separate file for Zephyr compiler_attributes.h and
move all Zephyr-specific definitions to this file. This is
a prerequisite to build with CONFIG_SOF_ZEPHYR_STRICT_HEADERS=y.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Sep 27, 2024
The SOF perf_cnt.h provides a simple performance counter interface that
is used in SOF to track performance at audio module and pipeline level.

Majority of the implementation is RTOS agnostic, relying on
sof_cycle_get_64() to sample platform clock, and timer_get_system() for
CPU clock, both defined in rtos/timer.h. There is however some
conditional rules for Zephyr to use timing_counter_get() if SOF is built
with CONFIG_TIMING_FUNCTIONS=y.

The amount of RTOS variation does not seem to warrant branching the
whole perf_cnt.h to RTOS layer. Move perf_cnt.h back to application
interface, so the single implementation can be shared.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
lgirdwood pushed a commit that referenced this issue Oct 1, 2024
The SOF perf_cnt.h provides a simple performance counter interface that
is used in SOF to track performance at audio module and pipeline level.

Majority of the implementation is RTOS agnostic, relying on
sof_cycle_get_64() to sample platform clock, and timer_get_system() for
CPU clock, both defined in rtos/timer.h. There is however some
conditional rules for Zephyr to use timing_counter_get() if SOF is built
with CONFIG_TIMING_FUNCTIONS=y.

The amount of RTOS variation does not seem to warrant branching the
whole perf_cnt.h to RTOS layer. Move perf_cnt.h back to application
interface, so the single implementation can be shared.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 8, 2024
Implement sof/lib/memory.h for Zephyr build and do not rely on
the xtos version for Zephyr builds.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 9, 2024
The SOF DMA RTOS interface is one of the largest and also one of the
hardest to maintain, as it's used a lot in SOF, and there is unfortunate
overlap in interface naming between Zephyr and XTOS.

To get the cleanup work started, branch out the sof/lib/dma.h for Zephyr
and remove the conditional definitions from both versions. This patch
maintains support for all build variants, including building SOF with
Zephyr but using XTOS drivers.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 9, 2024
The SOF mailbox.h provides an interface to host-DSP mailboxes, which is
used widely in SOF IPC and debug code.

This interface is OS agnostistic and only relies on platform/lib/mailbox.h
to define the mailbox locations, and rtos/cache.h to define portable cache
primitives to invalidate/writeback mailbox data before/after use.

The amount of RTOS variation does not seem to warrant branching the
whole mailbox.h to RTOS layer. Move mailbox.h back to application
interface, so the single implementation can be shared.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Oct 9, 2024

Added: xtos/sof/trace/preproc.h

kv2019i added a commit that referenced this issue Oct 10, 2024
Implement sof/lib/memory.h for Zephyr build and do not rely on
the xtos version for Zephyr builds.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
lgirdwood pushed a commit that referenced this issue Oct 16, 2024
The SOF DMA RTOS interface is one of the largest and also one of the
hardest to maintain, as it's used a lot in SOF, and there is unfortunate
overlap in interface naming between Zephyr and XTOS.

To get the cleanup work started, branch out the sof/lib/dma.h for Zephyr
and remove the conditional definitions from both versions. This patch
maintains support for all build variants, including building SOF with
Zephyr but using XTOS drivers.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
lgirdwood pushed a commit that referenced this issue Oct 16, 2024
The SOF mailbox.h provides an interface to host-DSP mailboxes, which is
used widely in SOF IPC and debug code.

This interface is OS agnostistic and only relies on platform/lib/mailbox.h
to define the mailbox locations, and rtos/cache.h to define portable cache
primitives to invalidate/writeback mailbox data before/after use.

The amount of RTOS variation does not seem to warrant branching the
whole mailbox.h to RTOS layer. Move mailbox.h back to application
interface, so the single implementation can be shared.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 17, 2024
Add Zephyr version of sof/init.h. This is used define main
entry points to the SOF application. The arch_init() entry
point is not needed on Zephyr.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 18, 2024
Move the SOF trace implementation back to app level. While
Zephyr logging is preferred solution for newer targets,
the old sof-logger can be supported in Zephyr and code
exists to support this usage.

Move the trace preprocessor back to app level and allow
it to be used if CONFIG_TRACE=y is set in the build.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 18, 2024
Add Zephyr version of sof/init.h. This is used define main
entry points to the SOF application. The arch_init() entry
point is not needed on Zephyr.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 21, 2024
Add Zephyr version of sof/trace/preproc.h interface. The full
preproc.h interface needed for CONFIG_TRACE is not duplicated
here, so this commit effectively prevents using CONFIG_TRACE
in Zephyr.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 21, 2024
Add Zephyr version of sof/init.h. This is used define main
entry points to the SOF application. The arch_init() entry
point is not needed on Zephyr.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
lgirdwood pushed a commit that referenced this issue Oct 21, 2024
Add Zephyr version of sof/init.h. This is used define main
entry points to the SOF application. The arch_init() entry
point is not needed on Zephyr.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 21, 2024
Add Zephyr version of sof/trace/preproc.h interface. The full
preproc.h interface needed for CONFIG_TRACE is not duplicated
here, so this commit effectively prevents using CONFIG_TRACE
in Zephyr.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit that referenced this issue Oct 25, 2024
Add Zephyr version of sof/trace/preproc.h interface. The full
preproc.h interface needed for CONFIG_TRACE is not duplicated
here, so this commit effectively prevents using CONFIG_TRACE
in Zephyr.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
kv2019i added a commit to kv2019i/sof that referenced this issue Oct 30, 2024
Remove CONFIG_SOF_ZEPHYR_STRICT_HEADERS and make strict headers mode the
only supported way to build SOF with Zephyr. This means SOF Zephyr
builds do not use any headers from sof/xtos/include anymore.

This change simplifies the SOF build as full RTOS adaptation is
in sof/zephyr/include for Zephyr builds.

Link: thesofproject#9015
Signed-off-by: Kai Vehmanen <[email protected]>
lgirdwood pushed a commit that referenced this issue Oct 30, 2024
Remove CONFIG_SOF_ZEPHYR_STRICT_HEADERS and make strict headers mode the
only supported way to build SOF with Zephyr. This means SOF Zephyr
builds do not use any headers from sof/xtos/include anymore.

This change simplifies the SOF build as full RTOS adaptation is
in sof/zephyr/include for Zephyr builds.

Link: #9015
Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Oct 31, 2024

With #9627 all work for this done, marking this as closed.

@kv2019i kv2019i closed this as completed Oct 31, 2024
@kv2019i
Copy link
Collaborator Author

kv2019i commented Nov 29, 2024

@LaurentiuM1234 wrote:

@kv2019i wrote:

Another approach might be to improve and scale up the "adsp_host_ipc" DT interface used by zephyr/soc/intel/
intel_adsp/common/ipc.c . This is essentially the driver interface to talk to host and I think for longterm it would be

I like the idea of using a driver for the IPC stuff. One aparent flaw I've noticed with our current mailbox implementation
is that it's not really MMU friendly. On imx93 (ARM64 architecture, MMU enabled) we had to deal with this by adding
some static phys-virt mappings inside mmu_regions.c. I'm thinking that, ideally, this shouldn't be required. Instead, we

I filed now a proper enhancement ticket for this #9697
Rest of platform layer starts to be simplified now -- a few tough nuts to crack left, mailbox.h is one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Zephyr Issues only observed with Zephyr integrated
Projects
None yet
Development

No branches or pull requests

3 participants