From 9df1b87b43bb68f6ff6c2a36fcb9c76b07f92231 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Mon, 27 Jan 2020 22:04:03 +0100 Subject: [PATCH 1/2] kpb: disable IRQs before calulation of read pointer This patch adds critical section around calculations of draining starting point (buff->r_ptr). Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 8e9e8c1ac257..c3373a27069a 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -64,6 +64,7 @@ struct comp_data { bool sync_draining_mode; /**< should we synchronize draining with * host? */ + spinlock_t lock; /**< locking mechanism for read pointer calculations */ }; /*! KPB private functions */ @@ -916,6 +917,7 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) (KPB_SAMPLE_CONTAINER_SIZE(sample_width) / 8) * kpb->config.channels; size_t period_bytes_limit = 0; + uint32_t flags; trace_kpb_with_ids(dev, "kpb_init_draining(): requested draining " "of %d [ms] from history buffer", @@ -940,6 +942,7 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) * in the history buffer. All we have to do now is to calculate * read pointer from which we will start draining. */ + spin_lock_irq(&kpb->lock, flags); do { /* Calculate how much data we have stored in * current buffer. @@ -992,6 +995,7 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) } } while (buff != first_buff); + spin_unlock_irq(&kpb->lock, flags); /* Should we drain in synchronized mode (sync_draining_mode)? * Note! We have already verified host params during From 3141107ecb6abb13ff66a13bcdf09cd758498019 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Tue, 28 Jan 2020 15:34:44 +0100 Subject: [PATCH 2/2] kpb: check if allocated buffer matches new host params This patch fixes the problem with format switch. The problem was when we allocated history buffer for smaller format like S16_LE and then dynamically changed format to some wider one like S24_LE. In this case KPB was detecting that buffer has already been allocated and used it what limits the amount of data we can store for new format. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index c3373a27069a..4a56592a4d89 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -74,7 +74,8 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli); static enum task_state kpb_draining_task(void *arg); static int kpb_buffer_data(struct comp_dev *dev, const struct comp_buffer *source, size_t size); -static size_t kpb_allocate_history_buffer(struct comp_data *kpb); +static size_t kpb_allocate_history_buffer(struct comp_data *kpb, + size_t hb_size_req); static void kpb_clear_history_buffer(struct hb *buff); static void kpb_free_history_buffer(struct hb *buff); static inline bool kpb_is_sample_width_supported(uint32_t sampling_width); @@ -189,12 +190,13 @@ static struct comp_dev *kpb_new(struct sof_ipc_comp *comp) * * \return: none. */ -static size_t kpb_allocate_history_buffer(struct comp_data *kpb) +static size_t kpb_allocate_history_buffer(struct comp_data *kpb, + size_t hb_size_req) { struct hb *history_buffer; struct hb *new_hb = NULL; /*! Total allocation size */ - size_t hb_size = kpb->buffer_size; + size_t hb_size = hb_size_req; /*! Current allocation size */ size_t ca_size = hb_size; /*! Memory caps priorites for history buffer */ @@ -330,6 +332,7 @@ static void kpb_free(struct comp_dev *dev) /* Reclaim memory occupied by history buffer */ kpb_free_history_buffer(kpb->history_buffer); kpb->history_buffer = NULL; + kpb->buffer_size = 0; /* remove scheduling */ schedule_task_free(&kpb->draining_task); @@ -385,7 +388,7 @@ static int kpb_prepare(struct comp_dev *dev) int i; struct list_item *blist; struct comp_buffer *sink; - size_t allocated_size; + size_t hb_size_req = KPB_MAX_BUFFER_SIZE(kpb->config.sampling_width); trace_kpb_with_ids(dev, "kpb_prepare()"); @@ -403,33 +406,42 @@ static int kpb_prepare(struct comp_dev *dev) if (ret == COMP_STATUS_STATE_ALREADY_SET) return PPL_STATUS_PATH_STOP; - /* Init private data */ + if (!validate_host_params(dev, kpb->host_period_size, + kpb->host_buffer_size)) { + trace_kpb_error("kpb_prepare() error: wrong host params."); + return -EINVAL; + } + kpb_change_state(kpb, KPB_STATE_PREPARING); + + /* Init private data */ kpb->kpb_no_of_clients = 0; kpb->buffered_data = 0; - kpb->buffer_size = KPB_MAX_BUFFER_SIZE(kpb->config.sampling_width); kpb->sel_sink = NULL; kpb->host_sink = NULL; - if (!validate_host_params(dev, kpb->host_period_size, - kpb->host_buffer_size)) { - trace_kpb_error("kpb_prepare() error: wrong host params."); - return -EINVAL; + if (kpb->history_buffer && kpb->buffer_size < hb_size_req) { + /* Host params has changed, we need to allocate new buffer */ + kpb_free_history_buffer(kpb->history_buffer); + kpb->history_buffer = NULL; } + if (!kpb->history_buffer) { /* Allocate history buffer */ - allocated_size = kpb_allocate_history_buffer(kpb); + kpb->buffer_size = kpb_allocate_history_buffer(kpb, + hb_size_req); /* Have we allocated what we requested? */ - if (allocated_size < kpb->buffer_size) { - trace_kpb_error("kpb_prepare() error: failed to allocate space for KPB buffer/s"); + if (kpb->buffer_size < hb_size_req) { + trace_kpb_error("kpb_prepare() error: failed to allocate space for KPB buffer"); kpb_free_history_buffer(kpb->history_buffer); kpb->history_buffer = NULL; + kpb->buffer_size = 0; return -EINVAL; } } /* Init history buffer */ - kpb_clear_history_buffer(kpb->history_buffer); + kpb_reset_history_buffer(kpb->history_buffer); /* Initialize clients data */ for (i = 0; i < KPB_MAX_NO_OF_CLIENTS; i++) {