-
Notifications
You must be signed in to change notification settings - Fork 321
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
intel_adsp: ace: cpu clock management #8182
intel_adsp: ace: cpu clock management #8182
Conversation
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 can only agree with this change, but the comments and values seem off?
e38a185
to
077d3c4
Compare
0dddd7e
to
a3d985d
Compare
a3d985d
to
c9e4707
Compare
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 except for two things: 1) we need a clean CI pass, and 2) please confirm we don't break git bisect.
@@ -51,7 +51,7 @@ static int basefw_config(uint32_t *data_offset, char *data) | |||
tuple = tlv_next(tuple); | |||
tlv_value_uint32_set(tuple, | |||
IPC4_SLOW_CLOCK_FREQ_HZ_FW_CFG, | |||
clock_get_freq(CPU_LPRO_FREQ_IDX)); | |||
clock_get_freq(CPU_LOWEST_FREQ_IDX)); |
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.
Hmm, will this break bisect with the previous git commit? If yes, please merge this change to the Zephyr baseline update.
Note: we should really avoid these kind of changes and provide transition period with Zephyr (i.e. add new defines in Zephyr, merge to SOF, then clean up old from Zephyr once done). E.g. now this will block @dbaluta 's PR to update Zephyr as there a interface change in Intel code.
If there's no dependency, then this please copy this note paragraphi to /dev/null.
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 corrected the order of commits to make bisection possible.
You're right about changes in zephyr. In this case it was possible to introduce them there that way so the kernel could be updated without any changes in SOF. I will remember about this next time.
c9e4707
to
fd594c0
Compare
Right, so now the remaining hurdle is this new build error on Intel MTL:
Affects #8235 as well. So we'd need a fix for this to be merged at the same time. |
@kv2019i sent a patch here: zephyrproject-rtos/zephyr#62854 |
4190a9f
to
d814fc2
Compare
d814fc2
to
077bab2
Compare
Oh man, @tmleman @dbaluta so SOF CI build is ok and pr-device-test tests even seem to pass, but:
Considering I could merge PRs on Monday with all-green in the PR results page, this is pretty disappointing, but it is what it is. It seems there are now multiple fails not related to actual SOF functionality, so I'd be inclined to merge this once mandatory CI checks passs, and file P1 bugs to track the sparse and windows build failures. |
Fix submitted: |
First you need a special version of sparse with @lyakh's fix. As always, check the build logs. In this case the "git clone sparse analyzer" step. Then: PATH=$PATH:$HOME/sparse
sof/scripts/xtensa-build-zephyr.py -p mtl --cmake-args=-DZEPHYR_SCA_VARIANT=sparse --cmake-args=-DCONFIG_LOG_USE_VLA=n |& tee sparse.log
scripts/parse_sparse_output.sh < sparse.log
Same as what CI does, no CI secret. |
Confirmed:
|
Maybe we should wait for that too zephyrproject-rtos/zephyr#63004 |
@tmleman zephyrproject-rtos/zephyr#63004 now merged, can you update this PR? |
This patch adds CPU_LOWEST_FREQ_IDX definition to keep compliance with changes in base_fw. Signed-off-by: Tomasz Leman <[email protected]>
This patch is changing value of slow clock in response for FwConfigGet from LP clock to the lowest clock is section "slow clock". Signed-off-by: Tomasz Leman <[email protected]>
ACE_1.5 and ACE_2.0 use only two clocks for DSP cores. First is WOVRCO and second is ACE IPLL. IPLL allows to configure it to work like LP RING Oscillator Clock or HP RING Oscillator Clock. Currently, the driver does not allow this, so I remove the frequency that cannot be achieved anyway. Clocks frequencies: WOV: 38.4 MHz IPLL: 393.216 MHz Signed-off-by: Tomasz Leman <[email protected]>
Changing max clock frequency for FPGA configuration. Signed-off-by: Tomasz Leman <[email protected]>
Zepych update: total of 1048 commits. Changes include: b2f7ea0523 soc: xtensa/intel_adsp/ace: fix _end location e560bd6b8c boards: intel_adsp: fix board compatible b4998c357e mm_drv: tlb: Fix compile time warning 759e07bebe intel_adsp: move memory window setup to PRE_KERNEL_1 492517b918 west.yml: Update NXP HAL SDK to 2.14 a5d1fd9857 soc: adsp: clk: update clock switch flow 9656056b19 dts: adsp: ace20: remove lp clock 50f0e223e8 dts: adsp: ace15: remove lp clock cf6d5f95b6 adsp: clk: ace: select ipll if wovrco is unavailable 2d835e1b29 dts: adsp: ace20: replace hp with ipll clock dcecda859c dts: adsp: ace15: replace hp with ipll clock 2f2689e3d3 intel_adsp: ace15: shim: update wovrco request bit ea9dd59460 yamllint: bindings: add ipll clock index 1ddabfa8d8 dai: intel: dmic: fix shadow variable b26921d776 dai: intel: dmic: New functions for writing fir coefficients cba9ec10c3 dai: intel: tgl: dmic: Refactor of dai_nhlt_dmic_dai_params_get function c28e8ba9ba dai: intel: dmic: Add pdm_base and pdm_idx variables in blob parser 2452aaad50 dai: intel: dmic: Separate fir configuration code into function f74fd8edaf dai: intel: ace: dmic: Add dai_dmic_start_fifo_packers function 76d03e798f dai: intel: ace: dmic: Using the WAIT_FOR macro in waiting functions 3fbaed4de9 dai: intel: ace: dmic: Refactor of dai_nhlt_dmic_dai_params_get function d7672af838 dai: intel: dmic: Combine PDM registers definitions 8ea53d49b6 dai: intel: dmic: nhlt: Move debug print code to a separate functions 81944c5c62 dai: intel: dmic: Move definitions of nhlt structures to a new file Signed-off-by: Tomasz Leman <[email protected]>
077bab2
to
911f89d
Compare
Nice, https://sof-ci.01.org/sofpr/PR8182/build13439/devicetest/index.html (all RED) I guess zephyrproject-rtos/zephyr#63004 was not tested on hardware...? It seems none of the resulting binaries boot anymore. |
I can repro the problem with a local build. DSP does not boot. But it's not zephyrproject-rtos/zephyr#63004 , if only that is reverted, boot still fails. More bisect is needed (plus it's not clear why some CI tests passed on this hardware). |
I'm closing this PR. Those changes and zephyr version update are made here #8268. |
Updated clock definitions for ACE family platforms.