Skip to content

Add new IPC for trace dma free #3197

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
plbossart merged 3 commits into from
Oct 21, 2021
Merged

Add new IPC for trace dma free #3197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
18b4087
ASoC: SOF: IPC: Add new IPC command to free trace DMA
ranj063 Oct 5, 2021
0a6e16c
ASoC: SOF: IPC: update ipc_log_header()
ranj063 Oct 15, 2021
d4de613
ASoC: SOF: trace: send DMA_TRACE_FREE IPC during release
ranj063 Oct 5, 2021
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
1 change: 1 addition & 0 deletions include/sound/sof/header.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
#define SOF_IPC_TRACE_DMA_POSITION SOF_CMD_TYPE(0x002)
#define SOF_IPC_TRACE_DMA_PARAMS_EXT SOF_CMD_TYPE(0x003)
#define SOF_IPC_TRACE_FILTER_UPDATE SOF_CMD_TYPE(0x004) /**< ABI3.17 */
#define SOF_IPC_TRACE_DMA_FREE SOF_CMD_TYPE(0x005) /**< ABI3.20 */

/* debug */
#define SOF_IPC_DEBUG_MEM_USAGE SOF_CMD_TYPE(0x001)
Expand Down
17 changes: 16 additions & 1 deletion sound/soc/sof/ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,22 @@ static void ipc_log_header(struct device *dev, u8 *text, u32 cmd)
}
break;
case SOF_IPC_GLB_TRACE_MSG:
str = "GLB_TRACE_MSG"; break;
str = "GLB_TRACE_MSG";
switch (type) {
case SOF_IPC_TRACE_DMA_PARAMS:
str2 = "DMA_PARAMS"; break;
case SOF_IPC_TRACE_DMA_POSITION:
str2 = "DMA_POSITION"; break;
case SOF_IPC_TRACE_DMA_PARAMS_EXT:
str2 = "DMA_PARAMS_EXT"; break;
case SOF_IPC_TRACE_FILTER_UPDATE:
str2 = "FILTER_UPDATE"; break;
case SOF_IPC_TRACE_DMA_FREE:
str2 = "DMA_FREE"; break;
default:
str2 = "unknown type"; break;
}
break;
case SOF_IPC_GLB_TEST_MSG:
str = "GLB_TEST_MSG";
switch (type) {
Expand Down
18 changes: 18 additions & 0 deletions sound/soc/sof/trace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,10 @@ EXPORT_SYMBOL(snd_sof_trace_notify_for_error);

void snd_sof_release_trace(struct snd_sof_dev *sdev)
{
struct sof_ipc_fw_ready *ready = &sdev->fw_ready;
struct sof_ipc_fw_version *v = &ready->version;
struct sof_ipc_cmd_hdr hdr;
struct sof_ipc_reply ipc_reply;
Copy link
Collaborator

@ujfalusi ujfalusi Oct 15, 2021

Choose a reason for hiding this comment

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

nitpick: can you swap these?

or move them local under the if(ABI) ?

Copy link
Member

@plbossart plbossart Oct 20, 2021

Choose a reason for hiding this comment

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

@ranj063 let's do this change suggested by @ujfalusi with a fixup

int ret;

if (!sdev->dtrace_is_supported || !sdev->dtrace_is_enabled)
Expand All @@ -549,6 +553,20 @@ void snd_sof_release_trace(struct snd_sof_dev *sdev)
dev_err(sdev->dev,
"error: snd_sof_dma_trace_trigger: stop: %d\n", ret);

/*
* stop and free trace DMA in the DSP. TRACE_DMA_FREE is only supported from
* ABI 3.20.0 onwards
*/
if (v->abi_version >= SOF_ABI_VER(3, 20, 0)) {
hdr.size = sizeof(hdr);
hdr.cmd = SOF_IPC_GLB_TRACE_MSG | SOF_IPC_TRACE_DMA_FREE;

ret = sof_ipc_tx_message(sdev->ipc, hdr.cmd, &hdr, hdr.size,
&ipc_reply, sizeof(ipc_reply));
if (ret < 0)
dev_err(sdev->dev, "DMA_TRACE_FREE failed with error: %d\n", ret);
Copy link
Member

@plbossart plbossart Oct 20, 2021

Choose a reason for hiding this comment

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

return ret?

or add a comment that on why we don't return?

This can be done with a fixup PR.

Copy link
Collaborator

@ujfalusi ujfalusi Oct 21, 2021

Choose a reason for hiding this comment

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

I think the intention is to continue to the host side release, even if the IPC fails, in this case this should be dev_warn()?

}

ret = snd_sof_dma_trace_release(sdev);
if (ret < 0)
dev_err(sdev->dev,
Expand Down