Skip to content

Conversation

@ujfalusi
Copy link
Contributor

@ujfalusi ujfalusi commented Dec 18, 2023

The default firmware file path is standardized among vendors as follows:

IPC3
    /lib/firmware/VENDOR/sof/
    ├── community
    │   └── sof-PLAT.ri
    ├── dbgkey
    │   └── sof-PLAT.ri
    └── sof-PLAT.ri
IPC4
    /lib/firmware/VENDOR/sof-ipc4/
    └── PLAT
        ├── community
        │   └── sof-PLAT.ri
        ├── dbgkey
        │   └── sof-PLAT.ri
        └── sof-PLAT.ri\n

Currently the binaries created by the build can only be used for direct
deployment on IPC3 platforms but if one builds different vendor firmwares
the files have to be manually picked and sorted out.
We have two flags: --fw-naming and --use-platform-subdir which can be
played with but still not going to produce deployable build.

Introduce a new flag: --deployable-build
With the flag specified all other modificators are going to be ignored and
the build will do the 'right thing' to create a directory structure which
can be deployed as it is to the target's firmware directory.

To achieve this several changes needed:
Update the platform aliases

PlatformConfig:

  • drop the name member and replace it with a vendor string
  • add a flag to indicate IPC4 platforms
    Later a new option can be added if needed for platforms which can be
    IPC3 or IPC4

Ignore fw-naming and use-platform-subdir in case of deployable build. The
options will be reset to their default in case they are changed.

symlink_or_copy extended to handle relative symlinks when the target and
link is not under the same directory.

The --deployable-build is disabled by default, it has to be enabled to
create deployable build for now.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com

The IPC4_CONFIG_OVERLAY is no longer used, remove it.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The following aliases are missing:

tgl: adl-n, rpl
tgl-h: rpl-s
mtl: arl

Update the aliases list accordingly.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Print platform alias information to reduce the need for guessing when
trying to figure out what platform supports what platform.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
@ujfalusi ujfalusi requested a review from marc-hb as a code owner December 18, 2023 11:33
@ujfalusi ujfalusi changed the title xtensa-build-zephyr.py: Add option to create deployable build xtensa-build-zephyr.py: Deployable build Dec 18, 2023
@ujfalusi
Copy link
Contributor Author

ujfalusi commented Dec 18, 2023

The resulting firmware build folder will be for command
./scripts/xtensa-build-zephyr.py -p --deployable-build tgl mtl imx8 imx8x:

build-sof-staging 
|-- sof 
|   |-- imx 
|   |   +-- sof 
|   |       |-- community 
|   |       |   |-- sof-imx8.ri         sha256=7276f0cf11e83e8b20ef892e0d34136055138355eb8039a11444f513fe1f1f9d
|   |       |   +-- sof-imx8x.ri        sha256=91c17e06e0b7319db729a1b344965245f946dbc20f551b6206981a33d0936021
|   |       |-- sof-imx8.ldc    sha256=6cab656a8f07f972babb2d68e5c50cadf1f9a0868bb5e4a0a8ab3e0953299739
|   |       +-- sof-imx8x.ldc   sha256=6cab656a8f07f972babb2d68e5c50cadf1f9a0868bb5e4a0a8ab3e0953299739
|   +-- intel 
|       +-- sof-ipc4 
|           |-- adl 
|           |   +-- community 
|           |       +-- sof-adl.ri 
|           |-- adl-n 
|           |   +-- community 
|           |       +-- sof-adl-n.ri 
|           |-- arl 
|           |   +-- community 
|           |       +-- sof-arl.ri 
|           |-- ehl 
|           |   +-- community 
|           |       +-- sof-ehl.ri 
|           |-- mtl 
|           |   +-- community 
|           |       +-- sof-mtl.ri      sha256=1c83c7efa21750142a19a8d73573cf7c5d36a1e224cad0cf05ceca1e01102a03
|           |-- rpl 
|           |   +-- community 
|           |       +-- sof-rpl.ri 
|           |-- sof-adl-n.ldc 
|           |-- sof-adl.ldc 
|           |-- sof-arl.ldc 
|           |-- sof-ehl.ldc 
|           |-- sof-mtl.ldc     sha256=74e16520af6c6f3f30c185fe5a3bb370c24a8b8cbbfe9317f62b0d27d5fa6521
|           |-- sof-rpl.ldc 
|           |-- sof-tgl.ldc     sha256=c77d6ab4268724d54a4cd61aece665cf99953e79eac2307b1ee6da47a6d0454a
|           +-- tgl 
|               +-- community 
|                   +-- sof-tgl.ri      sha256=9b6ba54b51bac6b9d1835bc92a6c68e1a3e00299677eea14517e4ec6713b5354

