| Message ID | 20190202043444.9308-1-katsuhiro@katsuster.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: rockchip: add HDMI sound node for rk3328-rock64 | expand |
Am Samstag, 2. Februar 2019, 05:34:44 CET schrieb Katsuhiro Suzuki: > This patch adds HDMI sound (I2S0) node and remove dma properties > from UART2 node for rock64. > > The DMAC of rk3328 can use 8 channels at same time. Currently, total > 7 channels are used as follows: > - I2S1 2ch > - UART2 2ch > - SPDIF 1ch > - SPI0 2ch > > HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel. > > UART2 can work without DMA resources, so this patch removes dma > allocation for UART2 and reuses it to I2S0. I don't follow that description. How can i2s0 re-use the uart2 dma channels? Looking at the dma table in the TRM, uart2 has channels 6+7 while i2s0 uses channels 11+12. They should just run concurrently? > > Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net> > --- > .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 24 ++++++++++++++++++- > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + > 2 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > index 2157a528276b..e21645aa3fa5 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > @@ -68,7 +68,8 @@ > sound { > compatible = "audio-graph-card"; > label = "rockchip,rk3328"; > - dais = <&i2s1_p0 > + dais = <&i2s0_p0 > + &i2s1_p0 > &spdif_p0>; > }; > > @@ -141,6 +142,12 @@ > > &hdmi { > status = "okay"; > + > + port@0 { > + hdmi_p0_0: endpoint { > + remote-endpoint = <&i2s0_p0_0>; > + }; > + }; > }; > > &hdmiphy { > @@ -256,6 +263,18 @@ > }; > }; > > +&i2s0 { > + status = "okay"; > + > + i2s0_p0: port { > + i2s0_p0_0: endpoint { > + dai-format = "i2s"; > + mclk-fs = <256>; > + remote-endpoint = <&hdmi_p0_0>; > + }; > + }; > +}; > + > &i2s1 { > status = "okay"; > > @@ -343,6 +362,9 @@ > > &uart2 { > status = "okay"; > + > + /delete-property/ dmas; > + /delete-property/ dma-names; > }; > > &u2phy { > diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > index 84f14b132e8f..374b5da93a35 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > @@ -665,6 +665,7 @@ > pinctrl-names = "default"; > pinctrl-0 = <&hdmi_cec &hdmii2c_xfer &hdmi_hpd>; > rockchip,grf = <&grf>; > + #sound-dai-cells = <0>; please make that a separate patch Heiko
Hello Heiko, On 2019/02/03 18:06, Heiko Stuebner wrote: > Am Samstag, 2. Februar 2019, 05:34:44 CET schrieb Katsuhiro Suzuki: >> This patch adds HDMI sound (I2S0) node and remove dma properties >> from UART2 node for rock64. >> >> The DMAC of rk3328 can use 8 channels at same time. Currently, total >> 7 channels are used as follows: >> - I2S1 2ch >> - UART2 2ch >> - SPDIF 1ch >> - SPI0 2ch >> >> HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel. >> >> UART2 can work without DMA resources, so this patch removes dma >> allocation for UART2 and reuses it to I2S0. > > I don't follow that description. How can i2s0 re-use the uart2 dma channels? > Looking at the dma table in the TRM, uart2 has channels 6+7 while i2s0 > uses channels 11+12. They should just run concurrently? > Sorry for confusing... 6 or 7 is as ID number of slave DMA channel. TRM calls it 'Req number'. Req number 6+7 and 11+12 can work concurrently but TRM says DMAC can transfer 8 DMA channels at same time. So all 16 Req numbers cannot activate at same time. It should be keep less or equal than 8 numbers. For details, DMAC of RK3328 is ARM PL330 (or compatible IP). - Local variable 'chan_id' of of_dma_pl330_xlate() is Req number. This is from Device-Tree info. - Array 'channels' of struct pl330_dmac is DMA channels of DMAC. pl330_request_channel() allocate DMA channel that is requested from other drivers. Local variable 'chans' has max channels can run concurrently. Current setting: channels chan_id 0 8 1 9 2 14 3 15 4 10 5 6 6 7 7 (not used) Best Regards, Katsuhiro Suzuki >> >> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net> >> --- >> .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 24 ++++++++++++++++++- >> arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + >> 2 files changed, 24 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> index 2157a528276b..e21645aa3fa5 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> @@ -68,7 +68,8 @@ >> sound { >> compatible = "audio-graph-card"; >> label = "rockchip,rk3328"; >> - dais = <&i2s1_p0 >> + dais = <&i2s0_p0 >> + &i2s1_p0 >> &spdif_p0>; >> }; >> >> @@ -141,6 +142,12 @@ >> >> &hdmi { >> status = "okay"; >> + >> + port@0 { >> + hdmi_p0_0: endpoint { >> + remote-endpoint = <&i2s0_p0_0>; >> + }; >> + }; >> }; >> >> &hdmiphy { >> @@ -256,6 +263,18 @@ >> }; >> }; >> >> +&i2s0 { >> + status = "okay"; >> + >> + i2s0_p0: port { >> + i2s0_p0_0: endpoint { >> + dai-format = "i2s"; >> + mclk-fs = <256>; >> + remote-endpoint = <&hdmi_p0_0>; >> + }; >> + }; >> +}; >> + >> &i2s1 { >> status = "okay"; >> >> @@ -343,6 +362,9 @@ >> >> &uart2 { >> status = "okay"; >> + >> + /delete-property/ dmas; >> + /delete-property/ dma-names; >> }; >> >> &u2phy { >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi >> index 84f14b132e8f..374b5da93a35 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi >> @@ -665,6 +665,7 @@ >> pinctrl-names = "default"; >> pinctrl-0 = <&hdmi_cec &hdmii2c_xfer &hdmi_hpd>; >> rockchip,grf = <&grf>; >> + #sound-dai-cells = <0>; > > please make that a separate patch > > > Heiko > > >
Hi, Am Montag, 4. Februar 2019, 13:59:37 CET schrieb Katsuhiro Suzuki: > On 2019/02/03 18:06, Heiko Stuebner wrote: > > Am Samstag, 2. Februar 2019, 05:34:44 CET schrieb Katsuhiro Suzuki: > >> This patch adds HDMI sound (I2S0) node and remove dma properties > >> from UART2 node for rock64. > >> > >> The DMAC of rk3328 can use 8 channels at same time. Currently, total > >> > >> 7 channels are used as follows: > >> - I2S1 2ch > >> - UART2 2ch > >> - SPDIF 1ch > >> - SPI0 2ch > >> > >> HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel. > >> > >> UART2 can work without DMA resources, so this patch removes dma > >> allocation for UART2 and reuses it to I2S0. > > > > I don't follow that description. How can i2s0 re-use the uart2 dma > > channels? Looking at the dma table in the TRM, uart2 has channels 6+7 > > while i2s0 uses channels 11+12. They should just run concurrently? > > Sorry for confusing... 6 or 7 is as ID number of slave DMA channel. > TRM calls it 'Req number'. Req number 6+7 and 11+12 can work > concurrently but TRM says DMAC can transfer 8 DMA channels at same > time. So all 16 Req numbers cannot activate at same time. It should > be keep less or equal than 8 numbers. But that "shortcoming" of having more requests than channels is not something specific to the pl330, instead most dma controllers have that "problem", which seems to get solved by the virt-dma mechanism of dmaengine - which pl330 doesn't use so far. (but see pl080 for example) The devicetree only describes the hardware and is never meant as a configuration space for kernel-code shortcomings. Heiko
Hello Heiko, On 2019/02/12 20:12, Heiko Stübner wrote: > Hi, > > Am Montag, 4. Februar 2019, 13:59:37 CET schrieb Katsuhiro Suzuki: >> On 2019/02/03 18:06, Heiko Stuebner wrote: >>> Am Samstag, 2. Februar 2019, 05:34:44 CET schrieb Katsuhiro Suzuki: >>>> This patch adds HDMI sound (I2S0) node and remove dma properties >>>> from UART2 node for rock64. >>>> >>>> The DMAC of rk3328 can use 8 channels at same time. Currently, total >>>> >>>> 7 channels are used as follows: >>>> - I2S1 2ch >>>> - UART2 2ch >>>> - SPDIF 1ch >>>> - SPI0 2ch >>>> >>>> HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel. >>>> >>>> UART2 can work without DMA resources, so this patch removes dma >>>> allocation for UART2 and reuses it to I2S0. >>> >>> I don't follow that description. How can i2s0 re-use the uart2 dma >>> channels? Looking at the dma table in the TRM, uart2 has channels 6+7 >>> while i2s0 uses channels 11+12. They should just run concurrently? >> >> Sorry for confusing... 6 or 7 is as ID number of slave DMA channel. >> TRM calls it 'Req number'. Req number 6+7 and 11+12 can work >> concurrently but TRM says DMAC can transfer 8 DMA channels at same >> time. So all 16 Req numbers cannot activate at same time. It should >> be keep less or equal than 8 numbers. > > But that "shortcoming" of having more requests than channels is not > something specific to the pl330, instead most dma controllers have that > "problem", which seems to get solved by the virt-dma mechanism of > dmaengine - which pl330 doesn't use so far. (but see pl080 for example) > I understand. I drop these patches. > The devicetree only describes the hardware and is never meant as a > configuration space for kernel-code shortcomings. > Yes, right. I don't want to use device-tree as configuration space, so which is better? - Fix the pl330 driver first and after that add HDMI-sound node to device-tree. - Just add HDMI-sound node to device-tree. If someone (include me) want to support virt-dma on pl330 driver, they will fix it. (PL330 will face error but all sounds works fine and UART still works fine with DMA error) > > Heiko > > > Best Regards, Katsuhiro Suzuki
Am Sonntag, 17. Februar 2019, 11:58:36 CET schrieb Katsuhiro Suzuki: > Hello Heiko, > > On 2019/02/12 20:12, Heiko Stübner wrote: > > Hi, > > > > Am Montag, 4. Februar 2019, 13:59:37 CET schrieb Katsuhiro Suzuki: > >> On 2019/02/03 18:06, Heiko Stuebner wrote: > >>> Am Samstag, 2. Februar 2019, 05:34:44 CET schrieb Katsuhiro Suzuki: > >>>> This patch adds HDMI sound (I2S0) node and remove dma properties > >>>> from UART2 node for rock64. > >>>> > >>>> The DMAC of rk3328 can use 8 channels at same time. Currently, total > >>>> > >>>> 7 channels are used as follows: > >>>> - I2S1 2ch > >>>> - UART2 2ch > >>>> - SPDIF 1ch > >>>> - SPI0 2ch > >>>> > >>>> HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel. > >>>> > >>>> UART2 can work without DMA resources, so this patch removes dma > >>>> allocation for UART2 and reuses it to I2S0. > >>> > >>> I don't follow that description. How can i2s0 re-use the uart2 dma > >>> channels? Looking at the dma table in the TRM, uart2 has channels 6+7 > >>> while i2s0 uses channels 11+12. They should just run concurrently? > >> > >> Sorry for confusing... 6 or 7 is as ID number of slave DMA channel. > >> TRM calls it 'Req number'. Req number 6+7 and 11+12 can work > >> concurrently but TRM says DMAC can transfer 8 DMA channels at same > >> time. So all 16 Req numbers cannot activate at same time. It should > >> be keep less or equal than 8 numbers. > > > > But that "shortcoming" of having more requests than channels is not > > something specific to the pl330, instead most dma controllers have that > > "problem", which seems to get solved by the virt-dma mechanism of > > dmaengine - which pl330 doesn't use so far. (but see pl080 for example) > > > > I understand. I drop these patches. > > > > The devicetree only describes the hardware and is never meant as a > > configuration space for kernel-code shortcomings. > > > > Yes, right. I don't want to use device-tree as configuration space, > so which is better? > > - Fix the pl330 driver first and after that add HDMI-sound node > to device-tree. > - Just add HDMI-sound node to device-tree. If someone (include me) > want to support virt-dma on pl330 driver, they will fix it. > (PL330 will face error but all sounds works fine and UART still > works fine with DMA error) I'd go with this second option :-) . Heiko
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts index 2157a528276b..e21645aa3fa5 100644 --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts @@ -68,7 +68,8 @@ sound { compatible = "audio-graph-card"; label = "rockchip,rk3328"; - dais = <&i2s1_p0 + dais = <&i2s0_p0 + &i2s1_p0 &spdif_p0>; }; @@ -141,6 +142,12 @@ &hdmi { status = "okay"; + + port@0 { + hdmi_p0_0: endpoint { + remote-endpoint = <&i2s0_p0_0>; + }; + }; }; &hdmiphy { @@ -256,6 +263,18 @@ }; }; +&i2s0 { + status = "okay"; + + i2s0_p0: port { + i2s0_p0_0: endpoint { + dai-format = "i2s"; + mclk-fs = <256>; + remote-endpoint = <&hdmi_p0_0>; + }; + }; +}; + &i2s1 { status = "okay"; @@ -343,6 +362,9 @@ &uart2 { status = "okay"; + + /delete-property/ dmas; + /delete-property/ dma-names; }; &u2phy { diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi index 84f14b132e8f..374b5da93a35 100644 --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi @@ -665,6 +665,7 @@ pinctrl-names = "default"; pinctrl-0 = <&hdmi_cec &hdmii2c_xfer &hdmi_hpd>; rockchip,grf = <&grf>; + #sound-dai-cells = <0>; status = "disabled"; ports {
This patch adds HDMI sound (I2S0) node and remove dma properties from UART2 node for rock64. The DMAC of rk3328 can use 8 channels at same time. Currently, total 7 channels are used as follows: - I2S1 2ch - UART2 2ch - SPDIF 1ch - SPI0 2ch HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel. UART2 can work without DMA resources, so this patch removes dma allocation for UART2 and reuses it to I2S0. Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net> --- .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 24 ++++++++++++++++++- arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + 2 files changed, 24 insertions(+), 1 deletion(-)