Re: [alsa-devel] Setting some clocks back to DUMMY fixes spdif output on imx6q wandboard rev B1

[Fabio Estevam]

On Wed, Aug 31, 2016 at 2:49 PM, Xavi Drudis Ferran  wrote:

> Thank you amd feel free to suggest more tests, but it is good enough
> as it is for me.

Ok, thanks for trying. So let's keep the SPDIF parent clock as is.

Re: [alsa-devel] Setting some clocks back to DUMMY fixes spdif output on imx6q wandboard rev B1

[Xavi Drudis Ferran]

El Wed, Aug 31, 2016 at 03:49:25PM +0200, Xavi Drudis Ferran deia:
> El Wed, Aug 31, 2016 at 10:11:13AM -0300, Fabio Estevam deia:
> > 2. SPDIF clock rate not accurate. Probably using PLL4 as SPDIF source
> > would help to get more accurate SPDIF clock rates.
> > 
> > Could you please try the untested change?
> > 
> > --- a/drivers/clk/imx/clk-imx6q.c
> > +++ b/drivers/clk/imx/clk-imx6q.c
> > @@ -623,7 +623,7 @@ static void __init imx6q_clocks_init(struct
> > device_node *ccm_node)
> > pr_warn("failed to set up CLKO: %d\n", ret);
> > 
> > /* Audio-related clocks configuration */
> > -   clk_set_parent(clk[IMX6QDL_CLK_SPDIF_SEL],
> > clk[IMX6QDL_CLK_PLL3_PFD3_454M]);
> > +   clk_set_parent(clk[IMX6QDL_CLK_SPDIF_SEL],
> > clk[IMX6QDL_CLK_PLL4_AUDIO_DIV]);
> > 
> > /* All existing boards with PCIe use LVDS1 */
> > if (IS_ENABLED(CONFIG_PCI_IMX6))
> >
> 
> I'm going to try. I'll take a while. I'll report the result later.
> 
> Thank you very much.

I just tried. Spdif output still works. I can't hear any difference. 

I've summarised the tests in a table:

Nominal
Hz   32000  44100  48000  96000 192000
ns   31250  22676  20833  10417   5208

Linux-libre-4.7 (unchanged)   (no spdif output) 
  
Hz   32226  43882  47965  95930 196428
ns   31031  22788  20849  10424   5091
deviation(ns)  219   -113-15 -8117

only core (SPDIF_GCLK), rxtx0 (CLK_OSC), rxtx1(SPDIF) & spba (spdif output)
Hz   31719  43859  47368  94736 187500
ns   31527  22800  2  10556   5333
deviation(ns) -277   -125   -278   -139   -125
 
without MLB (the rest  unchanged)   (spdif output) 
Hz   32226  43859  47368  94736 187500
ns   31031  22800  2  10556   5333
deviation(ns)  219   -125   -278   -139   -125

without MLB, and PLL4 instead of PLL3 for SPDIF (spdif output)   
Hz   32226  44836  49107  93750 187500
ns   31031  22304  20364  10667   5333
deviation(ns)  219372470   -250   -125

I saw page 121 of
http://cache.freescale.com/files/32bit/doc/data_sheet/IMX6DQIEC.pdf
And it seems like the the margin for the SPDIF clock would be 16 ns
so I've just inverted the frequencies to compare, but I'm not convinced
it's relevant. 

Here's the extract from dmesg with your patch (and .dtsi like mainline
except MLB replaced with DUMMY).