@ujfalusi ujfalusi force-pushed the peter/pr/deployable_build_01 branch from 3b2e3b3 to 03abba1 Compare December 18, 2023 12:43
The default firmware file path is standardized among vendors as follows:
IPC3
    /lib/firmware/VENDOR/sof/
    ├── community
    │   └── sof-PLAT.ri
    ├── dbgkey
    │   └── sof-PLAT.ri
    └── sof-PLAT.ri
IPC4
    /lib/firmware/VENDOR/sof-ipc4/
    └── PLAT
        ├── community
        │   └── sof-PLAT.ri
        ├── dbgkey
        │   └── sof-PLAT.ri
        └── sof-PLAT.ri\n

Currently the binaries created by the build can only be used for direct
deployment on IPC3 platforms but if one builds different vendor firmwares
the files have to be manually picked and sorted out.
We have two flags: --fw-naming and --use-platform-subdir which can be
played with but still not going to produce deployable build.

Introduce a new flag: --deployable-build
With the flag specified all other modificators are going to be ignored and
the build will do the 'right thing' to create a directory structure which
can be deployed as it is to the target's firmware directory.

To achieve this several changes needed:
PlatformConfig:
- drop the name member and replace it with a vendor string
- add a flag to indicate IPC4 platforms
  Later a new option can be added if needed for platforms which can be
  IPC3 or IPC4

Ignore fw-naming and use-platform-subdir in case of deployable build. The
options will be reset to their default in case they are changed.

symlink_or_copy extended to handle relative symlinks when the target and
link is not under the same directory.

The --deployable-build is disabled by default, it has to be enabled to
create deployable build for now.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
@ujfalusi
Copy link
Contributor Author

Changes since v1:

  • properly clean the staging directory (remove IPC4 deployable build directories)

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks @ujfalusi for doing this! A few minor comments inline, but no blockers.

if args.use_platform_subdir:
args.use_platform_subdir = False
print("The option '--use-platform-subdir' is ignored for deployable builds.")

Copy link
Collaborator

Choose a reason for hiding this comment

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

One thing that is not obvious in either git commit or inline comments is what is the relation between deployable_build and the old options. If I've understood right, this is really a transition mechanism, we should not need the old fw-naming/platform-subdir options anymore once. I.e. deployable-build can itself become a deprecate and just be the default. I mean when/why would one need a non-deployable build? :)

Copy link
Contributor Author

@ujfalusi ujfalusi Dec 18, 2023

Choose a reason for hiding this comment

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

@kv2019i, @marc-hb, we sort of had deployable IPC3 output (no --fw-naming and --use-platform-subdir).
For IPC4 we never had deployable layout, not even with the variation of the two flags.
--fw-naming forces --use-platform-subdir and you will end up with build-sof-staging/sof/tgl/community/dsp_basefw.bin and bunch of .ldc files under sof directory, you still need to sort out the adl, rpl, etc things.
--use-platform-subdir alone would create build-sof-staging/sof/tgl/community/sof-tgl.ri and the alias symlinks right beside the main platform firmware, again, not deployable.

