-----Original Message----- From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com] Sent: Wednesday, February 15, 2017 6:49 PM To: Keyon Jie yang.jie@linux.intel.com Cc: sound-open-firmware@alsa-project.org; Zhang, Keqiao keqiao.zhang@intel.com; Jie, Yang yang.jie@intel.com; Ingalsuo, Seppo seppo.ingalsuo@intel.com Subject: Re: [Sound-open-firmware] [PATCH 4/5] host: rename host_pos_read to host_pos_local and update comments
On Wed, 2017-02-15 at 18:07 +0800, Keyon Jie wrote:
We are using host_pos_read not only for playback, for capture, we use it as the host side buffer write pointer, so, here rename it to host_pos_local and add comments to explain that.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
src/audio/host.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/src/audio/host.c b/src/audio/host.c index 61d2b62..f814603 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_pos_local; /* the real host buffer local read/write
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_app_pos; /* host buffer app write/read pos, udpate
via IPC */
Why dont we just rename to use r_ptr and w_ptr naming, that makes it very simple to understand. This way the copy() always updates read and write ptrs without worrying about naming.
Um, this means we may need more codes to judge the stream direction, e.g. if(stream->direction == playback) *host_pos = r_ptr; else *host_pos = w_ptr;
but let me think of that further and try to figure out the modification, to see if it can make life easier really.
Thanks, ~Keyon
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 */ @@ -91,24 +91,24 @@ struct host_data {
static inline 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)
- if (hd->host_pos_local < hd->host_app_pos)
hd->host_avail = hd->host_app_pos - hd->host_pos_local;- else if (hd->host_pos_local == hd->host_app_pos) hd->host_avail = hd->host_size; /* full */ else
hd->host_avail = hd->host_size -hd->host_pos_read +
hd->host_avail = hd->host_size -hd->host_pos_local + hd->host_app_pos;static inline 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)
- if (hd->host_pos_local < hd->host_app_pos)
hd->host_avail = hd->host_app_pos - hd->host_pos_local;- else if (hd->host_pos_local == hd->host_app_pos) hd->host_avail = 0; /* empty */ else
hd->host_avail = hd->host_size -hd->host_pos_read +
hd->host_avail = hd->host_size -hd->host_pos_local + hd->host_app_pos;+163,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_pos_local += local_elem->size;
/* buffer overlap ? */
- if (hd->host_pos_read >= hd->host_size)
hd->host_pos_read = 0;
if (hd->host_pos_local >= hd->host_size)
hd->host_pos_local = 0;host_update_buffer_consume(hd);
/* send IPC message to driver if needed */ @@ -178,7 +178,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_pos_local; ipc_stream_send_notification(dev, &hd->cp); }@@ -281,13 +281,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_pos_local += local_elem->size;
/* buffer overlap ? */
- if (hd->host_pos_read >= hd->host_size)
hd->host_pos_read = 0;
- if (hd->host_pos_local >= hd->host_size)
hd->host_pos_local = 0;
*hd->host_pos = hd->host_pos_read;
*hd->host_pos = hd->host_pos_local;/* recalc available buffer space */ comp_update_buffer_consume(hd->dma_buffer);
@@ -381,7 +381,7 @@ 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->host_pos_local; /* send the last notification to host */ ipc_stream_send_notification(dev, &hd->cp);@@ -632,7 +632,7 @@ static int host_prepare(struct comp_dev *dev)
if (hd->host_pos) *hd->host_pos = 0;
- hd->host_pos_read = 0;
- hd->host_pos_local = 0; hd->host_period_pos = 0; hd->host_period_bytes = hd->params.period_frames * hd->params.frame_size; @@ -
674,7 +674,7
@@ static int host_pointer_reset(struct comp_dev *dev) if (hd->host_pos) *hd->host_pos = 0; hd->host_app_pos = 0;
- hd->host_pos_read = 0;
hd->host_pos_local = 0; hd->host_period_pos = 0;
host_update_buffer_consume(hd);