intel_adsp: ace: cpu clock management by tmleman · Pull Request #8182 · thesofproject/sof

tmleman · 2023-09-11T11:14:41Z

Updated clock definitions for ACE family platforms.

intel_adsp: ace: cpu clock management zephyrproject-rtos/zephyr#62452

plbossart

I can only agree with this change, but the comments and values seem off?

kv2019i

Looks good except for two things: 1) we need a clean CI pass, and 2) please confirm we don't break git bisect.

kv2019i · 2023-09-20T11:33:18Z

 	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));


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.

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.

kv2019i · 2023-09-20T15:44:13Z

Right, so now the remaining hurdle is this new build error on Intel MTL:

In file included from /zep_workspace/zephyr/include/zephyr/sys/mem_manage.h:104,
                 from /zep_workspace/zephyr/include/zephyr/sys/device_mmio.h:47,
                 from /zep_workspace/zephyr/include/zephyr/device.h:15,
                 from /zep_workspace/zephyr/drivers/mm/mm_drv_intel_adsp.h:17,
                 from /zep_workspace/zephyr/drivers/mm/mm_drv_intel_adsp_mtl_tlb.c:23:
/zep_workspace/zephyr/drivers/mm/mm_drv_intel_adsp_mtl_tlb.c: In function 'sys_mm_drv_mm_init':
/zep_workspace/zephyr/include/zephyr/sys/__assert.h:44:52: error: format '%p' expects argument of type 'void *', but argument 2 has type 'long unsigned int' [-Werror=format=]

Affects #8235 as well. So we'd need a fix for this to be merged at the same time.

dbaluta · 2023-09-20T15:50:05Z

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

kv2019i · 2023-09-22T13:41:42Z

Oh man, @tmleman @dbaluta so SOF CI build is ok and pr-device-test tests even seem to pass, but:

we have one mandatory Intel CI failing
all Windows builds fail (lost ninja, no idea what is causing this)
the sparse build is broken after the picolibc change

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.

kv2019i · 2023-09-22T13:42:43Z

@marc-hb @lyakh Any insights on the sparse build fail? I couldn't make it work locally even without this PR, so I have no immediate insight how to go about to fix this...

marc-hb · 2023-09-22T20:39:33Z

all Windows builds fail (lost ninja, no idea what is causing this)

Fix submitted:

[BUG] [Windows CI] CMake was unable to find a build program corresponding to "Ninja" #8250

marc-hb · 2023-09-22T20:46:53Z

I couldn't make it work locally even without this PR

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.

marc-hb · 2023-09-22T21:21:16Z

the sparse build is broken after the picolibc change
https://github.com/thesofproject/sof/actions/runs/6272123572/job/17033041652?pr=8182

Confirmed:

CONFIG_PICOLIBC + sparse = zephyr/posix/sys/stat.h:40:11: error: unable to open 'sys/_timespec.h' zephyrproject-rtos/zephyr#63003

CONFIG_MINIMAL_LIBC=y in an overlay should be enough to avoid it. EDIT: merged.

tmleman · 2023-09-25T11:39:59Z

Maybe we should wait for that too zephyrproject-rtos/zephyr#63004

kv2019i · 2023-09-25T16:41:06Z

@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 <tomasz.m.leman@intel.com>

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 <tomasz.m.leman@intel.com>

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 <tomasz.m.leman@intel.com>

Changing max clock frequency for FPGA configuration. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>

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 <tomasz.m.leman@intel.com>

kv2019i · 2023-09-25T18:34:05Z

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.

kv2019i · 2023-09-26T09:24:17Z

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

tmleman · 2023-09-27T12:29:48Z

I'm closing this PR. Those changes and zephyr version update are made here #8268.

tmleman requested review from dbaluta, kv2019i, lbetlej, lgirdwood, mmaka1 and plbossart as code owners September 11, 2023 11:14

plbossart reviewed Sep 12, 2023

View reviewed changes

Comment thread app/overlays/mtl/fpga_overlay.conf Outdated

