Re: [PATCH v5 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Hi Kieran, Kieran Bingham, Tue, Aug 25, 2020 16:57:04 +0200: > On 18/06/2020 17:32, Kieran Bingham wrote: > > On 02/04/2020 19:35, Alex Riesen 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. > >> ... > >> --- > >> .../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +- > >> arch/arm64/boot/dts/renesas/r8a77961.dtsi | 1 + > >> .../boot/dts/renesas/salvator-common.dtsi | 47 +-- > > Once again I'm back trying to test this series, and one issue I've had > is that the board I have (r8a77951-salvator-xs.dts) isn't included in > this DT update. > > For v6, Should we include the relevant changes to all the following? Ok. I shall add them as a separate patch though, as I have no way to verify those boards (and some verification seem to be in order...) > arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts > arch/arm64/boot/dts/renesas/r8a77951-salvator-x.dts > arch/arm64/boot/dts/renesas/r8a77960-salvator-x.dts > arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts > arch/arm64/boot/dts/renesas/salvator-x.dtsi > > And perhaps handle the salvator-xs in a second (yet very similar) patch? > > arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts > arch/arm64/boot/dts/renesas/r8a77960-salvator-xs.dts > arch/arm64/boot/dts/renesas/r8a77961-salvator-xs.dts > arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts > arch/arm64/boot/dts/renesas/salvator-xs.dtsi > > I think I've added the relevant entries to my dtb, but I haven't > successfully captured audio yet. > > I can see the device being listed through arecord: > > kbingham@salvator-xs:~$ arecord -l > List of CAPTURE Hardware Devices > card 0: rcarsound [rcar-sound], device 0: rsnd-dai.0-ak4613-hifi > ak4613-hifi-0 [] > Subdevices: 0/1 > Subdevice #0: subdevice #0 > card 0: rcarsound [rcar-sound], device 3: rsnd-dai.3-adv748x-i2s > adv748x.4-0070-3 [] > Subdevices: 1/1 > Subdevice #0: subdevice #0 > > But as yet, everything I try to record fails or is empty silence. > > Debugging ... Does it fail somewhere in the ASoC infrastructure? If so, how'd you find out where exactly and what fails? Asking, because when I was writing this code I ended up adding quite a bit of tracing into the SoC core to figure that out, and I just hope there is a better way to get at the diagnostics. > >> 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 @@ { > >> _card { > >>dais = <_port0 /* ak4613 */ > >>_port1 /* HDMI0 */ > >> - _port2>; /* HDMI1 */ > >> + _port2 /* HDMI1 */ > >> + _port3>; /* adv7482 hdmi-in */ > > > > Ah - that was confusing at first... but HDMI0 and HDMI1 are *outputs*, > > where of course the adv7482 is an input ;-) I shall add an "output" to HDMI0 and HDMI1. > > Otherwise, I can't spot anything else yet so: > > > > Reviewed-by: Kieran Bingham Thanks! > > But I fear there may have been some churn around here, so it would be > > good to see a rebase too. Of course, I shall rebase on top of linux-media/master. Should I wait with submission until you get data out of your boards? Regards, Alex
Re: [PATCH v5 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Hi Alex, On 18/06/2020 17:32, Kieran Bingham wrote: > Hi Alex, > > On 02/04/2020 19:35, Alex Riesen 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 >> >> -- >> >> v5: Add dummy ssi4 node to the rcar sound card in r8a77961, as the >> devices (Salvator-X 2nd version with R-Car M3 W+) also reference >> salvator-common.dtsi. >> Suggested-by: Geert Uytterhoeven >> >> 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 _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 >> >> v2: The driver provides only MCLK clock, not the SCLK and LRCLK, >> which are part of the I2S protocol. >> >> Suggested-by: Laurent Pinchart >> --- >> .../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +- >> arch/arm64/boot/dts/renesas/r8a77961.dtsi | 1 + >> .../boot/dts/renesas/salvator-common.dtsi | 47 +-- Once again I'm back trying to test this series, and one issue I've had is that the board I have (r8a77951-salvator-xs.dts) isn't included in this DT update. For v6, Should we include the relevant changes to all the following? arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts arch/arm64/boot/dts/renesas/r8a77951-salvator-x.dts arch/arm64/boot/dts/renesas/r8a77960-salvator-x.dts arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts arch/arm64/boot/dts/renesas/salvator-x.dtsi And perhaps handle the salvator-xs in a second (yet very similar) patch? arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts arch/arm64/boot/dts/renesas/r8a77960-salvator-xs.dts arch/arm64/boot/dts/renesas/r8a77961-salvator-xs.dts arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts arch/arm64/boot/dts/renesas/salvator-xs.dtsi I think I've added the relevant entries to my dtb, but I haven't successfully captured audio yet. I can see the device being listed through arecord: kbingham@salvator-xs:~$ arecord -l List of CAPTURE Hardware Devices card 0: rcarsound [rcar-sound], device 0: rsnd-dai.0-ak4613-hifi ak4613-hifi-0 [] Subdevices: 0/1 Subdevice #0: subdevice #0 card 0: rcarsound [rcar-sound], device 3: rsnd-dai.3-adv748x-i2s adv748x.4-0070-3 [] Subdevices: 1/1 Subdevice #0: subdevice #0 But as yet, everything I try to record fails or is empty silence. Debugging ... -- Regards Kieran >> 3 files changed, 45 insertions(+), 6 deletions(-) >> >> 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 @@ { >> _card { >> dais = <_port0 /* ak4613 */ >> _port1 /* HDMI0 */ >> -_port2>; /* HDMI1 */ >> +_port2 /* HDMI1 */ >> +_port3>; /* adv7482 hdmi-in */ > > Ah - that was confusing at first... but HDMI0 and HDMI1 are *outputs*, > where of course the adv7482 is an input ;-) > > > Otherwise, I can't spot anything else yet so: > > Reviewed-by: Kieran Bingham > > But I fear there may have been some churn around here, so it would be > good to see a rebase too. > > -- > Kieran > > > >> }; >> >> _phy2 { >> diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi >> b/arch/arm64/boot/dts/renesas/r8a77961.dtsi >> index be3824bda632..b79907beaf31 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi >> @@ -861,6 +861,7 @@ rcar_sound,src { >>
Re: [PATCH v5 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Kieran Bingham, Thu, Jun 18, 2020 18:32:55 +0200: > On 02/04/2020 19:35, Alex Riesen wrote: > > --- a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts > > +++ b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts > > @@ -146,7 +146,8 @@ { > > _card { > > dais = <_port0 /* ak4613 */ > > _port1 /* HDMI0 */ > > - _port2>; /* HDMI1 */ > > + _port2 /* HDMI1 */ > > + _port3>; /* adv7482 hdmi-in */ > > Ah - that was confusing at first... but HDMI0 and HDMI1 are *outputs*, > where of course the adv7482 is an input ;-) And ak4613 is both... Why? Are the "dais" of a sound_card more commonly outputs than inputs? > Otherwise, I can't spot anything else yet so: > > Reviewed-by: Kieran Bingham > > But I fear there may have been some churn around here, so it would be > good to see a rebase too. Just rebased the series on top of linux-media/master and the only conflict was in adding ssi4 node to rcar_sound,ssi in r8a77961.dtsi, as there was an addition of ssi2 in the same line. Thanks, Alex
Re: [PATCH v5 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Hi Alex, On 02/04/2020 19:35, Alex Riesen 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 > > -- > > v5: Add dummy ssi4 node to the rcar sound card in r8a77961, as the > devices (Salvator-X 2nd version with R-Car M3 W+) also reference > salvator-common.dtsi. > Suggested-by: Geert Uytterhoeven > > 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 _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 > > v2: The driver provides only MCLK clock, not the SCLK and LRCLK, > which are part of the I2S protocol. > > Suggested-by: Laurent Pinchart > --- > .../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +- > arch/arm64/boot/dts/renesas/r8a77961.dtsi | 1 + > .../boot/dts/renesas/salvator-common.dtsi | 47 +-- > 3 files changed, 45 insertions(+), 6 deletions(-) > > 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 @@ { > _card { > dais = <_port0 /* ak4613 */ > _port1 /* HDMI0 */ > - _port2>; /* HDMI1 */ > + _port2 /* HDMI1 */ > + _port3>; /* adv7482 hdmi-in */ Ah - that was confusing at first... but HDMI0 and HDMI1 are *outputs*, where of course the adv7482 is an input ;-) Otherwise, I can't spot anything else yet so: Reviewed-by: Kieran Bingham But I fear there may have been some churn around here, so it would be good to see a rebase too. -- Kieran > }; > > _phy2 { > diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi > b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > index be3824bda632..b79907beaf31 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > @@ -861,6 +861,7 @@ rcar_sound,src { > rcar_sound,ssi { > ssi0: ssi-0 { }; > ssi1: ssi-1 { }; > + ssi4: ssi-4 { }; > }; > }; > > 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 = <>; > interrupt-names = "intrq1", "intrq2"; > @@ -510,6 +511,15 @@ adv7482_txb: endpoint { > remote-endpoint = <_in>; > }; > }; > + > + port@c { > + reg = <12>; > + > + adv7482_i2s: endpoint { > + remote-endpoint = <_endpoint3>; > + system-clock-direction-out; > + }; > + }; > }; > > csa_vdd: adc@7c