[Sound-open-firmware] [RFC PATCH 1/6] ipc: skip host page table set up for hostless tone pipeline

[Ranjani Sridharan]

On Thu, 2018-06-14 at 14:38 +0800, zhigangw wrote:
> 
> On 2018年06月14日 14:13, Ranjani Sridharan wrote:
> > On Thu, 2018-06-14 at 14:08 +0800, zhigangw wrote:
> > > On 2018年06月14日 11:29, Ranjani Sridharan wrote:
> > > > Hostless pipelines such as the tone pipeline do not need
> > > > host page table while setting up pcm params. Walk pipeline
> > > > in both directions to ensure it is hostless.
> > > > 
> > > > Signed-off-by: Ranjani Sridharan <ranjani.sridharan at linux.intel
> > > > .com
> > > > ---
> > > >    src/ipc/handler.c | 76
> > > > ++++++++++++++++++++++++++++++++++++++++++++++-
> > > >    1 file changed, 75 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/ipc/handler.c b/src/ipc/handler.c
> > > > index 581ce9e..8c46cc2 100644
> > > > --- a/src/ipc/handler.c
> > > > +++ b/src/ipc/handler.c
> > > > @@ -34,6 +34,7 @@
> > > >     *
> > > >     */
> > > >    
> > > > +#include <stdbool.h>
> > > >    #include <sof/debug.h>
> > > >    #include <sof/timer.h>
> > > >    #include <sof/interrupt.h>
> > > > @@ -83,6 +84,71 @@ static inline struct sof_ipc_hdr
> > > > *mailbox_validate(void)
> > > >    	return hdr;
> > > >    }
> > > >    
> > > > +/* check if a pipeline is hostless when walking downstream */
> > > > +static bool is_hostless_downstream(struct comp_dev *current)
> > > > +{
> > > > +	struct list_item *clist;
> > > > +
> > > > +	/* check if current is a HOST comp */
> > > > +	if (current->comp.type == SOF_COMP_HOST ||
> > > > +	    current->comp.type == SOF_COMP_SG_HOST)
> > > > +		return false;
> > > > +
> > > > +	/* check if the pipeline has a HOST comp downstream */
> > > > +	list_for_item(clist, &current->bsink_list) {
> > > > +		struct comp_buffer *buffer;
> > > > +
> > > > +		buffer = container_of(clist, struct
> > > > comp_buffer,
> > > > source_list);
> > > > +
> > > > +		/* don't go downstream if this component is
> > > > not
> > > > connected */
> > > > +		if (!buffer->connected)
> > > > +			continue;
> > > > +
> > > > +		/* dont go downstream if this comp belongs to
> > > > another pipe */
> > > > +		if (buffer->sink->comp.pipeline_id != current-
> > > > > comp.pipeline_id)
> > > > 
> > > > +			continue;
> > > > +
> > > > +		/* return if there's a host comp downstream */
> > > > +		if (!is_hostless_downstream(buffer->sink))
> > > > +			return false;
> > > > +	}
> > > > +
> > > > +	return true;
> > > > +}
> > > > +
> > > > +/* check if a pipeline is hostless when walking upstream */
> > > > +static bool is_hostless_upstream(struct comp_dev *current)
> > > > +{
> > > > +	struct list_item *clist;
> > > > +
> > > > +	/* check if current is a HOST comp */
> > > > +	if (current->comp.type == SOF_COMP_HOST ||
> > > > +	    current->comp.type == SOF_COMP_SG_HOST)
> > > > +		return false;
> > > > +
> > > > +	/* check if the pipeline has a HOST comp upstream */
> > > > +	list_for_item(clist, &current->bsource_list) {
> > > > +		struct comp_buffer *buffer;
> > > > +
> > > > +		buffer = container_of(clist, struct
> > > > comp_buffer,
> > > > source_list);
> > > 
> > > I think it should be:
> > > buffer = container_of(clist, struct comp_buffer, sink_list);
> > 
> > Oh yes, Thanks Zhigang. The dangers of copy & paste :)
> 
> there is another location which has the similarly issue;
> 
> it should be: if (!is_hostless_upstream(buffer->source))

Thanks, again!

> thanks
> ~zhigang
> 
> > > thanks
> > > ~zhigang
> > > 
> > > > +
> > > > +		/* don't go upstream if this component is not
> > > > connected */
> > > > +		if (!buffer->connected)
> > > > +			continue;
> > > > +
> > > > +		/* dont go upstream if this comp belongs to
> > > > another pipeline */
> > > > +		if (buffer->source->comp.pipeline_id !=
> > > > +		    current->comp.pipeline_id)
> > > > +			continue;
> > > > +
> > > > +		/* return if there is a host comp upstream */
> > > > +		if (!is_hostless_upstream(buffer->sink))
> > > > +			return false;
> > > > +	}
> > > > +
> > > > +	return true;
> > > > +}
> > > > +
> > > >    /*
> > > >     * Stream IPC Operations.
> > > >     */
> > > > @@ -125,8 +191,14 @@ static int ipc_stream_pcm_params(uint32_t
> > > > stream)
> > > >    	cd = pcm_dev->cd;
> > > >    	cd->params = pcm_params->params;
> > > >    
> > > > -#ifdef CONFIG_HOST_PTABLE
> > > > +	/*
> > > > +	 * walk in both directions to check if the pipeline is
> > > > hostless
> > > > +	 * skip page table set up if it is
> > > > +	 */
> > > > +	if (is_hostless_downstream(cd) &&
> > > > is_hostless_upstream(cd))
> > > > +		goto pipe_params;
> > > >    
> > > > +#ifdef CONFIG_HOST_PTABLE
> > > >    	list_init(&elem_list);
> > > >    
> > > >    	/* use DMA to read in compressed page table
> > > > ringbuffer
> > > > from host */
> > > > @@ -163,6 +235,8 @@ static int ipc_stream_pcm_params(uint32_t
> > > > stream)
> > > >    	}
> > > >    #endif
> > > >    
> > > > +pipe_params:
> > > > +
> > > >    	/* configure pipeline audio params */
> > > >    	err = pipeline_params(pcm_dev->cd->pipeline,
> > > > pcm_dev->cd,
> > > > pcm_params);
> > > >    	if (err < 0) {
> 
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware