2013-08-28 09:34:36

by Jean-Francois Moine

[permalink] [raw]
Subject: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

This patch activates the audio device of the Cubox and sets the i2s
and S/PDIF pins.

Signed-off-by: Jean-Francois Moine <[email protected]>
---
arch/arm/boot/dts/dove-cubox.dts | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
index 4af59b6..003d95b 100644
--- a/arch/arm/boot/dts/dove-cubox.dts
+++ b/arch/arm/boot/dts/dove-cubox.dts
@@ -132,3 +132,16 @@
reg = <0>;
};
};
+
+&pinctrl {
+ pmx_audio1_i2s1_spdifo: pmx-audio1-i2s1-spdifo {
+ marvell,pins = "mpp_audio1";
+ marvell,function = "i2s1/spdifo";
+ };
+};
+
+&i2s1 {
+ status = "okay";
+ pinctrl-0 = <&pmx_audio1_i2s1_spdifo>;
+ pinctrl-names = "default";
+};


--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/


2013-08-28 10:15:05

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On 08/28/2013 11:35 AM, Jean-Francois Moine wrote:
> This patch activates the audio device of the Cubox and sets the i2s
> and S/PDIF pins.
>
> Signed-off-by: Jean-Francois Moine <[email protected]>
> ---
> arch/arm/boot/dts/dove-cubox.dts | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
> index 4af59b6..003d95b 100644
> --- a/arch/arm/boot/dts/dove-cubox.dts
> +++ b/arch/arm/boot/dts/dove-cubox.dts
> @@ -132,3 +132,16 @@
> reg = <0>;
> };
> };
> +
> +&pinctrl {
> + pmx_audio1_i2s1_spdifo: pmx-audio1-i2s1-spdifo {
> + marvell,pins = "mpp_audio1";
> + marvell,function = "i2s1/spdifo";
> + };
> +};
> +
> +&i2s1 {
> + status = "okay";
> + pinctrl-0 = <&pmx_audio1_i2s1_spdifo>;
> + pinctrl-names = "default";
> +};

Please also add the external clock connected from si5351.

Sebastian

2013-08-31 10:50:33

by Jean-Francois Moine

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Wed, 28 Aug 2013 12:14:24 +0200
Sebastian Hesselbarth <[email protected]> wrote:

> > +&i2s1 {
> > + status = "okay";
> > + pinctrl-0 = <&pmx_audio1_i2s1_spdifo>;
> > + pinctrl-names = "default";
> > +};
>
> Please also add the external clock connected from si5351.

Hi Sebastian and Russell,

I tried it on my Cubox (<&si5351 2>), and I have problems with HDMI
output and some audio streams like webradios with sample rates 32 or
22.05 kHz.

According to the Dove specification, the audio controller works with
the samples rates 44.1, 48 and 96 kHz, so, I don't see the usage of the
external clock, except when using the two audio controllers with
different sample rates.

But, BTW, as the kirkwood-i2 driver is written, this last case does not
work: for 44.1, 48 and 96 kHz, the external clock is never used and
there is only one DCO.

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/

2013-08-31 11:31:36

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Sat, Aug 31, 2013 at 12:51:28PM +0200, Jean-Francois Moine wrote:
> Hi Sebastian and Russell,
>
> I tried it on my Cubox (<&si5351 2>), and I have problems with HDMI
> output and some audio streams like webradios with sample rates 32 or
> 22.05 kHz.

