[PATCH 0/4] Rockchip: the vendor video codec for reference

[PATCH 0/4] Rockchip: the vendor video codec for reference

[Randy Li]

Those patches are not for merging, I won't dream on it. As I said in
previous email, this driver is used for checking the status of the other
drivers. I have checked this driver would memory dump, its output looks
well. The reason I didn't use the video output is VOP driver doesn't
work well.

Also I want to offer a reference for those people who want to develop
the V4L2 request API driver for Rockchip and you can use this driver to
comparing the result as well. I should said either the one for sunxi nor
chromium have not reached the vendor production level.

I want to point out the obvious problem of the current V4L2 driver, I
think that is the one I developed years ago and refresh it in the last
year, then it is merged now.

That driver won't aware the parallel problem between the devices. The
video codec in Rockchip is very complex, the decoder and encoder are
NOT paired in some platforms. Also a decoder or encoder can share some
resource with the other decoder or encoder, requesting a mux in the GRF
device. We call those devices sharing resource a combo, they may or may
not having a individual IOMMU for each of its child devices. Also any of
those devies allowing support less codecs than its full state.

The RK3328 is a good example of that,
---------------------------------------------
| Video decoder for H264, VP8, JPEG, MPEG-4 Part 2
| Video decoder for AVS+
|--------------------------------------------
| Video decoder for H265, H265, VP9
|--------------------------------------------
| Video encoder for H265,
---------------------------------------------
| Video encoder for H264, JPEG
---------------------------------------------
that is why I rewrote the device tree files in these patches, the current
device tree is not suitable for the other platforms.

Besides, the V4L2 driver don't support the error correction and tolerance or
status recovery. That is more about the parser in the userspace but a
common parser won't be able to do that, different video codec vendor
would request a different method, also V4L2 request API is not suitable
to feedback status.

Anyway, the current developing for decoder is at lease acceptable for
those simple situations. But I think encoder would become a big issue,
lucky, it seems that nobody care about encoder. If you have ever looked
the video encoder for chromium, you would feel disgusted, it is not
Google's fault, just not suitable for V4L2.

I don't want to talk much about encoder here, basially the encoder is
much simpler than decoder requesting less dynamic settings, but those
dynamic settings request a more real time feedback or related.

I don't see much advantage the V4L2 brings, only the compatibility left.
The previous vendor driver would have DMA buffer problem but it is
solved in this one. Ndufresne told me about buffer fencing, but I don't
think it is useful only complication. That is why I didn't develop the
V4L2 for a long time.

I would attend the FOSDEM 2019, if you have any problem, I think you can
catch me easily there.

Randy Li (4):
  staging: video: rockchip: video codec for vendor API
  staging: video: rockchip: fixup for upstream
  staging: video: rockchip: add video codec
  arm64: dts: rockchip: add video codec for rk3399

 .../boot/dts/rockchip/rk3399-sapphire.dtsi    |  29 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  68 +-
 drivers/staging/Kconfig                       |   2 +
 drivers/staging/Makefile                      |   1 +
 drivers/staging/rockchip-mpp/Kconfig          |  52 +
 drivers/staging/rockchip-mpp/Makefile         |  16 +
 drivers/staging/rockchip-mpp/mpp_debug.h      |  87 ++
 drivers/staging/rockchip-mpp/mpp_dev_common.c | 971 ++++++++++++++++++
 drivers/staging/rockchip-mpp/mpp_dev_common.h | 219 ++++
 drivers/staging/rockchip-mpp/mpp_dev_rkvdec.c | 855 +++++++++++++++
 drivers/staging/rockchip-mpp/mpp_dev_vdpu1.c  | 614 +++++++++++
 drivers/staging/rockchip-mpp/mpp_dev_vdpu2.c  | 576 +++++++++++
 drivers/staging/rockchip-mpp/mpp_dev_vepu1.c  | 480 +++++++++
 drivers/staging/rockchip-mpp/mpp_dev_vepu2.c  | 477 +++++++++
 drivers/staging/rockchip-mpp/mpp_iommu_dma.c  | 292 ++++++
 drivers/staging/rockchip-mpp/mpp_iommu_dma.h  |  42 +
 drivers/staging/rockchip-mpp/mpp_service.c    | 197 ++++
 drivers/staging/rockchip-mpp/mpp_service.h    |  38 +
 include/uapi/video/rk_vpu_service.h           | 101 ++
 19 files changed, 5110 insertions(+), 7 deletions(-)
 create mode 100644 drivers/staging/rockchip-mpp/Kconfig
 create mode 100644 drivers/staging/rockchip-mpp/Makefile
 create mode 100644 drivers/staging/rockchip-mpp/mpp_debug.h
 create mode 100644 drivers/staging/rockchip-mpp/mpp_dev_common.c
 create mode 100644 drivers/staging/rockchip-mpp/mpp_dev_common.h
 create mode 100644 drivers/staging/rockchip-mpp/mpp_dev_rkvdec.c
 create mode 100644 drivers/staging/rockchip-mpp/mpp_dev_vdpu1.c
 create mode 100644 drivers/staging/rockchip-mpp/mpp_dev_vdpu2.c
 create mode 100644 drivers/staging/rockchip-mpp/mpp_dev_vepu1.c
 create mode 100644 drivers/staging/rockchip-mpp/mpp_dev_vepu2.c
 create mode 100644 drivers/staging/rockchip-mpp/mpp_iommu_dma.c
 create mode 100644 drivers/staging/rockchip-mpp/mpp_iommu_dma.h
 create mode 100644 drivers/staging/rockchip-mpp/mpp_service.c
 create mode 100644 drivers/staging/rockchip-mpp/mpp_service.h
 create mode 100644 include/uapi/video/rk_vpu_service.h

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] staging: video: rockchip: fixup for upstream

