[PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[Paul Kocialkowski]

Hi,

This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
It also contains a fix for the YUV2YUV case that was not properly handled.

Cheers,

Paul

Paul Kocialkowski (4):
  dt-bindings: rockchip-rga: Add PX30 compatible
  arm64: dts: rockchip: Add RGA support to the PX30
  media: rockchip: rga: Add support for the PX30 compatible
  media: rockchip: rga: Only set output CSC mode for RGB input

 .../devicetree/bindings/media/rockchip-rga.txt |  1 +
 arch/arm64/boot/dts/rockchip/px30.dtsi         | 11 +++++++++++
 drivers/media/platform/rockchip/rga/rga-hw.c   | 18 +++++++++++-------
 drivers/media/platform/rockchip/rga/rga.c      |  4 +++-
 4 files changed, 26 insertions(+), 8 deletions(-)

-- 
2.26.0

[PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[Paul Kocialkowski]

Hi,

This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
It also contains a fix for the YUV2YUV case that was not properly handled.

Cheers,

Paul

Paul Kocialkowski (4):
  dt-bindings: rockchip-rga: Add PX30 compatible
  arm64: dts: rockchip: Add RGA support to the PX30
  media: rockchip: rga: Add support for the PX30 compatible
  media: rockchip: rga: Only set output CSC mode for RGB input

 .../devicetree/bindings/media/rockchip-rga.txt |  1 +
 arch/arm64/boot/dts/rockchip/px30.dtsi         | 11 +++++++++++
 drivers/media/platform/rockchip/rga/rga-hw.c   | 18 +++++++++++-------
 drivers/media/platform/rockchip/rga/rga.c      |  4 +++-
 4 files changed, 26 insertions(+), 8 deletions(-)

-- 
2.26.0

[PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[Paul Kocialkowski]

Hi,

This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
It also contains a fix for the YUV2YUV case that was not properly handled.

Cheers,

Paul

Paul Kocialkowski (4):
  dt-bindings: rockchip-rga: Add PX30 compatible
  arm64: dts: rockchip: Add RGA support to the PX30
  media: rockchip: rga: Add support for the PX30 compatible
  media: rockchip: rga: Only set output CSC mode for RGB input

 .../devicetree/bindings/media/rockchip-rga.txt |  1 +
 arch/arm64/boot/dts/rockchip/px30.dtsi         | 11 +++++++++++
 drivers/media/platform/rockchip/rga/rga-hw.c   | 18 +++++++++++-------
 drivers/media/platform/rockchip/rga/rga.c      |  4 +++-
 4 files changed, 26 insertions(+), 8 deletions(-)

-- 
2.26.0


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

[PATCH 1/4] dt-bindings: rockchip-rga: Add PX30 compatible

[Paul Kocialkowski]

Add a new compatible for the PX30 Rockchip SoC, which also features
a RGA block.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 Documentation/devicetree/bindings/media/rockchip-rga.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.txt b/Documentation/devicetree/bindings/media/rockchip-rga.txt
index fd5276abfad6..45a9effd05f7 100644
--- a/Documentation/devicetree/bindings/media/rockchip-rga.txt
+++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
@@ -6,6 +6,7 @@ BitBLT, alpha blending and image blur/sharpness.
 
 Required properties:
 - compatible: value should be one of the following
+		"rockchip,px30-rga";
 		"rockchip,rk3288-rga";
 		"rockchip,rk3399-rga";
 
-- 
2.26.0

[PATCH 1/4] dt-bindings: rockchip-rga: Add PX30 compatible

[Paul Kocialkowski]

Add a new compatible for the PX30 Rockchip SoC, which also features
a RGA block.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 Documentation/devicetree/bindings/media/rockchip-rga.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.txt b/Documentation/devicetree/bindings/media/rockchip-rga.txt
index fd5276abfad6..45a9effd05f7 100644
--- a/Documentation/devicetree/bindings/media/rockchip-rga.txt
+++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
@@ -6,6 +6,7 @@ BitBLT, alpha blending and image blur/sharpness.
 
 Required properties:
 - compatible: value should be one of the following
+		"rockchip,px30-rga";
 		"rockchip,rk3288-rga";
 		"rockchip,rk3399-rga";
 
-- 
2.26.0

[PATCH 1/4] dt-bindings: rockchip-rga: Add PX30 compatible

[Paul Kocialkowski]

Add a new compatible for the PX30 Rockchip SoC, which also features
a RGA block.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 Documentation/devicetree/bindings/media/rockchip-rga.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.txt b/Documentation/devicetree/bindings/media/rockchip-rga.txt
index fd5276abfad6..45a9effd05f7 100644
--- a/Documentation/devicetree/bindings/media/rockchip-rga.txt
+++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
@@ -6,6 +6,7 @@ BitBLT, alpha blending and image blur/sharpness.
 
 Required properties:
 - compatible: value should be one of the following
+		"rockchip,px30-rga";
 		"rockchip,rk3288-rga";
 		"rockchip,rk3399-rga";
 
-- 
2.26.0


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

[PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

The PX30 features a RGA block: add the necessary node to support it.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index 75908c587511..4bfbee9d4123 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
 		status = "disabled";
 	};
 
+	rga: rga@ff480000 {
+		compatible = "rockchip,px30-rga";
+		reg = <0x0 0xff480000 0x0 0x10000>;
+		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
+		clock-names = "aclk", "hclk", "sclk";
+		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
+		reset-names = "core", "axi", "ahb";
+		power-domains = <&power PX30_PD_VO>;
+	};
+
 	qos_gmac: qos@ff518000 {
 		compatible = "syscon";
 		reg = <0x0 0xff518000 0x0 0x20>;
-- 
2.26.0

[PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

The PX30 features a RGA block: add the necessary node to support it.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index 75908c587511..4bfbee9d4123 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
 		status = "disabled";
 	};
 
+	rga: rga@ff480000 {
+		compatible = "rockchip,px30-rga";
+		reg = <0x0 0xff480000 0x0 0x10000>;
+		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
+		clock-names = "aclk", "hclk", "sclk";
+		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
+		reset-names = "core", "axi", "ahb";
+		power-domains = <&power PX30_PD_VO>;
+	};
+
 	qos_gmac: qos@ff518000 {
 		compatible = "syscon";
 		reg = <0x0 0xff518000 0x0 0x20>;
-- 
2.26.0

[PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

The PX30 features a RGA block: add the necessary node to support it.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index 75908c587511..4bfbee9d4123 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
 		status = "disabled";
 	};
 
+	rga: rga@ff480000 {
+		compatible = "rockchip,px30-rga";
+		reg = <0x0 0xff480000 0x0 0x10000>;
+		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
+		clock-names = "aclk", "hclk", "sclk";
+		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
+		reset-names = "core", "axi", "ahb";
+		power-domains = <&power PX30_PD_VO>;
+	};
+
 	qos_gmac: qos@ff518000 {
 		compatible = "syscon";
 		reg = <0x0 0xff518000 0x0 0x20>;
-- 
2.26.0


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

[PATCH 3/4] media: rockchip: rga: Add support for the PX30 compatible

[Paul Kocialkowski]

The PX30 SoC has a RGA block, so add the associated compatible to
support it.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/platform/rockchip/rga/rga.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index e9ff12b6b5bb..0ebeb9b4c14a 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -880,7 +880,6 @@ static int rga_probe(struct platform_device *pdev)
 	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
 					   &rga->cmdbuf_phy, GFP_KERNEL,
 					   DMA_ATTR_WRITE_COMBINE);
-
 	rga->src_mmu_pages =
 		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
 	rga->dst_mmu_pages =
@@ -955,6 +954,9 @@ static const struct dev_pm_ops rga_pm = {
 };
 
 static const struct of_device_id rockchip_rga_match[] = {
+	{
+		.compatible = "rockchip,px30-rga",
+	},
 	{
 		.compatible = "rockchip,rk3288-rga",
 	},
-- 
2.26.0

[PATCH 3/4] media: rockchip: rga: Add support for the PX30 compatible

[Paul Kocialkowski]

The PX30 SoC has a RGA block, so add the associated compatible to
support it.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/platform/rockchip/rga/rga.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index e9ff12b6b5bb..0ebeb9b4c14a 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -880,7 +880,6 @@ static int rga_probe(struct platform_device *pdev)
 	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
 					   &rga->cmdbuf_phy, GFP_KERNEL,
 					   DMA_ATTR_WRITE_COMBINE);
-
 	rga->src_mmu_pages =
 		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
 	rga->dst_mmu_pages =
@@ -955,6 +954,9 @@ static const struct dev_pm_ops rga_pm = {
 };
 
 static const struct of_device_id rockchip_rga_match[] = {
+	{
+		.compatible = "rockchip,px30-rga",
+	},
 	{
 		.compatible = "rockchip,rk3288-rga",
 	},
-- 
2.26.0

[PATCH 3/4] media: rockchip: rga: Add support for the PX30 compatible

[Paul Kocialkowski]

The PX30 SoC has a RGA block, so add the associated compatible to
support it.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/platform/rockchip/rga/rga.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index e9ff12b6b5bb..0ebeb9b4c14a 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -880,7 +880,6 @@ static int rga_probe(struct platform_device *pdev)
 	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
 					   &rga->cmdbuf_phy, GFP_KERNEL,
 					   DMA_ATTR_WRITE_COMBINE);
-
 	rga->src_mmu_pages =
 		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
 	rga->dst_mmu_pages =
@@ -955,6 +954,9 @@ static const struct dev_pm_ops rga_pm = {
 };
 
 static const struct of_device_id rockchip_rga_match[] = {
+	{
+		.compatible = "rockchip,px30-rga",
+	},
 	{
 		.compatible = "rockchip,rk3288-rga",
 	},
-- 
2.26.0


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

[PATCH 4/4] media: rockchip: rga: Only set output CSC mode for RGB input

[Paul Kocialkowski]

Setting the output CSC mode is required for a YUV output, but must not
be set when the input is also YUV. Doing this (as tested with a YUV420P
to YUV420P conversion) results in wrong colors.

Adapt the logic to only set the CSC mode when the output is YUV and the
input is RGB.

Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/platform/rockchip/rga/rga-hw.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
index 4be6dcf292ff..cbffcf986ccf 100644
--- a/drivers/media/platform/rockchip/rga/rga-hw.c
+++ b/drivers/media/platform/rockchip/rga/rga-hw.c
@@ -216,13 +216,17 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 	}
 
 	if (ctx->out.fmt->hw_format >= RGA_COLOR_FMT_YUV422SP) {
-		switch (ctx->out.colorspace) {
-		case V4L2_COLORSPACE_REC709:
-			dst_info.data.csc_mode = RGA_SRC_CSC_MODE_BT709_R0;
-			break;
-		default:
-			dst_info.data.csc_mode = RGA_DST_CSC_MODE_BT601_R0;
-			break;
+		if (ctx->in.fmt->hw_format < RGA_COLOR_FMT_YUV422SP) {
+			switch (ctx->out.colorspace) {
+			case V4L2_COLORSPACE_REC709:
+				dst_info.data.csc_mode =
+					RGA_SRC_CSC_MODE_BT709_R0;
+				break;
+			default:
+				dst_info.data.csc_mode =
+					RGA_DST_CSC_MODE_BT601_R0;
+				break;
+			}
 		}
 	}
 
-- 
2.26.0

[PATCH 4/4] media: rockchip: rga: Only set output CSC mode for RGB input

[Paul Kocialkowski]

Setting the output CSC mode is required for a YUV output, but must not
be set when the input is also YUV. Doing this (as tested with a YUV420P
to YUV420P conversion) results in wrong colors.

Adapt the logic to only set the CSC mode when the output is YUV and the
input is RGB.

Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/platform/rockchip/rga/rga-hw.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
index 4be6dcf292ff..cbffcf986ccf 100644
--- a/drivers/media/platform/rockchip/rga/rga-hw.c
+++ b/drivers/media/platform/rockchip/rga/rga-hw.c
@@ -216,13 +216,17 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 	}
 
 	if (ctx->out.fmt->hw_format >= RGA_COLOR_FMT_YUV422SP) {
-		switch (ctx->out.colorspace) {
-		case V4L2_COLORSPACE_REC709:
-			dst_info.data.csc_mode = RGA_SRC_CSC_MODE_BT709_R0;
-			break;
-		default:
-			dst_info.data.csc_mode = RGA_DST_CSC_MODE_BT601_R0;
-			break;
+		if (ctx->in.fmt->hw_format < RGA_COLOR_FMT_YUV422SP) {
+			switch (ctx->out.colorspace) {
+			case V4L2_COLORSPACE_REC709:
+				dst_info.data.csc_mode =
+					RGA_SRC_CSC_MODE_BT709_R0;
+				break;
+			default:
+				dst_info.data.csc_mode =
+					RGA_DST_CSC_MODE_BT601_R0;
+				break;
+			}
 		}
 	}
 
-- 
2.26.0

[PATCH 4/4] media: rockchip: rga: Only set output CSC mode for RGB input

[Paul Kocialkowski]

Setting the output CSC mode is required for a YUV output, but must not
be set when the input is also YUV. Doing this (as tested with a YUV420P
to YUV420P conversion) results in wrong colors.

Adapt the logic to only set the CSC mode when the output is YUV and the
input is RGB.

Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/platform/rockchip/rga/rga-hw.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
index 4be6dcf292ff..cbffcf986ccf 100644
--- a/drivers/media/platform/rockchip/rga/rga-hw.c
+++ b/drivers/media/platform/rockchip/rga/rga-hw.c
@@ -216,13 +216,17 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 	}
 
 	if (ctx->out.fmt->hw_format >= RGA_COLOR_FMT_YUV422SP) {
-		switch (ctx->out.colorspace) {
-		case V4L2_COLORSPACE_REC709:
-			dst_info.data.csc_mode = RGA_SRC_CSC_MODE_BT709_R0;
-			break;
-		default:
-			dst_info.data.csc_mode = RGA_DST_CSC_MODE_BT601_R0;
-			break;
+		if (ctx->in.fmt->hw_format < RGA_COLOR_FMT_YUV422SP) {
+			switch (ctx->out.colorspace) {
+			case V4L2_COLORSPACE_REC709:
+				dst_info.data.csc_mode =
+					RGA_SRC_CSC_MODE_BT709_R0;
+				break;
+			default:
+				dst_info.data.csc_mode =
+					RGA_DST_CSC_MODE_BT601_R0;
+				break;
+			}
 		}
 	}
 
-- 
2.26.0


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

Re: [PATCH 3/4] media: rockchip: rga: Add support for the PX30 compatible

[Hans Verkuil]

On 16/04/2020 13:50, Paul Kocialkowski wrote:
> The PX30 SoC has a RGA block, so add the associated compatible to
> support it.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/media/platform/rockchip/rga/rga.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index e9ff12b6b5bb..0ebeb9b4c14a 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -880,7 +880,6 @@ static int rga_probe(struct platform_device *pdev)
>  	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
>  					   &rga->cmdbuf_phy, GFP_KERNEL,
>  					   DMA_ATTR_WRITE_COMBINE);
> -

Spurious change?

Regards,

	Hans

>  	rga->src_mmu_pages =
>  		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
>  	rga->dst_mmu_pages =
> @@ -955,6 +954,9 @@ static const struct dev_pm_ops rga_pm = {
>  };
>  
>  static const struct of_device_id rockchip_rga_match[] = {
> +	{
> +		.compatible = "rockchip,px30-rga",
> +	},
>  	{
>  		.compatible = "rockchip,rk3288-rga",
>  	},
> 

Re: [PATCH 3/4] media: rockchip: rga: Add support for the PX30 compatible

[Hans Verkuil]

On 16/04/2020 13:50, Paul Kocialkowski wrote:
> The PX30 SoC has a RGA block, so add the associated compatible to
> support it.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/media/platform/rockchip/rga/rga.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index e9ff12b6b5bb..0ebeb9b4c14a 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -880,7 +880,6 @@ static int rga_probe(struct platform_device *pdev)
>  	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
>  					   &rga->cmdbuf_phy, GFP_KERNEL,
>  					   DMA_ATTR_WRITE_COMBINE);
> -

Spurious change?

Regards,

	Hans

>  	rga->src_mmu_pages =
>  		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
>  	rga->dst_mmu_pages =
> @@ -955,6 +954,9 @@ static const struct dev_pm_ops rga_pm = {
>  };
>  
>  static const struct of_device_id rockchip_rga_match[] = {
> +	{
> +		.compatible = "rockchip,px30-rga",
> +	},
>  	{
>  		.compatible = "rockchip,rk3288-rga",
>  	},
> 

Re: [PATCH 3/4] media: rockchip: rga: Add support for the PX30 compatible

[Hans Verkuil]

On 16/04/2020 13:50, Paul Kocialkowski wrote:
> The PX30 SoC has a RGA block, so add the associated compatible to
> support it.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/media/platform/rockchip/rga/rga.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index e9ff12b6b5bb..0ebeb9b4c14a 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -880,7 +880,6 @@ static int rga_probe(struct platform_device *pdev)
>  	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
>  					   &rga->cmdbuf_phy, GFP_KERNEL,
>  					   DMA_ATTR_WRITE_COMBINE);
> -

Spurious change?

Regards,

	Hans

>  	rga->src_mmu_pages =
>  		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
>  	rga->dst_mmu_pages =
> @@ -955,6 +954,9 @@ static const struct dev_pm_ops rga_pm = {
>  };
>  
>  static const struct of_device_id rockchip_rga_match[] = {
> +	{
> +		.compatible = "rockchip,px30-rga",
> +	},
>  	{
>  		.compatible = "rockchip,rk3288-rga",
>  	},
> 


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

Re: [PATCH 3/4] media: rockchip: rga: Add support for the PX30 compatible

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

Hi,

On Thu 16 Apr 20, 13:58, Hans Verkuil wrote:
> On 16/04/2020 13:50, Paul Kocialkowski wrote:
> > The PX30 SoC has a RGA block, so add the associated compatible to
> > support it.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  drivers/media/platform/rockchip/rga/rga.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> > index e9ff12b6b5bb..0ebeb9b4c14a 100644
> > --- a/drivers/media/platform/rockchip/rga/rga.c
> > +++ b/drivers/media/platform/rockchip/rga/rga.c
> > @@ -880,7 +880,6 @@ static int rga_probe(struct platform_device *pdev)
> >  	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
> >  					   &rga->cmdbuf_phy, GFP_KERNEL,
> >  					   DMA_ATTR_WRITE_COMBINE);
> > -
> 
> Spurious change?

Ah sorry about that. Will fix in v2.

Cheers,

Paul

> Regards,
> 
> 	Hans
> 
> >  	rga->src_mmu_pages =
> >  		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
> >  	rga->dst_mmu_pages =
> > @@ -955,6 +954,9 @@ static const struct dev_pm_ops rga_pm = {
> >  };
> >  
> >  static const struct of_device_id rockchip_rga_match[] = {
> > +	{
> > +		.compatible = "rockchip,px30-rga",
> > +	},
> >  	{
> >  		.compatible = "rockchip,rk3288-rga",
> >  	},
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/4] media: rockchip: rga: Add support for the PX30 compatible

[Paul Kocialkowski]

[-- Attachment #1.1: Type: text/plain, Size: 1499 bytes --]

Hi,

On Thu 16 Apr 20, 13:58, Hans Verkuil wrote:
> On 16/04/2020 13:50, Paul Kocialkowski wrote:
> > The PX30 SoC has a RGA block, so add the associated compatible to
> > support it.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  drivers/media/platform/rockchip/rga/rga.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> > index e9ff12b6b5bb..0ebeb9b4c14a 100644
> > --- a/drivers/media/platform/rockchip/rga/rga.c
> > +++ b/drivers/media/platform/rockchip/rga/rga.c
> > @@ -880,7 +880,6 @@ static int rga_probe(struct platform_device *pdev)
> >  	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
> >  					   &rga->cmdbuf_phy, GFP_KERNEL,
> >  					   DMA_ATTR_WRITE_COMBINE);
> > -
> 
> Spurious change?

Ah sorry about that. Will fix in v2.

Cheers,

Paul

> Regards,
> 
> 	Hans
> 
> >  	rga->src_mmu_pages =
> >  		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
> >  	rga->dst_mmu_pages =
> > @@ -955,6 +954,9 @@ static const struct dev_pm_ops rga_pm = {
> >  };
> >  
> >  static const struct of_device_id rockchip_rga_match[] = {
> > +	{
> > +		.compatible = "rockchip,px30-rga",
> > +	},
> >  	{
> >  		.compatible = "rockchip,rk3288-rga",
> >  	},
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[Ezequiel Garcia]

Hi Paul,

Thanks for the patch.

On Thu, 2020-04-16 at 13:50 +0200, Paul Kocialkowski wrote:
> Hi,
> 
> This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
> It also contains a fix for the YUV2YUV case that was not properly handled.
> 

How have you been testing this?

Thanks,
Ezequiel

> Cheers,
> 
> Paul
> 
> Paul Kocialkowski (4):
>   dt-bindings: rockchip-rga: Add PX30 compatible
>   arm64: dts: rockchip: Add RGA support to the PX30
>   media: rockchip: rga: Add support for the PX30 compatible
>   media: rockchip: rga: Only set output CSC mode for RGB input
> 
>  .../devicetree/bindings/media/rockchip-rga.txt |  1 +
>  arch/arm64/boot/dts/rockchip/px30.dtsi         | 11 +++++++++++
>  drivers/media/platform/rockchip/rga/rga-hw.c   | 18 +++++++++++-------
>  drivers/media/platform/rockchip/rga/rga.c      |  4 +++-
>  4 files changed, 26 insertions(+), 8 deletions(-)
> 

Re: [PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[Ezequiel Garcia]

Hi Paul,

Thanks for the patch.

On Thu, 2020-04-16 at 13:50 +0200, Paul Kocialkowski wrote:
> Hi,
> 
> This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
> It also contains a fix for the YUV2YUV case that was not properly handled.
> 

How have you been testing this?

Thanks,
Ezequiel

> Cheers,
> 
> Paul
> 
> Paul Kocialkowski (4):
>   dt-bindings: rockchip-rga: Add PX30 compatible
>   arm64: dts: rockchip: Add RGA support to the PX30
>   media: rockchip: rga: Add support for the PX30 compatible
>   media: rockchip: rga: Only set output CSC mode for RGB input
> 
>  .../devicetree/bindings/media/rockchip-rga.txt |  1 +
>  arch/arm64/boot/dts/rockchip/px30.dtsi         | 11 +++++++++++
>  drivers/media/platform/rockchip/rga/rga-hw.c   | 18 +++++++++++-------
>  drivers/media/platform/rockchip/rga/rga.c      |  4 +++-
>  4 files changed, 26 insertions(+), 8 deletions(-)
> 

Re: [PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[Ezequiel Garcia]

Hi Paul,

Thanks for the patch.

On Thu, 2020-04-16 at 13:50 +0200, Paul Kocialkowski wrote:
> Hi,
> 
> This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
> It also contains a fix for the YUV2YUV case that was not properly handled.
> 

How have you been testing this?

Thanks,
Ezequiel

> Cheers,
> 
> Paul
> 
> Paul Kocialkowski (4):
>   dt-bindings: rockchip-rga: Add PX30 compatible
>   arm64: dts: rockchip: Add RGA support to the PX30
>   media: rockchip: rga: Add support for the PX30 compatible
>   media: rockchip: rga: Only set output CSC mode for RGB input
> 
>  .../devicetree/bindings/media/rockchip-rga.txt |  1 +
>  arch/arm64/boot/dts/rockchip/px30.dtsi         | 11 +++++++++++
>  drivers/media/platform/rockchip/rga/rga-hw.c   | 18 +++++++++++-------
>  drivers/media/platform/rockchip/rga/rga.c      |  4 +++-
>  4 files changed, 26 insertions(+), 8 deletions(-)
> 



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

Re: [PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 1331 bytes --]

Hi Ezequiel,

On Thu 16 Apr 20, 09:22, Ezequiel Garcia wrote:
> Hi Paul,
> 
> Thanks for the patch.
> 
> On Thu, 2020-04-16 at 13:50 +0200, Paul Kocialkowski wrote:
> > Hi,
> > 
> > This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
> > It also contains a fix for the YUV2YUV case that was not properly handled.
> 
> How have you been testing this?

I tested it with a standalone utility setting V4L2_PIX_FMT_YUV420 on both
output and capture. The issue should be pretty easy to reproduce.

Cheers,

Paul

> Thanks,
> Ezequiel
> 
> > Cheers,
> > 
> > Paul
> > 
> > Paul Kocialkowski (4):
> >   dt-bindings: rockchip-rga: Add PX30 compatible
> >   arm64: dts: rockchip: Add RGA support to the PX30
> >   media: rockchip: rga: Add support for the PX30 compatible
> >   media: rockchip: rga: Only set output CSC mode for RGB input
> > 
> >  .../devicetree/bindings/media/rockchip-rga.txt |  1 +
> >  arch/arm64/boot/dts/rockchip/px30.dtsi         | 11 +++++++++++
> >  drivers/media/platform/rockchip/rga/rga-hw.c   | 18 +++++++++++-------
> >  drivers/media/platform/rockchip/rga/rga.c      |  4 +++-
> >  4 files changed, 26 insertions(+), 8 deletions(-)
> > 
> 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

[Paul Kocialkowski]

[-- Attachment #1.1: Type: text/plain, Size: 1331 bytes --]

Hi Ezequiel,

On Thu 16 Apr 20, 09:22, Ezequiel Garcia wrote:
> Hi Paul,
> 
> Thanks for the patch.
> 
> On Thu, 2020-04-16 at 13:50 +0200, Paul Kocialkowski wrote:
> > Hi,
> > 
> > This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
> > It also contains a fix for the YUV2YUV case that was not properly handled.
> 
> How have you been testing this?

I tested it with a standalone utility setting V4L2_PIX_FMT_YUV420 on both
output and capture. The issue should be pretty easy to reproduce.

Cheers,

Paul

> Thanks,
> Ezequiel
> 
> > Cheers,
> > 
> > Paul
> > 
> > Paul Kocialkowski (4):
> >   dt-bindings: rockchip-rga: Add PX30 compatible
> >   arm64: dts: rockchip: Add RGA support to the PX30
> >   media: rockchip: rga: Add support for the PX30 compatible
> >   media: rockchip: rga: Only set output CSC mode for RGB input
> > 
> >  .../devicetree/bindings/media/rockchip-rga.txt |  1 +
> >  arch/arm64/boot/dts/rockchip/px30.dtsi         | 11 +++++++++++
> >  drivers/media/platform/rockchip/rga/rga-hw.c   | 18 +++++++++++-------
> >  drivers/media/platform/rockchip/rga/rga.c      |  4 +++-
> >  4 files changed, 26 insertions(+), 8 deletions(-)
> > 
> 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

Hi Paul,

The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
has been approved by robh.
Maybe place dts patches at the end of a patch serie.
Could you include a &rga patch if your device is supported in mainline,
so we can test with:
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml

Johan

On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> The PX30 features a RGA block: add the necessary node to support it.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> index 75908c587511..4bfbee9d4123 100644
> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>  		status = "disabled";
>  	};
>  
> +	rga: rga@ff480000 {
> +		compatible = "rockchip,px30-rga";
> +		reg = <0x0 0xff480000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> +		clock-names = "aclk", "hclk", "sclk";
> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> +		reset-names = "core", "axi", "ahb";
> +		power-domains = <&power PX30_PD_VO>;

		status = "disabled";

> +	};
> +
>  	qos_gmac: qos@ff518000 {
>  		compatible = "syscon";
>  		reg = <0x0 0xff518000 0x0 0x20>;
> 

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

Hi Paul,

The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
has been approved by robh.
Maybe place dts patches at the end of a patch serie.
Could you include a &rga patch if your device is supported in mainline,
so we can test with:
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml

Johan

On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> The PX30 features a RGA block: add the necessary node to support it.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> index 75908c587511..4bfbee9d4123 100644
> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>  		status = "disabled";
>  	};
>  
> +	rga: rga@ff480000 {
> +		compatible = "rockchip,px30-rga";
> +		reg = <0x0 0xff480000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> +		clock-names = "aclk", "hclk", "sclk";
> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> +		reset-names = "core", "axi", "ahb";
> +		power-domains = <&power PX30_PD_VO>;

		status = "disabled";

> +	};
> +
>  	qos_gmac: qos@ff518000 {
>  		compatible = "syscon";
>  		reg = <0x0 0xff518000 0x0 0x20>;
> 

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

Hi Paul,

The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
has been approved by robh.
Maybe place dts patches at the end of a patch serie.
Could you include a &rga patch if your device is supported in mainline,
so we can test with:
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml

Johan

On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> The PX30 features a RGA block: add the necessary node to support it.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> index 75908c587511..4bfbee9d4123 100644
> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>  		status = "disabled";
>  	};
>  
> +	rga: rga@ff480000 {
> +		compatible = "rockchip,px30-rga";
> +		reg = <0x0 0xff480000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> +		clock-names = "aclk", "hclk", "sclk";
> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> +		reset-names = "core", "axi", "ahb";
> +		power-domains = <&power PX30_PD_VO>;

		status = "disabled";

> +	};
> +
>  	qos_gmac: qos@ff518000 {
>  		compatible = "syscon";
>  		reg = <0x0 0xff518000 0x0 0x20>;
> 


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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 2315 bytes --]

Hi,

On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> Hi Paul,
> 
> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> has been approved by robh.

Huh, I looked around for ongoing related work but missed it.
I'll definitely rebase on top of your series and use the yaml description
instead. Thanks!

> Maybe place dts patches at the end of a patch serie.
> Could you include a &rga patch if your device is supported in mainline,
> so we can test with:
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml

I tested with the PX30 EVB so I can surely add a node there if that turns
out necessary (see below).

> Johan
> 
> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> > The PX30 features a RGA block: add the necessary node to support it.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > index 75908c587511..4bfbee9d4123 100644
> > --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> >  		status = "disabled";
> >  	};
> >  
> > +	rga: rga@ff480000 {
> > +		compatible = "rockchip,px30-rga";
> > +		reg = <0x0 0xff480000 0x0 0x10000>;
> > +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > +		clock-names = "aclk", "hclk", "sclk";
> > +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > +		reset-names = "core", "axi", "ahb";
> > +		power-domains = <&power PX30_PD_VO>;
> 
> 		status = "disabled";

As of 5.6, the rk3399 has the node enabled by default. Did that change?

Since it's a standalone block that has no I/O dependency, I don't really see
the point of disabling it by default.

What do you think?

Cheers,

Paul

> > +	};
> > +
> >  	qos_gmac: qos@ff518000 {
> >  		compatible = "syscon";
> >  		reg = <0x0 0xff518000 0x0 0x20>;
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

[-- Attachment #1.1: Type: text/plain, Size: 2315 bytes --]

Hi,

On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> Hi Paul,
> 
> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> has been approved by robh.

Huh, I looked around for ongoing related work but missed it.
I'll definitely rebase on top of your series and use the yaml description
instead. Thanks!

> Maybe place dts patches at the end of a patch serie.
> Could you include a &rga patch if your device is supported in mainline,
> so we can test with:
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml

I tested with the PX30 EVB so I can surely add a node there if that turns
out necessary (see below).

> Johan
> 
> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> > The PX30 features a RGA block: add the necessary node to support it.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > index 75908c587511..4bfbee9d4123 100644
> > --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> >  		status = "disabled";
> >  	};
> >  
> > +	rga: rga@ff480000 {
> > +		compatible = "rockchip,px30-rga";
> > +		reg = <0x0 0xff480000 0x0 0x10000>;
> > +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > +		clock-names = "aclk", "hclk", "sclk";
> > +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > +		reset-names = "core", "axi", "ahb";
> > +		power-domains = <&power PX30_PD_VO>;
> 
> 		status = "disabled";

As of 5.6, the rk3399 has the node enabled by default. Did that change?

Since it's a standalone block that has no I/O dependency, I don't really see
the point of disabling it by default.

What do you think?

Cheers,

Paul

> > +	};
> > +
> >  	qos_gmac: qos@ff518000 {
> >  		compatible = "syscon";
> >  		reg = <0x0 0xff518000 0x0 0x20>;
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
>> Hi Paul,
>>
>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
>> has been approved by robh.
> 
> Huh, I looked around for ongoing related work but missed it.
> I'll definitely rebase on top of your series and use the yaml description
> instead. Thanks!
> 
>> Maybe place dts patches at the end of a patch serie.
>> Could you include a &rga patch if your device is supported in mainline,
>> so we can test with:
>> make ARCH=arm64 dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> 
> I tested with the PX30 EVB so I can surely add a node there if that turns
> out necessary (see below).
> 
>> Johan
>>
>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
>>> The PX30 features a RGA block: add the necessary node to support it.
>>>
>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> index 75908c587511..4bfbee9d4123 100644
>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>>>  		status = "disabled";
>>>  	};
>>>  
>>> +	rga: rga@ff480000 {
>>> +		compatible = "rockchip,px30-rga";
>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>> +		clock-names = "aclk", "hclk", "sclk";
>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>> +		reset-names = "core", "axi", "ahb";
>>> +		power-domains = <&power PX30_PD_VO>;
>>
>> 		status = "disabled";
> 
> As of 5.6, the rk3399 has the node enabled by default. Did that change?

'status' disappeared during review for rk3399 between v2 and v3, but
doesn't mention the reason. If someone can give more info here?

https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/

https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/

> 
> Since it's a standalone block that has no I/O dependency, I don't really see
> the point of disabling it by default.

Vop, hdmi and other video devices are also disabled.
Follow the rest I think...

> 
> What do you think?
> 
> Cheers,
> 
> Paul
> 
>>> +	};
>>> +
>>>  	qos_gmac: qos@ff518000 {
>>>  		compatible = "syscon";
>>>  		reg = <0x0 0xff518000 0x0 0x20>;
>>>
>>
> 

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
>> Hi Paul,
>>
>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
>> has been approved by robh.
> 
> Huh, I looked around for ongoing related work but missed it.
> I'll definitely rebase on top of your series and use the yaml description
> instead. Thanks!
> 
>> Maybe place dts patches at the end of a patch serie.
>> Could you include a &rga patch if your device is supported in mainline,
>> so we can test with:
>> make ARCH=arm64 dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> 
> I tested with the PX30 EVB so I can surely add a node there if that turns
> out necessary (see below).
> 
>> Johan
>>
>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
>>> The PX30 features a RGA block: add the necessary node to support it.
>>>
>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> index 75908c587511..4bfbee9d4123 100644
>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>>>  		status = "disabled";
>>>  	};
>>>  
>>> +	rga: rga@ff480000 {
>>> +		compatible = "rockchip,px30-rga";
>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>> +		clock-names = "aclk", "hclk", "sclk";
>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>> +		reset-names = "core", "axi", "ahb";
>>> +		power-domains = <&power PX30_PD_VO>;
>>
>> 		status = "disabled";
> 
> As of 5.6, the rk3399 has the node enabled by default. Did that change?

'status' disappeared during review for rk3399 between v2 and v3, but
doesn't mention the reason. If someone can give more info here?

https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org/

https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org/

> 
> Since it's a standalone block that has no I/O dependency, I don't really see
> the point of disabling it by default.

Vop, hdmi and other video devices are also disabled.
Follow the rest I think...

> 
> What do you think?
> 
> Cheers,
> 
> Paul
> 
>>> +	};
>>> +
>>>  	qos_gmac: qos@ff518000 {
>>>  		compatible = "syscon";
>>>  		reg = <0x0 0xff518000 0x0 0x20>;
>>>
>>
> 

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
>> Hi Paul,
>>
>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
>> has been approved by robh.
> 
> Huh, I looked around for ongoing related work but missed it.
> I'll definitely rebase on top of your series and use the yaml description
> instead. Thanks!
> 
>> Maybe place dts patches at the end of a patch serie.
>> Could you include a &rga patch if your device is supported in mainline,
>> so we can test with:
>> make ARCH=arm64 dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> 
> I tested with the PX30 EVB so I can surely add a node there if that turns
> out necessary (see below).
> 
>> Johan
>>
>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
>>> The PX30 features a RGA block: add the necessary node to support it.
>>>
>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> index 75908c587511..4bfbee9d4123 100644
>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>>>  		status = "disabled";
>>>  	};
>>>  
>>> +	rga: rga@ff480000 {
>>> +		compatible = "rockchip,px30-rga";
>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>> +		clock-names = "aclk", "hclk", "sclk";
>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>> +		reset-names = "core", "axi", "ahb";
>>> +		power-domains = <&power PX30_PD_VO>;
>>
>> 		status = "disabled";
> 
> As of 5.6, the rk3399 has the node enabled by default. Did that change?

'status' disappeared during review for rk3399 between v2 and v3, but
doesn't mention the reason. If someone can give more info here?

https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/

https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/

> 
> Since it's a standalone block that has no I/O dependency, I don't really see
> the point of disabling it by default.

Vop, hdmi and other video devices are also disabled.
Follow the rest I think...

> 
> What do you think?
> 
> Cheers,
> 
> Paul
> 
>>> +	};
>>> +
>>>  	qos_gmac: qos@ff518000 {
>>>  		compatible = "syscon";
>>>  		reg = <0x0 0xff518000 0x0 0x20>;
>>>
>>
> 


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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 3677 bytes --]

Hi,

On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> >> Hi Paul,
> >>
> >> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> >> has been approved by robh.
> > 
> > Huh, I looked around for ongoing related work but missed it.
> > I'll definitely rebase on top of your series and use the yaml description
> > instead. Thanks!
> > 
> >> Maybe place dts patches at the end of a patch serie.
> >> Could you include a &rga patch if your device is supported in mainline,
> >> so we can test with:
> >> make ARCH=arm64 dtbs_check
> >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> > 
> > I tested with the PX30 EVB so I can surely add a node there if that turns
> > out necessary (see below).
> > 
> >> Johan
> >>
> >> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> >>> The PX30 features a RGA block: add the necessary node to support it.
> >>>
> >>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> >>> ---
> >>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> index 75908c587511..4bfbee9d4123 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> >>>  		status = "disabled";
> >>>  	};
> >>>  
> >>> +	rga: rga@ff480000 {
> >>> +		compatible = "rockchip,px30-rga";
> >>> +		reg = <0x0 0xff480000 0x0 0x10000>;
> >>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> >>> +		clock-names = "aclk", "hclk", "sclk";
> >>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> >>> +		reset-names = "core", "axi", "ahb";
> >>> +		power-domains = <&power PX30_PD_VO>;
> >>
> >> 		status = "disabled";
> > 
> > As of 5.6, the rk3399 has the node enabled by default. Did that change?
> 
> 'status' disappeared during review for rk3399 between v2 and v3, but
> doesn't mention the reason. If someone can give more info here?
> 
> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> 
> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> 
> > 
> > Since it's a standalone block that has no I/O dependency, I don't really see
> > the point of disabling it by default.
> 
> Vop, hdmi and other video devices are also disabled.
> Follow the rest I think...

Well, these blocks do have related I/O ports so it makes sense not to enable
them by default because of pinmux, or because there might be no connector
populated/routed.

For a memory to memory internal block, I don't see any reason why.
It's definitely not board-specific and having to add these nodes for every board
that has them is kind of a pain and might be overlooked. This will easily result
in the feature not being available for end users without having to change the
dt.

Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
I think these are better examples to follow.

Cheers,

Paul

> > 
> > What do you think?
> > 
> > Cheers,
> > 
> > Paul
> > 
> >>> +	};
> >>> +
> >>>  	qos_gmac: qos@ff518000 {
> >>>  		compatible = "syscon";
> >>>  		reg = <0x0 0xff518000 0x0 0x20>;
> >>>
> >>
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

[-- Attachment #1.1: Type: text/plain, Size: 3677 bytes --]

Hi,

On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> >> Hi Paul,
> >>
> >> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> >> has been approved by robh.
> > 
> > Huh, I looked around for ongoing related work but missed it.
> > I'll definitely rebase on top of your series and use the yaml description
> > instead. Thanks!
> > 
> >> Maybe place dts patches at the end of a patch serie.
> >> Could you include a &rga patch if your device is supported in mainline,
> >> so we can test with:
> >> make ARCH=arm64 dtbs_check
> >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> > 
> > I tested with the PX30 EVB so I can surely add a node there if that turns
> > out necessary (see below).
> > 
> >> Johan
> >>
> >> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> >>> The PX30 features a RGA block: add the necessary node to support it.
> >>>
> >>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> >>> ---
> >>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> index 75908c587511..4bfbee9d4123 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> >>>  		status = "disabled";
> >>>  	};
> >>>  
> >>> +	rga: rga@ff480000 {
> >>> +		compatible = "rockchip,px30-rga";
> >>> +		reg = <0x0 0xff480000 0x0 0x10000>;
> >>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> >>> +		clock-names = "aclk", "hclk", "sclk";
> >>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> >>> +		reset-names = "core", "axi", "ahb";
> >>> +		power-domains = <&power PX30_PD_VO>;
> >>
> >> 		status = "disabled";
> > 
> > As of 5.6, the rk3399 has the node enabled by default. Did that change?
> 
> 'status' disappeared during review for rk3399 between v2 and v3, but
> doesn't mention the reason. If someone can give more info here?
> 
> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> 
> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> 
> > 
> > Since it's a standalone block that has no I/O dependency, I don't really see
> > the point of disabling it by default.
> 
> Vop, hdmi and other video devices are also disabled.
> Follow the rest I think...

Well, these blocks do have related I/O ports so it makes sense not to enable
them by default because of pinmux, or because there might be no connector
populated/routed.

For a memory to memory internal block, I don't see any reason why.
It's definitely not board-specific and having to add these nodes for every board
that has them is kind of a pain and might be overlooked. This will easily result
in the feature not being available for end users without having to change the
dt.

Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
I think these are better examples to follow.

Cheers,

Paul

> > 
> > What do you think?
> > 
> > Cheers,
> > 
> > Paul
> > 
> >>> +	};
> >>> +
> >>>  	qos_gmac: qos@ff518000 {
> >>>  		compatible = "syscon";
> >>>  		reg = <0x0 0xff518000 0x0 0x20>;
> >>>
> >>
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

On 4/16/20 3:55 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
>> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
>>> Hi,
>>>
>>> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
>>>> Hi Paul,
>>>>
>>>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
>>>> has been approved by robh.
>>>
>>> Huh, I looked around for ongoing related work but missed it.
>>> I'll definitely rebase on top of your series and use the yaml description
>>> instead. Thanks!
>>>
>>>> Maybe place dts patches at the end of a patch serie.
>>>> Could you include a &rga patch if your device is supported in mainline,
>>>> so we can test with:
>>>> make ARCH=arm64 dtbs_check
>>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
>>>
>>> I tested with the PX30 EVB so I can surely add a node there if that turns
>>> out necessary (see below).
>>>
>>>> Johan
>>>>
>>>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
>>>>> The PX30 features a RGA block: add the necessary node to support it.
>>>>>
>>>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>>>>> ---
>>>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>>>>  1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> index 75908c587511..4bfbee9d4123 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>>>>>  		status = "disabled";
>>>>>  	};
>>>>>  
>>>>> +	rga: rga@ff480000 {
>>>>> +		compatible = "rockchip,px30-rga";
>>>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
>>>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>>>> +		clock-names = "aclk", "hclk", "sclk";
>>>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>>>> +		reset-names = "core", "axi", "ahb";
>>>>> +		power-domains = <&power PX30_PD_VO>;
>>>>
>>>> 		status = "disabled";
>>>
>>> As of 5.6, the rk3399 has the node enabled by default. Did that change?
>>
>> 'status' disappeared during review for rk3399 between v2 and v3, but
>> doesn't mention the reason. If someone can give more info here?
>>
>> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
>>
>> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
>>
>>>
>>> Since it's a standalone block that has no I/O dependency, I don't really see
>>> the point of disabling it by default.
>>
>> Vop, hdmi and other video devices are also disabled.
>> Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.

From PX30 TRM-Part1:

Power domain is shared by vop and dsi.
It's up to the user what blocks he/she enables and what power it uses.

PD_VO: VOP_M, VOP_S, RGA and DSI

> 
> Cheers,
> 
> Paul
> 
>>>
>>> What do you think?
>>>
>>> Cheers,
>>>
>>> Paul
>>>
>>>>> +	};
>>>>> +
>>>>>  	qos_gmac: qos@ff518000 {
>>>>>  		compatible = "syscon";
>>>>>  		reg = <0x0 0xff518000 0x0 0x20>;
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

On 4/16/20 3:55 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
>> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
>>> Hi,
>>>
>>> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
>>>> Hi Paul,
>>>>
>>>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
>>>> has been approved by robh.
>>>
>>> Huh, I looked around for ongoing related work but missed it.
>>> I'll definitely rebase on top of your series and use the yaml description
>>> instead. Thanks!
>>>
>>>> Maybe place dts patches at the end of a patch serie.
>>>> Could you include a &rga patch if your device is supported in mainline,
>>>> so we can test with:
>>>> make ARCH=arm64 dtbs_check
>>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
>>>
>>> I tested with the PX30 EVB so I can surely add a node there if that turns
>>> out necessary (see below).
>>>
>>>> Johan
>>>>
>>>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
>>>>> The PX30 features a RGA block: add the necessary node to support it.
>>>>>
>>>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
>>>>> ---
>>>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>>>>  1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> index 75908c587511..4bfbee9d4123 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>>>>>  		status = "disabled";
>>>>>  	};
>>>>>  
>>>>> +	rga: rga@ff480000 {
>>>>> +		compatible = "rockchip,px30-rga";
>>>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
>>>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>>>> +		clock-names = "aclk", "hclk", "sclk";
>>>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>>>> +		reset-names = "core", "axi", "ahb";
>>>>> +		power-domains = <&power PX30_PD_VO>;
>>>>
>>>> 		status = "disabled";
>>>
>>> As of 5.6, the rk3399 has the node enabled by default. Did that change?
>>
>> 'status' disappeared during review for rk3399 between v2 and v3, but
>> doesn't mention the reason. If someone can give more info here?
>>
>> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org/
>>
>> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org/
>>
>>>
>>> Since it's a standalone block that has no I/O dependency, I don't really see
>>> the point of disabling it by default.
>>
>> Vop, hdmi and other video devices are also disabled.
>> Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.

From PX30 TRM-Part1:

Power domain is shared by vop and dsi.
It's up to the user what blocks he/she enables and what power it uses.

PD_VO: VOP_M, VOP_S, RGA and DSI

> 
> Cheers,
> 
> Paul
> 
>>>
>>> What do you think?
>>>
>>> Cheers,
>>>
>>> Paul
>>>
>>>>> +	};
>>>>> +
>>>>>  	qos_gmac: qos@ff518000 {
>>>>>  		compatible = "syscon";
>>>>>  		reg = <0x0 0xff518000 0x0 0x20>;
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Johan Jonker]

On 4/16/20 3:55 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
>> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
>>> Hi,
>>>
>>> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
>>>> Hi Paul,
>>>>
>>>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
>>>> has been approved by robh.
>>>
>>> Huh, I looked around for ongoing related work but missed it.
>>> I'll definitely rebase on top of your series and use the yaml description
>>> instead. Thanks!
>>>
>>>> Maybe place dts patches at the end of a patch serie.
>>>> Could you include a &rga patch if your device is supported in mainline,
>>>> so we can test with:
>>>> make ARCH=arm64 dtbs_check
>>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
>>>
>>> I tested with the PX30 EVB so I can surely add a node there if that turns
>>> out necessary (see below).
>>>
>>>> Johan
>>>>
>>>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
>>>>> The PX30 features a RGA block: add the necessary node to support it.
>>>>>
>>>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>>>>> ---
>>>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>>>>  1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> index 75908c587511..4bfbee9d4123 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>>>>>  		status = "disabled";
>>>>>  	};
>>>>>  
>>>>> +	rga: rga@ff480000 {
>>>>> +		compatible = "rockchip,px30-rga";
>>>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
>>>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>>>> +		clock-names = "aclk", "hclk", "sclk";
>>>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>>>> +		reset-names = "core", "axi", "ahb";
>>>>> +		power-domains = <&power PX30_PD_VO>;
>>>>
>>>> 		status = "disabled";
>>>
>>> As of 5.6, the rk3399 has the node enabled by default. Did that change?
>>
>> 'status' disappeared during review for rk3399 between v2 and v3, but
>> doesn't mention the reason. If someone can give more info here?
>>
>> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
>>
>> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
>>
>>>
>>> Since it's a standalone block that has no I/O dependency, I don't really see
>>> the point of disabling it by default.
>>
>> Vop, hdmi and other video devices are also disabled.
>> Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.

From PX30 TRM-Part1:

Power domain is shared by vop and dsi.
It's up to the user what blocks he/she enables and what power it uses.

PD_VO: VOP_M, VOP_S, RGA and DSI

> 
> Cheers,
> 
> Paul
> 
>>>
>>> What do you think?
>>>
>>> Cheers,
>>>
>>> Paul
>>>
>>>>> +	};
>>>>> +
>>>>>  	qos_gmac: qos@ff518000 {
>>>>>  		compatible = "syscon";
>>>>>  		reg = <0x0 0xff518000 0x0 0x20>;
>>>>>
>>>>
>>>
>>
> 


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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Ezequiel Garcia]

On Thu, 2020-04-16 at 15:55 +0200, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> > On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> > > > Hi Paul,
> > > > 
> > > > The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> > > > has been approved by robh.
> > > 
> > > Huh, I looked around for ongoing related work but missed it.
> > > I'll definitely rebase on top of your series and use the yaml description
> > > instead. Thanks!
> > > 
> > > > Maybe place dts patches at the end of a patch serie.
> > > > Could you include a &rga patch if your device is supported in mainline,
> > > > so we can test with:
> > > > make ARCH=arm64 dtbs_check
> > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> > > 
> > > I tested with the PX30 EVB so I can surely add a node there if that turns
> > > out necessary (see below).
> > > 
> > > > Johan
> > > > 
> > > > On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> > > > > The PX30 features a RGA block: add the necessary node to support it.
> > > > > 
> > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > ---
> > > > >  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> > > > >  1 file changed, 11 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > index 75908c587511..4bfbee9d4123 100644
> > > > > --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> > > > >  		status = "disabled";
> > > > >  	};
> > > > >  
> > > > > +	rga: rga@ff480000 {
> > > > > +		compatible = "rockchip,px30-rga";
> > > > > +		reg = <0x0 0xff480000 0x0 0x10000>;
> > > > > +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > > > > +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > > > > +		clock-names = "aclk", "hclk", "sclk";
> > > > > +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > > > > +		reset-names = "core", "axi", "ahb";
> > > > > +		power-domains = <&power PX30_PD_VO>;
> > > > 
> > > > 		status = "disabled";
> > > 
> > > As of 5.6, the rk3399 has the node enabled by default. Did that change?
> > 
> > 'status' disappeared during review for rk3399 between v2 and v3, but
> > doesn't mention the reason. If someone can give more info here?
> > 
> > https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > > Since it's a standalone block that has no I/O dependency, I don't really see
> > > the point of disabling it by default.
> > 
> > Vop, hdmi and other video devices are also disabled.
> > Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.
> 

It's exactly as Paul just said :-)

Not board specific, no reason to disable, and also
no way to know under what condition you'd enable.

Thanks,
Ezequiel

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Ezequiel Garcia]

On Thu, 2020-04-16 at 15:55 +0200, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> > On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> > > > Hi Paul,
> > > > 
> > > > The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> > > > has been approved by robh.
> > > 
> > > Huh, I looked around for ongoing related work but missed it.
> > > I'll definitely rebase on top of your series and use the yaml description
> > > instead. Thanks!
> > > 
> > > > Maybe place dts patches at the end of a patch serie.
> > > > Could you include a &rga patch if your device is supported in mainline,
> > > > so we can test with:
> > > > make ARCH=arm64 dtbs_check
> > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> > > 
> > > I tested with the PX30 EVB so I can surely add a node there if that turns
> > > out necessary (see below).
> > > 
> > > > Johan
> > > > 
> > > > On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> > > > > The PX30 features a RGA block: add the necessary node to support it.
> > > > > 
> > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > ---
> > > > >  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> > > > >  1 file changed, 11 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > index 75908c587511..4bfbee9d4123 100644
> > > > > --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> > > > >  		status = "disabled";
> > > > >  	};
> > > > >  
> > > > > +	rga: rga@ff480000 {
> > > > > +		compatible = "rockchip,px30-rga";
> > > > > +		reg = <0x0 0xff480000 0x0 0x10000>;
> > > > > +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > > > > +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > > > > +		clock-names = "aclk", "hclk", "sclk";
> > > > > +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > > > > +		reset-names = "core", "axi", "ahb";
> > > > > +		power-domains = <&power PX30_PD_VO>;
> > > > 
> > > > 		status = "disabled";
> > > 
> > > As of 5.6, the rk3399 has the node enabled by default. Did that change?
> > 
> > 'status' disappeared during review for rk3399 between v2 and v3, but
> > doesn't mention the reason. If someone can give more info here?
> > 
> > https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > > Since it's a standalone block that has no I/O dependency, I don't really see
> > > the point of disabling it by default.
> > 
> > Vop, hdmi and other video devices are also disabled.
> > Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.
> 

It's exactly as Paul just said :-)

Not board specific, no reason to disable, and also
no way to know under what condition you'd enable.

Thanks,
Ezequiel

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Ezequiel Garcia]

On Thu, 2020-04-16 at 15:55 +0200, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> > On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> > > > Hi Paul,
> > > > 
> > > > The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> > > > has been approved by robh.
> > > 
> > > Huh, I looked around for ongoing related work but missed it.
> > > I'll definitely rebase on top of your series and use the yaml description
> > > instead. Thanks!
> > > 
> > > > Maybe place dts patches at the end of a patch serie.
> > > > Could you include a &rga patch if your device is supported in mainline,
> > > > so we can test with:
> > > > make ARCH=arm64 dtbs_check
> > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> > > 
> > > I tested with the PX30 EVB so I can surely add a node there if that turns
> > > out necessary (see below).
> > > 
> > > > Johan
> > > > 
> > > > On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> > > > > The PX30 features a RGA block: add the necessary node to support it.
> > > > > 
> > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > ---
> > > > >  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> > > > >  1 file changed, 11 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > index 75908c587511..4bfbee9d4123 100644
> > > > > --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > > > > @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> > > > >  		status = "disabled";
> > > > >  	};
> > > > >  
> > > > > +	rga: rga@ff480000 {
> > > > > +		compatible = "rockchip,px30-rga";
> > > > > +		reg = <0x0 0xff480000 0x0 0x10000>;
> > > > > +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > > > > +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > > > > +		clock-names = "aclk", "hclk", "sclk";
> > > > > +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > > > > +		reset-names = "core", "axi", "ahb";
> > > > > +		power-domains = <&power PX30_PD_VO>;
> > > > 
> > > > 		status = "disabled";
> > > 
> > > As of 5.6, the rk3399 has the node enabled by default. Did that change?
> > 
> > 'status' disappeared during review for rk3399 between v2 and v3, but
> > doesn't mention the reason. If someone can give more info here?
> > 
> > https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > > Since it's a standalone block that has no I/O dependency, I don't really see
> > > the point of disabling it by default.
> > 
> > Vop, hdmi and other video devices are also disabled.
> > Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.
> 

It's exactly as Paul just said :-)

Not board specific, no reason to disable, and also
no way to know under what condition you'd enable.

Thanks,
Ezequiel


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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 4675 bytes --]

On Thu 16 Apr 20, 16:02, Johan Jonker wrote:
> On 4/16/20 3:55 PM, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> >> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> >>> Hi,
> >>>
> >>> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> >>>> Hi Paul,
> >>>>
> >>>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> >>>> has been approved by robh.
> >>>
> >>> Huh, I looked around for ongoing related work but missed it.
> >>> I'll definitely rebase on top of your series and use the yaml description
> >>> instead. Thanks!
> >>>
> >>>> Maybe place dts patches at the end of a patch serie.
> >>>> Could you include a &rga patch if your device is supported in mainline,
> >>>> so we can test with:
> >>>> make ARCH=arm64 dtbs_check
> >>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> >>>
> >>> I tested with the PX30 EVB so I can surely add a node there if that turns
> >>> out necessary (see below).
> >>>
> >>>> Johan
> >>>>
> >>>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> >>>>> The PX30 features a RGA block: add the necessary node to support it.
> >>>>>
> >>>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> >>>>> ---
> >>>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> >>>>>  1 file changed, 11 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> index 75908c587511..4bfbee9d4123 100644
> >>>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> >>>>>  		status = "disabled";
> >>>>>  	};
> >>>>>  
> >>>>> +	rga: rga@ff480000 {
> >>>>> +		compatible = "rockchip,px30-rga";
> >>>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
> >>>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> >>>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> >>>>> +		clock-names = "aclk", "hclk", "sclk";
> >>>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> >>>>> +		reset-names = "core", "axi", "ahb";
> >>>>> +		power-domains = <&power PX30_PD_VO>;
> >>>>
> >>>> 		status = "disabled";
> >>>
> >>> As of 5.6, the rk3399 has the node enabled by default. Did that change?
> >>
> >> 'status' disappeared during review for rk3399 between v2 and v3, but
> >> doesn't mention the reason. If someone can give more info here?
> >>
> >> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> >>
> >> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> >>
> >>>
> >>> Since it's a standalone block that has no I/O dependency, I don't really see
> >>> the point of disabling it by default.
> >>
> >> Vop, hdmi and other video devices are also disabled.
> >> Follow the rest I think...
> > 
> > Well, these blocks do have related I/O ports so it makes sense not to enable
> > them by default because of pinmux, or because there might be no connector
> > populated/routed.
> > 
> > For a memory to memory internal block, I don't see any reason why.
> > It's definitely not board-specific and having to add these nodes for every board
> > that has them is kind of a pain and might be overlooked. This will easily result
> > in the feature not being available for end users without having to change the
> > dt.
> > 
> > Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> > I think these are better examples to follow.
> 
> From PX30 TRM-Part1:
> 
> Power domain is shared by vop and dsi.
> It's up to the user what blocks he/she enables and what power it uses.
> 
> PD_VO: VOP_M, VOP_S, RGA and DSI

Hum, there is no direct correlation between "node is enabled in dt" and "related
hardware block consumes power". And removing nodes from dt is certainly not an
appropriate way to do power management! Device-tree is a way to represent the
hardware, not a configuration interface (well, besides the /chosen node).

Besides, the RGA driver seems to have good runtime pm support, so be assured
that its clocks will be off when unused.

Cheers,

Paul

> > 
> > Cheers,
> > 
> > Paul
> > 
> >>>
> >>> What do you think?
> >>>
> >>> Cheers,
> >>>
> >>> Paul
> >>>
> >>>>> +	};
> >>>>> +
> >>>>>  	qos_gmac: qos@ff518000 {
> >>>>>  		compatible = "syscon";
> >>>>>  		reg = <0x0 0xff518000 0x0 0x20>;
> >>>>>
> >>>>
> >>>
> >>
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 4761 bytes --]

On Thu 16 Apr 20, 16:02, Johan Jonker wrote:
> On 4/16/20 3:55 PM, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> >> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> >>> Hi,
> >>>
> >>> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> >>>> Hi Paul,
> >>>>
> >>>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> >>>> has been approved by robh.
> >>>
> >>> Huh, I looked around for ongoing related work but missed it.
> >>> I'll definitely rebase on top of your series and use the yaml description
> >>> instead. Thanks!
> >>>
> >>>> Maybe place dts patches at the end of a patch serie.
> >>>> Could you include a &rga patch if your device is supported in mainline,
> >>>> so we can test with:
> >>>> make ARCH=arm64 dtbs_check
> >>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> >>>
> >>> I tested with the PX30 EVB so I can surely add a node there if that turns
> >>> out necessary (see below).
> >>>
> >>>> Johan
> >>>>
> >>>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> >>>>> The PX30 features a RGA block: add the necessary node to support it.
> >>>>>
> >>>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
> >>>>> ---
> >>>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> >>>>>  1 file changed, 11 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> index 75908c587511..4bfbee9d4123 100644
> >>>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> >>>>>  		status = "disabled";
> >>>>>  	};
> >>>>>  
> >>>>> +	rga: rga@ff480000 {
> >>>>> +		compatible = "rockchip,px30-rga";
> >>>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
> >>>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> >>>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> >>>>> +		clock-names = "aclk", "hclk", "sclk";
> >>>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> >>>>> +		reset-names = "core", "axi", "ahb";
> >>>>> +		power-domains = <&power PX30_PD_VO>;
> >>>>
> >>>> 		status = "disabled";
> >>>
> >>> As of 5.6, the rk3399 has the node enabled by default. Did that change?
> >>
> >> 'status' disappeared during review for rk3399 between v2 and v3, but
> >> doesn't mention the reason. If someone can give more info here?
> >>
> >> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org/
> >>
> >> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org/
> >>
> >>>
> >>> Since it's a standalone block that has no I/O dependency, I don't really see
> >>> the point of disabling it by default.
> >>
> >> Vop, hdmi and other video devices are also disabled.
> >> Follow the rest I think...
> > 
> > Well, these blocks do have related I/O ports so it makes sense not to enable
> > them by default because of pinmux, or because there might be no connector
> > populated/routed.
> > 
> > For a memory to memory internal block, I don't see any reason why.
> > It's definitely not board-specific and having to add these nodes for every board
> > that has them is kind of a pain and might be overlooked. This will easily result
> > in the feature not being available for end users without having to change the
> > dt.
> > 
> > Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> > I think these are better examples to follow.
> 
> From PX30 TRM-Part1:
> 
> Power domain is shared by vop and dsi.
> It's up to the user what blocks he/she enables and what power it uses.
> 
> PD_VO: VOP_M, VOP_S, RGA and DSI

Hum, there is no direct correlation between "node is enabled in dt" and "related
hardware block consumes power". And removing nodes from dt is certainly not an
appropriate way to do power management! Device-tree is a way to represent the
hardware, not a configuration interface (well, besides the /chosen node).

Besides, the RGA driver seems to have good runtime pm support, so be assured
that its clocks will be off when unused.

Cheers,

Paul

> > 
> > Cheers,
> > 
> > Paul
> > 
> >>>
> >>> What do you think?
> >>>
> >>> Cheers,
> >>>
> >>> Paul
> >>>
> >>>>> +	};
> >>>>> +
> >>>>>  	qos_gmac: qos@ff518000 {
> >>>>>  		compatible = "syscon";
> >>>>>  		reg = <0x0 0xff518000 0x0 0x20>;
> >>>>>
> >>>>
> >>>
> >>
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Paul Kocialkowski]

[-- Attachment #1.1: Type: text/plain, Size: 4675 bytes --]

On Thu 16 Apr 20, 16:02, Johan Jonker wrote:
> On 4/16/20 3:55 PM, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> >> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> >>> Hi,
> >>>
> >>> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> >>>> Hi Paul,
> >>>>
> >>>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> >>>> has been approved by robh.
> >>>
> >>> Huh, I looked around for ongoing related work but missed it.
> >>> I'll definitely rebase on top of your series and use the yaml description
> >>> instead. Thanks!
> >>>
> >>>> Maybe place dts patches at the end of a patch serie.
> >>>> Could you include a &rga patch if your device is supported in mainline,
> >>>> so we can test with:
> >>>> make ARCH=arm64 dtbs_check
> >>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> >>>
> >>> I tested with the PX30 EVB so I can surely add a node there if that turns
> >>> out necessary (see below).
> >>>
> >>>> Johan
> >>>>
> >>>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> >>>>> The PX30 features a RGA block: add the necessary node to support it.
> >>>>>
> >>>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> >>>>> ---
> >>>>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> >>>>>  1 file changed, 11 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> index 75908c587511..4bfbee9d4123 100644
> >>>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> >>>>>  		status = "disabled";
> >>>>>  	};
> >>>>>  
> >>>>> +	rga: rga@ff480000 {
> >>>>> +		compatible = "rockchip,px30-rga";
> >>>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
> >>>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> >>>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> >>>>> +		clock-names = "aclk", "hclk", "sclk";
> >>>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> >>>>> +		reset-names = "core", "axi", "ahb";
> >>>>> +		power-domains = <&power PX30_PD_VO>;
> >>>>
> >>>> 		status = "disabled";
> >>>
> >>> As of 5.6, the rk3399 has the node enabled by default. Did that change?
> >>
> >> 'status' disappeared during review for rk3399 between v2 and v3, but
> >> doesn't mention the reason. If someone can give more info here?
> >>
> >> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> >>
> >> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> >>
> >>>
> >>> Since it's a standalone block that has no I/O dependency, I don't really see
> >>> the point of disabling it by default.
> >>
> >> Vop, hdmi and other video devices are also disabled.
> >> Follow the rest I think...
> > 
> > Well, these blocks do have related I/O ports so it makes sense not to enable
> > them by default because of pinmux, or because there might be no connector
> > populated/routed.
> > 
> > For a memory to memory internal block, I don't see any reason why.
> > It's definitely not board-specific and having to add these nodes for every board
> > that has them is kind of a pain and might be overlooked. This will easily result
> > in the feature not being available for end users without having to change the
> > dt.
> > 
> > Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> > I think these are better examples to follow.
> 
> From PX30 TRM-Part1:
> 
> Power domain is shared by vop and dsi.
> It's up to the user what blocks he/she enables and what power it uses.
> 
> PD_VO: VOP_M, VOP_S, RGA and DSI

Hum, there is no direct correlation between "node is enabled in dt" and "related
hardware block consumes power". And removing nodes from dt is certainly not an
appropriate way to do power management! Device-tree is a way to represent the
hardware, not a configuration interface (well, besides the /chosen node).

Besides, the RGA driver seems to have good runtime pm support, so be assured
that its clocks will be off when unused.

Cheers,

Paul

> > 
> > Cheers,
> > 
> > Paul
> > 
> >>>
> >>> What do you think?
> >>>
> >>> Cheers,
> >>>
> >>> Paul
> >>>
> >>>>> +	};
> >>>>> +
> >>>>>  	qos_gmac: qos@ff518000 {
> >>>>>  		compatible = "syscon";
> >>>>>  		reg = <0x0 0xff518000 0x0 0x20>;
> >>>>>
> >>>>
> >>>
> >>
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Robin Murphy]

On 2020-04-16 2:55 pm, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
>> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
>>> Hi,
>>>
>>> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
>>>> Hi Paul,
>>>>
>>>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
>>>> has been approved by robh.
>>>
>>> Huh, I looked around for ongoing related work but missed it.
>>> I'll definitely rebase on top of your series and use the yaml description
>>> instead. Thanks!
>>>
>>>> Maybe place dts patches at the end of a patch serie.
>>>> Could you include a &rga patch if your device is supported in mainline,
>>>> so we can test with:
>>>> make ARCH=arm64 dtbs_check
>>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
>>>
>>> I tested with the PX30 EVB so I can surely add a node there if that turns
>>> out necessary (see below).
>>>
>>>> Johan
>>>>
>>>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
>>>>> The PX30 features a RGA block: add the necessary node to support it.
>>>>>
>>>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>>>>   1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> index 75908c587511..4bfbee9d4123 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>>>>>   		status = "disabled";
>>>>>   	};
>>>>>   
>>>>> +	rga: rga@ff480000 {
>>>>> +		compatible = "rockchip,px30-rga";
>>>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
>>>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>>>> +		clock-names = "aclk", "hclk", "sclk";
>>>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>>>> +		reset-names = "core", "axi", "ahb";
>>>>> +		power-domains = <&power PX30_PD_VO>;
>>>>
>>>> 		status = "disabled";
>>>
>>> As of 5.6, the rk3399 has the node enabled by default. Did that change?
>>
>> 'status' disappeared during review for rk3399 between v2 and v3, but
>> doesn't mention the reason. If someone can give more info here?
>>
>> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
>>
>> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
>>
>>>
>>> Since it's a standalone block that has no I/O dependency, I don't really see
>>> the point of disabling it by default.
>>
>> Vop, hdmi and other video devices are also disabled.
>> Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.

Yes, as far as I'm aware the general preference for things that are 
entirely internal to the SoC and don't have any external dependencies 
like regulators or pinctrl settings is to leave them enabled by default. 
There's nothing to gain from disabling them, and in fact if the hardware 
would otherwise just sit there idle in its out-of-reset state then 
allowing a driver to bind and enable power management may be a distinct 
benefit.

Whether a board wires up video output or not is also largely orthogonal 
to whether internal graphics/video accelerators are useful. Consider how 
many people use their NAS box for media transcoding, vs. how many would 
ever plug a display directly into said box if it even has a connector. 
If the RGA can be wired into some software format 
conversion/scaling/whatever pipeline then it's useful full stop.

Robin.

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Robin Murphy]

On 2020-04-16 2:55 pm, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
>> On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
>>> Hi,
>>>
>>> On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
>>>> Hi Paul,
>>>>
>>>> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
>>>> has been approved by robh.
>>>
>>> Huh, I looked around for ongoing related work but missed it.
>>> I'll definitely rebase on top of your series and use the yaml description
>>> instead. Thanks!
>>>
>>>> Maybe place dts patches at the end of a patch serie.
>>>> Could you include a &rga patch if your device is supported in mainline,
>>>> so we can test with:
>>>> make ARCH=arm64 dtbs_check
>>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
>>>
>>> I tested with the PX30 EVB so I can surely add a node there if that turns
>>> out necessary (see below).
>>>
>>>> Johan
>>>>
>>>> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
>>>>> The PX30 features a RGA block: add the necessary node to support it.
>>>>>
>>>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>>>>   1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> index 75908c587511..4bfbee9d4123 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>>>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
>>>>>   		status = "disabled";
>>>>>   	};
>>>>>   
>>>>> +	rga: rga@ff480000 {
>>>>> +		compatible = "rockchip,px30-rga";
>>>>> +		reg = <0x0 0xff480000 0x0 0x10000>;
>>>>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>>>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>>>> +		clock-names = "aclk", "hclk", "sclk";
>>>>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>>>> +		reset-names = "core", "axi", "ahb";
>>>>> +		power-domains = <&power PX30_PD_VO>;
>>>>
>>>> 		status = "disabled";
>>>
>>> As of 5.6, the rk3399 has the node enabled by default. Did that change?
>>
>> 'status' disappeared during review for rk3399 between v2 and v3, but
>> doesn't mention the reason. If someone can give more info here?
>>
>> https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
>>
>> https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
>>
>>>
>>> Since it's a standalone block that has no I/O dependency, I don't really see
>>> the point of disabling it by default.
>>
>> Vop, hdmi and other video devices are also disabled.
>> Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.

Yes, as far as I'm aware the general preference for things that are 
entirely internal to the SoC and don't have any external dependencies 
like regulators or pinctrl settings is to leave them enabled by default. 
There's nothing to gain from disabling them, and in fact if the hardware 
would otherwise just sit there idle in its out-of-reset state then 
allowing a driver to bind and enable power management may be a distinct 
benefit.

Whether a board wires up video output or not is also largely orthogonal 
to whether internal graphics/video accelerators are useful. Consider how 
many people use their NAS box for media transcoding, vs. how many would 
ever plug a display directly into said box if it even has a connector. 
If the RGA can be wired into some software format 
conversion/scaling/whatever pipeline then it's useful full stop.

Robin.

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

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Heiko Stuebner]

