iMX6 SPDIF issues

[kgroeneveld]

I have recently run into some problems using SPDIF on iMX6. I first noticed the issue when reading the ALSA “RX Sample Rate” control to determine if the SPDIF receiver is locked and what the sample rate is. Sometimes this returns a seemingly random value. While trying to debug this I noticed that reading from the SPDIF registers themselves in the kernel or via the user space regmap debug interface often seems to give random values.

Even though the sabresd board does not have SPDIF in or out connections I managed to trigger similar symptoms on the sabresd board.

Test Setup:

1. sabresd board with iMX6 Quad

2. mainline kernel v4.2

3. patch device tree to enable spdif (patch file attached)

The SPDIF interface should then be available on ALSA device hw:1,0.

Run speaker-test to generate output on SPDIF:

speaker-test -Dhw:1,0 -fS24_LE -c2 -r44100

While running speaker-test monitor the values in /proc/sys/debug/regmap/2004000.spdif/registers and you can see some registers changing that should be constant.

For example:

cd /proc/sys/debug/regmap/2004000.spdif

watch -n0 -d=cumulative cat registers

This will highlight registers that are changing. Some of them include status bits which are expected to change. Some registers such as 0x50 should be constant but are not.  It can take a couple minutes to see the unexpected changes.

Or you can do something like:

while true; do grep "50:" registers | grep -v 188; done

On my system register 50 usually returns 00000188. By running the above command you can see that sometimes it returns a different corrupted value.  Using printk's in the spdif kernel driver I can also see this issue so it isn't just a quirk of the regmap debug interface.

I also duplicated the same issue on a custom iMX6 Solo board. However, on an iMX6 Sololite board everything seems okay. In either case audio out and in is generally working but I can't monitor the RX status correctly.

Does anyone have any ideas why this is happening?

Thanks,

Kevin

Original Attachment has been moved to: sabresd_dt_spdif.patch.zip

[fabio_estevam]

Hi Kevin,

Applied your patch against 4.2 and still could not see a value different than '50: 00000188'

speaker-test -Dhw:1,0 -fS24_LE -c2 -r44100 &

mount -t debugfs none /sys/kernel/debug

cat /sys/kernel/debug/regmap/2004000.spdif/registers | grep 50

50: 00000188

I read it several times during 10 minutes and the result has been always the same so far.

Regards,

Fabio Estevam

[kgroeneveld]

Hi Fabio,

Thanks for trying to reproduce the issue. If I just run a single "cat /sys/kernel/debug/regmap/2004000.spdif/registers | grep 50" I have never seen an error either.  I have to run it in a loop.  For example:

while true; do grep 50 /proc/sys/debug/regmap/2004000.spdif/registers | grep -v 188; done

50: 008ba2e8

50: 008ba2e8

50: 008ba2e8

50: 008ba2e8

50: 008ba2e8

50: 008ba2e8

It took a couple minutes for the above command to show the six wrong values. If I run the test on a board with the SPDIF receiver actually receiving audio the frequency of the errors seems to increase.

Kevin

[fabio_estevam]

Ok, Kevin I am to reproduce this now.

Will investigate this issue.

[fabio_estevam]

Kevin,

Are you able to reproduce it on FSL 3.14 GA kernel?

[kgroeneveld]

Hi Fabio,

I have not tested with an FSL kernel.  Maybe tomorrow I will try this.

Have you found you cannot reproduce the issue with FSL 3.14 GA?

Kevin

[wangshengjiu · Accepted solution]

FabioEstevam​, kgroeneveld​

Hi

      I think it can not be reproduced in imx3.14_ga. The reason is that there are several patches has not been upstreamed to community kernel.

      If you want to resolve this issue in community version. please add below changes.

--- a/arch/arm/boot/dts/imx6qdl.dtsi

+++ b/arch/arm/boot/dts/imx6qdl.dtsi

@@ -219,15 +219,15 @@

                                               <&sdma 15 18 0>;

                                        dma-names = "rx", "tx";

                                        clocks = <&clks IMX6QDL_CLK_SPDIF_GCLK>, <&clks IMX6QDL_CLK_OSC>,

-                                                <&clks IMX6QDL_CLK_SPDIF>, <&clks IMX6QDL_CLK_DUMMY>,

-                                                <&clks IMX6QDL_CLK_DUMMY>, <&clks IMX6QDL_CLK_DUMMY>,

-                                                <&clks IMX6QDL_CLK_DUMMY>, <&clks IMX6QDL_CLK_DUMMY>,

