samples: benchmarks: coremark: migrate to STM logging on nrf54h20dk #17479

kapi-no · 2024-09-25T12:46:15Z

The CoreMark sample now uses logging in the STM standalone mode for the nRF54H20 DK target.

Ref: NCSDK-27517

TODO items:

~~STM log formatting to follow the style used with the CONFIG_LOG_MODE_MINIMAL option~~
Consider adding log flush operations for the ETR target that is responsible for forwarding the logs to the UART instance (CPUAPP)
Investigate issues with CONFIG_COMPILER_OPT="-O3" that cause reboots.
Expand the documentation

NordicBuilder · 2024-09-25T12:49:51Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 15

Inputs:

Sources:

sdk-nrf: PR head: 8aabb18701fb7da017228e951fd29a4fd2ef0d35

more details

sdk-nrf:

PR head: 8aabb18701fb7da017228e951fd29a4fd2ef0d35
merge base: 7504e21df95756c5fabe8968a50ecca6729f8d69
target head (main): d34297a836f9a1c0cd3730a1529cc17a0966a7f5
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (19)

doc
│  ├── nrf
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
samples
│  ├── benchmarks
│  │  ├── coremark
│  │  │  ├── Kconfig.sysbuild
│  │  │  ├── README.rst
│  │  │  ├── boards
│  │  │  │  ├── nrf52833dk_nrf52833.conf
│  │  │  │  ├── nrf52840dk_nrf52840.conf
│  │  │  │  ├── nrf52dk_nrf52832.conf
│  │  │  │  ├── nrf5340dk_nrf5340_cpuapp.conf
│  │  │  │  ├── nrf5340dk_nrf5340_cpunet.conf
│  │  │  │  ├── nrf54h20dk_nrf54h20_cpuapp.conf
│  │  │  │  ├── nrf54h20dk_nrf54h20_cpuapp.overlay
│  │  │  │  ├── nrf54h20dk_nrf54h20_cpuppr.conf
│  │  │  │  ├── nrf54h20dk_nrf54h20_cpuppr.overlay
│  │  │  │  ├── nrf54h20dk_nrf54h20_cpurad.conf
│  │  │  │  ├── nrf54h20dk_nrf54h20_cpurad.overlay
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  ├── prj.conf
│  │  │  ├── prj_multiple_threads.conf
│  │  │  ├── sample.yaml
│  │  │  ├── src
│  │  │  │  │ main.c

Outputs:

Toolchain

Version: f51bdba1d9
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:f51bdba1d9_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister
- sdk-nrf test count: 49
✅ Integration tests

Disabled integration tests

- desktop52_verification
- doc-internal
- test_ble_nrf_config
- test-fw-nrfconnect-apps
- test-fw-nrfconnect-ble_mesh
- test-fw-nrfconnect-ble_samples
- test-fw-nrfconnect-boot
- test-fw-nrfconnect-chip
- test-fw-nrfconnect-fem
- test-fw-nrfconnect-nfc
- test-fw-nrfconnect-nrf-iot_libmodem-nrf
- test-fw-nrfconnect-nrf-iot_lwm2m
- test-fw-nrfconnect-nrf-iot_mosh
- test-fw-nrfconnect-nrf-iot_nrf_provisioning
- test-fw-nrfconnect-nrf-iot_positioning
- test-fw-nrfconnect-nrf-iot_samples
- test-fw-nrfconnect-nrf-iot_serial_lte_modem
- test-fw-nrfconnect-nrf-iot_thingy91
- test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- test-fw-nrfconnect-nrf_crypto
- test-fw-nrfconnect-ps
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-rs
- test-fw-nrfconnect-tfm
- test-fw-nrfconnect-thread
- test-fw-nrfconnect-zigbee
- test-low-level
- test-sdk-audio
- test-sdk-dfu
- test-sdk-find-my
- test-sdk-mcuboot
- test-sdk-pmic-samples
- test-sdk-sidewalk
- test-sdk-wifi

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2024-09-25T13:11:37Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

kapi-no · 2024-09-25T14:39:11Z

Third item on TODO list is potentially fixed by the following PR:

nrfconnect/sdk-zephyr#2049

kapi-no · 2024-09-25T15:01:47Z

Standard rebase

kapi-no · 2024-09-25T15:03:20Z

small fix for UART_CONSOLE

kapi-no · 2024-10-08T09:56:13Z

Rebasing to fix conflicts after upmerge

kapi-no · 2024-11-05T09:14:45Z

Rebased on top of the newest main branch

kapi-no · 2024-11-05T12:40:17Z

Added changes to address the missing items

kapi-no · 2024-11-05T12:41:54Z