Am Donnerstag, 16. April 2020, 15:55:19 CEST schrieb Paul Kocialkowski:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> > On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> > >> Hi Paul,
> > >>
> > >> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> > >> has been approved by robh.
> > > 
> > > Huh, I looked around for ongoing related work but missed it.
> > > I'll definitely rebase on top of your series and use the yaml description
> > > instead. Thanks!
> > > 
> > >> Maybe place dts patches at the end of a patch serie.
> > >> Could you include a &rga patch if your device is supported in mainline,
> > >> so we can test with:
> > >> make ARCH=arm64 dtbs_check
> > >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> > > 
> > > I tested with the PX30 EVB so I can surely add a node there if that turns
> > > out necessary (see below).
> > > 
> > >> Johan
> > >>
> > >> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> > >>> The PX30 features a RGA block: add the necessary node to support it.
> > >>>
> > >>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > >>> ---
> > >>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> > >>>  1 file changed, 11 insertions(+)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> index 75908c587511..4bfbee9d4123 100644
> > >>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> > >>>  		status = "disabled";
> > >>>  	};
> > >>>  
> > >>> +	rga: rga@ff480000 {
> > >>> +		compatible = "rockchip,px30-rga";
> > >>> +		reg = <0x0 0xff480000 0x0 0x10000>;
> > >>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > >>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > >>> +		clock-names = "aclk", "hclk", "sclk";
> > >>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > >>> +		reset-names = "core", "axi", "ahb";
> > >>> +		power-domains = <&power PX30_PD_VO>;
> > >>
> > >> 		status = "disabled";
> > > 
> > > As of 5.6, the rk3399 has the node enabled by default. Did that change?
> > 
> > 'status' disappeared during review for rk3399 between v2 and v3, but
> > doesn't mention the reason. If someone can give more info here?
> > 
> > https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > > 
> > > Since it's a standalone block that has no I/O dependency, I don't really see
> > > the point of disabling it by default.
> > 
> > Vop, hdmi and other video devices are also disabled.
> > Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.

just to add my me too ... what Paul, Ezequiel and Robin said - so no status
for soc internal components without any board-specifics


Heiko

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Heiko Stuebner]

