SolidRun / linux-fslc

Linux kernel source tree
Other
12 stars 26 forks source link

Fix SPDIF output #51

Closed warped-rudi closed 6 years ago

warped-rudi commented 6 years ago

This patch set brings back SPDIF output. The primary reason for the regression was, that our imx6qdl.dtsi contains references to invalid clock sources. This did not hurt much in 3.14, because due to the ability to modify SPDIF0_CLK_ROOT, such a clock source was only chosen when less common sample rates (88.2kHz or 176.4kHz ) were requested. For unknown reason, 4.9. doesn't allow to tune SPDIF0_CLK_ROOT any more. This causes the rate matching code to select one of the bad clock sources for pretty much all sample rates, effectively killing SPDIF. The first patch removes the invalid clock entries, while the 2 others contain some less important improvements.

However, the fact that SPDIF0_CLK_ROOT will not be adjusted any more causes a much bigger deviation of the output sample frequency from it's desired value than we used to have:

3.14:

best rate for 32000Hz sample rate is 32005Hz 0.01% best rate for 44100Hz sample rate is 44132Hz 0.07% best rate for 48000Hz sample rate is 48008Hz 0.01% best rate for 88200Hz sample rate is 87719Hz -0.54% best rate for 96000Hz sample rate is 96017Hz 0.01% best rate for 176400Hz sample rate is 177631Hz 0.69% best rate for 192000Hz sample rate is 192034Hz 0.01%

4.9:

best rate for 32000Hz sample rate is 32226Hz 0.70% best rate for 44100Hz sample rate is 44407Hz 0.69% best rate for 48000Hz sample rate is 49107Hz 2.30% best rate for 88200Hz sample rate is 88815Hz 0.69% best rate for 96000Hz sample rate is 93750Hz 2.34% best rate for 176400Hz sample rate is 171875Hz 2.56% best rate for 192000Hz sample rate is 187500Hz 2.34%

Maybe someone has an idea why SPDIF0_CLK_ROOT is not adapted any more.

jnettlet commented 6 years ago

In 3.14 I re-enabled this behaviour by the following commits f741852fb7adc614a76de674d4da26f78c89d778 and 805eb3f1122b6bc116d84b443f00d03e87f8d25f

This was for glitchless remuxing that I never really saw as a problem while testing. I have those patches on my list of things to test now that the main patchset is stable.

warped-rudi commented 6 years ago

So we have to remember to put the rate adjustment code back into the SPDIF driver once these patches get re-integrated. BTW, the faulty imx6qdl.dtsi is also in 4.14 mainline.

jnettlet commented 6 years ago

I finally had some time to look at this and the most straightforward solution is to just manually set the spdif clock at boot to half speed. 227368421Hz This with the existing code produces this clock selection.

[ 7.093800] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 32000Hz sample rate [ 7.093807] fsl-spdif-dai 2004000.spdif: use txclk df 111 for 32000Hz sample rate [ 7.103266] fsl-spdif-dai 2004000.spdif: use rxtx6 as tx clock source for 44100Hz sample rate [ 7.103273] fsl-spdif-dai 2004000.spdif: use txclk df 94 for 44100Hz sample rate [ 7.103360] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 48000Hz sample rate [ 7.103367] fsl-spdif-dai 2004000.spdif: use txclk df 74 for 48000Hz sample rate [ 7.113084] fsl-spdif-dai 2004000.spdif: use rxtx6 as tx clock source for 88200Hz sample rate [ 7.113090] fsl-spdif-dai 2004000.spdif: use txclk df 47 for 88200Hz sample rate [ 7.113173] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 96000Hz sample rate [ 7.113179] fsl-spdif-dai 2004000.spdif: use txclk df 37 for 96000Hz sample rate [ 7.123250] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 176400Hz sample rate [ 7.123256] fsl-spdif-dai 2004000.spdif: use txclk df 20 for 176400Hz sample rate [ 7.133268] fsl-spdif-dai 2004000.spdif: use rxtx6 as tx clock source for 192000Hz sample rate [ 7.133274] fsl-spdif-dai 2004000.spdif: use txclk df 21 for 192000Hz sample rate This is not too bad. If I then pull in your clock selection code on top of it I get the following clocks for spdif.

