[Sound-open-firmware] [PATCH 3/3] ipc: replace ipc position update with memory window

Liam Girdwood liam.r.girdwood at linux.intel.com
Thu Feb 22 17:09:37 CET 2018


On Mon, 2018-02-12 at 14:20 +0800, Xiuli Pan wrote:
> From: Pan Xiuli <xiuli.pan at linux.intel.com>
> 
> Add memeory window for stream region. And position offset binding for
> component. Replace IPC position update with memory window update.
> 
> Signed-off-by: Pan Xiuli <xiuli.pan at linux.intel.com>
> 
> ---
> Test with:
> Mininow max rt5651
> SOF master: 019637ab250daa53c15da0a0a98c54f1c58d8ca3
> SOF-Tool master: 33e4b0cc6f6a44e3e7ee849c04c515a5537242c7
> https://github.com/plbossart/sound/tree/topic/sof-v4.14:
> 6fa721a8b7c6567eea0a2181bf9a3d2a12c31b00
> ---
>  src/include/reef/audio/pipeline.h |  4 ++++
>  src/include/reef/ipc.h            |  5 +++++
>  src/include/reef/mailbox.h        |  7 +++++++
>  src/ipc/intel-ipc.c               | 43 +++++++++++++++++++++++++++++----------
>  src/ipc/ipc.c                     | 26 +++++++++++++++++++++++
>  5 files changed, 74 insertions(+), 11 deletions(-)
> 
> diff --git a/src/include/reef/audio/pipeline.h b/src/include/reef/audio/pipeline.h
> index 08a3f30..3207c7f 100644
> --- a/src/include/reef/audio/pipeline.h
> +++ b/src/include/reef/audio/pipeline.h
> @@ -69,6 +69,10 @@ struct pipeline {
>  	struct task pipe_task;		/* pipeline processing task */
>  	struct comp_dev *sched_comp;	/* component that drives scheduling in this pipe */
>  	struct comp_dev *source_comp;	/* source component for this pipe */
> +
> +	/* position update */
> +	uint32_t posn_index;		/* position update array index */
> +	uint32_t posn_offset;		/* position update array offset*/


why do we need offset ? surely we access this as an array on both host
and DSP ? (as the structure is in the IPC UAPI)

>  };
>  
>  /* static pipeline */
> diff --git a/src/include/reef/ipc.h b/src/include/reef/ipc.h
> index bb814be..8b6c049 100644
> --- a/src/include/reef/ipc.h
> +++ b/src/include/reef/ipc.h
> @@ -103,6 +103,9 @@ struct ipc {
>  	/* DMA for Trace*/
>  	struct dma_trace_data dmat;
>  
> +	/* mmap for posn_offset */
> +	void *posn_map[PLATFORM_MAX_STREAMS];

Should this be a pointer to our IPC pos structure rather than void * ?

> +
>  	void *private;
>  };
>  
> @@ -169,4 +172,6 @@ int ipc_comp_dai_config(struct ipc *ipc, struct sof_ipc_dai_config *config);
>  /* send DMA trace host buffer position to host */
>  int ipc_dma_trace_send_position(void);
>  
> +/* get posn offset by pipeline. */
> +int ipc_get_posn_offset(struct ipc *ipc, struct pipeline *pipe);
>  #endif
> diff --git a/src/include/reef/mailbox.h b/src/include/reef/mailbox.h
> index c0580a9..f134548 100644
> --- a/src/include/reef/mailbox.h
> +++ b/src/include/reef/mailbox.h
> @@ -78,4 +78,11 @@
>  	dcache_invalidate_region((void*)(MAILBOX_HOSTBOX_BASE + src), bytes); \
>  	rmemcpy(dest, (void*)(MAILBOX_HOSTBOX_BASE + src), bytes);
>  
> +#define mailbox_stream_write(dest, src, bytes) \
> +	do { \
> +		rmemcpy((void *)(MAILBOX_STREAM_BASE + dest), src, bytes); \
> +		dcache_writeback_region((void *)(MAILBOX_STREAM_BASE + dest), \
> +					bytes); \
> +	} while (0)
> +
>  #endif
> diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c
> index 54a0812..9d3cf2d 100644
> --- a/src/ipc/intel-ipc.c
> +++ b/src/ipc/intel-ipc.c
> @@ -236,7 +236,7 @@ static int ipc_stream_pcm_params(uint32_t stream)
>  	struct sof_ipc_pcm_params_reply reply;
>  	struct ipc_comp_dev *pcm_dev;
>  	struct comp_dev *cd;
> -	int err;
> +	int err, posn_offset;
>  
>  	trace_ipc("SAl");
>  
> @@ -290,13 +290,17 @@ static int ipc_stream_pcm_params(uint32_t stream)
>  		goto error;
>  	}
>  
> -
> +	posn_offset = ipc_get_posn_offset(_ipc, pcm_dev->cd->pipeline);
> +	if (posn_offset < 0) {
> +		trace_ipc_error("eAo");
> +		goto error;
> +	}
>  	/* write component values to the outbox */
>  	reply.rhdr.hdr.size = sizeof(reply);
>  	reply.rhdr.hdr.cmd = stream;
>  	reply.rhdr.error = 0;
>  	reply.comp_id = pcm_params->comp_id;
> -	reply.posn_offset = 0; /* TODO: set this up for mmaped components */
> +	reply.posn_offset = posn_offset;

we can rename this to posn_index.

>  	mailbox_hostbox_write(0, &reply, sizeof(reply));
>  	return 1;
>  
> @@ -352,15 +356,18 @@ static int ipc_stream_position(uint32_t header)
>  	}
>  
>  	/* set message fields - TODO; get others */
> -	posn.rhdr.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_POSITION;
> +	posn.rhdr.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_POSITION |
> +			    stream->comp_id;
>  	posn.rhdr.hdr.size = sizeof(posn);
>  	posn.comp_id = stream->comp_id;
>  
>  	/* get the stream positions and timestamps */
>  	pipeline_get_timestamp(pcm_dev->cd->pipeline, pcm_dev->cd, &posn);
>  
> -	/* copy positions to outbox */
> -	mailbox_hostbox_write(0, &posn, sizeof(posn));
> +	/* copy positions to stream region */
> +	mailbox_stream_write(pcm_dev->cd->pipeline->posn_offset,
> +			     &posn, sizeof(posn));
> +

This stream copy is not needed as the pipeline should copy this data on
every schedule. That way host can read it async.

>  	return 1;
>  }
>  
> @@ -368,24 +375,38 @@ static int ipc_stream_position(uint32_t header)
>  int ipc_stream_send_position(struct comp_dev *cdev,
>  	struct sof_ipc_stream_posn *posn)
>  {
> -	posn->rhdr.hdr.cmd =  SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_POSITION;
> +	struct sof_ipc_hdr hdr;
> +
> +	tracev_ipc("Pos");
> +	posn->rhdr.hdr.cmd =  SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_POSITION |
> +			      cdev->comp.id;
>  	posn->rhdr.hdr.size = sizeof(*posn);
>  	posn->comp_id = cdev->comp.id;
>  
> -	return ipc_queue_host_message(_ipc, posn->rhdr.hdr.cmd, posn,
> -		sizeof(*posn), NULL, 0, NULL, NULL, 1);
> +	hdr.cmd = posn->rhdr.hdr.cmd;
> +	hdr.size = sizeof(hdr);
> +
> +	mailbox_stream_write(cdev->pipeline->posn_offset, posn, sizeof(*posn));
> +	return ipc_queue_host_message(_ipc, posn->rhdr.hdr.cmd, &hdr,
> +				      sizeof(hdr), NULL, 0, NULL, NULL, 0);
>  }
>  
>  /* send stream position TODO: send compound message  */
>  int ipc_stream_send_xrun(struct comp_dev *cdev,
>  	struct sof_ipc_stream_posn *posn)
>  {
> +	struct sof_ipc_hdr hdr;
> +
>  	posn->rhdr.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_TRIG_XRUN;
>  	posn->rhdr.hdr.size = sizeof(*posn);
>  	posn->comp_id = cdev->comp.id;
>  
> -	return ipc_queue_host_message(_ipc, posn->rhdr.hdr.cmd, posn,
> -		sizeof(*posn), NULL, 0, NULL, NULL, 1);
> +	hdr.cmd = posn->rhdr.hdr.cmd;
> +	hdr.size = sizeof(hdr);
> +
> +	mailbox_stream_write(cdev->pipeline->posn_offset, posn, sizeof(*posn));
> +	return ipc_queue_host_message(_ipc, posn->rhdr.hdr.cmd, &hdr,
> +				      sizeof(hdr), NULL, 0, NULL, NULL, 0);
>  }
>  
>  static int ipc_stream_trigger(uint32_t header)
> diff --git a/src/ipc/ipc.c b/src/ipc/ipc.c
> index 61b0cf2..f18f7d2 100644
> --- a/src/ipc/ipc.c
> +++ b/src/ipc/ipc.c
> @@ -77,6 +77,28 @@ struct ipc_comp_dev *ipc_get_comp(struct ipc *ipc, uint32_t id)
>  	return NULL;
>  }
>  
> +int ipc_get_posn_offset(struct ipc *ipc, struct pipeline *pipe)
> +{
> +	int i;
> +	uint32_t posn_size = sizeof(struct sof_ipc_stream_posn);
> +
> +	for (i = 0; i < PLATFORM_MAX_STREAMS; i++) {
> +		if (ipc->posn_map[i] == pipe && pipe->posn_index == i)
> +			return pipe->posn_offset;
> +	}
> +
> +	for (i = 0; i < PLATFORM_MAX_STREAMS; i++) {
> +		if (ipc->posn_map[i] == NULL) {
> +			ipc->posn_map[i] = pipe;
> +			pipe->posn_index = i;
> +			pipe->posn_offset = i * posn_size;
> +			return pipe->posn_offset;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  int ipc_comp_new(struct ipc *ipc, struct sof_ipc_comp *comp)
>  {
>  	struct comp_dev *cd;
> @@ -345,12 +367,16 @@ int ipc_comp_dai_config(struct ipc *ipc, struct sof_ipc_dai_config *config)
>  
>  int ipc_init(struct reef *reef)
>  {
> +	int i;
>  	trace_ipc("IPI");
>  
>  	/* init ipc data */
>  	reef->ipc = rzalloc(RZONE_SYS, RFLAGS_NONE, sizeof(*reef->ipc));
>  	reef->ipc->comp_data = rzalloc(RZONE_SYS, RFLAGS_NONE, SOF_IPC_MSG_MAX_SIZE);
>  
> +	for (i = 0; i < PLATFORM_MAX_STREAMS; i++)
> +		reef->ipc->posn_map[i] = NULL;
> +
>  	list_init(&reef->ipc->comp_list);
>  
>  	return platform_ipc_init(reef->ipc);


More information about the Sound-open-firmware mailing list