[Sound-open-firmware] [PATCH 4/5][RFC] APL/CNL: Implement host DMA setting and position update on APL/CNL.

[yanwang]

On Tue, 2018-03-13 at 10:53 +0000, Liam Girdwood wrote:
> On Tue, 2018-03-13 at 18:16 +0800, yan.wang at linux.intel.com wrote:
> > 
> > From: Yan Wang <yan.wang at linux.intel.com>
> > 
> > The host DMA has the different config method:
> > 1. Get DMA channel based on stream tag.
> > 2. Prepare DMA elem list based on local DMA trace buffer which make
> > sure that one trace record data can be transferred at least.
> > 3. Need start DMA on firmware side before trigger start on kernel
> > side.
> > 4. Because no DMA callback in host DMA, add position update
> > calling at the end of trace_work().
> > 
> > Signed-off-by: Yan Wang <yan.wang at linux.intel.com>
> > ---
> >  src/lib/dma-trace.c | 73
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> > 
> > diff --git a/src/lib/dma-trace.c b/src/lib/dma-trace.c
> > index b08e7fb..51e413c 100644
> > --- a/src/lib/dma-trace.c
> > +++ b/src/lib/dma-trace.c
> > @@ -120,6 +120,10 @@ out:
> >  
> >  	spin_unlock_irq(&d->lock, flags);
> >  
> > +#if defined CONFIG_DMA_GW
> > +	ipc_dma_trace_send_position();
> > +#endif
> 
> Why do we only send trace position to host with GW DMA ? If there is
> a valid
> reason then we need to comment that reason next to the code.

Because GW DMA hasn't dma completion callback for doing this.
I will add comments for this.

> 
> 
> > 
> > +
> >  	/* reschedule the trace copying work */
> >  	return DMA_TRACE_PERIOD;
> >  }
> > @@ -163,7 +167,11 @@ int dma_trace_init_complete(struct
> > dma_trace_data *d)
> >  	trace_buffer("dtn");
> >  
> >  	/* init DMA copy context */
> > +#if defined CONFIG_DMA_GW
> > +	ret = dma_copy_new(&d->dc, DMA_HOST_IN_DMAC);
> > +#else
> >  	ret = dma_copy_new(&d->dc, PLATFORM_TRACE_DMAC);
> > +#endif
> 
> Why not define PLATFORM_TRACE_DMAC as DMA_HOST_IN_DMAC in platform.h
> for APL and
> CNL ?

sure.

> 
> > 
> >  	if (ret < 0) {
> >  		trace_buffer_error("edm");
> >  		rfree(buffer->addr);
> > @@ -195,8 +203,73 @@ int dma_trace_host_buffer(struct
> > dma_trace_data *d,
> > struct dma_sg_elem *elem,
> >  	return 0;
> >  }
> >  
> > +#if defined CONFIG_DMA_GW
> > +
> > +static int dma_trace_start(struct dma_trace_data *d)
> > +{
> > +	struct dma_sg_config config;
> > +	struct dma_sg_elem *e;
> > +	uint32_t elem_size, elem_addr, elem_num;
> > +	int err = 0;
> > +	int i;
> > +
> > +	err = dma_set_channel(&d->dc, d->stream_tag);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	/* size of every trace record */
> > +	elem_size = sizeof(uint64_t) * 2;
> > +
> > +	/* Initialize address of local elem */
> > +	elem_addr = (uint32_t)d->dmatb.addr;
> > +
> > +	/* the number of elem list */
> > +	elem_num = DMA_TRACE_LOCAL_SIZE / elem_size;
> > +
> > +	config.direction = DMA_DIR_LMEM_TO_HMEM;
> > +	config.src_width = sizeof(uint32_t);
> > +	config.dest_width = sizeof(uint32_t);
> > +	config.cyclic = 0;
> > +	list_init(&config.elem_list);
> > +
> > +	/* generate local elem list for local trace buffer */
> > +	e = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*e) *
> > elem_num);
> > +	if (!e)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < elem_num; i++) {
> > +		e[i].dest = 0;
> > +		e[i].src = elem_addr;
> > +		e[i].size = elem_size; /* the minimum size of DMA
> > copy */
> > +
> > +		list_item_append(&e[i].list, &config.elem_list);
> > +		elem_addr += elem_size;
> > +	}
> > +
> > +	err = dma_set_config(d->dc.dmac, d->dc.chan, &config);
> > +	if (err < 0) {
> > +		rfree(e);
> > +		return err;
> > +	}
> > +
> > +	err = dma_start(d->dc.dmac, d->dc.chan);
> > +
> > +	rfree(e);
> > +	return err;
> > +}
> > +
> > +#endif
> > +
> >  int dma_trace_enable(struct dma_trace_data *d)
> >  {
> > +#if defined CONFIG_DMA_GW
> > +	int err;
> > +
> > +	err = dma_trace_start(d);
> > +	if (err < 0)
> > +		return err;
> > +#endif
> 
> Can you comment why we start trace for GW DMA here.

Yes.
Thanks.

Yan Wang

> 
> > 
> > +
> >  	/* validate DMA context */
> >  	if (d->dc.dmac == NULL || d->dc.chan < 0) {
> >  		trace_error_atomic(TRACE_CLASS_BUFFER, "eem");
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ
> VAT No: 860 2173 47
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware