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 <[email protected]>
---
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);
if (!(rv_readl(rtd->acp3x_base + mmACP_BTTDM_ITER) & BIT(0)) &&
!(rv_readl(rtd->acp3x_base + mmACP_BTTDM_IRER) & BIT(0)))
--
2.7.4
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 <[email protected]>
> ---
> 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
[AMD Official Use Only - Internal Distribution Only]
-----Original Message-----
From: Agrawal, Akshu <[email protected]>
Sent: Friday, March 20, 2020 9:39 AM
To: RAVULAPATI, VISHNU VARDHAN RAO <[email protected]>
Cc: Deucher, Alexander <[email protected]>; [email protected]; Liam Girdwood <[email protected]>; Jaroslav Kysela <[email protected]>; Takashi Iwai <[email protected]>; Agrawal, Akshu <[email protected]>; Wei Yongjun <[email protected]>; moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM... <[email protected]>; open list <[email protected]>
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
> <[email protected]>
> ---
> 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
On 3/24/2020 3:46 PM, RAVULAPATI, VISHNU VARDHAN RAO wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
>
>
> -----Original Message-----
> From: Agrawal, Akshu <[email protected]>
> Sent: Friday, March 20, 2020 9:39 AM
> To: RAVULAPATI, VISHNU VARDHAN RAO <[email protected]>
> Cc: Deucher, Alexander <[email protected]>; [email protected]; Liam Girdwood <[email protected]>; Jaroslav Kysela <[email protected]>; Takashi Iwai <[email protected]>; Agrawal, Akshu <[email protected]>; Wei Yongjun <[email protected]>; moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM... <[email protected]>; open list <[email protected]>
> 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
>> <[email protected]>
>> ---
>> 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.