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

[Liam Girdwood]

On Tue, 2017-12-05 at 18:40 +0800, yan.wang at linux.intel.com wrote:
> From: Yan Wang <yan.wang at linux.intel.com>
> 
> Host message queue is long sometimes. So when one new IPC message
> like 
> DMA trace host offset is pushed into, the previous same type IPC
> meesage may
> haven't been sent. It is better to update the previous same type IPC
> message
> instead of sending 2 messages because it not only reduces the waiting
> number of 
> host message queue but also send the latest information as soon as
> possible.
> 
> The following is details of implementation:
> 1. Add "replace" parameter for ipc_queue_host_message() to indicate
> whether
> check duplicate message in host message queue which is decided by
> sender.
> 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.
> 
> 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, 1);
>  }
>  
>  /* 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, 1);
>  }
>  
>  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)

I think we also need to check struct sof_ipc_stream_posn.comp_id here
too and check if the comp IDs match.

I think all the calls to this func are passing a struct
sof_ipc_stream_pos as the data, but we may need a simple switch
statement that checks cmd type and determines the correct msg_find() to
use (as future users of this feature may send different structures as
data).

Liam

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