[ 7.097530] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 32000Hz sample rate [ 7.097537] fsl-spdif-dai 2004000.spdif: use txclk df 111 for 32000Hz sample rate [ 7.106923] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 44100Hz sample rate [ 7.106930] fsl-spdif-dai 2004000.spdif: use txclk df 81 for 44100Hz sample rate [ 7.107028] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 48000Hz sample rate [ 7.107035] fsl-spdif-dai 2004000.spdif: use txclk df 74 for 48000Hz sample rate [ 7.116646] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 88200Hz sample rate [ 7.116652] fsl-spdif-dai 2004000.spdif: use txclk df 40 for 88200Hz sample rate [ 7.116736] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 96000Hz sample rate [ 7.116741] fsl-spdif-dai 2004000.spdif: use txclk df 37 for 96000Hz sample rate [ 7.126705] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 176400Hz sample rate [ 7.126711] fsl-spdif-dai 2004000.spdif: use txclk df 20 for 176400Hz sample rate [ 7.136617] fsl-spdif-dai 2004000.spdif: use rxtx0 as tx clock source for 192000Hz sample rate [ 7.136623] fsl-spdif-dai 2004000.spdif: use txclk df 2 for 192000Hz sample rate root@sr-imx6:~# [ 57.936420] fsl-spdif-dai 2004000.spdif: expected clock rate = 227328000 [ 57.943227] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 57.949898] fsl-spdif-dai 2004000.spdif: set sample rate to 32005Hz for 32000Hz playback [ 57.958115] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 57.963606] fsl-spdif-dai 2004000.spdif: STCSCL: 0xc00300 [ 69.082796] fsl-spdif-dai 2004000.spdif: expected clock rate = 228614400 [ 69.089607] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 69.096252] fsl-spdif-dai 2004000.spdif: set sample rate to 43859Hz for 44100Hz playback [ 69.104477] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 69.109985] fsl-spdif-dai 2004000.spdif: STCSCL: 0x000f00 [ 82.233965] fsl-spdif-dai 2004000.spdif: expected clock rate = 227328000 [ 82.240820] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 82.247462] fsl-spdif-dai 2004000.spdif: set sample rate to 48008Hz for 48000Hz playback [ 82.255649] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 82.261144] fsl-spdif-dai 2004000.spdif: STCSCL: 0x400b00 [ 100.905857] fsl-spdif-dai 2004000.spdif: expected clock rate = 225792000 [ 100.912652] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 100.919284] fsl-spdif-dai 2004000.spdif: set sample rate to 88815Hz for 88200Hz playback [ 100.927481] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 100.932977] fsl-spdif-dai 2004000.spdif: STCSCL: 0x100e00 [ 101.408370] systemd-journald[300]: Sent WATCHDOG=1 notification. [ 126.448181] fsl-spdif-dai 2004000.spdif: expected clock rate = 227328000 [ 126.454996] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 126.461636] fsl-spdif-dai 2004000.spdif: set sample rate to 96017Hz for 96000Hz playback [ 126.469832] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 126.475340] fsl-spdif-dai 2004000.spdif: STCSCL: 0x500a00 [ 144.088683] fsl-spdif-dai 2004000.spdif: expected clock rate = 225792000 [ 144.095488] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 144.102126] fsl-spdif-dai 2004000.spdif: set sample rate to 177631Hz for 176400Hz playback [ 144.110492] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 144.115985] fsl-spdif-dai 2004000.spdif: STCSCL: 0x300c00 [ 172.198981] fsl-spdif-dai 2004000.spdif: expected clock rate = 24576000 [ 172.205706] fsl-spdif-dai 2004000.spdif: actual clock rate = 24000000 [ 172.212256] fsl-spdif-dai 2004000.spdif: set sample rate to 187500Hz for 192000Hz playback [ 172.220659] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 172.226150] fsl-spdif-dai 2004000.spdif: STCSCL: 0x700800 If I then re-add the MLB clock I get the following

