Mediatek Audio DSPs: 8186/8188/8196

[andyross]

This is getting hot again so re-pushing a stale PR with a few bits added. Support for Zephyr builds for the Audio DSPs on MT8186, MT8188 and MT8196 SOCs.

[andyross]

Add shotgun gaggle of Xtensa reviewers.

[nordicjm]

See query about cpu cluster first, cpu clusters should be used if you are targetting a specific CPU/cluster on a SoC, the SoC name should not include the CPU

[andyross]

OK, I puzzled out the bits needed to add the (IMHO, still needless) cpucluster to the soc tree. Please see if this matches what you expect.

I will say though, the process... wasn't smooth. Things like "CONFIG_SOC value does not match string defined in soc.yml" or "Cannot find soc directory" should be obvious error messages and instead are inscrutable secondary build failures. I spent a lot more time than I'd have expected stuffing message(WARNING ...) lines into cmake files. Also... the DTS filename changes. Why? The DTS file is a board asset, not a SOC thing. It lives in the board directory. It used to be named the same as the board, but now it isn't, because of something that changed in the SOC layer?

(Also rebased around a collision with llext support having been added to mt8195)

[nordicjm]

Changes are OK but you could alternatively rename the board to mt8188, then the filenames would be mt8188_adps.dts as it was before, and means your minimal build string would be west build -b mt8188//adps. Please squash the changes to the socs/boards added in earlier commits into the earlier commits

[andyross]

I'd really rather have it be a separate patch if it's OK with you, for symmetry with the older already-merged device that is being changed and just to have a record I know I can look back to.

And the name still creeps me out. Again, this is not an integration for the whole laptop chip. It's just audio firmware.

[nordicjm]

How about if you flip the order of converting existing boards to the new format as the first commit then have the new commits adding the final board names in the commits without having a fixup commit at the end?

[andyross]

Does that really rise to a -1? It's a non-trivial rebase, and I personally like the order here as it makes the "complicated" change obvious.

[nordicjm]

You are adding a commit 04ed839 and then fixing it in the next commit eab814b so yes. It's fine to have the commits in that order, but the first one should be changed to add the new boards with the correct names outright, then the second commit for updating existing boards, there is no point to adding a commit which has no use, is wrong and is fixed up in a later commit.

[andyross]

I'm going to try one more time: I really don't want to do that, because if I do that then I'm not going to have a reference to go back to explaining how I did this the first time. I understand you view that last commit as "obvious" renaming, but I'm not so talented and had to spend a lot of effort to fix things based on my pre-existing understanding of how zephyr board/soc integration works. I'd like to know I can get back to it to see how it worked.

Basically, can you pretty-please remove your -1 here? I really don't think commit order should be a merge-blocking kind of thing. It's a big project and we all need to recognize that different people have different aesthetic senses of how a git log should look.

[nordicjm]

We have a whole porting guide here: https://docs.zephyrproject.org/latest/hardware/porting/board_porting.html#hw-model-v2
We have an automatic porting script here: https://github.com/zephyrproject-rtos/zephyr/blob/main/scripts/utils/board_v1_to_v2.py
We have a whole branch here showing conversions we did for in-tree boards: https://github.com/zephyrproject-rtos/zephyr/tree/collab-hwm
Why do we need a new board to be added that, as part of the same PR, adds the board wrongly in the first commit by not using the things specifically defined for hwmv2 and also should anyone want to bisect a failure for this board would result in them having to use a different board name if they get to the first commit?

If you think the zephyr documentation is lacking, you can open an issue about it or submit a PR improving it, if you want to reference to see how you ported something then you can host that yourself locally on or github or anywhere

[andyross]

adds the board wrongly in the first commit by not using the things specifically defined for hwmv2

Yeah, I don't want this to become a big monkey fight, but that's distressingly overstated. In fact I checked: there's nothing in the existing docs that demands the use of a cpucluster, it just talks about what the syntax means. The demand for particular structure was your personal design aesthetic. And... that's fine! Your code, your model, you get to make demands like that.

But it's not correct to characterize the early patches as "incorrect" either, given that they matched the idioms used by many (probably most) existing boards in the tree. So if you do make those demands, part of the job of being a good maintainer is staying with the PR as developers push back with questions and attempts at compromise and not walking away in pique.

[nordicjm]

It is not my personal design aesthetic, it's the design and whole purpose of hwmv2 (which was pioneered and designed by Torsten not myself) and all boards (in tree that is, I don't really care about out of tree boards, people can do as they please) need to follow that. hwmv2 was agreed by people to be brought in and used going forward with all the features it has, a lot of time was spent on things like terminology and having the right parts of devices in SoCs in discussions to get the system that everyone wanted. Is hwmv2 the best thing since sliced bread? No, but is it a huge marked improvement over the wild west that was hwmv1 which had so many irregularities and people just doing whatever which was specific to them and incompatible with others, it brings organisation to what previously was chaos. Is hwmv2 complete? No, is hwmv2 unfathomable? No, can hwmv2 be improved to make things easier and better to support for more complex use cases? Absolutely, hence why we have open issues for improving features and adding new functionality. And really porting a board or SoC should not be hard, I ported 100s in the conversion all manually by hand, made a lot of mistakes that needed to get patched up but the conversion was not hard but I work on the build system and looked at hwmv2 before that anyway, so if you find something to be hard in doing that or documentation is wrong then by all means open issues about them. On the board/SoC PRs I've seen with adsp (which have all been NXP except for this) they have all be added as CPU clusters.

