Skip to content

SOF client support (auxiliary bus) #3007

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

Closed
ujfalusi wants to merge 32 commits into from
Closed

SOF client support (auxiliary bus) #3007

Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
a87a2b5
ASoC: SOF: Move the definition of enum snd_sof_fw_state to global header
ujfalusi Aug 13, 2021
4d512ec
ASoC: SOF: Introduce a macro to set the firmware state
ujfalusi Aug 16, 2021
44b2a43
ASoC: SOF: debug: Print out the fw_state along with the DSP dump
ujfalusi Aug 18, 2021
9115f5b
ASoC: SOF: Introduce new firmware state: SOF_FW_CRASHED
ujfalusi Aug 17, 2021
476d4c0
ASoC: soc-component: Add support module get/put on open for compresse…
ujfalusi Jul 23, 2021
7274b1c
ASoC: SOF: Introduce IPC SOF client support
ranj063 Oct 22, 2020
6a772ce
ASoC: SOF: core/ops: Add support for client registration
ranj063 Oct 22, 2020
a858cfa
ASoC: SOF: core: Unregister machine driver before IPC and debugfs
ujfalusi Jul 23, 2021
49abfdc
ASoC: SOF: ipc: Read and pass the whole message to handlers for IPC e…
ujfalusi Jul 6, 2021
11f3b3a
ASoC: SOF: clients: Add support for IPC rx and firmware state change
ujfalusi Jul 6, 2021
20c9323
ASoC: SOF: clients: Add support for auxdev suspend/resume handling
ujfalusi Jul 9, 2021
9e32894
ASoC: SOF: clients: Add API to get the SOF firmware version
ujfalusi Jul 7, 2021
bcaad35
ASoC: SOF: clients: Add API to manage the module refcount of SOF core
ujfalusi Jul 21, 2021
92a48c2
ASoC: SOF: intel: hda-trace: Pass the dma buffer pointer to hda_dsp_t…
ujfalusi Jul 9, 2021
9e675f2
ASoC: SOF: Split up utils.c into sof-utils and iomem-utils
ujfalusi Jul 15, 2021
deb5a0c
ASoC: SOF: Convert the generic IPC flood test into SOF client
ranj063 Oct 22, 2020
cba8df9
ASoC: SOF: sof-client-ipc-test: Protection against removal while in use
ujfalusi Jul 22, 2021
64a91b8
ASoC: SOF: sof-client-ipc-test: Block the test if the firmware has cr…
ujfalusi Aug 20, 2021
5f585a8
ASoC: SOF: Convert the generic probe support to SOF client
ujfalusi Jun 9, 2021
d6a888d
ASoC: SOF: sof-client-probes: Add module parameter to enable probes s…
ujfalusi Aug 9, 2021
6677eec
ASoC: SOF: sof-client-probes: Protection against removal while in use
ujfalusi Jul 22, 2021
68d6c63
ASoC: SOF: sof-client-probes: Block the capture if the firmware is cr…
ujfalusi Aug 20, 2021
39459f7
ASoC: SOF: Add optional SOF client for dma-trace support
ujfalusi Jul 6, 2021
9e6b984
ASoC: SOF: imx: Enable SOF client version of dma-trace
ujfalusi Aug 18, 2021
e8bab41
ASoC: SOF: Switch to the client driver for dma-trace support
ujfalusi Jul 19, 2021
5c75bc5
ASoC: SOF: sof-client-dma-trace: Add protection against file/module r…
ujfalusi Jul 21, 2021
b3089af
ASoC: SOF: sof-client-dma-trace: Block the dtrace if the firmware has…
ujfalusi Aug 20, 2021
b269552
ASoC: SOF: sof-client-dma-trace: Simplify count adjustment in trace_read
ujfalusi Jul 16, 2021
fd4a66c
ASoC: SOF: sof-client-dma-trace: Coding style cleanups
ujfalusi Jul 16, 2021
5e3f492
ASoC: SOF: sof-client-ipc-test: Code cleanup for consistency
ujfalusi Jul 22, 2021
f763eea
ASoC: SOF: sof-client-probes: Code cleanup for consistency
ujfalusi Jul 22, 2021
3bd20bb
ASoC: SOF: sof-client-dma-trace: Code cleanup for consistency
ujfalusi Jul 22, 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
76 changes: 36 additions & 40 deletions sound/soc/sof/ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
#include "sof-audio.h"
#include "ops.h"

static void ipc_trace_message(struct snd_sof_dev *sdev, u32 msg_type);
static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_cmd);
typedef void (*ipc_rx_callback)(struct snd_sof_dev *sdev, void *full_msg);

static void ipc_trace_message(struct snd_sof_dev *sdev, void *full_msg);
static void ipc_stream_message(struct snd_sof_dev *sdev, void *full_msg);

@plbossart plbossart Aug 24, 2021

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 don't get the concept of passing a void pointer without any indication of size to the client. How would they know how much data they can read from the 'full_msg' area?

@ujfalusi ujfalusi Aug 26, 2021

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is the full (and valid IPC message) and the handlers should cast it to the struct they use internally. The size of the message is in the message itself, in the header.
We could use struct sof_ipc_cmd_hdr *full_msg, but it is not really the full message or we can say that it is a pointer to a header, but then it has to be documented that the header pointer is pointing to the beginning of the full message.

I prefer it to be void and let the handlers decide how to interpret the message they are given, in the 'core' we don't really care about any of the details.


/*
* IPC message Tx/Rx message handling.
Expand Down Expand Up @@ -389,44 +391,30 @@ void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id)
}
EXPORT_SYMBOL(snd_sof_ipc_reply);

static void ipc_comp_notification(struct snd_sof_dev *sdev,
struct sof_ipc_cmd_hdr *hdr)
static void ipc_comp_notification(struct snd_sof_dev *sdev, void *full_msg)
{
struct sof_ipc_cmd_hdr *hdr = full_msg;
u32 msg_type = hdr->cmd & SOF_CMD_TYPE_MASK;
struct sof_ipc_ctrl_data *cdata;
int ret;

switch (msg_type) {
case SOF_IPC_COMP_GET_VALUE:
case SOF_IPC_COMP_GET_DATA:
cdata = kmalloc(hdr->size, GFP_KERNEL);
if (!cdata)
return;

/* read back full message */
ret = snd_sof_ipc_msg_data(sdev, NULL, cdata, hdr->size);
if (ret < 0) {
dev_err(sdev->dev,
"error: failed to read component event: %d\n", ret);
goto err;
}
break;
default:
dev_err(sdev->dev, "error: unhandled component message %#x\n", msg_type);
return;
}

snd_sof_control_notify(sdev, cdata);

err:
kfree(cdata);
snd_sof_control_notify(sdev, full_msg);
}

/* DSP firmware has sent host a message */
void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev)
{
ipc_rx_callback rx_callback = NULL;
struct sof_ipc_cmd_hdr hdr;
u32 cmd, type;
void *full_msg;
u32 cmd;
int err;

/* read back header */
Expand All @@ -438,7 +426,6 @@ void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev)
ipc_log_header(sdev->dev, "ipc rx", hdr.cmd);

cmd = hdr.cmd & SOF_GLB_TYPE_MASK;
type = hdr.cmd & SOF_CMD_TYPE_MASK;

/* check message type */
switch (cmd) {
Expand All @@ -463,20 +450,34 @@ void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev)
case SOF_IPC_GLB_PM_MSG:
break;
case SOF_IPC_GLB_COMP_MSG:
ipc_comp_notification(sdev, &hdr);
rx_callback = ipc_comp_notification;
break;
case SOF_IPC_GLB_STREAM_MSG:
/* need to pass msg id into the function */
ipc_stream_message(sdev, hdr.cmd);
rx_callback = ipc_stream_message;
break;
case SOF_IPC_GLB_TRACE_MSG:
ipc_trace_message(sdev, type);
rx_callback = ipc_trace_message;
break;
default:
dev_err(sdev->dev, "error: unknown DSP message 0x%x\n", cmd);
break;
}

if (rx_callback) {

@ranj063 ranj063 Jul 23, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why not return in the default case, then you can remove the init for rx_Callback and the check here as well no?

@ujfalusi ujfalusi Jul 23, 2021

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We might have a client who is interested in a new, unknown message? But then the de_err should be dev_info

/* read the full message as there we have rx handler for it */
full_msg = kmalloc(hdr.size, GFP_KERNEL);
if (!full_msg)
return;

err = snd_sof_ipc_msg_data(sdev, NULL, full_msg, hdr.size);
if (err < 0)
dev_err(sdev->dev, "failed to read message\n");
else
rx_callback(sdev, full_msg);

kfree(full_msg);
}

ipc_log_header(sdev->dev, "ipc rx done", hdr.cmd);
}
EXPORT_SYMBOL(snd_sof_ipc_msgs_rx);
Expand All @@ -485,19 +486,14 @@ EXPORT_SYMBOL(snd_sof_ipc_msgs_rx);
* IPC trace mechanism.
*/

static void ipc_trace_message(struct snd_sof_dev *sdev, u32 msg_type)
static void ipc_trace_message(struct snd_sof_dev *sdev, void *full_msg)
{
struct sof_ipc_dma_trace_posn posn;
int ret;
struct sof_ipc_cmd_hdr *hdr = full_msg;
u32 msg_type = hdr->cmd & SOF_CMD_TYPE_MASK;

switch (msg_type) {
case SOF_IPC_TRACE_DMA_POSITION:
/* read back full message */
ret = snd_sof_ipc_msg_data(sdev, NULL, &posn, sizeof(posn));
if (ret < 0)
dev_warn(sdev->dev, "failed to read trace position: %d\n", ret);
else
snd_sof_trace_update_pos(sdev, &posn);
snd_sof_trace_update_pos(sdev, full_msg);
break;
default:
dev_err(sdev->dev, "error: unhandled trace message %#x\n", msg_type);
Expand Down Expand Up @@ -576,11 +572,11 @@ static void ipc_xrun(struct snd_sof_dev *sdev, u32 msg_id)
}

/* stream notifications from DSP FW */
static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_cmd)
static void ipc_stream_message(struct snd_sof_dev *sdev, void *full_msg)
{
/* get msg cmd type and msd id */
u32 msg_type = msg_cmd & SOF_CMD_TYPE_MASK;
u32 msg_id = SOF_IPC_MESSAGE_ID(msg_cmd);
struct sof_ipc_cmd_hdr *hdr = full_msg;
u32 msg_type = hdr->cmd & SOF_CMD_TYPE_MASK;
u32 msg_id = SOF_IPC_MESSAGE_ID(hdr->cmd);

switch (msg_type) {
case SOF_IPC_STREAM_POSITION:
Expand Down