[Sound-open-firmware] [PATCH v2 4/5] host: change host buffer pointer to w_ptr/r_ptr and update comments
Keyon Jie
yang.jie at linux.intel.com
Tue Feb 21 06:42:54 CET 2017
On 2017年02月21日 03:56, Liam Girdwood wrote:
> On Thu, 2017-02-16 at 16:08 +0800, Keyon Jie wrote:
>> 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 at 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 */
>
> Do we really need the host side buffer data ? We only need to know the
> host period size, buffer size and base address. We can get rid of the
> rest except anything used to calculate whether a host period has
> elapsed.
The host_period_bytes, host_period_pos, and *host_pos are used for host
period elapsed calculating and sharing, we don't have much rest items
here, most of them are for buffer status management(w_ptr/r_ptr/avail/free).
>
> One other thing is that the host component used to create a list of
> period elems for local and host buffers. We dont use the elem list for
> host side any more so this can be removed too.
Currently we are still using element list for host buffers, at least for
byt, we cannot remove it ATM, right?
>
> We also dont need the host prefix either on the names since it's obvious
> they are host specific, the local data/elems is using struct buffer.
The 'host' prefix is intend to meaning that those members are for
host[buffer] side, others(without 'host' prefix) are for host
component(local) itself.
Thanks,
~Keyon
>
> Liam
>
>> 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);
>
>
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>
More information about the Sound-open-firmware
mailing list