All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v13 0/2] Add dts and bindings update for i.MX8QM/QXP JPEG codec
@ 2021-05-22 21:10 ` Mirela Rabulea (OSS)
  0 siblings, 0 replies; 18+ messages in thread
From: Mirela Rabulea (OSS) @ 2021-05-22 21:10 UTC (permalink / raw)
  To: robh+dt, shawnguo, aisheng.dong, guoniu.zhou, linux-arm-kernel, mchehab
  Cc: peng.fan, s.hauer, linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, daniel.baluta,
	robert.chiras, laurentiu.palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

From: Mirela Rabulea <mirela.rabulea@nxp.com>

Add the dts files for i.MX8QM/QXP JPEG codec (previous patch was v12).
The bindings for i.MX8QXP were already upstream, add i.MX8QM compatible,
this is a new patch.

Mirela Rabulea (2):
  media: dt-bindings: imx-jpeg: Add compatible for i.MX8QM JPEG codec
  arm64: dts: imx: Add jpeg encoder/decoder nodes

 .../bindings/media/nxp,imx8-jpeg.yaml         | 17 ++--
 .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 80 +++++++++++++++++++
 .../boot/dts/freescale/imx8qm-ss-img.dtsi     | 12 +++
 arch/arm64/boot/dts/freescale/imx8qm.dtsi     |  2 +
 .../boot/dts/freescale/imx8qxp-ss-img.dtsi    | 13 +++
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  2 +
 6 files changed, 119 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi

-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v13 0/2] Add dts and bindings update for i.MX8QM/QXP JPEG codec
@ 2021-05-22 21:10 ` Mirela Rabulea (OSS)
  0 siblings, 0 replies; 18+ messages in thread
From: Mirela Rabulea (OSS) @ 2021-05-22 21:10 UTC (permalink / raw)
  To: robh+dt, shawnguo, aisheng.dong, guoniu.zhou, linux-arm-kernel, mchehab
  Cc: peng.fan, s.hauer, linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, daniel.baluta,
	robert.chiras, laurentiu.palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

From: Mirela Rabulea <mirela.rabulea@nxp.com>

Add the dts files for i.MX8QM/QXP JPEG codec (previous patch was v12).
The bindings for i.MX8QXP were already upstream, add i.MX8QM compatible,
this is a new patch.

Mirela Rabulea (2):
  media: dt-bindings: imx-jpeg: Add compatible for i.MX8QM JPEG codec
  arm64: dts: imx: Add jpeg encoder/decoder nodes

 .../bindings/media/nxp,imx8-jpeg.yaml         | 17 ++--
 .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 80 +++++++++++++++++++
 .../boot/dts/freescale/imx8qm-ss-img.dtsi     | 12 +++
 arch/arm64/boot/dts/freescale/imx8qm.dtsi     |  2 +
 .../boot/dts/freescale/imx8qxp-ss-img.dtsi    | 13 +++
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  2 +
 6 files changed, 119 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi

-- 
2.17.1


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v13 1/2] media: dt-bindings: imx-jpeg: Add compatible for i.MX8QM JPEG codec
  2021-05-22 21:10 ` Mirela Rabulea (OSS)
@ 2021-05-22 21:10   ` Mirela Rabulea (OSS)
  -1 siblings, 0 replies; 18+ messages in thread
From: Mirela Rabulea (OSS) @ 2021-05-22 21:10 UTC (permalink / raw)
  To: robh+dt, shawnguo, aisheng.dong, guoniu.zhou, linux-arm-kernel, mchehab
  Cc: peng.fan, s.hauer, linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, daniel.baluta,
	robert.chiras, laurentiu.palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

From: Mirela Rabulea <mirela.rabulea@nxp.com>

Add two more compatibles: "nxp,imx8qm-jpgdec" and " nxp,imx8qm-jpgenc".
Also update the compatible property to ensure mutually exclusive usage of
encoder and decoder compatibles.
Update examples.

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
 .../bindings/media/nxp,imx8-jpeg.yaml           | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
index 5d13cbb5251b..5cc7b6a94c44 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
@@ -16,12 +16,15 @@ description: |-
 
 properties:
   compatible:
-    items:
-      - enum:
-            # JPEG decoder
-          - nxp,imx8qxp-jpgdec
-            # JPEG encoder
-          - nxp,imx8qxp-jpgenc
+    oneOf:
+      - items:
+          anyOf:
+            - const: nxp,imx8qxp-jpgdec
+            - const: nxp,imx8qm-jpgdec
+      - items:
+          anyOf:
+            - const: nxp,imx8qxp-jpgenc
+            - const: nxp,imx8qm-jpgenc
 
   reg:
     maxItems: 1
@@ -69,7 +72,7 @@ examples:
     };
 
     jpegenc: jpegenc@58450000 {
-        compatible = "nxp,imx8qxp-jpgenc";
+        compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
         reg = <0x58450000 0x00050000 >;
         interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
                      <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v13 1/2] media: dt-bindings: imx-jpeg: Add compatible for i.MX8QM JPEG codec
@ 2021-05-22 21:10   ` Mirela Rabulea (OSS)
  0 siblings, 0 replies; 18+ messages in thread
From: Mirela Rabulea (OSS) @ 2021-05-22 21:10 UTC (permalink / raw)
  To: robh+dt, shawnguo, aisheng.dong, guoniu.zhou, linux-arm-kernel, mchehab
  Cc: peng.fan, s.hauer, linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, daniel.baluta,
	robert.chiras, laurentiu.palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

From: Mirela Rabulea <mirela.rabulea@nxp.com>

Add two more compatibles: "nxp,imx8qm-jpgdec" and " nxp,imx8qm-jpgenc".
Also update the compatible property to ensure mutually exclusive usage of
encoder and decoder compatibles.
Update examples.

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
 .../bindings/media/nxp,imx8-jpeg.yaml           | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
index 5d13cbb5251b..5cc7b6a94c44 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
@@ -16,12 +16,15 @@ description: |-
 
 properties:
   compatible:
-    items:
-      - enum:
-            # JPEG decoder
-          - nxp,imx8qxp-jpgdec
-            # JPEG encoder
-          - nxp,imx8qxp-jpgenc
+    oneOf:
+      - items:
+          anyOf:
+            - const: nxp,imx8qxp-jpgdec
+            - const: nxp,imx8qm-jpgdec
+      - items:
+          anyOf:
+            - const: nxp,imx8qxp-jpgenc
+            - const: nxp,imx8qm-jpgenc
 
   reg:
     maxItems: 1
@@ -69,7 +72,7 @@ examples:
     };
 
     jpegenc: jpegenc@58450000 {
-        compatible = "nxp,imx8qxp-jpgenc";
+        compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
         reg = <0x58450000 0x00050000 >;
         interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
                      <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
  2021-05-22 21:10 ` Mirela Rabulea (OSS)
