Move the SOF app from zephyr to where it belongs #6004

nashif · 2022-07-13T02:40:28Z

This moves (not deleted in zephyr yet) the SOF app into SOF, where it belongs.
No need for duplicated overlays and one place to build everything.

You now use scripts or west directly to build this and able to make all changes needed in the configuration files without having go via zpehyr or go through the complexity of a second layer of configurations (overlays)

zephyr: use zephyr prefix for includes
zephyr: app: move main SOF app from zephyr samples
zephyr: module: point to where the app is
app: zephyr: remove overlays and adapt script

lgirdwood

LGTM - thank you @nashif

@marc-hb @keqiaozhang @wszypelt does anything need to be done for CI to use this update ?

@aborisovich good for you and Windows ?

aborisovich

Quick review, two minor python issues.
Going to test Windows build in a minute,
As I understand the idea we are going to use overlays as common for everyone?
I am not sure this is right.
Yes we can do this for debug_overlay.conf but it does not make sense to use IPC4 overlay with anothing older than TGL - it probably won't even build and nobody did it before.
I would go back to old approach with platform names, something like this:

Then we can see which overlays are what for. Also please note that "platform" is better term here because we have some platforms in SOF that are build without Zephyr.
One more thing - I would change a name from "app" to "cSOF" maybe?
Like "Converged SOF" so it matches our official name :-)

scripts/xtensa-build-zephyr.py

nashif · 2022-07-13T13:57:24Z

Yes we can do this for debug_overlay.conf but it does not make sense to use IPC4 overlay with anothing older than TGL - it probably won't even build and nobody did it before.

Sure, this can be put in a strucutre with platform names.

I would go back to old approach with platform names, something like this:

boards/ is a preset folder name in the zephyr build system, the right configuration file is pulled automatically based on what you are building, so, no need for overlay magic, very useful if you are not building using the python script.

One more thing - I would change a name from "app" to "cSOF" maybe?
Like "Converged SOF" so it matches our official name :-)

app is just a placeholder, you guys decide what name to use...

marc-hb · 2022-07-13T23:22:07Z

Thanks Anas for helping with this, much appreciated.

@marc-hb @keqiaozhang @wszypelt does anything need to be done for CI to use this update ?

Sorry I don't know why the Zephyr build was successful in https://github.com/thesofproject/sof/runs/7322499212 but failed in https://sof-ci.01.org/sofpr/PR6004/build891/build (error below). The changes to sof/scripts/xtensa-build-zephyr.py are small and simple; they look good to me.

SOF_TOP=/srv/home/jenkins/workspace/sof_generic_build
west_top=/srv/home/jenkins/workspace
In dir: /srv/home/jenkins/workspace; running command: west status hal_xtensa sof
FATAL ERROR: file not found: /srv/home/jenkins/workspace/sof_generic_build/west.yml

@keqiaozhang, @XiaoyunWu6666 ?

Jenkins does a bit more with sof/scripts/xtensa-build-zephyr.py, I found other suspicious errors in jenkins job 3667:

07:05:26 Traceback (most recent call last):
07:05:26   File "scripts/xtensa-build-zephyr.py", line 631, in <module>
07:05:26     main()
07:05:26   File "scripts/xtensa-build-zephyr.py", line 624, in main
07:05:26     run_clone_mode()
07:05:26   File "scripts/xtensa-build-zephyr.py", line 594, in run_clone_mode
07:05:26     raise RuntimeError("Zephyr found already! Not downloading it again")
07:05:26 RuntimeError: Zephyr found already! Not downloading it again
07:05:26 fatal: cannot change to 'zephyrproject/zephyr': No such file or directory

aborisovich

Sorry for delays in testing Windows build (i need to configure my environment).
I'll let you know as soon as its verified.
Meanwhile found one more thing to python (enable MTL build).
Rest of changes looks pretty good to me.

aborisovich · 2022-07-13T23:48:09Z

scripts/xtensa-build-zephyr.py

