Skip to content

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Mar 11, 2021

Straight cherry-pick -x, no conflict.

marc-hb added 19 commits March 11, 2021 13:22
The script already logs the full CMake command that is re-usable outside
this script... except for the PATH change. Expose that sneaky PATH
change.

Debugging every build issue starts with peeling the too many layers of
indirection.

Also fix some minor issue in the help message.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 419806f)
As reported in thesofproject#3491, there is confusion between platform names, CPU
names, PCH names, toolchain names, signing schemes and what not. For
instance in "build_apl_gcc/sof-apl.ri", the first "apl" matches the name
of a defconfig file while the second "apl" matches the name of a signing
scheme.

When building out-of-source, there is no reason to vary the filenames of
the output depending on the build configuration, changing the name of
the build directory is enough. This simplifies automation logic
including the next commit that adds a new install/GNUmakefile.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 0e60ec8)
More specifically replacing sof-bin/go.sh and sof-bin/publish.sh and
also sof/scripts/sof-target-install.sh eventually.

"make install" code has always belonged to source repositories because
developers need to install too and we want everyone to use the same
installers. It's also easier to have all the information in a single
place.

Once the layout in sof-bin mirrors the /lib/firmware/intel layout
exactly, sof-bin does not need any installation code any more.

Mixing source and binaries in the same repo is also a "code smell",
notably because it forces branching them together.

Using a higher level build tool for installation instead of plain
scripts has a few benefits:

- Multiple entry points: easy to invoke (and test) any part of the
  installer individually
- ... while invoking dependencies automatically.
- Other features "for free" like:
  - errexit
  - error messages like "dunno how to build file x"
  - commands are logged by default

- Also gets rid of most of the large code duplication in go.sh and
  publish.sh, so:
  - Enabling or disabling a platform is a 3-character change
  - Allows platform selection in local config file (even just one platform)
  - Much harder to add inconsistencies
  - Much easier to review correctness, for instance no need to
    scrutinize every line to see which platforms are aliased.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit c6edc5f)
Fix $pwd confusion and remove bogus ../local/bin PATH that does not
exist.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit e31afb3)
Take $BUILD_TOOLS_DIR from the environment if present.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit ffaeaaa)
The tools build is independent from the firmware build. The next step is
to invoke it from here if needed.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit f9545c8)
One-touch "make -C installer rsync" combines fast incremental build,
staging and deploy in one command.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 4798096)
Simple check of the "tree" output

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 5dea5ae)
Doc update missing from the most recent pull request.

Also warn against `make -jN stage rsync`

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 453c686)
In the future we should probably extract the array of XTENSA_SYSTEM
values out of xtensa-build-all.sh

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 25fc451)
commit e31afb3 ("xtensa-build-all.sh: make it runnable from
anywhere") missed this.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 5b34b81)
Add this to the -h usage message:

This script supports XtensaTools but only when installed in a specific
directory structure, example:

myXtensa/
└── install/
    ├── builds/
    │   ├── RD-2012.5-linux/
    │   │   └── Intel_HiFiEP/
    │   └── RG-2017.8-linux/
    │       ├── LX4_langwell_audio_17_8/
    │       └── X4H3I16w2D48w3a_2017_8/
    └── tools/
        ├── RD-2012.5-linux/
        │   └── XtensaTools/
        └── RG-2017.8-linux/
            └── XtensaTools/

$ XTENSA_TOOLS_ROOT=/path/to/myXtensa ./scripts/xtensa-build-all.sh ...

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 6468490)
XTENSA_TOOLS_ROOT is required by xtensa-build-all.sh anyway, so don't
force the user to say twice that they want xcc.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 907f869)
Notably: xt-xcc --show-config failed with: No such file or directory

... when the directory exists but is wrong.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit f57de59)
Fixes:

 rm -rf staging ; make aliases
 ln: failed to create symbolic link 'staging/sof/sof-glk.ri': No such
 file or directory

This also happens on a brand new checkout when building in parallel with
make -j because symbolic links don't have any dependency. Example at:

  https://github.com/marc-hb/sof/runs/2036288013

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 969f370)
add tgl-h

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 1371d47)
As CMake forks one compiler process for each source file, the XTensa
compiler spends much more time idle waiting for the license server over
the network than actually using CPU or disk.

On my VM with 16 virtual cores, rebuilding one platform from scratch
with this commit goes down from 12s to less than 9s: more than 25%
faster. With Ninja it goes down from 11s to less than 8s. My license
server is 25ms away: a closer server does not need as many threads while
a more distant server would obviously benefit for even more
threads... while already getting an even better improvement than 25%
from just 3 times more threads! It's complicated and we probably don't
want to start the build by measuring latency to the license server.

The entire, purely local _gcc_ build is so fast (~ 1s) that observing any
the difference between -j nproc and -j nproc*N is practically impossible
so let's not waste RAM when building with gcc.

Also: log the $XTENSA_SYSTEM variable as it is required for incremental
builds; remove one apostrophe in the here-doc usage as it breaks the
parser of some editor (jed).

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 716efc0)
Gets the length of a no-op "make topologies" from 380 down to 140
lines. From 300 to 200 for one "make signed" platform.

Ninja is more quiet by default.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit b809cae)
Building "sof" does not rebuild the .ri firmware file. Switch to the
"bin" target which is what ./scripts/xtensa-build-all.sh builds.

