From d94eafd59c59b74c6c94ce486e4a0126edcbc954 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Wed, 15 Apr 2020 18:17:57 +0800 Subject: [PATCH 1/3] buffer: add helper to check if buffer format is matched to param Add a helper buffer_param_match() to check if the configured runtime buffer frame format/rate/channels are matched to the param ones Signed-off-by: Keyon Jie --- src/include/sof/audio/buffer.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index 647debc5ad0a..acd2e151c244 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -295,4 +295,25 @@ static inline int buffer_set_params(struct comp_buffer *buffer, return 0; } +static inline bool buffer_params_match(struct comp_buffer *buffer, + struct sof_ipc_stream_params *params, + uint32_t flag) +{ + assert(params && buffer); + + if ((flag & BUFF_PARAMS_FRAME_FMT) && + buffer->stream.frame_fmt != params->frame_fmt) + return false; + + if ((flag & BUFF_PARAMS_RATE) && + buffer->stream.rate != params->rate) + return false; + + if ((flag & BUFF_PARAMS_CHANNELS) && + buffer->stream.channels != params->channels) + return false; + + return true; +} + #endif /* __SOF_AUDIO_BUFFER_H__ */ From 50f79899ca3e799b7fb1a21a2555800184b7df0b Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Sat, 30 May 2020 01:25:00 +0800 Subject: [PATCH 2/3] pipeline: add a flag to indicate if the buffer is being traversed Add a flag 'walking' to the buffer struct, to indicate if the buffer is just being traversed while walking through the graph, and don't go back to the buffer which is already walked. Signed-off-by: Keyon Jie --- src/audio/pipeline.c | 6 ++++++ src/include/sof/audio/buffer.h | 1 + 2 files changed, 7 insertions(+) diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 1c255f4b9c35..d5f393a98272 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -169,6 +169,10 @@ static int pipeline_for_each_comp(struct comp_dev *current, list_for_item(clist, buffer_list) { buffer = buffer_from_list(clist, struct comp_buffer, dir); + /* don't go back to the buffer which already walked */ + if (buffer->walking) + continue; + /* execute operation on buffer */ if (ctx->buff_func) ctx->buff_func(buffer, ctx->buff_data); @@ -182,8 +186,10 @@ static int pipeline_for_each_comp(struct comp_dev *current, /* continue further */ if (ctx->comp_func) { + buffer->walking = true; int err = ctx->comp_func(buffer_comp, buffer, ctx, dir); + buffer->walking = false; if (err < 0) return err; } diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index acd2e151c244..5339dbcc7773 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -111,6 +111,7 @@ struct comp_buffer { uint16_t chmap[SOF_IPC_MAX_CHANNELS]; /**< channel map - SOF_CHMAP_ */ bool hw_params_configured; /**< indicates whether hw params were set */ + bool walking; /**< indicates if the buffer is being walking */ }; struct buffer_cb_transact { From 49e9c7258cbdf4512d77221b59c913f919e9293b Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Wed, 15 Apr 2020 19:10:49 +0800 Subject: [PATCH 3/3] pipeline: add negotiation before propagating params For different connected pipelines, they are usually connected via a component with >1 source or sink buffers, as most of those components don't have ability to converse frame format or sample rate, we need add a params negotiation mechanism between those pipelines. If all buffers are iterated in the params propagation direction, no negotiation needed, otherwise we need to iterate all other buffers in the opposite direction, to make sure all connected buffers have chance to take part in the negotiation. The negotiation should happen before we can propagate the params further, when doing negotiation in a branch/buffer, we should check: If a branch is active, check if params is matched, return error to reject the whole .params() requirement if not. If a branch is inactive, force update the params to its calling buffer, to make sure all branches are aligned on the format. We are propagating the params to branched buffer, and the subsequent component's .params() or .prepare() should be responsible to calibrate the buffer params if needed. For example, a component who has different channels buffers should explicitly configure the channels for its branched buffers (the ones connected to another pipeline). Signed-off-by: Keyon Jie --- src/audio/pipeline.c | 58 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index d5f393a98272..da125ebde285 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -389,11 +389,64 @@ static int pipeline_comp_hw_params(struct comp_dev *current, return 0; } +static int pipeline_comp_params_neg(struct comp_dev *current, + struct comp_buffer *calling_buf, + struct pipeline_walk_context *ctx, + int dir) +{ + struct pipeline_data *ppl_data = ctx->comp_data; + uint32_t flags = 0; + int err = 0; + + pipe_dbg(ppl_data->p, "pipeline_comp_params_neg(), current->comp.id = %u, dir = %u", + dev_comp_id(current), dir); + + /* check if 'current' is already configured */ + if (current->state != COMP_STATE_INIT && + current->state != COMP_STATE_READY) { + /* return 0 if params matches */ + if (buffer_params_match(calling_buf, + &ppl_data->params->params, + BUFF_PARAMS_FRAME_FMT | + BUFF_PARAMS_RATE)) + return 0; + /* + * the param is conflict with an active pipeline, + * drop an error and reject the .params() command. + */ + pipe_err(ppl_data->p, "pipeline_comp_params_neg(): params conflict with existed active pipeline!"); + return -EINVAL; + } + + /* + * Negotiation only happen when the current component has > 1 + * source or sink, we are propagating the params to branched + * buffers, and the subsequent component's .params() or .prepare() + * should be responsible for calibrating if needed. For example, + * a component who has different channels input/output buffers + * should explicitly configure the channels of the branched buffers. + */ + if (calling_buf) { + buffer_lock(calling_buf, &flags); + err = buffer_set_params(calling_buf, + &ppl_data->params->params, + BUFFER_UPDATE_FORCE); + buffer_unlock(calling_buf, flags); + } + + return err; +} + static int pipeline_comp_params(struct comp_dev *current, struct comp_buffer *calling_buf, struct pipeline_walk_context *ctx, int dir) { struct pipeline_data *ppl_data = ctx->comp_data; + struct pipeline_walk_context param_neg_ctx = { + .comp_func = pipeline_comp_params_neg, + .comp_data = ppl_data, + .skip_incomplete = true, + }; int stream_direction = ppl_data->params->params.direction; int end_type; int err; @@ -427,6 +480,11 @@ static int pipeline_comp_params(struct comp_dev *current, if (current->state == COMP_STATE_ACTIVE) return 0; + /* do params negotiation with other branches(opposite direction) */ + err = pipeline_for_each_comp(current, ¶m_neg_ctx, !dir); + if (err < 0 || err == PPL_STATUS_PATH_STOP) + return err; + /* set comp direction */ current->direction = ppl_data->params->params.direction;