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

[yanwang]

On Thu, 2017-11-23 at 03:07 +0000, Jie, Yang wrote:
> > 
> > -----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.inte
> > l.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

Host offset of DMA trace need be updated after DMA transfer completes.
This patch wants solve:
If previous update message of host offset hasn't been sent because of
many the other messages, set the latest host offset for it.
"duplicate" may cause some misunderstanding, I could add more
description into commit message.
Thanks.

Yan Wang

> 
> > 
> > 
> > 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-firmwar
> > e
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware