[Sound-open-firmware] [PATCH] ipc: mailbox: simplify mailbox usage.

Liam Girdwood liam.r.girdwood at linux.intel.com
Wed Sep 6 00:54:00 CEST 2017


There can be some confusion over mailbox naming wrt to in/out and up/down
naming with respect to DSP or host context.

Rename the mailboxes to imply either host initiated or DSP initiated IPC
so that it's obvious to driver and FW engineers which is the correct
mailbox to use.

This change also simplifies the IPC command replies by returning 1 if
the command function creates the reply message otherwise a generic
reply is used.

Signed-off-by: Liam Girdwood <liam.r.girdwood at linux.intel.com>
---
 src/include/reef/mailbox.h                       | 40 ++++++++++++------------
 src/include/uapi/ipc.h                           |  8 ++---
 src/ipc/byt-ipc.c                                | 29 +++++++++--------
 src/ipc/intel-ipc.c                              | 38 +++++++++++-----------
 src/platform/baytrail/include/platform/mailbox.h | 18 +++++------
 src/platform/baytrail/platform.c                 | 16 ++++++----
 6 files changed, 77 insertions(+), 72 deletions(-)

diff --git a/src/include/reef/mailbox.h b/src/include/reef/mailbox.h
index 9233587..c0580a9 100644
--- a/src/include/reef/mailbox.h
+++ b/src/include/reef/mailbox.h
@@ -44,17 +44,17 @@
 #define mailbox_get_exception_size() \
 	MAILBOX_EXCEPTION_SIZE
 
-#define mailbox_get_outbox_base() \
-	MAILBOX_OUTBOX_BASE
+#define mailbox_get_dspbox_base() \
+	MAILBOX_DSPBOX_BASE
 
-#define mailbox_get_outbox_size() \
-	MAILBOX_OUTBOX_SIZE
+#define mailbox_get_dspbox_size() \
+	MAILBOX_DSPBOX_SIZE
 
-#define mailbox_get_inbox_base() \
-	MAILBOX_INBOX_BASE
+#define mailbox_get_hostbox_base() \
+	MAILBOX_HOSTBOX_BASE
 
-#define mailbox_get_inbox_size() \
-	MAILBOX_INBOX_SIZE
+#define mailbox_get_hostbox_size() \
+	MAILBOX_HOSTBOX_SIZE
 
 #define mailbox_get_debug_base() \
 	MAILBOX_DEBUG_BASE
@@ -62,20 +62,20 @@
 #define mailbox_get_debug_size() \
 	MAILBOX_DEBUG_SIZE
 
-#define mailbox_outbox_write(dest, src, bytes) \
-	rmemcpy((void*)(MAILBOX_OUTBOX_BASE + dest), src, bytes); \
-	dcache_writeback_region((void*)(MAILBOX_OUTBOX_BASE + dest), bytes);
+#define mailbox_dspbox_write(dest, src, bytes) \
+	rmemcpy((void*)(MAILBOX_DSPBOX_BASE + dest), src, bytes); \
+	dcache_writeback_region((void*)(MAILBOX_DSPBOX_BASE + dest), bytes);
 
-#define mailbox_outbox_read(dest, src, bytes) \
-	dcache_invalidate_region((void*)(MAILBOX_OUTBOX_BASE + src), bytes); \
-	rmemcpy(dest, (void*)(MAILBOX_OUTBOX_BASE + src), bytes);
+#define mailbox_dspbox_read(dest, src, bytes) \
+	dcache_invalidate_region((void*)(MAILBOX_DSPBOX_BASE + src), bytes); \
+	rmemcpy(dest, (void*)(MAILBOX_DSPBOX_BASE + src), bytes);
 
-#define mailbox_inbox_write(dest, src, bytes) \
-	rmemcpy((void*)(MAILBOX_INBOX_BASE + dest), src, bytes); \
-	dcache_writeback_region((void*)(MAILBOX_INBOX_BASE + dest), bytes);
+#define mailbox_hostbox_write(dest, src, bytes) \
+	rmemcpy((void*)(MAILBOX_HOSTBOX_BASE + dest), src, bytes); \
+	dcache_writeback_region((void*)(MAILBOX_HOSTBOX_BASE + dest), bytes);
 
-#define mailbox_inbox_read(dest, src, bytes) \
-	dcache_invalidate_region((void*)(MAILBOX_INBOX_BASE + src), bytes); \
-	rmemcpy(dest, (void*)(MAILBOX_INBOX_BASE + src), bytes);
+#define mailbox_hostbox_read(dest, src, bytes) \
+	dcache_invalidate_region((void*)(MAILBOX_HOSTBOX_BASE + src), bytes); \
+	rmemcpy(dest, (void*)(MAILBOX_HOSTBOX_BASE + src), bytes);
 
 #endif
diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h
index c8cdedb..fb7e3b9 100644
--- a/src/include/uapi/ipc.h
+++ b/src/include/uapi/ipc.h
@@ -742,10 +742,10 @@ struct sof_ipc_fw_version {
 /* FW ready Message - sent by firmware when boot has completed */
 struct sof_ipc_fw_ready {
 	struct sof_ipc_hdr hdr;
-	uint32_t inbox_offset;
-	uint32_t outbox_offset;
-	uint32_t inbox_size;
-	uint32_t outbox_size;
+	uint32_t dspbox_offset;	 /* dsp initiated IPC mailbox */
+	uint32_t hostbox_offset; /* host initiated IPC mailbox */
+	uint32_t dspbox_size;
+	uint32_t hostbox_size;
 	struct sof_ipc_fw_version version;
 } __attribute__((packed));
 
diff --git a/src/ipc/byt-ipc.c b/src/ipc/byt-ipc.c
index 1483804..077fdde 100644
--- a/src/ipc/byt-ipc.c
+++ b/src/ipc/byt-ipc.c
@@ -68,7 +68,7 @@ static void do_notify(void)
 
 	/* copy the data returned from DSP */
 	if (msg->rx_size && msg->rx_size < SOF_IPC_MSG_MAX_SIZE)
-		mailbox_inbox_read(msg->rx_data, 0, msg->rx_size);
+		mailbox_dspbox_read(msg->rx_data, 0, msg->rx_size);
 
 	/* any callback ? */
 	if (msg->cb)
@@ -128,21 +128,24 @@ void ipc_platform_do_cmd(struct ipc *ipc)
 
 	tracev_ipc("Cmd");
 
-	/* clear old mailbox return values */
-	reply.hdr.cmd = SOF_IPC_GLB_REPLY;
-	reply.hdr.size = sizeof(reply);
-	reply.error = 0;
-	mailbox_outbox_write(0, &reply, sizeof(reply));
-
 	/* perform command and return any error */
 	err = ipc_cmd();
-	if (err < 0) {
-		/* read component values from the inbox */
+	if (err > 0) {
+		goto done; /* reply created and copied by cmd() */
+	} else if (err < 0) {
+		/* send std error reply */
 		reply.error = err;
-
-		/* write error back to outbox */
-		mailbox_outbox_write(0, &reply, sizeof(reply));
+	} else if (err == 0) {
+		/* send std reply */
+		reply.error = 0;
 	}
+
+	/* send std error/ok reply */
+	reply.hdr.cmd = SOF_IPC_GLB_REPLY;
+	reply.hdr.size = sizeof(reply);
+	mailbox_hostbox_write(0, &reply, sizeof(reply));
+
+done:
 	ipc->host_pending = 0;
 
 	/* clear BUSY bit and set DONE bit - accept new messages */
@@ -185,7 +188,7 @@ void ipc_platform_send_msg(struct ipc *ipc)
 
 	/* now send the message */
 	msg = list_first_item(&ipc->msg_list, struct ipc_msg, list);
-	mailbox_outbox_write(0, msg->tx_data, msg->tx_size);
+	mailbox_dspbox_write(0, msg->tx_data, msg->tx_size);
 	list_item_del(&msg->list);
 	ipc->dsp_msg = msg;
 	tracev_ipc("Msg");
diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c
index 4f397c1..0208ff6 100644
--- a/src/ipc/intel-ipc.c
+++ b/src/ipc/intel-ipc.c
@@ -69,7 +69,7 @@ static inline struct sof_ipc_hdr *mailbox_validate(void)
 	struct sof_ipc_hdr *hdr = _ipc->comp_data;
 
 	/* read component values from the inbox */
-	mailbox_inbox_read(hdr, 0, sizeof(*hdr));
+	mailbox_hostbox_read(hdr, 0, sizeof(*hdr));
 
 	/* validate component header */
 	if (hdr->size > SOF_IPC_MSG_MAX_SIZE) {
@@ -78,7 +78,7 @@ static inline struct sof_ipc_hdr *mailbox_validate(void)
 	}
 
 	/* read rest of component data */
-	mailbox_inbox_read(hdr + 1, sizeof(*hdr), hdr->size - sizeof(*hdr));
+	mailbox_hostbox_read(hdr + 1, sizeof(*hdr), hdr->size - sizeof(*hdr));
 	return hdr;
 }
 
@@ -262,8 +262,8 @@ static int ipc_stream_pcm_params(uint32_t stream)
 	reply.rhdr.error = 0;
 	reply.comp_id = pcm_params->comp_id;
 	reply.posn_offset = 0; /* TODO: set this up for mmaped components */
-	mailbox_outbox_write(0, &reply, sizeof(reply));
-	return 0;
+	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	return 1;
 
 error:
 	err = pipeline_reset(pcm_dev->cd->pipeline, pcm_dev->cd);
@@ -318,8 +318,8 @@ static int ipc_stream_position(uint32_t header)
 	pipeline_get_timestamp(pcm_dev->cd->pipeline, pcm_dev->cd, &posn);
 
 	/* copy positions to outbox */
-	mailbox_outbox_write(0, &posn, sizeof(posn));
-	return 0;
+	mailbox_hostbox_write(0, &posn, sizeof(posn));
+	return 1;
 }
 
 /* send stream position */
@@ -484,9 +484,8 @@ static int ipc_pm_context_size(uint32_t header)
 	/* TODO: calculate the context and size of host buffers required */
 
 	/* write the context to the host driver */
-	mailbox_outbox_write(0, &pm_ctx, sizeof(pm_ctx));
-
-	return 0;
+	mailbox_hostbox_write(0, &pm_ctx, sizeof(pm_ctx));
+	return 1;
 }
 
 static int ipc_pm_context_save(uint32_t header)
@@ -518,11 +517,11 @@ static int ipc_pm_context_save(uint32_t header)
 	//reply.entries_no = 0;
 
 	/* write the context to the host driver */
-	mailbox_outbox_write(0, pm_ctx, sizeof(*pm_ctx));
+	mailbox_hostbox_write(0, pm_ctx, sizeof(*pm_ctx));
 
 	//iipc->pm_prepare_D3 = 1;
 
-	return 0;
+	return 1;
 }
 
 static int ipc_pm_context_restore(uint32_t header)
@@ -583,9 +582,8 @@ static int ipc_comp_value(uint32_t header, uint32_t cmd)
 	}
 
 	/* write component values to the outbox */
-	mailbox_outbox_write(0, data, data->hdr.size);
-
-	return 0;
+	mailbox_hostbox_write(0, data, data->hdr.size);
+	return 1;
 }
 
 static int ipc_glb_comp_message(uint32_t header)
@@ -628,8 +626,8 @@ static int ipc_glb_tplg_comp_new(uint32_t header)
 	reply.rhdr.hdr.cmd = header;
 	reply.rhdr.error = 0;
 	reply.offset = 0; /* TODO: set this up for mmaped components */
-	mailbox_outbox_write(0, &reply, sizeof(reply));
-	return 0;
+	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	return 1;
 }
 
 static int ipc_glb_tplg_buffer_new(uint32_t header)
@@ -651,8 +649,8 @@ static int ipc_glb_tplg_buffer_new(uint32_t header)
 	reply.rhdr.hdr.cmd = header;
 	reply.rhdr.error = 0;
 	reply.offset = 0; /* TODO: set this up for mmaped components */
-	mailbox_outbox_write(0, &reply, sizeof(reply));
-	return 0;
+	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	return 1;
 }
 
 static int ipc_glb_tplg_pipe_new(uint32_t header)
@@ -674,8 +672,8 @@ static int ipc_glb_tplg_pipe_new(uint32_t header)
 	reply.rhdr.hdr.cmd = header;
 	reply.rhdr.error = 0;
 	reply.offset = 0; /* TODO: set this up for mmaped components */
-	mailbox_outbox_write(0, &reply, sizeof(reply));
-	return 0;
+	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	return 1;
 }
 
 static int ipc_glb_tplg_pipe_complete(uint32_t header)
