Skip to contentSkip to conversation

ASoC: Intel: IPC/mailbox code refactoring

PATCH Applied Jin Yao 3 participants 8 messages Apr 7, 2015 – Apr 10, 2015

Subsystems (3)
lore
Share & tools
lore

Jin Yao

author
#0 lore raw

cover ASoC: Intel: IPC/mailbox code refactoring

Hi Mark,

Currently in Intel SST driver, some very similar or duplicated IPC/mailbox processing code are used in different platforms.

This is a patch series which refactors the common IPC/mailbox code into generic APIs and use the new APIs in Baytrail and Broadwell platforms.

This patch series depends on Jie Yang's following patches for build:
ASoC: Intel: create atom folder and move atom platform files in
ASoC: Intel: create baytrail folder and move baytrail platform files in
ASoC: Intel: create boards folder and move sst boards files in
ASoC: Intel: create haswell folder and move haswell platform files in
ASoC: Intel: create common folder and move common files in

Since now Jie Yang's patch series has been applied in intel branch so this patch series will be no build issue.

Thanks
Jin Yao

Jin Yao (3):
ASoC: Intel: Refactor common IPC/mailbox code into generic APIs
ASoC: Intel: Use the generic IPC/mailbox APIs in Baytrail
ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

 sound/soc/intel/baytrail/sst-baytrail-ipc.c | 360 ++++++--------------------
 sound/soc/intel/common/Makefile             |   3 +-
 sound/soc/intel/common/sst-ipc.c            | 294 +++++++++++++++++++++
 sound/soc/intel/common/sst-ipc.h            |  91 +++++++
 sound/soc/intel/haswell/sst-haswell-ipc.c   | 382 +++++++---------------------
 5 files changed, 551 insertions(+), 579 deletions(-)
 create mode 100644 sound/soc/intel/common/sst-ipc.c
 create mode 100644 sound/soc/intel/common/sst-ipc.h
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/3 ASoC: Intel: Refactor common IPC/mailbox code into generic APIs

Currently in Intel SST driver, some similar IPC/mailbox processing code are used in different platforms (e.g. in baytrail/broadwell).

This patch extracts the common code and creates new files (sst-ipc.c/sst-ipc.h) to contain the common code and provide the generic APIs for IPC/mailbox processing.

---
 sound/soc/intel/common/Makefile  |   3 +-
 sound/soc/intel/common/sst-ipc.c | 294 +++++++++++++++++++++++++++++++++++++++
 sound/soc/intel/common/sst-ipc.h |  91 ++++++++++++
 3 files changed, 387 insertions(+), 1 deletion(-)
 create mode 100644 sound/soc/intel/common/sst-ipc.c
 create mode 100644 sound/soc/intel/common/sst-ipc.h
sound/soc/intel/common/Makefile +2 -1
index 3df0e1c..f24154c 100644
@@ -1,6 +1,7 @@
 snd-soc-sst-dsp-objs := sst-dsp.o sst-firmware.o
 snd-soc-sst-acpi-objs := sst-acpi.o
+snd-soc-sst-ipc-objs := sst-ipc.o
 
-obj-$(CONFIG_SND_SOC_INTEL_SST) += snd-soc-sst-dsp.o
+obj-$(CONFIG_SND_SOC_INTEL_SST) += snd-soc-sst-dsp.o snd-soc-sst-ipc.o
 obj-$(CONFIG_SND_SOC_INTEL_SST_ACPI) += snd-soc-sst-acpi.o
 
sound/soc/intel/common/sst-ipc.c new file +294 -0

294 lines not on this page · raw message

sound/soc/intel/common/sst-ipc.h new file +91 -0

91 lines not on this page · raw message

Signature · 1 line
1.9.1
author
#2 lore raw

PATCH 2/3 ASoC: Intel: Use the generic IPC/mailbox APIs in Baytrail

In reply to Jin Yao

Use the generic IPC/mailbox APIs to replace the original processing code for Baytrail platform.

---
 sound/soc/intel/baytrail/sst-baytrail-ipc.c | 360 ++++++----------------------
 1 file changed, 77 insertions(+), 283 deletions(-)
sound/soc/intel/baytrail/sst-baytrail-ipc.c +77 -283
index aabb9b0..1efb33b 100644
@@ -31,6 +31,7 @@
 #include "sst-baytrail-ipc.h"
 #include "../common/sst-dsp.h"
 #include "../common/sst-dsp-priv.h"
+#include "../common/sst-ipc.h"
 
 /* IPC message timeout */
 #define IPC_TIMEOUT_MSECS	300
@@ -142,23 +143,6 @@ struct sst_byt_fw_init {
 	u8 debug_info;
 } __packed;
 
-/* driver internal IPC message structure */
-struct ipc_message {
-	struct list_head list;
-	u64 header;
-
-	/* direction wrt host CPU */
-	char tx_data[SST_BYT_IPC_MAX_PAYLOAD_SIZE];
-	size_t tx_size;
-	char rx_data[SST_BYT_IPC_MAX_PAYLOAD_SIZE];
-	size_t rx_size;
-
-	wait_queue_head_t waitq;
-	bool complete;
-	bool wait;
-	int errno;
-};
-
 struct sst_byt_stream;
 struct sst_byt;
 
@@ -195,14 +179,7 @@ struct sst_byt {
 	struct sst_fw *fw;
 
 	/* IPC messaging */
-	struct list_head tx_list;
-	struct list_head rx_list;
-	struct list_head empty_list;
-	wait_queue_head_t wait_txq;
-	struct task_struct *tx_thread;
-	struct kthread_worker kworker;
-	struct kthread_work kwork;
-	struct ipc_message *msg;
+	struct sst_generic_ipc ipc;
 };
 
 static inline u64 sst_byt_header(int msg_id, int data, bool large, int str_id)
@@ -246,209 +223,6 @@ static struct sst_byt_stream *sst_byt_get_stream(struct sst_byt *byt,
 	return NULL;
 }
 