Fixes: 4798096 ("installer: (re)build firmware, topologies and user
space tools)

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit 21f4e74)
@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 12, 2021

https://sof-ci.01.org/sof-pr-viewer/#/build/PR3925/build6158955 fails to build CNL, ICL and TGL because it tries do apply changes that are clearly compatible only with the current master branch and not with the older stable-v1.7:

rm -fr override.config
echo "CONFIG_COMP_CODEC_ADAPTER=y" >> override.config
echo "CONFIG_CADENCE_CODEC=y" >> override.config
echo "CONFIG_CADENCE_CODEC_WRAPPER=y" >> override.config
make overrideconfig
make bin -j8 VERBOSE=1 |& tee -a CMakeFiles/stdout.log
cp generated/.config config
Command return code: 2
Command error output: error: patch failed: src/audio/codec_adapter/codec/cadence.c:21
error: src/audio/codec_adapter/codec/cadence.c: patch does not apply
error: patch failed: src/include/sof/audio/codec_adapter/codec/cadence.h:20
error: src/include/sof/audio/codec_adapter/codec/cadence.h: patch does not apply

These CONFIG_ were added in commit 9fcfd75, after branching 1.7

@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 12, 2021

https://sof-ci.01.org/sofpr/PR3925/build8314/devicetest/?model=BYT_MB_NOCODEC&testcase=check-alsabat-headset-playback fails like this. I don't know what to think of it.

2021-03-11 22:29:12 UTC [REMOTE_INFO] check the PCMs before alsabat test
2021-03-11 22:29:14 UTC [REMOTE_COMMAND] alsabat -Phw:sofnocodec,0 --standalone -n 240000 -r 48000 -c 2 -F 997
2021-03-11 22:29:15 UTC [REMOTE_COMMAND] alsabat -Chw:sofnocodec,0 -c 2 -r 48000 -F 997
WARNING: Signal overflow!
 FAIL: Peak freq too high 2991.21 Hz
 FAIL: Peak freq too high 6979.25 Hz
...
Channel 1 - Checking for target frequency 997.00 Hz
Amplitude: 37014.4; Percentage: [112]
Detected peak at 996.83 Hz of 37.48 dB
 Total 41.1 dB from 990.97 to 1004.88 Hz
 PASS: Peak detected at target frequency
Detected peak at 2991.21 Hz of 28.81 dB
 Total 41.5 dB from 2989.75 to 2991.21 Hz
Detected peak at 6979.25 Hz of 20.72 dB
 Total 41.5 dB from 6979.25 to 6979.25 Hz
Detected at least 3 signal(s) in total

Return value is -1003

@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 12, 2021

https://sof-ci.01.org/sofpr/PR3925/build8314/devicetest/?model=BDW_WSB_RT286&testcase=multiple-pipeline-playback failed like this. Maybe #3462?

2021-03-11 22:24:24 UTC [REMOTE_INFO] /home/ubuntu/sof-test/test-case/multiple-pipeline.sh will use topology /lib/firmware/intel/sof-tplg/sof-bdw-rt286.tplg to run the test case
2021-03-11 22:24:24 UTC [REMOTE_INFO] Pipeline list to ignore is specified, will ignore 'pcm=HDA Digital' in test case
2021-03-11 22:24:24 UTC [REMOTE_INFO] Run command to get pipeline parameters
2021-03-11 22:24:24 UTC [REMOTE_COMMAND] sof-tplgreader.py /lib/firmware/intel/sof-tplg/sof-bdw-rt286.tplg -f 'type:capture' -b ' pcm:HDA Digital' -s 0 -e
2021-03-11 22:24:24 UTC [REMOTE_INFO] Testing: Low Latency [hw:0,0]
2021-03-11 22:24:24 UTC [REMOTE_COMMAND] arecord   -D hw:0,0 -c 2 -r 48000 -f S16_LE /dev/null -q
2021-03-11 22:24:24 UTC [REMOTE_INFO] sleep 5s for sound device wakeup
aplay: pcm_write:2061: write error: Input/output error
aplay: pcm_write:2061: write error: Input/output error
2021-03-11 22:24:29 UTC [REMOTE_INFO] checking pipeline status
2021-03-11 22:24:29 UTC [REMOTE_ERROR] Running process count is 1, but 3 is expected
6870 arecord -D hw:0,0 -c 2 -r 48000 -f S16_LE /dev/null -q

@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 12, 2021

This PR changes build scripts only so it can't be causing the failures above. Everything else is green.

@marc-hb marc-hb marked this pull request as ready for review March 12, 2021 01:07
@marc-hb marc-hb requested a review from aiChaoSONG March 12, 2021 01:08
@marc-hb marc-hb requested a review from fredoh9 March 12, 2021 01:08
@lgirdwood
Copy link
Member

@zrombel any comment here ? The build is working on Jenkins so are we missing anything for internal CI here ?

@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 18, 2021

@zrombel any comment here ? The build is working on Jenkins so are we missing anything for internal CI here ?

As I described above, @zrombel has confirmed that QB tries to make some unversioned, master-specific changes on the fly before building and does not know it does not support branches.

Other failures are clearly unrelated.

@lgirdwood lgirdwood merged commit 47d077d into thesofproject:stable-v1.7 Mar 18, 2021
@marc-hb marc-hb deleted the installer-1.7 branch March 18, 2021 15:27
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