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

[zhigangw]

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
~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) {
>>