[v2] ASoC: fsl_ssi: Fix channel swap on playback start

Commit Message

Fabio Estevam April 4, 2017, 5:12 p.m. UTC

On Tue, Apr 4, 2017 at 8:38 AM, Fabio Estevam <[e-mail]> wrote:

> I have the following SSI1 values (with the original 4.1-rc5 + pictrl patch):
>
> # cat /sys/kernel/debug/regmap/2028000.ssi/registers
> 00: 00000200
> 04: 00000000
> 10: 0000105b

Bits 6-5 (I2S_MODE) of register SCR are 10 of register SCR, which
means I2S slave mode.

The MX6 Reference Manual states:

"In I2S slave mode(SSI_SCR[6:5]=10), the following settings are
internally overridden by
the hardware:

Normal mode is selected (SSI_SCR[3]=0)
Tx frame sync length set to one-bit-long-frame (SSI_STCR[1]=1)
Rx frame sync length set to one-bit-long-frame (SSI_SRCR[1]=1)"

so I tried not to use the the one-bit-long-frame (since ENGcm06222 is
about bit length frame sync) override and changed it to I2S normal
mode instead:


and I do not get the channel swap anymore.

In what cases could we safely switch to I2S normal mode?

Thanks

Comments

Arnaud Mouiche April 4, 2017, 8:09 p.m. UTC | #1

On 04/04/2017 19:12, Fabio Estevam wrote:
> On Tue, Apr 4, 2017 at 8:38 AM, Fabio Estevam <[e-mail]> wrote:
>
>> I have the following SSI1 values (with the original 4.1-rc5 + pictrl patch):
>>
>> # cat /sys/kernel/debug/regmap/2028000.ssi/registers
>> 00: 00000200
>> 04: 00000000
>> 10: 0000105b
> Bits 6-5 (I2S_MODE) of register SCR are 10 of register SCR, which
> means I2S slave mode.
>
> The MX6 Reference Manual states:
>
> "In I2S slave mode(SSI_SCR[6:5]=10), the following settings are
> internally overridden by
> the hardware:
>
> Normal mode is selected (SSI_SCR[3]=0)
> Tx frame sync length set to one-bit-long-frame (SSI_STCR[1]=1)
> Rx frame sync length set to one-bit-long-frame (SSI_SRCR[1]=1)"
>
> so I tried not to use the the one-bit-long-frame (since ENGcm06222 is
> about bit length frame sync) override and changed it to I2S normal
> mode instead:
>
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -973,7 +973,7 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
>                          ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_MASTER;
>                          break;
>                  case SND_SOC_DAIFMT_CBM_CFM:
> -                       ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_SLAVE;
> +                       ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_NORMAL;
>                          break;
>                  default:
>                          return -EINVAL;
>
> and I do not get the channel swap anymore.
>
> In what cases could we safely switch to I2S normal mode?

SCR bit 3 (NET) is also set, so you should be in network mode with a 
long frame sync.
In fact, you can entirely simulate a I2S behavior using Network mode. 
you should just be careful about the way everything is configured (eg. 
place of samples in the stream)


Also, I just read the ENGcm06222 chip errata.
http://www.nxp.com/assets/documents/data/en/errata/IMX35CE.pdf

and I don't understand why it affects us in this case.
- you are slave in your case and you don't send the Fsync
- it talk about writing EN and TE in the same frame (so with less than 
1/44100s).
=> Writing the register at once is simply a good way to be sure it is 
effective... except if it takes more than a frame to configure the whole 
stuff.
And I also don't understand how all of this could have work so long 
before in "Capture, then later, playback scenario", were TE is set very 
long after EN...

Arnaud

>
> Thanks

Patch

--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -973,7 +973,7 @@  static int _fsl_ssi_set_dai_fmt(struct device *dev,
                        ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_MASTER;
                        break;
                case SND_SOC_DAIFMT_CBM_CFM:
-                       ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_SLAVE;
+                       ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_NORMAL;
                        break;
                default:
                        return -EINVAL;