@ 2021-05-22 21:10   ` Mirela Rabulea (OSS)
  -1 siblings, 0 replies; 18+ messages in thread
From: Mirela Rabulea (OSS) @ 2021-05-22 21:10 UTC (permalink / raw)
  To: robh+dt, shawnguo, aisheng.dong, guoniu.zhou, linux-arm-kernel, mchehab
  Cc: peng.fan, s.hauer, linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, daniel.baluta,
	robert.chiras, laurentiu.palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

From: Mirela Rabulea <mirela.rabulea@nxp.com>

Add dts for imaging subsytem, include jpeg nodes here.
Tested on imx8qxp/qm.

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
Changes in v13:
  Adress feedback from Aisheng Dong and update the commit message:
  - Move jpeg compatibles from imx8-ss-img.dtsi to imx8qxp-ss-img.dtsi
  - Add imx8qm-ss-img.dtsi for i.MX8QM

 .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 80 +++++++++++++++++++
 .../boot/dts/freescale/imx8qm-ss-img.dtsi     | 12 +++
 arch/arm64/boot/dts/freescale/imx8qm.dtsi     |  2 +
 .../boot/dts/freescale/imx8qxp-ss-img.dtsi    | 13 +++
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  2 +
 5 files changed, 109 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
new file mode 100644
index 000000000000..4b8456bb4712
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019-2021 NXP
+ * Zhou Guoniu <guoniu.zhou@nxp.com>
+ */
+img_subsys: bus@58000000 {
+	compatible = "simple-bus";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0x58000000 0x0 0x58000000 0x1000000>;
+
+	img_ipg_clk: clock-img-ipg {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <200000000>;
+		clock-output-names = "img_ipg_clk";
+	};
+
+	jpegdec: jpegdec@58400000 {
+		reg = <0x58400000 0x00050000 >;
+		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
+			 <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
+		clock-names = "per", "ipg";
+		assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
+				  <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
+		assigned-clock-rates = <200000000>, <200000000>;
+		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
+				<&pd IMX_SC_R_MJPEG_DEC_S0>,
+				<&pd IMX_SC_R_MJPEG_DEC_S1>,
+				<&pd IMX_SC_R_MJPEG_DEC_S2>,
+				<&pd IMX_SC_R_MJPEG_DEC_S3>;
+	};
+
+	jpegenc: jpegenc@58450000 {
+		reg = <0x58450000 0x00050000 >;
+		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
+			 <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
+		clock-names = "per", "ipg";
+		assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
+				  <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
+		assigned-clock-rates = <200000000>, <200000000>;
+		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
+				<&pd IMX_SC_R_MJPEG_ENC_S0>,
+				<&pd IMX_SC_R_MJPEG_ENC_S1>,
+				<&pd IMX_SC_R_MJPEG_ENC_S2>,
+				<&pd IMX_SC_R_MJPEG_ENC_S3>;
+	};
+
+	img_jpeg_dec_lpcg: clock-controller@585d0000 {
+		compatible = "fsl,imx8qxp-lpcg";
+		reg = <0x585d0000 0x10000>;
+		#clock-cells = <1>;
+		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
+		clock-indices = <IMX_LPCG_CLK_0>,
+				<IMX_LPCG_CLK_4>;
+		clock-output-names = "img_jpeg_dec_lpcg_clk",
+				     "img_jpeg_dec_lpcg_ipg_clk";
+		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
+	};
+
+	img_jpeg_enc_lpcg: clock-controller@585f0000 {
+		compatible = "fsl,imx8qxp-lpcg";
+		reg = <0x585f0000 0x10000>;
+		#clock-cells = <1>;
+		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
+		clock-indices = <IMX_LPCG_CLK_0>,
+				<IMX_LPCG_CLK_4>;
+		clock-output-names = "img_jpeg_enc_lpcg_clk",
+				     "img_jpeg_enc_lpcg_ipg_clk";
+		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
+	};
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
new file mode 100644
index 000000000000..7764b4146e0a
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 NXP
+ */
+
+&jpegdec {
+	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec";
+};
+
+&jpegenc {
+	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgenc";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
index 12cd059b339b..aebbe2b84aa1 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
@@ -166,11 +166,13 @@
 	};
 
 	/* sorted in register address */
+	#include "imx8-ss-img.dtsi"
 	#include "imx8-ss-dma.dtsi"
 	#include "imx8-ss-conn.dtsi"
 	#include "imx8-ss-lsio.dtsi"
 };
 
+#include "imx8qm-ss-img.dtsi"
 #include "imx8qm-ss-dma.dtsi"
 #include "imx8qm-ss-conn.dtsi"
 #include "imx8qm-ss-lsio.dtsi"
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
new file mode 100644
index 000000000000..3a087317591d
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 NXP
+ *	Dong Aisheng <aisheng.dong@nxp.com>
+ */
+
+&jpegdec {
+	compatible = "nxp,imx8qxp-jpgdec";
+};
+
+&jpegenc {
+	compatible = "nxp,imx8qxp-jpgenc";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 1e6b4995091e..a625fb6bdc62 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -258,12 +258,14 @@
 	};
 
 	/* sorted in register address */
+	#include "imx8-ss-img.dtsi"
 	#include "imx8-ss-adma.dtsi"
 	#include "imx8-ss-conn.dtsi"
 	#include "imx8-ss-ddr.dtsi"
 	#include "imx8-ss-lsio.dtsi"
 };
 
+#include "imx8qxp-ss-img.dtsi"
 #include "imx8qxp-ss-adma.dtsi"
 #include "imx8qxp-ss-conn.dtsi"
 #include "imx8qxp-ss-lsio.dtsi"
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
@ 2021-05-22 21:10   ` Mirela Rabulea (OSS)
  0 siblings, 0 replies; 18+ messages in thread
From: Mirela Rabulea (OSS) @ 2021-05-22 21:10 UTC (permalink / raw)
  To: robh+dt, shawnguo, aisheng.dong, guoniu.zhou, linux-arm-kernel, mchehab
  Cc: peng.fan, s.hauer, linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, daniel.baluta,
	robert.chiras, laurentiu.palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

From: Mirela Rabulea <mirela.rabulea@nxp.com>

Add dts for imaging subsytem, include jpeg nodes here.
Tested on imx8qxp/qm.

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
Changes in v13:
  Adress feedback from Aisheng Dong and update the commit message:
  - Move jpeg compatibles from imx8-ss-img.dtsi to imx8qxp-ss-img.dtsi
  - Add imx8qm-ss-img.dtsi for i.MX8QM

 .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 80 +++++++++++++++++++
 .../boot/dts/freescale/imx8qm-ss-img.dtsi     | 12 +++
 arch/arm64/boot/dts/freescale/imx8qm.dtsi     |  2 +
 .../boot/dts/freescale/imx8qxp-ss-img.dtsi    | 13 +++
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  2 +
 5 files changed, 109 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
new file mode 100644
index 000000000000..4b8456bb4712
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019-2021 NXP
+ * Zhou Guoniu <guoniu.zhou@nxp.com>
+ */
+img_subsys: bus@58000000 {
+	compatible = "simple-bus";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0x58000000 0x0 0x58000000 0x1000000>;
+
+	img_ipg_clk: clock-img-ipg {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <200000000>;
+		clock-output-names = "img_ipg_clk";
+	};
+
+	jpegdec: jpegdec@58400000 {
+		reg = <0x58400000 0x00050000 >;
+		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
+			 <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
+		clock-names = "per", "ipg";
+		assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
+				  <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
+		assigned-clock-rates = <200000000>, <200000000>;
+		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
+				<&pd IMX_SC_R_MJPEG_DEC_S0>,
+				<&pd IMX_SC_R_MJPEG_DEC_S1>,
+				<&pd IMX_SC_R_MJPEG_DEC_S2>,
+				<&pd IMX_SC_R_MJPEG_DEC_S3>;
+	};
+
+	jpegenc: jpegenc@58450000 {
+		reg = <0x58450000 0x00050000 >;
+		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
+			 <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
+		clock-names = "per", "ipg";
+		assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
+				  <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
+		assigned-clock-rates = <200000000>, <200000000>;
+		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
+				<&pd IMX_SC_R_MJPEG_ENC_S0>,
+				<&pd IMX_SC_R_MJPEG_ENC_S1>,
+				<&pd IMX_SC_R_MJPEG_ENC_S2>,
+				<&pd IMX_SC_R_MJPEG_ENC_S3>;
+	};
+
+	img_jpeg_dec_lpcg: clock-controller@585d0000 {
+		compatible = "fsl,imx8qxp-lpcg";
+		reg = <0x585d0000 0x10000>;
+		#clock-cells = <1>;
+		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
+		clock-indices = <IMX_LPCG_CLK_0>,
+				<IMX_LPCG_CLK_4>;
+		clock-output-names = "img_jpeg_dec_lpcg_clk",
+				     "img_jpeg_dec_lpcg_ipg_clk";
+		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
+	};
+
+	img_jpeg_enc_lpcg: clock-controller@585f0000 {
+		compatible = "fsl,imx8qxp-lpcg";
+		reg = <0x585f0000 0x10000>;
+		#clock-cells = <1>;
+		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
+		clock-indices = <IMX_LPCG_CLK_0>,
+				<IMX_LPCG_CLK_4>;
+		clock-output-names = "img_jpeg_enc_lpcg_clk",
+				     "img_jpeg_enc_lpcg_ipg_clk";
+		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
+	};
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
new file mode 100644
index 000000000000..7764b4146e0a
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 NXP
+ */
+
+&jpegdec {
+	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec";
+};
+
+&jpegenc {
+	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgenc";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
index 12cd059b339b..aebbe2b84aa1 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
@@ -166,11 +166,13 @@
 	};
 
 	/* sorted in register address */
+	#include "imx8-ss-img.dtsi"
 	#include "imx8-ss-dma.dtsi"
 	#include "imx8-ss-conn.dtsi"
 	#include "imx8-ss-lsio.dtsi"
 };
 
+#include "imx8qm-ss-img.dtsi"
 #include "imx8qm-ss-dma.dtsi"
 #include "imx8qm-ss-conn.dtsi"
 #include "imx8qm-ss-lsio.dtsi"
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
new file mode 100644
index 000000000000..3a087317591d
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 NXP
+ *	Dong Aisheng <aisheng.dong@nxp.com>
+ */
+
+&jpegdec {
+	compatible = "nxp,imx8qxp-jpgdec";
+};
+
+&jpegenc {
+	compatible = "nxp,imx8qxp-jpgenc";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 1e6b4995091e..a625fb6bdc62 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -258,12 +258,14 @@
 	};
 
 	/* sorted in register address */
+	#include "imx8-ss-img.dtsi"
 	#include "imx8-ss-adma.dtsi"
 	#include "imx8-ss-conn.dtsi"
 	#include "imx8-ss-ddr.dtsi"
 	#include "imx8-ss-lsio.dtsi"
 };
 
+#include "imx8qxp-ss-img.dtsi"
 #include "imx8qxp-ss-adma.dtsi"
 #include "imx8qxp-ss-conn.dtsi"
 #include "imx8qxp-ss-lsio.dtsi"
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* RE: [PATCH v13 1/2] media: dt-bindings: imx-jpeg: Add compatible for i.MX8QM JPEG codec
  2021-05-22 21:10   ` Mirela Rabulea (OSS)
@ 2021-05-24  3:19     ` Aisheng Dong
  -1 siblings, 0 replies; 18+ messages in thread
From: Aisheng Dong @ 2021-05-24  3:19 UTC (permalink / raw)
  To: Mirela Rabulea (OSS),
	robh+dt, shawnguo, G.n. Zhou, linux-arm-kernel, mchehab
  Cc: Peng Fan, s.hauer, dl-linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, Daniel Baluta,
	Robert Chiras, Laurentiu Palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

