[PATCH] ASoC: amd: Changing Audio Format does not reflect.

[Agrawal, Akshu]

On 3/24/2020 3:46 PM, RAVULAPATI, VISHNU VARDHAN RAO wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
>
>
> -----Original Message-----
> From: Agrawal, Akshu <Akshu.Agrawal at amd.com>
> Sent: Friday, March 20, 2020 9:39 AM
> To: RAVULAPATI, VISHNU VARDHAN RAO <Vishnuvardhanrao.Ravulapati at amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; broonie at kernel.org; Liam Girdwood <lgirdwood at gmail.com>; Jaroslav Kysela <perex at perex.cz>; Takashi Iwai <tiwai at suse.com>; Agrawal, Akshu <Akshu.Agrawal at amd.com>; Wei Yongjun <weiyongjun1 at huawei.com>; moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM... <alsa-devel at alsa-project.org>; open list <linux-kernel at vger.kernel.org>
> Subject: Re: [PATCH] ASoC: amd: Changing Audio Format does not reflect.
>
>
> On 3/19/2020 4:52 PM, Ravulapati Vishnu vardhan rao wrote:
>> When you run aplay subsequently as below by changing the stream format:
>>
>> aplay -Dhw:2,0 -c2 -fS16_LE -r48000 /dev/zero -vv -d 5;aplay -Dhw:2,0
>> -c2 -fS24_LE -r48000 /dev/zero -vv
>> as a single command, the format gets corrupted and audio does not play.
>>
>> So clear the ACP_(I2S/BT)TDM_ITER/IRER register when dma stops.
>>
>> Signed-off-by: Ravulapati Vishnu vardhan rao
>> <Vishnuvardhanrao.Ravulapati at amd.com>
>> ---
>>    sound/soc/amd/raven/acp3x-i2s.c | 4 +---
>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/sound/soc/amd/raven/acp3x-i2s.c
>> b/sound/soc/amd/raven/acp3x-i2s.c index 3a3c47e..b07c50a 100644
>> --- a/sound/soc/amd/raven/acp3x-i2s.c
>> +++ b/sound/soc/amd/raven/acp3x-i2s.c
>> @@ -240,9 +240,7 @@ static int acp3x_i2s_trigger(struct snd_pcm_substream *substream,
>>    				reg_val = mmACP_I2STDM_IRER;
>>    			}
>>    		}
>> -		val = rv_readl(rtd->acp3x_base + reg_val);
>> -		val = val & ~BIT(0);
>> -		rv_writel(val, rtd->acp3x_base + reg_val);
>> +		rv_writel(0, rtd->acp3x_base + reg_val);
> Clearing the entire register might result in issues.
>
> IMO better fix is to have a sample_len mask for ITER and IRER register.
>
> Use it to clear sample length bits in acp3x_i2s_hwparams function before setting the new format resolution.
>
>
> Thanks,
>
> Akshu
>
>
> When stream is open once again all the values will be set.
> So with clearing ITER/IRER to 0 when device closes there will be no issues.
>
> -Vishnu

The issue is in below code:

         val = rv_readl(rtd->acp3x_base + reg_val);
         val = val | (rtd->xfer_resolution  << 3);
         rv_writel(val, rtd->acp3x_base + reg_val);

where you are setting bits without first clearing them. If initial value 
was 10 and you want to set 01, you will end up with 11.

The correct way is to use mask and clear the bits before using | to 
avoid previous residual values.