[Sound-open-firmware] [PATCH] Add support to replace stale stream/trace position updates.

[Jie, Yang]

>-----Original Message-----
>From: sound-open-firmware-bounces at alsa-project.org [mailto:sound-open-
>firmware-bounces at alsa-project.org] On Behalf Of yan.wang at linux.intel.com
>Sent: Thursday, November 23, 2017 10:48 AM
>To: sound-open-firmware at alsa-project.org
>Cc: Yan Wang <yan.wang at linux.intel.com>
>Subject: [Sound-open-firmware] [PATCH] Add support to replace stale
>stream/trace position updates.
>
>From: Yan Wang <yan.wang at linux.intel.com>
>
>1. Add "replace" parameter for ipc_queue_host_message() to indicate whether
>check duplicate message in host message queue.
>2. Add msg_find() to search duplicate message.
>3. If replace flag is true, search and replace duplicate message.
>4. For the message of host offset of DMA trace, enable replace logic.
>
>It will avoid that the host message queue is full because there is too many IPC
>messages of host offset of DMA trace.

Does the "duplicate" message are true duplicated? IMO, we should not send
duplicate message except that it is critical/urgent and no respond for it from
host side.

If it is meaning that the DMA trace sending too much messages(with too high
frequency), we should reduce the frequency, instead of adding message 
replacing logic which makes it more complicate and error-prone. 

Thanks,
~Keyon

>
>Signed-off-by: Yan Wang <yan.wang at linux.intel.com>
>---
> src/include/reef/ipc.h |  2 +-
> src/ipc/intel-ipc.c    | 45 ++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 35 insertions(+), 12 deletions(-)
>
>diff --git a/src/include/reef/ipc.h b/src/include/reef/ipc.h index
>fcab45b..bb814be 100644
>--- a/src/include/reef/ipc.h
>+++ b/src/include/reef/ipc.h
>@@ -125,7 +125,7 @@ int ipc_stream_send_xrun(struct comp_dev *cdev,
>
> int ipc_queue_host_message(struct ipc *ipc, uint32_t header,
> 	void *tx_data, size_t tx_bytes, void *rx_data,
>-	size_t rx_bytes, void (*cb)(void*, void*), void *cb_data);
>+	size_t rx_bytes, void (*cb)(void*, void*), void *cb_data, uint32_t
>+replace);
> int ipc_send_short_msg(uint32_t msg);
>
> void ipc_platform_do_cmd(struct ipc *ipc); diff --git a/src/ipc/intel-ipc.c
>b/src/ipc/intel-ipc.c index e13b541..391907e 100644
>--- a/src/ipc/intel-ipc.c
>+++ b/src/ipc/intel-ipc.c
>@@ -367,7 +367,7 @@ int ipc_stream_send_position(struct comp_dev *cdev,
> 	posn->comp_id = cdev->comp.id;
>
> 	return ipc_queue_host_message(_ipc, posn->rhdr.hdr.cmd, posn,
>-		sizeof(*posn), NULL, 0, NULL, NULL);
>+		sizeof(*posn), NULL, 0, NULL, NULL, 0);
> }
>
> /* send stream position TODO: send compound message  */ @@ -379,7 +379,7
>@@ int ipc_stream_send_xrun(struct comp_dev *cdev,
> 	posn->comp_id = cdev->comp.id;
>
> 	return ipc_queue_host_message(_ipc, posn->rhdr.hdr.cmd, posn,
>-		sizeof(*posn), NULL, 0, NULL, NULL);
>+		sizeof(*posn), NULL, 0, NULL, NULL, 0);
> }
>
> static int ipc_stream_trigger(uint32_t header) @@ -644,7 +644,7 @@ int
>ipc_dma_trace_send_position(void)
> 	posn.rhdr.hdr.size = sizeof(posn);
>
> 	return ipc_queue_host_message(_ipc, posn.rhdr.hdr.cmd, &posn,
>-		sizeof(posn), NULL, 0, NULL, NULL);
>+		sizeof(posn), NULL, 0, NULL, NULL, 1);
> }
>
> static int ipc_glb_debug_message(uint32_t header) @@ -899,19 +899,40 @@
>static inline struct ipc_msg *msg_get_empty(struct ipc *ipc)
> 	return msg;
> }
>
>+static inline struct ipc_msg *msg_find(struct ipc *ipc, uint32_t
>+header) {
>+	struct list_item *plist;
>+	struct ipc_msg *msg = NULL;
>+
>+	list_for_item(plist, &ipc->msg_list) {
>+		msg = container_of(plist, struct ipc_msg, list);
>+		if (msg->header == header)
>+			return msg;
>+	}
>+
>+	return NULL;
>+}
>
> int ipc_queue_host_message(struct ipc *ipc, uint32_t header,
> 	void *tx_data, size_t tx_bytes, void *rx_data,
>-	size_t rx_bytes, void (*cb)(void*, void*), void *cb_data)
>+	size_t rx_bytes, void (*cb)(void*, void*), void *cb_data, uint32_t
>+replace)
> {
>-	struct ipc_msg *msg;
>-	uint32_t flags;
>+	struct ipc_msg *msg = NULL;
>+	uint32_t flags, found = 0;
> 	int ret = 0;
>
> 	spin_lock_irq(&ipc->lock, flags);
>
>-	/* get a free message */
>-	msg = msg_get_empty(ipc);
>+	/* do we need to replace an existing message? */
>+	if (replace)
>+		msg = msg_find(ipc, header);
>+
>+	/* do we need to use a new empty message? */
>+	if (msg)
>+		found = 1;
>+	else
>+		msg = msg_get_empty(ipc);
>+
> 	if (msg == NULL) {
> 		trace_ipc_error("eQb");
> 		ret = -EBUSY;
>@@ -929,9 +950,11 @@ int ipc_queue_host_message(struct ipc *ipc, uint32_t
>header,
> 	if (tx_bytes > 0 && tx_bytes < SOF_IPC_MSG_MAX_SIZE)
> 		rmemcpy(msg->tx_data, tx_data, tx_bytes);
>
>-	/* now queue the message */
>-	ipc->dsp_pending = 1;
>-	list_item_append(&msg->list, &ipc->msg_list);
>+	if (!found) {
>+		/* now queue the message */
>+		ipc->dsp_pending = 1;
>+		list_item_append(&msg->list, &ipc->msg_list);
>+	}
>
> out:
> 	spin_unlock_irq(&ipc->lock, flags);
>--
>2.7.4
>
>_______________________________________________
>Sound-open-firmware mailing list
>Sound-open-firmware at alsa-project.org
>http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware