thesofproject / sof

Sound Open Firmware
Other
510 stars 301 forks source link

[FEATURE] SOF HIFI customization #8697

Open btian1 opened 5 months ago

btian1 commented 5 months ago

Is your feature request related to a problem? Please describe. Currently, each module's version select are done based on predefined macro in ( core-isa.h), which is not able to be configurable. code list below, take volume as example:

#if defined(__XCC__)
# include <xtensa/config/core-isa.h>
# if XCHAL_HAVE_HIFI4
#  define VOLUME_HIFI4
# elif XCHAL_HAVE_HIFI3
#  define VOLUME_HIFI3
# else
#  define VOLUME_GENERIC
# endif
#else
# define VOLUME_GENERIC
#endif

Describe the solution you'd like Prefer use Kconfig to provide a choice for developer, then module version can be changed in a unified way.

Describe alternatives you've considered There are multiple ways to resolve this request:

  1. add -C=-DCONFIG_VOLUME_HIFI3=y or similar to build cmd line to provide switch for volume hifi3/hifi4/generic. pros: nothing need change. cons: if want to switch with multiple modules, cmd line would be much longer and not easy to know which are take effect and which not.

  2. current implement in PR: https://github.com/thesofproject/sof/pull/8682/ pros: it achieved configurable and remove select code in header file(listed above). cons: add a new build env and need align with test bench build. personally, I still think this is the good way, it removed #if in each module header, and configurable for module also done.

  3. provide choice make generic as default and set HIFI version in each platform Kconfig pros: implementation is simple. cons:

    1. As kai mentioned, there are cloud build for mtl, and no xtensa tool chain, which will cause build error.
    2. need address each platform to add correct HIFI version kconfig.
  4. provide choice and make a DUMMY(name can be discussed, just use it as default choice), keep current header selection. pros: this is a very simple implementation, without any break and can achieve module customization. cons: still need header above code support to get the real default HIFI version, this is a mediate solution.

Please address your comments here and we need finalize which solution is best for now.

marc-hb commented 5 months ago

You're jumping to solution and mixing up logical requirements, user interface, implementation details and other issues.

The very first steps are to detail the high level requirements and then the user interface. Only after we can get lost in pull requests and implementation details.

By chance I had a very quick chat with @lgirdwood about this. I understood the first and main requirements to be this:

So. assuming I understood these very high level requirements correctly, then something like -C=-DCONFIG_VOLUME_HIFI3=y will be part of the solution because such a Kconfig flag matches these Kconfig requirements DIRECTLY. The final solution may have additional bells and whistles and we don't know yet how it will be implemented in detail but in any case -C=-DCONFIG_VOLUME_HIFI3=y is part of the requirements.

if want to switch with multiple modules, cmd line would be much longer and not easy to know which are take effect and which not.

A shortcut to change multiple modules at once sounds nice but AFAIK it's not part of the main requirements yet and the ability to change a single module at a time will still be there anyway. So work can start on the per-component Kconfig already. If needed, changing multiple components at once can be added later in SEPARATE PRs

https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs

btian1 commented 5 months ago

@marc-hb , thanks for your input, @lgirdwood , please double confirm two point listed, if this is the case, I think 4 is matching the case, we can create a new Kconfig for each module named with Kconfig.level(Kconfig.hifi) provide a choice inside this config, and included by each module's main config.

kv2019i commented 5 months ago

I think one consideration is that do we need to have a independent configuration of optimizations (in the default build) that is not derived from toolchain headers. If yes (= this is a requirement), then we need a new overlay. This could be useful e.g. in upgrading toolchains (components would not be silently downgraded when switching toolchains).

If we add option to override the selection on a per-component basis, that would seem to go a long way.

btian1 commented 5 months ago

for this independent configuration you mentioned(a new overlay), it should depend on platform, right? if so, does it have issue on cloud build as well, or cloud build will skip this overlay build parameter?

For now, I would suggest use above I listed, i.e, add a new Kconfig.hifi to each module to provide HIFI choice to each module.

lgirdwood commented 5 months ago

This needs to come from Kconfig as the single BKC for each compiler. We will have different optimization levels for GCC (CI and community), Cadence xcc (older production), Cadence clang (newer production) and upstream clang (later this year). i.e. we can have overlays for each platform + compiler 1) mtl_gcc_overlay 2) mtl_cadence_overlay and later 3) mtl_clang_overlay (opensource clang with HiFi)

marc-hb commented 5 months ago

Status update: #8787 has just been merged.