`[ 7.095735] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 32000Hz sample rate [ 7.095743] fsl-spdif-dai 2004000.spdif: use txclk df 111 for 32000Hz sample rate [ 7.105521] fsl-spdif-dai 2004000.spdif: use rxtx6 as tx clock source for 44100Hz sample rate [ 7.105528] fsl-spdif-dai 2004000.spdif: use txclk df 94 for 44100Hz sample rate [ 7.105618] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 48000Hz sample rate [ 7.105625] fsl-spdif-dai 2004000.spdif: use txclk df 74 for 48000Hz sample rate [ 7.115739] fsl-spdif-dai 2004000.spdif: use rxtx6 as tx clock source for 88200Hz sample rate [ 7.115745] fsl-spdif-dai 2004000.spdif: use txclk df 47 for 88200Hz sample rate [ 7.115832] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 96000Hz sample rate [ 7.115837] fsl-spdif-dai 2004000.spdif: use txclk df 37 for 96000Hz sample rate [ 7.125853] fsl-spdif-dai 2004000.spdif: use rxtx1 as tx clock source for 176400Hz sample rate [ 7.125859] fsl-spdif-dai 2004000.spdif: use txclk df 20 for 176400Hz sample rate [ 7.135841] fsl-spdif-dai 2004000.spdif: use rxtx6 as tx clock source for 192000Hz sample rate [ 7.135847] fsl-spdif-dai 2004000.spdif: use txclk df 21 for 192000Hz sample rate root@sr-imx6:~# [ 38.500667] fsl-spdif-dai 2004000.spdif: expected clock rate = 227328000 [ 38.507500] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 38.514150] fsl-spdif-dai 2004000.spdif: set sample rate to 32005Hz for 32000Hz playback [ 38.522338] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 38.527827] fsl-spdif-dai 2004000.spdif: STCSCL: 0xc00300 [ 43.700832] systemd-journald[308]: Data hash table of /run/log/journal/1dbd8a77f670499fa6a11ecb5e4e2b20/sy stem.journal has a fill level at 75.0 (6472 of 8625 items, 4968448 file size, 767 bytes per hash table item), suggesting rotation. [ 43.721874] systemd-journald[308]: /run/log/journal/1dbd8a77f670499fa6a11ecb5e4e2b20/system.journal: Journ al header limits reached or header out-of-date, rotating.

[ 43.745469] systemd-journald[308]: Reserving 8625 entries in hash table.

[ 43.758369] systemd-journald[308]: Vacuuming done, freed 0B of archived journals from /run/log/journal/1db d8a77f670499fa6a11ecb5e4e2b20. [ 48.950834] fsl-spdif-dai 2004000.spdif: expected clock rate = 227328000 [ 48.957643] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 48.964281] fsl-spdif-dai 2004000.spdif: set sample rate to 32005Hz for 32000Hz playback [ 48.972511] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 48.978016] fsl-spdif-dai 2004000.spdif: STCSCL: 0xc00300 [ 54.262730] fsl-spdif-dai 2004000.spdif: expected clock rate = 265305600 [ 54.269532] fsl-spdif-dai 2004000.spdif: actual clock rate = 264000000 [ 54.276155] fsl-spdif-dai 2004000.spdif: set sample rate to 43882Hz for 44100Hz playback [ 54.284304] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 54.289759] fsl-spdif-dai 2004000.spdif: STCSCL: 0x000f00 [ 62.470383] fsl-spdif-dai 2004000.spdif: expected clock rate = 227328000 [ 62.477185] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 62.483816] fsl-spdif-dai 2004000.spdif: set sample rate to 48008Hz for 48000Hz playback [ 62.492034] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 62.497535] fsl-spdif-dai 2004000.spdif: STCSCL: 0x400b00 [ 68.309783] fsl-spdif-dai 2004000.spdif: expected clock rate = 227328000 [ 68.316579] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 68.323199] fsl-spdif-dai 2004000.spdif: set sample rate to 96017Hz for 96000Hz playback [ 68.331416] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 68.336929] fsl-spdif-dai 2004000.spdif: STCSCL: 0x500a00 [ 74.773844] fsl-spdif-dai 2004000.spdif: expected clock rate = 225792000 [ 74.780658] fsl-spdif-dai 2004000.spdif: actual clock rate = 227368421 [ 74.787291] fsl-spdif-dai 2004000.spdif: set sample rate to 177631Hz for 176400Hz playback [ 74.795660] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 74.801151] fsl-spdif-dai 2004000.spdif: STCSCL: 0x300c00 [ 80.621801] fsl-spdif-dai 2004000.spdif: expected clock rate = 258048000 [ 80.628620] fsl-spdif-dai 2004000.spdif: actual clock rate = 264000000 [ 80.635256] fsl-spdif-dai 2004000.spdif: set sample rate to 196428Hz for 192000Hz playback [ 80.643665] fsl-spdif-dai 2004000.spdif: STCSCH: 0x204000 [ 80.649165] fsl-spdif-dai 2004000.spdif: STCSCL: 0x700800 ` These numbers definitely pull us in closer to the 3.14 kernel although we are still missing the proper settings for the 44100, 88200, clocks. I have a feeling the only way to do this correct is to call imx_clk_set for each rate and figure and then see if it succeeds before testing it.

