[Sound-open-firmware] [PATCH] sof: Add volume component state check

[Ranjani Sridharan]

On Tue, 2018-03-27 at 11:49 +0800, zhigang.wu wrote:
> 
> On 2018年03月27日 11:08, Ranjani Sridharan wrote:
> > On Tue, 2018-03-27 at 09:42 +0800, Wu Zhigang wrote:
> > > Add the state check in volume_copy function,
> > > this can low down the xrun error possibility:
> > > For example:
> > > when firmware receive the PAUSE command, the HOST and VOLUME
> > > component are set to PAUSE state. at this time DMA IRQ happen,
> > > the pipeline is scheduled. host_copy() function will be called,
> > > but jump out immediately because of PAUSE state. but
> > > volume_copy()
> > > did not have this state check, the xrun will happen.
> > > 
> > > Signed-off-by: Wu Zhigang <zhigang.wu at linux.intel.com>
> > > Reviewed-by: Keyon Jie <yang.jie at linux.intel.com>
> > > 
> > > ---
> > > Tested with
> > > apl-gpmrb board
> > > 
> > > kernel: https://github.com/plbossart/sound.gittt  branch:
> > > topic/sof-
> > > v4.14
> > > 	4881a4bd906f8b52bebd209b88ff920005550d53
> > > 
> > > firmware: git://git.alsa-project.org/sound-open-
> > > firmware.git  branch:
> > > 1.1-stable
> > >          1f8c6c4d2018aac6994c75f60cee54765435d424
> > > 
> > > tools:	branch: 1.1-stable
> > > 	cc91c73aa3e91eea35abdeb76d578b97f718feff
> > > ---
> > >   src/audio/volume.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/src/audio/volume.c b/src/audio/volume.c
> > > index df926c5..2652765 100644
> > > --- a/src/audio/volume.c
> > > +++ b/src/audio/volume.c
> > > @@ -836,6 +836,9 @@ static int volume_copy(struct comp_dev *dev)
> > >   
> > >   	tracev_volume("cpy");
> > >   
> > > +	if (dev->state != COMP_STATE_ACTIVE)
> > > +		return 0;
> > 
> > Zhigang, pipeline_copy_from_upstream() and
> > pipeline_copy_to_downstream() both have the check to make sure we
> > propogate copy only if the component is active. Will those not
> > handle
> > this situation as well?
> 
> if the pipeline has more than one volume or other kinds of
> components,
> or the volume component is not the second component in the pipeline.
> this kind of check is not enough.
> so we have to add the state check in all of component's copy
> operation 
> to avoid this issue in future.

Sorry Zhigang. I'm struggling to understand the problem. Could you
please share your pipeline that you're seeing the issue with?
> 
> thanks
> br
>          --zhigang.wu
> 
> > > +
> > >   	/* volume components will only ever have 1 source and 1
> > > sink
> > > buffer */
> > >   	source = list_first_item(&dev->bsource_list, struct
> > > comp_buffer, sink_list);
> > >   	sink = list_first_item(&dev->bsink_list, struct
> > > comp_buffer,
> > > source_list);
> > 
> > _______________________________________________
> > Sound-open-firmware mailing list
> > Sound-open-firmware at alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmwar
> > e
> 
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware