Re: [PATCH 1/2] ASoC: AMD: Fix simultaneous playback and capture on different channel

[Agrawal, Akshu]

On 9/10/2018 5:08 PM, Mark Brown wrote:
> On Mon, Sep 10, 2018 at 01:36:29PM +0530, Akshu Agrawal wrote:
>> If capture and playback are started on different channel (I2S/BT)
>> there is a possibilty that channel information passed from machine driver
>> is overwritten before the configuration is done in dma driver.
>> Example:
>> 113.597588: cz_max_startup: ---playback sets BT channel
>> 113.597694: cz_dmic1_startup: ---capture sets I2S channel
>> 113.597979: acp_dma_hw_params: ---configures capture for I2S channel
>> 113.598114: acp_dma_hw_params: ---configures playback for I2S channel
>>
>> This is fixed by having lock between startup and prepare. This ensures
>> no other codec startup gets called between a codec's startup(where channel
>> info is set) and hw_params(where channel info is read).
> 
> This isn't viable - the driver will deadlock if the application hits an
> error and never gets to startup, or if the application tries to
> simultaneously configure two channels (ie, do all the prepares and then
> all the parameter configuration and then startup).

We can avoid deadlock by having another mutex_unlock in the shutdown
call of each of codec's ops. Wouldn't in all possible termination
scenarios, it will cleanup and exit via shutdown callback?

Having said that I think there is a better approach to this, is by
having 2 separate instance variable for playback and capture for passing
instance info from machine driver to dma driver. Respective codec in
machine driver will set the capture/playback instance. dma driver on the
basis of substream->stream can read the correct one. No fear of deadlock
in this.

Thanks,
Akshu