[µTMV] apps: Fix Zephyr code example for STM32F746 boards #7772
Conversation
|
@mdw-octoml @mehrdadh @areusch Hi folks. I'm wondering if I could get a review for that change from you :) |
tutorials/micro/micro_tflite.py
Outdated
| # | ||
| # repo_root = subprocess.check_output(["git", "rev-parse", "--show-toplevel"], encoding='utf-8').strip() | ||
| # project_dir = f"{repo_root}/tests/micro/qemu/zephyr-runtime" | ||
| # project_dir = f"{repo_root}/apps/microtvm/zephyr/demo_runtime" |
There was a problem hiding this comment.
nit: could you please use os.path.join(repo_root, "apps", "microtvm", ...) instead?
There was a problem hiding this comment.
@mehrdadh Sure, done for v2. I also included the new .conf file for disco board into tests/lint/check_file_type.py (I forgot it the first time and CI warned about it. BTW, I pushed with -f v2 so we don't get the additional lines in the commit message when the commits are squashed. I think that's really confusing on TVM project and I'll bring it up on our next community meeting.
|
Thanks @gromero ! Sorry for breaking this. |
gromero
force-pushed
the
for-review-broken-disco
branch
from
d8107ff to
5f31a48
Compare
@mdw-octoml Hi Matt! No worries. Thanks for the quick reviews @mdw-octoml and @mehrdadh |
@mehrdadh I'm building against Zephyr 2.4.0. I'm not testing on the reference VM, rather I'm testing on the physical discovery board and running I think we need to improve the CI somehow because commit |
gromero
force-pushed
the
for-review-broken-disco
branch
from
5f31a48 to
8006bff
Compare
|
hm, I wonder if the test failure is due to |
|
@areusch yeah you are right. Yesterday I tested without this flag and large memory size which gives error. You can either set the CONFIG_MAIN_STACK_SIZE flag or limit memory in "main.c". Not sure which one makes more sense here. |
|
@areusch @mehrdadh are you talking about the current CI error (https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-7772/3/pipeline) ? |
|
@gromero yeah |
gromero
force-pushed
the
for-review-broken-disco
branch
from
8006bff to
4f2de0f
Compare
gromero
force-pushed
the
for-review-broken-disco
branch
5 times, most recently
from
9a99ca8 to
44b8abd
Compare
Commit c39a6e2 "Clean up uTVM demo runtime, add ONNX model test and tutorial (apache#7557)" changed the location of the Zephyr code example to apps/ so this commit updates the tutorial examples under tutorials/micro to reflect the new location where src/main.c resides. Since commit c39a6e2 also split Zephyr configuration per boards, under project's boards/, this commit also adds a proper config for the STM32F746 Discovery board and fixes a nit in the comment in boards/nucleo_f746zg.conf. It also removes CONFIG_MAIN_STACK_SIZE=50 for disco and nucleo boards since the MCUs for both boads are Cortex-m7 based, not Contex-m33. For the new boards/stm32f746g_disco.conf CONFIG_TEST_RANDOM_GENERATOR=y is set, otherwise when CONFIG_ENTROPY_GENERATOR=y linking will fail with the following error: rand32.h:33: undefined reference to `z_impl_sys_rand32_get' Finally, the size of 'uart_data' temporary buffer is reduced a bit (to 8 bytes) to free some additional bytes in SRAM, since most MCUs have a 1-byte FIFO, like it happens with Cortex-M-based MCUs. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
gromero
force-pushed
the
for-review-broken-disco
branch
from
44b8abd to
d05da61
Compare
|
@mehrdadh @mdw-octoml @areusch Hi folks, Finally the CI is happy with my changes plus it didn't give up testing it and completed fine :) Could I get reviews for that new version, please? |
|
LGTM! |
|
|
||
| # For random number generation. | ||
| CONFIG_ENTROPY_GENERATOR=y | ||
| CONFIG_TEST_RANDOM_GENERATOR=y |
There was a problem hiding this comment.
I have tested this on Nucleo board on reference VM with zephyr 2.4 without this flag. Maybe you're using a different zephyr version?
There was a problem hiding this comment.
Not really, I'm using Zephyr 2.4.0, the HEAD is at:
commit 7a3b253ced7333f5c0269387a7f3ed1dee69739d (tag: zephyr-v2.4.0, tag: v2.4.0)
Author: Maureen Helm <maureen.helm@nxp.com>
Date: Sat Sep 26 11:40:18 2020 -0500
release: Zephyr 2.4.0
Set version to 2.4.0.
Signed-off-by: Maureen Helm <maureen.helm@nxp.com>
Maybe it's a difference regarding nucleo vs disco board on Zephyr. I know it's a bit unexpected because these boards are quite alike, but in the past I found some remarkable differences regarding the symbol addresses in zephyr.elf, for instance, so I would be not surprised if the kconfig files used are different and the one for disco board needs that config. On my last PR version I removed it for the nucleo boards and left it only for the disco boards.
|
Thanks @areusch ! |
Commit c39a6e2 "Clean up uTVM demo runtime, add ONNX model test and tutorial (apache#7557)" changed the location of the Zephyr code example to apps/ so this commit updates the tutorial examples under tutorials/micro to reflect the new location where src/main.c resides. Since commit c39a6e2 also split Zephyr configuration per boards, under project's boards/, this commit also adds a proper config for the STM32F746 Discovery board and fixes a nit in the comment in boards/nucleo_f746zg.conf. It also removes CONFIG_MAIN_STACK_SIZE=50 for disco and nucleo boards since the MCUs for both boads are Cortex-m7 based, not Contex-m33. For the new boards/stm32f746g_disco.conf CONFIG_TEST_RANDOM_GENERATOR=y is set, otherwise when CONFIG_ENTROPY_GENERATOR=y linking will fail with the following error: rand32.h:33: undefined reference to `z_impl_sys_rand32_get' Finally, the size of 'uart_data' temporary buffer is reduced a bit (to 8 bytes) to free some additional bytes in SRAM, since most MCUs have a 1-byte FIFO, like it happens with Cortex-M-based MCUs. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Commit c39a6e2 "Clean up uTVM demo runtime, add ONNX model test and tutorial (apache#7557)" changed the location of the Zephyr code example to apps/ so this commit updates the tutorial examples under tutorials/micro to reflect the new location where src/main.c resides. Since commit c39a6e2 also split Zephyr configuration per boards, under project's boards/, this commit also adds a proper config for the STM32F746 Discovery board and fixes a nit in the comment in boards/nucleo_f746zg.conf. It also removes CONFIG_MAIN_STACK_SIZE=50 for disco and nucleo boards since the MCUs for both boads are Cortex-m7 based, not Contex-m33. For the new boards/stm32f746g_disco.conf CONFIG_TEST_RANDOM_GENERATOR=y is set, otherwise when CONFIG_ENTROPY_GENERATOR=y linking will fail with the following error: rand32.h:33: undefined reference to `z_impl_sys_rand32_get' Finally, the size of 'uart_data' temporary buffer is reduced a bit (to 8 bytes) to free some additional bytes in SRAM, since most MCUs have a 1-byte FIFO, like it happens with Cortex-M-based MCUs. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Commit c39a6e2 "Clean up uTVM demo runtime, add ONNX model test and tutorial (apache#7557)" changed the location of the Zephyr code example to apps/ so this commit updates the tutorial examples under tutorials/micro to reflect the new location where src/main.c resides. Since commit c39a6e2 also split Zephyr configuration per boards, under project's boards/, this commit also adds a proper config for the STM32F746 Discovery board and fixes a nit in the comment in boards/nucleo_f746zg.conf. It also removes CONFIG_MAIN_STACK_SIZE=50 for disco and nucleo boards since the MCUs for both boads are Cortex-m7 based, not Contex-m33. For the new boards/stm32f746g_disco.conf CONFIG_TEST_RANDOM_GENERATOR=y is set, otherwise when CONFIG_ENTROPY_GENERATOR=y linking will fail with the following error: rand32.h:33: undefined reference to `z_impl_sys_rand32_get' Finally, the size of 'uart_data' temporary buffer is reduced a bit (to 8 bytes) to free some additional bytes in SRAM, since most MCUs have a 1-byte FIFO, like it happens with Cortex-M-based MCUs. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Commit c39a6e2 "Clean up uTVM demo runtime, add ONNX model test and tutorial (apache#7557)" changed the location of the Zephyr code example to apps/ so this commit updates the tutorial examples under tutorials/micro to reflect the new location where src/main.c resides. Since commit c39a6e2 also split Zephyr configuration per boards, under project's boards/, this commit also adds a proper config for the STM32F746 Discovery board and fixes a nit in the comment in boards/nucleo_f746zg.conf. It also removes CONFIG_MAIN_STACK_SIZE=50 for disco and nucleo boards since the MCUs for both boads are Cortex-m7 based, not Contex-m33. For the new boards/stm32f746g_disco.conf CONFIG_TEST_RANDOM_GENERATOR=y is set, otherwise when CONFIG_ENTROPY_GENERATOR=y linking will fail with the following error: rand32.h:33: undefined reference to `z_impl_sys_rand32_get' Finally, the size of 'uart_data' temporary buffer is reduced a bit (to 8 bytes) to free some additional bytes in SRAM, since most MCUs have a 1-byte FIFO, like it happens with Cortex-M-based MCUs. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
4 participants
Hi,
Could the following changes be reviewed please?
They aim to make microTVM (HEAD) work again for STM32F746 disco board.
Commit c39a6e2 "Clean up uTVM demo runtime, add ONNX model test and
tutorial (#7557)" changed the location of the Zephyr code example to
apps/ so this commit updates the tutorial examples under tutorials/micro
to reflect the new location where src/main.c resides.
Since commit c39a6e2 also split Zephyr configuration per boards,
under project's boards/, this commits also adds a proper config for the
STM32F746 Discovery board and fixes a nit in the comment in
boards/nucleo_f746zg.conf. It also removes CONFIG_MAIN_STACK_SIZE=50 for
disco and nucleo boards since the MCUs for both boads are Cortex-m7
based, not Contex-m33.
For the new boards/stm32f746g_disco.conf board config file
CONFIG_TEST_RANDOM_GENERATOR=y is set
otherwise when CONFIG_ENTROPY_GENERATOR=y linkage will fail with:
rand32.h:33: undefined reference to `z_impl_sys_rand32_get'
Finally this commit sets the UART ring buffer size back to 512 bytes,
only for disco and nucleo boards, otherwise the new value introduced
by commit c39a6e2 makes noinit section not fitting the SRAM region
on these devices, so linkage fails with:
ld: zephyr_prebuilt.elf section
noinit' will not fit in regionSRAM'ld: section .intList VMA [0000000020050000,00000000200500e7] overlaps section noinit VMA [000000002004f5d8,0000000020050317]
ld: region `SRAM' overflowed by 792 bytes
Signed-off-by: Gustavo Romero gustavo.romero@linaro.org
Thanks and best regards,
Gustavo