Skip to content
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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

andrew-mtk
Copy link

Add SOF support for hifi4 adsp found in mt8365 SOC.

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]>
@sofci
Copy link
Collaborator

sofci commented Feb 11, 2025

Can one of the admins verify this patch?

reply test this please to run this test once

@lgirdwood
Copy link
Member

test this please

@lgirdwood
Copy link
Member

Adding @andyross

Copy link
Member

@lgirdwood lgirdwood left a 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);
Copy link
Collaborator

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

Copy link
Author

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.

Copy link
Collaborator

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

Copy link
Author

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

Copy link
Collaborator

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

Copy link
Author

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@kv2019i
Copy link
Collaborator

kv2019i commented Feb 14, 2025

@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 ).

@cgturner1
Copy link

cgturner1 commented Feb 14, 2025

@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

Copy link
Contributor

@andyross andyross left a 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.

@lgirdwood
Copy link
Member

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@lgirdwood
Copy link
Member

@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

@cgturner1 we should drop the docker image now and just use the Zephyr SDK for public toolchains.
@andyross I'm guessing you will be adding MTK toolchains to Zephyr.

@lgirdwood
Copy link
Member

@yefei-a pls review.

@andrew-mtk
Copy link
Author

andrew-mtk commented Feb 21, 2025

@cgturner1 we should drop the docker image now and just use the Zephyr SDK for public toolchains. @andyross I'm guessing you will be adding MTK toolchains to Zephyr.

@lgirdwood , I have already submitted PR with mt8365 ADSP toolchain to Zephyr SDK: PR #869

@lgirdwood
Copy link
Member

@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.

@andrew-mtk
Copy link
Author

@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
ada2a42
scripts: Add support for MediaTek mt8365 platform
2751ee8

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.

7 participants