You added configuration for MTL board, please add missing python dictionary so we can build it too:
{
"name": "mtl",
"PLAT_CONFIG": "intel_adsp_ace15_mtpm",
"XTENSA_CORE": "ace10_LX7HiFi4_RI_2020_5",
"XTENSA_TOOLS_VERSION": f"RI-2020.5{xtensa_tools_version_postfix}",
"RIMAGE_KEY": pathlib.Path("modules", "audio", "sof", "keys", "otc_private_key_3k.pem
},

I will probably remove it and leave it to you to add this alongside other mtl related changes.

keqiaozhang · 2022-07-14T08:18:48Z

SOFCI TEST

keqiaozhang · 2022-07-14T08:29:50Z

Jenkins does a bit more with sof/scripts/xtensa-build-zephyr.py, I found other suspicious errors in jenkins job 3667:

@marc-hb , this build failure is not caused by this PR. I checked the Jenkins log and found that our sofbld10 failed to clone zephyrporject, because there's .west folder in /srv/home/jenkins/workspace/. I have deleted this folder and zephyr build passed.

aborisovich · 2022-07-14T10:05:38Z

Sorry for delays in testing Windows build (i need to configure my environment). I'll let you know as soon as its verified. Meanwhile found one more thing to python (enable MTL build).

Confirmed Windows build working (built TGL successfully).

aborisovich · 2022-07-14T10:18:26Z

@marc-hb , this build failure is not caused by this PR. I checked the Jenkins log and found that our sofbld10 failed to clone zephyrporject, because there's .west folder in /srv/home/jenkins/workspace/. I have deleted this folder and zephyr build passed.

It may be my fault as I'm messing around with processes in PR6005, sorry.

marc-hb · 2022-07-14T16:26:14Z

It may be my fault as I'm messing around with processes in #6005, sorry.

Thanks @aborisovich , very good catch I didn't realize 6005 could break CI even before getting merged but because of big west differences then yes of course it can! Ideally it shouldn't but in practice it can because our workers clean but don't clone everything from scratch. I prefixed the Github PR title with [SKIP CI] to stop sof-jenkins from looking at it. This affects sof-jenkins CI ONLY, ignored by everything else. You do not need to change any commit message or anything.

marc-hb

https://sof-ci.01.org/sofpr/PR6004/build896/devicetest/ is all green expect one unrelated and unfortunately common suspend/resume failure.

The Python, CMake and C changes LGTM, thanks! The checkpatch warnings seem harmless, someone should check.

I don't know about the overlays and .conf files but I blindly trust @nashif with that :-) But @aborisovich must approve.

lgirdwood · 2022-07-15T08:41:14Z

@aborisovich good for you now ?

aborisovich

Never trust anyone blindly, we are all developers ;-)
I just wanted to remove boards/intel_adsp_ace15_mtpm.conf from this PR just for consistency (or add it to python so it is buildable). But maybe it does not matter that much.
Everything else looks perfect, windows build works too, approving.

nashif · 2022-07-15T10:44:16Z

I just wanted to remove boards/intel_adsp_ace15_mtpm.conf from this PR just for consistency (or add it to python so it is buildable).

removed

Zephyr includes are now under a zephyr namespace, so use this. Signed-off-by: Anas Nashif <anas.nashif@intel.com>

Make the main SOF app part of SOF, where it belongs. No need for any overlays (which just duplicated the sample .conf anyways) and one place to build everything. This now does not depend on zephyr samples, which have a different purpose completely. Signed-off-by: Anas Nashif <anas.nashif@intel.com>

This allows building SOF from zephyr for CI and testing purposes. Signed-off-by: Anas Nashif <anas.nashif@intel.com>

Remove overlays as they are the same board configurations available in app/. Keep common overlays and change script to reflect those changes. Signed-off-by: Anas Nashif <anas.nashif@intel.com>

lgirdwood · 2022-07-15T13:04:23Z

@wszypelt can you check CI, this should not break runtime tests

wszypelt · 2022-07-15T15:03:04Z

@lgirdwood
It seems to me that everything should be working properly now, I have made retests for this and other PR

lgirdwood · 2022-07-15T15:52:20Z

@lgirdwood It seems to me that everything should be working properly now, I have made retests for this and other PR

Thanks for checking @wszypelt - I will rerun as the CI error log is still not loading for me.

lgirdwood · 2022-07-15T15:52:29Z

SOFCI TEST

lgirdwood · 2022-07-15T16:44:37Z

@wszypelt I've rerun the CI and still getting empty logs for the test failures. Are we good to merge ?

marc-hb · 2022-07-15T17:07:50Z

@lgirdwood the magic comment "SOFCI TEST" is scanned by SOFCI TEST only, QB does not care.

I looked at the internal 9877300 logs for https://sof-ci.01.org/sof-pr-viewer/#/build/PR6004/build9877263 and this can merged. I confirm that the build in QB was successful, only the deployment to 2 devices (TGL and WHL) failed:

pytest tests/sof/fw_00_basic/test_00_load_fw_basic.py::TestLoadFwBasic   --fw="C:\Testing\Builds\pull-6004-merge_740929\CNL\sof-cnl.ri" --ldc-file="C:\Testing\Builds\pull-6004-merge_740929\CNL\sof-cnl.ldc" --zabbix --tests-startup-label='SMOKE_pull-6004-merge_740929' ...
...
04:34:51,363 INFO  - INTERNALERROR>   File "/usr/local/lib/python3.6/dist-packages/execnet/gateway_socket.py", line 91, in create_io
04:34:51,363 INFO  - INTERNALERROR>     sock.connect((host, port))
04:34:51,363 INFO  - INTERNALERROR> ConnectionRefusedError: [Errno 111] Connection refused