jnettlet commented 6 years ago

wow did github just munge that paste. Regardless I need to see how 3.14 is getting those fractional clocks. If we can narrow that down then I think we will be in good shape.

warped-rudi commented 6 years ago

Did you actually get any output when the MLB clock is selected? I don't. And I just found this. The documentation is not really clear, but table 43.1 of IMX6DRQM.PDF mentions MLB_CLK as external signal. I can only guess that it might be used when passing data from the MLB interface directly over to SPDIF. But I don't understand what the difference between "MLB clock input" and "MLB PHY clock input" (section 59.5.18) is.

I have a feeling the only way to do this correct is to call imx_clk_set for each rate and figure and then see if it succeeds before testing it.

That's what I think as well. From my understanding, the .dtsi should give useful defaults. Since it cannot know anything about external clock sources like SPDIF_EXT_CLK, ASRC_EXT_CLK, ESAI_HCKT and MLB_CLK, these clocks should be defined as dummy/null. If a specific board makes use of one of these, it has to override the clocks in a specialized .dts. Imagine a design, that uses an external oscillator that can be switched via GPIO to produce multiples of 44.1kHz and 48kHz (the Armada Cubox did contain such a device). IMHO, it should be possible to define this completely via *.dts without any special driver coding. I.e. one would create an new clock node and specify it's name/index as rxtx3 (if connected to SPDIF_EXT_CLK) in an overridden spdif node. The SPDIF driver then needs to select the best matching clock rate, which the current code doesn't do and the 3.14 code didn't do either.

jnettlet commented 6 years ago

This is fixed with commit d420090a78de3585a5cb33001645c251f2e85350 I allow the spdif driver to change the root clock, but only if it can properly disable and enable the other clocks. This is a limitation of the mux based on NXP's documentation. This allows us to keep the clock setup the same but still achieve the accurate clocking for the SPDIF driver.

I have also taken your 1st and 3rd patches because I agree that they are correct.

Sorry about the branch rebase however the latest lts patchset brought in a lot of conflicting patches and a rebase was cleaner than a merge. Hopefully they won't do this again in the future.