[Sound-open-firmware] [PATCH 3/3] ipc: replace ipc position update with memory window
Liam Girdwood
liam.r.girdwood at linux.intel.com
Fri Feb 23 09:18:04 CET 2018
On Fri, 2018-02-23 at 03:31 +0000, Pan, Xiuli wrote:
> > 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)
>
> The offset is keep align with the old uapi. And help to check if the offset is
> validate in ASOC.
No need to support old uapi from 0.95.
>
> > > };
> > >
> > > /* 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 * ?
>
> Yes. will fix.
>
> > > +
> > > 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.
>
> This will change uapi for ASOC and SOF, and lose chance to check
> if offset is validate.
This is fine.
Liam
>
> > > 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.
>
> OK, I will check when we need to update this whole structure or just need to
> update some filed.
>
> Will send a v2 patch set after test on BYT and APL.
>
> Thanks
> Xiuli
>
> > > 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