I doubt this PR even affects QB at all.

(https://sof-ci.01.org/sofpr/PR6004/build906/devicetest/ is all green except for one unfortunately common suspend/resume failure.)

lgirdwood approved these changes Jul 13, 2022

View reviewed changes

nashif marked this pull request as ready for review July 13, 2022 11:08

nashif requested review from aborisovich, dbaluta, lbetlej, marc-hb, mmaka1 and plbossart as code owners July 13, 2022 11:08

aborisovich suggested changes Jul 13, 2022

View reviewed changes

scripts/xtensa-build-zephyr.py Outdated Show resolved Hide resolved

scripts/xtensa-build-zephyr.py Outdated Show resolved Hide resolved

nashif force-pushed the zephyr_app branch 2 times, most recently from 704d6cf to 06ccfac Compare July 13, 2022 13:45

nashif force-pushed the zephyr_app branch from 06ccfac to dbf12e6 Compare July 13, 2022 14:00

marc-hb mentioned this pull request Jul 13, 2022

add sparse build support #5984

Closed

aborisovich suggested changes Jul 13, 2022

View reviewed changes

aborisovich mentioned this pull request Jul 14, 2022

[SKIP CI] Use west manifest in SOF #6005

Merged

marc-hb approved these changes Jul 14, 2022

View reviewed changes

marc-hb self-requested a review July 14, 2022 22:39

aborisovich approved these changes Jul 15, 2022

View reviewed changes

nashif force-pushed the zephyr_app branch from dbf12e6 to 3f06c61 Compare July 15, 2022 10:44

nashif added 2 commits July 15, 2022 06:51

zephyr: use zephyr prefix for includes

60c9e95

Zephyr includes are now under a zephyr namespace, so use this. Signed-off-by: Anas Nashif <anas.nashif@intel.com>

nashif added 2 commits July 15, 2022 06:51

zephyr: module: point to where the app is

a881cb8

This allows building SOF from zephyr for CI and testing purposes. Signed-off-by: Anas Nashif <anas.nashif@intel.com>

app: zephyr: remove overlays and adapt script

7f22382

Remove overlays as they are the same board configurations available in app/. Keep common overlays and change script to reflect those changes. Signed-off-by: Anas Nashif <anas.nashif@intel.com>

nashif force-pushed the zephyr_app branch from 3f06c61 to 7f22382 Compare July 15, 2022 10:51

nashif requested review from akloniex, libinyang and mrajwa as code owners July 15, 2022 10:51

aborisovich mentioned this pull request Jul 18, 2022

Removed obsolete xtensa-build-zephyr.sh #6016

Merged

lgirdwood merged commit afd28fb into thesofproject:main Jul 18, 2022

marc-hb mentioned this pull request Nov 2, 2023

power_domain_intel_adsp.c: revert recent INIT_PRIORITY change zephyrproject-rtos/zephyr#64712

Merged

Move the SOF app from zephyr to where it belongs #6004

Move the SOF app from zephyr to where it belongs #6004

Uh oh!

Conversation

nashif commented Jul 13, 2022

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

aborisovich left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nashif commented Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marc-hb commented Jul 13, 2022

Uh oh!

aborisovich left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aborisovich Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

nashif Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

keqiaozhang commented Jul 14, 2022

Uh oh!

keqiaozhang commented Jul 14, 2022

Uh oh!

aborisovich commented Jul 14, 2022

Uh oh!

aborisovich commented Jul 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marc-hb commented Jul 14, 2022

Uh oh!

marc-hb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Jul 15, 2022

Uh oh!

aborisovich left a comment

Choose a reason for hiding this comment

Uh oh!

nashif commented Jul 15, 2022

Uh oh!

lgirdwood commented Jul 15, 2022

Uh oh!

wszypelt commented Jul 15, 2022

Uh oh!

lgirdwood commented Jul 15, 2022

Uh oh!

lgirdwood commented Jul 15, 2022

Uh oh!

lgirdwood commented Jul 15, 2022

Uh oh!

marc-hb commented Jul 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aborisovich left a comment •

edited

Loading

nashif commented Jul 13, 2022 •

edited

Loading

aborisovich left a comment •

edited

Loading

aborisovich commented Jul 14, 2022 •

edited

Loading

marc-hb left a comment •

edited

Loading