Rebased on top of the newest main branch

kapi-no · 2024-11-05T13:08:31Z

Added changelog entries.

samples/benchmarks/coremark/boards/nrf52833dk_nrf52833.conf

zycz · 2024-11-05T13:19:57Z

samples/benchmarks/coremark/src/main.c

+		return;
+	}
+
+	LOG_INF("Logging is stalled until the logging core finishes the CoreMark benchmark\n");


until the application core finishes - shouldn't be more specific here - it might be not obvious to user that application core is doing logging.

Hmm, I wanted to generalize it as we may change the logging core. Perhaps until this core finishes? The user gets the domain prefix next to this log

wouldn't then it say that for every core and "Logging is stalled" only when appcore is busy?

I wonder if even we need this as a log. Comment won't be enough?

I think the log here is helpful and is not very intrusive (one log per app core benchmark start). When the app core starts executing the benchmark it looks as if the system stalled. Apart from that, users may omit the note from Readme and get confused when logs do not appear when they are running benchmarks on remaining cores.

Logging is blocked for all cores until this core finishes the CoreMark benchmark

samples/benchmarks/coremark/boards/nrf54h20dk_nrf54h20_cpurad.overlay

samples/benchmarks/coremark/boards/nrf54h20dk_nrf54h20_cpurad.conf

samples/benchmarks/coremark/README.rst

samples/benchmarks/coremark/sample.yaml

nordicjm · 2024-11-05T15:23:25Z

samples/benchmarks/coremark/src/main.c

@@ -130,7 +130,7 @@ int main(void)
 			k_panic();
 		}

-		LOG_INF("Press %s to start the test ...",  BUTTON_LABEL);
+		LOG_INF("Press %s to start the test ...\n",  BUTTON_LABEL);


this doesn't really improve readability, it looks really weird in the output

This sample already uses break lines in other places, I wanted to add them here for consistency so that we have a line break before every wait period (e.g. k_sem_take).

samples/benchmarks/coremark/Kconfig.sysbuild

nordicjm · 2024-11-05T15:26:02Z

samples/benchmarks/coremark/src/main.c

