| Message ID | 20190105183150.20266-5-ayaka@soulik.info (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rockchip: the vendor video codec for reference | expand |
Hi Randy, Thanks a lot for this patches. They are really useful to provide more insight into the VPU hardware. This change will make the vpu encoder and vpu decoder completely independent, can they really work in parallel? Could you provide more details about what is shared between these devices? Thanks a lot! On Sun, 2019-01-06 at 02:31 +0800, Randy Li wrote: > It offers an example how a full features video codec > should be configured. > > The original clocks assignment don't look good, if the clocks > lower than 300MHZ, most of decoing tasks would suffer from > timeout problem, 500MHZ is also a little high for RK3399 > running in a stable state. > > Signed-off-by: Randy Li <ayaka@soulik.info> > --- > .../boot/dts/rockchip/rk3399-sapphire.dtsi | 29 ++++++++ > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 68 +++++++++++++++++-- > 2 files changed, 90 insertions(+), 7 deletions(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi > index 946d3589575a..c3db878bae45 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi > @@ -632,6 +632,35 @@ > dr_mode = "host"; > }; > > +&rkvdec { > + status = "okay"; > +}; > + > +&rkvdec_srv { > + status = "okay"; > +}; > + > +&vdec_mmu { > + status = "okay"; > +}; > + > +&vdpu { > + status = "okay"; > +}; > + > +&vepu { > + status = "okay"; > +}; > + > +&vpu_service { > + status = "okay"; > +}; > + > +&vpu_mmu { > + status = "okay"; > + > +}; > + > &vopb { > status = "okay"; > }; > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > index b22b2e40422b..5fa3247e7bf0 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > @@ -1242,16 +1242,39 @@ > status = "disabled"; > }; > > - vpu: video-codec@ff650000 { > - compatible = "rockchip,rk3399-vpu"; > - reg = <0x0 0xff650000 0x0 0x800>; > - interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>, > - <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; > - interrupt-names = "vepu", "vdpu"; > + vpu_service: vpu-srv { > + compatible = "rockchip,mpp-service"; > + status = "disabled"; > + }; > + > + vepu: vpu-encoder@ff650000 { > + compatible = "rockchip,vpu-encoder-v2"; > + reg = <0x0 0xff650000 0x0 0x400>; > + interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>; > + interrupt-names = "irq_enc"; > clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; > - clock-names = "aclk", "hclk"; > + clock-names = "aclk_vcodec", "hclk_vcodec"; > + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; > + reset-names = "video_h", "video_a"; > iommus = <&vpu_mmu>; > power-domains = <&power RK3399_PD_VCODEC>; > + rockchip,srv = <&vpu_service>; > + status = "disabled"; > + }; > + > + vdpu: vpu-decoder@ff650400 { > + compatible = "rockchip,vpu-decoder-v2"; > + reg = <0x0 0xff650400 0x0 0x400>; > + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; > + interrupt-names = "irq_dec"; > + iommus = <&vpu_mmu>; > + clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; > + clock-names = "aclk_vcodec", "hclk_vcodec"; > + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; > + reset-names = "video_h", "video_a"; > + power-domains = <&power RK3399_PD_VCODEC>; > + rockchip,srv = <&vpu_service>; > + status = "disabled"; > }; > > vpu_mmu: iommu@ff650800 { > @@ -1261,11 +1284,42 @@ > interrupt-names = "vpu_mmu"; > clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; > clock-names = "aclk", "iface"; > + assigned-clocks = <&cru ACLK_VCODEC_PRE>; > + assigned-clock-parents = <&cru PLL_GPLL>; > #iommu-cells = <0>; > power-domains = <&power RK3399_PD_VCODEC>; > status = "disabled"; > }; > > + rkvdec_srv: rkvdec-srv { > + compatible = "rockchip,mpp-service"; > + status = "disabled"; > + }; > + > + rkvdec: video-decoder@ff660000 { > + compatible = "rockchip,video-decoder-v1"; > + reg = <0x0 0xff660000 0x0 0x400>; > + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH 0>; > + interrupt-names = "irq_dec"; > + clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>, > + <&cru SCLK_VDU_CA>, <&cru SCLK_VDU_CORE>; > + clock-names = "aclk_vcodec", "hclk_vcodec", > + "clk_cabac", "clk_core"; > + assigned-clocks = <&cru ACLK_VDU_PRE>, <&cru SCLK_VDU_CA>, > + <&cru SCLK_VDU_CORE>; > + assigned-clock-parents = <&cru PLL_NPLL>, <&cru PLL_NPLL>, > + <&cru PLL_NPLL>; > + resets = <&cru SRST_H_VDU>, <&cru SRST_A_VDU>, > + <&cru SRST_VDU_CORE>, <&cru SRST_VDU_CA>, > + <&cru SRST_A_VDU_NOC>, <&cru SRST_H_VDU_NOC>; > + reset-names = "video_h", "video_a", "video_core", "video_cabac", > + "niu_a", "niu_h"; > + power-domains = <&power RK3399_PD_VDU>; > + rockchip,srv = <&rkvdec_srv>; > + iommus = <&vdec_mmu>; > + status = "disabled"; > + }; > + > vdec_mmu: iommu@ff660480 { > compatible = "rockchip,iommu"; > reg = <0x0 0xff660480 0x0 0x40>, <0x0 0xff6604c0 0x0 0x40>;
> On Jan 6, 2019, at 10:22 PM, Ezequiel Garcia <ezequiel@collabora.com> wrote: > > Hi Randy, > > Thanks a lot for this patches. They are really useful > to provide more insight into the VPU hardware. > > This change will make the vpu encoder and vpu decoder > completely independent, can they really work in parallel? As I said it depends on the platform, but with this patch, the user space would think they can work at the same time. BTW, not only the decoder, there is a post processor with the decoder, it can be used as part of the decoder pipeline with only a macro block delay or process the data from an external buffer. I forget to write a note on what this driver doesn’t present. The real one would have much complex scheduler system, but this one is just a queue. More task management feature are not there. Also the clock boosting is removing and the loading analysis, which are useful for encoder, especially on the rv1108. > Could you provide more details about what is > shared between these devices? No, if Rockchip doesn’t tell, my mouth is sealed. > Thanks a lot! > >> On Sun, 2019-01-06 at 02:31 +0800, Randy Li wrote: >> It offers an example how a full features video codec >> should be configured. >> >> The original clocks assignment don't look good, if the clocks >> lower than 300MHZ, most of decoing tasks would suffer from >> timeout problem, 500MHZ is also a little high for RK3399 >> running in a stable state. >> >> Signed-off-by: Randy Li <ayaka@soulik.info> >> --- >> .../boot/dts/rockchip/rk3399-sapphire.dtsi | 29 ++++++++ >> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 68 +++++++++++++++++-- >> 2 files changed, 90 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >> index 946d3589575a..c3db878bae45 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >> @@ -632,6 +632,35 @@ >> dr_mode = "host"; >> }; >> >> +&rkvdec { >> + status = "okay"; >> +}; >> + >> +&rkvdec_srv { >> + status = "okay"; >> +}; >> + >> +&vdec_mmu { >> + status = "okay"; >> +}; >> + >> +&vdpu { >> + status = "okay"; >> +}; >> + >> +&vepu { >> + status = "okay"; >> +}; >> + >> +&vpu_service { >> + status = "okay"; >> +}; >> + >> +&vpu_mmu { >> + status = "okay"; >> + >> +}; >> + >> &vopb { >> status = "okay"; >> }; >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> index b22b2e40422b..5fa3247e7bf0 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> @@ -1242,16 +1242,39 @@ >> status = "disabled"; >> }; >> >> - vpu: video-codec@ff650000 { >> - compatible = "rockchip,rk3399-vpu"; >> - reg = <0x0 0xff650000 0x0 0x800>; >> - interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>, >> - <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; >> - interrupt-names = "vepu", "vdpu"; >> + vpu_service: vpu-srv { >> + compatible = "rockchip,mpp-service"; >> + status = "disabled"; >> + }; >> + >> + vepu: vpu-encoder@ff650000 { >> + compatible = "rockchip,vpu-encoder-v2"; >> + reg = <0x0 0xff650000 0x0 0x400>; >> + interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>; >> + interrupt-names = "irq_enc"; >> clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >> - clock-names = "aclk", "hclk"; >> + clock-names = "aclk_vcodec", "hclk_vcodec"; >> + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; >> + reset-names = "video_h", "video_a"; >> iommus = <&vpu_mmu>; >> power-domains = <&power RK3399_PD_VCODEC>; >> + rockchip,srv = <&vpu_service>; >> + status = "disabled"; >> + }; >> + >> + vdpu: vpu-decoder@ff650400 { >> + compatible = "rockchip,vpu-decoder-v2"; >> + reg = <0x0 0xff650400 0x0 0x400>; >> + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; >> + interrupt-names = "irq_dec"; >> + iommus = <&vpu_mmu>; >> + clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >> + clock-names = "aclk_vcodec", "hclk_vcodec"; >> + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; >> + reset-names = "video_h", "video_a"; >> + power-domains = <&power RK3399_PD_VCODEC>; >> + rockchip,srv = <&vpu_service>; >> + status = "disabled"; >> }; >> >> vpu_mmu: iommu@ff650800 { >> @@ -1261,11 +1284,42 @@ >> interrupt-names = "vpu_mmu"; >> clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >> clock-names = "aclk", "iface"; >> + assigned-clocks = <&cru ACLK_VCODEC_PRE>; >> + assigned-clock-parents = <&cru PLL_GPLL>; >> #iommu-cells = <0>; >> power-domains = <&power RK3399_PD_VCODEC>; >> status = "disabled"; >> }; >> >> + rkvdec_srv: rkvdec-srv { >> + compatible = "rockchip,mpp-service"; >> + status = "disabled"; >> + }; >> + >> + rkvdec: video-decoder@ff660000 { >> + compatible = "rockchip,video-decoder-v1"; >> + reg = <0x0 0xff660000 0x0 0x400>; >> + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH 0>; >> + interrupt-names = "irq_dec"; >> + clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>, >> + <&cru SCLK_VDU_CA>, <&cru SCLK_VDU_CORE>; >> + clock-names = "aclk_vcodec", "hclk_vcodec", >> + "clk_cabac", "clk_core"; >> + assigned-clocks = <&cru ACLK_VDU_PRE>, <&cru SCLK_VDU_CA>, >> + <&cru SCLK_VDU_CORE>; >> + assigned-clock-parents = <&cru PLL_NPLL>, <&cru PLL_NPLL>, >> + <&cru PLL_NPLL>; >> + resets = <&cru SRST_H_VDU>, <&cru SRST_A_VDU>, >> + <&cru SRST_VDU_CORE>, <&cru SRST_VDU_CA>, >> + <&cru SRST_A_VDU_NOC>, <&cru SRST_H_VDU_NOC>; >> + reset-names = "video_h", "video_a", "video_core", "video_cabac", >> + "niu_a", "niu_h"; >> + power-domains = <&power RK3399_PD_VDU>; >> + rockchip,srv = <&rkvdec_srv>; >> + iommus = <&vdec_mmu>; >> + status = "disabled"; >> + }; >> + >> vdec_mmu: iommu@ff660480 { >> compatible = "rockchip,iommu"; >> reg = <0x0 0xff660480 0x0 0x40>, <0x0 0xff6604c0 0x0 0x40>; > >
> On Jan 6, 2019, at 10:22 PM, Ezequiel Garcia <ezequiel@collabora.com> wrote: > > Hi Randy, > > Thanks a lot for this patches. They are really useful > to provide more insight into the VPU hardware. > > This change will make the vpu encoder and vpu decoder > completely independent, can they really work in parallel? As I said it depends on the platform, but with this patch, the user space would think they can work at the same time. BTW, not only the decoder, there is a post processor with the decoder, it can be used as part of the decoder pipeline with only a macro block delay or process the data from an external buffer. I forget to write a note on what this driver doesn’t present. The real one would have much complex scheduler system, but this one is just a queue. More task management feature are not there. Also the clock boosting is removing and the loading analysis, which are useful for encoder, especially on the rv1108. > Could you provide more details about what is > shared between these devices? No, if Rockchip doesn’t tell, my mouth is sealed. > Thanks a lot! > >> On Sun, 2019-01-06 at 02:31 +0800, Randy Li wrote: >> It offers an example how a full features video codec >> should be configured. >> >> The original clocks assignment don't look good, if the clocks >> lower than 300MHZ, most of decoing tasks would suffer from >> timeout problem, 500MHZ is also a little high for RK3399 >> running in a stable state. >> >> Signed-off-by: Randy Li <ayaka@soulik.info> >> --- >> .../boot/dts/rockchip/rk3399-sapphire.dtsi | 29 ++++++++ >> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 68 +++++++++++++++++-- >> 2 files changed, 90 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >> index 946d3589575a..c3db878bae45 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >> @@ -632,6 +632,35 @@ >> dr_mode = "host"; >> }; >> >> +&rkvdec { >> + status = "okay"; >> +}; >> + >> +&rkvdec_srv { >> + status = "okay"; >> +}; >> + >> +&vdec_mmu { >> + status = "okay"; >> +}; >> + >> +&vdpu { >> + status = "okay"; >> +}; >> + >> +&vepu { >> + status = "okay"; >> +}; >> + >> +&vpu_service { >> + status = "okay"; >> +}; >> + >> +&vpu_mmu { >> + status = "okay"; >> + >> +}; >> + >> &vopb { >> status = "okay"; >> }; >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> index b22b2e40422b..5fa3247e7bf0 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> @@ -1242,16 +1242,39 @@ >> status = "disabled"; >> }; >> >> - vpu: video-codec@ff650000 { >> - compatible = "rockchip,rk3399-vpu"; >> - reg = <0x0 0xff650000 0x0 0x800>; >> - interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>, >> - <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; >> - interrupt-names = "vepu", "vdpu"; >> + vpu_service: vpu-srv { >> + compatible = "rockchip,mpp-service"; >> + status = "disabled"; >> + }; >> + >> + vepu: vpu-encoder@ff650000 { >> + compatible = "rockchip,vpu-encoder-v2"; >> + reg = <0x0 0xff650000 0x0 0x400>; >> + interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>; >> + interrupt-names = "irq_enc"; >> clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >> - clock-names = "aclk", "hclk"; >> + clock-names = "aclk_vcodec", "hclk_vcodec"; >> + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; >> + reset-names = "video_h", "video_a"; >> iommus = <&vpu_mmu>; >> power-domains = <&power RK3399_PD_VCODEC>; >> + rockchip,srv = <&vpu_service>; >> + status = "disabled"; >> + }; >> + >> + vdpu: vpu-decoder@ff650400 { >> + compatible = "rockchip,vpu-decoder-v2"; >> + reg = <0x0 0xff650400 0x0 0x400>; >> + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; >> + interrupt-names = "irq_dec"; >> + iommus = <&vpu_mmu>; >> + clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >> + clock-names = "aclk_vcodec", "hclk_vcodec"; >> + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; >> + reset-names = "video_h", "video_a"; >> + power-domains = <&power RK3399_PD_VCODEC>; >> + rockchip,srv = <&vpu_service>; >> + status = "disabled"; >> }; >> >> vpu_mmu: iommu@ff650800 { >> @@ -1261,11 +1284,42 @@ >> interrupt-names = "vpu_mmu"; >> clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >> clock-names = "aclk", "iface"; >> + assigned-clocks = <&cru ACLK_VCODEC_PRE>; >> + assigned-clock-parents = <&cru PLL_GPLL>; >> #iommu-cells = <0>; >> power-domains = <&power RK3399_PD_VCODEC>; >> status = "disabled"; >> }; >> >> + rkvdec_srv: rkvdec-srv { >> + compatible = "rockchip,mpp-service"; >> + status = "disabled"; >> + }; >> + >> + rkvdec: video-decoder@ff660000 { >> + compatible = "rockchip,video-decoder-v1"; >> + reg = <0x0 0xff660000 0x0 0x400>; >> + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH 0>; >> + interrupt-names = "irq_dec"; >> + clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>, >> + <&cru SCLK_VDU_CA>, <&cru SCLK_VDU_CORE>; >> + clock-names = "aclk_vcodec", "hclk_vcodec", >> + "clk_cabac", "clk_core"; >> + assigned-clocks = <&cru ACLK_VDU_PRE>, <&cru SCLK_VDU_CA>, >> + <&cru SCLK_VDU_CORE>; >> + assigned-clock-parents = <&cru PLL_NPLL>, <&cru PLL_NPLL>, >> + <&cru PLL_NPLL>; >> + resets = <&cru SRST_H_VDU>, <&cru SRST_A_VDU>, >> + <&cru SRST_VDU_CORE>, <&cru SRST_VDU_CA>, >> + <&cru SRST_A_VDU_NOC>, <&cru SRST_H_VDU_NOC>; >> + reset-names = "video_h", "video_a", "video_core", "video_cabac", >> + "niu_a", "niu_h"; >> + power-domains = <&power RK3399_PD_VDU>; >> + rockchip,srv = <&rkvdec_srv>; >> + iommus = <&vdec_mmu>; >> + status = "disabled"; >> + }; >> + >> vdec_mmu: iommu@ff660480 { >> compatible = "rockchip,iommu"; >> reg = <0x0 0xff660480 0x0 0x40>, <0x0 0xff6604c0 0x0 0x40>; > >
On Sun, 2019-01-06 at 23:05 +0800, Ayaka wrote: > > On Jan 6, 2019, at 10:22 PM, Ezequiel Garcia <ezequiel@collabora.com> wrote: > > > > Hi Randy, > > > > Thanks a lot for this patches. They are really useful > > to provide more insight into the VPU hardware. > > > > This change will make the vpu encoder and vpu decoder > > completely independent, can they really work in parallel? > As I said it depends on the platform, but with this patch, the user space would think they can work at the same time. I think there is some confusion. The devicetree is one thing: it is a hardware representation, a way to describe the hardware, for the kernel/bootloader to parse. The userspace view will depend on the driver implementation. The current devicetree and driver (without your patches), model the VPU as a single piece of hardware, exposing a decoder and an encoder. The V4L driver will then create two video devices, i.e. /dev/videoX and /dev/videoY. So userspace sees an independent view of the devices. However, they are internally connected, and thus we can easily avoid two jobs running in parallel. So, in other words, if the VPU can issue a decoder and encoder job in parallel, then it's useful to model it as two independent devices. Otherwise, it's better not to. I hope this can clarify things a bit for you! PS: Too bad I won't be at FOSDEM to discuss this personally. Thanks, Ezequiel > BTW, not only the decoder, there is a post processor with the decoder, it can be used as part of the decoder pipeline with only a macro block delay > or process the data from an external buffer. > I forget to write a note on what this driver doesn’t present. The real one would have much complex scheduler system, but this one is just a queue. > More task management feature are not there. > Also the clock boosting is removing and the loading analysis, which are useful for encoder, especially on the rv1108. > > Could you provide more details about what is > > shared between these devices? > No, if Rockchip doesn’t tell, my mouth is sealed. > > Thanks a lot! > > > > > On Sun, 2019-01-06 at 02:31 +0800, Randy Li wrote: > > > It offers an example how a full features video codec > > > should be configured. > > > > > > The original clocks assignment don't look good, if the clocks > > > lower than 300MHZ, most of decoing tasks would suffer from > > > timeout problem, 500MHZ is also a little high for RK3399 > > > running in a stable state. > > > > > > Signed-off-by: Randy Li <ayaka@soulik.info> > > > --- > > > .../boot/dts/rockchip/rk3399-sapphire.dtsi | 29 ++++++++ > > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 68 +++++++++++++++++-- > > > 2 files changed, 90 insertions(+), 7 deletions(-) > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi > > > index 946d3589575a..c3db878bae45 100644 > > > --- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi > > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi > > > @@ -632,6 +632,35 @@ > > > dr_mode = "host"; > > > }; > > > > > > +&rkvdec { > > > + status = "okay"; > > > +}; > > > + > > > +&rkvdec_srv { > > > + status = "okay"; > > > +}; > > > + > > > +&vdec_mmu { > > > + status = "okay"; > > > +}; > > > + > > > +&vdpu { > > > + status = "okay"; > > > +}; > > > + > > > +&vepu { > > > + status = "okay"; > > > +}; > > > + > > > +&vpu_service { > > > + status = "okay"; > > > +}; > > > + > > > +&vpu_mmu { > > > + status = "okay"; > > > + > > > +}; > > > + > > > &vopb { > > > status = "okay"; > > > }; > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > > > index b22b2e40422b..5fa3247e7bf0 100644 > > > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > > > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > > > @@ -1242,16 +1242,39 @@ > > > status = "disabled"; > > > }; > > > > > > - vpu: video-codec@ff650000 { > > > - compatible = "rockchip,rk3399-vpu"; > > > - reg = <0x0 0xff650000 0x0 0x800>; > > > - interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>, > > > - <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; > > > - interrupt-names = "vepu", "vdpu"; > > > + vpu_service: vpu-srv { > > > + compatible = "rockchip,mpp-service"; > > > + status = "disabled"; > > > + }; > > > + > > > + vepu: vpu-encoder@ff650000 { > > > + compatible = "rockchip,vpu-encoder-v2"; > > > + reg = <0x0 0xff650000 0x0 0x400>; > > > + interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>; > > > + interrupt-names = "irq_enc"; > > > clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; > > > - clock-names = "aclk", "hclk"; > > > + clock-names = "aclk_vcodec", "hclk_vcodec"; > > > + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; > > > + reset-names = "video_h", "video_a"; > > > iommus = <&vpu_mmu>; > > > power-domains = <&power RK3399_PD_VCODEC>; > > > + rockchip,srv = <&vpu_service>; > > > + status = "disabled"; > > > + }; > > > + > > > + vdpu: vpu-decoder@ff650400 { > > > + compatible = "rockchip,vpu-decoder-v2"; > > > + reg = <0x0 0xff650400 0x0 0x400>; > > > + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; > > > + interrupt-names = "irq_dec"; > > > + iommus = <&vpu_mmu>; > > > + clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; > > > + clock-names = "aclk_vcodec", "hclk_vcodec"; > > > + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; > > > + reset-names = "video_h", "video_a"; > > > + power-domains = <&power RK3399_PD_VCODEC>; > > > + rockchip,srv = <&vpu_service>; > > > + status = "disabled"; > > > }; > > > > > > vpu_mmu: iommu@ff650800 { > > > @@ -1261,11 +1284,42 @@ > > > interrupt-names = "vpu_mmu"; > > > clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; > > > clock-names = "aclk", "iface"; > > > + assigned-clocks = <&cru ACLK_VCODEC_PRE>; > > > + assigned-clock-parents = <&cru PLL_GPLL>; > > > #iommu-cells = <0>; > > > power-domains = <&power RK3399_PD_VCODEC>; > > > status = "disabled"; > > > }; > > > > > > + rkvdec_srv: rkvdec-srv { > > > + compatible = "rockchip,mpp-service"; > > > + status = "disabled"; > > > + }; > > > + > > > + rkvdec: video-decoder@ff660000 { > > > + compatible = "rockchip,video-decoder-v1"; > > > + reg = <0x0 0xff660000 0x0 0x400>; > > > + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH 0>; > > > + interrupt-names = "irq_dec"; > > > + clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>, > > > + <&cru SCLK_VDU_CA>, <&cru SCLK_VDU_CORE>; > > > + clock-names = "aclk_vcodec", "hclk_vcodec", > > > + "clk_cabac", "clk_core"; > > > + assigned-clocks = <&cru ACLK_VDU_PRE>, <&cru SCLK_VDU_CA>, > > > + <&cru SCLK_VDU_CORE>; > > > + assigned-clock-parents = <&cru PLL_NPLL>, <&cru PLL_NPLL>, > > > + <&cru PLL_NPLL>; > > > + resets = <&cru SRST_H_VDU>, <&cru SRST_A_VDU>, > > > + <&cru SRST_VDU_CORE>, <&cru SRST_VDU_CA>, > > > + <&cru SRST_A_VDU_NOC>, <&cru SRST_H_VDU_NOC>; > > > + reset-names = "video_h", "video_a", "video_core", "video_cabac", > > > + "niu_a", "niu_h"; > > > + power-domains = <&power RK3399_PD_VDU>; > > > + rockchip,srv = <&rkvdec_srv>; > > > + iommus = <&vdec_mmu>; > > > + status = "disabled"; > > > + }; > > > + > > > vdec_mmu: iommu@ff660480 { > > > compatible = "rockchip,iommu"; > > > reg = <0x0 0xff660480 0x0 0x40>, <0x0 0xff6604c0 0x0 0x40>;
Sent from my iPad > On Jan 7, 2019, at 12:04 AM, Ezequiel Garcia <ezequiel@collabora.com> wrote: > > On Sun, 2019-01-06 at 23:05 +0800, Ayaka wrote: >>> On Jan 6, 2019, at 10:22 PM, Ezequiel Garcia <ezequiel@collabora.com> wrote: >>> >>> Hi Randy, >>> >>> Thanks a lot for this patches. They are really useful >>> to provide more insight into the VPU hardware. >>> >>> This change will make the vpu encoder and vpu decoder >>> completely independent, can they really work in parallel? >> As I said it depends on the platform, but with this patch, the user space would think they can work at the same time. > > > I think there is some confusion. > > The devicetree is one thing: it is a hardware representation, > a way to describe the hardware, for the kernel/bootloader to > parse. > > The userspace view will depend on the driver implementation. > > The current devicetree and driver (without your patches), > model the VPU as a single piece of hardware, exposing a decoder > and an encoder. > > The V4L driver will then create two video devices, i.e. /dev/videoX > and /dev/videoY. So userspace sees an independent view of the > devices. > I knew that, the problem is that the driver should not always create a decoder and encoder pair, they may not exist at some platforms, even some platforms doesn’t have a encoder. You may have a look on the rk3328 I post on the first email as example. > However, they are internally connected, and thus we can > easily avoid two jobs running in parallel. > That is what the mpp service did in my patches, handing the relationship between each devices. And it is not a easy work, maybe a 4k decoder would be blocked by another high frame rate encoding work or another decoder session. The vendor kernel have more worry about this, but not in this version. > So, in other words, if the VPU can issue a decoder and encoder > job in parallel, then it's useful to model it as two independent > devices. Otherwise, it's better not to. > > I hope this can clarify things a bit for you! > I would review the request API for those codecs structures and some basic designs, people from LibreELEC told me a few noticed inform. > PS: Too bad I won't be at FOSDEM to discuss this personally. I am sure Paulk from bootlin would be there and some others guy related, I have not cared about this for a little long time, there are more huge problem I should point out now. > > Thanks, > Ezequiel > >> BTW, not only the decoder, there is a post processor with the decoder, it can be used as part of the decoder pipeline with only a macro block delay >> or process the data from an external buffer. >> I forget to write a note on what this driver doesn’t present. The real one would have much complex scheduler system, but this one is just a queue. >> More task management feature are not there. >> Also the clock boosting is removing and the loading analysis, which are useful for encoder, especially on the rv1108. >>> Could you provide more details about what is >>> shared between these devices? >> No, if Rockchip doesn’t tell, my mouth is sealed. >>> Thanks a lot! >>> >>>> On Sun, 2019-01-06 at 02:31 +0800, Randy Li wrote: >>>> It offers an example how a full features video codec >>>> should be configured. >>>> >>>> The original clocks assignment don't look good, if the clocks >>>> lower than 300MHZ, most of decoing tasks would suffer from >>>> timeout problem, 500MHZ is also a little high for RK3399 >>>> running in a stable state. >>>> >>>> Signed-off-by: Randy Li <ayaka@soulik.info> >>>> --- >>>> .../boot/dts/rockchip/rk3399-sapphire.dtsi | 29 ++++++++ >>>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 68 +++++++++++++++++-- >>>> 2 files changed, 90 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >>>> index 946d3589575a..c3db878bae45 100644 >>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi >>>> @@ -632,6 +632,35 @@ >>>> dr_mode = "host"; >>>> }; >>>> >>>> +&rkvdec { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&rkvdec_srv { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&vdec_mmu { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&vdpu { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&vepu { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&vpu_service { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&vpu_mmu { >>>> + status = "okay"; >>>> + >>>> +}; >>>> + >>>> &vopb { >>>> status = "okay"; >>>> }; >>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >>>> index b22b2e40422b..5fa3247e7bf0 100644 >>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi >>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >>>> @@ -1242,16 +1242,39 @@ >>>> status = "disabled"; >>>> }; >>>> >>>> - vpu: video-codec@ff650000 { >>>> - compatible = "rockchip,rk3399-vpu"; >>>> - reg = <0x0 0xff650000 0x0 0x800>; >>>> - interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>, >>>> - <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; >>>> - interrupt-names = "vepu", "vdpu"; >>>> + vpu_service: vpu-srv { >>>> + compatible = "rockchip,mpp-service"; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + vepu: vpu-encoder@ff650000 { >>>> + compatible = "rockchip,vpu-encoder-v2"; >>>> + reg = <0x0 0xff650000 0x0 0x400>; >>>> + interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>; >>>> + interrupt-names = "irq_enc"; >>>> clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >>>> - clock-names = "aclk", "hclk"; >>>> + clock-names = "aclk_vcodec", "hclk_vcodec"; >>>> + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; >>>> + reset-names = "video_h", "video_a"; >>>> iommus = <&vpu_mmu>; >>>> power-domains = <&power RK3399_PD_VCODEC>; >>>> + rockchip,srv = <&vpu_service>; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + vdpu: vpu-decoder@ff650400 { >>>> + compatible = "rockchip,vpu-decoder-v2"; >>>> + reg = <0x0 0xff650400 0x0 0x400>; >>>> + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; >>>> + interrupt-names = "irq_dec"; >>>> + iommus = <&vpu_mmu>; >>>> + clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >>>> + clock-names = "aclk_vcodec", "hclk_vcodec"; >>>> + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; >>>> + reset-names = "video_h", "video_a"; >>>> + power-domains = <&power RK3399_PD_VCODEC>; >>>> + rockchip,srv = <&vpu_service>; >>>> + status = "disabled"; >>>> }; >>>> >>>> vpu_mmu: iommu@ff650800 { >>>> @@ -1261,11 +1284,42 @@ >>>> interrupt-names = "vpu_mmu"; >>>> clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; >>>> clock-names = "aclk", "iface"; >>>> + assigned-clocks = <&cru ACLK_VCODEC_PRE>; >>>> + assigned-clock-parents = <&cru PLL_GPLL>; >>>> #iommu-cells = <0>; >>>> power-domains = <&power RK3399_PD_VCODEC>; >>>> status = "disabled"; >>>> }; >>>> >>>> + rkvdec_srv: rkvdec-srv { >>>> + compatible = "rockchip,mpp-service"; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + rkvdec: video-decoder@ff660000 { >>>> + compatible = "rockchip,video-decoder-v1"; >>>> + reg = <0x0 0xff660000 0x0 0x400>; >>>> + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH 0>; >>>> + interrupt-names = "irq_dec"; >>>> + clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>, >>>> + <&cru SCLK_VDU_CA>, <&cru SCLK_VDU_CORE>; >>>> + clock-names = "aclk_vcodec", "hclk_vcodec", >>>> + "clk_cabac", "clk_core"; >>>> + assigned-clocks = <&cru ACLK_VDU_PRE>, <&cru SCLK_VDU_CA>, >>>> + <&cru SCLK_VDU_CORE>; >>>> + assigned-clock-parents = <&cru PLL_NPLL>, <&cru PLL_NPLL>, >>>> + <&cru PLL_NPLL>; >>>> + resets = <&cru SRST_H_VDU>, <&cru SRST_A_VDU>, >>>> + <&cru SRST_VDU_CORE>, <&cru SRST_VDU_CA>, >>>> + <&cru SRST_A_VDU_NOC>, <&cru SRST_H_VDU_NOC>; >>>> + reset-names = "video_h", "video_a", "video_core", "video_cabac", >>>> + "niu_a", "niu_h"; >>>> + power-domains = <&power RK3399_PD_VDU>; >>>> + rockchip,srv = <&rkvdec_srv>; >>>> + iommus = <&vdec_mmu>; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> vdec_mmu: iommu@ff660480 { >>>> compatible = "rockchip,iommu"; >>>> reg = <0x0 0xff660480 0x0 0x40>, <0x0 0xff6604c0 0x0 0x40>; > >
On Sun, 6 Jan 2019 at 13:16, Ayaka <ayaka@soulik.info> wrote: > > > > Sent from my iPad > > > On Jan 7, 2019, at 12:04 AM, Ezequiel Garcia <ezequiel@collabora.com> wrote: > > > > On Sun, 2019-01-06 at 23:05 +0800, Ayaka wrote: > >>> On Jan 6, 2019, at 10:22 PM, Ezequiel Garcia <ezequiel@collabora.com> wrote: > >>> > >>> Hi Randy, > >>> > >>> Thanks a lot for this patches. They are really useful > >>> to provide more insight into the VPU hardware. > >>> > >>> This change will make the vpu encoder and vpu decoder > >>> completely independent, can they really work in parallel? > >> As I said it depends on the platform, but with this patch, the user space would think they can work at the same time. > > > > > > I think there is some confusion. > > > > The devicetree is one thing: it is a hardware representation, > > a way to describe the hardware, for the kernel/bootloader to > > parse. > > > > The userspace view will depend on the driver implementation. > > > > The current devicetree and driver (without your patches), > > model the VPU as a single piece of hardware, exposing a decoder > > and an encoder. > > > > The V4L driver will then create two video devices, i.e. /dev/videoX > > and /dev/videoY. So userspace sees an independent view of the > > devices. > > > I knew that, the problem is that the driver should not always create a decoder and encoder pair, they may not exist at some platforms, even some platforms doesn’t have a encoder. You may have a look on the rk3328 I post on the first email as example. That is correct. But that still doesn't tackle my question: is the hardware able to run a decoding and an encoding job in parallel? If not, then it's wrong to describe them as independent entities. > > However, they are internally connected, and thus we can > > easily avoid two jobs running in parallel. > > > That is what the mpp service did in my patches, handing the relationship between each devices. And it is not a easy work, maybe a 4k decoder would be blocked by another high frame rate encoding work or another decoder session. The vendor kernel have more worry about this, but not in this version. Right. That is one way to design it. Another way is having a single devicetree node for the VPU encoder/decoder "complex". Thanks for the input!
Hello Ezequiel Sent from my iPad > On Jan 7, 2019, at 1:21 AM, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: > >> On Sun, 6 Jan 2019 at 13:16, Ayaka <ayaka@soulik.info> wrote: >> >> >> >> Sent from my iPad >> >>> On Jan 7, 2019, at 12:04 AM, Ezequiel Garcia <ezequiel@collabora.com> wrote: >>> >>> On Sun, 2019-01-06 at 23:05 +0800, Ayaka wrote: >>>>> On Jan 6, 2019, at 10:22 PM, Ezequiel Garcia <ezequiel@collabora.com> wrote: >>>>> >>>>> Hi Randy, >>>>> >>>>> Thanks a lot for this patches. They are really useful >>>>> to provide more insight into the VPU hardware. >>>>> >>>>> This change will make the vpu encoder and vpu decoder >>>>> completely independent, can they really work in parallel? >>>> As I said it depends on the platform, but with this patch, the user space would think they can work at the same time. >>> >>> >>> I think there is some confusion. >>> >>> The devicetree is one thing: it is a hardware representation, >>> a way to describe the hardware, for the kernel/bootloader to >>> parse. >>> >>> The userspace view will depend on the driver implementation. >>> >>> The current devicetree and driver (without your patches), >>> model the VPU as a single piece of hardware, exposing a decoder >>> and an encoder. >>> >>> The V4L driver will then create two video devices, i.e. /dev/videoX >>> and /dev/videoY. So userspace sees an independent view of the >>> devices. >>> >> I knew that, the problem is that the driver should not always create a decoder and encoder pair, they may not exist at some platforms, even some platforms doesn’t have a encoder. You may have a look on the rk3328 I post on the first email as example. > > That is correct. But that still doesn't tackle my question: is the > hardware able to run a decoding and an encoding job in parallel? > For rk3328, yes, you see I didn’t draw them in the same box. > If not, then it's wrong to describe them as independent entities. > >>> However, they are internally connected, and thus we can >>> easily avoid two jobs running in parallel. >>> >> That is what the mpp service did in my patches, handing the relationship between each devices. And it is not a easy work, maybe a 4k decoder would be blocked by another high frame rate encoding work or another decoder session. The vendor kernel have more worry about this, but not in this version. > > Right. That is one way to design it. Another way is having a single > devicetree node for the VPU encoder/decoder "complex". No, you can’t assume which one is in the combo group, it can be various. you see, in the rk3328, the vdpu is paired with an avs+ decoder. That is why I use a virtual device standing for scheduler. > > Thanks for the input! > -- > Ezequiel García, VanguardiaSur > www.vanguardiasur.com.ar
On Mon, Jan 7, 2019 at 2:30 AM Ayaka <ayaka@soulik.info> wrote: > > Hello Ezequiel > > Sent from my iPad > > > On Jan 7, 2019, at 1:21 AM, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: > > > >> On Sun, 6 Jan 2019 at 13:16, Ayaka <ayaka@soulik.info> wrote: > >> > >> > >> > >> Sent from my iPad > >> > >>> On Jan 7, 2019, at 12:04 AM, Ezequiel Garcia <ezequiel@collabora.com> wrote: > >>> > >>> On Sun, 2019-01-06 at 23:05 +0800, Ayaka wrote: > >>>>> On Jan 6, 2019, at 10:22 PM, Ezequiel Garcia <ezequiel@collabora.com> wrote: > >>>>> > >>>>> Hi Randy, > >>>>> > >>>>> Thanks a lot for this patches. They are really useful > >>>>> to provide more insight into the VPU hardware. > >>>>> > >>>>> This change will make the vpu encoder and vpu decoder > >>>>> completely independent, can they really work in parallel? > >>>> As I said it depends on the platform, but with this patch, the user space would think they can work at the same time. > >>> > >>> > >>> I think there is some confusion. > >>> > >>> The devicetree is one thing: it is a hardware representation, > >>> a way to describe the hardware, for the kernel/bootloader to > >>> parse. > >>> > >>> The userspace view will depend on the driver implementation. > >>> > >>> The current devicetree and driver (without your patches), > >>> model the VPU as a single piece of hardware, exposing a decoder > >>> and an encoder. > >>> > >>> The V4L driver will then create two video devices, i.e. /dev/videoX > >>> and /dev/videoY. So userspace sees an independent view of the > >>> devices. > >>> > >> I knew that, the problem is that the driver should not always create a decoder and encoder pair, they may not exist at some platforms, even some platforms doesn’t have a encoder. You may have a look on the rk3328 I post on the first email as example. > > > > That is correct. But that still doesn't tackle my question: is the > > hardware able to run a decoding and an encoding job in parallel? > > > For rk3328, yes, you see I didn’t draw them in the same box. > > If not, then it's wrong to describe them as independent entities. > > > >>> However, they are internally connected, and thus we can > >>> easily avoid two jobs running in parallel. > >>> > >> That is what the mpp service did in my patches, handing the relationship between each devices. And it is not a easy work, maybe a 4k decoder would be blocked by another high frame rate encoding work or another decoder session. The vendor kernel have more worry about this, but not in this version. > > > > Right. That is one way to design it. Another way is having a single > > devicetree node for the VPU encoder/decoder "complex". > No, you can’t assume which one is in the combo group, it can be various. you see, in the rk3328, the vdpu is paired with an avs+ decoder. That is why I use a virtual device standing for scheduler. First of all, thanks for all the input. Having more understanding of the hardware and shortcomings of the current V4L2 APIs is really important to let us further evolve the API and make sure that it works for further use cases. As for the Device Tree itself, it doesn't always describe the hardware in 100%. Most of the time it's just the necessary information to choose and instantiate the right drivers and bind to the right hardware resources. The information on which hardware instances on the SoC can work independently can of course be described in DT (e.g. by sub-nodes of a video-codec complex OR a set of phandles, e.g. rockchip,shared-instances), but it's also perfectly fine to defer this kind of knowledge to the drivers themselves. Best regards, Tomasz
Sent from my iPad > On Jan 8, 2019, at 2:33 PM, Tomasz Figa <tfiga@chromium.org> wrote: > >> On Mon, Jan 7, 2019 at 2:30 AM Ayaka <ayaka@soulik.info> wrote: >> >> Hello Ezequiel >> >> Sent from my iPad >> >>>> On Jan 7, 2019, at 1:21 AM, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: >>>> >>>> On Sun, 6 Jan 2019 at 13:16, Ayaka <ayaka@soulik.info> wrote: >>>> >>>> >>>> >>>> Sent from my iPad >>>> >>>>> On Jan 7, 2019, at 12:04 AM, Ezequiel Garcia <ezequiel@collabora.com> wrote: >>>>> >>>>> On Sun, 2019-01-06 at 23:05 +0800, Ayaka wrote: >>>>>>> On Jan 6, 2019, at 10:22 PM, Ezequiel Garcia <ezequiel@collabora.com> wrote: >>>>>>> >>>>>>> Hi Randy, >>>>>>> >>>>>>> Thanks a lot for this patches. They are really useful >>>>>>> to provide more insight into the VPU hardware. >>>>>>> >>>>>>> This change will make the vpu encoder and vpu decoder >>>>>>> completely independent, can they really work in parallel? >>>>>> As I said it depends on the platform, but with this patch, the user space would think they can work at the same time. >>>>> >>>>> >>>>> I think there is some confusion. >>>>> >>>>> The devicetree is one thing: it is a hardware representation, >>>>> a way to describe the hardware, for the kernel/bootloader to >>>>> parse. >>>>> >>>>> The userspace view will depend on the driver implementation. >>>>> >>>>> The current devicetree and driver (without your patches), >>>>> model the VPU as a single piece of hardware, exposing a decoder >>>>> and an encoder. >>>>> >>>>> The V4L driver will then create two video devices, i.e. /dev/videoX >>>>> and /dev/videoY. So userspace sees an independent view of the >>>>> devices. >>>>> >>>> I knew that, the problem is that the driver should not always create a decoder and encoder pair, they may not exist at some platforms, even some platforms doesn’t have a encoder. You may have a look on the rk3328 I post on the first email as example. >>> >>> That is correct. But that still doesn't tackle my question: is the >>> hardware able to run a decoding and an encoding job in parallel? >>> >> For rk3328, yes, you see I didn’t draw them in the same box. >>> If not, then it's wrong to describe them as independent entities. >>> >>>>> However, they are internally connected, and thus we can >>>>> easily avoid two jobs running in parallel. >>>>> >>>> That is what the mpp service did in my patches, handing the relationship between each devices. And it is not a easy work, maybe a 4k decoder would be blocked by another high frame rate encoding work or another decoder session. The vendor kernel have more worry about this, but not in this version. >>> >>> Right. That is one way to design it. Another way is having a single >>> devicetree node for the VPU encoder/decoder "complex". >> No, you can’t assume which one is in the combo group, it can be various. you see, in the rk3328, the vdpu is paired with an avs+ decoder. That is why I use a virtual device standing for scheduler. > > First of all, thanks for all the input. Having more understanding of > the hardware and shortcomings of the current V4L2 APIs is really > important to let us further evolve the API and make sure that it works > for further use cases. I replied the problems of the v4l2 request API in the other threads. I am waiting the feedback from those threads. > > As for the Device Tree itself, it doesn't always describe the hardware > in 100%. Also please note the merged device tree for the video codec won’t fix for most of the rockchip platform. > Most of the time it's just the necessary information to > choose and instantiate the right drivers and bind to the right > hardware resources. The information on which hardware instances on the > SoC can work independently can of course be described in DT (e.g. by > sub-nodes of a video-codec complex OR a set of phandles, e.g. > rockchip,shared-instances), but it's also perfectly fine to defer this > kind of knowledge to the drivers themselves. I wish there is a common mechanism for those device would share some resources. Although there is a multiple functions framework, but that is not I want. They are multiple functions but they are used at the same time not separately. > > Best regards, > Tomasz
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi index 946d3589575a..c3db878bae45 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi @@ -632,6 +632,35 @@ dr_mode = "host"; }; +&rkvdec { + status = "okay"; +}; + +&rkvdec_srv { + status = "okay"; +}; + +&vdec_mmu { + status = "okay"; +}; + +&vdpu { + status = "okay"; +}; + +&vepu { + status = "okay"; +}; + +&vpu_service { + status = "okay"; +}; + +&vpu_mmu { + status = "okay"; + +}; + &vopb { status = "okay"; }; diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index b22b2e40422b..5fa3247e7bf0 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi @@ -1242,16 +1242,39 @@ status = "disabled"; }; - vpu: video-codec@ff650000 { - compatible = "rockchip,rk3399-vpu"; - reg = <0x0 0xff650000 0x0 0x800>; - interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>, - <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; - interrupt-names = "vepu", "vdpu"; + vpu_service: vpu-srv { + compatible = "rockchip,mpp-service"; + status = "disabled"; + }; + + vepu: vpu-encoder@ff650000 { + compatible = "rockchip,vpu-encoder-v2"; + reg = <0x0 0xff650000 0x0 0x400>; + interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH 0>; + interrupt-names = "irq_enc"; clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; - clock-names = "aclk", "hclk"; + clock-names = "aclk_vcodec", "hclk_vcodec"; + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; + reset-names = "video_h", "video_a"; iommus = <&vpu_mmu>; power-domains = <&power RK3399_PD_VCODEC>; + rockchip,srv = <&vpu_service>; + status = "disabled"; + }; + + vdpu: vpu-decoder@ff650400 { + compatible = "rockchip,vpu-decoder-v2"; + reg = <0x0 0xff650400 0x0 0x400>; + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH 0>; + interrupt-names = "irq_dec"; + iommus = <&vpu_mmu>; + clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; + clock-names = "aclk_vcodec", "hclk_vcodec"; + resets = <&cru SRST_H_VCODEC>, <&cru SRST_A_VCODEC>; + reset-names = "video_h", "video_a"; + power-domains = <&power RK3399_PD_VCODEC>; + rockchip,srv = <&vpu_service>; + status = "disabled"; }; vpu_mmu: iommu@ff650800 { @@ -1261,11 +1284,42 @@ interrupt-names = "vpu_mmu"; clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>; clock-names = "aclk", "iface"; + assigned-clocks = <&cru ACLK_VCODEC_PRE>; + assigned-clock-parents = <&cru PLL_GPLL>; #iommu-cells = <0>; power-domains = <&power RK3399_PD_VCODEC>; status = "disabled"; }; + rkvdec_srv: rkvdec-srv { + compatible = "rockchip,mpp-service"; + status = "disabled"; + }; + + rkvdec: video-decoder@ff660000 { + compatible = "rockchip,video-decoder-v1"; + reg = <0x0 0xff660000 0x0 0x400>; + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH 0>; + interrupt-names = "irq_dec"; + clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>, + <&cru SCLK_VDU_CA>, <&cru SCLK_VDU_CORE>; + clock-names = "aclk_vcodec", "hclk_vcodec", + "clk_cabac", "clk_core"; + assigned-clocks = <&cru ACLK_VDU_PRE>, <&cru SCLK_VDU_CA>, + <&cru SCLK_VDU_CORE>; + assigned-clock-parents = <&cru PLL_NPLL>, <&cru PLL_NPLL>, + <&cru PLL_NPLL>; + resets = <&cru SRST_H_VDU>, <&cru SRST_A_VDU>, + <&cru SRST_VDU_CORE>, <&cru SRST_VDU_CA>, + <&cru SRST_A_VDU_NOC>, <&cru SRST_H_VDU_NOC>; + reset-names = "video_h", "video_a", "video_core", "video_cabac", + "niu_a", "niu_h"; + power-domains = <&power RK3399_PD_VDU>; + rockchip,srv = <&rkvdec_srv>; + iommus = <&vdec_mmu>; + status = "disabled"; + }; + vdec_mmu: iommu@ff660480 { compatible = "rockchip,iommu"; reg = <0x0 0xff660480 0x0 0x40>, <0x0 0xff6604c0 0x0 0x40>;
It offers an example how a full features video codec should be configured. The original clocks assignment don't look good, if the clocks lower than 300MHZ, most of decoing tasks would suffer from timeout problem, 500MHZ is also a little high for RK3399 running in a stable state. Signed-off-by: Randy Li <ayaka@soulik.info> --- .../boot/dts/rockchip/rk3399-sapphire.dtsi | 29 ++++++++ arch/arm64/boot/dts/rockchip/rk3399.dtsi | 68 +++++++++++++++++-- 2 files changed, 90 insertions(+), 7 deletions(-)