[RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[John Stultz]

Add entry for k3-dma driver and i2s/hdmi audio devices.

This enables HDMI audio output.

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Antonio Borneo <borneo.antonio@gmail.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
Olof/Arnd: I'm having trouble getting a repsonse from Wei on
this patch, and want to try to avoid missing another merge
window. Might you consider queuing this directly?

v2:
* Split core i2s entry into dtsi and hdmi specific bits into
  hikey dts
v4:
* Rework simple-card to use many-dai-links method, as
  there may be other links in the future
---
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++++++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 26 ++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index 5cdfe73..c916929 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -468,6 +468,21 @@
 			method = "smc";
 		};
 	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,name = "hikey-hdmi";
+
+		simple-audio-card,dai-link@0 {          /* I2S - HDMI */
+			format = "i2s";
+			cpu {
+				sound-dai = <&i2s0 0>;
+			};
+			codec {
+				sound-dai = <&adv7533>;
+			};
+		};
+	};
 };
 
 &uart2 {
@@ -508,6 +523,7 @@
 		interrupts = <1 2>;
 		pd-gpio = <&gpio0 4 0>;
 		adi,dsi-lanes = <4>;
+		#sound-dai-cells = <0>;
 
 		port {
 			adv7533_in: endpoint {
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 5013e4b..f2e218c 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -332,6 +332,19 @@
 			status = "disabled";
 		};
 
+		dma0: dma@f7370000 {
+			compatible = "hisilicon,k3-dma-1.0";
+			reg = <0x0 0xf7370000 0x0 0x1000>;
+			#dma-cells = <1>;
+			dma-channels = <15>;
+			dma-requests = <32>;
+			interrupts = <0 84 4>;
+			clocks = <&sys_ctrl HI6220_EDMAC_ACLK>;
+			dma-no-cci;
+			dma-type = "hi6220_dma";
+			status = "ok";
+		};
+
 		dual_timer0: timer@f8008000 {
 			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x0 0xf8008000 0x0 0x1000>;
@@ -805,6 +818,19 @@
 			#thermal-sensor-cells = <1>;
 		};
 
+		i2s0: i2s@f7118000{
+			compatible = "hisilicon,hi6210-i2s";
+			reg = <0x0 0xf7118000 0x0 0x8000>; /* i2s unit */
+			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>; /* 155 "DigACodec_intr"-32 */
+			clocks = <&sys_ctrl HI6220_DACODEC_PCLK>,
+				 <&sys_ctrl HI6220_BBPPLL0_DIV>;
+			clock-names = "dacodec", "i2s-base";
+			dmas = <&dma0 15 &dma0 14>;
+			dma-names = "rx", "tx";
+			hisilicon,sysctrl-syscon = <&sys_ctrl>;
+			#sound-dai-cells = <1>;
+		};
+
 		thermal-zones {
 
 			cls0: cls0 {
-- 
2.7.4

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

On Mon, Jun 12, 2017 at 01:52:46PM -0700, John Stultz wrote:

> +	sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "hikey-hdmi";

Now the graph card has been merged it's probably a good idea to be using
that, it's generally a better idea for pretty much all use cases.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[John Stultz]

On Mon, Jun 12, 2017 at 3:10 PM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Jun 12, 2017 at 01:52:46PM -0700, John Stultz wrote:
>
>> +     sound {
>> +             compatible = "simple-audio-card";
>> +             simple-audio-card,name = "hikey-hdmi";
>
> Now the graph card has been merged it's probably a good idea to be using
> that, it's generally a better idea for pretty much all use cases.

I've taken a shot today trying to convert over to the audio-graph-card
method (which isn't yet upstream, but in -next), but I've been running
into some quirks.

Part of the issue is the adv7533 bridge already has a endpoint port
entry for the dsi output. So when I add the codec_endpoint port, it
wants the two port entries to be enumerated, so I get something like:

port@0 {
    adv7533_in: endpoint {
        remote-endpoint = <&dsi_out0>;
    };
};
port@1 {
    codec_endpoint: endpoint {
        remote-endpoint = <&i2s0_cpu_endpoint>;
    };
};

But this causes it to try to link to hdmi-hifi.1 (which doesn't exist)
instead of hdmi-hifi.0.

If I instead swap the entries, so the codec_endpoint is first on port
0, then the the audio link is properly setup, but the dsi
initialization falls over.

Any suggestions here?

thanks
-john

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[Kuninori Morimoto]

Hi John

> On Mon, Jun 12, 2017 at 3:10 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Jun 12, 2017 at 01:52:46PM -0700, John Stultz wrote:
> >
> >> +     sound {
> >> +             compatible = "simple-audio-card";
> >> +             simple-audio-card,name = "hikey-hdmi";
> >
> > Now the graph card has been merged it's probably a good idea to be using
> > that, it's generally a better idea for pretty much all use cases.
> 
> I've taken a shot today trying to convert over to the audio-graph-card
> method (which isn't yet upstream, but in -next), but I've been running
> into some quirks.
> 
> Part of the issue is the adv7533 bridge already has a endpoint port
> entry for the dsi output. So when I add the codec_endpoint port, it
> wants the two port entries to be enumerated, so I get something like:
> 
> port@0 {
>     adv7533_in: endpoint {
>         remote-endpoint = <&dsi_out0>;
>     };
> };
> port@1 {
>     codec_endpoint: endpoint {
>         remote-endpoint = <&i2s0_cpu_endpoint>;
>     };
> };
> 
> But this causes it to try to link to hdmi-hifi.1 (which doesn't exist)
> instead of hdmi-hifi.0.
> 
> If I instead swap the entries, so the codec_endpoint is first on port
> 0, then the the audio link is properly setup, but the dsi
> initialization falls over.

I think you want to exchange port@1 as ID=0 for ALSA SoC ?
If so, we already has .of_xlate_dai_id callback for this purpose.

Does below help your issue ?

	commit 73b17f1a65c881fcf97109d77056006da2d40152
	commit a180e8b988437b3e84a1b501ac4d073467602ca6

Samplle codes are

	linux/sound/soc/codecs/hdmi-codec.c :: hdmi_of_xlate_dai_id
	https://patchwork.kernel.org/patch/9732285/ (I posted, but not yet applied)

Best regards
---
Kuninori Morimoto

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[John Stultz]

On Mon, Jun 12, 2017 at 7:09 PM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
> Hi John
>
>> On Mon, Jun 12, 2017 at 3:10 PM, Mark Brown <broonie@kernel.org> wrote:
>> > On Mon, Jun 12, 2017 at 01:52:46PM -0700, John Stultz wrote:
>> >
>> >> +     sound {
>> >> +             compatible = "simple-audio-card";
>> >> +             simple-audio-card,name = "hikey-hdmi";
>> >
>> > Now the graph card has been merged it's probably a good idea to be using
>> > that, it's generally a better idea for pretty much all use cases.
>>
>> I've taken a shot today trying to convert over to the audio-graph-card
>> method (which isn't yet upstream, but in -next), but I've been running
>> into some quirks.
>>
>> Part of the issue is the adv7533 bridge already has a endpoint port
>> entry for the dsi output. So when I add the codec_endpoint port, it
>> wants the two port entries to be enumerated, so I get something like:
>>
>> port@0 {
>>     adv7533_in: endpoint {
>>         remote-endpoint = <&dsi_out0>;
>>     };
>> };
>> port@1 {
>>     codec_endpoint: endpoint {
>>         remote-endpoint = <&i2s0_cpu_endpoint>;
>>     };
>> };
>>
>> But this causes it to try to link to hdmi-hifi.1 (which doesn't exist)
>> instead of hdmi-hifi.0.
>>
>> If I instead swap the entries, so the codec_endpoint is first on port
>> 0, then the the audio link is properly setup, but the dsi
>> initialization falls over.
>
> I think you want to exchange port@1 as ID=0 for ALSA SoC ?
> If so, we already has .of_xlate_dai_id callback for this purpose.
>
> Does below help your issue ?
>
>         commit 73b17f1a65c881fcf97109d77056006da2d40152
>         commit a180e8b988437b3e84a1b501ac4d073467602ca6
>
> Samplle codes are
>
>         linux/sound/soc/codecs/hdmi-codec.c :: hdmi_of_xlate_dai_id
>         https://patchwork.kernel.org/patch/9732285/ (I posted, but not yet applied)

:/ So with the above approach I did manage to get it working. And I'm
not opposed to migrating to this, but it doesn't really feel finished
at the moment (the magic hard coding of port 2 ==> 0 feels a bit
hackish, but I'm not sure how else one can use the same port style dts
description across the two namespaces of display and audio).  While
I'm sure for many cases the graph approach is much cleaner, I'm not
sure for this one how its improving over the much simpler
simple-audio-card binding.

So yea, I'm ok with migrating to this, but I'm also not super
enthusiastic about delaying (I'm guessing likely to 4.14 since we have
new unqueued code dependencies to get in) the enabling of this
functionality in order to moving to something that at least for this
case "seems" a bit more hackish.

The simple-audio-card bindings are still valid, so maybe could we
merge the proposed dts change I've submitted and then look at
migrating over to the audio-card-graph once the dependencies are all
in place?

thanks
-john

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

On Tue, Jun 13, 2017 at 12:40:28PM -0700, John Stultz wrote:

> description across the two namespaces of display and audio).  While
> I'm sure for many cases the graph approach is much cleaner, I'm not
> sure for this one how its improving over the much simpler
> simple-audio-card binding.

It meana you're not limited to single point to point digital linka which
scales better (and is much the same reason why the video stuff uses
graphs too). 

> The simple-audio-card bindings are still valid, so maybe could we
> merge the proposed dts change I've submitted and then look at
> migrating over to the audio-card-graph once the dependencies are all
> in place?

Which dependencies sorry?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[John Stultz]

On Tue, Jun 13, 2017 at 12:54 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 13, 2017 at 12:40:28PM -0700, John Stultz wrote:
>
>> description across the two namespaces of display and audio).  While
>> I'm sure for many cases the graph approach is much cleaner, I'm not
>> sure for this one how its improving over the much simpler
>> simple-audio-card binding.
>
> It meana you're not limited to single point to point digital linka which
> scales better (and is much the same reason why the video stuff uses
> graphs too).
>
>> The simple-audio-card bindings are still valid, so maybe could we
>> merge the proposed dts change I've submitted and then look at
>> migrating over to the audio-card-graph once the dependencies are all
>> in place?
>
> Which dependencies sorry?

So the adv7511 driver needs to add a .get_dai_id() hook to renumber
the port to dai id.

thanks
-john

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

On Tue, Jun 13, 2017 at 12:57:44PM -0700, John Stultz wrote:
> On Tue, Jun 13, 2017 at 12:54 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Tue, Jun 13, 2017 at 12:40:28PM -0700, John Stultz wrote:

> >> The simple-audio-card bindings are still valid, so maybe could we
> >> merge the proposed dts change I've submitted and then look at
> >> migrating over to the audio-card-graph once the dependencies are all
> >> in place?

> > Which dependencies sorry?

> So the adv7511 driver needs to add a .get_dai_id() hook to renumber
> the port to dai id.

Oh, that should be pretty trivial though?  Especially given that it
seems like it's been difficult to get DT changes to this platform merged
I'm a bit nervous about the followup here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[John Stultz]

On Tue, Jun 13, 2017 at 1:16 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 13, 2017 at 12:57:44PM -0700, John Stultz wrote:
>> On Tue, Jun 13, 2017 at 12:54 PM, Mark Brown <broonie@kernel.org> wrote:
>> > On Tue, Jun 13, 2017 at 12:40:28PM -0700, John Stultz wrote:
>
>> >> The simple-audio-card bindings are still valid, so maybe could we
>> >> merge the proposed dts change I've submitted and then look at
>> >> migrating over to the audio-card-graph once the dependencies are all
>> >> in place?
>
>> > Which dependencies sorry?
>
>> So the adv7511 driver needs to add a .get_dai_id() hook to renumber
>> the port to dai id.
>
> Oh, that should be pretty trivial though?  Especially given that it
> seems like it's been difficult to get DT changes to this platform merged
> I'm a bit nervous about the followup here.

Its a trivial bit of code, but its in the drm directory and I suspect
at this stage I'm not going to be able to get maintainer attention for
it in time for 4.13 (plus since it depends on things in the ASoC tree,
so it needs cross-maintainer acks and probably to go through the ASoC
tree).

Plus the new dts changes with the graph need review (where as the
existing ones have been sent out 5 or so times w/o comment) before
they are merged.  Its just a ton of extra friction that doesn't gain
much and just keeps functionality from working upstream.

As for your concern about follow up, I've got the patches lined up here:
  https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=31e8b980d93087cf30cd708fb0b2cc48e906d003

  https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=f24a0ccdeeb46ce3eb8864fd43f7a34c673ff9da

And will revise and submit the adv7511 one for initial review today.

So again, I'm happy to migrate over. But I just think, as the graph
stuff doesn't actually benefit us much in this specific case (there is
only one supported audio out path right now), its not worth blocking
getting the functionality working upstream for another 3-6 months.

thanks
-john

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]

On Tue, Jun 13, 2017 at 01:42:15PM -0700, John Stultz wrote:

> Its a trivial bit of code, but its in the drm directory and I suspect
> at this stage I'm not going to be able to get maintainer attention for
> it in time for 4.13 (plus since it depends on things in the ASoC tree,
> so it needs cross-maintainer acks and probably to go through the ASoC
> tree).

Can I apply it directly?  Given that it's all ASoC code it might be the
most expedient way to handle this.  It doesn't look like there's any DRM
changes to meaningfully conflict with.

> Plus the new dts changes with the graph need review (where as the
> existing ones have been sent out 5 or so times w/o comment) before
> they are merged.  Its just a ton of extra friction that doesn't gain
> much and just keeps functionality from working upstream.

What's happened here is that because you've been sending it for so long
the code around it changed so there's new review comments.

> As for your concern about follow up, I've got the patches lined up here:
>   https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=31e8b980d93087cf30cd708fb0b2cc48e906d003

>   https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=f24a0ccdeeb46ce3eb8864fd43f7a34c673ff9da

> And will revise and submit the adv7511 one for initial review today.

> So again, I'm happy to migrate over. But I just think, as the graph
> stuff doesn't actually benefit us much in this specific case (there is
> only one supported audio out path right now), its not worth blocking
> getting the functionality working upstream for another 3-6 months.

If it might take two kernel releases to get such a trivial change in
then that's not giving me a warm fuzzy feeling...   there's something
not working very well with the HiKey support overall with frequent
process problems (only today I had someone resubmitting already applied
code).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[John Stultz]

On Tue, Jun 13, 2017 at 2:46 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 13, 2017 at 01:42:15PM -0700, John Stultz wrote:
>
>> Its a trivial bit of code, but its in the drm directory and I suspect
>> at this stage I'm not going to be able to get maintainer attention for
>> it in time for 4.13 (plus since it depends on things in the ASoC tree,
>> so it needs cross-maintainer acks and probably to go through the ASoC
>> tree).
>
> Can I apply it directly?  Given that it's all ASoC code it might be the
> most expedient way to handle this.  It doesn't look like there's any DRM
> changes to meaningfully conflict with.

I just sent it out, and I'll leave it to your discretion as to if it
can be picked up and go through your tree or if we need to wait for
acks or the next merge window.

>> Plus the new dts changes with the graph need review (where as the
>> existing ones have been sent out 5 or so times w/o comment) before
>> they are merged.  Its just a ton of extra friction that doesn't gain
>> much and just keeps functionality from working upstream.
>
> What's happened here is that because you've been sending it for so long
> the code around it changed so there's new review comments.
>
>> As for your concern about follow up, I've got the patches lined up here:
>>   https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=31e8b980d93087cf30cd708fb0b2cc48e906d003
>
>>   https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=f24a0ccdeeb46ce3eb8864fd43f7a34c673ff9da
>
>> And will revise and submit the adv7511 one for initial review today.
>
>> So again, I'm happy to migrate over. But I just think, as the graph
>> stuff doesn't actually benefit us much in this specific case (there is
>> only one supported audio out path right now), its not worth blocking
>> getting the functionality working upstream for another 3-6 months.
>
> If it might take two kernel releases to get such a trivial change in
> then that's not giving me a warm fuzzy feeling...   there's something
> not working very well with the HiKey support overall with frequent
> process problems (only today I had someone resubmitting already applied
> code).

Well, I can't speak to other things, but we do seem to have trouble
getting dts changes picked up via Wei, and we probably need to add
some extra maintainers to make sure things don't drop through. I think
Ulf was considering this?

thanks
-john

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]

On Tue, Jun 13, 2017 at 03:08:09PM -0700, John Stultz wrote:

> Well, I can't speak to other things, but we do seem to have trouble
> getting dts changes picked up via Wei, and we probably need to add
> some extra maintainers to make sure things don't drop through. I think
> Ulf was considering this?

How about you?  :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RESEND x3][PATCH v4] arm64: dts: hi6220: Add k3-dma and i2s/hdmi audio support

[John Stultz]

On Wed, Jun 14, 2017 at 7:42 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 13, 2017 at 03:08:09PM -0700, John Stultz wrote:
>
>> Well, I can't speak to other things, but we do seem to have trouble
>> getting dts changes picked up via Wei, and we probably need to add
>> some extra maintainers to make sure things don't drop through. I think
>> Ulf was considering this?
>
> How about you?  :)

I'm open to it. Though its a bit self-serving at the moment. :)

I also worry my expertise around the hardware isn't high enough to be
able to properly review submissions, but I am happy to look over,
collect patches, and test on the hardware I have.

thanks
-john