| Message ID | ad15f80df51c95a7c24498bb0bd3a46f55fbb62e.1585218857.git.alexander.riesen@cetitec.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | media: adv748x: add support for HDMI audio | expand |
Hi Alex, On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > As all known variants of the Salvator board have the HDMI decoder > chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482 > endpoint and the connection definitions are placed in the common board > file. > > For the same reason, the CLK_C clock line and I2C configuration (similar > to the ak4613, on the same interface) are added into the common file. > > Signed-off-by: Alexander Riesen <alexander.riesen@cetitec.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi > @@ -460,7 +460,7 @@ pca9654: gpio@20 { > #gpio-cells = <2>; > }; > > - video-receiver@70 { > + adv7482_hdmi_in: video-receiver@70 { > compatible = "adi,adv7482"; > reg = <0x70 0x71 0x72 0x73 0x74 0x75 > 0x60 0x61 0x62 0x63 0x64 0x65>; > @@ -469,6 +469,7 @@ video-receiver@70 { > > #address-cells = <1>; > #size-cells = <0>; > + #clock-cells = <0>; /* the MCLK for I2S output */ > > interrupt-parent = <&gpio6>; > interrupt-names = "intrq1", "intrq2"; > @@ -510,6 +511,15 @@ adv7482_txb: endpoint { > remote-endpoint = <&csi20_in>; > }; > }; > + > + port@c { > + reg = <12>; > + > + adv7482_i2s: endpoint { > + remote-endpoint = <&rsnd_endpoint3>; > + system-clock-direction-out; > + }; > + }; As the adv748x driver just ignores "invalid" endpoints... > @@ -733,8 +744,8 @@ &rcar_sound { > pinctrl-0 = <&sound_pins &sound_clk_pins>; > pinctrl-names = "default"; > > - /* Single DAI */ > - #sound-dai-cells = <0>; > + /* multi DAI */ > + #sound-dai-cells = <1>; > > /* audio_clkout0/1/2/3 */ > #clock-cells = <1>; > @@ -758,8 +769,19 @@ &rcar_sound { > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>, > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>, > <&audio_clk_a>, <&cs2000>, > - <&audio_clk_c>, > + <&adv7482_hdmi_in>, > <&cpg CPG_CORE CPG_AUDIO_CLK_I>; ... and the rsnd driver ignores nonexistent-clocks, the DT change has no hard dependency on the driver change, and won't introduce regressions when included, right? > @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint { > capture = <&ssi1 &src1 &dvc1>; > }; > }; > + rsnd_port3: port@3 { > + reg = <3>; > + rsnd_endpoint3: endpoint { > + remote-endpoint = <&adv7482_i2s>; > + > + dai-tdm-slot-num = <8>; > + dai-tdm-slot-width = <32>; > + dai-format = "left_j"; > + mclk-fs = <256>; > + bitclock-master = <&adv7482_i2s>; > + frame-master = <&adv7482_i2s>; > + > + capture = <&ssi4>; > + }; > + }; > }; > }; However, as salvator-common.dtsi is shared by all Salvator-X(S) variants, you'll have to add a dummy ssi4 node to r8a77961.dtsi first. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, I'm sorry for late reply. Some unrelated happenings here in south Germany. Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200: > On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi > > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi > > @@ -510,6 +511,15 @@ adv7482_txb: endpoint { > > remote-endpoint = <&csi20_in>; > > }; > > }; > > + > > + port@c { > > + reg = <12>; > > + > > + adv7482_i2s: endpoint { > > + remote-endpoint = <&rsnd_endpoint3>; > > + system-clock-direction-out; > > + }; > > + }; > > As the adv748x driver just ignores "invalid" endpoints... > > > @@ -758,8 +769,19 @@ &rcar_sound { > > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>, > > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>, > > <&audio_clk_a>, <&cs2000>, > > - <&audio_clk_c>, > > + <&adv7482_hdmi_in>, > > <&cpg CPG_CORE CPG_AUDIO_CLK_I>; > > ... and the rsnd driver ignores nonexistent-clocks, the DT change has no > hard dependency on the driver change, and won't introduce regressions > when included, right? Well, it maybe won't, but isn't it a little ... implicit? And I'm no haste to include the changes, if you mean I can (or should) submit the device tree patch separately. > > @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint { > > capture = <&ssi1 &src1 &dvc1>; > > }; > > }; > > + rsnd_port3: port@3 { > > + reg = <3>; > > + rsnd_endpoint3: endpoint { > > + remote-endpoint = <&adv7482_i2s>; > > + > > + dai-tdm-slot-num = <8>; > > + dai-tdm-slot-width = <32>; > > + dai-format = "left_j"; > > + mclk-fs = <256>; > > + bitclock-master = <&adv7482_i2s>; > > + frame-master = <&adv7482_i2s>; > > + > > + capture = <&ssi4>; > > + }; > > + }; > > }; > > }; > > However, as salvator-common.dtsi is shared by all Salvator-X(S) variants, > you'll have to add a dummy ssi4 node to r8a77961.dtsi first. I see. There are even two dummy SSI nodes already. I would prefer to submit the change together with other Salvator device tree changes. Is that alright? Regards, Alex
Hi Alex, On Thu, Apr 2, 2020 at 5:03 PM Alex Riesen <alexander.riesen@cetitec.com> wrote: > Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200: > > On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi > > > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi > > > @@ -510,6 +511,15 @@ adv7482_txb: endpoint { > > > remote-endpoint = <&csi20_in>; > > > }; > > > }; > > > + > > > + port@c { > > > + reg = <12>; > > > + > > > + adv7482_i2s: endpoint { > > > + remote-endpoint = <&rsnd_endpoint3>; > > > + system-clock-direction-out; > > > + }; > > > + }; > > > > As the adv748x driver just ignores "invalid" endpoints... > > > > > @@ -758,8 +769,19 @@ &rcar_sound { > > > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>, > > > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>, > > > <&audio_clk_a>, <&cs2000>, > > > - <&audio_clk_c>, > > > + <&adv7482_hdmi_in>, > > > <&cpg CPG_CORE CPG_AUDIO_CLK_I>; > > > > ... and the rsnd driver ignores nonexistent-clocks, the DT change has no > > hard dependency on the driver change, and won't introduce regressions > > when included, right? > > Well, it maybe won't, but isn't it a little ... implicit? > And I'm no haste to include the changes, if you mean I can (or should) submit > the device tree patch separately. OK, fine for me to postpone (that'll be for v5.9, I guess?). > > > @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint { > > > capture = <&ssi1 &src1 &dvc1>; > > > }; > > > }; > > > + rsnd_port3: port@3 { > > > + reg = <3>; > > > + rsnd_endpoint3: endpoint { > > > + remote-endpoint = <&adv7482_i2s>; > > > + > > > + dai-tdm-slot-num = <8>; > > > + dai-tdm-slot-width = <32>; > > > + dai-format = "left_j"; > > > + mclk-fs = <256>; > > > + bitclock-master = <&adv7482_i2s>; > > > + frame-master = <&adv7482_i2s>; > > > + > > > + capture = <&ssi4>; > > > + }; > > > + }; > > > }; > > > }; > > > > However, as salvator-common.dtsi is shared by all Salvator-X(S) variants, > > you'll have to add a dummy ssi4 node to r8a77961.dtsi first. > > I see. There are even two dummy SSI nodes already. I would prefer to submit > the change together with other Salvator device tree changes. Is that alright? Fine for me. Gr{oetje,eeting}s, Geert
Geert Uytterhoeven, Thu, Apr 02, 2020 17:26:15 +0200: > On Thu, Apr 2, 2020 at 5:03 PM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200: > > > On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > > > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi > > > > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi > > > > @@ -510,6 +511,15 @@ adv7482_txb: endpoint { > > > > remote-endpoint = <&csi20_in>; > > > > }; > > > > }; > > > > + > > > > + port@c { > > > > + reg = <12>; > > > > + > > > > + adv7482_i2s: endpoint { > > > > + remote-endpoint = <&rsnd_endpoint3>; > > > > + system-clock-direction-out; > > > > + }; > > > > + }; > > > > > > As the adv748x driver just ignores "invalid" endpoints... > > > > > > > @@ -758,8 +769,19 @@ &rcar_sound { > > > > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>, > > > > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>, > > > > <&audio_clk_a>, <&cs2000>, > > > > - <&audio_clk_c>, > > > > + <&adv7482_hdmi_in>, > > > > <&cpg CPG_CORE CPG_AUDIO_CLK_I>; > > > > > > ... and the rsnd driver ignores nonexistent-clocks, the DT change has no > > > hard dependency on the driver change, and won't introduce regressions > > > when included, right? > > > > Well, it maybe won't, but isn't it a little ... implicit? > > And I'm no haste to include the changes, if you mean I can (or should) submit > > the device tree patch separately. > > OK, fine for me to postpone (that'll be for v5.9, I guess?). > Looks scary :) But yes, fine with me too. v5 with ssi4 dummy in a moment. Regards, Alex
diff --git a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts index 2438825c9b22..e16c146808b6 100644 --- a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts +++ b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts @@ -146,7 +146,8 @@ &sata { &sound_card { dais = <&rsnd_port0 /* ak4613 */ &rsnd_port1 /* HDMI0 */ - &rsnd_port2>; /* HDMI1 */ + &rsnd_port2 /* HDMI1 */ + &rsnd_port3>; /* adv7482 hdmi-in */ }; &usb2_phy2 { diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi index 98bbcafc8c0d..ead7f8d7a929 100644 --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi @@ -460,7 +460,7 @@ pca9654: gpio@20 { #gpio-cells = <2>; }; - video-receiver@70 { + adv7482_hdmi_in: video-receiver@70 { compatible = "adi,adv7482"; reg = <0x70 0x71 0x72 0x73 0x74 0x75 0x60 0x61 0x62 0x63 0x64 0x65>; @@ -469,6 +469,7 @@ video-receiver@70 { #address-cells = <1>; #size-cells = <0>; + #clock-cells = <0>; /* the MCLK for I2S output */ interrupt-parent = <&gpio6>; interrupt-names = "intrq1", "intrq2"; @@ -510,6 +511,15 @@ adv7482_txb: endpoint { remote-endpoint = <&csi20_in>; }; }; + + port@c { + reg = <12>; + + adv7482_i2s: endpoint { + remote-endpoint = <&rsnd_endpoint3>; + system-clock-direction-out; + }; + }; }; csa_vdd: adc@7c { @@ -684,7 +694,8 @@ sdhi3_pins_uhs: sd3_uhs { }; sound_pins: sound { - groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a"; + groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a", + "ssi4_data", "ssi4_ctrl"; function = "ssi"; }; @@ -733,8 +744,8 @@ &rcar_sound { pinctrl-0 = <&sound_pins &sound_clk_pins>; pinctrl-names = "default"; - /* Single DAI */ - #sound-dai-cells = <0>; + /* multi DAI */ + #sound-dai-cells = <1>; /* audio_clkout0/1/2/3 */ #clock-cells = <1>; @@ -758,8 +769,19 @@ &rcar_sound { <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>, <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>, <&audio_clk_a>, <&cs2000>, - <&audio_clk_c>, + <&adv7482_hdmi_in>, <&cpg CPG_CORE CPG_AUDIO_CLK_I>; + clock-names = "ssi-all", + "ssi.9", "ssi.8", "ssi.7", "ssi.6", + "ssi.5", "ssi.4", "ssi.3", "ssi.2", + "ssi.1", "ssi.0", + "src.9", "src.8", "src.7", "src.6", + "src.5", "src.4", "src.3", "src.2", + "src.1", "src.0", + "mix.1", "mix.0", + "ctu.1", "ctu.0", + "dvc.0", "dvc.1", + "clk_a", "clk_b", "clk_c", "clk_i"; ports { #address-cells = <1>; @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint { capture = <&ssi1 &src1 &dvc1>; }; }; + rsnd_port3: port@3 { + reg = <3>; + rsnd_endpoint3: endpoint { + remote-endpoint = <&adv7482_i2s>; + + dai-tdm-slot-num = <8>; + dai-tdm-slot-width = <32>; + dai-format = "left_j"; + mclk-fs = <256>; + bitclock-master = <&adv7482_i2s>; + frame-master = <&adv7482_i2s>; + + capture = <&ssi4>; + }; + }; }; };
As all known variants of the Salvator board have the HDMI decoder chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482 endpoint and the connection definitions are placed in the common board file. For the same reason, the CLK_C clock line and I2C configuration (similar to the ak4613, on the same interface) are added into the common file. Signed-off-by: Alexander Riesen <alexander.riesen@cetitec.com> -- v2: Also add ssi4_ctrl pin group in the sound pins. The pins are responsible for SCK4 (sample clock) WS4 and (word boundary input), and are required for SSI audio input over I2S. The adv748x shall provide its own implementation of the output clock (MCLK), connected to the audio_clk_c line of the R-Car SoC. If the frequency of the ADV748x MCLK were fixed, the clock implementation were not necessary, but it does not seem so: the MCLK depends on the value in a speed multiplier register and the input sample rate (48kHz). Remove audio clock C from the clocks of adv7482. The clocks property of the video-receiver node lists the input clocks of the device, which is quite the opposite from the original intention: the adv7482 on Salvator X boards is a provide of the MCLK clock for I2S audio output. Remove old definition of &sound_card.dais and reduce size of changes in the Salvator-X specific device tree source. Declare video-receiver a clock producer, as the adv748x driver implements the master clock used I2S audio output. Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> v2: The driver provides only MCLK clock, not the SCLK and LRCLK, which are part of the I2S protocol. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- .../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +- .../boot/dts/renesas/salvator-common.dtsi | 47 +++++++++++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-)