[Sound-open-firmware] [PATCH v3 0/7] Fix capture doesn't work issue
This series fix the capture doesn't work after W/R pointer check implemented: 1. Make sure the component .params() works for capture; 2. Fix host side buffer for capture; 3. rename host side buffer pointer to make it more understandable for capture also. 4. integrate produce and consume version host_update_buffer to fix capture issue. 5. make the host buffer pointers check configurable. Changes in v3: 1. remove superfluous prefix 'host_' for members in host_data struct and rename to make them more understandable; 2. make the host buffer pointers check configurable, when disable, will save about 0.6KB in firmware binary size. Changes in v2: 1. change to use w_ptr/r_ptr host buffer pointers to make it easy to understand. Keyon Jie (7): component: set source buffer params for capture component .params() host: only update host_avail and host_free in host_update_buffer_xx() host: host_copy(): add handle for capture host: change host buffer pointer to w_ptr/r_ptr and update comments host: integrate consume and produce to host_update_buffer host: remove 'host_' prefix for host_data struct host: add HOST_SIDE_BUFFER_CHECK to make host side buffer check configurable src/audio/host.c | 173 +++++++++++++++++++++++-------------- src/audio/volume.c | 39 ++++++--- src/include/reef/audio/component.h | 13 +++ 3 files changed, 145 insertions(+), 80 deletions(-) -- 2.7.4
Considering of capture pipeline, when doing .params(), we need set params to each component's source buffer. This patch implement that for host/volume component, as mixer is only for playback ATM, and dai don't have source buffer for capture, we don't need change them ATM. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com> --- src/audio/host.c | 7 ++++++- src/audio/volume.c | 39 ++++++++++++++++++++++++-------------- src/include/reef/audio/component.h | 13 +++++++++++++ 3 files changed, 44 insertions(+), 15 deletions(-) diff --git a/src/audio/host.c b/src/audio/host.c index 069c6a2..12aee59 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -535,10 +535,12 @@ static int host_params(struct comp_dev *dev, struct stream_params *params) /* set params */ hd->params = *params; - comp_set_sink_params(dev, params); /* determine source and sink buffer elems */ if (params->direction == STREAM_DIRECTION_PLAYBACK) { + /* set sink buffer params */ + comp_set_sink_params(dev, params); + hd->source = &hd->host; hd->sink = &hd->local; hd->dma_buffer = list_first_item(&dev->bsink_list, @@ -546,6 +548,9 @@ static int host_params(struct comp_dev *dev, struct stream_params *params) hd->period = &hd->dma_buffer->desc.source_period; config->direction = DMA_DIR_HMEM_TO_LMEM; } else { + /* set source buffer params */ + comp_set_source_params(dev, params); + hd->source = &hd->local; hd->sink = &hd->host; hd->dma_buffer = list_first_item(&dev->bsource_list, diff --git a/src/audio/volume.c b/src/audio/volume.c index 1eb6004..bdaf87f 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -341,26 +341,37 @@ static void volume_free(struct comp_dev *dev) /* set component audio stream paramters */ static int volume_params(struct comp_dev *dev, struct stream_params *params) { - struct stream_params sink_params = *params; - struct comp_buffer *sink; - - /* volume components will only ever have 1 source and 1 sink buffer */ - sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); + struct stream_params buffer_params = *params; + struct comp_buffer *next_buf; + struct comp_dev *next_dev; + + if (params->direction == STREAM_DIRECTION_PLAYBACK) { + /* volume components will only ever have 1 sink buffer */ + next_buf = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); + next_dev = next_buf->sink; + } else { + /* volume components will only ever have 1 source buffer */ + next_buf = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); + next_dev = next_buf->source; + } /* hard coded until new IPC is ready */ - if (sink->sink->is_host) { - sink_params.pcm.format = STREAM_FORMAT_S16_LE; - sink_params.frame_size = 2 * params->channels; /* 16bit container */ - } else if (sink->sink->is_dai) { - sink_params.pcm.format = PLATFORM_SSP_STREAM_FORMAT; - sink_params.frame_size = 4 * params->channels; /* 32bit container */ + if (next_dev->is_host) { + buffer_params.pcm.format = STREAM_FORMAT_S16_LE; + buffer_params.frame_size = 2 * params->channels; /* 16bit container */ + } else if (next_dev->is_dai) { + buffer_params.pcm.format = PLATFORM_SSP_STREAM_FORMAT; + buffer_params.frame_size = 4 * params->channels; /* 32bit container */ } else { - sink_params.pcm.format = STREAM_FORMAT_S32_LE; - sink_params.frame_size = 4 * params->channels; /* 32bit container */ + buffer_params.pcm.format = STREAM_FORMAT_S32_LE; + buffer_params.frame_size = 4 * params->channels; /* 32bit container */ } /* dont do any data transformation */ - comp_set_sink_params(dev, &sink_params); + if (params->direction == STREAM_DIRECTION_PLAYBACK) + comp_set_sink_params(dev, &buffer_params); + else + comp_set_source_params(dev, &buffer_params); return 0; } diff --git a/src/include/reef/audio/component.h b/src/include/reef/audio/component.h index 1e784f5..be45d81 100644 --- a/src/include/reef/audio/component.h +++ b/src/include/reef/audio/component.h @@ -420,4 +420,17 @@ static inline void comp_set_sink_params(struct comp_dev *dev, } } +static inline void comp_set_source_params(struct comp_dev *dev, + struct stream_params *params) +{ + struct list_item *clist; + struct comp_buffer *source; + + list_for_item(clist, &dev->bsource_list) { + + source = container_of(clist, struct comp_buffer, sink_list); + source->params = *params; + } +} + #endif -- 2.7.4
We should not modify host_avail or host_free directly, and need replace that with host_update_buffer_produce/consume(). Theoretically, each time we update host_app_pos, host_pos_read, or host_size, we should call host_update_buffer_xx() to update the buffer. Here we add update to buffer new, host prepare, AVAIL_UPDATE cmd, and pointer_reset. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com> --- src/audio/host.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/audio/host.c b/src/audio/host.c index 12aee59..b604ddc 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -638,6 +638,8 @@ static int host_prepare(struct comp_dev *dev) hd->params.period_frames * hd->params.frame_size; hd->split_remaining = 0; + host_update_buffer_consume(hd); + dev->preload = PLAT_HOST_PERIODS; dev->state = COMP_STATE_PREPARE; @@ -674,8 +676,8 @@ static int host_pointer_reset(struct comp_dev *dev) hd->host_app_pos = 0; hd->host_pos_read = 0; hd->host_period_pos = 0; - hd->host_size = 0; - hd->host_avail= 0; + + host_update_buffer_consume(hd); return 0; } @@ -732,7 +734,11 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data) case COMP_CMD_AVAIL_UPDATE: app_pos = (struct ipc_intel_ipc_stream_set_position *)data; hd->host_app_pos = app_pos->position; - host_update_buffer_produce(hd); + + if (hd->params.direction == STREAM_DIRECTION_PLAYBACK) + host_update_buffer_produce(hd); + else + host_update_buffer_consume(hd); break; case COMP_CMD_VOLUME: vol_dev = host_volume_component(dev); @@ -760,6 +766,8 @@ static int host_buffer(struct comp_dev *dev, struct dma_sg_elem *elem, *e = *elem; hd->host_size = host_size; + host_update_buffer_consume(hd); + list_item_append(&e->list, &hd->host.elem_list); return 0; } -- 2.7.4
For playback, don't copy if there is no available data; For capture, don't copy if there is no free host side buffer. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com> --- src/audio/host.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/audio/host.c b/src/audio/host.c index b604ddc..f15dd33 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -815,7 +815,12 @@ static int host_copy(struct comp_dev *dev) if (dev->state != COMP_STATE_RUNNING) return 0; - if (hd->host_avail == 0) + /* don't copy if the host side buffer is not ready */ + if ((hd->params.direction == STREAM_DIRECTION_PLAYBACK) + && (hd->host_avail == 0)) + return 0; + if ((hd->params.direction == STREAM_DIRECTION_CAPTURE) + && (hd->host_free == 0)) return 0; /* do DMA transfer */ -- 2.7.4
We are using host_pos_read not only for playback, for capture, using it as the host side buffer write pointer, this is somewhat confusing, to make it more clear and simple to understand, here change to use w_ptr and r_ptr, and add comments to explain that. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com> --- src/audio/host.c | 74 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/src/audio/host.c b/src/audio/host.c index f15dd33..419aca3 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -71,11 +71,11 @@ struct host_data { struct hc_buf host; struct hc_buf local; uint32_t host_size; - volatile uint32_t *host_pos; /* read pos, update to mailbox for host side */ - uint32_t host_pos_read; /* host buffer read pos in bytes */ - uint32_t host_period_bytes; /* host period size in bytes */ - uint32_t host_period_pos; /* position in current host perid */ - uint32_t host_app_pos; /* host buffer app write pos, points to mailbox */ + volatile uint32_t *host_pos; /* read/write pos, update to mailbox for host side */ + uint32_t host_w_ptr; /* the host buffer write pos in bytes */ + uint32_t host_r_ptr; /* the host buffer read pos in bytes */ + uint32_t host_period_bytes; /* host period size(host_pos update period) in bytes */ + uint32_t host_period_pos; /* position in current host period */ uint32_t host_avail; /* host buffer available size */ uint32_t host_free; /* host buffer free size */ /* pointers set during params to host or local above */ @@ -89,27 +89,25 @@ struct host_data { struct comp_position cp; }; -static inline void host_update_buffer_produce(struct host_data *hd) +static void host_update_buffer_produce(struct host_data *hd) { - if (hd->host_pos_read < hd->host_app_pos) - hd->host_avail = hd->host_app_pos - hd->host_pos_read; - else if (hd->host_pos_read == hd->host_app_pos) - hd->host_avail = hd->host_size; /* full */ + if (hd->host_r_ptr < hd->host_w_ptr) + hd->host_avail = hd->host_w_ptr - hd->host_r_ptr; + else if (hd->host_r_ptr == hd->host_w_ptr) + hd->host_avail = hd->host_size; else - hd->host_avail = hd->host_size -hd->host_pos_read + - hd->host_app_pos; + hd->host_avail = hd->host_size -hd->host_r_ptr + hd->host_w_ptr; hd->host_free = hd->host_size - hd->host_avail; } -static inline void host_update_buffer_consume(struct host_data *hd) +static void host_update_buffer_consume(struct host_data *hd) { - if (hd->host_pos_read < hd->host_app_pos) - hd->host_avail = hd->host_app_pos - hd->host_pos_read; - else if (hd->host_pos_read == hd->host_app_pos) - hd->host_avail = 0; /* empty */ + if (hd->host_r_ptr < hd->host_w_ptr) + hd->host_avail = hd->host_w_ptr - hd->host_r_ptr; + else if (hd->host_r_ptr == hd->host_w_ptr) + hd->host_avail = 0; else - hd->host_avail = hd->host_size -hd->host_pos_read + - hd->host_app_pos; + hd->host_avail = hd->host_size -hd->host_r_ptr + hd->host_w_ptr; hd->host_free = hd->host_size - hd->host_avail; } @@ -163,11 +161,11 @@ static void host_dma_cb_playback(struct comp_dev *dev, comp_update_buffer_produce(hd->dma_buffer); /* new local period, update host buffer position blks */ - hd->host_pos_read += local_elem->size; + hd->host_r_ptr += local_elem->size; /* buffer overlap ? */ - if (hd->host_pos_read >= hd->host_size) - hd->host_pos_read = 0; + if (hd->host_r_ptr >= hd->host_size) + hd->host_r_ptr = 0; host_update_buffer_consume(hd); /* send IPC message to driver if needed */ @@ -178,7 +176,7 @@ static void host_dma_cb_playback(struct comp_dev *dev, if (hd->host_avail) { /* update for host side */ if (hd->host_pos) { - *hd->host_pos = hd->host_pos_read; + *hd->host_pos = hd->host_r_ptr; ipc_stream_send_notification(dev, &hd->cp); } } @@ -281,13 +279,13 @@ static void host_dma_cb_capture(struct comp_dev *dev, #endif /* new local period, update host buffer position blks */ - hd->host_pos_read += local_elem->size; + hd->host_w_ptr += local_elem->size; /* buffer overlap ? */ - if (hd->host_pos_read >= hd->host_size) - hd->host_pos_read = 0; + if (hd->host_w_ptr >= hd->host_size) + hd->host_w_ptr = 0; if (hd->host_pos) - *hd->host_pos = hd->host_pos_read; + *hd->host_pos = hd->host_w_ptr; /* recalc available buffer space */ comp_update_buffer_consume(hd->dma_buffer); @@ -381,7 +379,9 @@ static uint32_t host_finish_work(void *data, uint32_t udelay) trace_comp("hFw"); /* update for host side */ if (hd->host_pos) { - *hd->host_pos = hd->host_pos_read; + *hd->host_pos = + hd->params.direction == STREAM_DIRECTION_PLAYBACK ? + hd->host_r_ptr : hd->host_w_ptr; /* send the last notification to host */ ipc_stream_send_notification(dev, &hd->cp); } @@ -617,12 +617,16 @@ static int host_prepare(struct comp_dev *dev) struct comp_buffer *dma_buffer; if (hd->params.direction == STREAM_DIRECTION_PLAYBACK) { + hd->host_r_ptr = 0; + dma_buffer = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); dma_buffer->r_ptr = dma_buffer->addr; dma_buffer->w_ptr = dma_buffer->addr; } else { + hd->host_w_ptr = 0; + dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); @@ -632,7 +636,6 @@ static int host_prepare(struct comp_dev *dev) if (hd->host_pos) *hd->host_pos = 0; - hd->host_pos_read = 0; hd->host_period_pos = 0; hd->host_period_bytes = hd->params.period_frames * hd->params.frame_size; @@ -673,8 +676,8 @@ static int host_pointer_reset(struct comp_dev *dev) /* reset buffer pointers */ if (hd->host_pos) *hd->host_pos = 0; - hd->host_app_pos = 0; - hd->host_pos_read = 0; + hd->host_r_ptr = 0; + hd->host_w_ptr = 0; hd->host_period_pos = 0; host_update_buffer_consume(hd); @@ -733,12 +736,15 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data) break; case COMP_CMD_AVAIL_UPDATE: app_pos = (struct ipc_intel_ipc_stream_set_position *)data; - hd->host_app_pos = app_pos->position; - if (hd->params.direction == STREAM_DIRECTION_PLAYBACK) + if (hd->params.direction == STREAM_DIRECTION_PLAYBACK) { + hd->host_w_ptr = app_pos->position; host_update_buffer_produce(hd); - else + } + else { + hd->host_r_ptr = app_pos->position; host_update_buffer_consume(hd); + } break; case COMP_CMD_VOLUME: vol_dev = host_volume_component(dev); -- 2.7.4
For capture, the read/write pointers are opposite, here we integrate produce/consume to host_update_buffer(hd, produce). We also change 'inline' to real function to save code size at the same time. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com> --- src/audio/host.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/audio/host.c b/src/audio/host.c index 419aca3..92b5cf3 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -89,23 +89,12 @@ struct host_data { struct comp_position cp; }; -static void host_update_buffer_produce(struct host_data *hd) +static void host_update_buffer(struct host_data *hd, uint32_t produce) { if (hd->host_r_ptr < hd->host_w_ptr) hd->host_avail = hd->host_w_ptr - hd->host_r_ptr; else if (hd->host_r_ptr == hd->host_w_ptr) - hd->host_avail = hd->host_size; - else - hd->host_avail = hd->host_size -hd->host_r_ptr + hd->host_w_ptr; - hd->host_free = hd->host_size - hd->host_avail; -} - -static void host_update_buffer_consume(struct host_data *hd) -{ - if (hd->host_r_ptr < hd->host_w_ptr) - hd->host_avail = hd->host_w_ptr - hd->host_r_ptr; - else if (hd->host_r_ptr == hd->host_w_ptr) - hd->host_avail = 0; + hd->host_avail = produce ? hd->host_size : 0; else hd->host_avail = hd->host_size -hd->host_r_ptr + hd->host_w_ptr; hd->host_free = hd->host_size - hd->host_avail; @@ -166,7 +155,7 @@ static void host_dma_cb_playback(struct comp_dev *dev, /* buffer overlap ? */ if (hd->host_r_ptr >= hd->host_size) hd->host_r_ptr = 0; - host_update_buffer_consume(hd); + host_update_buffer(hd, 0); /* consume */ /* send IPC message to driver if needed */ hd->host_period_pos += local_elem->size; @@ -641,7 +630,7 @@ static int host_prepare(struct comp_dev *dev) hd->params.period_frames * hd->params.frame_size; hd->split_remaining = 0; - host_update_buffer_consume(hd); + host_update_buffer(hd, 0); /* consume */ dev->preload = PLAT_HOST_PERIODS; @@ -680,7 +669,7 @@ static int host_pointer_reset(struct comp_dev *dev) hd->host_w_ptr = 0; hd->host_period_pos = 0; - host_update_buffer_consume(hd); + host_update_buffer(hd, 0); /* consume */ return 0; } @@ -739,11 +728,11 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data) if (hd->params.direction == STREAM_DIRECTION_PLAYBACK) { hd->host_w_ptr = app_pos->position; - host_update_buffer_produce(hd); + host_update_buffer(hd, 1); /* produce */ } else { hd->host_r_ptr = app_pos->position; - host_update_buffer_consume(hd); + host_update_buffer(hd, 0); /* consume */ } break; case COMP_CMD_VOLUME: @@ -772,7 +761,7 @@ static int host_buffer(struct comp_dev *dev, struct dma_sg_elem *elem, *e = *elem; hd->host_size = host_size; - host_update_buffer_consume(hd); + host_update_buffer(hd, 0); /* consume */ list_item_append(&e->list, &hd->host.elem_list); return 0; -- 2.7.4
The struct host_data is designed to host the host component specific datas, so 'host_' prefix there looks superfluous. Here remove them, rename some members of host_data, and change comments to make them understandable. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com> --- src/audio/host.c | 92 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/src/audio/host.c b/src/audio/host.c index 92b5cf3..195cd6c 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -72,12 +72,12 @@ struct host_data { struct hc_buf local; uint32_t host_size; volatile uint32_t *host_pos; /* read/write pos, update to mailbox for host side */ - uint32_t host_w_ptr; /* the host buffer write pos in bytes */ - uint32_t host_r_ptr; /* the host buffer read pos in bytes */ - uint32_t host_period_bytes; /* host period size(host_pos update period) in bytes */ - uint32_t host_period_pos; /* position in current host period */ - uint32_t host_avail; /* host buffer available size */ - uint32_t host_free; /* host buffer free size */ + uint32_t buff_w_off; /* the host side buffer write offset in bytes */ + uint32_t buff_r_off; /* the host side buffer read offset in bytes */ + uint32_t report_period; /* host_pos report/update to host side period, in bytes */ + uint32_t report_pos; /* position in current report period */ + uint32_t buff_avail; /* host side buffer available size */ + uint32_t buff_free; /* host side buffer free size */ /* pointers set during params to host or local above */ struct hc_buf *source; struct hc_buf *sink; @@ -91,13 +91,13 @@ struct host_data { static void host_update_buffer(struct host_data *hd, uint32_t produce) { - if (hd->host_r_ptr < hd->host_w_ptr) - hd->host_avail = hd->host_w_ptr - hd->host_r_ptr; - else if (hd->host_r_ptr == hd->host_w_ptr) - hd->host_avail = produce ? hd->host_size : 0; + if (hd->buff_r_off < hd->buff_w_off) + hd->buff_avail = hd->buff_w_off - hd->buff_r_off; + else if (hd->buff_r_off == hd->buff_w_off) + hd->buff_avail = produce ? hd->host_size : 0; else - hd->host_avail = hd->host_size -hd->host_r_ptr + hd->host_w_ptr; - hd->host_free = hd->host_size - hd->host_avail; + hd->buff_avail = hd->host_size -hd->buff_r_off + hd->buff_w_off; + hd->buff_free = hd->host_size - hd->buff_avail; } static inline struct dma_sg_elem *next_buffer(struct hc_buf *hc) @@ -150,22 +150,22 @@ static void host_dma_cb_playback(struct comp_dev *dev, comp_update_buffer_produce(hd->dma_buffer); /* new local period, update host buffer position blks */ - hd->host_r_ptr += local_elem->size; + hd->buff_r_off += local_elem->size; /* buffer overlap ? */ - if (hd->host_r_ptr >= hd->host_size) - hd->host_r_ptr = 0; + if (hd->buff_r_off >= hd->host_size) + hd->buff_r_off = 0; host_update_buffer(hd, 0); /* consume */ /* send IPC message to driver if needed */ - hd->host_period_pos += local_elem->size; - if (hd->host_period_pos >= hd->host_period_bytes) { - hd->host_period_pos = 0; + hd->report_pos += local_elem->size; + if (hd->report_pos >= hd->report_period) { + hd->report_pos = 0; /* for the last bytes/period, send notification later */ - if (hd->host_avail) { + if (hd->buff_avail) { /* update for host side */ if (hd->host_pos) { - *hd->host_pos = hd->host_r_ptr; + *hd->host_pos = hd->buff_r_off; ipc_stream_send_notification(dev, &hd->cp); } } @@ -204,8 +204,8 @@ static void host_dma_cb_playback(struct comp_dev *dev, local_elem->size = next_size; /* check if avail is enough, otherwise, drain the last bytes and stop */ - if (hd->host_avail < local_elem->size) { - if (hd->host_avail == 0) { + if (hd->buff_avail < local_elem->size) { + if (hd->buff_avail == 0) { /* end of stream, stop */ next->size = DMA_RELOAD_END; need_copy = 0; @@ -217,12 +217,12 @@ static void host_dma_cb_playback(struct comp_dev *dev, } /* end of stream, drain the last bytes */ - local_elem->size = hd->host_avail; + local_elem->size = hd->buff_avail; /* the split_remaining may not be copied anymore, but, let's make it - correct. we have only hd->host_avail data, so the split_remaining - should be (next_size - hd->host_avail) bigger */ - hd->split_remaining += next_size - hd->host_avail; + correct. we have only hd->buff_avail data, so the split_remaining + should be (next_size - hd->buff_avail) bigger */ + hd->split_remaining += next_size - hd->buff_avail; } next_copy: @@ -268,21 +268,21 @@ static void host_dma_cb_capture(struct comp_dev *dev, #endif /* new local period, update host buffer position blks */ - hd->host_w_ptr += local_elem->size; + hd->buff_w_off += local_elem->size; /* buffer overlap ? */ - if (hd->host_w_ptr >= hd->host_size) - hd->host_w_ptr = 0; + if (hd->buff_w_off >= hd->host_size) + hd->buff_w_off = 0; if (hd->host_pos) - *hd->host_pos = hd->host_w_ptr; + *hd->host_pos = hd->buff_w_off; /* recalc available buffer space */ comp_update_buffer_consume(hd->dma_buffer); /* send IPC message to driver if needed */ - hd->host_period_pos += local_elem->size; - if (hd->host_period_pos >= hd->host_period_bytes) { - hd->host_period_pos = 0; + hd->report_pos += local_elem->size; + if (hd->report_pos >= hd->report_period) { + hd->report_pos = 0; ipc_stream_send_notification(dev, &hd->cp); } @@ -370,7 +370,7 @@ static uint32_t host_finish_work(void *data, uint32_t udelay) if (hd->host_pos) { *hd->host_pos = hd->params.direction == STREAM_DIRECTION_PLAYBACK ? - hd->host_r_ptr : hd->host_w_ptr; + hd->buff_r_off : hd->buff_w_off; /* send the last notification to host */ ipc_stream_send_notification(dev, &hd->cp); } @@ -606,7 +606,7 @@ static int host_prepare(struct comp_dev *dev) struct comp_buffer *dma_buffer; if (hd->params.direction == STREAM_DIRECTION_PLAYBACK) { - hd->host_r_ptr = 0; + hd->buff_r_off = 0; dma_buffer = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); @@ -614,7 +614,7 @@ static int host_prepare(struct comp_dev *dev) dma_buffer->r_ptr = dma_buffer->addr; dma_buffer->w_ptr = dma_buffer->addr; } else { - hd->host_w_ptr = 0; + hd->buff_w_off = 0; dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); @@ -625,8 +625,8 @@ static int host_prepare(struct comp_dev *dev) if (hd->host_pos) *hd->host_pos = 0; - hd->host_period_pos = 0; - hd->host_period_bytes = + hd->report_pos = 0; + hd->report_period = hd->params.period_frames * hd->params.frame_size; hd->split_remaining = 0; @@ -665,9 +665,9 @@ static int host_pointer_reset(struct comp_dev *dev) /* reset buffer pointers */ if (hd->host_pos) *hd->host_pos = 0; - hd->host_r_ptr = 0; - hd->host_w_ptr = 0; - hd->host_period_pos = 0; + hd->buff_r_off = 0; + hd->buff_w_off = 0; + hd->report_pos = 0; host_update_buffer(hd, 0); /* consume */ @@ -727,11 +727,11 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data) app_pos = (struct ipc_intel_ipc_stream_set_position *)data; if (hd->params.direction == STREAM_DIRECTION_PLAYBACK) { - hd->host_w_ptr = app_pos->position; + hd->buff_w_off = app_pos->position; host_update_buffer(hd, 1); /* produce */ } else { - hd->host_r_ptr = app_pos->position; + hd->buff_r_off = app_pos->position; host_update_buffer(hd, 0); /* consume */ } break; @@ -792,7 +792,7 @@ static int host_reset(struct comp_dev *dev) host_pointer_reset(dev); hd->host_pos = NULL; - hd->host_period_bytes = 0; + hd->report_period = 0; hd->source = NULL; hd->sink = NULL; dev->state = COMP_STATE_INIT; @@ -812,10 +812,10 @@ static int host_copy(struct comp_dev *dev) /* don't copy if the host side buffer is not ready */ if ((hd->params.direction == STREAM_DIRECTION_PLAYBACK) - && (hd->host_avail == 0)) + && (hd->buff_avail == 0)) return 0; if ((hd->params.direction == STREAM_DIRECTION_CAPTURE) - && (hd->host_free == 0)) + && (hd->buff_free == 0)) return 0; /* do DMA transfer */ -- 2.7.4
It is possible that we may don't like check host side buffer and assume there are always datas/room for playback/capture, so here we add a config item HOST_SIDE_BUFFER_CHECK to make it configurable, and disable this buffer check may save about 0.6KB to the firmware binary size. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com> --- src/audio/host.c | 50 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/src/audio/host.c b/src/audio/host.c index 195cd6c..80ee3fe 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -50,6 +50,8 @@ #define tracev_host(__e) tracev_event(TRACE_CLASS_HOST, __e) #define trace_host_error(__e) trace_error(TRACE_CLASS_HOST, __e) +//#define HOST_SIDE_BUFFER_CHECK + struct hc_buf { /* host buffer info */ struct list_item elem_list; @@ -65,19 +67,23 @@ struct host_data { completion_t complete; struct period_desc *period; struct comp_buffer *dma_buffer; - struct work work; /* local and host DMA buffer info */ struct hc_buf host; struct hc_buf local; uint32_t host_size; + /* host possition reporting related */ volatile uint32_t *host_pos; /* read/write pos, update to mailbox for host side */ - uint32_t buff_w_off; /* the host side buffer write offset in bytes */ - uint32_t buff_r_off; /* the host side buffer read offset in bytes */ - uint32_t report_period; /* host_pos report/update to host side period, in bytes */ - uint32_t report_pos; /* position in current report period */ - uint32_t buff_avail; /* host side buffer available size */ - uint32_t buff_free; /* host side buffer free size */ + uint32_t report_period; /* host_pos report/update to host side period, in bytes */ + uint32_t report_pos; /* position in current report period */ + uint32_t buff_w_off; /* the host side buffer write offset in bytes */ + uint32_t buff_r_off; /* the host side buffer read offset in bytes */ +#ifdef HOST_SIDE_BUFFER_CHECK + struct work work; + /* host side buffer management related */ + uint32_t buff_avail; /* host side buffer available size */ + uint32_t buff_free; /* host side buffer free size */ +#endif /* pointers set during params to host or local above */ struct hc_buf *source; struct hc_buf *sink; @@ -89,6 +95,7 @@ struct host_data { struct comp_position cp; }; +#ifdef HOST_SIDE_BUFFER_CHECK static void host_update_buffer(struct host_data *hd, uint32_t produce) { if (hd->buff_r_off < hd->buff_w_off) @@ -99,7 +106,11 @@ static void host_update_buffer(struct host_data *hd, uint32_t produce) hd->buff_avail = hd->host_size -hd->buff_r_off + hd->buff_w_off; hd->buff_free = hd->host_size - hd->buff_avail; } - +#else +static void host_update_buffer(struct host_data *hd, uint32_t produce) +{ +} +#endif static inline struct dma_sg_elem *next_buffer(struct hc_buf *hc) { struct dma_sg_elem *elem; @@ -161,6 +172,7 @@ static void host_dma_cb_playback(struct comp_dev *dev, hd->report_pos += local_elem->size; if (hd->report_pos >= hd->report_period) { hd->report_pos = 0; +#ifdef HOST_SIDE_BUFFER_CHECK /* for the last bytes/period, send notification later */ if (hd->buff_avail) { /* update for host side */ @@ -169,6 +181,13 @@ static void host_dma_cb_playback(struct comp_dev *dev, ipc_stream_send_notification(dev, &hd->cp); } } +#else + /* update for host side */ + if (hd->host_pos) { + *hd->host_pos = hd->buff_r_off; + ipc_stream_send_notification(dev, &hd->cp); + } +#endif } local_elem->src += local_elem->size; @@ -203,6 +222,7 @@ static void host_dma_cb_playback(struct comp_dev *dev, } local_elem->size = next_size; +#ifdef HOST_SIDE_BUFFER_CHECK /* check if avail is enough, otherwise, drain the last bytes and stop */ if (hd->buff_avail < local_elem->size) { if (hd->buff_avail == 0) { @@ -226,6 +246,7 @@ static void host_dma_cb_playback(struct comp_dev *dev, } next_copy: +#endif if (need_copy) { next->src = local_elem->src; next->dest = local_elem->dest; @@ -347,6 +368,7 @@ static void host_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next) host_dma_cb_capture(dev, next); } +#ifdef HOST_SIDE_BUFFER_CHECK /* We need to wait until the last bytes/period is finished in dai, before we * can notify host side about that, otherwise, host side will trigger stop too * early, and we will miss rendering them. @@ -378,8 +400,7 @@ static uint32_t host_finish_work(void *data, uint32_t udelay) return 0; } - - +#endif static struct comp_dev *host_new(uint32_t type, uint32_t index, uint32_t direction) @@ -410,8 +431,9 @@ static struct comp_dev *host_new(uint32_t type, uint32_t index, hd->dma = dma_get(DMA_ID_DMAC0); if (hd->dma == NULL) goto error; +#ifdef HOST_SIDE_BUFFER_CHECK work_init(&hd->work, host_finish_work, dev, WORK_ASYNC); - +#endif /* init buffer elems */ list_init(&hd->config.elem_list); list_init(&hd->host.elem_list); @@ -694,7 +716,9 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data) { struct host_data *hd = comp_get_drvdata(dev); struct comp_dev *vol_dev = NULL; +#ifdef HOST_SIDE_BUFFER_CHECK struct ipc_intel_ipc_stream_set_position *app_pos; +#endif int ret = 0; // TODO: align cmd macros. @@ -723,6 +747,7 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data) case COMP_CMD_IPC_MMAP_RPOS: hd->host_pos = data; break; +#ifdef HOST_SIDE_BUFFER_CHECK case COMP_CMD_AVAIL_UPDATE: app_pos = (struct ipc_intel_ipc_stream_set_position *)data; @@ -735,6 +760,7 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data) host_update_buffer(hd, 0); /* consume */ } break; +#endif case COMP_CMD_VOLUME: vol_dev = host_volume_component(dev); if (vol_dev != NULL) @@ -810,6 +836,7 @@ static int host_copy(struct comp_dev *dev) if (dev->state != COMP_STATE_RUNNING) return 0; +#ifdef HOST_SIDE_BUFFER_CHECK /* don't copy if the host side buffer is not ready */ if ((hd->params.direction == STREAM_DIRECTION_PLAYBACK) && (hd->buff_avail == 0)) @@ -817,6 +844,7 @@ static int host_copy(struct comp_dev *dev) if ((hd->params.direction == STREAM_DIRECTION_CAPTURE) && (hd->buff_free == 0)) return 0; +#endif /* do DMA transfer */ wait_init(&hd->complete); -- 2.7.4
participants (1)
-
Keyon Jie