Skip to content

ipc4: fixes for waiting. #8000

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.

Sign up for GitHub

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

Merged
lgirdwood merged 3 commits into from
Aug 3, 2023
Merged

ipc4: fixes for waiting. #8000

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba311d8
ipc4: more dbg context for pipeline state changes
lrgirdwo Aug 1, 2023
19cfb8d
ipc4: relax the IPC timeout checks and be nicer to other threads
lrgirdwo Aug 1, 2023
664b94b
ipc4: ipc atomic state should be reset after timeout.
lrgirdwo Aug 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions src/ipc/ipc4/handler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,10 @@ int set_pipeline_state(struct ipc_comp_dev *ppl_icd, uint32_t cmd,
int status;
int ret;

tr_dbg(&ipc_tr, "ipc4 set pipeline %d state %x:", ppl_icd->id, cmd);

status = ppl_icd->pipeline->status;
tr_dbg(&ipc_tr, "ipc4 set pipeline %d cmd state %x: from state %x", ppl_icd->id,
cmd, status);

/* source & sink components are set when pipeline is set to COMP_STATE_INIT */
if (status != COMP_STATE_INIT) {
int host_id;
Expand All @@ -252,6 +253,8 @@ int set_pipeline_state(struct ipc_comp_dev *ppl_icd, uint32_t cmd,
case COMP_STATE_READY:
cmd = COMP_TRIGGER_PRE_START;

tr_dbg(&ipc_tr, "ipc4: set pipeline %d new cmd state %x: params",
ppl_icd->id, cmd);
ret = ipc4_pcm_params(host);
if (ret < 0)
return IPC4_INVALID_REQUEST;
Expand All @@ -267,6 +270,8 @@ int set_pipeline_state(struct ipc_comp_dev *ppl_icd, uint32_t cmd,
case SOF_IPC4_PIPELINE_STATE_RESET:
switch (status) {
case COMP_STATE_INIT:
tr_dbg(&ipc_tr, "ipc4: set pipeline %d new cmd state %x: reset from init",
ppl_icd->id, cmd);
ret = ipc4_pipeline_complete(ipc, ppl_icd->id);
if (ret < 0)
ret = IPC4_INVALID_REQUEST;
Expand All @@ -277,6 +282,8 @@ int set_pipeline_state(struct ipc_comp_dev *ppl_icd, uint32_t cmd,
return 0;
case COMP_STATE_ACTIVE:
case COMP_STATE_PAUSED:
tr_dbg(&ipc_tr, "ipc4: set pipeline %d new cmd state %x: pause from reset",

@plbossart plbossart Aug 2, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would write active/pause from reset since there's a fallback case, but that's a nit-pick

@lgirdwood lgirdwood Aug 3, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh, the state changes confused me a bit here. It probably needs more inline commentry from an IPC4 expert.

ppl_icd->id, cmd);
ret = pipeline_trigger(host->cd->pipeline, host->cd, COMP_TRIGGER_STOP);
if (ret < 0) {
ipc_cmd_err(&ipc_tr, "ipc: comp %d trigger 0x%x failed %d",
Expand All @@ -297,6 +304,8 @@ int set_pipeline_state(struct ipc_comp_dev *ppl_icd, uint32_t cmd,
* executing in the pipeline task.
*/
if (!*delayed) {
tr_dbg(&ipc_tr, "ipc4: set pipeline %d new cmd state %x: delayed reset",
ppl_icd->id, cmd);
ret = pipeline_reset(host->cd->pipeline, host->cd);
if (ret < 0)
ret = IPC4_INVALID_REQUEST;
Expand All @@ -306,6 +315,8 @@ int set_pipeline_state(struct ipc_comp_dev *ppl_icd, uint32_t cmd,
case SOF_IPC4_PIPELINE_STATE_PAUSED:
switch (status) {
case COMP_STATE_INIT:
tr_dbg(&ipc_tr, "ipc4: set pipeline %d new cmd state %x: pause from init",
ppl_icd->id, cmd);
ret = ipc4_pipeline_complete(ipc, ppl_icd->id);
if (ret < 0)
ret = IPC4_INVALID_REQUEST;
Expand All @@ -330,12 +341,17 @@ int set_pipeline_state(struct ipc_comp_dev *ppl_icd, uint32_t cmd,
return IPC4_INVALID_REQUEST;
}

tr_dbg(&ipc_tr, "ipc4: set pipeline %d new cmd state %x: from state %x", ppl_icd->id,
cmd, status);

/* trigger the component */
ret = pipeline_trigger(host->cd->pipeline, host->cd, cmd);
if (ret < 0) {
ipc_cmd_err(&ipc_tr, "ipc: comp %d trigger 0x%x failed %d", ppl_icd->id, cmd, ret);
ret = IPC4_PIPELINE_STATE_NOT_SET;
} else if (ret == PPL_STATUS_SCHEDULED) {
tr_dbg(&ipc_tr, "ipc4: set pipeline %d new cmd state %x: trigger delayed",
ppl_icd->id, cmd);
Comment thread
btian1 marked this conversation as resolved.
Outdated
*delayed = true;
ret = 0;
}
Expand Down Expand Up @@ -380,15 +396,17 @@ static void ipc_compound_msg_done(uint32_t msg_id, int error)
}
}

/* wait for IPCs to complete on other cores and be nice to any LL work */
static int ipc_wait_for_compound_msg(void)
{
int try_count = 30;
int try_count = 30; /* timeout out is 30 x 10ms so 300ms for IPC */

while (atomic_read(&msg_data.delayed_reply)) {
k_sleep(Z_TIMEOUT_US(250));
k_sleep(Z_TIMEOUT_MS(10));
Comment thread
btian1 marked this conversation as resolved.
Outdated

if (!try_count--) {
ipc_cmd_err(&ipc_tr, "failed to wait schedule thread");
atomic_set(&msg_data.delayed_reply, 0);
ipc_cmd_err(&ipc_tr, "ipc4: failed to wait schedule thread");
return IPC4_FAILURE;
}
}
Expand Down