> From: Mirela Rabulea (OSS) <mirela.rabulea@oss.nxp.com>
> Sent: Sunday, May 23, 2021 5:11 AM
> 
> Add two more compatibles: "nxp,imx8qm-jpgdec" and " nxp,imx8qm-jpgenc".
> Also update the compatible property to ensure mutually exclusive usage of
> encoder and decoder compatibles.
> Update examples.
> 
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
>  .../bindings/media/nxp,imx8-jpeg.yaml           | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> index 5d13cbb5251b..5cc7b6a94c44 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> @@ -16,12 +16,15 @@ description: |-
> 
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -            # JPEG decoder
> -          - nxp,imx8qxp-jpgdec
> -            # JPEG encoder
> -          - nxp,imx8qxp-jpgenc
> +    oneOf:
> +      - items:
> +          anyOf:
> +            - const: nxp,imx8qxp-jpgdec
> +            - const: nxp,imx8qm-jpgdec
> +      - items:
> +          anyOf:

I might wish to avoid using anyOF for jpeg cases and defined in a more
Straightforward way.

e.g.
properties:
  compatible:
    oneOf:
      - items:
        - enum:
              # JPEG decoder
          - nxp,imx8qxp-jpgdec
            # JPEG encoder
          - nxp,imx8qxp-jpgenc
      - items:
          - enum:
              - nxp,imx8qm-jpgdec
          - const: nxp,imx8qxp-jpgdec
      - items:
          - enum:
              - nxp,imx8qm-jpgenc
          - const: nxp,imx8qxp-jpgenc
Could you check if it works?

Regards
Aisheng

> +            - const: nxp,imx8qxp-jpgenc
> +            - const: nxp,imx8qm-jpgenc
> 
>    reg:
>      maxItems: 1
> @@ -69,7 +72,7 @@ examples:
>      };
> 
>      jpegenc: jpegenc@58450000 {
> -        compatible = "nxp,imx8qxp-jpgenc";
> +        compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
>          reg = <0x58450000 0x00050000 >;
>          interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
>                       <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v13 1/2] media: dt-bindings: imx-jpeg: Add compatible for i.MX8QM JPEG codec
@ 2021-05-24  3:19     ` Aisheng Dong
  0 siblings, 0 replies; 18+ messages in thread
From: Aisheng Dong @ 2021-05-24  3:19 UTC (permalink / raw)
  To: Mirela Rabulea (OSS),
	robh+dt, shawnguo, G.n. Zhou, linux-arm-kernel, mchehab
  Cc: Peng Fan, s.hauer, dl-linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, Daniel Baluta,
	Robert Chiras, Laurentiu Palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

> From: Mirela Rabulea (OSS) <mirela.rabulea@oss.nxp.com>
> Sent: Sunday, May 23, 2021 5:11 AM
> 
> Add two more compatibles: "nxp,imx8qm-jpgdec" and " nxp,imx8qm-jpgenc".
> Also update the compatible property to ensure mutually exclusive usage of
> encoder and decoder compatibles.
> Update examples.
> 
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
>  .../bindings/media/nxp,imx8-jpeg.yaml           | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> index 5d13cbb5251b..5cc7b6a94c44 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> @@ -16,12 +16,15 @@ description: |-
> 
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -            # JPEG decoder
> -          - nxp,imx8qxp-jpgdec
> -            # JPEG encoder
> -          - nxp,imx8qxp-jpgenc
> +    oneOf:
> +      - items:
> +          anyOf:
> +            - const: nxp,imx8qxp-jpgdec
> +            - const: nxp,imx8qm-jpgdec
> +      - items:
> +          anyOf:

I might wish to avoid using anyOF for jpeg cases and defined in a more
Straightforward way.

e.g.
properties:
  compatible:
    oneOf:
      - items:
        - enum:
              # JPEG decoder
          - nxp,imx8qxp-jpgdec
            # JPEG encoder
          - nxp,imx8qxp-jpgenc
      - items:
          - enum:
              - nxp,imx8qm-jpgdec
          - const: nxp,imx8qxp-jpgdec
      - items:
          - enum:
              - nxp,imx8qm-jpgenc
          - const: nxp,imx8qxp-jpgenc
Could you check if it works?

Regards
Aisheng

> +            - const: nxp,imx8qxp-jpgenc
> +            - const: nxp,imx8qm-jpgenc
> 
>    reg:
>      maxItems: 1
> @@ -69,7 +72,7 @@ examples:
>      };
> 
>      jpegenc: jpegenc@58450000 {
> -        compatible = "nxp,imx8qxp-jpgenc";
> +        compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
>          reg = <0x58450000 0x00050000 >;
>          interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
>                       <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> --
> 2.17.1

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
  2021-05-22 21:10   ` Mirela Rabulea (OSS)
@ 2021-05-24  3:28     ` Aisheng Dong
  -1 siblings, 0 replies; 18+ messages in thread
From: Aisheng Dong @ 2021-05-24  3:28 UTC (permalink / raw)
  To: Mirela Rabulea (OSS),
	robh+dt, shawnguo, G.n. Zhou, linux-arm-kernel, mchehab
  Cc: Peng Fan, s.hauer, dl-linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, Daniel Baluta,
	Robert Chiras, Laurentiu Palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

> From: Mirela Rabulea (OSS) <mirela.rabulea@oss.nxp.com>
> Sent: Sunday, May 23, 2021 5:11 AM
> Subject: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
> 

s/imx/imx8

> 
> Add dts for imaging subsytem, include jpeg nodes here.
> Tested on imx8qxp/qm.
> 
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
> Changes in v13:
>   Adress feedback from Aisheng Dong and update the commit message:
>   - Move jpeg compatibles from imx8-ss-img.dtsi to imx8qxp-ss-img.dtsi
>   - Add imx8qm-ss-img.dtsi for i.MX8QM
> 
>  .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 80 +++++++++++++++++++
>  .../boot/dts/freescale/imx8qm-ss-img.dtsi     | 12 +++
>  arch/arm64/boot/dts/freescale/imx8qm.dtsi     |  2 +
>  .../boot/dts/freescale/imx8qxp-ss-img.dtsi    | 13 +++
>  arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  2 +
>  5 files changed, 109 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> new file mode 100644
> index 000000000000..4b8456bb4712
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019-2021 NXP
> + * Zhou Guoniu <guoniu.zhou@nxp.com>
> + */
> +img_subsys: bus@58000000 {
> +	compatible = "simple-bus";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> +
> +	img_ipg_clk: clock-img-ipg {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <200000000>;
> +		clock-output-names = "img_ipg_clk";
> +	};
> +
> +	jpegdec: jpegdec@58400000 {

Node should be disabled by default.
And enable it in board dts including LPCG.

> +		reg = <0x58400000 0x00050000 >;
> +		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> +			 <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> +		clock-names = "per", "ipg";
> +		assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> +				  <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> +		assigned-clock-rates = <200000000>, <200000000>;
> +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
> +				<&pd IMX_SC_R_MJPEG_DEC_S0>,
> +				<&pd IMX_SC_R_MJPEG_DEC_S1>,
> +				<&pd IMX_SC_R_MJPEG_DEC_S2>,
> +				<&pd IMX_SC_R_MJPEG_DEC_S3>;
> +	};
> +
> +	jpegenc: jpegenc@58450000 {

Ditto

> +		reg = <0x58450000 0x00050000 >;
> +		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> +			 <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> +		clock-names = "per", "ipg";
> +		assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> +				  <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> +		assigned-clock-rates = <200000000>, <200000000>;
> +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
> +				<&pd IMX_SC_R_MJPEG_ENC_S0>,
> +				<&pd IMX_SC_R_MJPEG_ENC_S1>,
> +				<&pd IMX_SC_R_MJPEG_ENC_S2>,
> +				<&pd IMX_SC_R_MJPEG_ENC_S3>;
> +	};
> +
> +	img_jpeg_dec_lpcg: clock-controller@585d0000 {

Ditto

> +		compatible = "fsl,imx8qxp-lpcg";
> +		reg = <0x585d0000 0x10000>;
> +		#clock-cells = <1>;
> +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> +		clock-indices = <IMX_LPCG_CLK_0>,
> +				<IMX_LPCG_CLK_4>;
> +		clock-output-names = "img_jpeg_dec_lpcg_clk",
> +				     "img_jpeg_dec_lpcg_ipg_clk";
> +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
> +	};
> +
> +	img_jpeg_enc_lpcg: clock-controller@585f0000 {
> +		compatible = "fsl,imx8qxp-lpcg";

Ditto

Otherwise, I'm fine with this patch.

> +		reg = <0x585f0000 0x10000>;
> +		#clock-cells = <1>;
> +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> +		clock-indices = <IMX_LPCG_CLK_0>,
> +				<IMX_LPCG_CLK_4>;
> +		clock-output-names = "img_jpeg_enc_lpcg_clk",
> +				     "img_jpeg_enc_lpcg_ipg_clk";
> +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> new file mode 100644
> index 000000000000..7764b4146e0a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021 NXP
> + */
> +
> +&jpegdec {
> +	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec"; };
> +
> +&jpegenc {
> +	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgenc"; };
> diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> index 12cd059b339b..aebbe2b84aa1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> @@ -166,11 +166,13 @@
>  	};
> 
>  	/* sorted in register address */
> +	#include "imx8-ss-img.dtsi"
>  	#include "imx8-ss-dma.dtsi"
>  	#include "imx8-ss-conn.dtsi"
>  	#include "imx8-ss-lsio.dtsi"
>  };
> 
> +#include "imx8qm-ss-img.dtsi"
>  #include "imx8qm-ss-dma.dtsi"
>  #include "imx8qm-ss-conn.dtsi"
>  #include "imx8qm-ss-lsio.dtsi"
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> new file mode 100644
> index 000000000000..3a087317591d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021 NXP
> + *	Dong Aisheng <aisheng.dong@nxp.com>
> + */
> +
> +&jpegdec {
> +	compatible = "nxp,imx8qxp-jpgdec";
> +};
> +
> +&jpegenc {
> +	compatible = "nxp,imx8qxp-jpgenc";
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 1e6b4995091e..a625fb6bdc62 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -258,12 +258,14 @@
>  	};
> 
>  	/* sorted in register address */
> +	#include "imx8-ss-img.dtsi"
>  	#include "imx8-ss-adma.dtsi"
>  	#include "imx8-ss-conn.dtsi"
>  	#include "imx8-ss-ddr.dtsi"
>  	#include "imx8-ss-lsio.dtsi"
>  };
> 
> +#include "imx8qxp-ss-img.dtsi"
>  #include "imx8qxp-ss-adma.dtsi"
>  #include "imx8qxp-ss-conn.dtsi"
>  #include "imx8qxp-ss-lsio.dtsi"
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
@ 2021-05-24  3:28     ` Aisheng Dong
  0 siblings, 0 replies; 18+ messages in thread
