DRAM: more cold functions #9850
Conversation
Move several initialisation functions to run from DRAM directly. Signed-off-by: Guennadi Liakhovetski <[email protected]>
Mark all IPC functions as "cold" to run them directly in DRAM. Signed-off-by: Guennadi Liakhovetski <[email protected]>
lyakh
requested review from
bardliao,
marcinszkudlinski,
pblaszko,
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
|
I understand why init functions should go to DRAM, but why IPC? |
@marcinszkudlinski the idea is that only audio protocols are "hot" - only schedulers and audio processing threads. Everything else can be "cold" and IPC processing is one of such large code areas. But if you have concerns that this can break something, let's discuss, maybe we're overlooking some use-cases? |
|
@lyakh not really I think - as long as we do have enough HPSRAM, use it. |
|
IPC part looks really suspicious, do you have any data what is the profit and perf drop? Especially when main CPU is under high load and we will lag more with DRAM access |
|
HPSRAM is precious, agree need to be really careful what we put in DRAM it should only be parts of IPC that are not time critical. i.e. trigger is time critical, but load module is not time critical. We need to find this balance, Linux only really cares about prepare()/trigger() driver ops and any associated IPCs. Don't know about Windows ? |
| @@ -197,7 +198,7 @@ int comp_buffer_connect(struct comp_dev *comp, uint32_t comp_core, | |||
| return pipeline_connect(comp, buffer, dir); | |||
| } | |||
|
|
|||
| int ipc_pipeline_complete(struct ipc *ipc, uint32_t comp_id) | |||
| __cold int ipc_pipeline_complete(struct ipc *ipc, uint32_t comp_id) | |||
There was a problem hiding this comment.
Can you check this, not sure if done in prepare() ? maybe for IPC3 only ?
There was a problem hiding this comment.
@lgirdwood this is only called from
ipc4_pipeline_complete()
ipc4_pipeline_prepare()
ipc4_set_pipeline_state() idc_ppl_state()
ipc4_process_glb_message() idc_cmd()
ipc_cmd() idc_handler()
ipc_platform_do_cmd() idc_ipc() P4WQ
ipc_do_cmd() idc_cmd()
EDF scheduler idc_handler()
P4WQ
so, it's only called from the EDF scheduler or from IDC P4WQ, both of which use the EDF_ZEPHYR_PRIORITY priority (currently 1)
|
@lgirdwood @marcinszkudlinski @abonislawski as far as I understand the worst would be cases when we're running close to 100% performance capacity and at that moment the user is issuing some IPCs - maybe to start an additional light stream. In principle we still have a couple of free DSP cycles to run an additional stream, but while preparing it, IPC processing adds significant DSP load. So, if we process IPCs in DRAM, that processing becomes slower. As long as we don't disable interrupts during IPC processing for too long, we still shouldn't disturb higher priority audio processing, running in parallel, but IPC response time will become longer. Is that what we're worried about? Is that important? Replying to @marcinszkudlinski - do we really lose LL cycles because of IPC processing? That shouldn't happen AFAICS? If we have code, locking interrupts, we have to identify and improve it... |
|
Replying to @marcinszkudlinski - do we really lose LL cycles because of IPC processing? That shouldn't happen AFAICS? If we have code, locking interrupts, we have to identify and improve it... We don't lose LL cycles since LL preempts low priority workloads/threads (even if workload TEXT is in DRAM, stack/heap will be SRAM). @jsarha can you share some data soon. Thanks |
Move all of IPC and some initialisation code to DRAM.