[RFT PATCH 0/7] Add Allwinner H3/H5/A64 HDMI audio

[Clément Péron]

Hi,

This is some work done by Marcus Cooper and Jernej Škrabec.
These patches are present in LibreElec kernel and have been
tested by LE users.

I have rework them to follow kernel rules but I can't test
them as I don't have H3, H5 or A64 boards.

If some sunxi users could confirm they worked as expected!

This serie goes in top off "Add H6 I2S support"[0].
You can find a branch for testing here[1].

Thanks,
Clement

0: https://lwn.net/Articles/817914/
1: https://github.com/clementperon/linux/tree/allwinner_hdmi

Marcus Cooper (7):
arm: dts: sunxi: h3/h5: Add DAI node for HDMI
arm: dts: sunxi: h3/h5: Add HDMI audio
arm64: dts: allwinner: a64: Add DAI node for HDMI
arm64: dts: allwinner: a64: Add HDMI audio
arm: sun8i: h3: Add HDMI audio to Orange Pi 2
arm: sun8i: h3: Add HDMI audio to Beelink X2
arm64: dts: allwinner: a64: Add HDMI audio to Pine64

arch/arm/boot/dts/sun8i-h3-beelink-x2.dts | 8 +++++
arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 8 +++++
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 34 ++++++++++++++++++
.../boot/dts/allwinner/sun50i-a64-pine64.dts | 8 +++++
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 35 +++++++++++++++++++
5 files changed, 93 insertions(+)

--
2.20.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add a simple-soundcard to link audio between HDMI and I2S.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>
Signed-off-by: Clément Péron <peron...@gmail.com>
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 9be13378d4df..3a4262294950 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -105,6 +105,25 @@
status = "disabled";
};

+ hdmi_sound: hdmi-sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,name = "allwinner,hdmi";
+ simple-audio-card,mclk-fs = <128>;
+ simple-audio-card,frame-inversion;
+ status = "disabled";
+
+ simple-audio-card,codec {
+ sound-dai = <&hdmi>;
+ };
+
+ simple-audio-card,cpu {
+ sound-dai = <&i2s2>;
+ dai-tdm-slot-num = <2>;
+ dai-tdm-slot-width = <32>;
+ };
+ };
+
soc {
compatible = "simple-bus";
#address-cells = <1>;
@@ -672,6 +691,7 @@
dmas = <&dma 27>;
resets = <&ccu RST_BUS_I2S2>;
dma-names = "tx";
+ allwinner,playback-channels = <8>;
status = "disabled";
};

@@ -806,6 +826,7 @@
};

hdmi: hdmi@1ee0000 {
+ #sound-dai-cells = <0>;
compatible = "allwinner,sun8i-h3-dw-hdmi",
"allwinner,sun8i-a83t-dw-hdmi";
reg = <0x01ee0000 0x10000>;
--
2.20.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add the new DAI block for I2S2 which is used for HDMI audio.

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 8d10fe6d06b6..e56e1e3d4b73 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -863,6 +863,20 @@
status = "disabled";
};

+ i2s2: i2s@1c22800 {
+ #sound-dai-cells = <0>;
+ compatible = "allwinner,sun50i-a64-i2s",
+ "allwinner,sun8i-h3-i2s";
+ reg = <0x01c22800 0x400>;
+ interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;
+ clock-names = "apb", "mod";
+ resets = <&ccu RST_BUS_I2S2>;
+ dma-names = "tx";
+ dmas = <&dma 27>;

+ status = "disabled";
+ };
+

dai: dai@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-a64-codec-i2s";
--
2.20.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Enable HDMI audio on the Orange Pi 2.

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
index 597c425d08ec..64e8e2829f27 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
@@ -144,6 +144,14 @@
};
};

+&hdmi_sound {
+ status = "okay";
+};
+
+&i2s2 {
+ status = "okay";
+};
+
&ir {
pinctrl-names = "default";
pinctrl-0 = <&r_ir_rx_pin>;
--
2.20.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add a simple-soundcard to link audio between HDMI and I2S.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 21 +++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index e56e1e3d4b73..08ab6b5e72a5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -102,6 +102,25 @@
status = "disabled";
};

+ hdmi_sound: hdmi-sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,name = "allwinner,hdmi";
+ simple-audio-card,mclk-fs = <128>;
+ simple-audio-card,frame-inversion;

+ status = "disabled";
+

+ simple-audio-card,codec {
+ sound-dai = <&hdmi>;
+ };
+
+ simple-audio-card,cpu {
+ sound-dai = <&i2s2>;
+ dai-tdm-slot-num = <2>;
+ dai-tdm-slot-width = <32>;
+ };
+ };
+

osc24M: osc24M_clk {
#clock-cells = <0>;
compatible = "fixed-clock";
@@ -874,6 +893,7 @@

resets = <&ccu RST_BUS_I2S2>;
dma-names = "tx";

dmas = <&dma 27>;

+ allwinner,playback-channels = <8>;
status = "disabled";
};