-static void sst_byt_ipc_shim_dbg(struct sst_byt *byt, const char *text)
-{
-	struct sst_dsp *sst = byt->dsp;
-	u64 isr, ipcd, imrx, ipcx;
-
-	ipcx = sst_dsp_shim_read64_unlocked(sst, SST_IPCX);
-	isr = sst_dsp_shim_read64_unlocked(sst, SST_ISRX);
-	ipcd = sst_dsp_shim_read64_unlocked(sst, SST_IPCD);
-	imrx = sst_dsp_shim_read64_unlocked(sst, SST_IMRX);
-
-	dev_err(byt->dev,
-		"ipc: --%s-- ipcx 0x%llx isr 0x%llx ipcd 0x%llx imrx 0x%llx\n",
-		text, ipcx, isr, ipcd, imrx);
-}
-
-/* locks held by caller */
-static struct ipc_message *sst_byt_msg_get_empty(struct sst_byt *byt)
-{
-	struct ipc_message *msg = NULL;
-
-	if (!list_empty(&byt->empty_list)) {
-		msg = list_first_entry(&byt->empty_list,
-				       struct ipc_message, list);
-		list_del(&msg->list);
-	}
-
-	return msg;
-}
-
-static void sst_byt_ipc_tx_msgs(struct kthread_work *work)
-{
-	struct sst_byt *byt =
-		container_of(work, struct sst_byt, kwork);
-	struct ipc_message *msg;
-	u64 ipcx;
-	unsigned long flags;
-
-	spin_lock_irqsave(&byt->dsp->spinlock, flags);
-	if (list_empty(&byt->tx_list)) {
-		spin_unlock_irqrestore(&byt->dsp->spinlock, flags);
-		return;
-	}
-
-	/* if the DSP is busy we will TX messages after IRQ */
-	ipcx = sst_dsp_shim_read64_unlocked(byt->dsp, SST_IPCX);
-	if (ipcx & SST_BYT_IPCX_BUSY) {
-		spin_unlock_irqrestore(&byt->dsp->spinlock, flags);
-		return;
-	}
-
-	msg = list_first_entry(&byt->tx_list, struct ipc_message, list);
-
-	list_move(&msg->list, &byt->rx_list);
-
-	/* send the message */
-	if (msg->header & IPC_HEADER_LARGE(true))
-		sst_dsp_outbox_write(byt->dsp, msg->tx_data, msg->tx_size);
-	sst_dsp_shim_write64_unlocked(byt->dsp, SST_IPCX, msg->header);
-
-	spin_unlock_irqrestore(&byt->dsp->spinlock, flags);
-}
-
-static inline void sst_byt_tx_msg_reply_complete(struct sst_byt *byt,
-						 struct ipc_message *msg)
-{
-	msg->complete = true;
-
-	if (!msg->wait)
-		list_add_tail(&msg->list, &byt->empty_list);
-	else
-		wake_up(&msg->waitq);
-}
-
-static void sst_byt_drop_all(struct sst_byt *byt)
-{
-	struct ipc_message *msg, *tmp;
-	unsigned long flags;
-
-	/* drop all TX and Rx messages before we stall + reset DSP */
-	spin_lock_irqsave(&byt->dsp->spinlock, flags);
-	list_for_each_entry_safe(msg, tmp, &byt->tx_list, list) {
-		list_move(&msg->list, &byt->empty_list);
-	}
-
-	list_for_each_entry_safe(msg, tmp, &byt->rx_list, list) {
-		list_move(&msg->list, &byt->empty_list);
-	}
-
-	spin_unlock_irqrestore(&byt->dsp->spinlock, flags);
-}
-
-static int sst_byt_tx_wait_done(struct sst_byt *byt, struct ipc_message *msg,
-				void *rx_data)
-{
-	unsigned long flags;
-	int ret;
-
-	/* wait for DSP completion */
-	ret = wait_event_timeout(msg->waitq, msg->complete,
-				 msecs_to_jiffies(IPC_TIMEOUT_MSECS));
-
-	spin_lock_irqsave(&byt->dsp->spinlock, flags);
-	if (ret == 0) {
-		list_del(&msg->list);
-		sst_byt_ipc_shim_dbg(byt, "message timeout");
-
-		ret = -ETIMEDOUT;
-	} else {
-
-		/* copy the data returned from DSP */
-		if (msg->rx_size)
-			memcpy(rx_data, msg->rx_data, msg->rx_size);
-		ret = msg->errno;
-	}
-
-	list_add_tail(&msg->list, &byt->empty_list);
-	spin_unlock_irqrestore(&byt->dsp->spinlock, flags);
-	return ret;
-}
-
-static int sst_byt_ipc_tx_message(struct sst_byt *byt, u64 header,
-				  void *tx_data, size_t tx_bytes,
-				  void *rx_data, size_t rx_bytes, int wait)
-{
-	unsigned long flags;
-	struct ipc_message *msg;
-
-	spin_lock_irqsave(&byt->dsp->spinlock, flags);
-
-	msg = sst_byt_msg_get_empty(byt);
-	if (msg == NULL) {
-		spin_unlock_irqrestore(&byt->dsp->spinlock, flags);
-		return -EBUSY;
-	}
-
-	msg->header = header;
-	msg->tx_size = tx_bytes;
-	msg->rx_size = rx_bytes;
-	msg->wait = wait;
-	msg->errno = 0;
-	msg->complete = false;
-
-	if (tx_bytes) {
-		/* msg content = lower 32-bit of the header + data */
-		*(u32 *)msg->tx_data = (u32)(header & (u32)-1);
-		memcpy(msg->tx_data + sizeof(u32), tx_data, tx_bytes);
-		msg->tx_size += sizeof(u32);
-	}
-
-	list_add_tail(&msg->list, &byt->tx_list);
-	spin_unlock_irqrestore(&byt->dsp->spinlock, flags);
-
-	queue_kthread_work(&byt->kworker, &byt->kwork);
-
-	if (wait)
-		return sst_byt_tx_wait_done(byt, msg, rx_data);
-	else
-		return 0;
-}
-
-static inline int sst_byt_ipc_tx_msg_wait(struct sst_byt *byt, u64 header,
-					  void *tx_data, size_t tx_bytes,
-					  void *rx_data, size_t rx_bytes)
-{
-	return sst_byt_ipc_tx_message(byt, header, tx_data, tx_bytes,
-				      rx_data, rx_bytes, 1);
-}
-
-static inline int sst_byt_ipc_tx_msg_nowait(struct sst_byt *byt, u64 header,
-						void *tx_data, size_t tx_bytes)
-{
-	return sst_byt_ipc_tx_message(byt, header, tx_data, tx_bytes,
-				      NULL, 0, 0);
-}
-
-static struct ipc_message *sst_byt_reply_find_msg(struct sst_byt *byt,
-						  u64 header)
-{
-	struct ipc_message *msg = NULL, *_msg;
-	u64 mask;
-
-	/* match reply to message sent based on msg and stream IDs */
-	mask = IPC_HEADER_MSG_ID_MASK |
-	       IPC_HEADER_STR_ID_MASK << IPC_HEADER_STR_ID_SHIFT;
-	header &= mask;
-
-	if (list_empty(&byt->rx_list)) {
-		dev_err(byt->dev,
-			"ipc: rx list is empty but received 0x%llx\n", header);
-		goto out;
-	}
-
-	list_for_each_entry(_msg, &byt->rx_list, list) {
-		if ((_msg->header & mask) == header) {
-			msg = _msg;
-			break;
-		}
-	}
-
-out:
-	return msg;
-}
-
 static void sst_byt_stream_update(struct sst_byt *byt, struct ipc_message *msg)
 {
 	struct sst_byt_stream *stream;
@@ -477,7 +251,7 @@ static int sst_byt_process_reply(struct sst_byt *byt, u64 header)
 {
 	struct ipc_message *msg;
 
-	msg = sst_byt_reply_find_msg(byt, header);
+	msg = sst_ipc_reply_find_msg(&byt->ipc, header);
 	if (msg == NULL)
 		return 1;
 
@@ -491,7 +265,7 @@ static int sst_byt_process_reply(struct sst_byt *byt, u64 header)
 
 	list_del(&msg->list);
 	/* wake up */
-	sst_byt_tx_msg_reply_complete(byt, msg);
+	sst_ipc_tx_msg_reply_complete(&byt->ipc, msg);
 
 	return 1;
 }
@@ -538,6 +312,7 @@ static irqreturn_t sst_byt_irq_thread(int irq, void *context)
 {
 	struct sst_dsp *sst = (struct sst_dsp *) context;
 	struct sst_byt *byt = sst_dsp_get_thread_context(sst);
+	struct sst_generic_ipc *ipc = &byt->ipc;
 	u64 header;
 	unsigned long flags;
 
@@ -569,7 +344,7 @@ static irqreturn_t sst_byt_irq_thread(int irq, void *context)
 	spin_unlock_irqrestore(&sst->spinlock, flags);
 
 	/* continue to send any remaining messages... */
-	queue_kthread_work(&byt->kworker, &byt->kwork);
+	queue_kthread_work(&ipc->kworker, &ipc->kwork);
 
 	return IRQ_HANDLED;
 }
@@ -656,7 +431,8 @@ int sst_byt_stream_commit(struct sst_byt *byt, struct sst_byt_stream *stream)
 	header = sst_byt_header(IPC_IA_ALLOC_STREAM,
 				sizeof(*str_req) + sizeof(u32),
 				true, stream->str_id);
-	ret = sst_byt_ipc_tx_msg_wait(byt, header, str_req, sizeof(*str_req),
+	ret = sst_ipc_tx_message_wait(&byt->ipc, header, str_req,
+				      sizeof(*str_req),
 				      reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(byt->dev, "ipc: error stream commit failed\n");
@@ -679,7 +455,7 @@ int sst_byt_stream_free(struct sst_byt *byt, struct sst_byt_stream *stream)
 		goto out;
 
 	header = sst_byt_header(IPC_IA_FREE_STREAM, 0, false, stream->str_id);
-	ret = sst_byt_ipc_tx_msg_wait(byt, header, NULL, 0, NULL, 0);
+	ret = sst_ipc_tx_message_wait(&byt->ipc, header, NULL, 0, NULL, 0);
 	if (ret < 0) {
 		dev_err(byt->dev, "ipc: free stream %d failed\n",
 			stream->str_id);
@@ -703,9 +479,11 @@ static int sst_byt_stream_operations(struct sst_byt *byt, int type,
 
 	header = sst_byt_header(type, 0, false, stream_id);
 	if (wait)
-		return sst_byt_ipc_tx_msg_wait(byt, header, NULL, 0, NULL, 0);
+		return sst_ipc_tx_message_wait(&byt->ipc, header, NULL,
+						0, NULL, 0);
 	else
-		return sst_byt_ipc_tx_msg_nowait(byt, header, NULL, 0);
+		return sst_ipc_tx_message_nowait(&byt->ipc, header,
+						NULL, 0);
 }
 
 /* stream ALSA trigger operations */
@@ -725,7 +503,7 @@ int sst_byt_stream_start(struct sst_byt *byt, struct sst_byt_stream *stream,
 	tx_msg = &start_stream;
 	size = sizeof(start_stream);
 
-	ret = sst_byt_ipc_tx_msg_nowait(byt, header, tx_msg, size);
+	ret = sst_ipc_tx_message_nowait(&byt->ipc, header, tx_msg, size);
 	if (ret < 0)
 		dev_err(byt->dev, "ipc: error failed to start stream %d\n",
 			stream->str_id);
@@ -790,23 +568,6 @@ int sst_byt_get_dsp_position(struct sst_byt *byt,
 	return do_div(fw_tstamp.ring_buffer_counter, buffer_size);
 }
 
-static int msg_empty_list_init(struct sst_byt *byt)
-{
-	struct ipc_message *msg;
-	int i;
-
-	byt->msg = kzalloc(sizeof(*msg) * IPC_EMPTY_LIST_SIZE, GFP_KERNEL);
-	if (byt->msg == NULL)
-		return -ENOMEM;
-
-	for (i = 0; i < IPC_EMPTY_LIST_SIZE; i++) {
-		init_waitqueue_head(&byt->msg[i].waitq);
-		list_add(&byt->msg[i].list, &byt->empty_list);
-	}
-
-	return 0;
-}
-
 struct sst_dsp *sst_byt_get_dsp(struct sst_byt *byt)
 {
 	return byt->dsp;
@@ -823,7 +584,7 @@ int sst_byt_dsp_suspend_late(struct device *dev, struct sst_pdata *pdata)
 
 	dev_dbg(byt->dev, "dsp reset\n");
 	sst_dsp_reset(byt->dsp);
-	sst_byt_drop_all(byt);
+	sst_ipc_drop_all(&byt->ipc);
 	dev_dbg(byt->dev, "dsp in reset\n");
 
 	dev_dbg(byt->dev, "free all blocks and unload fw\n");
@@ -876,9 +637,52 @@ int sst_byt_dsp_wait_for_ready(struct device *dev, struct sst_pdata *pdata)
 }
 EXPORT_SYMBOL_GPL(sst_byt_dsp_wait_for_ready);
 
+static void byt_tx_msg(struct sst_generic_ipc *ipc, struct ipc_message *msg)
+{
+	if (msg->header & IPC_HEADER_LARGE(true))
+		sst_dsp_outbox_write(ipc->dsp, msg->tx_data, msg->tx_size);
+
+	sst_dsp_shim_write64_unlocked(ipc->dsp, SST_IPCX, msg->header);
+}
+
+static void byt_shim_dbg(struct sst_generic_ipc *ipc, const char *text)
+{
+	struct sst_dsp *sst = ipc->dsp;
+	u64 isr, ipcd, imrx, ipcx;
+
+	ipcx = sst_dsp_shim_read64_unlocked(sst, SST_IPCX);
+	isr = sst_dsp_shim_read64_unlocked(sst, SST_ISRX);
+	ipcd = sst_dsp_shim_read64_unlocked(sst, SST_IPCD);
+	imrx = sst_dsp_shim_read64_unlocked(sst, SST_IMRX);
+
+	dev_err(ipc->dev,
+		"ipc: --%s-- ipcx 0x%llx isr 0x%llx ipcd 0x%llx imrx 0x%llx\n",
+		text, ipcx, isr, ipcd, imrx);
+}
+
+static void byt_tx_data_copy(struct ipc_message *msg, char *tx_data,
+	size_t tx_size)
+{
+	/* msg content = lower 32-bit of the header + data */
+	*(u32 *)msg->tx_data = (u32)(msg->header & (u32)-1);
+	memcpy(msg->tx_data + sizeof(u32), tx_data, tx_size);
+	msg->tx_size += sizeof(u32);
+}
+
+static u64 byt_reply_msg_match(u64 header, u64 *mask)
+{
+	/* match reply to message sent based on msg and stream IDs */
+	*mask = IPC_HEADER_MSG_ID_MASK |
+	       IPC_HEADER_STR_ID_MASK << IPC_HEADER_STR_ID_SHIFT;
+	header &= *mask;
+
+	return header;
+}
+
 int sst_byt_dsp_init(struct device *dev, struct sst_pdata *pdata)
 {
 	struct sst_byt *byt;
+	struct sst_generic_ipc *ipc;
 	struct sst_fw *byt_sst_fw;
 	struct sst_byt_fw_init init;
 	int err;
@@ -889,39 +693,30 @@ int sst_byt_dsp_init(struct device *dev, struct sst_pdata *pdata)
 	if (byt == NULL)
 		return -ENOMEM;
 
-	byt->dev = dev;
-	INIT_LIST_HEAD(&byt->stream_list);
-	INIT_LIST_HEAD(&byt->tx_list);
-	INIT_LIST_HEAD(&byt->rx_list);
-	INIT_LIST_HEAD(&byt->empty_list);
-	init_waitqueue_head(&byt->boot_wait);
-	init_waitqueue_head(&byt->wait_txq);
+	ipc = &byt->ipc;
+	ipc->dev = dev;
+	ipc->ops.tx_msg = byt_tx_msg;
+	ipc->ops.shim_dbg = byt_shim_dbg;
+	ipc->ops.tx_data_copy = byt_tx_data_copy;
+	ipc->ops.reply_msg_match = byt_reply_msg_match;
 
-	err = msg_empty_list_init(byt);
-	if (err < 0)
-		return -ENOMEM;
-
-	/* start the IPC message thread */
-	init_kthread_worker(&byt->kworker);
-	byt->tx_thread = kthread_run(kthread_worker_fn,
-				     &byt->kworker, "%s",
-				     dev_name(byt->dev));
-	if (IS_ERR(byt->tx_thread)) {
-		err = PTR_ERR(byt->tx_thread);
-		dev_err(byt->dev, "error failed to create message TX task\n");
-		goto err_free_msg;
-	}
-	init_kthread_work(&byt->kwork, sst_byt_ipc_tx_msgs);
+	err = sst_ipc_init(ipc);
+	if (err != 0)
+		goto ipc_init_err;
 
+	INIT_LIST_HEAD(&byt->stream_list);
+	init_waitqueue_head(&byt->boot_wait);
 	byt_dev.thread_context = byt;
 
 	/* init SST shim */
 	byt->dsp = sst_dsp_new(dev, &byt_dev, pdata);
 	if (byt->dsp == NULL) {
 		err = -ENODEV;
-		goto dsp_err;
+		goto dsp_new_err;
 	}
 
+	ipc->dsp = byt->dsp;
+
 	/* keep the DSP in reset state for base FW loading */
 	sst_dsp_reset(byt->dsp);
 
@@ -961,10 +756,10 @@ boot_err:
 	sst_fw_free(byt_sst_fw);
 fw_err:
 	sst_dsp_free(byt->dsp);
-dsp_err:
-	kthread_stop(byt->tx_thread);
-err_free_msg:
-	kfree(byt->msg);
+dsp_new_err:
+	sst_ipc_fini(ipc);
+ipc_init_err:
+	kfree(byt);
 
 	return err;
 }
@@ -977,7 +772,6 @@ void sst_byt_dsp_free(struct device *dev, struct sst_pdata *pdata)
 	sst_dsp_reset(byt->dsp);
 	sst_fw_free_all(byt->dsp);
 	sst_dsp_free(byt->dsp);
-	kthread_stop(byt->tx_thread);
-	kfree(byt->msg);
+	sst_ipc_fini(&byt->ipc);
 }
 EXPORT_SYMBOL_GPL(sst_byt_dsp_free);
Signature · 1 line
1.9.1
author
#3 lore raw

PATCH 3/3 ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

In reply to Jin Yao

Use the generic IPC/mailbox APIs to replace the original processing code for Broadwell platform.

---
 sound/soc/intel/haswell/sst-haswell-ipc.c | 382 +++++++-----------------------
 1 file changed, 87 insertions(+), 295 deletions(-)
sound/soc/intel/haswell/sst-haswell-ipc.c +87 -295
index 28667d8..d75f09e 100644
@@ -36,6 +36,7 @@
 #include "sst-haswell-ipc.h"
 #include "../common/sst-dsp.h"
 #include "../common/sst-dsp-priv.h"
+#include "../common/sst-ipc.h"
 
 /* Global Message - Generic */
 #define IPC_GLB_TYPE_SHIFT	24
@@ -210,23 +211,6 @@ struct sst_hsw_ipc_fw_ready {
 	u8 fw_info[IPC_MAX_MAILBOX_BYTES - 5 * sizeof(u32)];
 } __attribute__((packed));
 
-struct ipc_message {
-	struct list_head list;
-	u32 header;
-
-	/* direction wrt host CPU */
-	char tx_data[IPC_MAX_MAILBOX_BYTES];
-	size_t tx_size;
-	char rx_data[IPC_MAX_MAILBOX_BYTES];
-	size_t rx_size;
-
-	wait_queue_head_t waitq;
-	bool pending;
-	bool complete;
-	bool wait;
-	int errno;
-};
-
 struct sst_hsw_stream;
 struct sst_hsw;
 
@@ -325,15 +309,7 @@ struct sst_hsw {
 	bool shutdown;
 
 	/* IPC messaging */
-	struct list_head tx_list;
-	struct list_head rx_list;
-	struct list_head empty_list;
-	wait_queue_head_t wait_txq;
-	struct task_struct *tx_thread;
-	struct kthread_worker kworker;
-	struct kthread_work kwork;
-	bool pending;
-	struct ipc_message *msg;
+	struct sst_generic_ipc ipc;
 
 	/* FW log stream */
 	struct sst_hsw_log_stream log_stream;
@@ -456,159 +432,6 @@ static struct sst_hsw_stream *get_stream_by_id(struct sst_hsw *hsw,
 	return NULL;
 }
 
-static void ipc_shim_dbg(struct sst_hsw *hsw, const char *text)
-{
-	struct sst_dsp *sst = hsw->dsp;
-	u32 isr, ipcd, imrx, ipcx;
-
-	ipcx = sst_dsp_shim_read_unlocked(sst, SST_IPCX);
-	isr = sst_dsp_shim_read_unlocked(sst, SST_ISRX);
-	ipcd = sst_dsp_shim_read_unlocked(sst, SST_IPCD);
-	imrx = sst_dsp_shim_read_unlocked(sst, SST_IMRX);
-
-	dev_err(hsw->dev, "ipc: --%s-- ipcx 0x%8.8x isr 0x%8.8x ipcd 0x%8.8x imrx 0x%8.8x\n",
-		text, ipcx, isr, ipcd, imrx);
-}
-
-/* locks held by caller */
-static struct ipc_message *msg_get_empty(struct sst_hsw *hsw)
-{
-	struct ipc_message *msg = NULL;
-
-	if (!list_empty(&hsw->empty_list)) {
-		msg = list_first_entry(&hsw->empty_list, struct ipc_message,
-			list);
-		list_del(&msg->list);
-	}
-
-	return msg;
-}
-
-static void ipc_tx_msgs(struct kthread_work *work)
-{
-	struct sst_hsw *hsw =
-		container_of(work, struct sst_hsw, kwork);
-	struct ipc_message *msg;
-	unsigned long flags;
-	u32 ipcx;
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	if (list_empty(&hsw->tx_list) || hsw->pending) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return;
-	}
-
-	/* if the DSP is busy, we will TX messages after IRQ.
-	 * also postpone if we are in the middle of procesing completion irq*/
-	ipcx = sst_dsp_shim_read_unlocked(hsw->dsp, SST_IPCX);
-	if (ipcx & (SST_IPCX_BUSY | SST_IPCX_DONE)) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return;
-	}
-
-	msg = list_first_entry(&hsw->tx_list, struct ipc_message, list);
-
-	list_move(&msg->list, &hsw->rx_list);
-
-	/* send the message */
-	sst_dsp_outbox_write(hsw->dsp, msg->tx_data, msg->tx_size);
-	sst_dsp_ipc_msg_tx(hsw->dsp, msg->header | SST_IPCX_BUSY);
-
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-}
-
-/* locks held by caller */
-static void tx_msg_reply_complete(struct sst_hsw *hsw, struct ipc_message *msg)
-{
-	msg->complete = true;
-	trace_ipc_reply("completed", msg->header);
-
-	if (!msg->wait)
-		list_add_tail(&msg->list, &hsw->empty_list);
-	else
-		wake_up(&msg->waitq);
-}
-
-static int tx_wait_done(struct sst_hsw *hsw, struct ipc_message *msg,
-	void *rx_data)
-{
-	unsigned long flags;
-	int ret;
-
-	/* wait for DSP completion (in all cases atm inc pending) */
-	ret = wait_event_timeout(msg->waitq, msg->complete,
-		msecs_to_jiffies(IPC_TIMEOUT_MSECS));
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-	if (ret == 0) {
-		ipc_shim_dbg(hsw, "message timeout");
-
-		trace_ipc_error("error message timeout for", msg->header);
-		list_del(&msg->list);
-		ret = -ETIMEDOUT;
-	} else {
-
-		/* copy the data returned from DSP */
-		if (msg->rx_size)
-			memcpy(rx_data, msg->rx_data, msg->rx_size);
-		ret = msg->errno;
-	}
-
-	list_add_tail(&msg->list, &hsw->empty_list);
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-	return ret;
-}
-
-static int ipc_tx_message(struct sst_hsw *hsw, u32 header, void *tx_data,
-	size_t tx_bytes, void *rx_data, size_t rx_bytes, int wait)
-{
-	struct ipc_message *msg;
-	unsigned long flags;
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	msg = msg_get_empty(hsw);
-	if (msg == NULL) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return -EBUSY;
-	}
-
-	if (tx_bytes)
-		memcpy(msg->tx_data, tx_data, tx_bytes);
-
-	msg->header = header;
-	msg->tx_size = tx_bytes;
-	msg->rx_size = rx_bytes;
-	msg->wait = wait;
-	msg->errno = 0;
-	msg->pending = false;
-	msg->complete = false;
-
-	list_add_tail(&msg->list, &hsw->tx_list);
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-
-	queue_kthread_work(&hsw->kworker, &hsw->kwork);
-
-	if (wait)
-		return tx_wait_done(hsw, msg, rx_data);
-	else
-		return 0;
-}
-
-static inline int ipc_tx_message_wait(struct sst_hsw *hsw, u32 header,
-	void *tx_data, size_t tx_bytes, void *rx_data, size_t rx_bytes)
-{
-	return ipc_tx_message(hsw, header, tx_data, tx_bytes, rx_data,
-		rx_bytes, 1);
-}
-
-static inline int ipc_tx_message_nowait(struct sst_hsw *hsw, u32 header,
-	void *tx_data, size_t tx_bytes)
-{
-	return ipc_tx_message(hsw, header, tx_data, tx_bytes, NULL, 0, 0);
-}
-
 static void hsw_fw_ready(struct sst_hsw *hsw, u32 header)
 {
 	struct sst_hsw_ipc_fw_ready fw_ready;
@@ -696,27 +519,6 @@ static void hsw_notification_work(struct work_struct *work)
 	sst_dsp_shim_update_bits(hsw->dsp, SST_IMRX, SST_IMRX_BUSY, 0);
 }
 
-static struct ipc_message *reply_find_msg(struct sst_hsw *hsw, u32 header)
-{
-	struct ipc_message *msg;
-
-	/* clear reply bits & status bits */
-	header &= ~(IPC_STATUS_MASK | IPC_GLB_REPLY_MASK);
-
-	if (list_empty(&hsw->rx_list)) {
-		dev_err(hsw->dev, "error: rx list empty but received 0x%x\n",
-			header);
-		return NULL;
-	}
-
-	list_for_each_entry(msg, &hsw->rx_list, list) {
-		if (msg->header == header)
-			return msg;
-	}
-
-	return NULL;
-}
-
 static void hsw_stream_update(struct sst_hsw *hsw, struct ipc_message *msg)
 {
 	struct sst_hsw_stream *stream;
@@ -755,7 +557,7 @@ static int hsw_process_reply(struct sst_hsw *hsw, u32 header)
 
 	trace_ipc_reply("processing -->", header);
 
-	msg = reply_find_msg(hsw, header);
+	msg = sst_ipc_reply_find_msg(&hsw->ipc, header);
 	if (msg == NULL) {
 		trace_ipc_error("error: can't find message header", header);
 		return -EIO;
@@ -766,14 +568,14 @@ static int hsw_process_reply(struct sst_hsw *hsw, u32 header)
 	case IPC_GLB_REPLY_PENDING:
 		trace_ipc_pending_reply("received", header);
 		msg->pending = true;
-		hsw->pending = true;
+		hsw->ipc.pending = true;
 		return 1;
 	case IPC_GLB_REPLY_SUCCESS:
 		if (msg->pending) {
 			trace_ipc_pending_reply("completed", header);
 			sst_dsp_inbox_read(hsw->dsp, msg->rx_data,
 				msg->rx_size);
-			hsw->pending = false;
+			hsw->ipc.pending = false;
 		} else {
 			/* copy data from the DSP */
 			sst_dsp_outbox_read(hsw->dsp, msg->rx_data,
@@ -829,7 +631,7 @@ static int hsw_process_reply(struct sst_hsw *hsw, u32 header)
 
 	/* wake up and return the error if we have waiters on this message ? */
 	list_del(&msg->list);
-	tx_msg_reply_complete(hsw, msg);
+	sst_ipc_tx_msg_reply_complete(&hsw->ipc, msg);
 
 	return 1;
 }
@@ -970,6 +772,7 @@ static irqreturn_t hsw_irq_thread(int irq, void *context)
 {
 	struct sst_dsp *sst = (struct sst_dsp *) context;
 	struct sst_hsw *hsw = sst_dsp_get_thread_context(sst);
+	struct sst_generic_ipc *ipc = &hsw->ipc;
 	u32 ipcx, ipcd;
 	int handled;
 	unsigned long flags;
@@ -1016,7 +819,7 @@ static irqreturn_t hsw_irq_thread(int irq, void *context)
 	spin_unlock_irqrestore(&sst->spinlock, flags);
 
 	/* continue to send any remaining messages... */
-	queue_kthread_work(&hsw->kworker, &hsw->kwork);
+	queue_kthread_work(&ipc->kworker, &ipc->kwork);
 
 	return IRQ_HANDLED;
 }
@@ -1026,7 +829,8 @@ int sst_hsw_fw_get_version(struct sst_hsw *hsw,
 {
 	int ret;
 
-	ret = ipc_tx_message_wait(hsw, IPC_GLB_TYPE(IPC_GLB_GET_FW_VERSION),
+	ret = sst_ipc_tx_message_wait(&hsw->ipc,
+		IPC_GLB_TYPE(IPC_GLB_GET_FW_VERSION),
 		NULL, 0, version, sizeof(*version));
 	if (ret < 0)
 		dev_err(hsw->dev, "error: get version failed\n");
@@ -1090,7 +894,8 @@ int sst_hsw_stream_set_volume(struct sst_hsw *hsw,
 		req->channel = channel;
 	}
 
-	ret = ipc_tx_message_wait(hsw, header, req, sizeof(*req), NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, req,
+		sizeof(*req), NULL, 0);
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: set stream volume failed\n");
 		return ret;
@@ -1155,7 +960,8 @@ int sst_hsw_mixer_set_volume(struct sst_hsw *hsw, u32 stage_id, u32 channel,
 	req.curve_type = hsw->curve_type;
 	req.target_volume = volume;
 
-	ret = ipc_tx_message_wait(hsw, header, &req, sizeof(req), NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &req,
+		sizeof(req), NULL, 0);
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: set mixer volume failed\n");
 		return ret;
@@ -1213,7 +1019,7 @@ int sst_hsw_stream_free(struct sst_hsw *hsw, struct sst_hsw_stream *stream)
 	stream->free_req.stream_id = stream->reply.stream_hw_id;
 	header = IPC_GLB_TYPE(IPC_GLB_FREE_STREAM);
 
-	ret = ipc_tx_message_wait(hsw, header, &stream->free_req,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &stream->free_req,
 		sizeof(stream->free_req), NULL, 0);
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: free stream %d failed\n",
@@ -1405,8 +1211,8 @@ int sst_hsw_stream_commit(struct sst_hsw *hsw, struct sst_hsw_stream *stream)
 
 	header = IPC_GLB_TYPE(IPC_GLB_ALLOCATE_STREAM);
 
-	ret = ipc_tx_message_wait(hsw, header, str_req, sizeof(*str_req),
-		reply, sizeof(*reply));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, str_req,
+		sizeof(*str_req), reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: stream commit failed\n");
 		return ret;
@@ -1455,7 +1261,8 @@ int sst_hsw_mixer_get_info(struct sst_hsw *hsw)
 
 	trace_ipc_request("get global mixer info", 0);
 
-	ret = ipc_tx_message_wait(hsw, header, NULL, 0, reply, sizeof(*reply));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, NULL, 0,
+		reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: get stream info failed\n");
 		return ret;
@@ -1476,9 +1283,10 @@ static int sst_hsw_stream_operations(struct sst_hsw *hsw, int type,
 	header |= (stream_id << IPC_STR_ID_SHIFT);
 
 	if (wait)
-		return ipc_tx_message_wait(hsw, header, NULL, 0, NULL, 0);
+		return sst_ipc_tx_message_wait(&hsw->ipc, header,
+			NULL, 0, NULL, 0);
 	else
-		return ipc_tx_message_nowait(hsw, header, NULL, 0);
+		return sst_ipc_tx_message_nowait(&hsw->ipc, header, NULL, 0);
 }
 
 /* Stream ALSA trigger operations */
@@ -1605,8 +1413,8 @@ int sst_hsw_device_set_config(struct sst_hsw *hsw,
 
 	header = IPC_GLB_TYPE(IPC_GLB_SET_DEVICE_FORMATS);
 
-	ret = ipc_tx_message_wait(hsw, header, &config, sizeof(config),
-		NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &config,
+		sizeof(config), NULL, 0);
 	if (ret < 0)
 		dev_err(hsw->dev, "error: set device formats failed\n");
 
@@ -1626,8 +1434,8 @@ int sst_hsw_dx_set_state(struct sst_hsw *hsw,
 
 	trace_ipc_request("PM enter Dx state", state);
 
-	ret = ipc_tx_message_wait(hsw, header, &state_, sizeof(state_),
-		dx, sizeof(*dx));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &state_,
+		sizeof(state_), dx, sizeof(*dx));
 	if (ret < 0) {
 		dev_err(hsw->dev, "ipc: error set dx state %d failed\n", state);
 		return ret;
@@ -1770,32 +1578,6 @@ static int sst_hsw_dx_state_restore(struct sst_hsw *hsw)
 	return 0;
 }
 
-static void sst_hsw_drop_all(struct sst_hsw *hsw)
-{
-	struct ipc_message *msg, *tmp;
-	unsigned long flags;
-	int tx_drop_cnt = 0, rx_drop_cnt = 0;
-
-	/* drop all TX and Rx messages before we stall + reset DSP */
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	list_for_each_entry_safe(msg, tmp, &hsw->tx_list, list) {
-		list_move(&msg->list, &hsw->empty_list);
-		tx_drop_cnt++;
-	}
-
-	list_for_each_entry_safe(msg, tmp, &hsw->rx_list, list) {
-		list_move(&msg->list, &hsw->empty_list);
-		rx_drop_cnt++;
-	}
-
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-
-	if (tx_drop_cnt || rx_drop_cnt)
-		dev_err(hsw->dev, "dropped IPC msg RX=%d, TX=%d\n",
-			tx_drop_cnt, rx_drop_cnt);
-}
-
 int sst_hsw_dsp_load(struct sst_hsw *hsw)
 {
 	struct sst_dsp *dsp = hsw->dsp;
@@ -1875,7 +1657,7 @@ int sst_hsw_dsp_runtime_suspend(struct sst_hsw *hsw)
 	if (ret < 0)
 		return ret;
 
-	sst_hsw_drop_all(hsw);
+	sst_ipc_drop_all(&hsw->ipc);
 
 	return 0;
 }
@@ -1933,23 +1715,6 @@ int sst_hsw_dsp_runtime_resume(struct sst_hsw *hsw)
 }
 #endif
 
-static int msg_empty_list_init(struct sst_hsw *hsw)
-{
-	int i;
-
-	hsw->msg = kzalloc(sizeof(struct ipc_message) *
-		IPC_EMPTY_LIST_SIZE, GFP_KERNEL);
-	if (hsw->msg == NULL)
-		return -ENOMEM;
-
-	for (i = 0; i < IPC_EMPTY_LIST_SIZE; i++) {
-		init_waitqueue_head(&hsw->msg[i].waitq);
-		list_add(&hsw->msg[i].list, &hsw->empty_list);
-	}
-
-	return 0;
-}
-
 struct sst_dsp *sst_hsw_get_dsp(struct sst_hsw *hsw)
 {
 	return hsw->dsp;
@@ -2184,7 +1949,7 @@ int sst_hsw_module_enable(struct sst_hsw *hsw,
 		config.scratch_mem.size, config.scratch_mem.offset,
 		config.map.module_entries[0].entry_point);
 
-	ret = ipc_tx_message_wait(hsw, header,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,
 			&config, sizeof(config), NULL, 0);
 	if (ret < 0)
 		dev_err(dev, "ipc: module enable failed - %d\n", ret);
@@ -2223,7 +1988,7 @@ int sst_hsw_module_disable(struct sst_hsw *hsw,
 			IPC_MODULE_OPERATION(IPC_MODULE_DISABLE) |
 			IPC_MODULE_ID(module_id);
 
-	ret = ipc_tx_message_wait(hsw, header,  NULL, 0, NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,  NULL, 0, NULL, 0);
 	if (ret < 0)
 		dev_err(dev, "module disable failed - %d\n", ret);
 	else
@@ -2277,7 +2042,7 @@ int sst_hsw_module_set_param(struct sst_hsw *hsw,
 	parameter->parameter_id = parameter_id;
 	parameter->data_size = param_size;
 
-	ret = ipc_tx_message_wait(hsw, header,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,
 		parameter, transfer_parameter_size , NULL, 0);
 	if (ret < 0)
 		dev_err(dev, "ipc: module set parameter failed - %d\n", ret);
@@ -2296,10 +2061,48 @@ static struct sst_dsp_device hsw_dev = {
 	.ops = &haswell_ops,
 };
 
+static void hsw_tx_msg(struct sst_generic_ipc *ipc, struct ipc_message *msg)
+{
+	/* send the message */
+	sst_dsp_outbox_write(ipc->dsp, msg->tx_data, msg->tx_size);
+	sst_dsp_ipc_msg_tx(ipc->dsp, msg->header);
+}
+
+static void hsw_shim_dbg(struct sst_generic_ipc *ipc, const char *text)
+{
+	struct sst_dsp *sst = ipc->dsp;
+	u32 isr, ipcd, imrx, ipcx;
+
+	ipcx = sst_dsp_shim_read_unlocked(sst, SST_IPCX);
+	isr = sst_dsp_shim_read_unlocked(sst, SST_ISRX);
+	ipcd = sst_dsp_shim_read_unlocked(sst, SST_IPCD);
+	imrx = sst_dsp_shim_read_unlocked(sst, SST_IMRX);
+
+	dev_err(ipc->dev,
+		"ipc: --%s-- ipcx 0x%8.8x isr 0x%8.8x ipcd 0x%8.8x imrx 0x%8.8x\n",
+		text, ipcx, isr, ipcd, imrx);
+}
+
+static void hsw_tx_data_copy(struct ipc_message *msg, char *tx_data,
+	size_t tx_size)
+{
+	memcpy(msg->tx_data, tx_data, tx_size);
+}
+
+static u64 hsw_reply_msg_match(u64 header, u64 *mask)
+{
+	/* clear reply bits & status bits */
+	header &= ~(IPC_STATUS_MASK | IPC_GLB_REPLY_MASK);
+	*mask = (u64)-1;
+
+	return header;
+}
+
 int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)
 {
 	struct sst_hsw_ipc_fw_version version;
 	struct sst_hsw *hsw;
+	struct sst_generic_ipc *ipc;
 	int ret;
 
 	dev_dbg(dev, "initialising Audio DSP IPC\n");
@@ -2308,39 +2111,30 @@ int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)
 	if (hsw == NULL)
 		return -ENOMEM;
 
-	hsw->dev = dev;
-	INIT_LIST_HEAD(&hsw->stream_list);
-	INIT_LIST_HEAD(&hsw->tx_list);
-	INIT_LIST_HEAD(&hsw->rx_list);
-	INIT_LIST_HEAD(&hsw->empty_list);
-	init_waitqueue_head(&hsw->boot_wait);
-	init_waitqueue_head(&hsw->wait_txq);
+	ipc = &hsw->ipc;
+	ipc->dev = dev;
+	ipc->ops.tx_msg = hsw_tx_msg;
+	ipc->ops.shim_dbg = hsw_shim_dbg;
+	ipc->ops.tx_data_copy = hsw_tx_data_copy;
+	ipc->ops.reply_msg_match = hsw_reply_msg_match;
 
-	ret = msg_empty_list_init(hsw);
-	if (ret < 0)
-		return -ENOMEM;
-
-	/* start the IPC message thread */
-	init_kthread_worker(&hsw->kworker);
-	hsw->tx_thread = kthread_run(kthread_worker_fn,
-					   &hsw->kworker, "%s",
-					   dev_name(hsw->dev));
-	if (IS_ERR(hsw->tx_thread)) {
-		ret = PTR_ERR(hsw->tx_thread);
-		dev_err(hsw->dev, "error: failed to create message TX task\n");
-		goto err_free_msg;
-	}
-	init_kthread_work(&hsw->kwork, ipc_tx_msgs);
+	ret = sst_ipc_init(ipc);
+	if (ret != 0)
+		goto ipc_init_err;
 
+	INIT_LIST_HEAD(&hsw->stream_list);
+	init_waitqueue_head(&hsw->boot_wait);
 	hsw_dev.thread_context = hsw;
 
 	/* init SST shim */
 	hsw->dsp = sst_dsp_new(dev, &hsw_dev, pdata);
 	if (hsw->dsp == NULL) {
 		ret = -ENODEV;
-		goto dsp_err;
+		goto dsp_new_err;
 	}
 
+	ipc->dsp = hsw->dsp;
+
 	/* allocate DMA buffer for context storage */
 	hsw->dx_context = dma_alloc_coherent(hsw->dsp->dma_dev,
 		SST_HSW_DX_CONTEXT_SIZE, &hsw->dx_context_paddr, GFP_KERNEL);
@@ -2404,11 +2198,10 @@ fw_err:
 			hsw->dx_context, hsw->dx_context_paddr);
 dma_err:
 	sst_dsp_free(hsw->dsp);
-dsp_err:
-	kthread_stop(hsw->tx_thread);
-err_free_msg:
-	kfree(hsw->msg);
-
+dsp_new_err:
+	sst_ipc_fini(ipc);
+ipc_init_err:
+	kfree(hsw);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sst_hsw_dsp_init);
@@ -2422,7 +2215,6 @@ void sst_hsw_dsp_free(struct device *dev, struct sst_pdata *pdata)
 	dma_free_coherent(hsw->dsp->dma_dev, SST_HSW_DX_CONTEXT_SIZE,
 			hsw->dx_context, hsw->dx_context_paddr);
 	sst_dsp_free(hsw->dsp);
-	kthread_stop(hsw->tx_thread);
-	kfree(hsw->msg);
+	sst_ipc_fini(&hsw->ipc);
 }
 EXPORT_SYMBOL_GPL(sst_hsw_dsp_free);
Signature · 1 line
1.9.1
#4 lore raw

Re: [PATCH 3/3] ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

Quoted 131 lines

-----Original Message-----
From: [email protected] [mailto:alsa-devel- [email protected]] On Behalf Of Jin Yao
Sent: Tuesday, April 07, 2015 9:34 AM
To: [email protected]; Girdwood, Liam R
Cc: [email protected]; Jin Yao
Subject: [alsa-devel] [PATCH 3/3] ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

Use the generic IPC/mailbox APIs to replace the original processing code for Broadwell platform.

---
 sound/soc/intel/haswell/sst-haswell-ipc.c | 382 +++++++-----------------------
 1 file changed, 87 insertions(+), 295 deletions(-)
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -80
index 28667d8..d75f09e 100644
@@ -36,6 +36,7 @@
 #include "sst-haswell-ipc.h"
 #include "../common/sst-dsp.h"
 #include "../common/sst-dsp-priv.h"
+#include "../common/sst-ipc.h"
 
 /* Global Message - Generic */
 #define IPC_GLB_TYPE_SHIFT	24
@@ -210,23 +211,6 @@ struct sst_hsw_ipc_fw_ready {
 	u8 fw_info[IPC_MAX_MAILBOX_BYTES - 5 * sizeof(u32)];  }
 __attribute__((packed));
 
-struct ipc_message {
-	struct list_head list;
-	u32 header;
-
-	/* direction wrt host CPU */
-	char tx_data[IPC_MAX_MAILBOX_BYTES];
-	size_t tx_size;
-	char rx_data[IPC_MAX_MAILBOX_BYTES];
-	size_t rx_size;
-
-	wait_queue_head_t waitq;
-	bool pending;
-	bool complete;
-	bool wait;
-	int errno;
-};
-
 struct sst_hsw_stream;
 struct sst_hsw;
 
@@ -325,15 +309,7 @@ struct sst_hsw {
 	bool shutdown;
 
 	/* IPC messaging */
-	struct list_head tx_list;
-	struct list_head rx_list;
-	struct list_head empty_list;
-	wait_queue_head_t wait_txq;
-	struct task_struct *tx_thread;
-	struct kthread_worker kworker;
-	struct kthread_work kwork;
-	bool pending;
-	struct ipc_message *msg;
+	struct sst_generic_ipc ipc;
 
 	/* FW log stream */
 	struct sst_hsw_log_stream log_stream;
@@ -456,159 +432,6 @@ static struct sst_hsw_stream
 *get_stream_by_id(struct sst_hsw *hsw,
 	return NULL;
 }
 
-static void ipc_shim_dbg(struct sst_hsw *hsw, const char *text) -{
-	struct sst_dsp *sst = hsw->dsp;
-	u32 isr, ipcd, imrx, ipcx;
-
-	ipcx = sst_dsp_shim_read_unlocked(sst, SST_IPCX);
-	isr = sst_dsp_shim_read_unlocked(sst, SST_ISRX);
-	ipcd = sst_dsp_shim_read_unlocked(sst, SST_IPCD);
-	imrx = sst_dsp_shim_read_unlocked(sst, SST_IMRX);
-
-	dev_err(hsw->dev, "ipc: --%s-- ipcx 0x%8.8x isr 0x%8.8x ipcd 0x%8.8x
 imrx 0x%8.8x\n",
-		text, ipcx, isr, ipcd, imrx);
-}
-
-/* locks held by caller */
-static struct ipc_message *msg_get_empty(struct sst_hsw *hsw) -{
-	struct ipc_message *msg = NULL;
-
-	if (!list_empty(&hsw->empty_list)) {
-		msg = list_first_entry(&hsw->empty_list, struct ipc_message,
-			list);
-		list_del(&msg->list);
-	}
-
-	return msg;
-}
-
-static void ipc_tx_msgs(struct kthread_work *work) -{
-	struct sst_hsw *hsw =
-		container_of(work, struct sst_hsw, kwork);
-	struct ipc_message *msg;
-	unsigned long flags;
-	u32 ipcx;
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	if (list_empty(&hsw->tx_list) || hsw->pending) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return;
-	}
-
-	/* if the DSP is busy, we will TX messages after IRQ.
-	 * also postpone if we are in the middle of procesing completion irq*/
-	ipcx = sst_dsp_shim_read_unlocked(hsw->dsp, SST_IPCX);
-	if (ipcx & (SST_IPCX_BUSY | SST_IPCX_DONE)) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return;
-	}
-
-	msg = list_first_entry(&hsw->tx_list, struct ipc_message, list);
-
-	list_move(&msg->list, &hsw->rx_list);
-
-	/* send the message */
-	sst_dsp_outbox_write(hsw->dsp, msg->tx_data, msg->tx_size);

sound/soc/intel/haswell/sst-haswell-ipc.c static struct sst_hsw_stream · line 524

Quoted 10 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -9
-	sst_dsp_ipc_msg_tx(hsw->dsp, msg->header | SST_IPCX_BUSY);
-
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-}
-
-/* locks held by caller */
-static void tx_msg_reply_complete(struct sst_hsw *hsw, struct
 ipc_message *msg) -{
-	msg->complete = true;
-	trace_ipc_reply("completed", msg->header);

[Keyon] these trace ipc logs will be removed with your patches applied?

Quoted 12 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -12
-
-	if (!msg->wait)
-		list_add_tail(&msg->list, &hsw->empty_list);
-	else
-		wake_up(&msg->waitq);
-}
-
-static int tx_wait_done(struct sst_hsw *hsw, struct ipc_message *msg,
-	void *rx_data)
-{
-	unsigned long flags;
-	int ret;

sound/soc/intel/haswell/sst-haswell-ipc.c static struct sst_hsw_stream *get_stream_by_id(struct sst_hsw *hsw, · line 547

Quoted 10 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -10
-
-	/* wait for DSP completion (in all cases atm inc pending) */
-	ret = wait_event_timeout(msg->waitq, msg->complete,
-		msecs_to_jiffies(IPC_TIMEOUT_MSECS));
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-	if (ret == 0) {
-		ipc_shim_dbg(hsw, "message timeout");
-
-		trace_ipc_error("error message timeout for", msg->header);

[Keyon] ditto.

Quoted 484 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -85
-		list_del(&msg->list);
-		ret = -ETIMEDOUT;
-	} else {
-
-		/* copy the data returned from DSP */
-		if (msg->rx_size)
-			memcpy(rx_data, msg->rx_data, msg->rx_size);
-		ret = msg->errno;
-	}
-
-	list_add_tail(&msg->list, &hsw->empty_list);
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-	return ret;
-}
-
-static int ipc_tx_message(struct sst_hsw *hsw, u32 header, void *tx_data,
-	size_t tx_bytes, void *rx_data, size_t rx_bytes, int wait)
-{
-	struct ipc_message *msg;
-	unsigned long flags;
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	msg = msg_get_empty(hsw);
-	if (msg == NULL) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return -EBUSY;
-	}
-
-	if (tx_bytes)
-		memcpy(msg->tx_data, tx_data, tx_bytes);
-
-	msg->header = header;
-	msg->tx_size = tx_bytes;
-	msg->rx_size = rx_bytes;
-	msg->wait = wait;
-	msg->errno = 0;
-	msg->pending = false;
-	msg->complete = false;
-
-	list_add_tail(&msg->list, &hsw->tx_list);
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-
-	queue_kthread_work(&hsw->kworker, &hsw->kwork);
-
-	if (wait)
-		return tx_wait_done(hsw, msg, rx_data);
-	else
-		return 0;
-}
-
-static inline int ipc_tx_message_wait(struct sst_hsw *hsw, u32 header,
-	void *tx_data, size_t tx_bytes, void *rx_data, size_t rx_bytes)
-{
-	return ipc_tx_message(hsw, header, tx_data, tx_bytes, rx_data,
-		rx_bytes, 1);
-}
-
-static inline int ipc_tx_message_nowait(struct sst_hsw *hsw, u32 header,
-	void *tx_data, size_t tx_bytes)
-{
-	return ipc_tx_message(hsw, header, tx_data, tx_bytes, NULL, 0, 0);
-}
-
 static void hsw_fw_ready(struct sst_hsw *hsw, u32 header)  {
 	struct sst_hsw_ipc_fw_ready fw_ready;
@@ -696,27 +519,6 @@ static void hsw_notification_work(struct work_struct
 *work)
 	sst_dsp_shim_update_bits(hsw->dsp, SST_IMRX, SST_IMRX_BUSY,
 0);  }
 
-static struct ipc_message *reply_find_msg(struct sst_hsw *hsw, u32 header)
-{
-	struct ipc_message *msg;
-
-	/* clear reply bits & status bits */
-	header &= ~(IPC_STATUS_MASK | IPC_GLB_REPLY_MASK);
-
-	if (list_empty(&hsw->rx_list)) {
-		dev_err(hsw->dev, "error: rx list empty but received
 0x%x\n",
-			header);
-		return NULL;
-	}
-
-	list_for_each_entry(msg, &hsw->rx_list, list) {
-		if (msg->header == header)
-			return msg;
-	}
-
-	return NULL;
-}
-
 static void hsw_stream_update(struct sst_hsw *hsw, struct ipc_message

*msg) {

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 	struct sst_hsw_stream *stream;
@@ -755,7 +557,7 @@ static int hsw_process_reply(struct sst_hsw *hsw,
 u32 header)
 
 	trace_ipc_reply("processing -->", header);
 
-	msg = reply_find_msg(hsw, header);
+	msg = sst_ipc_reply_find_msg(&hsw->ipc, header);
 	if (msg == NULL) {
 		trace_ipc_error("error: can't find message header", header);
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -2
 		return -EIO;
@@ -766,14 +568,14 @@ static int hsw_process_reply(struct sst_hsw *hsw,
 u32 header)
 	case IPC_GLB_REPLY_PENDING:
 		trace_ipc_pending_reply("received", header);
 		msg->pending = true;
-		hsw->pending = true;
+		hsw->ipc.pending = true;
 		return 1;
 	case IPC_GLB_REPLY_SUCCESS:
 		if (msg->pending) {
 			trace_ipc_pending_reply("completed", header);
 			sst_dsp_inbox_read(hsw->dsp, msg->rx_data,
 				msg->rx_size);
-			hsw->pending = false;
+			hsw->ipc.pending = false;
 		} else {
 			/* copy data from the DSP */

sst_dsp_outbox_read(hsw->dsp, msg->rx_data, @@
-829,7 +631,7 @@ static int hsw_process_reply(struct sst_hsw *hsw, u32 header)

/* wake up and return the error if we have waiters on this message ?
*/

sound/soc/intel/haswell/sst-haswell-ipc.c +3 -2
 	list_del(&msg->list);
-	tx_msg_reply_complete(hsw, msg);
+	sst_ipc_tx_msg_reply_complete(&hsw->ipc, msg);
 
 	return 1;
 }
@@ -970,6 +772,7 @@ static irqreturn_t hsw_irq_thread(int irq, void
 *context)  {
 	struct sst_dsp *sst = (struct sst_dsp *) context;
 	struct sst_hsw *hsw = sst_dsp_get_thread_context(sst);
+	struct sst_generic_ipc *ipc = &hsw->ipc;
 	u32 ipcx, ipcd;
 	int handled;
 	unsigned long flags;
@@ -1016,7 +819,7 @@ static irqreturn_t hsw_irq_thread(int irq, void
 *context)
 	spin_unlock_irqrestore(&sst->spinlock, flags);
 
 	/* continue to send any remaining messages... */
-	queue_kthread_work(&hsw->kworker, &hsw->kwork);
+	queue_kthread_work(&ipc->kworker, &ipc->kwork);
 
 	return IRQ_HANDLED;

}

sound/soc/intel/haswell/sst-haswell-ipc.c +2 -1
@@ -1026,7 +829,8 @@ int sst_hsw_fw_get_version(struct sst_hsw *hsw,  {
 	int ret;
 
-	ret = ipc_tx_message_wait(hsw,
 IPC_GLB_TYPE(IPC_GLB_GET_FW_VERSION),
+	ret = sst_ipc_tx_message_wait(&hsw->ipc,
+		IPC_GLB_TYPE(IPC_GLB_GET_FW_VERSION),
 		NULL, 0, version, sizeof(*version));
 	if (ret < 0)
 		dev_err(hsw->dev, "error: get version failed\n"); @@ -

1090,7 +894,8 @@ int sst_hsw_stream_set_volume(struct sst_hsw *hsw,

sound/soc/intel/haswell/sst-haswell-ipc.c +4 -2
 		req->channel = channel;
 	}
 
-	ret = ipc_tx_message_wait(hsw, header, req, sizeof(*req), NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, req,
+		sizeof(*req), NULL, 0);
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: set stream volume failed\n");
 		return ret;
@@ -1155,7 +960,8 @@ int sst_hsw_mixer_set_volume(struct sst_hsw *hsw,
 u32 stage_id, u32 channel,
 	req.curve_type = hsw->curve_type;
 	req.target_volume = volume;
 
-	ret = ipc_tx_message_wait(hsw, header, &req, sizeof(req), NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &req,
+		sizeof(req), NULL, 0);
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: set mixer volume failed\n");
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 		return ret;
@@ -1213,7 +1019,7 @@ int sst_hsw_stream_free(struct sst_hsw *hsw,
 struct sst_hsw_stream *stream)
 	stream->free_req.stream_id = stream->reply.stream_hw_id;
 	header = IPC_GLB_TYPE(IPC_GLB_FREE_STREAM);
 
-	ret = ipc_tx_message_wait(hsw, header, &stream->free_req,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &stream-
 >free_req,
 		sizeof(stream->free_req), NULL, 0);
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -0
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: free stream %d failed\n", @@ -

1405,8 +1211,8 @@ int sst_hsw_stream_commit(struct sst_hsw *hsw, struct sst_hsw_stream *stream)

sound/soc/intel/haswell/sst-haswell-ipc.c +4 -3
 	header = IPC_GLB_TYPE(IPC_GLB_ALLOCATE_STREAM);
 
-	ret = ipc_tx_message_wait(hsw, header, str_req, sizeof(*str_req),
-		reply, sizeof(*reply));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, str_req,
+		sizeof(*str_req), reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: stream commit failed\n");
 		return ret;
@@ -1455,7 +1261,8 @@ int sst_hsw_mixer_get_info(struct sst_hsw *hsw)
 
 	trace_ipc_request("get global mixer info", 0);
 
-	ret = ipc_tx_message_wait(hsw, header, NULL, 0, reply,
 sizeof(*reply));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, NULL, 0,
+		reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: get stream info failed\n");
sound/soc/intel/haswell/sst-haswell-ipc.c +3 -2
 		return ret;
@@ -1476,9 +1283,10 @@ static int sst_hsw_stream_operations(struct
 sst_hsw *hsw, int type,
 	header |= (stream_id << IPC_STR_ID_SHIFT);
 
 	if (wait)
-		return ipc_tx_message_wait(hsw, header, NULL, 0, NULL, 0);
+		return sst_ipc_tx_message_wait(&hsw->ipc, header,
+			NULL, 0, NULL, 0);
 	else
-		return ipc_tx_message_nowait(hsw, header, NULL, 0);
+		return sst_ipc_tx_message_nowait(&hsw->ipc, header,
 NULL, 0);
 }

/* Stream ALSA trigger operations */

sound/soc/intel/haswell/sst-haswell-ipc.c +4 -4
@@ -1605,8 +1413,8 @@ int sst_hsw_device_set_config(struct sst_hsw
 *hsw,
 
 	header = IPC_GLB_TYPE(IPC_GLB_SET_DEVICE_FORMATS);
 
-	ret = ipc_tx_message_wait(hsw, header, &config, sizeof(config),
-		NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &config,
+		sizeof(config), NULL, 0);
 	if (ret < 0)
 		dev_err(hsw->dev, "error: set device formats failed\n");
@@ -1626,8 +1434,8 @@ int sst_hsw_dx_set_state(struct sst_hsw *hsw,
 
 	trace_ipc_request("PM enter Dx state", state);
 
-	ret = ipc_tx_message_wait(hsw, header, &state_, sizeof(state_),
-		dx, sizeof(*dx));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &state_,
+		sizeof(state_), dx, sizeof(*dx));
 	if (ret < 0) {
 		dev_err(hsw->dev, "ipc: error set dx state %d failed\n",
 state);
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -26
 		return ret;
@@ -1770,32 +1578,6 @@ static int sst_hsw_dx_state_restore(struct
 sst_hsw *hsw)
 	return 0;
 }
 
-static void sst_hsw_drop_all(struct sst_hsw *hsw) -{
-	struct ipc_message *msg, *tmp;
-	unsigned long flags;
-	int tx_drop_cnt = 0, rx_drop_cnt = 0;
-
-	/* drop all TX and Rx messages before we stall + reset DSP */
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	list_for_each_entry_safe(msg, tmp, &hsw->tx_list, list) {
-		list_move(&msg->list, &hsw->empty_list);
-		tx_drop_cnt++;
-	}
-
-	list_for_each_entry_safe(msg, tmp, &hsw->rx_list, list) {
-		list_move(&msg->list, &hsw->empty_list);
-		rx_drop_cnt++;
-	}
-
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-
-	if (tx_drop_cnt || rx_drop_cnt)
-		dev_err(hsw->dev, "dropped IPC msg RX=%d, TX=%d\n",
-			tx_drop_cnt, rx_drop_cnt);
-}
-
 int sst_hsw_dsp_load(struct sst_hsw *hsw)  {
 	struct sst_dsp *dsp = hsw->dsp;
@@ -1875,7 +1657,7 @@ int sst_hsw_dsp_runtime_suspend(struct sst_hsw
 *hsw)
 	if (ret < 0)
 		return ret;
 
-	sst_hsw_drop_all(hsw);
+	sst_ipc_drop_all(&hsw->ipc);
 
 	return 0;

}

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -17
@@ -1933,23 +1715,6 @@ int sst_hsw_dsp_runtime_resume(struct sst_hsw
 *hsw)  }  #endif
 
-static int msg_empty_list_init(struct sst_hsw *hsw) -{
-	int i;
-
-	hsw->msg = kzalloc(sizeof(struct ipc_message) *
-		IPC_EMPTY_LIST_SIZE, GFP_KERNEL);
-	if (hsw->msg == NULL)
-		return -ENOMEM;
-
-	for (i = 0; i < IPC_EMPTY_LIST_SIZE; i++) {
-		init_waitqueue_head(&hsw->msg[i].waitq);
-		list_add(&hsw->msg[i].list, &hsw->empty_list);
-	}
-
-	return 0;
-}
-
 struct sst_dsp *sst_hsw_get_dsp(struct sst_hsw *hsw)  {
 	return hsw->dsp;
@@ -2184,7 +1949,7 @@ int sst_hsw_module_enable(struct sst_hsw *hsw,
 		config.scratch_mem.size, config.scratch_mem.offset,
 		config.map.module_entries[0].entry_point);
 
-	ret = ipc_tx_message_wait(hsw, header,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,
 			&config, sizeof(config), NULL, 0);
 	if (ret < 0)
 		dev_err(dev, "ipc: module enable failed - %d\n", ret); @@ -

2223,7 +1988,7 @@ int sst_hsw_module_disable(struct sst_hsw *hsw,

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 			IPC_MODULE_OPERATION(IPC_MODULE_DISABLE) |
 			IPC_MODULE_ID(module_id);
 
-	ret = ipc_tx_message_wait(hsw, header,  NULL, 0, NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,  NULL, 0, NULL,

0);

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 	if (ret < 0)
 		dev_err(dev, "module disable failed - %d\n", ret);
 	else
@@ -2277,7 +2042,7 @@ int sst_hsw_module_set_param(struct sst_hsw
 *hsw,
 	parameter->parameter_id = parameter_id;
 	parameter->data_size = param_size;
 
-	ret = ipc_tx_message_wait(hsw, header,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,
 		parameter, transfer_parameter_size , NULL, 0);
 	if (ret < 0)
sound/soc/intel/haswell/sst-haswell-ipc.c +35 -0
 		dev_err(dev, "ipc: module set parameter failed - %d\n", ret);
@@ -2296,10 +2061,48 @@ static struct sst_dsp_device hsw_dev = {
 	.ops = &haswell_ops,
 };
 
+static void hsw_tx_msg(struct sst_generic_ipc *ipc, struct ipc_message
+*msg) {
+	/* send the message */
+	sst_dsp_outbox_write(ipc->dsp, msg->tx_data, msg->tx_size);
+	sst_dsp_ipc_msg_tx(ipc->dsp, msg->header); }
+
+static void hsw_shim_dbg(struct sst_generic_ipc *ipc, const char *text)
+{
+	struct sst_dsp *sst = ipc->dsp;
+	u32 isr, ipcd, imrx, ipcx;
+
+	ipcx = sst_dsp_shim_read_unlocked(sst, SST_IPCX);
+	isr = sst_dsp_shim_read_unlocked(sst, SST_ISRX);
+	ipcd = sst_dsp_shim_read_unlocked(sst, SST_IPCD);
+	imrx = sst_dsp_shim_read_unlocked(sst, SST_IMRX);
+
+	dev_err(ipc->dev,
+		"ipc: --%s-- ipcx 0x%8.8x isr 0x%8.8x ipcd 0x%8.8x imrx
 0x%8.8x\n",
+		text, ipcx, isr, ipcd, imrx);
+}
+
+static void hsw_tx_data_copy(struct ipc_message *msg, char *tx_data,
+	size_t tx_size)
+{
+	memcpy(msg->tx_data, tx_data, tx_size); }
+
+static u64 hsw_reply_msg_match(u64 header, u64 *mask) {
+	/* clear reply bits & status bits */
+	header &= ~(IPC_STATUS_MASK | IPC_GLB_REPLY_MASK);
+	*mask = (u64)-1;
+
+	return header;
+}
+
 int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)  {
 	struct sst_hsw_ipc_fw_version version;
 	struct sst_hsw *hsw;
+	struct sst_generic_ipc *ipc;
 	int ret;
 
 	dev_dbg(dev, "initialising Audio DSP IPC\n"); @@ -2308,39 +2111,30

@@ int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)

sound/soc/intel/haswell/sst-haswell-ipc.c +6 -19
 	if (hsw == NULL)
 		return -ENOMEM;
 
-	hsw->dev = dev;
-	INIT_LIST_HEAD(&hsw->stream_list);
-	INIT_LIST_HEAD(&hsw->tx_list);
-	INIT_LIST_HEAD(&hsw->rx_list);
-	INIT_LIST_HEAD(&hsw->empty_list);
-	init_waitqueue_head(&hsw->boot_wait);
-	init_waitqueue_head(&hsw->wait_txq);
+	ipc = &hsw->ipc;
+	ipc->dev = dev;
+	ipc->ops.tx_msg = hsw_tx_msg;
+	ipc->ops.shim_dbg = hsw_shim_dbg;
+	ipc->ops.tx_data_copy = hsw_tx_data_copy;
+	ipc->ops.reply_msg_match = hsw_reply_msg_match;
 
-	ret = msg_empty_list_init(hsw);
-	if (ret < 0)
-		return -ENOMEM;
-
-	/* start the IPC message thread */
-	init_kthread_worker(&hsw->kworker);
-	hsw->tx_thread = kthread_run(kthread_worker_fn,
-					   &hsw->kworker, "%s",
-					   dev_name(hsw->dev));
-	if (IS_ERR(hsw->tx_thread)) {
-		ret = PTR_ERR(hsw->tx_thread);
-		dev_err(hsw->dev, "error: failed to create message TX

task\n");

sound/soc/intel/haswell/sst-haswell-ipc.c +8 -4
-		goto err_free_msg;
-	}
-	init_kthread_work(&hsw->kwork, ipc_tx_msgs);
+	ret = sst_ipc_init(ipc);
+	if (ret != 0)
+		goto ipc_init_err;
 
+	INIT_LIST_HEAD(&hsw->stream_list);
+	init_waitqueue_head(&hsw->boot_wait);
 	hsw_dev.thread_context = hsw;
 
 	/* init SST shim */
 	hsw->dsp = sst_dsp_new(dev, &hsw_dev, pdata);
 	if (hsw->dsp == NULL) {
 		ret = -ENODEV;
-		goto dsp_err;
+		goto dsp_new_err;
 	}
 
+	ipc->dsp = hsw->dsp;
+
 	/* allocate DMA buffer for context storage */
 	hsw->dx_context = dma_alloc_coherent(hsw->dsp->dma_dev,
 		SST_HSW_DX_CONTEXT_SIZE, &hsw->dx_context_paddr,

GFP_KERNEL); @@ -2404,11 +2198,10 @@ fw_err:

sound/soc/intel/haswell/sst-haswell-ipc.c +5 -7
 			hsw->dx_context, hsw->dx_context_paddr);
 dma_err:
 	sst_dsp_free(hsw->dsp);
-dsp_err:
-	kthread_stop(hsw->tx_thread);
-err_free_msg:
-	kfree(hsw->msg);
-
+dsp_new_err:
+	sst_ipc_fini(ipc);
+ipc_init_err:
+	kfree(hsw);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sst_hsw_dsp_init);
@@ -2422,7 +2215,6 @@ void sst_hsw_dsp_free(struct device *dev, struct
 sst_pdata *pdata)
 	dma_free_coherent(hsw->dsp->dma_dev,
 SST_HSW_DX_CONTEXT_SIZE,
 			hsw->dx_context, hsw->dx_context_paddr);
 	sst_dsp_free(hsw->dsp);
-	kthread_stop(hsw->tx_thread);
-	kfree(hsw->msg);
+	sst_ipc_fini(&hsw->ipc);

}
EXPORT_SYMBOL_GPL(sst_hsw_dsp_free);
--
1.9.1

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

author
#5 lore raw

Re: [PATCH 3/3] ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

Hi Keyon,

Yes, I removed the trace_ipc_reply and trace_ipc_error. Because this is only in original sst-haswell-ipc.c but not in other platforms. And for the trace_ipc_error we have the similar debug log ipc_shim_dbg so finally I decide to remove this trace ipc log.

Thanks
Jin Yao

Quoted 649 lines · Yang
Quoted 131 lines

-----Original Message-----
From: [email protected] [mailto:alsa-devel- [email protected]] On Behalf Of Jin Yao
Sent: Tuesday, April 07, 2015 9:34 AM
To: [email protected]; Girdwood, Liam R
Cc: [email protected]; Jin Yao
Subject: [alsa-devel] [PATCH 3/3] ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

Use the generic IPC/mailbox APIs to replace the original processing code for Broadwell platform.

---
 sound/soc/intel/haswell/sst-haswell-ipc.c | 382 +++++++-----------------------
 1 file changed, 87 insertions(+), 295 deletions(-)
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -80
index 28667d8..d75f09e 100644
@@ -36,6 +36,7 @@
 #include "sst-haswell-ipc.h"
 #include "../common/sst-dsp.h"
 #include "../common/sst-dsp-priv.h"
+#include "../common/sst-ipc.h"
 
 /* Global Message - Generic */
 #define IPC_GLB_TYPE_SHIFT	24
@@ -210,23 +211,6 @@ struct sst_hsw_ipc_fw_ready {
 	u8 fw_info[IPC_MAX_MAILBOX_BYTES - 5 * sizeof(u32)];  }
 __attribute__((packed));
 
-struct ipc_message {
-	struct list_head list;
-	u32 header;
-
-	/* direction wrt host CPU */
-	char tx_data[IPC_MAX_MAILBOX_BYTES];
-	size_t tx_size;
-	char rx_data[IPC_MAX_MAILBOX_BYTES];
-	size_t rx_size;
-
-	wait_queue_head_t waitq;
-	bool pending;
-	bool complete;
-	bool wait;
-	int errno;
-};
-
 struct sst_hsw_stream;
 struct sst_hsw;
 
@@ -325,15 +309,7 @@ struct sst_hsw {
 	bool shutdown;
 
 	/* IPC messaging */
-	struct list_head tx_list;
-	struct list_head rx_list;
-	struct list_head empty_list;
-	wait_queue_head_t wait_txq;
-	struct task_struct *tx_thread;
-	struct kthread_worker kworker;
-	struct kthread_work kwork;
-	bool pending;
-	struct ipc_message *msg;
+	struct sst_generic_ipc ipc;
 
 	/* FW log stream */
 	struct sst_hsw_log_stream log_stream;
@@ -456,159 +432,6 @@ static struct sst_hsw_stream
 *get_stream_by_id(struct sst_hsw *hsw,
 	return NULL;
 }
 
-static void ipc_shim_dbg(struct sst_hsw *hsw, const char *text) -{
-	struct sst_dsp *sst = hsw->dsp;
-	u32 isr, ipcd, imrx, ipcx;
-
-	ipcx = sst_dsp_shim_read_unlocked(sst, SST_IPCX);
-	isr = sst_dsp_shim_read_unlocked(sst, SST_ISRX);
-	ipcd = sst_dsp_shim_read_unlocked(sst, SST_IPCD);
-	imrx = sst_dsp_shim_read_unlocked(sst, SST_IMRX);
-
-	dev_err(hsw->dev, "ipc: --%s-- ipcx 0x%8.8x isr 0x%8.8x ipcd 0x%8.8x
 imrx 0x%8.8x\n",
-		text, ipcx, isr, ipcd, imrx);
-}
-
-/* locks held by caller */
-static struct ipc_message *msg_get_empty(struct sst_hsw *hsw) -{
-	struct ipc_message *msg = NULL;
-
-	if (!list_empty(&hsw->empty_list)) {
-		msg = list_first_entry(&hsw->empty_list, struct ipc_message,
-			list);
-		list_del(&msg->list);
-	}
-
-	return msg;
-}
-
-static void ipc_tx_msgs(struct kthread_work *work) -{
-	struct sst_hsw *hsw =
-		container_of(work, struct sst_hsw, kwork);
-	struct ipc_message *msg;
-	unsigned long flags;
-	u32 ipcx;
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	if (list_empty(&hsw->tx_list) || hsw->pending) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return;
-	}
-
-	/* if the DSP is busy, we will TX messages after IRQ.
-	 * also postpone if we are in the middle of procesing completion irq*/
-	ipcx = sst_dsp_shim_read_unlocked(hsw->dsp, SST_IPCX);
-	if (ipcx & (SST_IPCX_BUSY | SST_IPCX_DONE)) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return;
-	}
-
-	msg = list_first_entry(&hsw->tx_list, struct ipc_message, list);
-
-	list_move(&msg->list, &hsw->rx_list);
-
-	/* send the message */
-	sst_dsp_outbox_write(hsw->dsp, msg->tx_data, msg->tx_size);

sound/soc/intel/haswell/sst-haswell-ipc.c static struct sst_hsw_stream · line 524

Quoted 10 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -9
-	sst_dsp_ipc_msg_tx(hsw->dsp, msg->header | SST_IPCX_BUSY);
-
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-}
-
-/* locks held by caller */
-static void tx_msg_reply_complete(struct sst_hsw *hsw, struct
 ipc_message *msg) -{
-	msg->complete = true;
-	trace_ipc_reply("completed", msg->header);

[Keyon] these trace ipc logs will be removed with your patches applied?

Quoted 12 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -12
-
-	if (!msg->wait)
-		list_add_tail(&msg->list, &hsw->empty_list);
-	else
-		wake_up(&msg->waitq);
-}
-
-static int tx_wait_done(struct sst_hsw *hsw, struct ipc_message *msg,
-	void *rx_data)
-{
-	unsigned long flags;
-	int ret;

sound/soc/intel/haswell/sst-haswell-ipc.c static struct sst_hsw_stream *get_stream_by_id(struct sst_hsw *hsw, · line 547

Quoted 10 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -10
-
-	/* wait for DSP completion (in all cases atm inc pending) */
-	ret = wait_event_timeout(msg->waitq, msg->complete,
-		msecs_to_jiffies(IPC_TIMEOUT_MSECS));
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-	if (ret == 0) {
-		ipc_shim_dbg(hsw, "message timeout");
-
-		trace_ipc_error("error message timeout for", msg->header);

[Keyon] ditto.

Quoted 484 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -85
-		list_del(&msg->list);
-		ret = -ETIMEDOUT;
-	} else {
-
-		/* copy the data returned from DSP */
-		if (msg->rx_size)
-			memcpy(rx_data, msg->rx_data, msg->rx_size);
-		ret = msg->errno;
-	}
-
-	list_add_tail(&msg->list, &hsw->empty_list);
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-	return ret;
-}
-
-static int ipc_tx_message(struct sst_hsw *hsw, u32 header, void *tx_data,
-	size_t tx_bytes, void *rx_data, size_t rx_bytes, int wait)
-{
-	struct ipc_message *msg;
-	unsigned long flags;
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	msg = msg_get_empty(hsw);
-	if (msg == NULL) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return -EBUSY;
-	}
-
-	if (tx_bytes)
-		memcpy(msg->tx_data, tx_data, tx_bytes);
-
-	msg->header = header;
-	msg->tx_size = tx_bytes;
-	msg->rx_size = rx_bytes;
-	msg->wait = wait;
-	msg->errno = 0;
-	msg->pending = false;
-	msg->complete = false;
-
-	list_add_tail(&msg->list, &hsw->tx_list);
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-
-	queue_kthread_work(&hsw->kworker, &hsw->kwork);
-
-	if (wait)
-		return tx_wait_done(hsw, msg, rx_data);
-	else
-		return 0;
-}
-
-static inline int ipc_tx_message_wait(struct sst_hsw *hsw, u32 header,
-	void *tx_data, size_t tx_bytes, void *rx_data, size_t rx_bytes)
-{
-	return ipc_tx_message(hsw, header, tx_data, tx_bytes, rx_data,
-		rx_bytes, 1);
-}
-
-static inline int ipc_tx_message_nowait(struct sst_hsw *hsw, u32 header,
-	void *tx_data, size_t tx_bytes)
-{
-	return ipc_tx_message(hsw, header, tx_data, tx_bytes, NULL, 0, 0);
-}
-
 static void hsw_fw_ready(struct sst_hsw *hsw, u32 header)  {
 	struct sst_hsw_ipc_fw_ready fw_ready;
@@ -696,27 +519,6 @@ static void hsw_notification_work(struct work_struct
 *work)
 	sst_dsp_shim_update_bits(hsw->dsp, SST_IMRX, SST_IMRX_BUSY,
 0);  }
 
-static struct ipc_message *reply_find_msg(struct sst_hsw *hsw, u32 header)
-{
-	struct ipc_message *msg;
-
-	/* clear reply bits & status bits */
-	header &= ~(IPC_STATUS_MASK | IPC_GLB_REPLY_MASK);
-
-	if (list_empty(&hsw->rx_list)) {
-		dev_err(hsw->dev, "error: rx list empty but received
 0x%x\n",
-			header);
-		return NULL;
-	}
-
-	list_for_each_entry(msg, &hsw->rx_list, list) {
-		if (msg->header == header)
-			return msg;
-	}
-
-	return NULL;
-}
-
 static void hsw_stream_update(struct sst_hsw *hsw, struct ipc_message

*msg) {

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 	struct sst_hsw_stream *stream;
@@ -755,7 +557,7 @@ static int hsw_process_reply(struct sst_hsw *hsw,
 u32 header)
 
 	trace_ipc_reply("processing -->", header);
 
-	msg = reply_find_msg(hsw, header);
+	msg = sst_ipc_reply_find_msg(&hsw->ipc, header);
 	if (msg == NULL) {
 		trace_ipc_error("error: can't find message header", header);
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -2
 		return -EIO;
@@ -766,14 +568,14 @@ static int hsw_process_reply(struct sst_hsw *hsw,
 u32 header)
 	case IPC_GLB_REPLY_PENDING:
 		trace_ipc_pending_reply("received", header);
 		msg->pending = true;
-		hsw->pending = true;
+		hsw->ipc.pending = true;
 		return 1;
 	case IPC_GLB_REPLY_SUCCESS:
 		if (msg->pending) {
 			trace_ipc_pending_reply("completed", header);
 			sst_dsp_inbox_read(hsw->dsp, msg->rx_data,
 				msg->rx_size);
-			hsw->pending = false;
+			hsw->ipc.pending = false;
 		} else {
 			/* copy data from the DSP */

sst_dsp_outbox_read(hsw->dsp, msg->rx_data, @@
-829,7 +631,7 @@ static int hsw_process_reply(struct sst_hsw *hsw, u32 header)

/* wake up and return the error if we have waiters on this message ?
*/

sound/soc/intel/haswell/sst-haswell-ipc.c +3 -2
 	list_del(&msg->list);
-	tx_msg_reply_complete(hsw, msg);
+	sst_ipc_tx_msg_reply_complete(&hsw->ipc, msg);
 
 	return 1;
 }
@@ -970,6 +772,7 @@ static irqreturn_t hsw_irq_thread(int irq, void
 *context)  {
 	struct sst_dsp *sst = (struct sst_dsp *) context;
 	struct sst_hsw *hsw = sst_dsp_get_thread_context(sst);
+	struct sst_generic_ipc *ipc = &hsw->ipc;
 	u32 ipcx, ipcd;
 	int handled;
 	unsigned long flags;
@@ -1016,7 +819,7 @@ static irqreturn_t hsw_irq_thread(int irq, void
 *context)
 	spin_unlock_irqrestore(&sst->spinlock, flags);
 
 	/* continue to send any remaining messages... */
-	queue_kthread_work(&hsw->kworker, &hsw->kwork);
+	queue_kthread_work(&ipc->kworker, &ipc->kwork);
 
 	return IRQ_HANDLED;

}

sound/soc/intel/haswell/sst-haswell-ipc.c +2 -1
@@ -1026,7 +829,8 @@ int sst_hsw_fw_get_version(struct sst_hsw *hsw,  {
 	int ret;
 
-	ret = ipc_tx_message_wait(hsw,
 IPC_GLB_TYPE(IPC_GLB_GET_FW_VERSION),
+	ret = sst_ipc_tx_message_wait(&hsw->ipc,
+		IPC_GLB_TYPE(IPC_GLB_GET_FW_VERSION),
 		NULL, 0, version, sizeof(*version));
 	if (ret < 0)
 		dev_err(hsw->dev, "error: get version failed\n"); @@ -

1090,7 +894,8 @@ int sst_hsw_stream_set_volume(struct sst_hsw *hsw,

sound/soc/intel/haswell/sst-haswell-ipc.c +4 -2
 		req->channel = channel;
 	}
 
-	ret = ipc_tx_message_wait(hsw, header, req, sizeof(*req), NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, req,
+		sizeof(*req), NULL, 0);
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: set stream volume failed\n");
 		return ret;
@@ -1155,7 +960,8 @@ int sst_hsw_mixer_set_volume(struct sst_hsw *hsw,
 u32 stage_id, u32 channel,
 	req.curve_type = hsw->curve_type;
 	req.target_volume = volume;
 
-	ret = ipc_tx_message_wait(hsw, header, &req, sizeof(req), NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &req,
+		sizeof(req), NULL, 0);
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: set mixer volume failed\n");
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 		return ret;
@@ -1213,7 +1019,7 @@ int sst_hsw_stream_free(struct sst_hsw *hsw,
 struct sst_hsw_stream *stream)
 	stream->free_req.stream_id = stream->reply.stream_hw_id;
 	header = IPC_GLB_TYPE(IPC_GLB_FREE_STREAM);
 
-	ret = ipc_tx_message_wait(hsw, header, &stream->free_req,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &stream-
 > free_req,
 		sizeof(stream->free_req), NULL, 0);
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -0
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: free stream %d failed\n", @@ -

1405,8 +1211,8 @@ int sst_hsw_stream_commit(struct sst_hsw *hsw, struct sst_hsw_stream *stream)

sound/soc/intel/haswell/sst-haswell-ipc.c +4 -3
 	header = IPC_GLB_TYPE(IPC_GLB_ALLOCATE_STREAM);
 
-	ret = ipc_tx_message_wait(hsw, header, str_req, sizeof(*str_req),
-		reply, sizeof(*reply));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, str_req,
+		sizeof(*str_req), reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: stream commit failed\n");
 		return ret;
@@ -1455,7 +1261,8 @@ int sst_hsw_mixer_get_info(struct sst_hsw *hsw)
 
 	trace_ipc_request("get global mixer info", 0);
 
-	ret = ipc_tx_message_wait(hsw, header, NULL, 0, reply,
 sizeof(*reply));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, NULL, 0,
+		reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: get stream info failed\n");
sound/soc/intel/haswell/sst-haswell-ipc.c +3 -2
 		return ret;
@@ -1476,9 +1283,10 @@ static int sst_hsw_stream_operations(struct
 sst_hsw *hsw, int type,
 	header |= (stream_id << IPC_STR_ID_SHIFT);
 
 	if (wait)
-		return ipc_tx_message_wait(hsw, header, NULL, 0, NULL, 0);
+		return sst_ipc_tx_message_wait(&hsw->ipc, header,
+			NULL, 0, NULL, 0);
 	else
-		return ipc_tx_message_nowait(hsw, header, NULL, 0);
+		return sst_ipc_tx_message_nowait(&hsw->ipc, header,
 NULL, 0);
 }

/* Stream ALSA trigger operations */

sound/soc/intel/haswell/sst-haswell-ipc.c +4 -4
@@ -1605,8 +1413,8 @@ int sst_hsw_device_set_config(struct sst_hsw
 *hsw,
 
 	header = IPC_GLB_TYPE(IPC_GLB_SET_DEVICE_FORMATS);
 
-	ret = ipc_tx_message_wait(hsw, header, &config, sizeof(config),
-		NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &config,
+		sizeof(config), NULL, 0);
 	if (ret < 0)
 		dev_err(hsw->dev, "error: set device formats failed\n");
@@ -1626,8 +1434,8 @@ int sst_hsw_dx_set_state(struct sst_hsw *hsw,
 
 	trace_ipc_request("PM enter Dx state", state);
 
-	ret = ipc_tx_message_wait(hsw, header, &state_, sizeof(state_),
-		dx, sizeof(*dx));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &state_,
+		sizeof(state_), dx, sizeof(*dx));
 	if (ret < 0) {
 		dev_err(hsw->dev, "ipc: error set dx state %d failed\n",
 state);
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -26
 		return ret;
@@ -1770,32 +1578,6 @@ static int sst_hsw_dx_state_restore(struct
 sst_hsw *hsw)
 	return 0;
 }
 
-static void sst_hsw_drop_all(struct sst_hsw *hsw) -{
-	struct ipc_message *msg, *tmp;
-	unsigned long flags;
-	int tx_drop_cnt = 0, rx_drop_cnt = 0;
-
-	/* drop all TX and Rx messages before we stall + reset DSP */
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	list_for_each_entry_safe(msg, tmp, &hsw->tx_list, list) {
-		list_move(&msg->list, &hsw->empty_list);
-		tx_drop_cnt++;
-	}
-
-	list_for_each_entry_safe(msg, tmp, &hsw->rx_list, list) {
-		list_move(&msg->list, &hsw->empty_list);
-		rx_drop_cnt++;
-	}
-
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-
-	if (tx_drop_cnt || rx_drop_cnt)
-		dev_err(hsw->dev, "dropped IPC msg RX=%d, TX=%d\n",
-			tx_drop_cnt, rx_drop_cnt);
-}
-
 int sst_hsw_dsp_load(struct sst_hsw *hsw)  {
 	struct sst_dsp *dsp = hsw->dsp;
@@ -1875,7 +1657,7 @@ int sst_hsw_dsp_runtime_suspend(struct sst_hsw
 *hsw)
 	if (ret < 0)
 		return ret;
 
-	sst_hsw_drop_all(hsw);
+	sst_ipc_drop_all(&hsw->ipc);
 
 	return 0;

}

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -17
@@ -1933,23 +1715,6 @@ int sst_hsw_dsp_runtime_resume(struct sst_hsw
 *hsw)  }  #endif
 
-static int msg_empty_list_init(struct sst_hsw *hsw) -{
-	int i;
-
-	hsw->msg = kzalloc(sizeof(struct ipc_message) *
-		IPC_EMPTY_LIST_SIZE, GFP_KERNEL);
-	if (hsw->msg == NULL)
-		return -ENOMEM;
-
-	for (i = 0; i < IPC_EMPTY_LIST_SIZE; i++) {
-		init_waitqueue_head(&hsw->msg[i].waitq);
-		list_add(&hsw->msg[i].list, &hsw->empty_list);
-	}
-
-	return 0;
-}
-
 struct sst_dsp *sst_hsw_get_dsp(struct sst_hsw *hsw)  {
 	return hsw->dsp;
@@ -2184,7 +1949,7 @@ int sst_hsw_module_enable(struct sst_hsw *hsw,
 		config.scratch_mem.size, config.scratch_mem.offset,
 		config.map.module_entries[0].entry_point);
 
-	ret = ipc_tx_message_wait(hsw, header,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,
 			&config, sizeof(config), NULL, 0);
 	if (ret < 0)
 		dev_err(dev, "ipc: module enable failed - %d\n", ret); @@ -

2223,7 +1988,7 @@ int sst_hsw_module_disable(struct sst_hsw *hsw,

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 			IPC_MODULE_OPERATION(IPC_MODULE_DISABLE) |
 			IPC_MODULE_ID(module_id);
 
-	ret = ipc_tx_message_wait(hsw, header,  NULL, 0, NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,  NULL, 0, NULL,

0);

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 	if (ret < 0)
 		dev_err(dev, "module disable failed - %d\n", ret);
 	else
@@ -2277,7 +2042,7 @@ int sst_hsw_module_set_param(struct sst_hsw
 *hsw,
 	parameter->parameter_id = parameter_id;
 	parameter->data_size = param_size;
 
-	ret = ipc_tx_message_wait(hsw, header,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,
 		parameter, transfer_parameter_size , NULL, 0);
 	if (ret < 0)
sound/soc/intel/haswell/sst-haswell-ipc.c +35 -0
 		dev_err(dev, "ipc: module set parameter failed - %d\n", ret);
@@ -2296,10 +2061,48 @@ static struct sst_dsp_device hsw_dev = {
 	.ops = &haswell_ops,
 };
 
+static void hsw_tx_msg(struct sst_generic_ipc *ipc, struct ipc_message
+*msg) {
+	/* send the message */
+	sst_dsp_outbox_write(ipc->dsp, msg->tx_data, msg->tx_size);
+	sst_dsp_ipc_msg_tx(ipc->dsp, msg->header); }
+
+static void hsw_shim_dbg(struct sst_generic_ipc *ipc, const char *text)
+{
+	struct sst_dsp *sst = ipc->dsp;
+	u32 isr, ipcd, imrx, ipcx;
+
+	ipcx = sst_dsp_shim_read_unlocked(sst, SST_IPCX);
+	isr = sst_dsp_shim_read_unlocked(sst, SST_ISRX);
+	ipcd = sst_dsp_shim_read_unlocked(sst, SST_IPCD);
+	imrx = sst_dsp_shim_read_unlocked(sst, SST_IMRX);
+
+	dev_err(ipc->dev,
+		"ipc: --%s-- ipcx 0x%8.8x isr 0x%8.8x ipcd 0x%8.8x imrx
 0x%8.8x\n",
+		text, ipcx, isr, ipcd, imrx);
+}
+
+static void hsw_tx_data_copy(struct ipc_message *msg, char *tx_data,
+	size_t tx_size)
+{
+	memcpy(msg->tx_data, tx_data, tx_size); }
+
+static u64 hsw_reply_msg_match(u64 header, u64 *mask) {
+	/* clear reply bits & status bits */
+	header &= ~(IPC_STATUS_MASK | IPC_GLB_REPLY_MASK);
+	*mask = (u64)-1;
+
+	return header;
+}
+
 int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)  {
 	struct sst_hsw_ipc_fw_version version;
 	struct sst_hsw *hsw;
+	struct sst_generic_ipc *ipc;
 	int ret;
 
 	dev_dbg(dev, "initialising Audio DSP IPC\n"); @@ -2308,39 +2111,30

@@ int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)

sound/soc/intel/haswell/sst-haswell-ipc.c +6 -19
 	if (hsw == NULL)
 		return -ENOMEM;
 
-	hsw->dev = dev;
-	INIT_LIST_HEAD(&hsw->stream_list);
-	INIT_LIST_HEAD(&hsw->tx_list);
-	INIT_LIST_HEAD(&hsw->rx_list);
-	INIT_LIST_HEAD(&hsw->empty_list);
-	init_waitqueue_head(&hsw->boot_wait);
-	init_waitqueue_head(&hsw->wait_txq);
+	ipc = &hsw->ipc;
+	ipc->dev = dev;
+	ipc->ops.tx_msg = hsw_tx_msg;
+	ipc->ops.shim_dbg = hsw_shim_dbg;
+	ipc->ops.tx_data_copy = hsw_tx_data_copy;
+	ipc->ops.reply_msg_match = hsw_reply_msg_match;
 
-	ret = msg_empty_list_init(hsw);
-	if (ret < 0)
-		return -ENOMEM;
-
-	/* start the IPC message thread */
-	init_kthread_worker(&hsw->kworker);
-	hsw->tx_thread = kthread_run(kthread_worker_fn,
-					   &hsw->kworker, "%s",
-					   dev_name(hsw->dev));
-	if (IS_ERR(hsw->tx_thread)) {
-		ret = PTR_ERR(hsw->tx_thread);
-		dev_err(hsw->dev, "error: failed to create message TX

task\n");

sound/soc/intel/haswell/sst-haswell-ipc.c +8 -4
-		goto err_free_msg;
-	}
-	init_kthread_work(&hsw->kwork, ipc_tx_msgs);
+	ret = sst_ipc_init(ipc);
+	if (ret != 0)
+		goto ipc_init_err;
 
+	INIT_LIST_HEAD(&hsw->stream_list);
+	init_waitqueue_head(&hsw->boot_wait);
 	hsw_dev.thread_context = hsw;
 
 	/* init SST shim */
 	hsw->dsp = sst_dsp_new(dev, &hsw_dev, pdata);
 	if (hsw->dsp == NULL) {
 		ret = -ENODEV;
-		goto dsp_err;
+		goto dsp_new_err;
 	}
 
+	ipc->dsp = hsw->dsp;
+
 	/* allocate DMA buffer for context storage */
 	hsw->dx_context = dma_alloc_coherent(hsw->dsp->dma_dev,
 		SST_HSW_DX_CONTEXT_SIZE, &hsw->dx_context_paddr,

GFP_KERNEL); @@ -2404,11 +2198,10 @@ fw_err:

sound/soc/intel/haswell/sst-haswell-ipc.c +5 -7
 			hsw->dx_context, hsw->dx_context_paddr);
 dma_err:
 	sst_dsp_free(hsw->dsp);
-dsp_err:
-	kthread_stop(hsw->tx_thread);
-err_free_msg:
-	kfree(hsw->msg);
-
+dsp_new_err:
+	sst_ipc_fini(ipc);
+ipc_init_err:
+	kfree(hsw);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sst_hsw_dsp_init);
@@ -2422,7 +2215,6 @@ void sst_hsw_dsp_free(struct device *dev, struct
 sst_pdata *pdata)
 	dma_free_coherent(hsw->dsp->dma_dev,
 SST_HSW_DX_CONTEXT_SIZE,
 			hsw->dx_context, hsw->dx_context_paddr);
 	sst_dsp_free(hsw->dsp);
-	kthread_stop(hsw->tx_thread);
-	kfree(hsw->msg);
+	sst_ipc_fini(&hsw->ipc);

}
EXPORT_SYMBOL_GPL(sst_hsw_dsp_free);
--
1.9.1

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Acked-by
#6 lore raw

Re: [PATCH 3/3] ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

Quoted 8 lines

-----Original Message-----
From: Jin, Yao [mailto:[email protected]]
Sent: Tuesday, April 07, 2015 8:07 PM
To: Jie, Yang; [email protected]; Girdwood, Liam R
Cc: [email protected]
Subject: Re: [alsa-devel] [PATCH 3/3] ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

Quoted 6 lines

Hi Keyon,

Yes, I removed the trace_ipc_reply and trace_ipc_error. Because this is only in original sst-haswell-ipc.c but not in other platforms. And for the trace_ipc_error we have the similar debug log ipc_shim_dbg so finally I decide to remove this trace ipc log.

It will only reduce some trace info(no trace_ipc_reply "completed" again) for Broadwell, anyway, we still have trace_ipc_reply "processing---", so, that's fine for me.

Quoted 668 lines

Thanks
Jin Yao

Quoted 70 lines · Yang
Quoted 70 lines

-----Original Message-----
From: [email protected] [mailto:alsa-devel- [email protected]] On Behalf Of Jin Yao
Sent: Tuesday, April 07, 2015 9:34 AM
To: [email protected]; Girdwood, Liam R
Cc: [email protected]; Jin Yao
Subject: [alsa-devel] [PATCH 3/3] ASoC: Intel: Use the generic IPC/mailbox APIs in Broadwell

Use the generic IPC/mailbox APIs to replace the original processing code for Broadwell platform.

---
 sound/soc/intel/haswell/sst-haswell-ipc.c | 382
+++++++-----------------------
 1 file changed, 87 insertions(+), 295 deletions(-)
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -26
index 28667d8..d75f09e 100644
@@ -36,6 +36,7 @@
 #include "sst-haswell-ipc.h"
 #include "../common/sst-dsp.h"
 #include "../common/sst-dsp-priv.h"
+#include "../common/sst-ipc.h"
 
 /* Global Message - Generic */
 #define IPC_GLB_TYPE_SHIFT	24
@@ -210,23 +211,6 @@ struct sst_hsw_ipc_fw_ready {
 	u8 fw_info[IPC_MAX_MAILBOX_BYTES - 5 * sizeof(u32)];  }
 __attribute__((packed));
 
-struct ipc_message {
-	struct list_head list;
-	u32 header;
-
-	/* direction wrt host CPU */
-	char tx_data[IPC_MAX_MAILBOX_BYTES];
-	size_t tx_size;
-	char rx_data[IPC_MAX_MAILBOX_BYTES];
-	size_t rx_size;
-
-	wait_queue_head_t waitq;
-	bool pending;
-	bool complete;
-	bool wait;
-	int errno;
-};
-
 struct sst_hsw_stream;
 struct sst_hsw;
 
@@ -325,15 +309,7 @@ struct sst_hsw {
 	bool shutdown;
 
 	/* IPC messaging */
-	struct list_head tx_list;
-	struct list_head rx_list;
-	struct list_head empty_list;
-	wait_queue_head_t wait_txq;
-	struct task_struct *tx_thread;
-	struct kthread_worker kworker;
-	struct kthread_work kwork;
-	bool pending;
-	struct ipc_message *msg;
+	struct sst_generic_ipc ipc;
 
Quoted 2 lines

sound/soc/intel/haswell/sst-haswell-ipc.c struct sst_hsw { · line 315

Quoted 2 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -0
 	/* FW log stream */
 	struct sst_hsw_log_stream log_stream; @@ -456,159 +432,6 @@

static

Quoted 93 lines
Quoted 67 lines

struct sst_hsw_stream *get_stream_by_id(struct sst_hsw *hsw,

sound/soc/intel/haswell/sst-haswell-ipc.c +0 -10
 	return NULL;
 }
 
-static void ipc_shim_dbg(struct sst_hsw *hsw, const char *text) -{
-	struct sst_dsp *sst = hsw->dsp;
-	u32 isr, ipcd, imrx, ipcx;
-
-	ipcx = sst_dsp_shim_read_unlocked(sst, SST_IPCX);
-	isr = sst_dsp_shim_read_unlocked(sst, SST_ISRX);
-	ipcd = sst_dsp_shim_read_unlocked(sst, SST_IPCD);
-	imrx = sst_dsp_shim_read_unlocked(sst, SST_IMRX);
-
-	dev_err(hsw->dev, "ipc: --%s-- ipcx 0x%8.8x isr 0x%8.8x ipcd 0x%8.8x

imrx 0x%8.8x\n",

sound/soc/intel/haswell/sst-haswell-ipc.c +0 -51
-		text, ipcx, isr, ipcd, imrx);
-}
-
-/* locks held by caller */
-static struct ipc_message *msg_get_empty(struct sst_hsw *hsw) -{
-	struct ipc_message *msg = NULL;
-
-	if (!list_empty(&hsw->empty_list)) {
-		msg = list_first_entry(&hsw->empty_list, struct ipc_message,
-			list);
-		list_del(&msg->list);
-	}
-
-	return msg;
-}
-
-static void ipc_tx_msgs(struct kthread_work *work) -{
-	struct sst_hsw *hsw =
-		container_of(work, struct sst_hsw, kwork);
-	struct ipc_message *msg;
-	unsigned long flags;
-	u32 ipcx;
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	if (list_empty(&hsw->tx_list) || hsw->pending) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return;
-	}
-
-	/* if the DSP is busy, we will TX messages after IRQ.
-	 * also postpone if we are in the middle of procesing completion irq*/
-	ipcx = sst_dsp_shim_read_unlocked(hsw->dsp, SST_IPCX);
-	if (ipcx & (SST_IPCX_BUSY | SST_IPCX_DONE)) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return;
-	}
-
-	msg = list_first_entry(&hsw->tx_list, struct ipc_message, list);
-
-	list_move(&msg->list, &hsw->rx_list);
-
-	/* send the message */
-	sst_dsp_outbox_write(hsw->dsp, msg->tx_data, msg->tx_size);
-	sst_dsp_ipc_msg_tx(hsw->dsp, msg->header | SST_IPCX_BUSY);
-
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-}
-
-/* locks held by caller */
-static void tx_msg_reply_complete(struct sst_hsw *hsw, struct

ipc_message *msg) -{

sound/soc/intel/haswell/sst-haswell-ipc.c static struct sst_hsw_stream *get_stream_by_id(struct sst_hsw *hsw, · line 525

Quoted 2 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -2
-	msg->complete = true;
-	trace_ipc_reply("completed", msg->header);

[Keyon] these trace ipc logs will be removed with your patches applied?

Quoted 12 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -12
-
-	if (!msg->wait)
-		list_add_tail(&msg->list, &hsw->empty_list);
-	else
-		wake_up(&msg->waitq);
-}
-
-static int tx_wait_done(struct sst_hsw *hsw, struct ipc_message *msg,
-	void *rx_data)
-{
-	unsigned long flags;
-	int ret;

sound/soc/intel/haswell/sst-haswell-ipc.c static struct sst_hsw_stream *get_stream_by_id(struct sst_hsw *hsw, · line 547

Quoted 10 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -10
-
-	/* wait for DSP completion (in all cases atm inc pending) */
-	ret = wait_event_timeout(msg->waitq, msg->complete,
-		msecs_to_jiffies(IPC_TIMEOUT_MSECS));
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-	if (ret == 0) {
-		ipc_shim_dbg(hsw, "message timeout");
-
-		trace_ipc_error("error message timeout for", msg->header);

[Keyon] ditto.

Quoted 16 lines
Quoted 6 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -6
-		list_del(&msg->list);
-		ret = -ETIMEDOUT;
-	} else {
-
-		/* copy the data returned from DSP */
-		if (msg->rx_size)

sound/soc/intel/haswell/sst-haswell-ipc.c static struct sst_hsw_stream *get_stream_by_id(struct sst_hsw *hsw,

Quoted 10 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -10
-			memcpy(rx_data, msg->rx_data, msg->rx_size);
-		ret = msg->errno;
-	}
-
-	list_add_tail(&msg->list, &hsw->empty_list);
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-	return ret;
-}
-
-static int ipc_tx_message(struct sst_hsw *hsw, u32 header, void

*tx_data,

Quoted 42 lines
Quoted 42 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -42
-	size_t tx_bytes, void *rx_data, size_t rx_bytes, int wait)
-{
-	struct ipc_message *msg;
-	unsigned long flags;
-
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	msg = msg_get_empty(hsw);
-	if (msg == NULL) {
-		spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-		return -EBUSY;
-	}
-
-	if (tx_bytes)
-		memcpy(msg->tx_data, tx_data, tx_bytes);
-
-	msg->header = header;
-	msg->tx_size = tx_bytes;
-	msg->rx_size = rx_bytes;
-	msg->wait = wait;
-	msg->errno = 0;
-	msg->pending = false;
-	msg->complete = false;
-
-	list_add_tail(&msg->list, &hsw->tx_list);
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-
-	queue_kthread_work(&hsw->kworker, &hsw->kwork);
-
-	if (wait)
-		return tx_wait_done(hsw, msg, rx_data);
-	else
-		return 0;
-}
-
-static inline int ipc_tx_message_wait(struct sst_hsw *hsw, u32 header,
-	void *tx_data, size_t tx_bytes, void *rx_data, size_t rx_bytes)
-{
-	return ipc_tx_message(hsw, header, tx_data, tx_bytes, rx_data,
-		rx_bytes, 1);
-}
-
Quoted 8 lines

sound/soc/intel/haswell/sst-haswell-ipc.c static struct sst_hsw_stream *get_stream_by_id(struct sst_hsw *hsw,

Quoted 8 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -6
-static inline int ipc_tx_message_nowait(struct sst_hsw *hsw, u32 header,
-	void *tx_data, size_t tx_bytes)
-{
-	return ipc_tx_message(hsw, header, tx_data, tx_bytes, NULL, 0, 0);
-}
-
 static void hsw_fw_ready(struct sst_hsw *hsw, u32 header)  {
 	struct sst_hsw_ipc_fw_ready fw_ready; @@ -696,27 +519,6 @@

static

Quoted 3 lines
Quoted 3 lines

void hsw_notification_work(struct work_struct
*work)
sst_dsp_shim_update_bits(hsw->dsp, SST_IMRX, SST_IMRX_BUSY,

0); }

Quoted 80 lines
Quoted 80 lines

-static struct ipc_message *reply_find_msg(struct sst_hsw *hsw, u32 header) -{

sound/soc/intel/haswell/sst-haswell-ipc.c +0 -7
-	struct ipc_message *msg;
-
-	/* clear reply bits & status bits */
-	header &= ~(IPC_STATUS_MASK | IPC_GLB_REPLY_MASK);
-
-	if (list_empty(&hsw->rx_list)) {
-		dev_err(hsw->dev, "error: rx list empty but received

0x%x\n",

sound/soc/intel/haswell/sst-haswell-ipc.c +0 -12
-			header);
-		return NULL;
-	}
-
-	list_for_each_entry(msg, &hsw->rx_list, list) {
-		if (msg->header == header)
-			return msg;
-	}
-
-	return NULL;
-}
-
 static void hsw_stream_update(struct sst_hsw *hsw, struct

ipc_message
*msg) {

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 	struct sst_hsw_stream *stream;
@@ -755,7 +557,7 @@ static int hsw_process_reply(struct sst_hsw *hsw,
 u32 header)
 
 	trace_ipc_reply("processing -->", header);
 
-	msg = reply_find_msg(hsw, header);
+	msg = sst_ipc_reply_find_msg(&hsw->ipc, header);
 	if (msg == NULL) {
 		trace_ipc_error("error: can't find message header", header);
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -2
 		return -EIO;
@@ -766,14 +568,14 @@ static int hsw_process_reply(struct sst_hsw
 *hsw,
 u32 header)
 	case IPC_GLB_REPLY_PENDING:
 		trace_ipc_pending_reply("received", header);
 		msg->pending = true;
-		hsw->pending = true;
+		hsw->ipc.pending = true;
 		return 1;
 	case IPC_GLB_REPLY_SUCCESS:
 		if (msg->pending) {
 			trace_ipc_pending_reply("completed", header);
 			sst_dsp_inbox_read(hsw->dsp, msg->rx_data,
 				msg->rx_size);
-			hsw->pending = false;
+			hsw->ipc.pending = false;
 		} else {
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -1
 			/* copy data from the DSP */
 			sst_dsp_outbox_read(hsw->dsp, msg->rx_data, @@
-829,7 +631,7 @@ static int hsw_process_reply(struct sst_hsw *hsw,

u32
header)

/* wake up and return the error if we have waiters on this message ?
*/

sound/soc/intel/haswell/sst-haswell-ipc.c +2 -1
 	list_del(&msg->list);
-	tx_msg_reply_complete(hsw, msg);
+	sst_ipc_tx_msg_reply_complete(&hsw->ipc, msg);
 
 	return 1;
 }
@@ -970,6 +772,7 @@ static irqreturn_t hsw_irq_thread(int irq, void
 *context)  {
 	struct sst_dsp *sst = (struct sst_dsp *) context;
 	struct sst_hsw *hsw = sst_dsp_get_thread_context(sst);
+	struct sst_generic_ipc *ipc = &hsw->ipc;
 	u32 ipcx, ipcd;
 	int handled;
 	unsigned long flags;
@@ -1016,7 +819,7 @@ static irqreturn_t hsw_irq_thread(int irq, void
 *context)
 	spin_unlock_irqrestore(&sst->spinlock, flags);
 
Quoted 7 lines

sound/soc/intel/haswell/sst-haswell-ipc.c int sst_hsw_fw_get_version(struct sst_hsw *hsw,

Quoted 7 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 	/* continue to send any remaining messages... */
-	queue_kthread_work(&hsw->kworker, &hsw->kwork);
+	queue_kthread_work(&ipc->kworker, &ipc->kwork);
 
 	return IRQ_HANDLED;

}

sound/soc/intel/haswell/sst-haswell-ipc.c +0 -0
@@ -1026,7 +829,8 @@ int sst_hsw_fw_get_version(struct sst_hsw *hsw,

{

Quoted 47 lines
Quoted 47 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -1
 	int ret;
 
-	ret = ipc_tx_message_wait(hsw,
 IPC_GLB_TYPE(IPC_GLB_GET_FW_VERSION),
+	ret = sst_ipc_tx_message_wait(&hsw->ipc,
+		IPC_GLB_TYPE(IPC_GLB_GET_FW_VERSION),
 		NULL, 0, version, sizeof(*version));
 	if (ret < 0)
 		dev_err(hsw->dev, "error: get version failed\n"); @@ -

1090,7 +894,8 @@ int sst_hsw_stream_set_volume(struct sst_hsw *hsw,

sound/soc/intel/haswell/sst-haswell-ipc.c +4 -2
 		req->channel = channel;
 	}
 
-	ret = ipc_tx_message_wait(hsw, header, req, sizeof(*req), NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, req,
+		sizeof(*req), NULL, 0);
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: set stream volume failed\n");
 		return ret;
@@ -1155,7 +960,8 @@ int sst_hsw_mixer_set_volume(struct sst_hsw
 *hsw,
 u32 stage_id, u32 channel,
 	req.curve_type = hsw->curve_type;
 	req.target_volume = volume;
 
-	ret = ipc_tx_message_wait(hsw, header, &req, sizeof(req), NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &req,
+		sizeof(req), NULL, 0);
 	if (ret < 0) {
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 		dev_err(hsw->dev, "error: set mixer volume failed\n");
 		return ret;
@@ -1213,7 +1019,7 @@ int sst_hsw_stream_free(struct sst_hsw *hsw,
 struct sst_hsw_stream *stream)
 	stream->free_req.stream_id = stream->reply.stream_hw_id;
 	header = IPC_GLB_TYPE(IPC_GLB_FREE_STREAM);
 
-	ret = ipc_tx_message_wait(hsw, header, &stream->free_req,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &stream-
 > free_req,
 		sizeof(stream->free_req), NULL, 0);
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -0
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: free stream %d failed\n", @@ -

1405,8 +1211,8 @@ int sst_hsw_stream_commit(struct sst_hsw *hsw, struct sst_hsw_stream *stream)

header = IPC_GLB_TYPE(IPC_GLB_ALLOCATE_STREAM);

Quoted 8 lines

sound/soc/intel/haswell/sst-haswell-ipc.c int sst_hsw_mixer_get_info(struct sst_hsw

Quoted 8 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -2
-	ret = ipc_tx_message_wait(hsw, header, str_req, sizeof(*str_req),
-		reply, sizeof(*reply));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, str_req,
+		sizeof(*str_req), reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: stream commit failed\n");
 		return ret;
@@ -1455,7 +1261,8 @@ int sst_hsw_mixer_get_info(struct sst_hsw

*hsw)

Quoted 38 lines
Quoted 38 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +5 -3
 	trace_ipc_request("get global mixer info", 0);
 
-	ret = ipc_tx_message_wait(hsw, header, NULL, 0, reply,
 sizeof(*reply));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, NULL, 0,
+		reply, sizeof(*reply));
 	if (ret < 0) {
 		dev_err(hsw->dev, "error: get stream info failed\n");
 		return ret;
@@ -1476,9 +1283,10 @@ static int sst_hsw_stream_operations(struct
 sst_hsw *hsw, int type,
 	header |= (stream_id << IPC_STR_ID_SHIFT);
 
 	if (wait)
-		return ipc_tx_message_wait(hsw, header, NULL, 0, NULL, 0);
+		return sst_ipc_tx_message_wait(&hsw->ipc, header,
+			NULL, 0, NULL, 0);
 	else
-		return ipc_tx_message_nowait(hsw, header, NULL, 0);
+		return sst_ipc_tx_message_nowait(&hsw->ipc, header,
 NULL, 0);
 }

/* Stream ALSA trigger operations */ @@ -1605,8 +1413,8 @@ int
sst_hsw_device_set_config(struct sst_hsw *hsw,

sound/soc/intel/haswell/sst-haswell-ipc.c +2 -2
 	header = IPC_GLB_TYPE(IPC_GLB_SET_DEVICE_FORMATS);
 
-	ret = ipc_tx_message_wait(hsw, header, &config, sizeof(config),
-		NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &config,
+		sizeof(config), NULL, 0);
 	if (ret < 0)
 		dev_err(hsw->dev, "error: set device formats failed\n");
@@ -1626,8 +1434,8 @@ int sst_hsw_dx_set_state(struct sst_hsw *hsw,
 
Quoted 8 lines

sound/soc/intel/haswell/sst-haswell-ipc.c int sst_hsw_dx_set_state(struct sst_hsw *hsw,

Quoted 8 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -2
 	trace_ipc_request("PM enter Dx state", state);
 
-	ret = ipc_tx_message_wait(hsw, header, &state_, sizeof(state_),
-		dx, sizeof(*dx));
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header, &state_,
+		sizeof(state_), dx, sizeof(*dx));
 	if (ret < 0) {
 		dev_err(hsw->dev, "ipc: error set dx state %d failed\n",

state);

Quoted 26 lines
Quoted 26 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -20
 		return ret;
@@ -1770,32 +1578,6 @@ static int sst_hsw_dx_state_restore(struct
 sst_hsw *hsw)
 	return 0;
 }
 
-static void sst_hsw_drop_all(struct sst_hsw *hsw) -{
-	struct ipc_message *msg, *tmp;
-	unsigned long flags;
-	int tx_drop_cnt = 0, rx_drop_cnt = 0;
-
-	/* drop all TX and Rx messages before we stall + reset DSP */
-	spin_lock_irqsave(&hsw->dsp->spinlock, flags);
-
-	list_for_each_entry_safe(msg, tmp, &hsw->tx_list, list) {
-		list_move(&msg->list, &hsw->empty_list);
-		tx_drop_cnt++;
-	}
-
-	list_for_each_entry_safe(msg, tmp, &hsw->rx_list, list) {
-		list_move(&msg->list, &hsw->empty_list);
-		rx_drop_cnt++;
-	}
-
-	spin_unlock_irqrestore(&hsw->dsp->spinlock, flags);
-
Quoted 8 lines

sound/soc/intel/haswell/sst-haswell-ipc.c int sst_hsw_dsp_runtime_suspend(struct

Quoted 8 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -5
-	if (tx_drop_cnt || rx_drop_cnt)
-		dev_err(hsw->dev, "dropped IPC msg RX=%d, TX=%d\n",
-			tx_drop_cnt, rx_drop_cnt);
-}
-
 int sst_hsw_dsp_load(struct sst_hsw *hsw)  {
 	struct sst_dsp *dsp = hsw->dsp;
@@ -1875,7 +1657,7 @@ int sst_hsw_dsp_runtime_suspend(struct

sst_hsw

Quoted 10 lines

sound/soc/intel/haswell/sst-haswell-ipc.c int sst_hsw_dsp_runtime_resume(struct

Quoted 10 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 *hsw)
 	if (ret < 0)
 		return ret;
 
-	sst_hsw_drop_all(hsw);
+	sst_ipc_drop_all(&hsw->ipc);
 
 	return 0;

}

sound/soc/intel/haswell/sst-haswell-ipc.c +0 -0
@@ -1933,23 +1715,6 @@ int sst_hsw_dsp_runtime_resume(struct

sst_hsw

Quoted 13 lines
Quoted 13 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -11
 *hsw)  }  #endif
 
-static int msg_empty_list_init(struct sst_hsw *hsw) -{
-	int i;
-
-	hsw->msg = kzalloc(sizeof(struct ipc_message) *
-		IPC_EMPTY_LIST_SIZE, GFP_KERNEL);
-	if (hsw->msg == NULL)
-		return -ENOMEM;
-
-	for (i = 0; i < IPC_EMPTY_LIST_SIZE; i++) {
-		init_waitqueue_head(&hsw->msg[i].waitq);
-		list_add(&hsw->msg[i].list, &hsw->empty_list);
Quoted 8 lines

sound/soc/intel/haswell/sst-haswell-ipc.c int sst_hsw_module_enable(struct sst_hsw

Quoted 8 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +0 -5
-	}
-
-	return 0;
-}
-
 struct sst_dsp *sst_hsw_get_dsp(struct sst_hsw *hsw)  {
 	return hsw->dsp;
@@ -2184,7 +1949,7 @@ int sst_hsw_module_enable(struct sst_hsw

*hsw,

Quoted 23 lines
Quoted 23 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 		config.scratch_mem.size, config.scratch_mem.offset,
 		config.map.module_entries[0].entry_point);
 
-	ret = ipc_tx_message_wait(hsw, header,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,
 			&config, sizeof(config), NULL, 0);
 	if (ret < 0)
 		dev_err(dev, "ipc: module enable failed - %d\n", ret); @@ -

2223,7 +1988,7 @@ int sst_hsw_module_disable(struct sst_hsw *hsw,

sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
 			IPC_MODULE_OPERATION(IPC_MODULE_DISABLE) |
 			IPC_MODULE_ID(module_id);
 
-	ret = ipc_tx_message_wait(hsw, header,  NULL, 0, NULL, 0);
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,  NULL, 0, NULL,

0);

sound/soc/intel/haswell/sst-haswell-ipc.c +0 -0
 	if (ret < 0)
 		dev_err(dev, "module disable failed - %d\n", ret);
 	else
@@ -2277,7 +2042,7 @@ int sst_hsw_module_set_param(struct sst_hsw
 *hsw,
 	parameter->parameter_id = parameter_id;
 	parameter->data_size = param_size;
 
Quoted 5 lines
Quoted 5 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +1 -1
-	ret = ipc_tx_message_wait(hsw, header,
+	ret = sst_ipc_tx_message_wait(&hsw->ipc, header,
 		parameter, transfer_parameter_size , NULL, 0);
 	if (ret < 0)

dev_err(dev, "ipc: module set parameter failed - %d\n", ret);

@@

Quoted 97 lines
Quoted 97 lines

-2296,10 +2061,48 @@ static struct sst_dsp_device hsw_dev = {

sound/soc/intel/haswell/sst-haswell-ipc.c +19 -0
 	.ops = &haswell_ops,
 };
 
+static void hsw_tx_msg(struct sst_generic_ipc *ipc, struct
+ipc_message
+*msg) {
+	/* send the message */
+	sst_dsp_outbox_write(ipc->dsp, msg->tx_data, msg->tx_size);
+	sst_dsp_ipc_msg_tx(ipc->dsp, msg->header); }
+
+static void hsw_shim_dbg(struct sst_generic_ipc *ipc, const char
+*text) {
+	struct sst_dsp *sst = ipc->dsp;
+	u32 isr, ipcd, imrx, ipcx;
+
+	ipcx = sst_dsp_shim_read_unlocked(sst, SST_IPCX);
+	isr = sst_dsp_shim_read_unlocked(sst, SST_ISRX);
+	ipcd = sst_dsp_shim_read_unlocked(sst, SST_IPCD);
+	imrx = sst_dsp_shim_read_unlocked(sst, SST_IMRX);
+
+	dev_err(ipc->dev,
+		"ipc: --%s-- ipcx 0x%8.8x isr 0x%8.8x ipcd 0x%8.8x imrx

0x%8.8x\n",

sound/soc/intel/haswell/sst-haswell-ipc.c +17 -0
+		text, ipcx, isr, ipcd, imrx);
+}
+
+static void hsw_tx_data_copy(struct ipc_message *msg, char *tx_data,
+	size_t tx_size)
+{
+	memcpy(msg->tx_data, tx_data, tx_size); }
+
+static u64 hsw_reply_msg_match(u64 header, u64 *mask) {
+	/* clear reply bits & status bits */
+	header &= ~(IPC_STATUS_MASK | IPC_GLB_REPLY_MASK);
+	*mask = (u64)-1;
+
+	return header;
+}
+
 int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)  {
 	struct sst_hsw_ipc_fw_version version;
 	struct sst_hsw *hsw;
+	struct sst_generic_ipc *ipc;
 	int ret;
 
 	dev_dbg(dev, "initialising Audio DSP IPC\n"); @@ -2308,39 +2111,30

@@ int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)

sound/soc/intel/haswell/sst-haswell-ipc.c +6 -19
 	if (hsw == NULL)
 		return -ENOMEM;
 
-	hsw->dev = dev;
-	INIT_LIST_HEAD(&hsw->stream_list);
-	INIT_LIST_HEAD(&hsw->tx_list);
-	INIT_LIST_HEAD(&hsw->rx_list);
-	INIT_LIST_HEAD(&hsw->empty_list);
-	init_waitqueue_head(&hsw->boot_wait);
-	init_waitqueue_head(&hsw->wait_txq);
+	ipc = &hsw->ipc;
+	ipc->dev = dev;
+	ipc->ops.tx_msg = hsw_tx_msg;
+	ipc->ops.shim_dbg = hsw_shim_dbg;
+	ipc->ops.tx_data_copy = hsw_tx_data_copy;
+	ipc->ops.reply_msg_match = hsw_reply_msg_match;
 
-	ret = msg_empty_list_init(hsw);
-	if (ret < 0)
-		return -ENOMEM;
-
-	/* start the IPC message thread */
-	init_kthread_worker(&hsw->kworker);
-	hsw->tx_thread = kthread_run(kthread_worker_fn,
-					   &hsw->kworker, "%s",
-					   dev_name(hsw->dev));
-	if (IS_ERR(hsw->tx_thread)) {
-		ret = PTR_ERR(hsw->tx_thread);
-		dev_err(hsw->dev, "error: failed to create message TX

task\n");

sound/soc/intel/haswell/sst-haswell-ipc.c +6 -4
-		goto err_free_msg;
-	}
-	init_kthread_work(&hsw->kwork, ipc_tx_msgs);
+	ret = sst_ipc_init(ipc);
+	if (ret != 0)
+		goto ipc_init_err;
 
+	INIT_LIST_HEAD(&hsw->stream_list);
+	init_waitqueue_head(&hsw->boot_wait);
 	hsw_dev.thread_context = hsw;
 
 	/* init SST shim */
 	hsw->dsp = sst_dsp_new(dev, &hsw_dev, pdata);
 	if (hsw->dsp == NULL) {
 		ret = -ENODEV;
-		goto dsp_err;
+		goto dsp_new_err;
 	}
Quoted 5 lines

sound/soc/intel/haswell/sst-haswell-ipc.c int sst_hsw_dsp_init(struct device *dev, struct sst_pdata *pdata)

Quoted 5 lines
sound/soc/intel/haswell/sst-haswell-ipc.c +2 -0
+	ipc->dsp = hsw->dsp;
+
 	/* allocate DMA buffer for context storage */
 	hsw->dx_context = dma_alloc_coherent(hsw->dsp->dma_dev,
 		SST_HSW_DX_CONTEXT_SIZE, &hsw->dx_context_paddr,

GFP_KERNEL); @@

Quoted 34 lines
Quoted 34 lines

-2404,11 +2198,10 @@ fw_err:

sound/soc/intel/haswell/sst-haswell-ipc.c +5 -7
 			hsw->dx_context, hsw->dx_context_paddr);
 dma_err:
 	sst_dsp_free(hsw->dsp);
-dsp_err:
-	kthread_stop(hsw->tx_thread);
-err_free_msg:
-	kfree(hsw->msg);
-
+dsp_new_err:
+	sst_ipc_fini(ipc);
+ipc_init_err:
+	kfree(hsw);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sst_hsw_dsp_init);
@@ -2422,7 +2215,6 @@ void sst_hsw_dsp_free(struct device *dev,
 struct sst_pdata *pdata)
 	dma_free_coherent(hsw->dsp->dma_dev,
 SST_HSW_DX_CONTEXT_SIZE,
 			hsw->dx_context, hsw->dx_context_paddr);
 	sst_dsp_free(hsw->dsp);
-	kthread_stop(hsw->tx_thread);
-	kfree(hsw->msg);
+	sst_ipc_fini(&hsw->ipc);

}
EXPORT_SYMBOL_GPL(sst_hsw_dsp_free);
--
1.9.1

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

maintainer Applied
#7 lore raw
In reply to Jin Yao
Quoted 4 lines · Jin Yao

Hi Mark,

Currently in Intel SST driver, some very similar or duplicated IPC/mailbox processing code are used in different platforms.

Applied all, thanks.