I would argue that the IPC3 was not really deployable either when you mix vendor platforms, they will be dumped at the same directory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would argue that the IPC3 was not really deployable either when you mix vendor platforms, they will be dumped at the same directory.

I "deployed" the IPC3 a lot, it "worked for me". When you write "vendor" do you mean "other than Intel"? Only Intel has ever been released in sof-bin. NXP relies on Yocto AFAIK and ChromeOS has of course a totally different and unique build system. That does not make this script useless outside Intel but I guess it reduces the appeal?

Copy link
Contributor Author

@ujfalusi ujfalusi Dec 19, 2023

Choose a reason for hiding this comment

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

@marc-hb,

Supported platforms: ['tgl', 'tgl-h', 'mtl', 'lnl', 'imx8', 'imx8x', 'imx8m', 'imx8ulp']

As long as it claims this, it should be possible to run

./scripts/xtensa-build-zephyr.py --all

and expect the result to be deployable as it is. It is not.

Don't get me wrong, I'm "deploying" the IPC4 firmware to my DUTs just fine:

scp ../build-sof-staging/sof/tgl/community/dsp_basefw.bin root@<tgl_DUT>:/lib/firmware/intel/sof-ipc4/tgl/community/sof-tgl.ri

but I cannot just grab the build dir and move it to tgz or to DUT as it is.

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

The purpose of this script (and of its .sh ancestor) was always to produce a "deployable" tree. Then IPC4 came and no one did the work until you right now (thanks!). Instead, we had CI deployment secrets. We also had some complex avs transition hacks?

I'm against backwards compatibility in this particular case. It makes the code complicated and difficult to follow. We should have a "flag day" instead where CI MUST cleanup most CI deployment secrets. That shouldn't be difficult: mostly getting rid of CI code. Backwards compatibility means CI secrets will be simplified later = never and it means this new option will STILL not be tested in CI and it will bitrot again.

@marc-hb
Copy link
Collaborator

marc-hb commented Dec 18, 2023

Could you submit the first 3 commits in a separate PR? They're pure fixes without any backward compatibility issue that we can fast track.

Instead, we had CI deployment secrets. We also had some complex avs transition hacks?

To be fair there was also a very severe lack of documentation, later fixed by @kv2019i
thesofproject/sof-docs@3c934c76bb2ac3

@ujfalusi
Copy link
Contributor Author

Could you submit the first 3 commits in a separate PR? They're pure fixes without any backward compatibility issue that we can fast track.

#8646 and #8647

Instead, we had CI deployment secrets. We also had some complex avs transition hacks?

Yes, the IPC4 is based on AVS transition and the kernel is guilty also, which I'm trying to rectify with thesofproject/linux#4382

@ujfalusi
Copy link
Contributor Author

The purpose of this script (and of its .sh ancestor) was always to produce a "deployable" tree. Then IPC4 came and no one did the work until you right now (thanks!). Instead, we had CI deployment secrets. We also had some complex avs transition hacks?

The AVS hack is the --fw-naming=AVS which would create non SOF filenames for the firmware. SOF should not have done that, but it was dictated by the kernel as well (we used the reference firmware and naming initially). cAVS2.5 did not planed to be end user IPC4 platform but we use it as a daily dev vehicle and it is better to not have colliding namespaces with the non SOF firmware.

@marc-hb, it is not only going to affect Intel, but iMX also, that's why I took the path I did. If @dbaluta or/and someone from NXP can confirm that they are OK with non backwards compatible changes in this, I'm OK to do that.

I'm against backwards compatibility in this particular case. It makes the code complicated and difficult to follow. We should have a "flag day" instead where CI MUST cleanup most CI deployment secrets. That shouldn't be difficult: mostly getting rid of CI code. Backwards compatibility means CI secrets will be simplified later = never and it means this new option will STILL not be tested in CI and it will bitrot again.

Yes, I tend to agree, but I have better hope that CI would do the right thing never the less.

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.

4 participants