From e85ce1d0dd649f163a74d11cb83e1bebf2f96110 Mon Sep 17 00:00:00 2001 From: Rajwa Date: Tue, 28 Jan 2020 22:28:16 +0100 Subject: [PATCH 1/3] kpb: add new state This adds new state KPB_STATE_INIT_DRAINING into KPB. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 15 ++++++++++++--- src/include/sof/audio/kpb.h | 1 + 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 39343d84ad86..df814d2f3c79 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -721,8 +721,11 @@ static int kpb_buffer_data(struct comp_dev *dev, tracev_kpb_with_ids(dev, "kpb_buffer_data()"); - if (kpb->state != KPB_STATE_RUN && kpb->state != KPB_STATE_DRAINING) + if (kpb->state != KPB_STATE_RUN && kpb->state != KPB_STATE_DRAINING) { + trace_kpb_error("kpb_buffer_data() error: wrong state! (current state %d, state log %x)", + kpb->state, kpb->state_log); return PPL_STATUS_PATH_STOP; + } if (kpb->state == KPB_STATE_DRAINING) draining_data->buffered_while_draining += size_to_copy; @@ -746,8 +749,8 @@ static int kpb_buffer_data(struct comp_dev *dev, current_time = platform_timer_get(timer); if (timeout < current_time) { trace_kpb_error("kpb_buffer_data(): timeout of %d [ms] (current state %d, state log %x)", - current_time - timeout, kpb->state, - kpb->state_log); + current_time - timeout, kpb->state, + kpb->state_log); return -ETIME; } @@ -922,6 +925,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; + enum kpb_state state_preserved = kpb->state; trace_kpb_with_ids(dev, "kpb_init_draining(): requested draining " "of %d [ms] from history buffer", @@ -946,6 +950,8 @@ 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. */ + kpb_change_state(kpb, KPB_STATE_INIT_DRAINING); + do { /* Calculate how much data we have stored in * current buffer. @@ -1042,6 +1048,9 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) /* Schedule draining task */ schedule_task(&kpb->draining_task, 0, 0); + + kpb_change_state(kpb, state_preserved); + } } diff --git a/src/include/sof/audio/kpb.h b/src/include/sof/audio/kpb.h index 196648f91fdf..d4e60983da36 100644 --- a/src/include/sof/audio/kpb.h +++ b/src/include/sof/audio/kpb.h @@ -61,6 +61,7 @@ enum kpb_state { KPB_STATE_PREPARING, KPB_STATE_RUN, KPB_STATE_BUFFERING, + KPB_STATE_INIT_DRAINING, KPB_STATE_DRAINING, KPB_STATE_HOST_COPY, KPB_STATE_RESETTING, From 6c9ff0f346c181c59c0b92d9675928675baaab80 Mon Sep 17 00:00:00 2001 From: Bartosz Kokoszko Date: Fri, 31 Jan 2020 10:05:48 +0100 Subject: [PATCH 2/3] kpb: delete return to preserved state in kpb_init_draining() We should handle situation that new pipeline_task() interrupts and invokes just after kpb_init_draining(). To do this we should stay in KPB_STATE_INIT_DRAINING and not go back to preserve state. Signed-off-by: Bartosz Kokoszko --- src/audio/kpb.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index df814d2f3c79..2bff0e2db9af 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -661,9 +661,10 @@ static int kpb_copy(struct comp_dev *dev) comp_update_buffer_consume(source, copy_bytes); break; + case KPB_STATE_INIT_DRAINING: case KPB_STATE_DRAINING: - /* In draining state we only buffer data in internal, - * history buffer. + /* In init_ draining and draining state we only buffer data in + * internal, history buffer. */ if (source->stream.avail <= kpb->buffer_size) { ret = kpb_buffer_data(dev, source, @@ -925,7 +926,6 @@ 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; - enum kpb_state state_preserved = kpb->state; trace_kpb_with_ids(dev, "kpb_init_draining(): requested draining " "of %d [ms] from history buffer", @@ -1048,9 +1048,6 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) /* Schedule draining task */ schedule_task(&kpb->draining_task, 0, 0); - - kpb_change_state(kpb, state_preserved); - } } From 94034bc95358ec9961cb550bb4658947c37a32c6 Mon Sep 17 00:00:00 2001 From: Bartosz Kokoszko Date: Tue, 28 Jan 2020 22:38:31 +0100 Subject: [PATCH 3/3] kpb: calculate free space in history buffer In KPB_STATE_INIT_DRAINING and KPB_STATE_DRAINING states in case of new kpb_copy() we do not check if we have enough free space in history buffer to store the data. This commit adds new private variable hb_free_space in order to track how much free space is available in history buffer. Signed-off-by: Bartosz Kokoszko --- src/audio/kpb.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 2bff0e2db9af..83b84d554c4f 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -59,6 +59,7 @@ struct comp_data { size_t buffered_data; /**< keeps info about amount of buffered data */ struct dd draining_task_data; size_t buffer_size; /**< size of internal history buffer */ + size_t hb_free_space; /**< free space in history buffer */ size_t host_buffer_size; /**< size of host buffer */ size_t host_period_size; /**< size of history period */ bool sync_draining_mode; /**< should we synchronize draining with @@ -663,12 +664,14 @@ static int kpb_copy(struct comp_dev *dev) break; case KPB_STATE_INIT_DRAINING: case KPB_STATE_DRAINING: - /* In init_ draining and draining state we only buffer data in + /* In init_draining and draining state we only buffer data in * internal, history buffer. */ if (source->stream.avail <= kpb->buffer_size) { - ret = kpb_buffer_data(dev, source, - source->stream.avail); + copy_bytes = MIN(source->stream.avail, + kpb->hb_free_space); + + ret = kpb_buffer_data(dev, source, copy_bytes); if (ret) { trace_kpb_error_with_ids(dev, "kpb_copy(): " "internal buffering " @@ -676,8 +679,8 @@ static int kpb_copy(struct comp_dev *dev) goto out; } - comp_update_buffer_consume(source, - source->stream.avail); + comp_update_buffer_consume(source, copy_bytes); + kpb->hb_free_space -= copy_bytes; } else { trace_kpb_error_with_ids(dev, "kpb_copy(): " "too much data to buffer."); @@ -722,7 +725,8 @@ static int kpb_buffer_data(struct comp_dev *dev, tracev_kpb_with_ids(dev, "kpb_buffer_data()"); - if (kpb->state != KPB_STATE_RUN && kpb->state != KPB_STATE_DRAINING) { + if (kpb->state != KPB_STATE_RUN && kpb->state != KPB_STATE_DRAINING && + kpb->state != KPB_STATE_INIT_DRAINING) { trace_kpb_error("kpb_buffer_data() error: wrong state! (current state %d, state log %x)", kpb->state, kpb->state_log); return PPL_STATUS_PATH_STOP; @@ -1005,6 +1009,12 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) } while (buff != first_buff); + kpb->hb_free_space = MAX(kpb->buffer_size - history_depth, + kpb->buffer_size - kpb->buffered_data); + + /* Change KPB internal state to DRAINING */ + kpb_change_state(kpb, KPB_STATE_DRAINING); + /* Should we drain in synchronized mode (sync_draining_mode)? * Note! We have already verified host params during * kpb_prepare(). @@ -1089,9 +1099,6 @@ static enum task_state kpb_draining_task(void *arg) pm_runtime_disable(PM_RUNTIME_DSP, PLATFORM_MASTER_CORE_ID); - /* Change KPB internal state to DRAINING */ - kpb_change_state(kpb, KPB_STATE_DRAINING); - draining_time_start = platform_timer_get(timer); while (history_depth > 0) { @@ -1136,6 +1143,7 @@ static enum task_state kpb_draining_task(void *arg) history_depth -= size_to_copy; drained += size_to_copy; period_bytes += size_to_copy; + kpb->hb_free_space += size_to_copy; if (move_buffer) { buff->r_ptr = buff->start_addr;