-
Notifications
You must be signed in to change notification settings - Fork 367
kpb: add multi-KPB WOV arbiter for 3-keyword DMIC capture with VAD #11022
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b96f61d
33a997d
42e3ef6
cd1fc04
fc5fc9d
180ba7d
472640e
effdb4a
b377fd1
a7c3027
a79115b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,3 +31,8 @@ CONFIG_GDBSTUB_ENTER_IMMEDIATELY=n | |
| # Testing with runtime filtering enabled ensures the same feature set is validated. | ||
| # Note: This setting has no effect if CONFIG_LOG_RUNTIME_FILTERING is disabled. | ||
| CONFIG_LOG_RUNTIME_DEFAULT_LEVEL=3 | ||
|
|
||
| # Record fatal exception breadcrumbs (PC/cause/vaddr) in HP-SRAM window0 so the | ||
| # crash is visible in the host dmesg "Firmware state" line when no console or | ||
| # mtrace output is available. | ||
| CONFIG_XTENSA_ADSP_FATAL_BREADCRUMB=y | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not part of this patch. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,9 +104,15 @@ if(NOT CONFIG_COMP_MODULE_SHARED_LIBRARY_BUILD) | |
| if(CONFIG_COMP_UP_DOWN_MIXER) | ||
| add_subdirectory(up_down_mixer) | ||
| endif() | ||
| if(CONFIG_COMP_VAD_GATE) | ||
| add_subdirectory(vad_gate) | ||
| endif() | ||
|
Comment on lines
+107
to
+109
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be in the vad gate patch. |
||
| if(CONFIG_COMP_VOLUME) | ||
| add_subdirectory(volume) | ||
| endif() | ||
| if(CONFIG_COMP_WOV_ARBITER) | ||
| add_subdirectory(wov_arbiter) | ||
| endif() | ||
| if(CONFIG_DTS_CODEC) | ||
| add_subdirectory(codec) | ||
| endif() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,14 @@ | |
| #include <sof/audio/component_ext.h> | ||
| #include <sof/audio/pipeline.h> | ||
| #include <sof/audio/kpb.h> | ||
| #define SOF_MODULE_API_PRIVATE | ||
| #include <sof/audio/module_adapter/module/generic.h> | ||
| #include <module/module/base.h> | ||
| #include <sof/audio/ipc-config.h> | ||
| #include <sof/common.h> | ||
| #include <rtos/panic.h> | ||
| #include <sof/ipc/msg.h> | ||
| #include <sof/ipc/topology.h> | ||
| #include <rtos/timer.h> | ||
| #include <rtos/alloc.h> | ||
| #include <rtos/clk.h> | ||
|
|
@@ -369,6 +373,9 @@ static int kpb_bind(struct comp_dev *dev, struct bind_info *bind_data) | |
| sink_buf_id = buf_get_id(sink); | ||
|
|
||
| if (sink_buf_id == buf_id) { | ||
| struct comp_dev *sc = comp_buffer_get_sink_component(sink); | ||
| comp_err(dev, "kpb_bind: buf_id=%d sink_comp=0x%x -> %s", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this an error condition? |
||
| buf_id, sc ? dev_comp_id(sc) : 0, sink_buf_id == 0 ? "sel_sink" : "host_sink"); | ||
| if (sink_buf_id == 0) | ||
| kpb->sel_sink = sink; | ||
| else | ||
|
|
@@ -887,6 +894,38 @@ static int kpb_prepare(struct comp_dev *dev) | |
| return -ENOMEM; | ||
| } | ||
|
|
||
| struct comp_buffer *sink; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets comment why this is needed, and what we are doing in these 2 new blocks. This does look like it would benefit a utility API. |
||
| comp_dev_for_each_consumer(dev, sink) { | ||
| struct comp_dev *sc = comp_buffer_get_sink_component(sink); | ||
| if (sc) { | ||
| comp_err(dev, "kpb consumer in bsink_list: comp_id=0x%x type=%d sink_buf=%p", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replace |
||
| dev_comp_id(sc), sc->drv ? sc->drv->type : -1, sink); | ||
| if (dev_comp_id(sc) != 0x10) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is 0x10? |
||
| kpb->sel_sink = sink; | ||
| else | ||
| kpb->host_sink = sink; | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose the loop should run exactly twice to only set each of these pointers once? Should we check or at least add a comment? |
||
| } | ||
| comp_err(dev, "kpb_params result: sel_sink=%p host_sink=%p", | ||
| kpb->sel_sink, kpb->host_sink); | ||
|
|
||
| if (kpb->sel_sink) { | ||
| struct comp_dev *sink_comp = comp_buffer_get_sink_component(kpb->sel_sink); | ||
| if (sink_comp && sink_comp->state == COMP_STATE_INIT) { | ||
| struct sof_ipc_stream_params sink_params; | ||
| memset_s(&sink_params, sizeof(sink_params), 0, sizeof(sink_params)); | ||
| sink_params.channels = kpb->config.channels ? kpb->config.channels : 2; | ||
| sink_params.rate = kpb->config.sampling_freq ? kpb->config.sampling_freq : 16000; | ||
| sink_params.sample_container_bytes = 4; | ||
| sink_params.sample_valid_bytes = 4; | ||
| sink_params.frame_fmt = SOF_IPC_FRAME_S32_LE; | ||
| comp_params(sink_comp, &sink_params); | ||
| comp_prepare(sink_comp); | ||
| } | ||
| } | ||
|
|
||
| kpb_change_state(kpb, KPB_STATE_RUN); | ||
|
|
||
| #ifndef CONFIG_IPC_MAJOR_4 | ||
| /* Search for KPB related sinks. | ||
| * NOTE! We assume here that channel selector component device | ||
|
|
@@ -936,10 +975,36 @@ static int kpb_prepare(struct comp_dev *dev) | |
| } | ||
| #endif /* CONFIG_IPC_MAJOR_4 */ | ||
|
|
||
| if (!kpb->sel_sink && !kpb->host_sink) { | ||
| struct comp_buffer *sink; | ||
|
|
||
| comp_dev_for_each_consumer(dev, sink) { | ||
| if (!kpb->sel_sink) | ||
| kpb->sel_sink = sink; | ||
| else if (!kpb->host_sink) | ||
| kpb->host_sink = sink; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here - shouldn't be overwriting, right? |
||
| } | ||
| } | ||
|
|
||
| if (!kpb->sel_sink) { | ||
| comp_err(dev, "could not find sink: sel_sink %p", | ||
| kpb->sel_sink); | ||
| ret = -EIO; | ||
| } else { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto re comments. |
||
| struct comp_dev *sink_comp = comp_buffer_get_sink_component(kpb->sel_sink); | ||
| if (sink_comp && sink_comp->state == COMP_STATE_INIT) { | ||
| struct sof_ipc_stream_params sink_params; | ||
| memset_s(&sink_params, sizeof(sink_params), 0, sizeof(sink_params)); | ||
| sink_params.channels = kpb->config.channels ? kpb->config.channels : 2; | ||
| sink_params.rate = kpb->config.sampling_freq ? kpb->config.sampling_freq : 16000; | ||
| sink_params.sample_container_bytes = 4; | ||
| sink_params.sample_valid_bytes = 4; | ||
| sink_params.frame_fmt = SOF_IPC_FRAME_S32_LE; | ||
| comp_params(sink_comp, &sink_params); | ||
| comp_prepare(sink_comp); | ||
| comp_info(dev, "kpb_prepare: prepared downstream sink_comp %d in state %d", | ||
| dev_comp_id(sink_comp), sink_comp->state); | ||
| } | ||
| } | ||
|
|
||
| kpb->sync_draining_mode = true; | ||
|
|
@@ -981,11 +1046,30 @@ static int kpb_reset(struct comp_dev *dev) | |
| switch (kpb->state) { | ||
| case KPB_STATE_BUFFERING: | ||
| case KPB_STATE_DRAINING: | ||
| /* KPB is performing some task now, | ||
| * terminate it gently. | ||
| /* If a host drain is in progress, terminate gently and let | ||
| * kpb_copy complete the reset once scheduled. When there is | ||
| * no host_sink (WOV-only path) the scheduler has already | ||
| * stopped by the time RESET arrives, so reset immediately. | ||
| */ | ||
| kpb_change_state(kpb, KPB_STATE_RESETTING); | ||
| ret = -EBUSY; | ||
| if (kpb->host_sink) { | ||
| kpb_change_state(kpb, KPB_STATE_RESETTING); | ||
| ret = -EBUSY; | ||
| break; | ||
| } | ||
| /* host_sink == NULL: immediate full reset (same as default) */ | ||
| kpb->hd.buffered = 0; | ||
| kpb->sel_sink = NULL; | ||
| kpb->host_sink = NULL; | ||
| kpb->host_buffer_size = 0; | ||
| kpb->host_period_size = 0; | ||
| for (i = 0; i < KPB_MAX_NO_OF_CLIENTS; i++) { | ||
| kpb->clients[i].state = KPB_CLIENT_UNREGISTERED; | ||
| kpb->clients[i].r_ptr = NULL; | ||
| } | ||
| if (kpb->hd.c_hb) | ||
| kpb_reset_history_buffer(kpb->hd.c_hb); | ||
| kpb_change_state(kpb, KPB_STATE_PREPARING); | ||
| ret = comp_set_state(dev, COMP_TRIGGER_RESET); | ||
| break; | ||
| case KPB_STATE_DISABLED: | ||
| case KPB_STATE_CREATED: | ||
|
|
@@ -1234,19 +1318,16 @@ static int kpb_copy(struct comp_dev *dev) | |
| sink = kpb->sel_sink; | ||
| ret = PPL_STATUS_PATH_STOP; | ||
|
|
||
| comp_dbg(dev, "kpb_copy: source_buf=%p sel_sink=%p avail=%u", | ||
| source, sink, audio_stream_get_avail_bytes(&source->stream)); | ||
|
|
||
| if (!sink) { | ||
| comp_err(dev, "no sink."); | ||
| ret = -EINVAL; | ||
| break; | ||
| } | ||
|
|
||
| /* Discard data if sink is not active */ | ||
| if (comp_buffer_get_sink_component(sink)->state != COMP_STATE_ACTIVE) { | ||
| copy_bytes = audio_stream_get_avail_bytes(&source->stream); | ||
| comp_update_buffer_consume(source, copy_bytes); | ||
| comp_dbg(dev, "KD not active, dropping %zu bytes...", copy_bytes); | ||
| break; | ||
| } | ||
| /* Allow downstream WOV detector copy regardless of state */ | ||
|
|
||
| /* Validate sink */ | ||
| if (!audio_stream_get_wptr(&sink->stream)) { | ||
|
|
@@ -1313,6 +1394,15 @@ static int kpb_copy(struct comp_dev *dev) | |
| else | ||
| comp_update_buffer_produce(sink, produced_bytes); | ||
|
|
||
| struct comp_dev *wov_comp = sink ? comp_buffer_get_sink_component(sink) : NULL; | ||
| if (wov_comp) { | ||
| comp_dbg(dev, "kpb_copy: produced=%u bytes, triggering wov=0x%x", | ||
| copy_bytes, dev_comp_id(wov_comp)); | ||
| comp_copy(wov_comp); | ||
| } else { | ||
| comp_err(dev, "kpb_copy: downstream sink_comp returned NULL!"); | ||
| } | ||
|
|
||
| comp_update_buffer_consume(source, copy_bytes); | ||
|
|
||
| break; | ||
|
|
@@ -1607,6 +1697,28 @@ static int kpb_register_client(struct comp_data *kpb, struct kpb_client *cli) | |
| static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) | ||
| { | ||
| struct comp_data *kpb = comp_get_drvdata(dev); | ||
|
|
||
| if (!kpb->host_sink) { | ||
| if (!kpb->sel_sink) { | ||
| comp_warn(dev, "kpb_init_draining: no drain path, skipping"); | ||
| return; | ||
| } | ||
| /* WOV-only path: no dedicated host PCM sink. Route history drain | ||
| * through sel_sink so wov passthrough delivers it to the arbiter. | ||
| * Set host_period_size to one real-time period so sync_draining_mode | ||
| * throttles the EDF drain task to match the LL pipeline rate. | ||
| */ | ||
| comp_warn(dev, "kpb_init_draining: no host_sink, draining via sel_sink"); | ||
| kpb->host_sink = kpb->sel_sink; | ||
| if (!kpb->host_period_size) { | ||
| size_t bpm = (size_t)KPB_SAMPLES_PER_MS * | ||
| (KPB_SAMPLE_CONTAINER_SIZE(kpb->config.sampling_width) / 8) * | ||
| kpb->config.channels; | ||
| kpb->host_period_size = bpm; | ||
| kpb->host_buffer_size = audio_stream_get_size(&kpb->sel_sink->stream); | ||
| } | ||
| } | ||
|
|
||
| bool is_sink_ready = (comp_buffer_get_sink_state(kpb->host_sink) == COMP_STATE_ACTIVE); | ||
| size_t sample_width = kpb->config.sampling_width; | ||
| size_t drain_req = (size_t)cli->drain_req * kpb->config.channels * | ||
|
|
@@ -1633,14 +1745,16 @@ 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, "sink not ready for draining"); | ||
| } else if (kpb->hd.buffered < drain_req || | ||
| cli->drain_req > KPB_MAX_DRAINING_REQ) { | ||
| comp_cl_err(&comp_kpb, "not enough data in history buffer"); | ||
| } else if (cli->drain_req > KPB_MAX_DRAINING_REQ) { | ||
| comp_cl_err(&comp_kpb, "drain request exceeds max"); | ||
| } else { | ||
| /* Draining accepted, find proper buffer to start reading | ||
| * At this point we are guaranteed that there is enough data | ||
| * in the history buffer. All we have to do now is to calculate | ||
| * read pointer from which we will start draining. | ||
| if (kpb->hd.buffered < drain_req) { | ||
| comp_cl_warn(&comp_kpb, "partial pre-roll: capping drain to buffered"); | ||
| drain_req = kpb->hd.buffered; | ||
| } | ||
| /* Draining accepted, find proper buffer to start reading. | ||
| * If less history than requested is buffered, drain_req is | ||
| * capped above so we drain whatever is available. | ||
| */ | ||
| kpb_lock(kpb); | ||
|
|
||
|
|
@@ -1750,8 +1864,11 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) | |
| comp_set_attribute(comp_buffer_get_sink_component(kpb->host_sink), | ||
| COMP_ATTR_COPY_TYPE, &kpb->force_copy_type); | ||
|
|
||
| /* Pause selector copy. */ | ||
| comp_buffer_get_sink_component(kpb->sel_sink)->state = COMP_STATE_PAUSED; | ||
| /* Pause selector copy to stop detection on stale drain data. | ||
| * Skip when sel_sink IS the drain path (wov passthrough needed). | ||
| */ | ||
| if (kpb->host_sink != kpb->sel_sink) | ||
| comp_buffer_get_sink_component(kpb->sel_sink)->state = COMP_STATE_PAUSED; | ||
|
|
||
| if (!pm_runtime_is_active(PM_RUNTIME_DSP, PLATFORM_PRIMARY_CORE_ID)) | ||
| pm_runtime_disable(PM_RUNTIME_DSP, PLATFORM_PRIMARY_CORE_ID); | ||
|
|
@@ -2368,7 +2485,7 @@ static void kpb_reset_history_buffer(struct history_buffer *buff) | |
| if (!buff) | ||
| return; | ||
|
|
||
| kpb_clear_history_buffer(buff); | ||
|
|
||
|
|
||
| do { | ||
| buff->w_ptr = buff->start_addr; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be =m