diff --git a/src/platform/baytrail/include/platform/mailbox.h b/src/platform/baytrail/include/platform/mailbox.h
index 7ef8545..b03887e 100644
--- a/src/platform/baytrail/include/platform/mailbox.h
+++ b/src/platform/baytrail/include/platform/mailbox.h
@@ -35,18 +35,18 @@
 
 #define MAILBOX_HOST_OFFSET	0x144000
 
-#define MAILBOX_OUTBOX_OFFSET	0x0
-#define MAILBOX_OUTBOX_SIZE	0x400
-#define MAILBOX_OUTBOX_BASE \
-	(MAILBOX_BASE + MAILBOX_OUTBOX_OFFSET)
+#define MAILBOX_DSPBOX_OFFSET	0x0
+#define MAILBOX_DSPBOX_SIZE	0x400
+#define MAILBOX_DSPBOX_BASE \
+	(MAILBOX_BASE + MAILBOX_DSPBOX_OFFSET)
 
-#define MAILBOX_INBOX_OFFSET	MAILBOX_OUTBOX_SIZE
-#define MAILBOX_INBOX_SIZE	0x400
-#define MAILBOX_INBOX_BASE \
-	(MAILBOX_BASE + MAILBOX_INBOX_OFFSET)
+#define MAILBOX_HOSTBOX_OFFSET	MAILBOX_DSPBOX_SIZE
+#define MAILBOX_HOSTBOX_SIZE	0x400
+#define MAILBOX_HOSTBOX_BASE \
+	(MAILBOX_BASE + MAILBOX_HOSTBOX_OFFSET)
 
 #define MAILBOX_EXCEPTION_OFFSET \
-	(MAILBOX_INBOX_SIZE + MAILBOX_OUTBOX_SIZE)
+	(MAILBOX_HOSTBOX_SIZE + MAILBOX_DSPBOX_SIZE)
 #define MAILBOX_EXCEPTION_SIZE	0x100
 #define MAILBOX_EXCEPTION_BASE \
 	(MAILBOX_BASE + MAILBOX_EXCEPTION_OFFSET)
diff --git a/src/platform/baytrail/platform.c b/src/platform/baytrail/platform.c
index ab111c9..b8c3e49 100644
--- a/src/platform/baytrail/platform.c
+++ b/src/platform/baytrail/platform.c
@@ -51,11 +51,15 @@
 #include <version.h>
 
 static const struct sof_ipc_fw_ready ready = {
-	/* for host, we need exchange the naming of inxxx and outxxx */
-	.inbox_offset = MAILBOX_HOST_OFFSET + MAILBOX_OUTBOX_OFFSET,
-	.outbox_offset = MAILBOX_HOST_OFFSET + MAILBOX_INBOX_OFFSET,
-	.inbox_size = MAILBOX_OUTBOX_SIZE,
-	.outbox_size = MAILBOX_INBOX_SIZE,
+	.hdr = {
+		.cmd = SOF_IPC_FW_READY,
+		.size = sizeof(struct sof_ipc_fw_ready),
+	},
+	/* dspbox is for DSP initiated IPC, hostbox is for host iniiated IPC */
+	.dspbox_offset = MAILBOX_HOST_OFFSET + MAILBOX_DSPBOX_OFFSET,
+	.hostbox_offset = MAILBOX_HOST_OFFSET + MAILBOX_HOSTBOX_OFFSET,
+	.dspbox_size = MAILBOX_DSPBOX_SIZE,
+	.hostbox_size = MAILBOX_HOSTBOX_SIZE,
 	.version = {
 		.build = REEF_BUILD,
 		.minor = REEF_MINOR,
@@ -83,7 +87,7 @@ int platform_boot_complete(uint32_t boot_message)
 {
 	uint64_t outbox = MAILBOX_HOST_OFFSET >> 3;
 
-	mailbox_outbox_write(0, &ready, sizeof(ready));
+	mailbox_dspbox_write(0, &ready, sizeof(ready));
 
 	/* now interrupt host to tell it we are done booting */
 	shim_write(SHIM_IPCDL, SOF_IPC_FW_READY | outbox);
-- 
2.11.0



More information about the Sound-open-firmware mailing list