[Randy Li]

Fixing those deprecated function from vendor kernel.
Removing those features don't exist in upstream kernel.

Signed-off-by: Randy Li <ayaka@soulik.info>
---
 drivers/staging/rockchip-mpp/mpp_dev_common.c | 12 ++++++------
 drivers/staging/rockchip-mpp/mpp_dev_common.h |  2 +-
 drivers/staging/rockchip-mpp/mpp_dev_rkvdec.c |  7 +++----
 drivers/staging/rockchip-mpp/mpp_dev_vdpu1.c  |  5 ++---
 drivers/staging/rockchip-mpp/mpp_dev_vdpu2.c  |  5 ++---
 drivers/staging/rockchip-mpp/mpp_dev_vepu1.c  |  5 ++---
 drivers/staging/rockchip-mpp/mpp_dev_vepu2.c  |  5 ++---
 7 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/rockchip-mpp/mpp_dev_common.c b/drivers/staging/rockchip-mpp/mpp_dev_common.c
index 159aa5d244ce..9a257c254d4d 100644
--- a/drivers/staging/rockchip-mpp/mpp_dev_common.c
+++ b/drivers/staging/rockchip-mpp/mpp_dev_common.c
@@ -937,18 +937,18 @@ EXPORT_SYMBOL(mpp_dev_read);
 void mpp_debug_time_record(struct mpp_task *task)
 {
 	if (unlikely(debug & DEBUG_TIMING) && task)
-		do_gettimeofday(&task->start);
+		getboottime64(&task->start);
 }
 EXPORT_SYMBOL(mpp_debug_time_record);
 
 void mpp_debug_time_diff(struct mpp_task *task)
 {
-	struct timeval end;
+	struct timespec64 end;
 
-	do_gettimeofday(&end);
-	mpp_debug(DEBUG_TIMING, "time: %ld us\n",
-		  (end.tv_sec  - task->start.tv_sec)  * 1000000 +
-		  (end.tv_usec - task->start.tv_usec));
+	getboottime64(&end);
+	mpp_debug(DEBUG_TIMING, "time: %lld ms\n",
+		  (end.tv_sec  - task->start.tv_sec)  * 1000 +
+		  (end.tv_nsec - task->start.tv_nsec) / 1000000);
 }
 EXPORT_SYMBOL(mpp_debug_time_diff);
 