Comment thread src/platform/lunarlake/include/platform/lib/clk.h

tmleman force-pushed the topic/upstream/pr/ace/clock_mng/def_update branch from e38a185 to 077d3c4 Compare September 14, 2023 08:08

tmleman requested a review from plbossart September 14, 2023 08:09

tmleman force-pushed the topic/upstream/pr/ace/clock_mng/def_update branch 2 times, most recently from 0dddd7e to a3d985d Compare September 18, 2023 11:07

tmleman changed the title ~~[DNM] intel_adsp: ace: cpu clock management~~ intel_adsp: ace: cpu clock management Sep 18, 2023

btian1 reviewed Sep 20, 2023

View reviewed changes

Comment thread src/platform/lunarlake/include/platform/lib/clk.h

tmleman force-pushed the topic/upstream/pr/ace/clock_mng/def_update branch from a3d985d to c9e4707 Compare September 20, 2023 11:07

tmleman mentioned this pull request Sep 20, 2023

west.yml: update Zephyr to 492517b918d26 #8235

Closed

kv2019i reviewed Sep 20, 2023

View reviewed changes

tmleman force-pushed the topic/upstream/pr/ace/clock_mng/def_update branch from c9e4707 to fd594c0 Compare September 20, 2023 12:19

tmleman requested review from abonislawski, jxstelter and marcinszkudlinski September 20, 2023 12:31

dbaluta approved these changes Sep 20, 2023

View reviewed changes

jxstelter approved these changes Sep 20, 2023

View reviewed changes

dnikodem approved these changes Sep 21, 2023

View reviewed changes

plbossart approved these changes Sep 21, 2023

View reviewed changes

btian1 approved these changes Sep 22, 2023

View reviewed changes

tmleman force-pushed the topic/upstream/pr/ace/clock_mng/def_update branch 3 times, most recently from 4190a9f to d814fc2 Compare September 22, 2023 08:17

tmleman force-pushed the topic/upstream/pr/ace/clock_mng/def_update branch from d814fc2 to 077bab2 Compare September 22, 2023 08:46

pblaszko approved these changes Sep 25, 2023

View reviewed changes

tmleman added 5 commits September 25, 2023 18:55

platform: posix: clk: add lowest freq index

5ea9071

This patch adds CPU_LOWEST_FREQ_IDX definition to keep compliance with changes in base_fw. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>

base_fw: get fw config

117146c

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 <tomasz.m.leman@intel.com>

ace: overlay: update clock frequency

bb60a3f

Changing max clock frequency for FPGA configuration. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>

tmleman force-pushed the topic/upstream/pr/ace/clock_mng/def_update branch from 077bab2 to 911f89d Compare September 25, 2023 17:04

tmleman mentioned this pull request Sep 26, 2023

west.yml: update Zephyr to ca3310145f8f #8264

Closed

tmleman closed this Sep 27, 2023

tmleman deleted the topic/upstream/pr/ace/clock_mng/def_update branch December 17, 2025 12:14

Conversation

tmleman commented Sep 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

kv2019i Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

tmleman Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

kv2019i commented Sep 20, 2023

Uh oh!

dbaluta commented Sep 20, 2023

Uh oh!

kv2019i commented Sep 22, 2023

Uh oh!

kv2019i commented Sep 22, 2023

Uh oh!

marc-hb commented Sep 22, 2023

Uh oh!

marc-hb commented Sep 22, 2023

Uh oh!

marc-hb commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmleman commented Sep 25, 2023

Uh oh!

kv2019i commented Sep 25, 2023

Uh oh!

kv2019i commented Sep 25, 2023 • edited by marc-hb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kv2019i commented Sep 26, 2023

Uh oh!

tmleman commented Sep 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

tmleman commented Sep 11, 2023 •

edited

Loading

marc-hb commented Sep 22, 2023 •

edited

Loading

kv2019i commented Sep 25, 2023 •

edited by marc-hb

Loading