Add initial support for HIFI4 DSP on imxrt600 board #71061
Conversation
dbaluta
commented
Apr 3, 2024
dbaluta
commented
- this adds SOC definitions
- DTS
- board definition
zephyrbot
requested review from
danieldegrasse,
decsny,
DerekSnell,
dleach02,
EmilioCBen,
mmahadevan108 and
yvanderv
dbaluta
force-pushed
the
mimxrt685_add_dsp
branch
from
0ab2781 to
50fcfe3
Compare
|
Changes since v1:
|
dbaluta
force-pushed
the
mimxrt685_add_dsp
branch
from
50fcfe3 to
ef1b52b
Compare
|
Changes since v2:
|
| @@ -115,3 +115,32 @@ choice USB_MCUX_CONTROLLER_TYPE | |||
| endchoice | |||
|
|
|||
| endif # SOC_MIMXRT685S_CM33 | |||
|
|
|||
| if SOC_MIMXRT685S_ADSP | |||
There was a problem hiding this comment.
Might be worth attempting to de-centralize Kconfig.defconfig here (aka have a per-cluster Kconfig.deconfig placed in each cluster's directory and sourced in the main Kconfig.defconfig). Should make the main Kconfig.defconfig easier to read and, overall, should be cleaner. Maybe same could be applicable to Kconfig and Kconfig.soc? Although I question if it would be worth doing it for Kconfig.soc since it doesn't contain much?
There was a problem hiding this comment.
I think this is too much of an overkill right now and we could do that later when this files gets bigger. Also, imxrt595 already has this implementation and prefer to keep it consistent.
Add SoC definitions and a linker file for the HIFI4 DSP found on the NXP i.MXRT685 microcontroller. Signed-off-by: Vit Stanicek <vit.stanicek@nxp.com> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
This adds a board for HIFI4 DSP found on i.MXRT865 microcontroller. It adds the initial configuration files, dts and documentation. Signed-off-by: Vit Stanicek <vit.stanicek@nxp.com> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
dbaluta
force-pushed
the
mimxrt685_add_dsp
branch
from
ef1b52b to
ef35404
Compare
|
Changes since v3:
|
|
|
||
| $ west build -b mimxrt685_evk/mimxrt685s/adsp samples/hello_world | ||
|
|
||
| Debugging can be directly carried out using the J-Link GDB server with ``xt-gdb`` connected. It's |
| { | ||
| int irq; | ||
|
|
||
| if (mask & BIT(0)) { |
There was a problem hiding this comment.
Do we not have #defines for all these bit masks?
There was a problem hiding this comment.
@dleach02 this file is auto-generated by Xtensa tools and prefer to keep it like that. Same approach is taken for all intances of this file for intel_adsp, nxp_adsp, espressif.
There was a problem hiding this comment.
General comment here- should we provide some form of code to boot the DSP at the SOC level? Currently, my understanding is that anyone wanting to use this enablement would need some application level code/build infrastructure to:
- build the ADSP image
- copy the ADSP image binary to the right offset in RAM (or potentially multiple offsets, if the data and text sections need to be located in different regions)
- Setup clocks for the DSP and release it from reset
I'm aware this may be out of scope for this PR, and that what we have here matches the enablement for the RT500 in tree. However, I think it would be useful for customers to have some in tree code to help booting the DSP core, and moreover would be helpful for testing purposes if we expand the ADSP enablement.
If we don't add code to do this, I think at a minimum outlining the steps required in the board documentation would be helpful
| if SOC_SERIES_IMXRT6XX | ||
|
|
||
| config MCUX_CORE_SUFFIX | ||
| default "_cm33" if SOC_MIMXRT685S_CM33 | ||
| default "_adsp" if SOC_MIMXRT685S_ADSP | ||
| endif |
There was a problem hiding this comment.
We probably want the endif here to be after the if block for SOC_MIMXRT685S_CM33, right? Like so:
if SOC_SERIES_IMXRT6XX
# Common Kconfigs, IE MCUX_CORE_SUFFIX
if SOC_MIMXRT685S_CM33
# RT685 CM33 specific Kconfigs
endif # SOC_MIMXRT685S_CM33
endif # SOC_SERIES_IMXRT6XX
| Stack space is reserved at the end of the RT685_ADSP_DATA_MEM | ||
| region, starting at RT685_ADSP_DATA_MEM_ADDR - RT685_ADSP_STACK_SIZE | ||
|
|
||
| config RT685_ADSP_RESET_MEM_ADDR |
There was a problem hiding this comment.
Given that these Kconfigs are used at the SOC level, should they be declared there?
| @@ -34,3 +34,4 @@ family: | |||
| - name: mimxrt685s | |||
| cpuclusters: | |||
| - name: cm33 | |||
| - name: adsp | |||
There was a problem hiding this comment.
For the RT595, we name the ADSP core f1, since it is a fusion F1. My rational for doing this is that it follows how we name ARM core clusters in tree. Would you prefer to keep the adsp naming here, or refer to this DSP as the hifi4? I don't have a strong preference
|
@danieldegrasse About initialisation of the DSP, I believe it's on the roadmap, as I've written a driver for exactly that purpose in #65229, which has been closed: https://github.com/zephyrproject-rtos/zephyr/pull/65229/files#diff-6916b9e13247d4852595c6b593424b9266520258663d0677a47518849078be7f @dbaluta just took some burden off me by trying to get my code adapted to HWM2 and merged, or, at least, that's my understanding. |
|
Nice! |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
@dbaluta Hi! Any chance this PR can be reopened and eventually merged? |
|
Thanks for the pointer @VitekST! |