[...]
[7.517394] etnaviv-gpu 13.gpu: model: GC2000, revision: 5108
[7.578089] imx_thermal 200.aips-bus:tempmon: Extended Commercial CPU 
temperature grade - max:105C critical:100C passive:95C
[7.594443] etnaviv-gpu 2204000.gpu: model: GC355, revision: 1215
[7.594454] etnaviv-gpu 2204000.gpu: Ignoring GPU with VG and FE2.0
[7.594459] etnaviv-gpu 2204000.gpu: hw init failed: -6
[7.653041] fsl-spdif-dai 2004000.spdif: enter fsl_spdif_probe
[7.656319] fsl-spdif-dai 2004000.spdif: enter fsl_spdif_probe_txclk
[7.704159] fsl-spdif-dai 2004000.spdif: use rxtx5 as tx clock source for 
32000Hz sample rate
[7.711719] fsl-spdif-dai 2004000.spdif: use txclk df 16 for 32000Hz sample 
rate
[7.718328] fsl-spdif-dai 2004000.spdif: use sysclk df 2 for 32000Hz sample 
rate
[7.724762] fsl-spdif-dai 2004000.spdif: the best rate for 32000Hz sample 
rate is 32226Hz
[7.732679] fsl-spdif-dai 2004000.spdif: enter fsl_spdif_probe_txclk
[7.760087] fsl-spdif-dai 2004000.spdif: use rxtx5 as tx clock source for 
44100Hz sample rate
[7.766536] fsl-spdif-dai 2004000.spdif: use txclk df 1 for 44100Hz sample 
rate
[7.772986] fsl-spdif-dai 2004000.spdif: use sysclk df 23 for 44100Hz sample 
rate
[7.780120] fsl-spdif-dai 2004000.spdif: the best rate for 44100Hz sample 
rate is 44836Hz
[7.788952] fsl-spdif-dai 2004000.spdif: enter fsl_spdif_probe_txclk
[7.825238] fsl-spdif-dai 2004000.spdif: use rxtx5 as tx clock source for 
48000Hz sample rate
[7.831515] fsl-spdif-dai 2004000.spdif: use txclk df 7 for 48000Hz sample 
rate
[7.837583] fsl-spdif-dai 2004000.spdif: use sysclk df 3 for 48000Hz sample 
rate
[7.843718] fsl-spdif-dai 2004000.spdif: the best rate for 48000Hz sample 
rate is 49107Hz
[7.849672] fsl-spdif-dai 2004000.spdif: enter fsl_spdif_probe_txclk
[7.878632] fsl-spdif-dai 2004000.spdif: use rxtx0 as tx clock source for 
96000Hz sample rate
[7.884550] fsl-spdif-dai 2004000.spdif: use txclk df 4 for 96000Hz sample 
rate
[7.890390] fsl-spdif-dai 2004000.spdif: the best rate for 96000Hz sample 
rate is 93750Hz
[7.896253] fsl-spdif-dai 2004000.spdif: enter fsl_spdif_probe_txclk
[7.921228] fsl-spdif-dai 

Re: [alsa-devel] Setting some clocks back to DUMMY fixes spdif output on imx6q wandboard rev B1

[Fabio Estevam]

Hi Xavi,

On Wed, Aug 31, 2016 at 10:47 AM, Xavi Drudis Ferran  wrote:

> If it's easy for you to send it yourself, I would prefer so and I'm
> grateful. If not, it'll be an exercise for me, no problem.

I have just submitted the patch with you on Cc.

If you could reply to it with your Tested-by tag, that would be great.

Thanks

Re: [alsa-devel] Setting some clocks back to DUMMY fixes spdif output on imx6q wandboard rev B1

[Xavi Drudis Ferran]

El Wed, Aug 31, 2016 at 10:11:13AM -0300, Fabio Estevam deia:
> 2. SPDIF clock rate not accurate. Probably using PLL4 as SPDIF source
> would help to get more accurate SPDIF clock rates.
> 
> Could you please try the untested change?
> 
> --- a/drivers/clk/imx/clk-imx6q.c
> +++ b/drivers/clk/imx/clk-imx6q.c
> @@ -623,7 +623,7 @@ static void __init imx6q_clocks_init(struct
> device_node *ccm_node)
> pr_warn("failed to set up CLKO: %d\n", ret);
> 
> /* Audio-related clocks configuration */
> -   clk_set_parent(clk[IMX6QDL_CLK_SPDIF_SEL],
> clk[IMX6QDL_CLK_PLL3_PFD3_454M]);
> +   clk_set_parent(clk[IMX6QDL_CLK_SPDIF_SEL],
> clk[IMX6QDL_CLK_PLL4_AUDIO_DIV]);
> 
> /* All existing boards with PCIe use LVDS1 */
> if (IS_ENABLED(CONFIG_PCI_IMX6))
>

I'm going to try. I'll take a while. I'll report the result later.

Thank you very much.

Re: [alsa-devel] Setting some clocks back to DUMMY fixes spdif output on imx6q wandboard rev B1

[Xavi Drudis Ferran]

El Wed, Aug 31, 2016 at 10:30:25AM -0300, Fabio Estevam deia:
> Xavi,
> 
> On Wed, Aug 31, 2016 at 10:11 AM, Fabio Estevam  wrote:
> 
> > Xavi,
> >
> > Care to send a formal patch with your change?
> 
> If you prefer, I can send this change to the ARM kernel mailing list.
>

Whatever is easier for you. I'll have to look up the formalities for
sending the patch myself (format, copyright, where to send, etc.)
since I've never sent a patch for linux. I don't believe such a simple
change can be copyrightable, but the original isn't mine, it's from
that URL I gave, https://community.nxp.com/thread/387131 so originally
from amb...@iwavesystems.com

> Please let me know what you prefer.
>

If it's easy for you to send it yourself, I would prefer so and I'm
grateful. If not, it'll be an exercise for me, no problem.

Re: [alsa-devel] Setting some clocks back to DUMMY fixes spdif output on imx6q wandboard rev B1

[Fabio Estevam]

Xavi,

On Wed, Aug 31, 2016 at 10:11 AM, Fabio Estevam  wrote:

