by Maxime Ripard

[permalink] [raw]

Subject: Re: [PATCH 2/7] arm: dts: sunxi: h3/h5: Add HDMI audio

Hi,

On Sun, Apr 26, 2020 at 02:04:37PM +0200, Cl?ment P?ron wrote:
> From: Marcus Cooper <[email protected]>
>
> Add a simple-soundcard to link audio between HDMI and I2S.
>
> Signed-off-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Marcus Cooper <[email protected]>
> Signed-off-by: Cl?ment P?ron <[email protected]>
> ---
> 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>;

This property doesn't exist in the binding

Maxime

Attachments:

(No filename) (1.49 kB)
signature.asc (235.00 B)
Download all attachments

2020-04-28 08:02:41

On Tue, Apr 28, 2020 at 10:54:00AM +0200, Cl?ment P?ron wrote:
> Hi Maxime,
>
> On Tue, 28 Apr 2020 at 10:00, Maxime Ripard <[email protected]> wrote:
> >
> > On Sun, Apr 26, 2020 at 02:04:39PM +0200, Cl?ment P?ron wrote:
> > > From: Marcus Cooper <[email protected]>
> > >
> > > Add a simple-soundcard to link audio between HDMI and I2S.
> > >
> > > Signed-off-by: Jernej Skrabec <[email protected]>
> > > Signed-off-by: Marcus Cooper <[email protected]>
> > > Signed-off-by: Cl?ment P?ron <[email protected]>
> > > ---
> > > 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

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

> 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

Attachments:

(No filename) (2.07 kB)
signature.asc (235.00 B)
Download all attachments

2020-04-28 16:51:17

by Clément Péron

[permalink] [raw]

Subject: Audio sound card name [was [PATCH 4/7] arm64: dts: allwinner: a64: Add HDMI audio]

Hi Mark, Rob,

On Tue, 28 Apr 2020 at 18:04, Maxime Ripard <[email protected]> wrote:
>
> On Tue, Apr 28, 2020 at 10:54:00AM +0200, Clément Péron wrote:
> > Hi Maxime,
> >
> > On Tue, 28 Apr 2020 at 10:00, Maxime Ripard <[email protected]> wrote:
> > >
> > > On Sun, Apr 26, 2020 at 02:04:39PM +0200, Clément Péron wrote:
> > > > From: Marcus Cooper <[email protected]>
> > > >
> > > > Add a simple-soundcard to link audio between HDMI and I2S.
> > > >
> > > > Signed-off-by: Jernej Skrabec <[email protected]>
> > > > Signed-off-by: Marcus Cooper <[email protected]>
> > > > Signed-off-by: Clément Péron <[email protected]>
> > > > ---
> > > > 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 :
> > 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

2020-04-28 16:59:53

2020-04-29 11:57:36

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.

Attachments:

(No filename) (2.00 kB)
signature.asc (499.00 B)
Download all attachments