[Sound-open-firmware] [PATCH 25/25] component: host/volume/mixer: reset buffers for stop cmd
Liam Girdwood
liam.r.girdwood at linux.intel.com
Thu Feb 9 11:21:50 CET 2017
On Thu, 2017-02-09 at 02:37 +0000, Jie, Yang wrote:
> > -----Original Message-----
> > From: Liam Girdwood [mailto:liam.r.girdwood at linux.intel.com]
> > Sent: Wednesday, February 8, 2017 8:17 PM
> > To: Keyon Jie <yang.jie at linux.intel.com>
> > Cc: sound-open-firmware at alsa-project.org; Jie, Yang <yang.jie at intel.com>;
> > Ingalsuo, Seppo <seppo.ingalsuo at intel.com>
> > Subject: Re: [Sound-open-firmware] [PATCH 25/25] component:
> > host/volume/mixer: reset buffers for stop cmd
> >
> > On Tue, 2017-02-07 at 22:03 +0800, Keyon Jie wrote:
> > > For command stop, excepting reset components, we may need reset the
> > > buffer next to the component, otherwise, if the next start command
> > > comes(without reset before it), the buffer pointers may be disordered.
> > >
> > > Signed-off-by: Keyon Jie <yang.jie at linux.intel.com>
> > > ---
> > > src/audio/host.c | 18 ++++++++++++++++++
> > > src/audio/mixer.c | 31 ++++++++++++++++++++++++++++++-
> > > src/audio/volume.c | 27 ++++++++++++++++++++++++++-
> > > 3 files changed, 74 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/audio/host.c b/src/audio/host.c index
> > > 28b5e7b..07f4953 100644
> > > --- a/src/audio/host.c
> > > +++ b/src/audio/host.c
> > > @@ -656,6 +656,7 @@ static int host_stop(struct comp_dev *dev) {
> > > struct host_data *hd = comp_get_drvdata(dev);
> > > struct dma_sg_elem *source_elem, *sink_elem, *local_elem;
> > > + struct list_item *clist;
> > >
> > > /* reset buffer pointers */
> > > if (hd->host_pos)
> > > @@ -689,6 +690,23 @@ static int host_stop(struct comp_dev *dev)
> > > local_elem->src = source_elem->src;
> > > hd->next_inc = hd->period->size;
> > >
> > > + /* now reset downstream buffer */
> > > + list_for_item(clist, &dev->bsink_list) {
> > > + struct comp_buffer *buffer;
> > > +
> > > + buffer = container_of(clist, struct comp_buffer, source_list);
> > > +
> > > + /* dont reset buffer if the component is not connected */
> > > + if (!buffer->connected)
> > > + continue;
> > > + /* reset buffer next to the component*/
> > > + bzero(buffer->addr, buffer->desc.size);
> > > + buffer->w_ptr = buffer->r_ptr = buffer->addr;
> > > + buffer->end_addr = buffer->addr + buffer->desc.size;
> > > + buffer->free = buffer->desc.size;
> > > + buffer->avail = 0;
> >
> > I'm seeing this reset code duplicated in a lot of places. lets create a new
> > component API call that does this reset.
>
> OK, I will create inline one comp_buffer_stop() and put it component.h?
>
Lets call it comp_buffer_reset()
It should contain :-
/* reset buffer next to the component*/
> > > + bzero(buffer->addr, buffer->desc.size);
> > > + buffer->w_ptr = buffer->r_ptr = buffer->addr;
> > > + buffer->end_addr = buffer->addr + buffer->desc.size;
> > > + buffer->free = buffer->desc.size;
> > > + buffer->avail = 0;
i.e. it clears buffer and resets pointers.
Liam
> Thanks,
> ~Keyon
>
> >
> > > + }
> > > +
> > > dev->state = COMP_STATE_SETUP;
> > > return 0;
> > > }
> > > diff --git a/src/audio/mixer.c b/src/audio/mixer.c index
> > > 2d021b7..4dccf7a 100644
> > > --- a/src/audio/mixer.c
> > > +++ b/src/audio/mixer.c
> > > @@ -175,6 +175,31 @@ static struct comp_dev*
> > mixer_volume_component(struct comp_dev *mixer)
> > > return comp_dev;
> > > }
> > >
> > > +static int mixer_stop(struct comp_dev *dev) {
> > > + struct list_item *clist;
> > > +
> > > + /* now reset downstream buffer */
> > > + list_for_item(clist, &dev->bsink_list) {
> > > + struct comp_buffer *buffer;
> > > +
> > > + buffer = container_of(clist, struct comp_buffer, source_list);
> > > +
> > > + /* dont reset buffer if the component is not connected */
> > > + if (!buffer->connected)
> > > + continue;
> > > + /* reset buffer next to the component*/
> > > + bzero(buffer->addr, buffer->desc.size);
> > > + buffer->w_ptr = buffer->r_ptr = buffer->addr;
> > > + buffer->end_addr = buffer->addr + buffer->desc.size;
> > > + buffer->free = buffer->desc.size;
> > > + buffer->avail = 0;
> > > + }
> > > +
> > > + dev->state = COMP_STATE_SETUP;
> > > + return 0;
> > > +}
> > > +
> > > /* used to pass standard and bespoke commands (with data) to
> > > component */ static int mixer_cmd(struct comp_dev *dev, int cmd, void
> > > *data) { @@ -184,7 +209,6 @@ static int mixer_cmd(struct comp_dev
> > > *dev, int cmd, void *data)
> > > switch(cmd) {
> > > case COMP_CMD_START:
> > > trace_mixer("MSa");
> > > - case COMP_CMD_STOP:
> > > case COMP_CMD_PAUSE:
> > > case COMP_CMD_RELEASE:
> > > case COMP_CMD_DRAIN:
> > > @@ -192,6 +216,11 @@ static int mixer_cmd(struct comp_dev *dev, int cmd,
> > void *data)
> > > case COMP_CMD_RESUME:
> > > finish = mixer_status_change(dev);
> > > break;
> > > + case COMP_CMD_STOP:
> > > + finish = mixer_status_change(dev);
> > > + if (finish == 0)
> > > + mixer_stop(dev);
> > > + break;
> > > case COMP_CMD_VOLUME:
> > > vol_dev = mixer_volume_component(dev);
> > > if (vol_dev != NULL)
> > > diff --git a/src/audio/volume.c b/src/audio/volume.c index
> > > a226334..032e78b 100644
> > > --- a/src/audio/volume.c
> > > +++ b/src/audio/volume.c
> > > @@ -389,6 +389,31 @@ static inline void volume_set_chan_unmute(struct
> > comp_dev *dev, int chan)
> > > cd->tvolume[chan] = cd->mvolume[chan]; }
> > >
> > > +static int volume_stop(struct comp_dev *dev) {
> > > + struct list_item *clist;
> > > +
> > > + /* now reset downstream buffer */
> > > + list_for_item(clist, &dev->bsink_list) {
> > > + struct comp_buffer *buffer;
> > > +
> > > + buffer = container_of(clist, struct comp_buffer, source_list);
> > > +
> > > + /* dont reset buffer if the component is not connected */
> > > + if (!buffer->connected)
> > > + continue;
> > > + /* reset buffer next to the component*/
> > > + bzero(buffer->addr, buffer->desc.size);
> > > + buffer->w_ptr = buffer->r_ptr = buffer->addr;
> > > + buffer->end_addr = buffer->addr + buffer->desc.size;
> > > + buffer->free = buffer->desc.size;
> > > + buffer->avail = 0;
> > > + }
> > > +
> > > + dev->state = COMP_STATE_SETUP;
> > > + return 0;
> > > +}
> > > +
> > > /* used to pass standard and bespoke commands (with data) to
> > > component */ static int volume_cmd(struct comp_dev *dev, int cmd,
> > > void *data) { @@ -432,7 +457,7 @@ static int volume_cmd(struct
> > > comp_dev *dev, int cmd, void *data)
> > > if (dev->state == COMP_STATE_RUNNING ||
> > > dev->state == COMP_STATE_DRAINING ||
> > > dev->state == COMP_STATE_PAUSED)
> > > - dev->state = COMP_STATE_SETUP;
> > > + volume_stop(dev);
> > > break;
> > > case COMP_CMD_PAUSE:
> > > /* only support pausing for running */
> >
>
> _______________________________________________
> 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