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

[zhigang.wu]

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.gitt  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.

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-firmware