-                                                <&clks IMX6QDL_CLK_DUMMY>;

+                                                <&clks IMX6QDL_CLK_SPDIF>, <&clks IMX6QDL_CLK_ASRC>,

+                                                <&clks IMX6QDL_CLK_DUMMY>, <&clks IMX6QDL_CLK_ESAI_EXTAL>,

+                                                <&clks IMX6QDL_CLK_IPG>, <&clks IMX6QDL_CLK_MLB>,

+                                                <&clks IMX6QDL_CLK_DUMMY>, <&clks IMX6QDL_CLK_SPBA>;

                                        clock-names = "core",  "rxtx0",

                                                      "rxtx1", "rxtx2",

                                                      "rxtx3", "rxtx4",

                                                      "rxtx5", "rxtx6",

-                                                     "rxtx7";

+                                                     "rxtx7", "dma";

                                        status = "disabled";

                                };

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index ab729f2..2f4cb37 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -106,6 +106,7 @@ struct fsl_spdif_priv {
        struct clk *rxclk;
        struct clk *coreclk;
        struct clk *sysclk;
+       struct clk *dmaclk;
        struct snd_dmaengine_dai_dma_data dma_params_tx;
        struct snd_dmaengine_dai_dma_data dma_params_rx;
};
@@ -466,6 +467,12 @@ static int fsl_spdif_startup(struct snd_pcm_substream *substream,
                        return ret;
                }

+               ret = clk_prepare_enable(spdif_priv->dmaclk);
+               if (ret) {
+                       dev_err(&pdev->dev, "failed to enable dma clock\n");
+                       return ret;
+               }
+
                ret = spdif_softreset(spdif_priv);
                if (ret) {
                        dev_err(&pdev->dev, "failed to soft reset\n");
@@ -540,6 +547,7 @@ static void fsl_spdif_shutdown(struct snd_pcm_substream *substream,
                spdif_intr_status_clear(spdif_priv);
                regmap_update_bits(regmap, REG_SPDIF_SCR,
                                SCR_LOW_POWER, SCR_LOW_POWER);
+               clk_disable_unprepare(spdif_priv->dmaclk);
                clk_disable_unprepare(spdif_priv->coreclk);
        }
}
@@ -1221,6 +1229,13 @@ static int fsl_spdif_probe(struct platform_device *pdev)
                return PTR_ERR(spdif_priv->coreclk);
        }

+       /* Get dma clock for dma script operation */
+       spdif_priv->dmaclk = devm_clk_get(&pdev->dev, "dma");
+       if (IS_ERR(spdif_priv->dmaclk)) {
+               dev_err(&pdev->dev, "no dma clock in devicetree\n");
+               return PTR_ERR(spdif_priv->dmaclk);
+       }
+
        /* Select clock source for rx/tx clock */
        spdif_priv->rxclk = devm_clk_get(&pdev->dev, "rxtx1");
        if (IS_ERR(spdif_priv->rxclk)) {

best regards

wang shengjiu

[kgroeneveld]

Hi Wang,

That patch fixes the problem. Often when I have problems like this I check the FSL kernel for relevant patches but for some reason I just never thought to check this time. I now see there is a similar dma/spba clock patch for the esai and asrc.  I should probably pull those into our kernel as well.

Thanks everyone for the help.

Kevin

[fabio_estevam]

Hi Kevin and Wang,

Glad to know the issue is solved. Could any of you send this change upstream?

Thanks,

Fabio Estevam

[kgroeneveld]

I wonder if the SSI driver should have a similar patch to enable the dma/spba clock?

[fabio_estevam]

Hi Kevin,

Could you please try these two spdif clock patches that were posted yesterday?

[PATCH V2 1/2] clk: imx6: Add SPDIF_GCLK clock in clock tree — Linux Device Tree

[PATCH V2 2/2] ARM: dts: imx6: change the core clock of spdif — Linux Device Tree

I will setup a mx6qsabresd board and try to reproduce this problem.

Regards,

Fabio Estevam

[kgroeneveld]

Hi Fabio,

I just tried applying those patches to kernel v4.2 and the problem is still occurring in my test setup.  I did have to make a minor change to get the patches to apply to v4.2.  The patches apply cleanly to v4.3-rc1 and master.  However when I compile v4.3-rc1 or master the result is not stable, with or without the spdif patches.

I was originally hopeful as a clocking problem does seem a possible cause to the symptoms I am seeing.  But after looking at the patches I wasn't really expecting them to help.

Kevin