From 6d67a9dce9c139202063eeeee11c8a00f2bd648c Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Wed, 4 Nov 2020 21:06:04 -0800 Subject: [PATCH] ipc: fix logic for buffer free A buffer should be prevented from being freed only if both the sink and the source widgets are active. In the case of dynamic pipeline loading, a buffer belonging to a pipeline will need to freed when that pipeline is no longer active but the other end of the buffer might still be active. So, change the logic to check if both the sink and source widget states are invalid to prevent the buffer from being freed. Also make sure that the buffer is disconnected from the active comp before it is freed. Signed-off-by: Ranjani Sridharan --- src/audio/pipeline.c | 15 ++++++++++++++ src/include/sof/audio/pipeline.h | 3 ++- src/ipc/ipc.c | 34 ++++++++++++++++++++++++++++---- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index f3f06b3bc..81c0a4d79 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -136,6 +136,21 @@ int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer, return 0; } +void pipeline_disconnect(struct comp_dev *comp, struct comp_buffer *buffer, int dir) +{ + uint32_t flags; + + if (dir == PPL_CONN_DIR_COMP_TO_BUFFER) + comp_info(comp, "disconnect buffer %d as sink", buffer->id); + else + comp_info(comp, "disconnect buffer %d as source", buffer->id); + + irq_local_disable(flags); + list_item_del(buffer_comp_list(buffer, dir)); + comp_writeback(comp); + irq_local_enable(flags); +} + struct pipeline_walk_context { int (*comp_func)(struct comp_dev *, struct comp_buffer *, struct pipeline_walk_context *, int); diff --git a/src/include/sof/audio/pipeline.h b/src/include/sof/audio/pipeline.h index 58d2f8aba..730e5d38f 100644 --- a/src/include/sof/audio/pipeline.h +++ b/src/include/sof/audio/pipeline.h @@ -221,9 +221,10 @@ struct pipeline *pipeline_new(struct sof_ipc_pipe_new *pipe_desc, struct comp_dev *cd); int pipeline_free(struct pipeline *p); -/* insert component in pipeline */ +/* insert/remove component in pipeline */ int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer, int dir); +void pipeline_disconnect(struct comp_dev *comp, struct comp_buffer *buffer, int dir); /* complete the pipeline */ int pipeline_complete(struct pipeline *p, struct comp_dev *source, diff --git a/src/ipc/ipc.c b/src/ipc/ipc.c index 35aefb45c..16cea90be 100644 --- a/src/ipc/ipc.c +++ b/src/ipc/ipc.c @@ -291,6 +291,9 @@ int ipc_buffer_free(struct ipc *ipc, uint32_t buffer_id) struct ipc_comp_dev *ibd; struct ipc_comp_dev *icd; struct list_item *clist; + struct comp_dev *sink = NULL, *source = NULL; + bool sink_active = false; + bool source_active = false; /* check whether buffer exists */ ibd = ipc_get_comp_by_id(ipc, buffer_id); @@ -309,13 +312,36 @@ int ipc_buffer_free(struct ipc *ipc, uint32_t buffer_id) /* check comp state if sink and source are valid */ if (ibd->cb->sink == icd->cd && - ibd->cb->sink->state != COMP_STATE_READY) - return -EINVAL; + ibd->cb->sink->state != COMP_STATE_READY) { + sink = ibd->cb->sink; + sink_active = true; + } + if (ibd->cb->source == icd->cd && - ibd->cb->source->state != COMP_STATE_READY) - return -EINVAL; + ibd->cb->source->state != COMP_STATE_READY) { + source = ibd->cb->source; + source_active = true; + } } + /* + * A buffer could be connected to 2 different pipelines. When one pipeline is freed, the + * buffer comp that belongs in this pipeline will need to be freed even when the other + * pipeline that the buffer is connected to is active. Check if both ends are active before + * freeing the buffer. + */ + if (sink_active && source_active) + return -EINVAL; + + /* + * Disconnect the buffer from the active component before freeing it. + */ + if (sink) + pipeline_disconnect(sink, ibd->cb, PPL_CONN_DIR_BUFFER_TO_COMP); + + if (source) + pipeline_disconnect(source, ibd->cb, PPL_CONN_DIR_COMP_TO_BUFFER); + /* free buffer and remove from list */ buffer_free(ibd->cb); list_item_del(&ibd->list);