2021-04-23 10:15:58

by Mirela Rabulea OSS

[permalink] [raw]
Subject: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes

From: Mirela Rabulea <[email protected]>

Add dts for imaging subsytem, include jpeg nodes here.
Tested on imx8qxp only, should work on imx8qm, but it was not tested.

Signed-off-by: Mirela Rabulea <[email protected]>
---
Changes in v11:
Adress feedback from Aisheng Dong:
- Rename img_jpeg_dec_clk/img_jpeg_enc_clk to jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other type of clk
- Drop the cameradev node, not needed for jpeg
- Match assigned-clocks & assigned-clock-rates

.../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82 +++++++++++++++++++
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 1 +
2 files changed, 83 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/imx8-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..c508e5d0c92b
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019-2021 NXP
+ * Zhou Guoniu <[email protected]>
+ */
+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";
+ };
+
+ 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>;
+ };
+
+ jpegdec: jpegdec@58400000 {
+ compatible = "nxp,imx8qxp-jpgdec";
+ 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 0>,
+ <&img_jpeg_dec_lpcg 1>;
+ clock-names = "per", "ipg";
+ assigned-clocks = <&img_jpeg_dec_lpcg 0>,
+ <&img_jpeg_dec_lpcg 1>;
+ 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 {
+ compatible = "nxp,imx8qxp-jpgenc";
+ 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 0>,
+ <&img_jpeg_enc_lpcg 1>;
+ clock-names = "per", "ipg";
+ assigned-clocks = <&img_jpeg_enc_lpcg 0>,
+ <&img_jpeg_enc_lpcg 1>;
+ 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>;
+ };
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 1e6b4995091e..2d9589309bd0 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -258,6 +258,7 @@
};

/* sorted in register address */
+ #include "imx8-ss-img.dtsi"
#include "imx8-ss-adma.dtsi"
#include "imx8-ss-conn.dtsi"
#include "imx8-ss-ddr.dtsi"
--
2.17.1


2021-05-13 07:40:41

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes

On Fri, Apr 23, 2021 at 01:14:14PM +0300, Mirela Rabulea (OSS) wrote:
> From: Mirela Rabulea <[email protected]>
>
> Add dts for imaging subsytem, include jpeg nodes here.
> Tested on imx8qxp only, should work on imx8qm, but it was not tested.
>
> Signed-off-by: Mirela Rabulea <[email protected]>

So the bindings and driver parts have been accepted already?

> ---
> Changes in v11:
> Adress feedback from Aisheng Dong:
> - Rename img_jpeg_dec_clk/img_jpeg_enc_clk to jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other type of clk
> - Drop the cameradev node, not needed for jpeg
> - Match assigned-clocks & assigned-clock-rates
>
> .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82 +++++++++++++++++++
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 1 +
> 2 files changed, 83 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8-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..c508e5d0c92b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019-2021 NXP
> + * Zhou Guoniu <[email protected]>
> + */
> +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";
> + };

Hmm, not sure a fixed-clock should be in the subsystem.

> +
> + 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>;
> + };
> +
> + jpegdec: jpegdec@58400000 {

Keep nodes sorted in unit address.

Shawn

> + compatible = "nxp,imx8qxp-jpgdec";
> + 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 0>,
> + <&img_jpeg_dec_lpcg 1>;
> + clock-names = "per", "ipg";
> + assigned-clocks = <&img_jpeg_dec_lpcg 0>,
> + <&img_jpeg_dec_lpcg 1>;
> + 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 {
> + compatible = "nxp,imx8qxp-jpgenc";
> + 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 0>,
> + <&img_jpeg_enc_lpcg 1>;
> + clock-names = "per", "ipg";
> + assigned-clocks = <&img_jpeg_enc_lpcg 0>,
> + <&img_jpeg_enc_lpcg 1>;
> + 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>;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 1e6b4995091e..2d9589309bd0 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -258,6 +258,7 @@
> };
>
> /* sorted in register address */
> + #include "imx8-ss-img.dtsi"
> #include "imx8-ss-adma.dtsi"
> #include "imx8-ss-conn.dtsi"
> #include "imx8-ss-ddr.dtsi"
> --
> 2.17.1
>

2021-05-14 10:53:09

by Abel Vesa

[permalink] [raw]
Subject: Re: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes

On 21-05-13 15:38:33, Shawn Guo wrote:
> On Fri, Apr 23, 2021 at 01:14:14PM +0300, Mirela Rabulea (OSS) wrote:
> > From: Mirela Rabulea <[email protected]>
> >
> > Add dts for imaging subsytem, include jpeg nodes here.
> > Tested on imx8qxp only, should work on imx8qm, but it was not tested.
> >
> > Signed-off-by: Mirela Rabulea <[email protected]>
>
> So the bindings and driver parts have been accepted already?
>
> > ---
> > Changes in v11:
> > Adress feedback from Aisheng Dong:
> > - Rename img_jpeg_dec_clk/img_jpeg_enc_clk to jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other type of clk
> > - Drop the cameradev node, not needed for jpeg
> > - Match assigned-clocks & assigned-clock-rates
> >
> > .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82 +++++++++++++++++++
> > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 1 +
> > 2 files changed, 83 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/freescale/imx8-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..c508e5d0c92b
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > @@ -0,0 +1,82 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2021 NXP
> > + * Zhou Guoniu <[email protected]>
> > + */
> > +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";
> > + };
>
> Hmm, not sure a fixed-clock should be in the subsystem.
>