Am Donnerstag, 16. April 2020, 15:55:19 CEST schrieb Paul Kocialkowski:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> > On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> > >> Hi Paul,
> > >>
> > >> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> > >> has been approved by robh.
> > > 
> > > Huh, I looked around for ongoing related work but missed it.
> > > I'll definitely rebase on top of your series and use the yaml description
> > > instead. Thanks!
> > > 
> > >> Maybe place dts patches at the end of a patch serie.
> > >> Could you include a &rga patch if your device is supported in mainline,
> > >> so we can test with:
> > >> make ARCH=arm64 dtbs_check
> > >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> > > 
> > > I tested with the PX30 EVB so I can surely add a node there if that turns
> > > out necessary (see below).
> > > 
> > >> Johan
> > >>
> > >> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> > >>> The PX30 features a RGA block: add the necessary node to support it.
> > >>>
> > >>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
> > >>> ---
> > >>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> > >>>  1 file changed, 11 insertions(+)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> index 75908c587511..4bfbee9d4123 100644
> > >>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> > >>>  		status = "disabled";
> > >>>  	};
> > >>>  
> > >>> +	rga: rga@ff480000 {
> > >>> +		compatible = "rockchip,px30-rga";
> > >>> +		reg = <0x0 0xff480000 0x0 0x10000>;
> > >>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > >>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > >>> +		clock-names = "aclk", "hclk", "sclk";
> > >>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > >>> +		reset-names = "core", "axi", "ahb";
> > >>> +		power-domains = <&power PX30_PD_VO>;
> > >>
> > >> 		status = "disabled";
> > > 
> > > As of 5.6, the rk3399 has the node enabled by default. Did that change?
> > 
> > 'status' disappeared during review for rk3399 between v2 and v3, but
> > doesn't mention the reason. If someone can give more info here?
> > 
> > https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org/
> > 
> > https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org/
> > 
> > > 
> > > Since it's a standalone block that has no I/O dependency, I don't really see
> > > the point of disabling it by default.
> > 
> > Vop, hdmi and other video devices are also disabled.
> > Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.

just to add my me too ... what Paul, Ezequiel and Robin said - so no status
for soc internal components without any board-specifics


Heiko

Re: [PATCH 2/4] arm64: dts: rockchip: Add RGA support to the PX30

[Heiko Stuebner]

Am Donnerstag, 16. April 2020, 15:55:19 CEST schrieb Paul Kocialkowski:
> Hi,
> 
> On Thu 16 Apr 20, 15:44, Johan Jonker wrote:
> > On 4/16/20 3:24 PM, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > On Thu 16 Apr 20, 15:02, Johan Jonker wrote:
> > >> Hi Paul,
> > >>
> > >> The conversion of rockchip-rga.txt to rockchip-rga.yaml by myself just
> > >> has been approved by robh.
> > > 
> > > Huh, I looked around for ongoing related work but missed it.
> > > I'll definitely rebase on top of your series and use the yaml description
> > > instead. Thanks!
> > > 
> > >> Maybe place dts patches at the end of a patch serie.
> > >> Could you include a &rga patch if your device is supported in mainline,
> > >> so we can test with:
> > >> make ARCH=arm64 dtbs_check
> > >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-rga.yaml
> > > 
> > > I tested with the PX30 EVB so I can surely add a node there if that turns
> > > out necessary (see below).
> > > 
> > >> Johan
> > >>
> > >> On 4/16/20 1:50 PM, Paul Kocialkowski wrote:
> > >>> The PX30 features a RGA block: add the necessary node to support it.
> > >>>
> > >>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > >>> ---
> > >>>  arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> > >>>  1 file changed, 11 insertions(+)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> index 75908c587511..4bfbee9d4123 100644
> > >>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > >>> @@ -1104,6 +1104,17 @@ vopl_mmu: iommu@ff470f00 {
> > >>>  		status = "disabled";
> > >>>  	};
> > >>>  
> > >>> +	rga: rga@ff480000 {
> > >>> +		compatible = "rockchip,px30-rga";
> > >>> +		reg = <0x0 0xff480000 0x0 0x10000>;
> > >>> +		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > >>> +		clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > >>> +		clock-names = "aclk", "hclk", "sclk";
> > >>> +		resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > >>> +		reset-names = "core", "axi", "ahb";
> > >>> +		power-domains = <&power PX30_PD_VO>;
> > >>
> > >> 		status = "disabled";
> > > 
> > > As of 5.6, the rk3399 has the node enabled by default. Did that change?
> > 
> > 'status' disappeared during review for rk3399 between v2 and v3, but
> > doesn't mention the reason. If someone can give more info here?
> > 
> > https://lore.kernel.org/lkml/1500101920-24039-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > https://lore.kernel.org/lkml/1501470460-12014-5-git-send-email-jacob-chen@iotwrt.com/
> > 
> > > 
> > > Since it's a standalone block that has no I/O dependency, I don't really see
> > > the point of disabling it by default.
> > 
> > Vop, hdmi and other video devices are also disabled.
> > Follow the rest I think...
> 
> Well, these blocks do have related I/O ports so it makes sense not to enable
> them by default because of pinmux, or because there might be no connector
> populated/routed.
> 
> For a memory to memory internal block, I don't see any reason why.
> It's definitely not board-specific and having to add these nodes for every board
> that has them is kind of a pain and might be overlooked. This will easily result
> in the feature not being available for end users without having to change the
> dt.
> 
> Also, the vpu node is always enabled on rockchip (and sunxi) platforms.
> I think these are better examples to follow.

just to add my me too ... what Paul, Ezequiel and Robin said - so no status
for soc internal components without any board-specifics


Heiko



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