> Xavi,
>
> Care to send a formal patch with your change?

If you prefer, I can send this change to the ARM kernel mailing list.

Please let me know what you prefer.

Thanks

Re: [alsa-devel] Setting some clocks back to DUMMY fixes spdif output on imx6q wandboard rev B1

[Fabio Estevam]

Hi Xavi/Nicolin,

On Wed, Aug 31, 2016 at 6:10 AM, Xavi Drudis Ferran  wrote:
> El Tue, Aug 30, 2016 at 09:21:01PM -0700, Nicolin Chen deia:
>>
>> No, the problem is not at the rate but the source -- Although the
>> MLB clock exists in the clock tree as a better rate provider, it
>> might not be correctly enabled or running at the rate it claims.
>>
>
>>
>> There are five MLB clocks sharing the same clock gate according
>> to CCM chapter in the Reference Manual of imx6q. But five clocks
>> come from three different parent clocks, and I am wondering if
>> the MLB clock that's connected to the S/PDIF module is really
>> derived from this AXI.
>>
>> Hope Fabio might be able to help on the clock tree issue here:)
>>
>
> I hope too, it's a little over my head, to be euphemistic.
>
>>
>> Another solution for you could be to change the rates of two of
>> those existing clocks to the perfect rates for 44.1KHz and 48KHz
>> respectively, 22579200Hz and 24576000Hz for example. (If you
>> only need one sample rate support, changing rxtx1 SPDIF clock
>> only then.)
>
> Thank you very much.  I'm not sure what practical problem that would
> solve for me, audio sounds quite right to my ears with the workaround
> (disabling MLB). I've looked page 121 of
> http://cache.freescale.com/files/32bit/doc/data_sheet/IMX6DQIEC.pdf
> And it seems like the the margin for the SPDIF clock would be 16 ns
> and I'm like 10 times out of spec. But I can't hear the problem.  I
> may try it one day to hear how it sounds.
>
> I'll try to remember it if I ever come across some problem with my audio.
> For now what I'd like is to stay as close to linux-libre mainline
> as possible, so the quick workaround is enough for me.
>
> Now for the general case, I'm not sure what the solution should be.
> Page 4 of the pdf above says MLB is not present in industrial "parts",
> only automotive, or consumer "parts". There are several versions of
> IMX6Q in the market.  What version must I have ? I guess consumer
> (with MLB) but I'm not sure... According to the wandboard-quad-rev-b1
> manual its consumer, MCIMX6Q5EYM10AC, so I should have MLB, I guess.
>
> $ cat /proc/cpuinfo
> processor  : 0
> model name : ARMv7 Processor rev 10 (v7l)
> BogoMIPS   : 7.54
> Features   : half thumb fastmult vfp edsp thumbee neon vfpv3 tls 
> vfpd32
> CPU implementer: 0x41
> CPU architecture: 7
> CPU variant   : 0x2
> CPU part  : 0xc09
> CPU revision  : 10
> [...]
>
> I can't tell what CPU part : 0xc09 means.
>
> In the reference manual pg 796 I see the same gate seems to affect Media
> Local Bus (MLB) clock and Digital Transmission Content Protection
> (DTCP). I don't use DTCP but I haven't done anything to disable it.
>
> http://www.nxp.com/files/32bit/doc/ref_manual/IMX6DQRM.pdf?fasp=1_TYPE=Reference%20Manuals_VENDOR=FREESCALE_FILE_FORMAT=pdf_ASSET=Documentation=.pdf

Sorry for the delay.

As far as I can see, there are two current issues:

1. Regression caused by: 833f2cbf7091099bae ("ARM: dts: imx6: change
the core clock of spdif").

Looks like that this commit did much more than just changing the core
clock of spdif.

It does not mention why MLB clock has been added. Looking at MX6Q RM I
do not see the connection between MLB and SPDIF.

So I agree with Xavi's suggestion of using the dummy_clk instead of mlb clock.

Xavi,

Care to send a formal patch with your change?

2. SPDIF clock rate not accurate. Probably using PLL4 as SPDIF source
would help to get more accurate SPDIF clock rates.

Could you please try the untested change?

--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -623,7 +623,7 @@ static void __init imx6q_clocks_init(struct
device_node *ccm_node)
pr_warn("failed to set up CLKO: %d\n", ret);

/* Audio-related clocks configuration */
-   clk_set_parent(clk[IMX6QDL_CLK_SPDIF_SEL],
clk[IMX6QDL_CLK_PLL3_PFD3_454M]);
+   clk_set_parent(clk[IMX6QDL_CLK_SPDIF_SEL],
clk[IMX6QDL_CLK_PLL4_AUDIO_DIV]);

/* All existing boards with PCIe use LVDS1 */
if (IS_ENABLED(CONFIG_PCI_IMX6))

Regards,

Fabio Estevam