From 74f5e480af74af7da7adf014f87f37ede307668d Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Mon, 30 Mar 2020 23:19:52 +0200 Subject: [PATCH 01/10] kpb: change history buffer struct name This patch changes the name of the "hb" struct to more meaningfull "history_buffer". Therefore code is more readable. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 102 ++++++++++++++++++------------------ src/include/sof/audio/kpb.h | 8 +-- 2 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 3ef07704ac0d..7735ac6ab8c6 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -66,7 +66,7 @@ struct comp_data { struct sof_kpb_config config; /**< component configuration data */ struct comp_buffer *sel_sink; /**< real time sink (channel selector )*/ struct comp_buffer *host_sink; /**< draining sink (client) */ - struct hb *history_buffer; /**< internal history buffer */ + struct history_buffer *hb; /**< internal history buffer */ 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 */ @@ -87,8 +87,8 @@ 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, size_t hb_size_req); -static void kpb_clear_history_buffer(struct hb *buff); -static void kpb_free_history_buffer(struct hb *buff); +static void kpb_clear_history_buffer(struct history_buffer *buff); +static void kpb_free_history_buffer(struct history_buffer *buff); static inline bool kpb_is_sample_width_supported(uint32_t sampling_width); static void kpb_copy_samples(struct comp_buffer *sink, struct comp_buffer *source, size_t size, @@ -98,7 +98,7 @@ static void kpb_drain_samples(void *source, struct audio_stream *sink, static void kpb_buffer_samples(const struct audio_stream *source, uint32_t start, void *sink, size_t size, size_t sample_width); -static void kpb_reset_history_buffer(struct hb *buff); +static void kpb_reset_history_buffer(struct history_buffer *buff); static inline bool validate_host_params(struct comp_dev *dev, size_t host_period_size, size_t host_buffer_size, @@ -182,7 +182,7 @@ static struct comp_dev *kpb_new(const struct comp_driver *drv, 0); /* no flags */ /* Init basic component data */ - kpb->history_buffer = NULL; + kpb->hb = NULL; kpb->kpb_no_of_clients = 0; kpb->state_log = 0; @@ -202,8 +202,8 @@ static struct comp_dev *kpb_new(const struct comp_driver *drv, 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; + struct history_buffer *hb; + struct history_buffer *new_hb = NULL; /*! Total allocation size */ size_t hb_size = hb_size_req; /*! Current allocation size */ @@ -219,13 +219,13 @@ static size_t kpb_allocate_history_buffer(struct comp_data *kpb, comp_cl_info(&comp_kpb, "kpb_allocate_history_buffer()"); /* Initialize history buffer */ - kpb->history_buffer = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, - sizeof(struct hb)); - if (!kpb->history_buffer) + kpb->hb = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, + sizeof(struct history_buffer)); + if (!kpb->hb) return 0; - kpb->history_buffer->next = kpb->history_buffer; - kpb->history_buffer->prev = kpb->history_buffer; - history_buffer = kpb->history_buffer; + kpb->hb->next = kpb->hb; + kpb->hb->prev = kpb->hb; + hb = kpb->hb; /* Allocate history buffer/s. KPB history buffer has a size of * KPB_MAX_BUFFER_SIZE, since there is no single memory block @@ -245,14 +245,14 @@ static size_t kpb_allocate_history_buffer(struct comp_data *kpb, comp_cl_info(&comp_kpb, "kpb new memory block: %d", ca_size); allocated_size += ca_size; - history_buffer->start_addr = new_mem_block; - history_buffer->end_addr = (char *)new_mem_block + + hb->start_addr = new_mem_block; + hb->end_addr = (char *)new_mem_block + ca_size; - history_buffer->w_ptr = new_mem_block; - history_buffer->r_ptr = new_mem_block; - history_buffer->state = KPB_BUFFER_FREE; + hb->w_ptr = new_mem_block; + hb->r_ptr = new_mem_block; + hb->state = KPB_BUFFER_FREE; hb_size -= ca_size; - history_buffer->next = kpb->history_buffer; + hb->next = kpb->hb; /* Do we need another buffer? */ if (hb_size > 0) { /* Yes, we still need at least one more buffer. @@ -260,15 +260,15 @@ static size_t kpb_allocate_history_buffer(struct comp_data *kpb, */ new_hb = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, - sizeof(struct hb)); + sizeof(struct history_buffer)); if (!new_hb) return 0; - history_buffer->next = new_hb; - new_hb->next = kpb->history_buffer; + hb->next = new_hb; + new_hb->next = kpb->hb; new_hb->state = KPB_BUFFER_OFF; - new_hb->prev = history_buffer; - history_buffer = new_hb; - kpb->history_buffer->prev = new_hb; + new_hb->prev = hb; + hb = new_hb; + kpb->hb->prev = new_hb; ca_size = hb_size; i++; } @@ -303,10 +303,10 @@ static size_t kpb_allocate_history_buffer(struct comp_data *kpb, * * \return none. */ -static void kpb_free_history_buffer(struct hb *buff) +static void kpb_free_history_buffer(struct history_buffer *buff) { - struct hb *_buff; - struct hb *first_buff = buff; + struct history_buffer *_buff; + struct history_buffer *first_buff = buff; comp_cl_info(&comp_kpb, "kpb_free_history_buffer()"); @@ -341,8 +341,8 @@ static void kpb_free(struct comp_dev *dev) notifier_unregister(dev, NULL, NOTIFIER_ID_KPB_CLIENT_EVT); /* Reclaim memory occupied by history buffer */ - kpb_free_history_buffer(kpb->history_buffer); - kpb->history_buffer = NULL; + kpb_free_history_buffer(kpb->hb); + kpb->hb = NULL; kpb->buffer_size = 0; /* remove scheduling */ @@ -462,13 +462,13 @@ static int kpb_prepare(struct comp_dev *dev) kpb->sel_sink = NULL; kpb->host_sink = NULL; - if (kpb->history_buffer && kpb->buffer_size < hb_size_req) { + if (kpb->hb && 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; + kpb_free_history_buffer(kpb->hb); + kpb->hb = NULL; } - if (!kpb->history_buffer) { + if (!kpb->hb) { /* Allocate history buffer */ kpb->buffer_size = kpb_allocate_history_buffer(kpb, hb_size_req); @@ -476,14 +476,14 @@ static int kpb_prepare(struct comp_dev *dev) /* Have we allocated what we requested? */ if (kpb->buffer_size < hb_size_req) { comp_cl_err(&comp_kpb, "kpb_prepare(): failed to allocate space for KPB buffer"); - kpb_free_history_buffer(kpb->history_buffer); - kpb->history_buffer = NULL; + kpb_free_history_buffer(kpb->hb); + kpb->hb = NULL; kpb->buffer_size = 0; return -EINVAL; } } /* Init history buffer */ - kpb_reset_history_buffer(kpb->history_buffer); + kpb_reset_history_buffer(kpb->hb); /* Initialize clients data */ for (i = 0; i < KPB_MAX_NO_OF_CLIENTS; i++) { @@ -495,8 +495,8 @@ static int kpb_prepare(struct comp_dev *dev) ret = notifier_register(dev, NULL, NOTIFIER_ID_KPB_CLIENT_EVT, kpb_event_handler); if (ret < 0) { - kpb_free_history_buffer(kpb->history_buffer); - kpb->history_buffer = NULL; + kpb_free_history_buffer(kpb->hb); + kpb->hb = NULL; return -ENOMEM; } @@ -577,11 +577,11 @@ static int kpb_reset(struct comp_dev *dev) default: kpb->buffered_data = 0; - if (kpb->history_buffer) { + if (kpb->hb) { /* Reset history buffer - zero its data, reset pointers * and states. */ - kpb_reset_history_buffer(kpb->history_buffer); + kpb_reset_history_buffer(kpb->hb); } /* Unregister KPB from notifications */ @@ -784,7 +784,7 @@ static int kpb_buffer_data(struct comp_dev *dev, size_t size_to_copy = size; size_t space_avail; struct comp_data *kpb = comp_get_drvdata(dev); - struct hb *buff = kpb->history_buffer; + struct history_buffer *buff = kpb->hb; uint32_t offset = 0; uint64_t timeout = 0; uint64_t current_time; @@ -882,7 +882,7 @@ static int kpb_buffer_data(struct comp_dev *dev, * we will know right away what is the current * write buffer */ - kpb->history_buffer = buff; + kpb->hb = buff; } /* Mark buffer as FREE */ buff->state = KPB_BUFFER_FREE; @@ -988,8 +988,8 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) size_t history_depth = cli->history_depth * kpb->config.channels * (kpb->config.sampling_freq / 1000) * (KPB_SAMPLE_CONTAINER_SIZE(sample_width) / 8); - struct hb *buff = kpb->history_buffer; - struct hb *first_buff = buff; + struct history_buffer *buff = kpb->hb; + struct history_buffer *first_buff = buff; size_t buffered = 0; size_t local_buffered; enum comp_copy_type copy_type = COMP_COPY_NORMAL; @@ -1100,7 +1100,7 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) /* Add one-time draining task into the scheduler. */ kpb->draining_task_data.sink = kpb->host_sink; - kpb->draining_task_data.history_buffer = buff; + kpb->draining_task_data.hb = buff; kpb->draining_task_data.history_depth = history_depth; kpb->draining_task_data.sample_width = sample_width; kpb->draining_task_data.drain_interval = drain_interval; @@ -1132,7 +1132,7 @@ static enum task_state kpb_draining_task(void *arg) { struct dd *draining_data = (struct dd *)arg; struct comp_buffer *sink = draining_data->sink; - struct hb *buff = draining_data->history_buffer; + struct history_buffer *buff = draining_data->hb; size_t history_depth = draining_data->history_depth; size_t sample_width = draining_data->sample_width; size_t size_to_read; @@ -1364,9 +1364,9 @@ static void kpb_buffer_samples(const struct audio_stream *source, * * \return: none. */ -static void kpb_clear_history_buffer(struct hb *buff) +static void kpb_clear_history_buffer(struct history_buffer *buff) { - struct hb *first_buff = buff; + struct history_buffer *first_buff = buff; void *start_addr; size_t size; @@ -1471,9 +1471,9 @@ static void kpb_copy_samples(struct comp_buffer *sink, * * \return none. */ -static void kpb_reset_history_buffer(struct hb *buff) +static void kpb_reset_history_buffer(struct history_buffer *buff) { - struct hb *first_buff = buff; + struct history_buffer *first_buff = buff; comp_cl_info(&comp_kpb, "kpb_reset_history_buffer()"); diff --git a/src/include/sof/audio/kpb.h b/src/include/sof/audio/kpb.h index 8ee2f7fa52ca..e7b602019963 100644 --- a/src/include/sof/audio/kpb.h +++ b/src/include/sof/audio/kpb.h @@ -98,19 +98,19 @@ enum kpb_id { KPB_HP, }; -struct hb { +struct history_buffer { enum buffer_state state; /**< state of the buffer */ void *start_addr; /**< buffer start address */ void *end_addr; /**< buffer end address */ void *w_ptr; /**< buffer write pointer */ void *r_ptr; /**< buffer read pointer */ - struct hb *next; /**< next history buffer */ - struct hb *prev; /**< next history buffer */ + struct history_buffer *next; /**< next history buffer */ + struct history_buffer *prev; /**< next history buffer */ }; struct dd { struct comp_buffer *sink; - struct hb *history_buffer; + struct history_buffer *hb; size_t history_depth; uint8_t is_draining_active; size_t sample_width; From d68bda0b7aa327dbd936ea70b6e4e1b53c5f8936 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 10:53:03 +0200 Subject: [PATCH 02/10] kpb: change the name of draining data structure This patch changes the name of "dd" structure to more meaningful "draining_data". Therefore code is more readable. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 8 +++++--- src/include/sof/audio/kpb.h | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 7735ac6ab8c6..9d690084fddb 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -68,7 +68,7 @@ struct comp_data { struct comp_buffer *host_sink; /**< draining sink (client) */ struct history_buffer *hb; /**< internal history buffer */ size_t buffered_data; /**< keeps info about amount of buffered data */ - struct dd draining_task_data; + struct draining_data draining_task_data; size_t buffer_size; /**< size of internal history buffer */ size_t host_buffer_size; /**< size of host buffer */ size_t host_period_size; /**< size of history period */ @@ -741,6 +741,7 @@ static int kpb_copy(struct comp_dev *dev) /* In draining and init draining we only buffer data in * the internal history buffer. */ + if (source->stream.avail <= kpb->buffer_size) { buffer_invalidate(source, source->stream.avail); ret = kpb_buffer_data(dev, source, @@ -789,7 +790,7 @@ static int kpb_buffer_data(struct comp_dev *dev, uint64_t timeout = 0; uint64_t current_time; enum kpb_state state_preserved = kpb->state; - struct dd *draining_data = &kpb->draining_task_data; + struct draining_data *draining_data = &kpb->draining_task_data; size_t sample_width = kpb->config.sampling_width; struct timer *timer = timer_get(); @@ -1072,6 +1073,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)? @@ -1130,7 +1132,7 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) */ static enum task_state kpb_draining_task(void *arg) { - struct dd *draining_data = (struct dd *)arg; + struct draining_data *draining_data = (struct draining_data *)arg; struct comp_buffer *sink = draining_data->sink; struct history_buffer *buff = draining_data->hb; size_t history_depth = draining_data->history_depth; diff --git a/src/include/sof/audio/kpb.h b/src/include/sof/audio/kpb.h index e7b602019963..41e70e44d16a 100644 --- a/src/include/sof/audio/kpb.h +++ b/src/include/sof/audio/kpb.h @@ -108,7 +108,8 @@ struct history_buffer { struct history_buffer *prev; /**< next history buffer */ }; -struct dd { +/* Draining task data */ +struct draining_data { struct comp_buffer *sink; struct history_buffer *hb; size_t history_depth; From 035f193e6284ee75c5c3c9be270c10ff190358d4 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 11:05:26 +0200 Subject: [PATCH 03/10] kpb: add new struct history_data This patch adds new struct called "history_data" which contains all variables related to history buffer namely, its size, amount of free and available data as well as address of current write buffer (history buffer is a collection of smaller buffers) Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 78 ++++++++++++++++++------------------- src/include/sof/audio/kpb.h | 7 ++++ 2 files changed, 45 insertions(+), 40 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 9d690084fddb..2f6dd5adaf05 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -66,15 +66,13 @@ struct comp_data { struct sof_kpb_config config; /**< component configuration data */ struct comp_buffer *sel_sink; /**< real time sink (channel selector )*/ struct comp_buffer *host_sink; /**< draining sink (client) */ - struct history_buffer *hb; /**< internal history buffer */ - size_t buffered_data; /**< keeps info about amount of buffered data */ struct draining_data draining_task_data; - size_t buffer_size; /**< size of internal 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 * host? */ + struct history_data hd; /** data related to history buffer */ spinlock_t lock; /**< locking mechanism for read pointer calculations */ }; @@ -182,7 +180,7 @@ static struct comp_dev *kpb_new(const struct comp_driver *drv, 0); /* no flags */ /* Init basic component data */ - kpb->hb = NULL; + kpb->hd.c_hb = NULL; kpb->kpb_no_of_clients = 0; kpb->state_log = 0; @@ -219,13 +217,13 @@ static size_t kpb_allocate_history_buffer(struct comp_data *kpb, comp_cl_info(&comp_kpb, "kpb_allocate_history_buffer()"); /* Initialize history buffer */ - kpb->hb = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, - sizeof(struct history_buffer)); - if (!kpb->hb) + kpb->hd.c_hb = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, + sizeof(struct history_buffer)); + if (!kpb->hd.c_hb) return 0; - kpb->hb->next = kpb->hb; - kpb->hb->prev = kpb->hb; - hb = kpb->hb; + kpb->hd.c_hb->next = kpb->hd.c_hb; + kpb->hd.c_hb->prev = kpb->hd.c_hb; + hb = kpb->hd.c_hb; /* Allocate history buffer/s. KPB history buffer has a size of * KPB_MAX_BUFFER_SIZE, since there is no single memory block @@ -252,7 +250,7 @@ static size_t kpb_allocate_history_buffer(struct comp_data *kpb, hb->r_ptr = new_mem_block; hb->state = KPB_BUFFER_FREE; hb_size -= ca_size; - hb->next = kpb->hb; + hb->next = kpb->hd.c_hb; /* Do we need another buffer? */ if (hb_size > 0) { /* Yes, we still need at least one more buffer. @@ -264,11 +262,11 @@ static size_t kpb_allocate_history_buffer(struct comp_data *kpb, if (!new_hb) return 0; hb->next = new_hb; - new_hb->next = kpb->hb; + new_hb->next = kpb->hd.c_hb; new_hb->state = KPB_BUFFER_OFF; new_hb->prev = hb; hb = new_hb; - kpb->hb->prev = new_hb; + kpb->hd.c_hb->prev = new_hb; ca_size = hb_size; i++; } @@ -341,9 +339,9 @@ static void kpb_free(struct comp_dev *dev) notifier_unregister(dev, NULL, NOTIFIER_ID_KPB_CLIENT_EVT); /* Reclaim memory occupied by history buffer */ - kpb_free_history_buffer(kpb->hb); - kpb->hb = NULL; - kpb->buffer_size = 0; + kpb_free_history_buffer(kpb->hd.c_hb); + kpb->hd.c_hb = NULL; + kpb->hd.buffer_size = 0; /* remove scheduling */ schedule_task_free(&kpb->draining_task); @@ -458,32 +456,32 @@ static int kpb_prepare(struct comp_dev *dev) /* Init private data */ kpb->kpb_no_of_clients = 0; - kpb->buffered_data = 0; + kpb->hd.buffered = 0; kpb->sel_sink = NULL; kpb->host_sink = NULL; - if (kpb->hb && kpb->buffer_size < hb_size_req) { + if (kpb->hd.c_hb && kpb->hd.buffer_size < hb_size_req) { /* Host params has changed, we need to allocate new buffer */ - kpb_free_history_buffer(kpb->hb); - kpb->hb = NULL; + kpb_free_history_buffer(kpb->hd.c_hb); + kpb->hd.c_hb = NULL; } - if (!kpb->hb) { + if (!kpb->hd.c_hb) { /* Allocate history buffer */ - kpb->buffer_size = kpb_allocate_history_buffer(kpb, - hb_size_req); + kpb->hd.buffer_size = kpb_allocate_history_buffer(kpb, + hb_size_req); /* Have we allocated what we requested? */ - if (kpb->buffer_size < hb_size_req) { + if (kpb->hd.buffer_size < hb_size_req) { comp_cl_err(&comp_kpb, "kpb_prepare(): failed to allocate space for KPB buffer"); - kpb_free_history_buffer(kpb->hb); - kpb->hb = NULL; - kpb->buffer_size = 0; + kpb_free_history_buffer(kpb->hd.c_hb); + kpb->hd.c_hb = NULL; + kpb->hd.buffer_size = 0; return -EINVAL; } } /* Init history buffer */ - kpb_reset_history_buffer(kpb->hb); + kpb_reset_history_buffer(kpb->hd.c_hb); /* Initialize clients data */ for (i = 0; i < KPB_MAX_NO_OF_CLIENTS; i++) { @@ -495,8 +493,8 @@ static int kpb_prepare(struct comp_dev *dev) ret = notifier_register(dev, NULL, NOTIFIER_ID_KPB_CLIENT_EVT, kpb_event_handler); if (ret < 0) { - kpb_free_history_buffer(kpb->hb); - kpb->hb = NULL; + kpb_free_history_buffer(kpb->hd.c_hb); + kpb->hd.c_hb = NULL; return -ENOMEM; } @@ -575,13 +573,13 @@ static int kpb_reset(struct comp_dev *dev) ret = comp_set_state(dev, COMP_TRIGGER_RESET); break; default: - kpb->buffered_data = 0; + kpb->hd.buffered = 0; - if (kpb->hb) { + if (kpb->hd.c_hb) { /* Reset history buffer - zero its data, reset pointers * and states. */ - kpb_reset_history_buffer(kpb->hb); + kpb_reset_history_buffer(kpb->hd.c_hb); } /* Unregister KPB from notifications */ @@ -678,7 +676,7 @@ static int kpb_copy(struct comp_dev *dev) /* Buffer source data internally in history buffer for future * use by clients. */ - if (source->stream.avail <= kpb->buffer_size) { + if (source->stream.avail <= kpb->hd.buffer_size) { ret = kpb_buffer_data(dev, source, copy_bytes); if (ret) { comp_err(dev, "kpb_copy(): internal buffering failed."); @@ -688,7 +686,7 @@ static int kpb_copy(struct comp_dev *dev) hb_free_space = kpb->buffer_size - kpb->buffered_data; - kpb->buffered_data += + kpb->hd.buffered += MIN(copy_bytes, hb_free_space); } } else { @@ -785,7 +783,7 @@ static int kpb_buffer_data(struct comp_dev *dev, size_t size_to_copy = size; size_t space_avail; struct comp_data *kpb = comp_get_drvdata(dev); - struct history_buffer *buff = kpb->hb; + struct history_buffer *buff = kpb->hd.c_hb; uint32_t offset = 0; uint64_t timeout = 0; uint64_t current_time; @@ -883,7 +881,7 @@ static int kpb_buffer_data(struct comp_dev *dev, * we will know right away what is the current * write buffer */ - kpb->hb = buff; + kpb->hd.c_hb = buff; } /* Mark buffer as FREE */ buff->state = KPB_BUFFER_FREE; @@ -989,7 +987,7 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) size_t history_depth = cli->history_depth * kpb->config.channels * (kpb->config.sampling_freq / 1000) * (KPB_SAMPLE_CONTAINER_SIZE(sample_width) / 8); - struct history_buffer *buff = kpb->hb; + struct history_buffer *buff = kpb->hd.c_hb; struct history_buffer *first_buff = buff; size_t buffered = 0; size_t local_buffered; @@ -1013,8 +1011,8 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) /* TODO: check also if client is registered */ } else if (!is_sink_ready) { comp_err(dev, "kpb_init_draining(): sink not ready for draining"); - } else if (kpb->buffered_data < history_depth || - kpb->buffer_size < history_depth) { + } else if (kpb->hd.buffered < history_depth || + kpb->hd.buffer_size < history_depth) { comp_cl_err(&comp_kpb, "kpb_init_draining(): not enough data in history buffer"); } else { /* Draining accepted, find proper buffer to start reading diff --git a/src/include/sof/audio/kpb.h b/src/include/sof/audio/kpb.h index 41e70e44d16a..be5a312979b0 100644 --- a/src/include/sof/audio/kpb.h +++ b/src/include/sof/audio/kpb.h @@ -122,6 +122,13 @@ struct draining_data { bool sync_mode_on; }; +struct history_data { + size_t buffer_size; /**< size of internal history buffer */ + size_t buffered; /**< amount of buffered data */ + size_t free; /** spce we can use to write new data */ + struct history_buffer *c_hb; /**< current buffer used for writing */ +}; + #ifdef UNIT_TEST void sys_comp_kpb_init(void); #endif From cfe04f8758770936e3f21c80915f3362c7022ab9 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 12:19:30 +0200 Subject: [PATCH 04/10] kpb: calculate free space in history buffer This additional variable says how much data can be written to history buffer and won't cause overwrite of data staged for draining. It is important to realize the history buffer is circular and its data gets overwrite all the time. So *free* in this case doesn't mean "amount of bytes before buffer is full". History buffer is *full* 99% of the time. However before the draining starts we "freeze" history buffer data so new samples can be written only if: HISTORY_BUFFER_SIZE - HISTORY_DEPTH > 0 The result of above subtraction is what we call "free" Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 2f6dd5adaf05..b99f9fcd4ee0 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -482,6 +482,7 @@ static int kpb_prepare(struct comp_dev *dev) } /* Init history buffer */ kpb_reset_history_buffer(kpb->hd.c_hb); + kpb->hd.free = kpb->hd.buffer_size; /* Initialize clients data */ for (i = 0; i < KPB_MAX_NO_OF_CLIENTS; i++) { @@ -1024,6 +1025,12 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) kpb_change_state(kpb, KPB_STATE_INIT_DRAINING); + /* Set history buffer size so new data won't overwrite those + * staged for draining. + */ + kpb->hd.free = kpb->hd.buffer_size - history_depth; + + /* Find buffer to start draining from */ do { /* Calculate how much data we have stored in * current buffer. @@ -1205,6 +1212,8 @@ static enum task_state kpb_draining_task(void *arg) history_depth -= size_to_copy; drained += size_to_copy; period_bytes += size_to_copy; + kpb->hd.free += MIN(kpb->hd.buffer_size - + kpb->hd.free, size_to_copy); if (move_buffer) { buff->r_ptr = buff->start_addr; From a4ea7bcf8471a74a00a8010acf86aad300804f87 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 11:58:10 +0200 Subject: [PATCH 05/10] kpb: add condition for buffering This patch adds the condition that if we are in draining or init draining state and pipeline_copy() appeared before we actually started the draining, plus the amount of new data can overwrite the data staged for draining then we should postpone buffering procedure for next period. At that time we assume draining task will have already drained some samples therefore making a space for new ones. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index b99f9fcd4ee0..8a804bbc76cd 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -741,19 +741,19 @@ static int kpb_copy(struct comp_dev *dev) * the internal history buffer. */ - if (source->stream.avail <= kpb->buffer_size) { - buffer_invalidate(source, source->stream.avail); - ret = kpb_buffer_data(dev, source, - source->stream.avail); + copy_bytes = MIN(source->stream.avail, kpb->hd.free); + if (copy_bytes) { + buffer_invalidate(source, copy_bytes); + ret = kpb_buffer_data(dev, source, copy_bytes); if (ret) { comp_err(dev, "kpb_copy(): internal buffering failed."); goto out; } - comp_update_buffer_consume(source, - source->stream.avail); + comp_update_buffer_consume(source, copy_bytes); } else { - comp_err(dev, "kpb_copy(): too much data to buffer."); + comp_err(dev, "kpb_copy(): buffering skipped (no data to copy, avail %d, free %d", + source->stream.avail, kpb->hd.free); } ret = PPL_STATUS_PATH_STOP; From e7ba688c32016c33fd780c7adab02923765dd157 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 12:32:32 +0200 Subject: [PATCH 06/10] kpb: simplify calculation of buffered data This patch reworks the calculation of buffered data. NOTE! We only keep record of buffered data up to the size of history buffer as there is no usecase beyond that. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 8a804bbc76cd..215e1a13596f 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -614,7 +614,6 @@ static int kpb_copy(struct comp_dev *dev) struct comp_buffer *sink = NULL; size_t copy_bytes = 0; size_t sample_width = kpb->config.sampling_width; - uint32_t hb_free_space; uint32_t flags = 0; comp_dbg(dev, "kpb_copy()"); @@ -683,13 +682,13 @@ static int kpb_copy(struct comp_dev *dev) comp_err(dev, "kpb_copy(): internal buffering failed."); goto out; } - if (kpb->buffered_data < kpb->buffer_size) { - hb_free_space = kpb->buffer_size - - kpb->buffered_data; - kpb->hd.buffered += - MIN(copy_bytes, hb_free_space); - } + /* Update buffered size. NOTE! We only record buffered + * data up to the size of history buffer. + */ + kpb->hd.buffered += MIN(kpb->hd.buffer_size - + kpb->hd.buffered, + copy_bytes); } else { comp_err(dev, "kpb_copy(): too much data to buffer."); } From 69a4aa7837dd23b2697855aa39499dd6cabe6678 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 14:20:28 +0200 Subject: [PATCH 07/10] kpb: change error message to warning This patch changes the error message caused by no bytes to be buffered from "error" to warning as this is not crirical error. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 215e1a13596f..d6e5626946f6 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -751,8 +751,8 @@ static int kpb_copy(struct comp_dev *dev) comp_update_buffer_consume(source, copy_bytes); } else { - comp_err(dev, "kpb_copy(): buffering skipped (no data to copy, avail %d, free %d", - source->stream.avail, kpb->hd.free); + comp_warn(dev, "kpb_copy(): buffering skipped (no data to copy, avail %d, free %d", + source->stream.avail, kpb->hd.free); } ret = PPL_STATUS_PATH_STOP; From 471c2c4f68132b0960384f6b0dc06e3eb425e99a Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 14:30:03 +0200 Subject: [PATCH 08/10] kpb: move update of buffered data to higher layer This patch moves the update of buffered data during draining from buffering function to the caller function. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index d6e5626946f6..08b78a1406fe 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -615,6 +615,7 @@ static int kpb_copy(struct comp_dev *dev) size_t copy_bytes = 0; size_t sample_width = kpb->config.sampling_width; uint32_t flags = 0; + struct draining_data *dd = &kpb->draining_task_data; comp_dbg(dev, "kpb_copy()"); @@ -744,6 +745,8 @@ static int kpb_copy(struct comp_dev *dev) if (copy_bytes) { buffer_invalidate(source, copy_bytes); ret = kpb_buffer_data(dev, source, copy_bytes); + dd->buffered_while_draining += copy_bytes; + if (ret) { comp_err(dev, "kpb_copy(): internal buffering failed."); goto out; @@ -788,7 +791,6 @@ static int kpb_buffer_data(struct comp_dev *dev, uint64_t timeout = 0; uint64_t current_time; enum kpb_state state_preserved = kpb->state; - struct draining_data *draining_data = &kpb->draining_task_data; size_t sample_width = kpb->config.sampling_width; struct timer *timer = timer_get(); @@ -806,9 +808,6 @@ static int kpb_buffer_data(struct comp_dev *dev, return PPL_STATUS_PATH_STOP; } - if (kpb->state == KPB_STATE_DRAINING) - draining_data->buffered_while_draining += size_to_copy; - kpb_change_state(kpb, KPB_STATE_BUFFERING); timeout = platform_timer_get(timer) + From 592549afbed1e7867c9e2196ff7f2f6b4f4b1072 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 14:37:53 +0200 Subject: [PATCH 09/10] kpb: change order of members in comp_data struct This patch changes the order of members in comp_data struct to improve memory access for related data. So now data that are commonly used together like *state* and *state_log* are put next to each other so fewer cache reads are needed. Signed-off-by: Marcin Rajwa --- src/audio/kpb.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 08b78a1406fe..2dc6ba18c424 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -56,24 +56,24 @@ DECLARE_SOF_UUID("kpb-task", kpb_task_uuid, 0xe50057a5, 0x8b27, 0x4db4, /* KPB private data, runtime data */ struct comp_data { - uint64_t state_log; /**< keeps record of KPB recent states */ enum kpb_state state; /**< current state of KPB component */ - uint32_t kpb_no_of_clients; /**< number of registered clients */ - struct kpb_client clients[KPB_MAX_NO_OF_CLIENTS]; - struct task draining_task; - uint32_t source_period_bytes; /**< source number of period bytes */ - uint32_t sink_period_bytes; /**< sink number of period bytes */ + uint64_t state_log; /**< keeps record of KPB recent states */ + spinlock_t lock; /**< locking mechanism for read pointer calculations */ struct sof_kpb_config config; /**< component configuration data */ + struct history_data hd; /** data related to history buffer */ + struct task draining_task; + struct draining_data draining_task_data; + struct kpb_client clients[KPB_MAX_NO_OF_CLIENTS]; struct comp_buffer *sel_sink; /**< real time sink (channel selector )*/ struct comp_buffer *host_sink; /**< draining sink (client) */ - struct draining_data draining_task_data; + uint32_t kpb_no_of_clients; /**< number of registered clients */ + uint32_t source_period_bytes; /**< source number of period bytes */ + uint32_t sink_period_bytes; /**< sink number of period bytes */ 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 * host? */ - struct history_data hd; /** data related to history buffer */ - spinlock_t lock; /**< locking mechanism for read pointer calculations */ }; /*! KPB private functions */ From 79a75a4883e3b46d90b48efbf7512b967bd15bc8 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 1 Apr 2020 15:21:17 +0200 Subject: [PATCH 10/10] topology: update min host buffer size for KWD This updates the minimal host buffer size to the minimum value which enables sefe draining for all pcm parameters. Signed-off-by: Marcin Rajwa --- tools/topology/sof/pipe-kfbm-capture.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/topology/sof/pipe-kfbm-capture.m4 b/tools/topology/sof/pipe-kfbm-capture.m4 index 1f7a094c38e3..7432ca69d92b 100644 --- a/tools/topology/sof/pipe-kfbm-capture.m4 +++ b/tools/topology/sof/pipe-kfbm-capture.m4 @@ -90,5 +90,5 @@ indir(`define', concat(`PIPELINE_PCM_', PIPELINE_ID), N_PCMC(PCM_ID)) dnl PCM_CAPTURE_LP_ADD(name, pipeline, capture) PCM_CAPTURE_LP_ADD(DMIC16k, PCM_ID, N_STS(PCM_ID)) -PCM_CAPABILITIES(N_STS(PCM_ID), `S32_LE,S24_LE,S16_LE', 16000, 16000, 2, PIPELINE_CHANNELS, 2, 160, 192, 256000, 256000, 1280000) +PCM_CAPABILITIES(N_STS(PCM_ID), `S32_LE,S24_LE,S16_LE', 16000, 16000, 2, PIPELINE_CHANNELS, 2, 160, 192, 256000, 537600, 1280000)