From: Aisheng Dong @ 2021-05-24  3:28 UTC (permalink / raw)
  To: Mirela Rabulea (OSS),
	robh+dt, shawnguo, G.n. Zhou, linux-arm-kernel, mchehab
  Cc: Peng Fan, s.hauer, dl-linux-imx, devicetree, hverkuil-cisco,
	linux-media, linux-kernel, paul.kocialkowski, Daniel Baluta,
	Robert Chiras, Laurentiu Palcu, p.zabel, ezequiel, kernel,
	Mirela Rabulea

> From: Mirela Rabulea (OSS) <mirela.rabulea@oss.nxp.com>
> Sent: Sunday, May 23, 2021 5:11 AM
> Subject: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
> 

s/imx/imx8

> 
> Add dts for imaging subsytem, include jpeg nodes here.
> Tested on imx8qxp/qm.
> 
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
> Changes in v13:
>   Adress feedback from Aisheng Dong and update the commit message:
>   - Move jpeg compatibles from imx8-ss-img.dtsi to imx8qxp-ss-img.dtsi
>   - Add imx8qm-ss-img.dtsi for i.MX8QM
> 
>  .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 80 +++++++++++++++++++
>  .../boot/dts/freescale/imx8qm-ss-img.dtsi     | 12 +++
>  arch/arm64/boot/dts/freescale/imx8qm.dtsi     |  2 +
>  .../boot/dts/freescale/imx8qxp-ss-img.dtsi    | 13 +++
>  arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  2 +
>  5 files changed, 109 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> new file mode 100644
> index 000000000000..4b8456bb4712
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019-2021 NXP
> + * Zhou Guoniu <guoniu.zhou@nxp.com>
> + */
> +img_subsys: bus@58000000 {
> +	compatible = "simple-bus";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> +
> +	img_ipg_clk: clock-img-ipg {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <200000000>;
> +		clock-output-names = "img_ipg_clk";
> +	};
> +
> +	jpegdec: jpegdec@58400000 {

Node should be disabled by default.
And enable it in board dts including LPCG.

> +		reg = <0x58400000 0x00050000 >;
> +		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> +			 <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> +		clock-names = "per", "ipg";
> +		assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> +				  <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> +		assigned-clock-rates = <200000000>, <200000000>;
> +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
> +				<&pd IMX_SC_R_MJPEG_DEC_S0>,
> +				<&pd IMX_SC_R_MJPEG_DEC_S1>,
> +				<&pd IMX_SC_R_MJPEG_DEC_S2>,
> +				<&pd IMX_SC_R_MJPEG_DEC_S3>;
> +	};
> +
> +	jpegenc: jpegenc@58450000 {

Ditto

> +		reg = <0x58450000 0x00050000 >;
> +		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> +			 <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> +		clock-names = "per", "ipg";
> +		assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> +				  <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> +		assigned-clock-rates = <200000000>, <200000000>;
> +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
> +				<&pd IMX_SC_R_MJPEG_ENC_S0>,
> +				<&pd IMX_SC_R_MJPEG_ENC_S1>,
> +				<&pd IMX_SC_R_MJPEG_ENC_S2>,
> +				<&pd IMX_SC_R_MJPEG_ENC_S3>;
> +	};
> +
> +	img_jpeg_dec_lpcg: clock-controller@585d0000 {

Ditto

> +		compatible = "fsl,imx8qxp-lpcg";
> +		reg = <0x585d0000 0x10000>;
> +		#clock-cells = <1>;
> +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> +		clock-indices = <IMX_LPCG_CLK_0>,
> +				<IMX_LPCG_CLK_4>;
> +		clock-output-names = "img_jpeg_dec_lpcg_clk",
> +				     "img_jpeg_dec_lpcg_ipg_clk";
> +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
> +	};
> +
> +	img_jpeg_enc_lpcg: clock-controller@585f0000 {
> +		compatible = "fsl,imx8qxp-lpcg";

Ditto

Otherwise, I'm fine with this patch.

> +		reg = <0x585f0000 0x10000>;
> +		#clock-cells = <1>;
> +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> +		clock-indices = <IMX_LPCG_CLK_0>,
> +				<IMX_LPCG_CLK_4>;
> +		clock-output-names = "img_jpeg_enc_lpcg_clk",
> +				     "img_jpeg_enc_lpcg_ipg_clk";
> +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> new file mode 100644
> index 000000000000..7764b4146e0a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021 NXP
> + */
> +
> +&jpegdec {
> +	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec"; };
> +
> +&jpegenc {
> +	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgenc"; };
> diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> index 12cd059b339b..aebbe2b84aa1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> @@ -166,11 +166,13 @@
>  	};
> 
>  	/* sorted in register address */
> +	#include "imx8-ss-img.dtsi"
>  	#include "imx8-ss-dma.dtsi"
>  	#include "imx8-ss-conn.dtsi"
>  	#include "imx8-ss-lsio.dtsi"
>  };
> 
> +#include "imx8qm-ss-img.dtsi"
>  #include "imx8qm-ss-dma.dtsi"
>  #include "imx8qm-ss-conn.dtsi"
>  #include "imx8qm-ss-lsio.dtsi"
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> new file mode 100644
> index 000000000000..3a087317591d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021 NXP
> + *	Dong Aisheng <aisheng.dong@nxp.com>
> + */
> +
> +&jpegdec {
> +	compatible = "nxp,imx8qxp-jpgdec";
> +};
> +
> +&jpegenc {
> +	compatible = "nxp,imx8qxp-jpgenc";
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 1e6b4995091e..a625fb6bdc62 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -258,12 +258,14 @@
>  	};
> 
>  	/* sorted in register address */
> +	#include "imx8-ss-img.dtsi"
>  	#include "imx8-ss-adma.dtsi"
>  	#include "imx8-ss-conn.dtsi"
>  	#include "imx8-ss-ddr.dtsi"
>  	#include "imx8-ss-lsio.dtsi"
>  };
> 
> +#include "imx8qxp-ss-img.dtsi"
>  #include "imx8qxp-ss-adma.dtsi"
>  #include "imx8qxp-ss-conn.dtsi"
>  #include "imx8qxp-ss-lsio.dtsi"
> --
> 2.17.1

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
  2021-05-24  3:28     ` Aisheng Dong
@ 2021-05-24  7:17       ` Mirela Rabulea
  -1 siblings, 0 replies; 18+ messages in thread
From: Mirela Rabulea @ 2021-05-24  7:17 UTC (permalink / raw)
  To: linux-arm-kernel, ezequiel, mchehab, shawnguo, robh+dt,
	Aisheng Dong, G.n. Zhou
  Cc: dl-linux-imx, linux-kernel, Laurentiu Palcu, linux-media,
	paul.kocialkowski, devicetree, Robert Chiras, p.zabel, Peng Fan,
	hverkuil-cisco, Daniel Baluta, kernel, s.hauer

Hi Aisheng, Ezequiel,

On Mon, 2021-05-24 at 03:28 +0000, Aisheng Dong wrote:
> > From: Mirela Rabulea (OSS) <mirela.rabulea@oss.nxp.com>
> > Sent: Sunday, May 23, 2021 5:11 AM
> > Subject: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder
> > nodes
> > 
> 
> s/imx/imx8

Ok, I'll use imx8 in patch subject.

> 
> > 
> > Add dts for imaging subsytem, include jpeg nodes here.
> > Tested on imx8qxp/qm.
> > 
> > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> > ---
> > Changes in v13:
> >   Adress feedback from Aisheng Dong and update the commit message:
> >   - Move jpeg compatibles from imx8-ss-img.dtsi to imx8qxp-ss-
> > img.dtsi
> >   - Add imx8qm-ss-img.dtsi for i.MX8QM
> > 
> >  .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 80
> > +++++++++++++++++++
> >  .../boot/dts/freescale/imx8qm-ss-img.dtsi     | 12 +++
> >  arch/arm64/boot/dts/freescale/imx8qm.dtsi     |  2 +
> >  .../boot/dts/freescale/imx8qxp-ss-img.dtsi    | 13 +++
> >  arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  2 +
> >  5 files changed, 109 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8qm-ss-
> > img.dtsi
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-
> > img.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > new file mode 100644
> > index 000000000000..4b8456bb4712
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > @@ -0,0 +1,80 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2021 NXP
> > + * Zhou Guoniu <guoniu.zhou@nxp.com>
> > + */
> > +img_subsys: bus@58000000 {
> > +	compatible = "simple-bus";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > +
> > +	img_ipg_clk: clock-img-ipg {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <200000000>;
> > +		clock-output-names = "img_ipg_clk";
> > +	};
> > +
> > +	jpegdec: jpegdec@58400000 {
> 
> Node should be disabled by default.
> And enable it in board dts including LPCG.

At version v5 of this patch, the node was disabled by default, and I
received this feedback from Ezequiel Garcia:

"Pure memory-to-memory are typically not enabled per-board, but just
per-platform.
So you can drop the disabled status here."

So, in v6 I made it enabled by default.

Any strong reasons for enabled/disabled per platform?

Thanks,
Mirela

> 
> > +		reg = <0x58400000 0x00050000 >;
> > +		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> > +			 <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> > +		clock-names = "per", "ipg";
> > +		assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> > +				  <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> > +		assigned-clock-rates = <200000000>, <200000000>;
> > +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S0>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S1>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S2>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S3>;
> > +	};
> > +
> > +	jpegenc: jpegenc@58450000 {
> 
> Ditto
> 
> > +		reg = <0x58450000 0x00050000 >;
> > +		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> > +			 <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> > +		clock-names = "per", "ipg";
> > +		assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> > +				  <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> > +		assigned-clock-rates = <200000000>, <200000000>;
> > +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S0>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S1>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S2>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S3>;
> > +	};
> > +
> > +	img_jpeg_dec_lpcg: clock-controller@585d0000 {
> 
> Ditto
> 
> > +		compatible = "fsl,imx8qxp-lpcg";
> > +		reg = <0x585d0000 0x10000>;
> > +		#clock-cells = <1>;
> > +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > +		clock-indices = <IMX_LPCG_CLK_0>,
> > +				<IMX_LPCG_CLK_4>;
> > +		clock-output-names = "img_jpeg_dec_lpcg_clk",
> > +				     "img_jpeg_dec_lpcg_ipg_clk";
> > +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
> > +	};
> > +
> > +	img_jpeg_enc_lpcg: clock-controller@585f0000 {
> > +		compatible = "fsl,imx8qxp-lpcg";
> 
> Ditto
> 
> Otherwise, I'm fine with this patch.
> 
> > +		reg = <0x585f0000 0x10000>;
> > +		#clock-cells = <1>;
> > +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > +		clock-indices = <IMX_LPCG_CLK_0>,
> > +				<IMX_LPCG_CLK_4>;
> > +		clock-output-names = "img_jpeg_enc_lpcg_clk",
> > +				     "img_jpeg_enc_lpcg_ipg_clk";
> > +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
> > +	};
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > new file mode 100644
> > index 000000000000..7764b4146e0a
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > @@ -0,0 +1,12 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2021 NXP
> > + */
> > +
> > +&jpegdec {
> > +	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec"; };
> > +
> > +&jpegenc {
> > +	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgenc"; };
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > index 12cd059b339b..aebbe2b84aa1 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > @@ -166,11 +166,13 @@
> >  	};
> > 
> >  	/* sorted in register address */
> > +	#include "imx8-ss-img.dtsi"
> >  	#include "imx8-ss-dma.dtsi"
> >  	#include "imx8-ss-conn.dtsi"
> >  	#include "imx8-ss-lsio.dtsi"
> >  };
> > 
> > +#include "imx8qm-ss-img.dtsi"
> >  #include "imx8qm-ss-dma.dtsi"
> >  #include "imx8qm-ss-conn.dtsi"
> >  #include "imx8qm-ss-lsio.dtsi"
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > new file mode 100644
> > index 000000000000..3a087317591d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2021 NXP
> > + *	Dong Aisheng <aisheng.dong@nxp.com>
> > + */
> > +
> > +&jpegdec {
> > +	compatible = "nxp,imx8qxp-jpgdec";
> > +};
> > +
> > +&jpegenc {
> > +	compatible = "nxp,imx8qxp-jpgenc";
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index 1e6b4995091e..a625fb6bdc62 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -258,12 +258,14 @@
> >  	};
> > 
> >  	/* sorted in register address */
> > +	#include "imx8-ss-img.dtsi"
> >  	#include "imx8-ss-adma.dtsi"
> >  	#include "imx8-ss-conn.dtsi"
> >  	#include "imx8-ss-ddr.dtsi"
> >  	#include "imx8-ss-lsio.dtsi"
> >  };
> > 
> > +#include "imx8qxp-ss-img.dtsi"
> >  #include "imx8qxp-ss-adma.dtsi"
> >  #include "imx8qxp-ss-conn.dtsi"
> >  #include "imx8qxp-ss-lsio.dtsi"
> > --
> > 2.17.1
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
@ 2021-05-24  7:17       ` Mirela Rabulea
  0 siblings, 0 replies; 18+ messages in thread
From: Mirela Rabulea @ 2021-05-24  7:17 UTC (permalink / raw)
  To: linux-arm-kernel, ezequiel, mchehab, shawnguo, robh+dt,
	Aisheng Dong, G.n. Zhou
  Cc: dl-linux-imx, linux-kernel, Laurentiu Palcu, linux-media,
	paul.kocialkowski, devicetree, Robert Chiras, p.zabel, Peng Fan,
	hverkuil-cisco, Daniel Baluta, kernel, s.hauer

Hi Aisheng, Ezequiel,

On Mon, 2021-05-24 at 03:28 +0000, Aisheng Dong wrote:
> > From: Mirela Rabulea (OSS) <mirela.rabulea@oss.nxp.com>
> > Sent: Sunday, May 23, 2021 5:11 AM
> > Subject: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder
> > nodes
> > 
> 
> s/imx/imx8

Ok, I'll use imx8 in patch subject.

> 
> > 
> > Add dts for imaging subsytem, include jpeg nodes here.
> > Tested on imx8qxp/qm.
> > 
> > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> > ---
> > Changes in v13:
> >   Adress feedback from Aisheng Dong and update the commit message:
> >   - Move jpeg compatibles from imx8-ss-img.dtsi to imx8qxp-ss-
> > img.dtsi
> >   - Add imx8qm-ss-img.dtsi for i.MX8QM
> > 
> >  .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 80
> > +++++++++++++++++++
> >  .../boot/dts/freescale/imx8qm-ss-img.dtsi     | 12 +++
> >  arch/arm64/boot/dts/freescale/imx8qm.dtsi     |  2 +
> >  .../boot/dts/freescale/imx8qxp-ss-img.dtsi    | 13 +++
> >  arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  2 +
> >  5 files changed, 109 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8qm-ss-
> > img.dtsi
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-
> > img.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > new file mode 100644
> > index 000000000000..4b8456bb4712
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > @@ -0,0 +1,80 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2021 NXP
> > + * Zhou Guoniu <guoniu.zhou@nxp.com>
> > + */
> > +img_subsys: bus@58000000 {
> > +	compatible = "simple-bus";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > +
> > +	img_ipg_clk: clock-img-ipg {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <200000000>;
> > +		clock-output-names = "img_ipg_clk";
> > +	};
> > +
> > +	jpegdec: jpegdec@58400000 {
> 
> Node should be disabled by default.
> And enable it in board dts including LPCG.

At version v5 of this patch, the node was disabled by default, and I
received this feedback from Ezequiel Garcia:

"Pure memory-to-memory are typically not enabled per-board, but just
per-platform.
So you can drop the disabled status here."

So, in v6 I made it enabled by default.

Any strong reasons for enabled/disabled per platform?

Thanks,
Mirela

> 
> > +		reg = <0x58400000 0x00050000 >;
> > +		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> > +			 <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> > +		clock-names = "per", "ipg";
> > +		assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> > +				  <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> > +		assigned-clock-rates = <200000000>, <200000000>;
> > +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S0>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S1>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S2>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S3>;
> > +	};
> > +
> > +	jpegenc: jpegenc@58450000 {
> 
> Ditto
> 
> > +		reg = <0x58450000 0x00050000 >;
> > +		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> > +			 <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> > +		clock-names = "per", "ipg";
> > +		assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> > +				  <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> > +		assigned-clock-rates = <200000000>, <200000000>;
> > +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S0>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S1>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S2>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S3>;
> > +	};
> > +
> > +	img_jpeg_dec_lpcg: clock-controller@585d0000 {
> 
> Ditto
> 
> > +		compatible = "fsl,imx8qxp-lpcg";
> > +		reg = <0x585d0000 0x10000>;
> > +		#clock-cells = <1>;
> > +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > +		clock-indices = <IMX_LPCG_CLK_0>,
> > +				<IMX_LPCG_CLK_4>;
> > +		clock-output-names = "img_jpeg_dec_lpcg_clk",
> > +				     "img_jpeg_dec_lpcg_ipg_clk";
> > +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
> > +	};
> > +
> > +	img_jpeg_enc_lpcg: clock-controller@585f0000 {
> > +		compatible = "fsl,imx8qxp-lpcg";
> 
> Ditto
> 
> Otherwise, I'm fine with this patch.
> 
> > +		reg = <0x585f0000 0x10000>;
> > +		#clock-cells = <1>;
> > +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > +		clock-indices = <IMX_LPCG_CLK_0>,
> > +				<IMX_LPCG_CLK_4>;
> > +		clock-output-names = "img_jpeg_enc_lpcg_clk",
> > +				     "img_jpeg_enc_lpcg_ipg_clk";
> > +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
> > +	};
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > new file mode 100644
> > index 000000000000..7764b4146e0a
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > @@ -0,0 +1,12 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2021 NXP
> > + */
> > +
> > +&jpegdec {
> > +	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec"; };
> > +
> > +&jpegenc {
> > +	compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgenc"; };
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > index 12cd059b339b..aebbe2b84aa1 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > @@ -166,11 +166,13 @@
> >  	};
> > 
> >  	/* sorted in register address */
> > +	#include "imx8-ss-img.dtsi"
> >  	#include "imx8-ss-dma.dtsi"
> >  	#include "imx8-ss-conn.dtsi"
> >  	#include "imx8-ss-lsio.dtsi"
> >  };
> > 
> > +#include "imx8qm-ss-img.dtsi"
> >  #include "imx8qm-ss-dma.dtsi"
> >  #include "imx8qm-ss-conn.dtsi"
> >  #include "imx8qm-ss-lsio.dtsi"
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > new file mode 100644
> > index 000000000000..3a087317591d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2021 NXP
> > + *	Dong Aisheng <aisheng.dong@nxp.com>
> > + */
> > +
> > +&jpegdec {
> > +	compatible = "nxp,imx8qxp-jpgdec";
> > +};
> > +
> > +&jpegenc {
> > +	compatible = "nxp,imx8qxp-jpgenc";
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index 1e6b4995091e..a625fb6bdc62 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -258,12 +258,14 @@
> >  	};
> > 
> >  	/* sorted in register address */
> > +	#include "imx8-ss-img.dtsi"
> >  	#include "imx8-ss-adma.dtsi"
> >  	#include "imx8-ss-conn.dtsi"
> >  	#include "imx8-ss-ddr.dtsi"
> >  	#include "imx8-ss-lsio.dtsi"
> >  };
> > 
> > +#include "imx8qxp-ss-img.dtsi"
> >  #include "imx8qxp-ss-adma.dtsi"
> >  #include "imx8qxp-ss-conn.dtsi"
> >  #include "imx8qxp-ss-lsio.dtsi"
> > --
> > 2.17.1
> 
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
  2021-05-24  7:17       ` Mirela Rabulea
@ 2021-06-11 13:33         ` Dong Aisheng
  -1 siblings, 0 replies; 18+ messages in thread
From: Dong Aisheng @ 2021-06-11 13:33 UTC (permalink / raw)
  To: Mirela Rabulea
  Cc: linux-arm-kernel, ezequiel, mchehab, shawnguo, robh+dt,
	Aisheng Dong, G.n. Zhou, dl-linux-imx, linux-kernel,
	Laurentiu Palcu, linux-media, paul.kocialkowski, devicetree,
	Robert Chiras, p.zabel, Peng Fan, hverkuil-cisco, Daniel Baluta,
	kernel, s.hauer

[...]

> > > +img_subsys: bus@58000000 {
> > > +   compatible = "simple-bus";
> > > +   #address-cells = <1>;
> > > +   #size-cells = <1>;
> > > +   ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > > +
> > > +   img_ipg_clk: clock-img-ipg {
> > > +           compatible = "fixed-clock";
> > > +           #clock-cells = <0>;
> > > +           clock-frequency = <200000000>;
> > > +           clock-output-names = "img_ipg_clk";
> > > +   };
> > > +
> > > +   jpegdec: jpegdec@58400000 {
> >
> > Node should be disabled by default.
> > And enable it in board dts including LPCG.
>
> At version v5 of this patch, the node was disabled by default, and I
> received this feedback from Ezequiel Garcia:
>
> "Pure memory-to-memory are typically not enabled per-board, but just
> per-platform.
> So you can drop the disabled status here."
>
> So, in v6 I made it enabled by default.
>
> Any strong reasons for enabled/disabled per platform?

AFAIK we usually only enable system basic features and let other
user selectable features disabled by default in dts.
Even for device LPCG clocks, if it's enabled by default and later
enter runtime suspend if no users, it still consumes power.

Regards
Aisheng

>
> Thanks,
> Mirela
>
> >
> > > +           reg = <0x58400000 0x00050000 >;
> > > +           interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
> > > +           clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> > > +                    <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> > > +           clock-names = "per", "ipg";
> > > +           assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> > > +                             <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> > > +           assigned-clock-rates = <200000000>, <200000000>;
> > > +           power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
> > > +                           <&pd IMX_SC_R_MJPEG_DEC_S0>,
> > > +                           <&pd IMX_SC_R_MJPEG_DEC_S1>,
> > > +                           <&pd IMX_SC_R_MJPEG_DEC_S2>,
> > > +                           <&pd IMX_SC_R_MJPEG_DEC_S3>;
> > > +   };
> > > +
> > > +   jpegenc: jpegenc@58450000 {
> >
> > Ditto
> >
> > > +           reg = <0x58450000 0x00050000 >;
> > > +           interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
> > > +           clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> > > +                    <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> > > +           clock-names = "per", "ipg";
> > > +           assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> > > +                             <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> > > +           assigned-clock-rates = <200000000>, <200000000>;
> > > +           power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
> > > +                           <&pd IMX_SC_R_MJPEG_ENC_S0>,
> > > +                           <&pd IMX_SC_R_MJPEG_ENC_S1>,
> > > +                           <&pd IMX_SC_R_MJPEG_ENC_S2>,
> > > +                           <&pd IMX_SC_R_MJPEG_ENC_S3>;
> > > +   };
> > > +
> > > +   img_jpeg_dec_lpcg: clock-controller@585d0000 {
> >
> > Ditto
> >
> > > +           compatible = "fsl,imx8qxp-lpcg";
> > > +           reg = <0x585d0000 0x10000>;
> > > +           #clock-cells = <1>;
> > > +           clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > > +           clock-indices = <IMX_LPCG_CLK_0>,
> > > +                           <IMX_LPCG_CLK_4>;
> > > +           clock-output-names = "img_jpeg_dec_lpcg_clk",
> > > +                                "img_jpeg_dec_lpcg_ipg_clk";
> > > +           power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
> > > +   };
> > > +
> > > +   img_jpeg_enc_lpcg: clock-controller@585f0000 {
> > > +           compatible = "fsl,imx8qxp-lpcg";
> >
> > Ditto
> >
> > Otherwise, I'm fine with this patch.
> >
> > > +           reg = <0x585f0000 0x10000>;
> > > +           #clock-cells = <1>;
> > > +           clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > > +           clock-indices = <IMX_LPCG_CLK_0>,
> > > +                           <IMX_LPCG_CLK_4>;
> > > +           clock-output-names = "img_jpeg_enc_lpcg_clk",
> > > +                                "img_jpeg_enc_lpcg_ipg_clk";
> > > +           power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
> > > +   };
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > > new file mode 100644
> > > index 000000000000..7764b4146e0a
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > > @@ -0,0 +1,12 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2021 NXP
> > > + */
> > > +
> > > +&jpegdec {
> > > +   compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec"; };
> > > +
> > > +&jpegenc {
> > > +   compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgenc"; };
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > > index 12cd059b339b..aebbe2b84aa1 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > > @@ -166,11 +166,13 @@
> > >     };
> > >
> > >     /* sorted in register address */
> > > +   #include "imx8-ss-img.dtsi"
> > >     #include "imx8-ss-dma.dtsi"
> > >     #include "imx8-ss-conn.dtsi"
> > >     #include "imx8-ss-lsio.dtsi"
> > >  };
> > >
> > > +#include "imx8qm-ss-img.dtsi"
> > >  #include "imx8qm-ss-dma.dtsi"
> > >  #include "imx8qm-ss-conn.dtsi"
> > >  #include "imx8qm-ss-lsio.dtsi"
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > > new file mode 100644
> > > index 000000000000..3a087317591d
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > > @@ -0,0 +1,13 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2021 NXP
> > > + * Dong Aisheng <aisheng.dong@nxp.com>
> > > + */
> > > +
> > > +&jpegdec {
> > > +   compatible = "nxp,imx8qxp-jpgdec";
> > > +};
> > > +
> > > +&jpegenc {
> > > +   compatible = "nxp,imx8qxp-jpgenc";
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > index 1e6b4995091e..a625fb6bdc62 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > @@ -258,12 +258,14 @@
> > >     };
> > >
> > >     /* sorted in register address */
> > > +   #include "imx8-ss-img.dtsi"
> > >     #include "imx8-ss-adma.dtsi"
> > >     #include "imx8-ss-conn.dtsi"
> > >     #include "imx8-ss-ddr.dtsi"
> > >     #include "imx8-ss-lsio.dtsi"
> > >  };
> > >
> > > +#include "imx8qxp-ss-img.dtsi"
> > >  #include "imx8qxp-ss-adma.dtsi"
> > >  #include "imx8qxp-ss-conn.dtsi"
> > >  #include "imx8qxp-ss-lsio.dtsi"
> > > --
> > > 2.17.1
> >
> >
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
@ 2021-06-11 13:33         ` Dong Aisheng
  0 siblings, 0 replies; 18+ messages in thread
From: Dong Aisheng @ 2021-06-11 13:33 UTC (permalink / raw)
  To: Mirela Rabulea
  Cc: linux-arm-kernel, ezequiel, mchehab, shawnguo, robh+dt,
	Aisheng Dong, G.n. Zhou, dl-linux-imx, linux-kernel,
	Laurentiu Palcu, linux-media, paul.kocialkowski, devicetree,
	Robert Chiras, p.zabel, Peng Fan, hverkuil-cisco, Daniel Baluta,
	kernel, s.hauer

[...]

> > > +img_subsys: bus@58000000 {
> > > +   compatible = "simple-bus";
> > > +   #address-cells = <1>;
> > > +   #size-cells = <1>;
> > > +   ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > > +
> > > +   img_ipg_clk: clock-img-ipg {
> > > +           compatible = "fixed-clock";
> > > +           #clock-cells = <0>;
> > > +           clock-frequency = <200000000>;
> > > +           clock-output-names = "img_ipg_clk";
> > > +   };
> > > +
> > > +   jpegdec: jpegdec@58400000 {
> >
> > Node should be disabled by default.
> > And enable it in board dts including LPCG.
>
> At version v5 of this patch, the node was disabled by default, and I
> received this feedback from Ezequiel Garcia:
>
> "Pure memory-to-memory are typically not enabled per-board, but just
> per-platform.
> So you can drop the disabled status here."
>
> So, in v6 I made it enabled by default.
>
> Any strong reasons for enabled/disabled per platform?

AFAIK we usually only enable system basic features and let other
user selectable features disabled by default in dts.
Even for device LPCG clocks, if it's enabled by default and later
enter runtime suspend if no users, it still consumes power.

Regards
Aisheng

>
> Thanks,
> Mirela
>
> >
> > > +           reg = <0x58400000 0x00050000 >;
> > > +           interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
> > > +           clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> > > +                    <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> > > +           clock-names = "per", "ipg";
> > > +           assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
> > > +                             <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
> > > +           assigned-clock-rates = <200000000>, <200000000>;
> > > +           power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
> > > +                           <&pd IMX_SC_R_MJPEG_DEC_S0>,
> > > +                           <&pd IMX_SC_R_MJPEG_DEC_S1>,
> > > +                           <&pd IMX_SC_R_MJPEG_DEC_S2>,
> > > +                           <&pd IMX_SC_R_MJPEG_DEC_S3>;
> > > +   };
> > > +
> > > +   jpegenc: jpegenc@58450000 {
> >
> > Ditto
> >
> > > +           reg = <0x58450000 0x00050000 >;
> > > +           interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> > > +                        <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
> > > +           clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> > > +                    <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> > > +           clock-names = "per", "ipg";
> > > +           assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
> > > +                             <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
> > > +           assigned-clock-rates = <200000000>, <200000000>;
> > > +           power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
> > > +                           <&pd IMX_SC_R_MJPEG_ENC_S0>,
> > > +                           <&pd IMX_SC_R_MJPEG_ENC_S1>,
> > > +                           <&pd IMX_SC_R_MJPEG_ENC_S2>,
> > > +                           <&pd IMX_SC_R_MJPEG_ENC_S3>;
> > > +   };
> > > +
> > > +   img_jpeg_dec_lpcg: clock-controller@585d0000 {
> >
> > Ditto
> >
> > > +           compatible = "fsl,imx8qxp-lpcg";
> > > +           reg = <0x585d0000 0x10000>;
> > > +           #clock-cells = <1>;
> > > +           clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > > +           clock-indices = <IMX_LPCG_CLK_0>,
> > > +                           <IMX_LPCG_CLK_4>;
> > > +           clock-output-names = "img_jpeg_dec_lpcg_clk",
> > > +                                "img_jpeg_dec_lpcg_ipg_clk";
> > > +           power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
> > > +   };
> > > +
> > > +   img_jpeg_enc_lpcg: clock-controller@585f0000 {
> > > +           compatible = "fsl,imx8qxp-lpcg";
> >
> > Ditto
> >
> > Otherwise, I'm fine with this patch.
> >
> > > +           reg = <0x585f0000 0x10000>;
> > > +           #clock-cells = <1>;
> > > +           clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > > +           clock-indices = <IMX_LPCG_CLK_0>,
> > > +                           <IMX_LPCG_CLK_4>;
> > > +           clock-output-names = "img_jpeg_enc_lpcg_clk",
> > > +                                "img_jpeg_enc_lpcg_ipg_clk";
> > > +           power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
> > > +   };
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > > new file mode 100644
> > > index 000000000000..7764b4146e0a
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-img.dtsi
> > > @@ -0,0 +1,12 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2021 NXP
> > > + */
> > > +
> > > +&jpegdec {
> > > +   compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgdec"; };
> > > +
> > > +&jpegenc {
> > > +   compatible = "nxp,imx8qm-jpgdec", "nxp,imx8qxp-jpgenc"; };
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > > index 12cd059b339b..aebbe2b84aa1 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qm.dtsi
> > > @@ -166,11 +166,13 @@
> > >     };
> > >
> > >     /* sorted in register address */
> > > +   #include "imx8-ss-img.dtsi"
> > >     #include "imx8-ss-dma.dtsi"
> > >     #include "imx8-ss-conn.dtsi"
> > >     #include "imx8-ss-lsio.dtsi"
> > >  };
> > >
> > > +#include "imx8qm-ss-img.dtsi"
> > >  #include "imx8qm-ss-dma.dtsi"
> > >  #include "imx8qm-ss-conn.dtsi"
> > >  #include "imx8qm-ss-lsio.dtsi"
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > > new file mode 100644
> > > index 000000000000..3a087317591d
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> > > @@ -0,0 +1,13 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2021 NXP
> > > + * Dong Aisheng <aisheng.dong@nxp.com>
> > > + */
> > > +
> > > +&jpegdec {
> > > +   compatible = "nxp,imx8qxp-jpgdec";
> > > +};
> > > +
> > > +&jpegenc {
> > > +   compatible = "nxp,imx8qxp-jpgenc";
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > index 1e6b4995091e..a625fb6bdc62 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > @@ -258,12 +258,14 @@
> > >     };
> > >
> > >     /* sorted in register address */
> > > +   #include "imx8-ss-img.dtsi"
> > >     #include "imx8-ss-adma.dtsi"
> > >     #include "imx8-ss-conn.dtsi"
> > >     #include "imx8-ss-ddr.dtsi"
> > >     #include "imx8-ss-lsio.dtsi"
> > >  };
> > >
> > > +#include "imx8qxp-ss-img.dtsi"
> > >  #include "imx8qxp-ss-adma.dtsi"
> > >  #include "imx8qxp-ss-conn.dtsi"
> > >  #include "imx8qxp-ss-lsio.dtsi"
> > > --
> > > 2.17.1
> >
> >
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
  2021-06-11 13:33         ` Dong Aisheng
@ 2021-06-11 15:00           ` Ezequiel Garcia
  -1 siblings, 0 replies; 18+ messages in thread