@@ -1171,6 +1191,7 @@
};

hdmi: hdmi@1ee0000 {
+ #sound-dai-cells = <0>;
compatible = "allwinner,sun50i-a64-dw-hdmi",

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Enable HDMI audio on Pine64.

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 8 ++++++++

1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 2165f238af13..c5939ba52f19 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -99,6 +99,10 @@

};
};

+&hdmi_sound {
+ status = "okay";
+};
+

&i2c1 {
status = "okay";
};
@@ -107,6 +111,10 @@
bias-pull-up;
};

+&i2s2 {
+ status = "okay";
+};
+

&mdio {
ext_rmii_phy1: ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
--
2.20.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Enable HDMI audio on the Beelink X2.

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm/boot/dts/sun8i-h3-beelink-x2.dts | 8 ++++++++

1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts b/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
index 45a24441ff18..f9bec6935120 100644
--- a/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
@@ -142,6 +142,14 @@

};
};

+&hdmi_sound {
+ status = "okay";
+};
+

+&i2s2 {
+ status = "okay";
+};
+

&ir {
linux,rc-map-name = "rc-tanix-tx3mini";
pinctrl-names = "default";
--
2.20.1

[Clément Péron]

Hi Maxime,

On Tue, 28 Apr 2020 at 10:00, Maxime Ripard <max...@cerno.tech> wrote:

>
> On Sun, Apr 26, 2020 at 02:04:39PM +0200, Clément Péron wrote:
> > From: Marcus Cooper <codek...@gmail.com>
> >
> > Add a simple-soundcard to link audio between HDMI and I2S.
> >
> > Signed-off-by: Jernej Skrabec <jernej....@siol.net>
> > Signed-off-by: Marcus Cooper <codek...@gmail.com>
> > Signed-off-by: Clément Péron <peron...@gmail.com>
> > ---
> > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 21 +++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > index e56e1e3d4b73..08ab6b5e72a5 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -102,6 +102,25 @@
> > status = "disabled";
> > };
> >
> > + hdmi_sound: hdmi-sound {
> > + compatible = "simple-audio-card";
> > + simple-audio-card,format = "i2s";
> > + simple-audio-card,name = "allwinner,hdmi";
>

> I'm not sure what the usual card name should be like though. I would assume that
> this should be something specific enough so that you're able to differentiate
> between boards / SoC so that the userspace can choose a different configuration
> based on it?

I really don't know what we should use here,
I just have a look at other SoC:
rk3328: "HDMI"
rk3399: "hdmi-sound"
r8a774c0-cat874: "CAT874 HDMI sound"

But maybe it's time to introduce proper name:
What about :
sun50i-a64-hdmi
sun50i-h6-hdmi
How should we handle this with h3 / h5 ?
settings the simple-audio-card,name in each SoC dts?
Or using sun50i-h3-hdmi as they are both identical?

Regards,
Clement


>
> If so, it's really too generic.
>
> Maxime

[Clément Péron]

Hi Mark, Rob,

On Tue, 28 Apr 2020 at 18:04, Maxime Ripard <max...@cerno.tech> wrote:

> > pat
> > sun50i-h6-hdmi
>
> It's pretty much what we've been using for the other sound cards we have, so it
> makes sense to me.

I have a question regarding the simple-audio-card,name.
In this patch, I would like to introduce a simple-audio-card for the
Allwinner A64 HDMI.

What should be the preferred name for this sound card?
"sun50i-a64-hdmi" ? "allwinner, sun50i-a64-hdmi" ?

Thanks for your tips,
Clement

>
> > How should we handle this with h3 / h5 ?
> > settings the simple-audio-card,name in each SoC dts?
> > Or using sun50i-h3-hdmi as they are both identical?
>

> Yeah, we can do the latter
>
> Maxime

[Mark Brown]

On Tue, Apr 28, 2020 at 06:49:10PM +0200, Clément Péron wrote:

> I have a question regarding the simple-audio-card,name.
> In this patch, I would like to introduce a simple-audio-card for the
> Allwinner A64 HDMI.

> What should be the preferred name for this sound card?
> "sun50i-a64-hdmi" ? "allwinner, sun50i-a64-hdmi" ?

The former is more idiomatic for ALSA.

[Robin Murphy]

I can at least speak for RK3328, and the reasoning there was that as the
user looking at what `aplay -l` says, I don't give a hoot about what the
SoC may be called, I see two cards and I want to know, with the least
amount of uncertainty, which one will make the sound come out of the
port that's labelled "HDMI" on the box ;)

Robin.

[Chen-Yu Tsai]

I agree. The user really doesn't care what SoC the system uses. The only
real requirement is to be able to tell which output the card is related
to, i.e. is it onboard or an external DAC, is it analog or HDMI, etc..

ChenYu

[Clément Péron]

Hi,

On Wed, 29 Apr 2020 at 10:17, Maxime Ripard <max...@cerno.tech> wrote:

> Yeah, but it's exactly the point.
>
> If we also end up with "HDMI" as our card name, then the userspace has no way to
> tell anymore if it's running from an rk3328 or an allwinner SoC, or something
> else entirely. And therefore it cannot really configure anything to work out of
> the box anymore.

Yes the final user doesn't care if it's sun50i A64 HDMI or sun50i H6
HDMI, but like Maxime said between final user and the sound-card name
you may want to have a configuration layer.
This configuration layer can apply different settings depending on the
sound card.
Having too generic name will make impossible specific card configuration.

Also you can have specific name per board which can be quite clear for
final user.
- sun50i-a64-hdmi
- sun50i-a64-spdif
is not so hard to understand that one point to the HDMI whereas the
other point to SPDIF.

Regards,
Clement

>
> Maxime

[Robin Murphy]

On 2020-04-29 9:17 am, Maxime Ripard wrote:
> On Wed, Apr 29, 2020 at 02:24:00PM +0800, Chen-Yu Tsai wrote:

> Yeah, but it's exactly the point.
>
> If we also end up with "HDMI" as our card name, then the userspace has no way to
> tell anymore if it's running from an rk3328 or an allwinner SoC, or something
> else entirely. And therefore it cannot really configure anything to work out of
> the box anymore.

OK, you're a userspace audio application - enlighten me as to what exact
chip you're running on here, and why you need to know:

card 0: HDMI [HDA ATI HDMI]

or how about here?

card 0: Intel [HDA Intel]


Furthermore, your argument works both ways - if the equivalent (or in
common cases like DesignWare IP blocks, exact same) thing across 3
different SoCs has 3 different names, then it's that much harder for
userspace that wants to present a consistent behaviour. I don't know
exactly why LibreELEC have downstream patches that standardise all the
Rockchip ones to "HDMI", but I can't help noting that they do.

With simple-audio-card we're talking about trivial interfaces that often
don't expose any controls at all, so there's unlikely to be much
'configuration' for userspace to do beyond choosing which card to output to.

Robin.

[Jernej Škrabec]

Dne sreda, 29. april 2020 ob 12:43:06 CEST je Robin Murphy napisal(a):

This combination (DesignWare HDMI controller + I2S) is same as on Rockchip.
Only difference is slightly different version of HDMI controller and different
I2S core. Not sure what kind of configuration do you have in mind, but all
these controllers support 2-8 channels, different sample sizes, even
passthrough mode can be set (but it's not implemented yet). I would say that
this audio output supports quiet a few possible configurations.

BTW, above patches were thoroughly tested in LibreELEC. The only missing thing
is extending I2S driver to support channel allocation through DT, but that's
outside the scope of this series. Currently only 2 channels will work
correctly.

Best regards,
Jernej

[Robin Murphy]

Right, what I'm getting at is that the stuff that matters is all regular
audio capabilities exposed via the appropriate APIs, and any choices are
going to be mostly based on the audio being played and what the receiver
on the other end of the link supports. I'm genuinely curious as to what
actual configuration decision could be made purely based on the SoC
name, regardless of the board, receiver, or the capabilities advertised
by the interface itself.

Thanks,
Robin.

[Mark Brown]

On Wed, Apr 29, 2020 at 11:43:06AM +0100, Robin Murphy wrote:
> On 2020-04-29 9:17 am, Maxime Ripard wrote:
> > On Wed, Apr 29, 2020 at 02:24:00PM +0800, Chen-Yu Tsai wrote:

Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

> > If we also end up with "HDMI" as our card name, then the userspace has no way to
> > tell anymore if it's running from an rk3328 or an allwinner SoC, or something
> > else entirely. And therefore it cannot really configure anything to work out of
> > the box anymore.

> OK, you're a userspace audio application - enlighten me as to what exact
> chip you're running on here, and why you need to know:

> card 0: HDMI [HDA ATI HDMI]

> or how about here?

> card 0: Intel [HDA Intel]

In the case of HDMI for embedded platforms since there is generally no
control in the audio path it is unlikely to make a *huge* difference,
though if there are expansion buses or multiple HDMI ports it can be
useful to help people identify which particular HDMI port it is. For
other cards the names are part of userspace working out which config
file to apply to the card so deduplication can help, and also the
plastics tend to matter.

> With simple-audio-card we're talking about trivial interfaces that often
> don't expose any controls at all, so there's unlikely to be much
> 'configuration' for userspace to do beyond choosing which card to output to.

This is a reasonable assumption for HDMI but it is not at all a
reasonable assumption for simple-audio-card in general - just because
the links between the SoC and the external components are simple that
doesn't mean that any of those components are simple, and even if the
hardware is simple that does not mean that configuration is unimportant
- the difference between full scale output and appropriate headphone
volumes is for example *extremely* important.