pipeline: register KCPS consumption and enable dynamic clock control #8019
Conversation
abonislawski
requested review from
lgirdwood,
plbossart,
mmaka1,
lbetlej and
dbaluta
as code owners
There was a problem hiding this comment.
Thanks @abonislawski ! Code looks good, a few minor nits for the documentation.
There was a problem hiding this comment.
@abonislawski ping - I think this fixes some reported issues.
Unfortunately not, I will post more info in issue thread |
abonislawski
force-pushed
the
cpc_main
branch
4 times, most recently
from
2ede417 to
eb53828
Compare
src/audio/pipeline/pipeline-stream.c
Outdated
| current->ipc_config.id, ppl_data->p->core, cd->cpc); | ||
| } | ||
|
|
||
| kcps = cd->cpc * 1000 / ppl_data->p->period; |
There was a problem hiding this comment.
I assume we do check period for non zero ? (since it comes from host)
There was a problem hiding this comment.
p->trigger.delay = (data.delay_ms * 1000 + p->period - 1) / p->period;
similar code, should checked.
There was a problem hiding this comment.
I saw this is a fixed value in ipc4_create_pipeline pipe->period = LL_TIMER_PERIOD_US;
anyway what 0 period means for pipeline? Just use module cpc value in such case?
There was a problem hiding this comment.
@lgirdwood this is only one unresolved issue after today's update. Im not sure what 0 period could mean?
and if this is error condition - can we get it so late at the trigger stage?
| @@ -86,3 +86,4 @@ CONFIG_DEBUG_COREDUMP_MEMORY_DUMP_MIN=y | |||
|
|
|||
| CONFIG_PROBE=y | |||
| CONFIG_PROBE_DMA_MAX=2 | |||
| CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL=y | |||
There was a problem hiding this comment.
@abonislawski I'm happy to merge this if this defaults to "n" at the moment until we can resolve all the opens. We also need to add and align all the CPCs on manifest (and that's still WIP, but > 50% done). CI tests for CPC also landing soon, so safe to make default "y" when all pieces are in place.
There was a problem hiding this comment.
What's the latest status, do we have any known opens in mainline left? @abonislawski
There was a problem hiding this comment.
@abonislawski can you confirm this is now working as expected ? I suppose CI will show us if any CPC numbers are worng, but only for tested modules today.
There was a problem hiding this comment.
It should work correctly, already used in a few MTL releases and we have cpc data for most (or all) of our modules.
Updated today with rebase & review fixes
|
@abonislawski , could you list here how you get each component cpc data? by below ways?
|
src/audio/pipeline/pipeline-stream.c
Outdated
| current->ipc_config.id, ppl_data->p->core, cd->cpc); | ||
| } | ||
|
|
||
| kcps = cd->cpc * 1000 / ppl_data->p->period; |
There was a problem hiding this comment.
p->trigger.delay = (data.delay_ms * 1000 + p->period - 1) / p->period;
similar code, should checked.
abonislawski
force-pushed
the
cpc_main
branch
2 times, most recently
from
08d11ef to
bb8b4d6
Compare
src/audio/pipeline/pipeline-stream.c
Outdated
| return PPL_STATUS_PATH_STOP; | ||
| } else { | ||
| kcps = cd->cpc * 1000 / current->period; | ||
| //tr_dbg(pipe, "Registering module: %#x KCPS consumption: %d, core: %d", |
|
@abonislawski re the CI test failures, are we missing anything on the kernel side. i.e. does kernel need to do any IP programming flow around DSP clock change ? |
|
@lgirdwood We shouldnt need anything from kernel side as this is already used on MTL branches.
I will check that part |
|
@abonislawski wrote:
At least in some cases, there are overruns recorded. This can be due to CPC being too low for debug build, leading to too low DSP freq used for a use-case: |
|
Still failure here on MTL nocodec https://sof-ci.01.org/sofpr/PR8019/build3645/devicetest/index.html?model=MTLP_RVP_NOCODEC&testcase=check-capture-10sec |
|
Seems to break the testbench: |
Such notification currently is used only in non-zephyr code (ll sched) so it is safe to remove for MTL because of some idc issues Signed-off-by: Adrian Bonislawski <[email protected]>
KCPS budged should be initialized before any KCPS adjustments Signed-off-by: Adrian Bonislawski <[email protected]>
abonislawski
force-pushed
the
cpc_main
branch
2 times, most recently
from
004335c to
96cda9a
Compare
|
This compilation failure seems related: |
abonislawski
force-pushed
the
cpc_main
branch
2 times, most recently
from
8d32afb to
12bd7d1
Compare
There was a problem hiding this comment.
@abonislawski LGTM,
@ujfalusi is kernel ready for KCPS ? i.e. if some modules have missing or wrong KCPS values then kernel will handle, with no audio impact and no dmesg spam ?
There was a problem hiding this comment.
Let me mark with a -1 so this is not accidentally merged. Code looks good, but we still have CI tests failing.
E.g. https://sof-ci.01.org/sofpr/PR8019/build3791/devicetest/index.html still in today's run
Register and unregister pipelines CPS consumption on run/pause Signed-off-by: Adrian Bonislawski <[email protected]> Signed-off-by: Krzysztof Frydryk <[email protected]> +Squashed commits: 924f0d2 pipeline: print warning if 0 KCPS received Signed-off-by: Adrian Bonislawski <[email protected]> 7bb6e7e pipeline: register CPS consumption for a proper core Signed-off-by: Adrian Bonislawski <[email protected]> 5f0c4e4 pipeline: make CPC data "opt-in" with fallback Signed-off-by: Liam Girdwood <[email protected]> Signed-off-by: Peter Ujfalusi <[email protected]>
On init, register consumption of 10MCPS for base fw if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL enabled Signed-off-by: Adrian Bonislawski <[email protected]> Signed-off-by: Krzysztof Frydryk <[email protected]>
This will enable dynamic clock control based on KCPS budget Signed-off-by: Adrian Bonislawski <[email protected]>
|
Internal and Linux CI pass I have also merged two ppl consumption functions into single pipeline_calc_cps_consumption |
This will enable dynamic clock control based on KCPS budget.
Mostly cherry-picked from mtl-005-drop-stable + kconfig
pipeline: Register and unregister pipelines CPS consumption on run/pause
+Squashed commits:
924f0d2 pipeline: print warning if 0 KCPS received
7bb6e7e pipeline: register CPS consumption for a proper core
5f0c4e4 pipeline: make CPC data "opt-in" with fallback
platform: register basefw CPS consumption on boot
+Squashed commit:
38b4cb1 platform: mtl: lowest clock as default
boards: mtl: enable KCPS dynamic clock