[nordicjm]

is this not needed?

[andyross]

Zephyr will zero its own bss after boot, so no. It's a hygiene thing only (to guarantee that all memory looks consistent, even things like .noinit sections), but disabled because on one of the boards I have it appears to glitch the bus and reboot the whole device.

[nashif]

That's right, these guys still require the Cadence toolchain.

that is something you can put in the yaml file, list the toolchains supported: xcc, xt-clang or zephyr if zephyr sdk is supported.

[andyross]

SDK and HAL patches up at zephyrproject-rtos/sdk-ng#842 and zephyrproject-rtos/hal_xtensa#34 (though I'm mixed on whether to add more toolchains to the SDK, seems like it's just as simple to use the SOF instructions to build your own out of thesofproject/xtensa-overlays for the rare users who want open source tooling), will update the code here in a few hours, have a shopping trip booked for the afternoon.

[andyross]

Giant rebase finished, the big file and kconfig rename is now upstream of the mt8196 patch, and folded into the addition of 818x. Twister added. New comments addressed. Can I ask folks to please get back promptly with more criticism and replies? I really do need this merged, and am a little shocked at how difficult the process has become.

[andyross]

Also can someone provide guidance on the sudden appearance of these "ruff" failure in the python scripts? This code is almost all in-tree already, it's just being touched by the PR to add a few lines. And some of the failures seem... questionable. It's NAKing a script because the imports statements... aren't sorted?!

[andyross]

It's NAKing a script because the imports statements... aren't sorted?!

Not even "not sorted alphabetically". It has its own ideas about package order that it doesn't seem to document.

Even better, apparently we can now no longer submit a change to a python script if it happens to include a hex constant with (gasp) lower case digits. I'll fix the scripts in this PR out of necessity and malicious compliance, but something seriously needs to change with this CI rule; it's not going to scale at all.

[kartben]

It's NAKing a script because the imports statements... aren't sorted?!

Not even "not sorted alphabetically". It has its own ideas about package order that it doesn't seem to document.

Even better, apparently we can now no longer submit a change to a python script if it happens to include a hex constant with (gasp) lower case digits. I'll fix the scripts in this PR out of necessity and malicious compliance, but something seriously needs to change with this CI rule; it's not going to scale at all.

@andyross if there are scripts that you're including verbatim from other projects, like it looks to be the case based on copyright notices, it would be fine to just add them to ruff-excludes.toml IMO. cc @pdgendt

[andyross]

No, they're my scripts. Just not new in this PR yet still failing on ruff checks because like most linters it doesn't understand the idea of "only check the deltas" (which is a really hard problem). It's the standard complaint about formatters: if the existing source isn't valid already, they have the effect of disallowing all changes.

[andyross]

LOL. I've iteratively fixed the ruffination errors I can duplicate. Now it's failing with:

 Python format error:Run 'ruff format soc/mediatek/mt8xxx/gen_img.py'
File:soc/mediatek/mt8xxx/gen_img.py
 Python format error:Run 'ruff format soc/mediatek/mt8xxx/mtk_adsp_load.py'
File:soc/mediatek/mt8xxx/mtk_adsp_load.py
Compliance error, check for error messages in the "Run Compliance Tests" step
You can run this step locally with the ./scripts/ci/check_compliance.py script.

... there's no error there! What do I fix?!

[kartben]

LOL. I've iteratively fixed the ruffination errors I can duplicate. Now it's failing with:

 Python format error:Run 'ruff format soc/mediatek/mt8xxx/gen_img.py'
File:soc/mediatek/mt8xxx/gen_img.py
 Python format error:Run 'ruff format soc/mediatek/mt8xxx/mtk_adsp_load.py'
File:soc/mediatek/mt8xxx/mtk_adsp_load.py
Compliance error, check for error messages in the "Run Compliance Tests" step
You can run this step locally with the ./scripts/ci/check_compliance.py script.

... there's no error there! What do I fix?!

Didn't realize you moved the files. So yeah, the ruff-excludes.toml entries needed to be updated after all :) which would have had the original intended behavior of not complaining about existing files.
Not sure about the remaining error though :/ I guess I would try to run the ruff format commands locally to see if they output any error that the CI script would not be showing

[andyross]

Ooph. Got it to work. What that text was trying to tell me is that "ruff format" reported that it had made changes to the file, but none that it logged. You can't just run ruff and fix what it complains about, you need to run ruff and commit what it doesn't complain about. :)

[andyross]

Sigh, unbelievable. Yes, indeed I did move the files as part of the big record demanded above for hwmv2 compliance. But obviously my brain didn't file that as a "move" since it was just a minor change request trying to get this thing merged. And in any case I guess I didn't realize there was a pre-existing exclude list added for scripts already in-tree (FWIW: git understands the idea of file motion, it wouldn't be impossible to write a checker that could update the .toml based on the PR being examined).

This... really isn't supposed to be this difficult. There was a time when you could add a new board to Zephyr with half an hour of work and someone would merge it instantly. Because it's a new board; it's by definition not going to break anything and you can always fix it later iteratively. Now we're a giant tangle of fiefdoms and processes and only-mostly-documented idioms, and the only tool anyone has to exert control is the -1 button in GitHub.