Some TVs will only accept certain sample rates - my (brand new) TV has
an audio block in the EDID containing this (starting at byte 0x92 -
note - I don't think it has to start at that offset):

23 09 07 01

0x23 = audio block code (7:5 = 1, 4:0 = size)
0x09 = 7:3 format code = 1 (PCM), 2:0 number of channels = 2
0x07 = 48kHz, 44.1kHz and 32kHz supported
0x01 = 16-bit

There is no bit to indicate 22.05kHz support - 32kHz is the lowest which
the EDID block can report.

Also, this new TV seems to be more fussy than the old - it remains silent
with 48kHz playback, but works with 44.1kHz. I've tried tweaking a fair
number of the registers both in the NXP and Dove, including the SPDIF
channel status, and nothing seems to make any difference. Given that
the old TV just converted HDMI to analogue - yes, really - and this one
doesn't, I suspect it has stricter checking of the HDMI data.

However, even with the EDID reporting a restricted set of sample rates,
this does not mean that we should restrict ALSAs selection: remember,
the SPDIF is also routed out through the TOSlink output, which can
(undetectably, since it is output only) support more sample rates than
the HDMI connected device.

In the longer term, we probably want to eventually connect the NXP into
the ASoC DPCM code, so that it can be informed about the properties of
the audio being fed to it. It can then compare the capabililties of the
connected device and disable HDMI audio output if they're not compatible.
That requires working DPCM first though.

> According to the Dove specification, the audio controller works with
> the samples rates 44.1, 48 and 96 kHz, so, I don't see the usage of the
> external clock, except when using the two audio controllers with
> different sample rates.

I don't see what the Dove specification has to do with that statement:
what the Dove spec says is that if you use just the internal DCO, then
only 44.1kHz, 48kHz and 96kHz are supported (with some trimming of that.)
However, the use of an external clock allows further rates to be supported.
If you have an external clock, there is no requirement to use the DCO for
those sample rates - you can if you wish, or you can use the external clock.

The mainline driver implements the use of the DCO for the standard 44.1,
48 and 96kHz rates, otherwise it uses the external clock if present. This
is entirely conformant with the Dove spec.

If you attempt to use both audio controllers on the Cubox with different
sample rates which aren't supported by the DCO, then they will fight over
the input clock rate. That's a limitation of the hardware, and there's
no real solution to that. As the Cubox does not wire up the first audio
controller, it should not be enabled in DT. If a board does use both, and
they both use the external clock input, then that's the time that this
needs to be solved.

We have more than enough problems to sort out without inventing new
problems.

> But, BTW, as the kirkwood-i2 driver is written, this last case does not
> work: for 44.1, 48 and 96 kHz, the external clock is never used and
> there is only one DCO.

This doesn't make sense. "this last case" - what "case" are you referring
to? "there is only one DCO" ? Yes, there is only one DCO, what is its
relevance to the statement you're making? And yes, in mainline we
currently use the DCO for the standard 44.1, 48 and 96kHz sample rates.
That's fine. Confused.

2013-08-31 11:54:46

by Jean-Francois Moine

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Sat, 31 Aug 2013 12:24:31 +0100
Russell King - ARM Linux <[email protected]> wrote:

> > According to the Dove specification, the audio controller works with
> > the samples rates 44.1, 48 and 96 kHz, so, I don't see the usage of the
> > external clock, except when using the two audio controllers with
> > different sample rates.
[snip]
> > But, BTW, as the kirkwood-i2 driver is written, this last case does not
> > work: for 44.1, 48 and 96 kHz, the external clock is never used and
> > there is only one DCO.
>
> This doesn't make sense. "this last case" - what "case" are you referring
> to? "there is only one DCO" ? Yes, there is only one DCO, what is its
> relevance to the statement you're making? And yes, in mainline we
> currently use the DCO for the standard 44.1, 48 and 96kHz sample rates.
> That's fine. Confused.

Sorry, it was clear in my head :)

The last case was "when using the two audio controllers with different
sample rates" (I should have added "in the set [44.1, 48, 96 kHz]").
Then, with or without the availability of external clocks, both
controllers will set the unique DCO to two different rates.

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/

2013-08-31 12:22:53

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Sat, Aug 31, 2013 at 01:55:19PM +0200, Jean-Francois Moine wrote:
> The last case was "when using the two audio controllers with different
> sample rates" (I should have added "in the set [44.1, 48, 96 kHz]").
> Then, with or without the availability of external clocks, both
> controllers will set the unique DCO to two different rates.

Ah, yes. The documentation is rather confusing too. In one part, it
does indeed say that there is only one DCO which is shared between the
two blocks.

However, each block has its own DCO register which allows it to be set
to a different frequency - maybe it accesses the same physical register.

In which case, it probably would be better that we use the external clock
for everything when it is available - Rabeeh's kernel already does that,
as does the kernel which I run on my Cubox. I'll pull that patch over to
the series for mainline later today:

a402fa7 ASoC: kirkwood: prefer external clock over internal clock