From: Ezequiel Garcia @ 2021-06-11 15:00 UTC (permalink / raw)
  To: Dong Aisheng, Mirela Rabulea
  Cc: linux-arm-kernel, mchehab, shawnguo, robh+dt, Aisheng Dong,
	G.n. Zhou, dl-linux-imx, linux-kernel, Laurentiu Palcu,
	linux-media, paul.kocialkowski, devicetree, Robert Chiras,
	p.zabel, Peng Fan, hverkuil-cisco, Daniel Baluta, kernel,
	s.hauer

On Fri, 2021-06-11 at 21:33 +0800, Dong Aisheng wrote:
> [...]
> 
> > > > +img_subsys: bus@58000000 {
> > > > +   compatible = "simple-bus";
> > > > +   #address-cells = <1>;
> > > > +   #size-cells = <1>;
> > > > +   ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > > > +
> > > > +   img_ipg_clk: clock-img-ipg {
> > > > +           compatible = "fixed-clock";
> > > > +           #clock-cells = <0>;
> > > > +           clock-frequency = <200000000>;
> > > > +           clock-output-names = "img_ipg_clk";
> > > > +   };
> > > > +
> > > > +   jpegdec: jpegdec@58400000 {
> > > 
> > > Node should be disabled by default.
> > > And enable it in board dts including LPCG.
> > 
> > At version v5 of this patch, the node was disabled by default, and I
> > received this feedback from Ezequiel Garcia:
> > 
> > "Pure memory-to-memory are typically not enabled per-board, but just
> > per-platform.
> > So you can drop the disabled status here."
> > 
> > So, in v6 I made it enabled by default.
> > 
> > Any strong reasons for enabled/disabled per platform?
> 
> AFAIK we usually only enable system basic features and let other
> user selectable features disabled by default in dts.
> Even for device LPCG clocks, if it's enabled by default and later
> enter runtime suspend if no users, it still consumes power.
> 

Well-written drivers shouldn't draw any power if not used.

And DT is about hardware-description, not about usage-description.
Which means, at the soc.dtsi level you disable devices that need
some board-level hardware thing to be enabled (e.g. a physical
connected, a regulator, etc.).

A pure memory-to-memory should be enabled by default, because
in practice you can't predict what the users a board will want
to use, nor the DT is the place for that.

Sticking to hardware description is the best way to get DT right :-)

Cheers,
Ezequiel


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
@ 2021-06-11 15:00           ` Ezequiel Garcia
  0 siblings, 0 replies; 18+ messages in thread
From: Ezequiel Garcia @ 2021-06-11 15:00 UTC (permalink / raw)
  To: Dong Aisheng, Mirela Rabulea
  Cc: linux-arm-kernel, mchehab, shawnguo, robh+dt, Aisheng Dong,
	G.n. Zhou, dl-linux-imx, linux-kernel, Laurentiu Palcu,
	linux-media, paul.kocialkowski, devicetree, Robert Chiras,
	p.zabel, Peng Fan, hverkuil-cisco, Daniel Baluta, kernel,
	s.hauer

On Fri, 2021-06-11 at 21:33 +0800, Dong Aisheng wrote:
> [...]
> 
> > > > +img_subsys: bus@58000000 {
> > > > +   compatible = "simple-bus";
> > > > +   #address-cells = <1>;
> > > > +   #size-cells = <1>;
> > > > +   ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > > > +
> > > > +   img_ipg_clk: clock-img-ipg {
> > > > +           compatible = "fixed-clock";
> > > > +           #clock-cells = <0>;
> > > > +           clock-frequency = <200000000>;
> > > > +           clock-output-names = "img_ipg_clk";
> > > > +   };
> > > > +
> > > > +   jpegdec: jpegdec@58400000 {
> > > 
> > > Node should be disabled by default.
> > > And enable it in board dts including LPCG.
> > 
> > At version v5 of this patch, the node was disabled by default, and I
> > received this feedback from Ezequiel Garcia:
> > 
> > "Pure memory-to-memory are typically not enabled per-board, but just
> > per-platform.
> > So you can drop the disabled status here."
> > 
> > So, in v6 I made it enabled by default.
> > 
> > Any strong reasons for enabled/disabled per platform?
> 
> AFAIK we usually only enable system basic features and let other
> user selectable features disabled by default in dts.
> Even for device LPCG clocks, if it's enabled by default and later
> enter runtime suspend if no users, it still consumes power.
> 

Well-written drivers shouldn't draw any power if not used.

And DT is about hardware-description, not about usage-description.
Which means, at the soc.dtsi level you disable devices that need
some board-level hardware thing to be enabled (e.g. a physical
connected, a regulator, etc.).

A pure memory-to-memory should be enabled by default, because
in practice you can't predict what the users a board will want
to use, nor the DT is the place for that.

Sticking to hardware description is the best way to get DT right :-)

Cheers,
Ezequiel


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
  2021-06-11 15:00           ` Ezequiel Garcia
@ 2021-06-18 12:47             ` Dong Aisheng
  -1 siblings, 0 replies; 18+ messages in thread
From: Dong Aisheng @ 2021-06-18 12:47 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Mirela Rabulea, linux-arm-kernel, mchehab, shawnguo, robh+dt,
	Aisheng Dong, G.n. Zhou, dl-linux-imx, linux-kernel,
	Laurentiu Palcu, linux-media, paul.kocialkowski, devicetree,
	Robert Chiras, p.zabel, Peng Fan, hverkuil-cisco, Daniel Baluta,
	kernel, s.hauer

On Fri, Jun 11, 2021 at 11:01 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Fri, 2021-06-11 at 21:33 +0800, Dong Aisheng wrote:
> > [...]
> >
> > > > > +img_subsys: bus@58000000 {
> > > > > +   compatible = "simple-bus";
> > > > > +   #address-cells = <1>;
> > > > > +   #size-cells = <1>;
> > > > > +   ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > > > > +
> > > > > +   img_ipg_clk: clock-img-ipg {
> > > > > +           compatible = "fixed-clock";
> > > > > +           #clock-cells = <0>;
> > > > > +           clock-frequency = <200000000>;
> > > > > +           clock-output-names = "img_ipg_clk";
> > > > > +   };
> > > > > +
> > > > > +   jpegdec: jpegdec@58400000 {
> > > >
> > > > Node should be disabled by default.
> > > > And enable it in board dts including LPCG.
> > >
> > > At version v5 of this patch, the node was disabled by default, and I
> > > received this feedback from Ezequiel Garcia:
> > >
> > > "Pure memory-to-memory are typically not enabled per-board, but just
> > > per-platform.
> > > So you can drop the disabled status here."
> > >
> > > So, in v6 I made it enabled by default.
> > >
> > > Any strong reasons for enabled/disabled per platform?
> >
> > AFAIK we usually only enable system basic features and let other
> > user selectable features disabled by default in dts.
> > Even for device LPCG clocks, if it's enabled by default and later
> > enter runtime suspend if no users, it still consumes power.
> >
>
> Well-written drivers shouldn't draw any power if not used.
>

LPCG won't draw power when not used. But the power domain used by LPCG
will still draw power when enter idle state.

> And DT is about hardware-description, not about usage-description.
> Which means, at the soc.dtsi level you disable devices that need
> some board-level hardware thing to be enabled (e.g. a physical
> connected, a regulator, etc.).
> A pure memory-to-memory should be enabled by default, because
> in practice you can't predict what the users a board will want
> to use, nor the DT is the place for that.

It makes sense to me. Thanks

Mirela,
Please follow up with Ezequiel's suggestion.
For LPCGS used by jpeg, you can keep it enabled by default.

Regards
Aisheng

>
> Sticking to hardware description is the best way to get DT right :-)
>
> Cheers,
> Ezequiel
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes
@ 2021-06-18 12:47             ` Dong Aisheng
  0 siblings, 0 replies; 18+ messages in thread
From: Dong Aisheng @ 2021-06-18 12:47 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Mirela Rabulea, linux-arm-kernel, mchehab, shawnguo, robh+dt,
	Aisheng Dong, G.n. Zhou, dl-linux-imx, linux-kernel,
	Laurentiu Palcu, linux-media, paul.kocialkowski, devicetree,
	Robert Chiras, p.zabel, Peng Fan, hverkuil-cisco, Daniel Baluta,
	kernel, s.hauer

On Fri, Jun 11, 2021 at 11:01 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Fri, 2021-06-11 at 21:33 +0800, Dong Aisheng wrote:
> > [...]
> >
> > > > > +img_subsys: bus@58000000 {
> > > > > +   compatible = "simple-bus";
> > > > > +   #address-cells = <1>;
> > > > > +   #size-cells = <1>;
> > > > > +   ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > > > > +
> > > > > +   img_ipg_clk: clock-img-ipg {
> > > > > +           compatible = "fixed-clock";
> > > > > +           #clock-cells = <0>;
> > > > > +           clock-frequency = <200000000>;
> > > > > +           clock-output-names = "img_ipg_clk";
> > > > > +   };
> > > > > +
> > > > > +   jpegdec: jpegdec@58400000 {
> > > >
> > > > Node should be disabled by default.
> > > > And enable it in board dts including LPCG.
> > >
> > > At version v5 of this patch, the node was disabled by default, and I
> > > received this feedback from Ezequiel Garcia:
> > >
> > > "Pure memory-to-memory are typically not enabled per-board, but just
> > > per-platform.
> > > So you can drop the disabled status here."
> > >
> > > So, in v6 I made it enabled by default.
> > >
> > > Any strong reasons for enabled/disabled per platform?
> >
> > AFAIK we usually only enable system basic features and let other
> > user selectable features disabled by default in dts.
> > Even for device LPCG clocks, if it's enabled by default and later
> > enter runtime suspend if no users, it still consumes power.
> >
>
> Well-written drivers shouldn't draw any power if not used.
>

LPCG won't draw power when not used. But the power domain used by LPCG
will still draw power when enter idle state.

> And DT is about hardware-description, not about usage-description.
> Which means, at the soc.dtsi level you disable devices that need
> some board-level hardware thing to be enabled (e.g. a physical
> connected, a regulator, etc.).
> A pure memory-to-memory should be enabled by default, because
> in practice you can't predict what the users a board will want
> to use, nor the DT is the place for that.

It makes sense to me. Thanks

Mirela,
Please follow up with Ezequiel's suggestion.
For LPCGS used by jpeg, you can keep it enabled by default.

Regards
Aisheng

>
> Sticking to hardware description is the best way to get DT right :-)
>
> Cheers,
> Ezequiel
>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-06-18 12:51 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-22 21:10 [PATCH v13 0/2] Add dts and bindings update for i.MX8QM/QXP JPEG codec Mirela Rabulea (OSS)
2021-05-22 21:10 ` Mirela Rabulea (OSS)
2021-05-22 21:10 ` [PATCH v13 1/2] media: dt-bindings: imx-jpeg: Add compatible for i.MX8QM " Mirela Rabulea (OSS)
2021-05-22 21:10   ` Mirela Rabulea (OSS)
2021-05-24  3:19   ` Aisheng Dong
2021-05-24  3:19     ` Aisheng Dong
2021-05-22 21:10 ` [PATCH v13 2/2] arm64: dts: imx: Add jpeg encoder/decoder nodes Mirela Rabulea (OSS)
2021-05-22 21:10   ` Mirela Rabulea (OSS)
2021-05-24  3:28   ` Aisheng Dong
2021-05-24  3:28     ` Aisheng Dong
2021-05-24  7:17     ` Mirela Rabulea
2021-05-24  7:17       ` Mirela Rabulea
2021-06-11 13:33       ` Dong Aisheng
2021-06-11 13:33         ` Dong Aisheng
2021-06-11 15:00         ` Ezequiel Garcia
2021-06-11 15:00           ` Ezequiel Garcia
2021-06-18 12:47           ` Dong Aisheng
2021-06-18 12:47             ` Dong Aisheng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.