+		if (IS_ENABLED(CONFIG_APP_MODE_FLASH_AND_RUN)) {
+			LOG_INF("CoreMark finished! Press the reset button to restart...\n");
+		} else {
+			LOG_INF("CoreMark finished! Press %s to restart ...\n", BUTTON_LABEL);


likewise here, jarring

kapi-no · 2024-11-13T13:22:18Z

Addressed reviewe comments

kapi-no · 2024-11-14T14:18:27Z

small update of the static function name

NordicBuilder · 2024-11-14T14:34:21Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publish GitHub Action.

Optimized the stack size in the multiple thread configuration of the CoreMark sample. Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

The CoreMark sample now uses logging in the STM standalone mode for the nRF54H20 DK target. Ref: NCSDK-27517 Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

Applied the same logging format in the standard logging mode as is currently used in the multi-domain logging mode in the CoreMark sample. Ref: NCSDK-27517 Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

kapi-no · 2024-11-14T15:56:08Z

Optimized the stack size in multiple thread configuration to fit the application image into TC RAM block.

kapi-no · 2024-11-14T16:07:53Z

Tested all configuration variants and they seemed to work fine after migration to STM-based logging

Enabled the logs in the CoreMark sample in the case of fatal errors. Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

kapi-no · 2024-11-15T09:12:11Z

One additional improvement to developer experience before I finalize this PR

nordicjm

The newlines in log output need further discussion

github-actions bot added doc-required PR must not be merged without tech writer approval. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Sep 25, 2024

kapi-no added the DNM label Sep 25, 2024

kapi-no force-pushed the coremark_stm branch from ef4112e to 7547a60 Compare September 25, 2024 15:01

kapi-no force-pushed the coremark_stm branch from 7547a60 to be446fd Compare September 25, 2024 15:03

kapi-no force-pushed the coremark_stm branch from be446fd to 244e883 Compare October 8, 2024 09:55

kapi-no force-pushed the coremark_stm branch from 244e883 to e1ab7dc Compare November 5, 2024 09:14

kapi-no force-pushed the coremark_stm branch from e1ab7dc to cbe6486 Compare November 5, 2024 12:33

kapi-no marked this pull request as ready for review November 5, 2024 12:39

kapi-no requested review from a team as code owners November 5, 2024 12:39

kapi-no force-pushed the coremark_stm branch from cbe6486 to 1fa8880 Compare November 5, 2024 12:41

kapi-no force-pushed the coremark_stm branch from 1fa8880 to 923c7e0 Compare November 5, 2024 13:08

kapi-no requested a review from a team as a code owner November 5, 2024 13:08

github-actions bot removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 5, 2024

zycz reviewed Nov 5, 2024

View reviewed changes

nordicjm reviewed Nov 5, 2024

View reviewed changes

mkapala-nordic self-requested a review November 7, 2024 14:16

kapi-no requested review from MarekPieta and nordicjm November 13, 2024 13:21

peknis approved these changes Nov 13, 2024

View reviewed changes

mkapala-nordic approved these changes Nov 13, 2024

View reviewed changes

zycz approved these changes Nov 14, 2024

View reviewed changes

kapi-no force-pushed the coremark_stm branch from ba59f44 to d874a10 Compare November 14, 2024 14:17

kapi-no requested review from MarekPieta and removed request for MarekPieta November 14, 2024 14:19

kapi-no removed the DNM label Nov 14, 2024

kapi-no added the DNM label Nov 14, 2024

kapi-no added 3 commits November 14, 2024 16:54

samples: benchmarks: coremark: optimize stack for multi thread config

4d3bb9d

Optimized the stack size in the multiple thread configuration of the CoreMark sample. Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

samples: benchmarks: coremark: migrate to STM logging on nrf54h20dk

dd330ee

The CoreMark sample now uses logging in the STM standalone mode for the nRF54H20 DK target. Ref: NCSDK-27517 Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

kapi-no force-pushed the coremark_stm branch from d874a10 to 51c6389 Compare November 14, 2024 15:54

kapi-no removed the DNM label Nov 14, 2024

mkapala-nordic approved these changes Nov 14, 2024

View reviewed changes

samples: benchmarks: coremark: enable logs from the fault handler

8aabb18

Enabled the logs in the CoreMark sample in the case of fatal errors. Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>

mkapala-nordic approved these changes Nov 15, 2024

View reviewed changes

zycz approved these changes Nov 15, 2024

View reviewed changes

nordicjm previously requested changes Nov 15, 2024

View reviewed changes

carlescufi merged commit 7ba09e9 into nrfconnect:main Nov 15, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

samples: benchmarks: coremark: migrate to STM logging on nrf54h20dk #17479

samples: benchmarks: coremark: migrate to STM logging on nrf54h20dk #17479

kapi-no commented Sep 25, 2024 •

edited

Loading

NordicBuilder commented Sep 25, 2024 •

edited

Loading

NordicBuilder commented Sep 25, 2024

kapi-no commented Sep 25, 2024

kapi-no commented Sep 25, 2024

kapi-no commented Sep 25, 2024

kapi-no commented Oct 8, 2024

kapi-no commented Nov 5, 2024

kapi-no commented Nov 5, 2024

kapi-no commented Nov 5, 2024

kapi-no commented Nov 5, 2024

zycz Nov 5, 2024

kapi-no Nov 5, 2024 •

edited

Loading

zycz Nov 6, 2024

mkapala-nordic Nov 7, 2024

kapi-no Nov 8, 2024

kapi-no Nov 13, 2024

nordicjm Nov 5, 2024

kapi-no Nov 6, 2024

nordicjm Nov 5, 2024

kapi-no commented Nov 13, 2024

kapi-no commented Nov 14, 2024

NordicBuilder commented Nov 14, 2024

kapi-no commented Nov 14, 2024

kapi-no commented Nov 14, 2024

kapi-no commented Nov 15, 2024

nordicjm left a comment

samples: benchmarks: coremark: migrate to STM logging on nrf54h20dk #17479

samples: benchmarks: coremark: migrate to STM logging on nrf54h20dk #17479

Conversation

kapi-no commented Sep 25, 2024 • edited Loading

NordicBuilder commented Sep 25, 2024 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

NordicBuilder commented Sep 25, 2024

kapi-no commented Sep 25, 2024

kapi-no commented Sep 25, 2024

kapi-no commented Sep 25, 2024

kapi-no commented Oct 8, 2024

kapi-no commented Nov 5, 2024

kapi-no commented Nov 5, 2024

kapi-no commented Nov 5, 2024

kapi-no commented Nov 5, 2024

Choose a reason for hiding this comment

kapi-no Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kapi-no commented Nov 13, 2024

kapi-no commented Nov 14, 2024

NordicBuilder commented Nov 14, 2024

kapi-no commented Nov 14, 2024

kapi-no commented Nov 14, 2024

kapi-no commented Nov 15, 2024

nordicjm left a comment

Choose a reason for hiding this comment

kapi-no commented Sep 25, 2024 •

edited

Loading

NordicBuilder commented Sep 25, 2024 •

edited

Loading

kapi-no Nov 5, 2024 •

edited

Loading