Re: [EXT] Re: [PATCH v5 05/10] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes

["Mirela Rabulea (OSS)" <mirela.rabulea@oss.nxp.com>]

Hi Ezequiel,

On Thu, 2020-11-12 at 05:36 -0300, Ezequiel Garcia wrote:
> 
> Hi Mirela,
> 
> On Thu, 2020-11-12 at 05:05 +0200, Mirela Rabulea (OSS) wrote:
> > From: Mirela Rabulea <mirela.rabulea@nxp.com>
> > 
> > Add jpeg decoder/encoder nodes, for now on imx8qxp only.
> > The same should work on imx8qm, but it was not tested.
> > 
> 
> Does imx8qm need changes in the dt bindings?
> 
> Unless you are aware of reasons preventing us from enabling
> it on imx8qm, then we could go for imx8qm as well (reusing
> imx8qxp- compatible).

I think it will be possible to reuse the same compatile for 8qm, too.
There is no dts for 8qm upstream for now, I understand Aisheng has
something started on that. We'll see how it goes.

> 
> > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8qxp-mek.dts |  8 ++++
> >  arch/arm64/boot/dts/freescale/imx8qxp.dtsi    | 37
> > +++++++++++++++++++
> >  2 files changed, 45 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > index 46437d3c7a04..a0ad9789e9b8 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > @@ -270,3 +270,11 @@
> >               >;
> >       };
> >  };
> > +
> > +&jpegdec {
> > +     status = "okay";
> > +};
> > +
> > +&jpegenc {
> > +     status = "okay";
> > +};
> 
> Please drop this. See below.

Done for the next version.

> 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index e46faac1fe71..1d9a16388fa8 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -629,4 +629,41 @@
> >                       };
> >               };
> >       };
> > +
> > +     img_subsys: bus@58000000 {
> > +             compatible = "simple-bus";
> > +             #address-cells = <1>;
> > +             #size-cells = <1>;
> > +             ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > +
> > +             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>;
> > +                     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>;
> > +                     status = "disabled";
> 
> Pure memory-to-memory are typically not enabled per-board,
> but just per-platform.
> 
> So you can drop the disabled status here.

Done for the next version.

Thanks,
Mirela

> 
> Thanks,
> Ezequiel
> 
> > +             };
> > +
> > +             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>;
> > +                     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>;
> > +                     status = "disabled";
> > +             };
> > +     };
> >  };
> 
>