diff --git a/drivers/staging/rockchip-mpp/mpp_dev_common.h b/drivers/staging/rockchip-mpp/mpp_dev_common.h
index 8a7dc7444dc3..76d53ec2c452 100644
--- a/drivers/staging/rockchip-mpp/mpp_dev_common.h
+++ b/drivers/staging/rockchip-mpp/mpp_dev_common.h
@@ -124,7 +124,7 @@ struct mpp_task {
 	struct work_struct work;
 
 	/* record context running start time */
-	struct timeval start;
+	struct timespec64 start;
 };
 
 /*
diff --git a/drivers/staging/rockchip-mpp/mpp_dev_rkvdec.c b/drivers/staging/rockchip-mpp/mpp_dev_rkvdec.c
index a3da27cfc10e..44d76eba901f 100644
--- a/drivers/staging/rockchip-mpp/mpp_dev_rkvdec.c
+++ b/drivers/staging/rockchip-mpp/mpp_dev_rkvdec.c
@@ -19,10 +19,10 @@
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/of_platform.h>
-#include <linux/rockchip/rockchip_sip.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <soc/rockchip/pm_domains.h>
+#include <soc/rockchip/rockchip_sip.h>
 
 #include "mpp_debug.h"
 #include "mpp_dev_common.h"
@@ -230,8 +230,7 @@ static int fill_scaling_list_pps(struct rkvdec_task *task, int fd, int offset,
 		return -ENOENT;
 	}
 
-	ret = dma_buf_begin_cpu_access(dmabuf, 0, dmabuf->size,
-				       DMA_FROM_DEVICE);
+	ret = dma_buf_begin_cpu_access(dmabuf, DMA_FROM_DEVICE);
 	if (ret) {
 		dev_err(dev, "can't access the pps buffer\n");
 		goto done;
@@ -277,7 +276,7 @@ static int fill_scaling_list_pps(struct rkvdec_task *task, int fd, int offset,
 
 done:
 	dma_buf_vunmap(dmabuf, vaddr);
-	dma_buf_end_cpu_access(dmabuf, 0, dmabuf->size, DMA_FROM_DEVICE);
+	dma_buf_end_cpu_access(dmabuf, DMA_FROM_DEVICE);
 	dma_buf_put(dmabuf);
 
 	return ret;
diff --git a/drivers/staging/rockchip-mpp/mpp_dev_vdpu1.c b/drivers/staging/rockchip-mpp/mpp_dev_vdpu1.c
index 4371a1a6080b..63ffd79bfe83 100644
--- a/drivers/staging/rockchip-mpp/mpp_dev_vdpu1.c
+++ b/drivers/staging/rockchip-mpp/mpp_dev_vdpu1.c
@@ -460,9 +460,8 @@ static int rockchip_mpp_rkvdpu_assign_reset(struct rockchip_rkvdpu_dev *dec_dev)
 {
 	struct rockchip_mpp_dev *mpp_dev = &dec_dev->mpp_dev;
 
-	/* TODO: use devm_reset_control_get_share() instead */
-	dec_dev->rst_a = devm_reset_control_get(mpp_dev->dev, "video_a");
-	dec_dev->rst_h = devm_reset_control_get(mpp_dev->dev, "video_h");
+	dec_dev->rst_a = devm_reset_control_get_shared(mpp_dev->dev, "video_a");
+	dec_dev->rst_h = devm_reset_control_get_shared(mpp_dev->dev, "video_h");
 
 	if (IS_ERR_OR_NULL(dec_dev->rst_a)) {
 		mpp_err("No aclk reset resource define\n");
diff --git a/drivers/staging/rockchip-mpp/mpp_dev_vdpu2.c b/drivers/staging/rockchip-mpp/mpp_dev_vdpu2.c
index b131790f72a3..5789c8940543 100644
--- a/drivers/staging/rockchip-mpp/mpp_dev_vdpu2.c
+++ b/drivers/staging/rockchip-mpp/mpp_dev_vdpu2.c
@@ -428,9 +428,8 @@ static int rockchip_mpp_rkvdpu_assign_reset(struct rockchip_rkvdpu_dev *dec_dev)
 {
 	struct rockchip_mpp_dev *mpp_dev = &dec_dev->mpp_dev;
 
-	/* TODO: use devm_reset_control_get_share() instead */
-	dec_dev->rst_a = devm_reset_control_get(mpp_dev->dev, "video_a");
-	dec_dev->rst_h = devm_reset_control_get(mpp_dev->dev, "video_h");
+	dec_dev->rst_a = devm_reset_control_get_shared(mpp_dev->dev, "video_a");
+	dec_dev->rst_h = devm_reset_control_get_shared(mpp_dev->dev, "video_h");
 
 	if (IS_ERR_OR_NULL(dec_dev->rst_a)) {
 		mpp_err("No aclk reset resource define\n");
diff --git a/drivers/staging/rockchip-mpp/mpp_dev_vepu1.c b/drivers/staging/rockchip-mpp/mpp_dev_vepu1.c
index 64619092c792..ebe8174e931e 100644
--- a/drivers/staging/rockchip-mpp/mpp_dev_vepu1.c
+++ b/drivers/staging/rockchip-mpp/mpp_dev_vepu1.c
@@ -330,9 +330,8 @@ static int rockchip_mpp_rkvepu_assign_reset(struct rockchip_rkvepu_dev *enc_dev)
 {
 	struct rockchip_mpp_dev *mpp_dev = &enc_dev->mpp_dev;
 
-	/* TODO: use devm_reset_control_get_share() instead */
-	enc_dev->rst_a = devm_reset_control_get(mpp_dev->dev, "video_a");
-	enc_dev->rst_h = devm_reset_control_get(mpp_dev->dev, "video_h");
+	enc_dev->rst_a = devm_reset_control_get_shared(mpp_dev->dev, "video_a");
+	enc_dev->rst_h = devm_reset_control_get_shared(mpp_dev->dev, "video_h");
 
 	if (IS_ERR_OR_NULL(enc_dev->rst_a)) {
 		mpp_err("No aclk reset resource define\n");
diff --git a/drivers/staging/rockchip-mpp/mpp_dev_vepu2.c b/drivers/staging/rockchip-mpp/mpp_dev_vepu2.c
index 48ec401145d5..889aaccf25c5 100644
--- a/drivers/staging/rockchip-mpp/mpp_dev_vepu2.c
+++ b/drivers/staging/rockchip-mpp/mpp_dev_vepu2.c
@@ -327,9 +327,8 @@ static int rockchip_mpp_rkvepu_assign_reset(struct rockchip_rkvepu_dev *enc_dev)
 {
 	struct rockchip_mpp_dev *mpp_dev = &enc_dev->mpp_dev;
 
-	/* TODO: use devm_reset_control_get_share() instead */
-	enc_dev->rst_a = devm_reset_control_get(mpp_dev->dev, "video_a");
-	enc_dev->rst_h = devm_reset_control_get(mpp_dev->dev, "video_h");
+	enc_dev->rst_a = devm_reset_control_get_shared(mpp_dev->dev, "video_a");
+	enc_dev->rst_h = devm_reset_control_get_shared(mpp_dev->dev, "video_h");
 
 	if (IS_ERR_OR_NULL(enc_dev->rst_a)) {
 		mpp_err("No aclk reset resource define\n");
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] staging: video: rockchip: add video codec

[Randy Li]

Signed-off-by: Randy Li <ayaka@soulik.info>
---
 drivers/staging/Kconfig  | 2 ++
 drivers/staging/Makefile | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index e4f608815c05..81634dd0a283 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -122,4 +122,6 @@ source "drivers/staging/axis-fifo/Kconfig"
 
 source "drivers/staging/erofs/Kconfig"
 
+source "drivers/staging/rockchip-mpp/Kconfig"
+
 endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 5868631e8f1b..22499c68c21e 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -51,3 +51,4 @@ obj-$(CONFIG_SOC_MT7621)	+= mt7621-dts/
 obj-$(CONFIG_STAGING_GASKET_FRAMEWORK)	+= gasket/
 obj-$(CONFIG_XIL_AXIS_FIFO)	+= axis-fifo/
 obj-$(CONFIG_EROFS_FS)		+= erofs/
+obj-$(CONFIG_ROCKCHIP_MPP_SERVICE)	+= rockchip-mpp/
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Randy Li]

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>;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Ezequiel Garcia]

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>;



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Ayaka]

> 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>;
> 
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Ayaka]

> 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>;
> 
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Ezequiel Garcia]

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>;



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Ayaka]

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>;
> 
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Ezequiel Garcia]

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!
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Ayaka]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Tomasz Figa]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: rockchip: add video codec for rk3399

[Ayaka]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel