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

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