[PATCH 0/3] SoC: SOF: ipc: Optimizations for tx message
Hi,
The series will drop the internal use of 'header' parameter which is always set to hdr->cmd.
The other simplification is to use the provided message directly as it is guarantied to be valid throughout the message sending and we can save memory by not allocating a temporary buffer, also saving on needles memcpy() operations.
Regards, Peter --- Peter Ujfalusi (3): ASoC: SOF: Intel: cnl: Use pm_gate->hdr.cmd in cnl_compact_ipc_compress() ASoC: SOF: ipc: Drop header parameter from sof_ipc_tx_message_unlocked() ASoC: SOF: ipc: Do not allocate buffer for msg_data
sound/soc/sof/intel/cnl.c | 6 ++---- sound/soc/sof/ipc.c | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 16 deletions(-)
Instead of first checking the msg->header (which is the hdr.cmd), use directly the cmd from the message itself.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/cnl.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index e615125d575e..1911e104f113 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -161,11 +161,9 @@ static void cnl_ipc_dsp_done(struct snd_sof_dev *sdev) static bool cnl_compact_ipc_compress(struct snd_sof_ipc_msg *msg, u32 *dr, u32 *dd) { - struct sof_ipc_pm_gate *pm_gate; - - if (msg->header == (SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_GATE)) { - pm_gate = msg->msg_data; + struct sof_ipc_pm_gate *pm_gate = msg->msg_data;
+ if (pm_gate->hdr.cmd == (SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_GATE)) { /* send the compact message via the primary register */ *dr = HDA_IPC_MSG_COMPACT | HDA_IPC_PM_GATE;
The snd_sof_ipc_msg.header is not used by platform code, there is no need to update it and the 'header' parameter for sof_ipc_tx_message_unlocked() can be dropped at the same time.
Instead of using the header parameter passed by the caller (which does by setting it to the hdr->cmd) use the hdr->cmd directly when logging.
At the same time make sure that there is a message passed to the tx_message function. All instances of the tx_message passes an IPC message, this check is placed to make sure the future users can not introduce bugs.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/ipc.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 5bcf906d90af..ec51daed8b31 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -294,14 +294,20 @@ static int tx_wait_done(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg, }
/* send IPC message from host to DSP */ -static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header, +static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, void *msg_data, size_t msg_bytes, void *reply_data, size_t reply_bytes) { + struct sof_ipc_cmd_hdr *hdr = msg_data; struct snd_sof_dev *sdev = ipc->sdev; struct snd_sof_ipc_msg *msg; int ret;
+ if (!msg_data || msg_bytes < sizeof(*hdr)) { + dev_err_ratelimited(sdev->dev, "No IPC message to send\n"); + return -EINVAL; + } + if (ipc->disable_ipc_tx || sdev->fw_state != SOF_FW_BOOT_COMPLETE) return -ENODEV;
@@ -314,15 +320,13 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header, /* initialise the message */ msg = &ipc->msg;
- msg->header = header; + /* attach message data */ + memcpy(msg->msg_data, msg_data, msg_bytes); msg->msg_size = msg_bytes; + msg->reply_size = reply_bytes; msg->reply_error = 0;
- /* attach any data */ - if (msg_bytes) - memcpy(msg->msg_data, msg_data, msg_bytes); - sdev->msg = msg;
ret = snd_sof_dsp_send_msg(sdev, msg); @@ -339,7 +343,7 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header, return ret; }
- ipc_log_header(sdev->dev, "ipc tx", msg->header); + ipc_log_header(sdev->dev, "ipc tx", hdr->cmd);
/* now wait for completion */ return tx_wait_done(ipc, msg, reply_data); @@ -385,7 +389,7 @@ int sof_ipc_tx_message_no_pm(struct snd_sof_ipc *ipc, u32 header, /* Serialise IPC TX */ mutex_lock(&ipc->tx_mutex);
- ret = sof_ipc_tx_message_unlocked(ipc, header, msg_data, msg_bytes, + ret = sof_ipc_tx_message_unlocked(ipc, msg_data, msg_bytes, reply_data, reply_bytes);
mutex_unlock(&ipc->tx_mutex); @@ -789,7 +793,6 @@ static int sof_set_get_large_ctrl_data(struct snd_sof_dev *sdev, memcpy(sparams->dst, sparams->src + offset, send_bytes);
err = sof_ipc_tx_message_unlocked(sdev->ipc, - partdata->rhdr.hdr.cmd, partdata, partdata->rhdr.hdr.size, partdata,
The sof_ipc_tx_message does not have support for async operations. There is no need to allocate a buffer and copy each message to it to be sent to the DSP, we can use the passed message data pointer directly.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/ipc.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index ec51daed8b31..16a0d7a059f3 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -321,7 +321,7 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, msg = &ipc->msg;
/* attach message data */ - memcpy(msg->msg_data, msg_data, msg_bytes); + msg->msg_data = msg_data; msg->msg_size = msg_bytes;
msg->reply_size = reply_bytes; @@ -1003,9 +1003,6 @@ int sof_ipc_init_msg_memory(struct snd_sof_dev *sdev) struct snd_sof_ipc_msg *msg;
msg = &sdev->ipc->msg; - msg->msg_data = devm_kzalloc(sdev->dev, SOF_IPC_MSG_MAX_SIZE, GFP_KERNEL); - if (!msg->msg_data) - return -ENOMEM;
msg->reply_data = devm_kzalloc(sdev->dev, SOF_IPC_MSG_MAX_SIZE, GFP_KERNEL); if (!msg->reply_data)
On Fri, 28 Jan 2022 15:36:17 +0200, Peter Ujfalusi wrote:
The series will drop the internal use of 'header' parameter which is always set to hdr->cmd.
The other simplification is to use the provided message directly as it is guarantied to be valid throughout the message sending and we can save memory by not allocating a temporary buffer, also saving on needles memcpy() operations.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/3] ASoC: SOF: Intel: cnl: Use pm_gate->hdr.cmd in cnl_compact_ipc_compress() commit: 5b6988fe844a298263821beef5fcc41286a048dc [2/3] ASoC: SOF: ipc: Drop header parameter from sof_ipc_tx_message_unlocked() commit: 73a548bd1fa3cbe5d18026230a34c1f058257536 [3/3] ASoC: SOF: ipc: Do not allocate buffer for msg_data commit: 2acfab7101140e93928a61ca48d7e442aa538dd7
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown -
Peter Ujfalusi