-
Notifications
You must be signed in to change notification settings - Fork 325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Mediatek mt8365 (xtos) platform #9830
base: main
Are you sure you want to change the base?
Conversation
Add mt8365.toml to support sof-mt8365.ri binary build. Signed-off-by: Andrew Perepech <[email protected]>
Add xtensa headers for mtk mt8365 platform. Signed-off-by: Andrew Perepech <[email protected]>
Add memory layout and register addresses for mt8365. [Cache] I-Cache: 32KB, 4-way Associativity D-Cache: 32KB, 4-way Associativity [Memory] DRAM: DSP can access DRAM shared with CPU Signed-off-by: Andrew Perepech <[email protected]>
Add interrupt, timer, and ipc drivers for mt8365 mt8365 DSP has 25 interrupts. Signed-off-by: Andrew Perepech <[email protected]>
mt8365 platform integrates a single-core HIFI4 DSP. The highest DSP operation frequency is 600MHz which requires 0.75v working voltage. otherwise, it should switch to 13M which can operate at the lowest working voltage 0.55v. Signed-off-by: Andrew Perepech <[email protected]>
Add mt8365 AFE common header and register header files. Add AFE platform for mt8365 audio. Signed-off-by: Andrew Perepech <[email protected]>
Add default config for mt8365 platform. Signed-off-by: Andrew Perepech <[email protected]>
Add CMakefile for MediaTek mt8365 platform to build it. Signed-off-by: Andrew Perepech <[email protected]>
Add mt8365 to Kconfig. Signed-off-by: Andrew Perepech <[email protected]>
Add MT8365 sinegen driver for debug purpose. Signed-off-by: Andrew Perepech <[email protected]>
Add mt8365 dai and dma platform driver. Signed-off-by: Andrew Perepech <[email protected]>
Add platform dai_init() to platform_init(). Add platform dai to build list. Signed-off-by: Andrew Perepech <[email protected]>
Add topology files for mt8365 including mt8365.m4 and sof-mt8365-mt6357.m4. Signed-off-by: Andrew Perepech <[email protected]>
Add support for building MediaTek mt8365 toolchains in docker image. Signed-off-by: Andrew Perepech <[email protected]>
Add MediaTek mt8365 platform to the DEFAULT_PLATFORMS list. Signed-off-by: Andrew Perepech <[email protected]>
Can one of the admins verify this patch?
|
test this please |
Adding @andyross |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but needs Mediatek/Google reviewers to approve.
|
||
/* must read 'low' first, and 'high' next */ | ||
low = io_reg_read(CNTCV_L); | ||
high = io_reg_read(CNTCV_H); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this not racy? what if low wraps immediately after you've read it out? Say, if you read low as 0xffffffff, at which point high was 0, then it steps, you read 1, so the result is 0x1ffffffff instead of 0xffffffff or 0x100000000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this condition previously with devmem2 utility. The value of CNTCV_H is latched by HW when CNTCV_L is read, so it should be safe to read 64-bit timer counter just once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtk30148 aha, interesting. Well, if the hardware does that, maybe it's safe, OTOH I cannot really imagine how that would work... Say, if I read CNTCV_H
half an hour after CNTCV_L
or however long it takes to wrap around multiple times, maybe a week later, it still would contain the value, corresponding to that long ago read CNTCV_L
? At the very least this is asking for a more detailed comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lyakh, This is exactly how it works. I just verified it with devmem2 one more time:
# date; devmem2 0x1d060008; devmem2 0x1d06000c;
Fri Jan 1 00:17:53 UTC 2010
/dev/mem opened.
Memory mapped at address 0xffff9ebcc000.
Read at address 0x1D060008 (0xffff9ebcc008): 0xF5F0DD20
/dev/mem opened.
Memory mapped at address 0xffffbb330000.
Read at address 0x1D06000C (0xffffbb33000c): 0x00000000
# date; devmem2 0x1d06000c; devmem2 0x1d060008; devmem2 0x1d06000c;
Fri Jan 1 00:39:09 UTC 2010
/dev/mem opened.
Memory mapped at address 0xffff91212000.
Read at address 0x1D06000C (0xffff9121200c): 0x00000000
/dev/mem opened.
Memory mapped at address 0xffff93291000.
Read at address 0x1D060008 (0xffff93291008): 0xD2C28D95
/dev/mem opened.
Memory mapped at address 0xffff8736d000.
Read at address 0x1D06000C (0xffff8736d00c): 0x00000004
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtk30148 it looks like the high part is only updated when the low part is read? So you can even read the high part multiple times? Cool. Well, yeah, a comment would be nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lyakh, I have added the comment. Please check it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
@cgturner1 This would need another docker update. This updates the Docker build and adds a new gcc-build-default-platforms targets, but until the docker image is update the CI will fail. @lgirdwood @thesofproject/maintainers I can't see any non-Intel people contributing images to docker hub for SOF project, so I guess this is depending on our actions, right? Ideally we should share this load as well. One option to make this easier with Zephyr is to just get rid of the Docker image in CI. Just use Zephyr SDK to get the public toolchains, and add the few tools (alsa-lib/alsa-utils) to west manifest (see #9791 ). |
thanks for the heads up..I see the changes on the docker build and can update it fairly painless now that I've gone through the first one myself...the changes should be fine as they are similar to the other mediatek platforms that build just fine...probably take an afternoon to build and test the new docker version...don't know if the preferred route is to update the docker first (assuming the changes in ada2a42 are all we need) and then merge this PR or merge this PR first then update the docker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No complaints here. This one isn't on my roadmap and no Zephyr work is planned from me. Though I do strongly recommend people investigate a port based on the newer work: a quick scan seems to show this is another extremely similar part and should work without too much trouble.
Yeah - I think similar too. We can take this and move deprecate later when Zephyr port is ready. @andyross @mtk30148 will need another approver (who has experience of the HW) before we can merge. |
Add a comment explaining that the CNTCV_L register must be read before the CNTCV_H to get a valid 64-bit timestamp value Signed-off-by: Andrew Perepech <[email protected]>
|
||
/* must read 'low' first, and 'high' next */ | ||
low = io_reg_read(CNTCV_L); | ||
high = io_reg_read(CNTCV_H); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
@cgturner1 we should drop the docker image now and just use the Zephyr SDK for public toolchains. |
@yefei-a pls review. |
@lgirdwood , I have already submitted PR with mt8365 ADSP toolchain to Zephyr SDK: PR #869 |
@andrew-mtk I think we can merge this now and circle back to the toolkit when the compiler part is ready, can you remove the docker part and that should pass the CI for merging now. We can then follow up the Zephyr SDK when its ready. |
@lgirdwood , should I remove both of these or just the last one? Than force push the updated branch? docker_build: Add support for MediaTek mt8365 |
Add SOF support for hifi4 adsp found in mt8365 SOC.