(This is copy'n'pasted with gpm, so is white-space damaged).
diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c
index 1c8177b..92cd467 100644
--- a/sound/soc/kirkwood/kirkwood-i2s.c
+++ b/sound/soc/kirkwood/kirkwood-i2s.c
@@ -222,20 +222,20 @@ static void kirkwood_set_rate(struct snd_soc_dai *dai,
{
uint32_t clks_ctrl;

- if (rate == 44100 || rate == 48000 || rate == 96000) {
- /* use internal dco for supported rates */
- dev_dbg(dai->dev, "%s: dco set rate = %lu\n",
- __func__, rate);
- kirkwood_set_dco(priv->io, rate);
-
- clks_ctrl = KIRKWOOD_MCLK_SOURCE_DCO;
- } else if (!IS_ERR(priv->extclk)) {
+ if (!IS_ERR(priv->extclk)) {
/* use optional external clk for other rates */
dev_dbg(dai->dev, "%s: extclk set rate = %lu -> %lu\n",
__func__, rate, 256 * rate);
clk_set_rate(priv->extclk, 256 * rate);

clks_ctrl = KIRKWOOD_MCLK_SOURCE_EXTCLK;
+ } else if (rate == 44100 || rate == 48000 || rate == 96000) {
+ /* use internal dco for supported rates */
+ dev_dbg(dai->dev, "%s: dco set rate = %lu\n",
+ __func__, rate);
+ kirkwood_set_dco(priv->io, rate);
+
+ clks_ctrl = KIRKWOOD_MCLK_SOURCE_DCO;
}
writel(clks_ctrl, priv->io + KIRKWOOD_CLOCKS_CTRL);
}

2013-09-01 09:27:05

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

Jean-Francois,
Sorry for the messy reply, posting from mobile.

Can you please try <&si5351 1>, iirc audio ist connected to it. I will also
look it up later.

Sebastian



On August 31, 2013 12:51:28 PM Jean-Francois Moine <[email protected]> wrote:
> On Wed, 28 Aug 2013 12:14:24 +0200
> Sebastian Hesselbarth <[email protected]> wrote:
>
> > > +&i2s1 {
> > > + status = "okay";
> > > + pinctrl-0 = <&pmx_audio1_i2s1_spdifo>;
> > > + pinctrl-names = "default";
> > > +};
> > Please also add the external clock connected from si5351.
>
> Hi Sebastian and Russell,
>
> I tried it on my Cubox (<&si5351 2>), and I have problems with HDMI
> output and some audio streams like webradios with sample rates 32 or
> 22.05 kHz.
>
> According to the Dove specification, the audio controller works with
> the samples rates 44.1, 48 and 96 kHz, so, I don't see the usage of the
> external clock, except when using the two audio controllers with
> different sample rates.
>
> But, BTW, as the kirkwood-i2 driver is written, this last case does not
> work: for 44.1, 48 and 96 kHz, the external clock is never used and
> there is only one DCO.
>
> --
> Ken ar c'hentañ | ** Breizh ha Linux atav! **
> Jef | http://moinejf.free.fr/

2013-09-26 08:09:39

by Jean-Francois Moine

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Sun, 01 Sep 2013 11:26:54 +0200
Sebastian Hesselbarth <[email protected]> wrote:

> Jean-Francois,
> Sorry for the messy reply, posting from mobile.
>
> Can you please try <&si5351 1>, iirc audio ist connected to it. I will also
> look it up later.

Sebastian,

In the Cubox, I changed the audio DT to:

&i2s1 {
status = "okay";
clocks = <&gate_clk 13>, <&si5351 1>;
clock-names = "internal", "extclk";
pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_gpio_13_audio1>;
};

and the sound does not get out correctly.

There is no sound at all, except when restarting playing which gives
some shrill sound for half a second at start time.

I lowered the rate in kirkwood-i2s, setting the clock by (* 128)
instead of (* 256), and, most of the time, this gives continuous shrill
and chopped sound at various sample rates (22.05, 32 and 44.1 kHz).
Lowering the clock to (* 64) gives no sound at all.

For more information, I use i2s input in the tda998x with audio rate
96kHz on HDMI output. The sound is fine with the internal dco (tested
with 44.1kHz only).

Some idea?

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/

2013-09-26 08:31:21

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On 09/26/2013 10:11 AM, Jean-Francois Moine wrote:
> In the Cubox, I changed the audio DT to:
>
> &i2s1 {
> status = "okay";
> clocks = <&gate_clk 13>, <&si5351 1>;
> clock-names = "internal", "extclk";
> pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_gpio_13_audio1>;

Have you checked that pinmux was configured correctly?

You are missing a

pinctrl-names = "default";

to actually execute the above pinmux setting.

> };
>
> and the sound does not get out correctly.
>
> There is no sound at all, except when restarting playing which gives
> some shrill sound for half a second at start time.

Could be clock setup stability issues on si5351. Will check that later
with a scope, maybe I also check spdif output rate with it too.

> I lowered the rate in kirkwood-i2s, setting the clock by (* 128)
> instead of (* 256), and, most of the time, this gives continuous shrill
> and chopped sound at various sample rates (22.05, 32 and 44.1 kHz).
> Lowering the clock to (* 64) gives no sound at all.

What about trying to get 48kHz working first? Does it work with
internal DCO and 48kHz? External audio clock should be 256*fs as stated
in dove datasheet.

> For more information, I use i2s input in the tda998x with audio rate
> 96kHz on HDMI output. The sound is fine with the internal dco (tested
> with 44.1kHz only).

Sorry, I don't understand the above "information". How can you have a
different input audio rate tested with 44k1 output rate?

> Some idea?

Have you ever tested your audio equipment with something different than
48k and 44k1 SPDIF PCM? Are you actually sure it works at all, i.e. do
you have different audio source with SPDIF >48kHz? Can you try optical
SPDIF instead of HDMI?

Just try to narrow it down, currently I cannot see why this behavior
should be limited to either si5351 or dove audio alone.

Sebastian

2013-09-26 11:26:50

by Jean-Francois Moine

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Thu, 26 Sep 2013 10:31:10 +0200
Sebastian Hesselbarth <[email protected]> wrote:

> On 09/26/2013 10:11 AM, Jean-Francois Moine wrote:
> > In the Cubox, I changed the audio DT to:
> >
> > &i2s1 {
> > status = "okay";
> > clocks = <&gate_clk 13>, <&si5351 1>;
> > clock-names = "internal", "extclk";
> > pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_gpio_13_audio1>;
>
> Have you checked that pinmux was configured correctly?
>
> You are missing a
>
> pinctrl-names = "default";
>
> to actually execute the above pinmux setting.

Thanks, the pinmux was not set correctly, and I added the "default".

[snip]
> What about trying to get 48kHz working first? Does it work with
> internal DCO and 48kHz? External audio clock should be 256*fs as stated
> in dove datasheet.

I have no input with 48kHz.

> > For more information, I use i2s input in the tda998x with audio rate
> > 96kHz on HDMI output. The sound is fine with the internal dco (tested
> > with 44.1kHz only).
>
> Sorry, I don't understand the above "information". How can you have a
> different input audio rate tested with 44k1 output rate?

The internal dco permits only 44.1, 44 and 96 kHz, so, vlc translates
the input rates smaller than 44.1 kHz to 44.1 kHz.

> Have you ever tested your audio equipment with something different than
> 48k and 44k1 SPDIF PCM? Are you actually sure it works at all, i.e. do
> you have different audio source with SPDIF >48kHz? Can you try optical
> SPDIF instead of HDMI?
>
> Just try to narrow it down, currently I cannot see why this behavior
> should be limited to either si5351 or dove audio alone.

I have no optical S/PDIF.

Well, I did more tests:

- i2s or s/pdif input by the tda998x work fine with the internal dco

- the pinmux seems correct:

device f10b4000.audio-controller
state default
type MUX_GROUP (2)
controlling device f10d0200.pin-ctrl
group mpp_audio1
function i2s1/spdifo

device f10b4000.audio-controller
state default
type MUX_GROUP (2)
controlling device f10d0200.pin-ctrl
group mpp13
function audio1

- I tried both <&si5351 1> and <&si5351 2>, and I get no sound with the
external clock.

- when running vlc with the external clock, the flux towards the
kirkwood driver is blocked and there is no sound interrupt.

47: 0 main-interrupt-ctrl 21 kirkwood-i2s

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/

2013-09-26 12:42:30

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On 09/26/2013 01:28 PM, Jean-Francois Moine wrote:
> On Thu, 26 Sep 2013 10:31:10 +0200
> Sebastian Hesselbarth <[email protected]> wrote:
>
>> On 09/26/2013 10:11 AM, Jean-Francois Moine wrote:
>>> In the Cubox, I changed the audio DT to:
>>>
>>> &i2s1 {
>>> status = "okay";
>>> clocks = <&gate_clk 13>, <&si5351 1>;
>>> clock-names = "internal", "extclk";
>>> pinctrl-0 = <&pmx_audio1_i2s1_spdifo &pmx_gpio_13_audio1>;
>>
>> Have you checked that pinmux was configured correctly?
>>
>> You are missing a
>>
>> pinctrl-names = "default";
>>
>> to actually execute the above pinmux setting.
>
> Thanks, the pinmux was not set correctly, and I added the "default".
>
> [snip]
>> What about trying to get 48kHz working first? Does it work with
>> internal DCO and 48kHz? External audio clock should be 256*fs as stated
>> in dove datasheet.
>
> I have no input with 48kHz.

Or 44k1, just something that works with internal dco. kirkwood-i2s
should request 256*fs, or 11289600 Hz for 44k1 audio rate from external
clock.

>>> For more information, I use i2s input in the tda998x with audio rate
>>> 96kHz on HDMI output. The sound is fine with the internal dco (tested
>>> with 44.1kHz only).
>>
>> Sorry, I don't understand the above "information". How can you have a
>> different input audio rate tested with 44k1 output rate?
>
> The internal dco permits only 44.1, 44 and 96 kHz, so, vlc translates
> the input rates smaller than 44.1 kHz to 44.1 kHz.

So you are actually testing 44k1 in any case. Let's stick with 44k1 for
now.

>> Have you ever tested your audio equipment with something different than
>> 48k and 44k1 SPDIF PCM? Are you actually sure it works at all, i.e. do
>> you have different audio source with SPDIF >48kHz? Can you try optical
>> SPDIF instead of HDMI?
>>
>> Just try to narrow it down, currently I cannot see why this behavior
>> should be limited to either si5351 or dove audio alone.
>
> I have no optical S/PDIF.

Ok, but 44k1 works with HDMI, we try to reproduce 44k1 with external
clock now.

> Well, I did more tests:
>
> - i2s or s/pdif input by the tda998x work fine with the internal dco

Ok.

> - the pinmux seems correct:
>
> device f10b4000.audio-controller
> state default
> type MUX_GROUP (2)
> controlling device f10d0200.pin-ctrl
> group mpp_audio1
> function i2s1/spdifo
>
> device f10b4000.audio-controller
> state default
> type MUX_GROUP (2)
> controlling device f10d0200.pin-ctrl
> group mpp13
> function audio1
>
> - I tried both <&si5351 1> and <&si5351 2>, and I get no sound with the
> external clock.
>
> - when running vlc with the external clock, the flux towards the
> kirkwood driver is blocked and there is no sound interrupt.
>
> 47: 0 main-interrupt-ctrl 21 kirkwood-i2s

Can you add "usleep_range(10000, 12000);" after clk_set_rate(extclk) in
kirkwood-i2s. That should be enough to get around any issues with
external clock not being stable. I will look up real requirements for
si5351 later (and actually this should move to clk-si5351.c later too).

Please make sure, you stick with 44k1 (or something that works with
DCO).

Sebastian

2013-09-28 14:27:44

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On 09/26/2013 01:28 PM, Jean-Francois Moine wrote:
> On Thu, 26 Sep 2013 10:31:10 +0200
> Sebastian Hesselbarth <[email protected]> wrote:
>> On 09/26/2013 10:11 AM, Jean-Francois Moine wrote:
>>> In the Cubox, I changed the audio DT to:
>>>
>>> &i2s1 {
>>> status = "okay";
>>> clocks = <&gate_clk 13>, <&si5351 1>;

Jean-Francois,

I can confirm the following for CuBox:

AU1_EXTCLK is connected to si5351 clkout 2. For the _current_
(v3.12-rc1) dove-cubox.dts that means that you'll have to exchange
the properties for &i2c0/si5351/clkout[12]. If you leave it the way
it is, clkout2 is not allowed to change the pll inside si5351 and
only try to get close to the requested clk rate by using output
dividers. Would be great if you can provide a corresponding patch
for v3.12 (no need to Cc stable).

Also, as you seem to push kirkwood-i2s DT forward, please get back
to clkout2 (<&si5351 2>) for the audio node as you did correctly in
the first place.

With above changes, SPDIF_EN, and forcing kirkwood-i2s to always use
extclk, I can play SPDIF audio with 44k1 and 48k. Other rates could
also work, but my audio equipment ignores anything else.

>> What about trying to get 48kHz working first? Does it work with
>> internal DCO and 48kHz? External audio clock should be 256*fs as stated
>> in dove datasheet.
>
> I have no input with 48kHz.

If you use e.g. 'mplayer -ao alsa -srate 48000 music.mp3' you can force
output audio rate to 48k. I am sure other tools have similar options.

>>> For more information, I use i2s input in the tda998x with audio rate
>>> 96kHz on HDMI output. The sound is fine with the internal dco (tested
>>> with 44.1kHz only).

What I am wondering here is, why you want to use i2s at all? On CuBox,
HDMI transmitter is connected to both i2s and spdif. IIRC, for the
transmitter it makes no difference if you provide i2s or spdif audio,
_but_ what you are loosing for sure is pass-through, i.e. sending
compressed AC3 directly.

Also, on consumer devices anything else than 44k1 and 48k audio is very
uncommon. Don't expect it to work at all. If you want high quality
audio, always use pass-through. If you are lucky, your audio equipment
will also accept mp3 or at least mp2 over it when using HDMI.

>> Sorry, I don't understand the above "information". How can you have a
>> different input audio rate tested with 44k1 output rate?
>
> The internal dco permits only 44.1, 44 and 96 kHz, so, vlc translates
> the input rates smaller than 44.1 kHz to 44.1 kHz.
>
>> Have you ever tested your audio equipment with something different than
>> 48k and 44k1 SPDIF PCM? Are you actually sure it works at all, i.e. do
>> you have different audio source with SPDIF >48kHz? Can you try optical
>> SPDIF instead of HDMI?
>>
>> Just try to narrow it down, currently I cannot see why this behavior
>> should be limited to either si5351 or dove audio alone.
>
> I have no optical S/PDIF.

As stated above, if you use SPDIF in for HDMI audio, you get the same
audio stream.

> Well, I did more tests:
>
> - i2s or s/pdif input by the tda998x work fine with the internal dco

Based on the patches you sent me, you do not enable SPDIF playback at
all. Are you aware of that?

> - the pinmux seems correct:
>
> device f10b4000.audio-controller
> state default
> type MUX_GROUP (2)
> controlling device f10d0200.pin-ctrl
> group mpp_audio1
> function i2s1/spdifo
>
> device f10b4000.audio-controller
> state default
> type MUX_GROUP (2)
> controlling device f10d0200.pin-ctrl
> group mpp13
> function audio1
>
> - I tried both <&si5351 1> and <&si5351 2>, and I get no sound with the
> external clock.

As stated above, with <&si5351 2> I can playback both 44k1 and 48k over
optical SPDIF. Haven't tested HDMI, but if that doesn't work, it's not
related to extclk.

Sebastian

> - when running vlc with the external clock, the flux towards the
> kirkwood driver is blocked and there is no sound interrupt.
>
> 47: 0 main-interrupt-ctrl 21 kirkwood-i2s
>

2013-10-03 16:14:17

by Jean-Francois Moine

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Sat, 28 Sep 2013 16:27:19 +0200
Sebastian Hesselbarth <[email protected]> wrote:

> Jean-Francois,
>
> I can confirm the following for CuBox:
>
> AU1_EXTCLK is connected to si5351 clkout 2. For the _current_
> (v3.12-rc1) dove-cubox.dts that means that you'll have to exchange
> the properties for &i2c0/si5351/clkout[12]. If you leave it the way
> it is, clkout2 is not allowed to change the pll inside si5351 and
> only try to get close to the requested clk rate by using output
> dividers. Would be great if you can provide a corresponding patch
> for v3.12 (no need to Cc stable).
>
> Also, as you seem to push kirkwood-i2s DT forward, please get back
> to clkout2 (<&si5351 2>) for the audio node as you did correctly in
> the first place.
>
> With above changes, SPDIF_EN, and forcing kirkwood-i2s to always use
> extclk, I can play SPDIF audio with 44k1 and 48k. Other rates could
> also work, but my audio equipment ignores anything else.

Sebastian,

Thanks for your change. I can now get audio at 44.1, 32 and 22.05 kHz
on HDMI with the external clock (via i2s, but s/pdif and other rates
should work too).

[snip]
> What I am wondering here is, why you want to use i2s at all? On CuBox,
> HDMI transmitter is connected to both i2s and spdif. IIRC, for the
> transmitter it makes no difference if you provide i2s or spdif audio,
> _but_ what you are loosing for sure is pass-through, i.e. sending
> compressed AC3 directly.
[snip]

It seems that my patch about adding s/pdif to the kirkwood audio driver
has not been accepted yet...

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/

2013-10-08 16:12:00

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

Jean-Francois,

On Thu, Oct 03, 2013 at 06:17:16PM +0200, Jean-Francois Moine wrote:
> It seems that my patch about adding s/pdif to the kirkwood audio driver
> has not been accepted yet...

Looking over that conversation does not give the impression that
concerns were resolved. I'm a little short on time atm, all I can add
definitively is that I agree with Thomas wrt the compatible strings.
They should be named after the first compatible IP block (in this case,
kirkwood).

Please submit a fix for v3.12-rc cleaning up what Mark Brown accepted so
we don't have to maintain it indefinitely, and we'll see if we can work
out the rest in fairly short order. Posting a new version of series
would help kick off that conversation... ;-)

thx,

Jason.

2013-10-08 17:56:34

by Jean-Francois Moine

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Tue, 8 Oct 2013 12:11:45 -0400
Jason Cooper <[email protected]> wrote:

> Jean-Francois,
>
> On Thu, Oct 03, 2013 at 06:17:16PM +0200, Jean-Francois Moine wrote:
> > It seems that my patch about adding s/pdif to the kirkwood audio driver
> > has not been accepted yet...
>
> Looking over that conversation does not give the impression that
> concerns were resolved. I'm a little short on time atm, all I can add
> definitively is that I agree with Thomas wrt the compatible strings.
> They should be named after the first compatible IP block (in this case,
> kirkwood).
>
> Please submit a fix for v3.12-rc cleaning up what Mark Brown accepted so
> we don't have to maintain it indefinitely, and we'll see if we can work
> out the rest in fairly short order. Posting a new version of series
> would help kick off that conversation... ;-)

Jason,

Thank you, I had forgotten this patch series. New submit done.

As it is a new development, it may go to 3.13. The compatible string
has been updated by Thomas and it is present in 3.12-rc1.

In my quoted message, I was thinking about the patch:

[PATCH] ASoC: kirkwood: add S/PDIF support

dated 24 Sep 2013.

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/

2013-10-08 18:00:55

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Dove: Add the audio device to the Cubox DT

On Tue, Oct 08, 2013 at 07:59:49PM +0200, Jean-Francois Moine wrote:
> On Tue, 8 Oct 2013 12:11:45 -0400
> Jason Cooper <[email protected]> wrote:
>
> > Jean-Francois,
> >
> > On Thu, Oct 03, 2013 at 06:17:16PM +0200, Jean-Francois Moine wrote:
> > > It seems that my patch about adding s/pdif to the kirkwood audio driver
> > > has not been accepted yet...
> >
> > Looking over that conversation does not give the impression that
> > concerns were resolved. I'm a little short on time atm, all I can add
> > definitively is that I agree with Thomas wrt the compatible strings.
> > They should be named after the first compatible IP block (in this case,
> > kirkwood).
> >
> > Please submit a fix for v3.12-rc cleaning up what Mark Brown accepted so
> > we don't have to maintain it indefinitely, and we'll see if we can work
> > out the rest in fairly short order. Posting a new version of series
> > would help kick off that conversation... ;-)
>
> Jason,
>
> Thank you, I had forgotten this patch series. New submit done.
>
> As it is a new development, it may go to 3.13. The compatible string
> has been updated by Thomas and it is present in 3.12-rc1.

ok, great!

thx,

Jason.