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

[Pierre-Louis Bossart]

On 12/1/17 2:19 AM, yan.wang at linux.intel.com wrote:
> 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.

Can you explain under what circumstances there would be duplicate 
messages? is this a feature that both host and DSP can use or an error case?

> 2. Add msg_find() to search duplicate message.
> 3. If replace flag is true, search and replace duplicate message.

Who sets this flag and why?

> 4. For the message of host offset of DMA trace, enable replace logic.
> 
> If there is un-sent IPC message of host offset of DMA trace in host message
> queue, update it with the latest host offset directly instead of appending one
> new IPC message.

that seems to be a different feature intended at optimizing the number 
of IPC messages?

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