[PATCH 0/3] SoC: SOF: ipc: Optimizations for tx message

[PATCH 0/3] SoC: SOF: ipc: Optimizations for tx message

[Peter Ujfalusi]

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(-)

-- 
2.35.0

[PATCH 1/3] ASoC: SOF: Intel: cnl: Use pm_gate->hdr.cmd in cnl_compact_ipc_compress()

[Peter Ujfalusi]

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;
 
-- 
2.35.0

[PATCH 2/3] ASoC: SOF: ipc: Drop header parameter from sof_ipc_tx_message_unlocked()

[Peter Ujfalusi]

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,
-- 
2.35.0

[PATCH 3/3] ASoC: SOF: ipc: Do not allocate buffer for msg_data

[Peter Ujfalusi]

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)
-- 
2.35.0

Re: [PATCH 0/3] SoC: SOF: ipc: Optimizations for tx message

[Mark Brown]

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