Agreed. Assuming the img_ipg_clk is only used on 8QXP, you could move it
into imx8qxp-ss-img.dtsi. This way every other platform that uses this
ss file will not be impacted.

> > +
> > + 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>;
> > + };
> > +
> > + jpegdec: jpegdec@58400000 {
>
> Keep nodes sorted in unit address.
>
> Shawn
>
> > + compatible = "nxp,imx8qxp-jpgdec";
> > + 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 0>,
> > + <&img_jpeg_dec_lpcg 1>;
> > + clock-names = "per", "ipg";
> > + assigned-clocks = <&img_jpeg_dec_lpcg 0>,
> > + <&img_jpeg_dec_lpcg 1>;
> > + 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 {
> > + compatible = "nxp,imx8qxp-jpgenc";
> > + 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 0>,
> > + <&img_jpeg_enc_lpcg 1>;
> > + clock-names = "per", "ipg";
> > + assigned-clocks = <&img_jpeg_enc_lpcg 0>,
> > + <&img_jpeg_enc_lpcg 1>;
> > + 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>;
> > + };
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index 1e6b4995091e..2d9589309bd0 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -258,6 +258,7 @@
> > };
> >
> > /* sorted in register address */
> > + #include "imx8-ss-img.dtsi"
> > #include "imx8-ss-adma.dtsi"
> > #include "imx8-ss-conn.dtsi"
> > #include "imx8-ss-ddr.dtsi"
> > --
> > 2.17.1
> >

2021-05-19 15:46:15

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes

> From: Shawn Guo <[email protected]>
> Sent: Thursday, May 13, 2021 3:39 PM
>
> On Fri, Apr 23, 2021 at 01:14:14PM +0300, Mirela Rabulea (OSS) wrote:
> > From: Mirela Rabulea <[email protected]>
> >
> > Add dts for imaging subsytem, include jpeg nodes here.
> > Tested on imx8qxp only, should work on imx8qm, but it was not tested.
> >
> > Signed-off-by: Mirela Rabulea <[email protected]>
>
> So the bindings and driver parts have been accepted already?
>
> > ---
> > Changes in v11:
> > Adress feedback from Aisheng Dong:
> > - Rename img_jpeg_dec_clk/img_jpeg_enc_clk to
> jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other type of clk
> > - Drop the cameradev node, not needed for jpeg
> > - Match assigned-clocks & assigned-clock-rates
> >
> > .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82
> +++++++++++++++++++
> > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 1 +
> > 2 files changed, 83 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/freescale/imx8-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..c508e5d0c92b
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > @@ -0,0 +1,82 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2021 NXP
> > + * Zhou Guoniu <[email protected]>
> > + */
> > +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";
> > + };
>
> Hmm, not sure a fixed-clock should be in the subsystem.

Each subsystem has its own fixed clock slice. (Independent with
other subsystems). So we put it in the subsystem dtsi.

Regards
Aisheng

2021-05-19 15:55:23

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes

> From: Abel Vesa <[email protected]>
> Sent: Friday, May 14, 2021 6:25 PM
>
> On 21-05-13 15:38:33, Shawn Guo wrote:
> > On Fri, Apr 23, 2021 at 01:14:14PM +0300, Mirela Rabulea (OSS) wrote:
> > > From: Mirela Rabulea <[email protected]>
> > >
> > > Add dts for imaging subsytem, include jpeg nodes here.
> > > Tested on imx8qxp only, should work on imx8qm, but it was not tested.
> > >
> > > Signed-off-by: Mirela Rabulea <[email protected]>
> >
> > So the bindings and driver parts have been accepted already?
> >
> > > ---
> > > Changes in v11:
> > > Adress feedback from Aisheng Dong:
> > > - Rename img_jpeg_dec_clk/img_jpeg_enc_clk to
> jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other type of clk
> > > - Drop the cameradev node, not needed for jpeg
> > > - Match assigned-clocks & assigned-clock-rates
> > >
> > > .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82
> +++++++++++++++++++
> > > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 1 +
> > > 2 files changed, 83 insertions(+)
> > > create mode 100644 arch/arm64/boot/dts/freescale/imx8-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..c508e5d0c92b
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > > @@ -0,0 +1,82 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2019-2021 NXP
> > > + * Zhou Guoniu <[email protected]> */
> > > +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";
> > > + };
> >
> > Hmm, not sure a fixed-clock should be in the subsystem.
> >
>
> Agreed. Assuming the img_ipg_clk is only used on 8QXP, you could move it
> into imx8qxp-ss-img.dtsi. This way every other platform that uses this ss file
> will not be impacted.
>

Imx_ipg_clk is used in this patch.
Here we define subsys.dtsi mostly based on MX8QXP.
MX8QM can overwrite it in QM subsys.dtsi if needed.

Regards
Aisheng


> > > +
> > > + 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>;
> > > + };
> > > +
> > > + jpegdec: jpegdec@58400000 {
> >
> > Keep nodes sorted in unit address.
> >
> > Shawn
> >
> > > + compatible = "nxp,imx8qxp-jpgdec";
> > > + 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 0>,
> > > + <&img_jpeg_dec_lpcg 1>;
> > > + clock-names = "per", "ipg";
> > > + assigned-clocks = <&img_jpeg_dec_lpcg 0>,
> > > + <&img_jpeg_dec_lpcg 1>;
> > > + 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 {
> > > + compatible = "nxp,imx8qxp-jpgenc";
> > > + 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 0>,
> > > + <&img_jpeg_enc_lpcg 1>;
> > > + clock-names = "per", "ipg";
> > > + assigned-clocks = <&img_jpeg_enc_lpcg 0>,
> > > + <&img_jpeg_enc_lpcg 1>;
> > > + 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>;
> > > + };
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > index 1e6b4995091e..2d9589309bd0 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > > @@ -258,6 +258,7 @@
> > > };
> > >
> > > /* sorted in register address */
> > > + #include "imx8-ss-img.dtsi"
> > > #include "imx8-ss-adma.dtsi"
> > > #include "imx8-ss-conn.dtsi"
> > > #include "imx8-ss-ddr.dtsi"
> > > --
> > > 2.17.1
> > >

2021-05-19 15:56:45

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes

> From: Mirela Rabulea (OSS) <[email protected]>
> Sent: Friday, April 23, 2021 6:14 PM
>
> Add dts for imaging subsytem, include jpeg nodes here.
> Tested on imx8qxp only, should work on imx8qm, but it was not tested.
>
> Signed-off-by: Mirela Rabulea <[email protected]>
> ---
> Changes in v11:
> Adress feedback from Aisheng Dong:
> - Rename img_jpeg_dec_clk/img_jpeg_enc_clk to
> jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other type of clk
> - Drop the cameradev node, not needed for jpeg
> - Match assigned-clocks & assigned-clock-rates
>
> .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82 +++++++++++++++++++
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 1 +
> 2 files changed, 83 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8-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..c508e5d0c92b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019-2021 NXP
> + * Zhou Guoniu <[email protected]>
> + */
> +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";
> + };
> +
> + 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>;
> + };
> +
> + jpegdec: jpegdec@58400000 {
> + compatible = "nxp,imx8qxp-jpgdec";
> + 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 0>,
> + <&img_jpeg_dec_lpcg 1>;

Pls fix LPCG index by using clock indice

> + clock-names = "per", "ipg";
> + assigned-clocks = <&img_jpeg_dec_lpcg 0>,
> + <&img_jpeg_dec_lpcg 1>;

Ditto

> + 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 {
> + compatible = "nxp,imx8qxp-jpgenc";
> + 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 0>,
> + <&img_jpeg_enc_lpcg 1>;

Ditto

> + clock-names = "per", "ipg";
> + assigned-clocks = <&img_jpeg_enc_lpcg 0>,
> + <&img_jpeg_enc_lpcg 1>;

Ditto

Regards
Aisheng

> + 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>;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 1e6b4995091e..2d9589309bd0 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -258,6 +258,7 @@
> };
>
> /* sorted in register address */
> + #include "imx8-ss-img.dtsi"
> #include "imx8-ss-adma.dtsi"
> #include "imx8-ss-conn.dtsi"
> #include "imx8-ss-ddr.dtsi"
> --
> 2.17.1

2021-05-19 19:13:00

by Mirela Rabulea

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes

Hi Shawn,

On Thu, 2021-05-13 at 15:38 +0800, Shawn Guo wrote:
>
> On Fri, Apr 23, 2021 at 01:14:14PM +0300, Mirela Rabulea (OSS) wrote:
> > From: Mirela Rabulea <[email protected]>
> >
> > Add dts for imaging subsytem, include jpeg nodes here.
> > Tested on imx8qxp only, should work on imx8qm, but it was not
> > tested.
> >
> > Signed-off-by: Mirela Rabulea <[email protected]>
>
> So the bindings and driver parts have been accepted already?

Yes, they are already in linux-next.

>
> > ---
> > Changes in v11:
> > Adress feedback from Aisheng Dong:
> > - Rename img_jpeg_dec_clk/img_jpeg_enc_clk to
> > jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other
> > type of clk
> > - Drop the cameradev node, not needed for jpeg
> > - Match assigned-clocks & assigned-clock-rates
> >
> > .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82
> > +++++++++++++++++++
> > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 1 +
> > 2 files changed, 83 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/freescale/imx8-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..c508e5d0c92b
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > @@ -0,0 +1,82 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2021 NXP
> > + * Zhou Guoniu <[email protected]>
> > + */
> > +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";
> > + };
>
> Hmm, not sure a fixed-clock should be in the subsystem.

I checked the reference manuals for 8QXP and 8QM (where this sybsystem
could be included) and this clock is the same, 200 MHz.
Also, as Aisheng mentioned, we will be able to overwrite in QM subsytem
if something is needed.
The same approach is done in the following subsystems, which also
contain fixed-clocks:
arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
arch/arm64/boot/dts/freescale/imx8-ss-dma.dtsi
arch/arm64/boot/dts/freescale/imx8-ss-lsio.dtsi

>
> > +
> > + 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>;
> > + };
> > +
> > + jpegdec: jpegdec@58400000 {
>
> Keep nodes sorted in unit address.

I fixed this in v12. Thanks for the feedback.

>
> Shawn
>
> > + compatible = "nxp,imx8qxp-jpgdec";
> > + 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 0>,
> > + <&img_jpeg_dec_lpcg 1>;
> > + clock-names = "per", "ipg";
> > + assigned-clocks = <&img_jpeg_dec_lpcg 0>,
> > + <&img_jpeg_dec_lpcg 1>;
> > + 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 {
> > + compatible = "nxp,imx8qxp-jpgenc";
> > + 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 0>,
> > + <&img_jpeg_enc_lpcg 1>;
> > + clock-names = "per", "ipg";
> > + assigned-clocks = <&img_jpeg_enc_lpcg 0>,
> > + <&img_jpeg_enc_lpcg 1>;
> > + 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>;
> > + };
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index 1e6b4995091e..2d9589309bd0 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -258,6 +258,7 @@
> > };
> >
> > /* sorted in register address */
> > + #include "imx8-ss-img.dtsi"
> > #include "imx8-ss-adma.dtsi"
> > #include "imx8-ss-conn.dtsi"
> > #include "imx8-ss-ddr.dtsi"
> > --
> > 2.17.1
> >