[RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Adam Ford]

The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
to be related to the video decoders used on the i.MX8MQ, but because of
how the Mini handles the power domains, the VPU driver does not need to
handle all the functions, nor does it support the post-processor,
so a new compatible flag is required.

With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
it's unclear to me if that's an acceptable alternative.

At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
results from Fluster. However, the G2 VPU appears to fail most tests.

./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
Ran 90/135 tests successfully               in 76.431 secs

 ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
Ran 55/61 tests successfully               in 21.454 secs

./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
Ran 0/303 tests successfully               in 20.016 secs

Each day seems to show more and more G2 submissions, and gstreamer seems to be 
still working on the VP9, so I am not sure if I should drop G2 as well.


Adam Ford (2):
  media: hantro: Add support for i.MX8M Mini
  arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2

 arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
 drivers/staging/media/hantro/hantro_drv.c   |  2 +
 drivers/staging/media/hantro/hantro_hw.h    |  2 +
 drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
 4 files changed, 102 insertions(+)

-- 
2.32.0

[RFC V2 1/2] media: hantro: Add support for i.MX8M Mini

[Adam Ford]

The i.MX8M Mini has a similar implementation of the Hantro G1 and
G2 decoders, but the Mini uses the vpu-blk-ctrl for handling the
VPU resets through the power domain system.  As such, there are
functions present in the 8MQ that are not applicable to the Mini
which requires the driver to have a different compatible flags.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index fb82b9297a2b..2aa1c520be50 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -592,6 +592,8 @@ static const struct of_device_id of_hantro_match[] = {
 	{ .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
 #endif
 #ifdef CONFIG_VIDEO_HANTRO_IMX8M
+	{ .compatible = "nxp,imx8mm-vpu", .data = &imx8mm_vpu_variant, },
+	{ .compatible = "nxp,imx8mm-vpu-g2", .data = &imx8mm_vpu_g2_variant },
 	{ .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
 	{ .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
 #endif
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 267a6d33a47b..ae7c3fff760c 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -211,6 +211,8 @@ enum hantro_enc_fmt {
 	ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
 };
 
+extern const struct hantro_variant imx8mm_vpu_g2_variant;
+extern const struct hantro_variant imx8mm_vpu_variant;
 extern const struct hantro_variant imx8mq_vpu_g2_variant;
 extern const struct hantro_variant imx8mq_vpu_variant;
 extern const struct hantro_variant px30_vpu_variant;
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index ea919bfb9891..c68516c00c6d 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -242,6 +242,32 @@ static const struct hantro_codec_ops imx8mq_vpu_g2_codec_ops[] = {
 	},
 };
 
+static const struct hantro_codec_ops imx8mm_vpu_codec_ops[] = {
+	[HANTRO_MODE_MPEG2_DEC] = {
+		.run = hantro_g1_mpeg2_dec_run,
+		.init = hantro_mpeg2_dec_init,
+		.exit = hantro_mpeg2_dec_exit,
+	},
+	[HANTRO_MODE_VP8_DEC] = {
+		.run = hantro_g1_vp8_dec_run,
+		.init = hantro_vp8_dec_init,
+		.exit = hantro_vp8_dec_exit,
+	},
+	[HANTRO_MODE_H264_DEC] = {
+		.run = hantro_g1_h264_dec_run,
+		.init = hantro_h264_dec_init,
+		.exit = hantro_h264_dec_exit,
+	},
+};
+
+static const struct hantro_codec_ops imx8mm_vpu_g2_codec_ops[] = {
+	[HANTRO_MODE_HEVC_DEC] = {
+		.run = hantro_g2_hevc_dec_run,
+		.init = hantro_hevc_dec_init,
+		.exit = hantro_hevc_dec_exit,
+	},
+};
+
 /*
  * VPU variants.
  */
@@ -257,6 +283,11 @@ static const struct hantro_irq imx8mq_g2_irqs[] = {
 static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" };
 static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" };
 
+static const char * const imx8mm_g1_clk_names[] = { "g1", "bus" };
+static const char * const imx8mm_g1_reg_names[] = { "g1" };
+static const char * const imx8mm_g2_clk_names[] = { "g2", "bus" };
+static const char * const imx8mm_g2_reg_names[] = { "g2" };
+
 const struct hantro_variant imx8mq_vpu_variant = {
 	.dec_fmts = imx8m_vpu_dec_fmts,
 	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
@@ -289,3 +320,29 @@ const struct hantro_variant imx8mq_vpu_g2_variant = {
 	.clk_names = imx8mq_clk_names,
 	.num_clocks = ARRAY_SIZE(imx8mq_clk_names),
 };
+
+const struct hantro_variant imx8mm_vpu_variant = {
+	.dec_fmts = imx8m_vpu_dec_fmts,
+	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
+	.codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
+		 HANTRO_H264_DECODER,
+	.codec_ops = imx8mm_vpu_codec_ops,
+	.irqs = imx8mq_irqs,
+	.num_irqs = ARRAY_SIZE(imx8mq_irqs),
+	.clk_names = imx8mm_g1_clk_names,
+	.num_clocks = ARRAY_SIZE(imx8mm_g1_clk_names),
+	.reg_names = imx8mm_g1_reg_names,
+	.num_regs = ARRAY_SIZE(imx8mm_g1_reg_names)
+};
+
+const struct hantro_variant imx8mm_vpu_g2_variant = {
+	.dec_offset = 0x0,
+	.dec_fmts = imx8m_vpu_g2_dec_fmts,
+	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_g2_dec_fmts),
+	.codec = HANTRO_HEVC_DECODER,
+	.codec_ops = imx8mm_vpu_g2_codec_ops,
+	.irqs = imx8mq_g2_irqs,
+	.num_irqs = ARRAY_SIZE(imx8mq_g2_irqs),
+	.clk_names = imx8mm_g2_clk_names,
+	.num_clocks = ARRAY_SIZE(imx8mm_g2_reg_names),
+};
-- 
2.32.0

Re: [RFC V2 1/2] media: hantro: Add support for i.MX8M Mini

[Ezequiel Garcia]

Hi Adam,

On Tue, 30 Nov 2021 at 22:33, Adam Ford <aford173@gmail.com> wrote:
>
> The i.MX8M Mini has a similar implementation of the Hantro G1 and
> h decoders, but the Mini uses the vpu-blk-ctrl for handling the
> VPU resets through the power domain system.  As such, there are
> functions present in the 8MQ that are not applicable to the Mini
> which requires the driver to have a different compatible flags.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index fb82b9297a2b..2aa1c520be50 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -592,6 +592,8 @@ static const struct of_device_id of_hantro_match[] = {
>         { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
>  #endif
>  #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> +       { .compatible = "nxp,imx8mm-vpu", .data = &imx8mm_vpu_variant, },
> +       { .compatible = "nxp,imx8mm-vpu-g2", .data = &imx8mm_vpu_g2_variant },
>         { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
>         { .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
>  #endif
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> index 267a6d33a47b..ae7c3fff760c 100644
> --- a/drivers/staging/media/hantro/hantro_hw.h
> +++ b/drivers/staging/media/hantro/hantro_hw.h
> @@ -211,6 +211,8 @@ enum hantro_enc_fmt {
>         ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
>  };
>
> +extern const struct hantro_variant imx8mm_vpu_g2_variant;
> +extern const struct hantro_variant imx8mm_vpu_variant;
>  extern const struct hantro_variant imx8mq_vpu_g2_variant;
>  extern const struct hantro_variant imx8mq_vpu_variant;
>  extern const struct hantro_variant px30_vpu_variant;
> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> index ea919bfb9891..c68516c00c6d 100644
> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> @@ -242,6 +242,32 @@ static const struct hantro_codec_ops imx8mq_vpu_g2_codec_ops[] = {
>         },
>  };
>
> +static const struct hantro_codec_ops imx8mm_vpu_codec_ops[] = {
> +       [HANTRO_MODE_MPEG2_DEC] = {
> +               .run = hantro_g1_mpeg2_dec_run,
> +               .init = hantro_mpeg2_dec_init,
> +               .exit = hantro_mpeg2_dec_exit,
> +       },
> +       [HANTRO_MODE_VP8_DEC] = {
> +               .run = hantro_g1_vp8_dec_run,
> +               .init = hantro_vp8_dec_init,
> +               .exit = hantro_vp8_dec_exit,
> +       },
> +       [HANTRO_MODE_H264_DEC] = {
> +               .run = hantro_g1_h264_dec_run,
> +               .init = hantro_h264_dec_init,
> +               .exit = hantro_h264_dec_exit,
> +       },
> +};
> +
> +static const struct hantro_codec_ops imx8mm_vpu_g2_codec_ops[] = {
> +       [HANTRO_MODE_HEVC_DEC] = {
> +               .run = hantro_g2_hevc_dec_run,
> +               .init = hantro_hevc_dec_init,
> +               .exit = hantro_hevc_dec_exit,
> +       },
> +};
> +

I believe you are missing VP9, which explains why you get
a zero fluster score.

Thanks,
Ezequiel

Re: [RFC V2 1/2] media: hantro: Add support for i.MX8M Mini

[Adam Ford]

On Wed, Dec 1, 2021 at 6:25 AM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> Hi Adam,
>
> On Tue, 30 Nov 2021 at 22:33, Adam Ford <aford173@gmail.com> wrote:
> >
> > The i.MX8M Mini has a similar implementation of the Hantro G1 and
> > h decoders, but the Mini uses the vpu-blk-ctrl for handling the
> > VPU resets through the power domain system.  As such, there are
> > functions present in the 8MQ that are not applicable to the Mini
> > which requires the driver to have a different compatible flags.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> > index fb82b9297a2b..2aa1c520be50 100644
> > --- a/drivers/staging/media/hantro/hantro_drv.c
> > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > @@ -592,6 +592,8 @@ static const struct of_device_id of_hantro_match[] = {
> >         { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
> >  #endif
> >  #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> > +       { .compatible = "nxp,imx8mm-vpu", .data = &imx8mm_vpu_variant, },
> > +       { .compatible = "nxp,imx8mm-vpu-g2", .data = &imx8mm_vpu_g2_variant },
> >         { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
> >         { .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
> >  #endif
> > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > index 267a6d33a47b..ae7c3fff760c 100644
> > --- a/drivers/staging/media/hantro/hantro_hw.h
> > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > @@ -211,6 +211,8 @@ enum hantro_enc_fmt {
> >         ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
> >  };
> >
> > +extern const struct hantro_variant imx8mm_vpu_g2_variant;
> > +extern const struct hantro_variant imx8mm_vpu_variant;
> >  extern const struct hantro_variant imx8mq_vpu_g2_variant;
> >  extern const struct hantro_variant imx8mq_vpu_variant;
> >  extern const struct hantro_variant px30_vpu_variant;
> > diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > index ea919bfb9891..c68516c00c6d 100644
> > --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > @@ -242,6 +242,32 @@ static const struct hantro_codec_ops imx8mq_vpu_g2_codec_ops[] = {
> >         },
> >  };
> >
> > +static const struct hantro_codec_ops imx8mm_vpu_codec_ops[] = {
> > +       [HANTRO_MODE_MPEG2_DEC] = {
> > +               .run = hantro_g1_mpeg2_dec_run,
> > +               .init = hantro_mpeg2_dec_init,
> > +               .exit = hantro_mpeg2_dec_exit,
> > +       },
> > +       [HANTRO_MODE_VP8_DEC] = {
> > +               .run = hantro_g1_vp8_dec_run,
> > +               .init = hantro_vp8_dec_init,
> > +               .exit = hantro_vp8_dec_exit,
> > +       },
> > +       [HANTRO_MODE_H264_DEC] = {
> > +               .run = hantro_g1_h264_dec_run,
> > +               .init = hantro_h264_dec_init,
> > +               .exit = hantro_h264_dec_exit,
> > +       },
> > +};
> > +
> > +static const struct hantro_codec_ops imx8mm_vpu_g2_codec_ops[] = {
> > +       [HANTRO_MODE_HEVC_DEC] = {
> > +               .run = hantro_g2_hevc_dec_run,
> > +               .init = hantro_hevc_dec_init,
> > +               .exit = hantro_hevc_dec_exit,
> > +       },
> > +};
> > +
>
> I believe you are missing VP9, which explains why you get
> a zero fluster score.

That's what I was thinking too and that's why I was wondering if I
should wait on G2 until more of those G2 patches have been finalized
and accepted.  Is there a way to test the HEVC?  I didn't see one in
the fluster list.

adam
>
> Thanks,
> Ezequiel

Re: [RFC V2 1/2] media: hantro: Add support for i.MX8M Mini

[Ezequiel Garcia]

On Wed, 1 Dec 2021 at 09:36, Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Dec 1, 2021 at 6:25 AM Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
> >
> > Hi Adam,
> >
> > On Tue, 30 Nov 2021 at 22:33, Adam Ford <aford173@gmail.com> wrote:
> > >
> > > The i.MX8M Mini has a similar implementation of the Hantro G1 and
> > > h decoders, but the Mini uses the vpu-blk-ctrl for handling the
> > > VPU resets through the power domain system.  As such, there are
> > > functions present in the 8MQ that are not applicable to the Mini
> > > which requires the driver to have a different compatible flags.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > >
> > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> > > index fb82b9297a2b..2aa1c520be50 100644
> > > --- a/drivers/staging/media/hantro/hantro_drv.c
> > > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > > @@ -592,6 +592,8 @@ static const struct of_device_id of_hantro_match[] = {
> > >         { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
> > >  #endif
> > >  #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> > > +       { .compatible = "nxp,imx8mm-vpu", .data = &imx8mm_vpu_variant, },
> > > +       { .compatible = "nxp,imx8mm-vpu-g2", .data = &imx8mm_vpu_g2_variant },
> > >         { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
> > >         { .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
> > >  #endif
> > > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > > index 267a6d33a47b..ae7c3fff760c 100644
> > > --- a/drivers/staging/media/hantro/hantro_hw.h
> > > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > > @@ -211,6 +211,8 @@ enum hantro_enc_fmt {
> > >         ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
> > >  };
> > >
> > > +extern const struct hantro_variant imx8mm_vpu_g2_variant;
> > > +extern const struct hantro_variant imx8mm_vpu_variant;
> > >  extern const struct hantro_variant imx8mq_vpu_g2_variant;
> > >  extern const struct hantro_variant imx8mq_vpu_variant;
> > >  extern const struct hantro_variant px30_vpu_variant;
> > > diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > index ea919bfb9891..c68516c00c6d 100644
> > > --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > @@ -242,6 +242,32 @@ static const struct hantro_codec_ops imx8mq_vpu_g2_codec_ops[] = {
> > >         },
> > >  };
> > >
> > > +static const struct hantro_codec_ops imx8mm_vpu_codec_ops[] = {
> > > +       [HANTRO_MODE_MPEG2_DEC] = {
> > > +               .run = hantro_g1_mpeg2_dec_run,
> > > +               .init = hantro_mpeg2_dec_init,
> > > +               .exit = hantro_mpeg2_dec_exit,
> > > +       },
> > > +       [HANTRO_MODE_VP8_DEC] = {
> > > +               .run = hantro_g1_vp8_dec_run,
> > > +               .init = hantro_vp8_dec_init,
> > > +               .exit = hantro_vp8_dec_exit,
> > > +       },
> > > +       [HANTRO_MODE_H264_DEC] = {
> > > +               .run = hantro_g1_h264_dec_run,
> > > +               .init = hantro_h264_dec_init,
> > > +               .exit = hantro_h264_dec_exit,
> > > +       },
> > > +};
> > > +
> > > +static const struct hantro_codec_ops imx8mm_vpu_g2_codec_ops[] = {
> > > +       [HANTRO_MODE_HEVC_DEC] = {
> > > +               .run = hantro_g2_hevc_dec_run,
> > > +               .init = hantro_hevc_dec_init,
> > > +               .exit = hantro_hevc_dec_exit,
> > > +       },
> > > +};
> > > +
> >
> > I believe you are missing VP9, which explains why you get
> > a zero fluster score.
>
> That's what I was thinking too and that's why I was wondering if I
> should wait on G2 until more of those G2 patches have been finalized
> and accepted.  Is there a way to test the HEVC?  I didn't see one in
> the fluster list.
>

VP9 is on its way to be merged. There is a pull request from Hans
already: see https://www.spinics.net/lists/linux-media/msg202448.html
which includes the git repository and tag you can merge/rebase to test
it.

It would be great if you can test G2 on top of that, but it's also fine
if you want to just submit G1 for now. Up to you.

Regarding HEVC, currently Benjamin is who knows best how to test it.
Thinking about it, perhaps we should document this somewhere?

Regards,
Ezequiel

Re: [RFC V2 1/2] media: hantro: Add support for i.MX8M Mini

[Nicolas Dufresne]

Le mercredi 01 décembre 2021 à 09:58 -0300, Ezequiel Garcia a écrit :
> On Wed, 1 Dec 2021 at 09:36, Adam Ford <aford173@gmail.com> wrote:
> > 
> > On Wed, Dec 1, 2021 at 6:25 AM Ezequiel Garcia
> > <ezequiel@vanguardiasur.com.ar> wrote:
> > > 
> > > Hi Adam,
> > > 
> > > On Tue, 30 Nov 2021 at 22:33, Adam Ford <aford173@gmail.com> wrote:
> > > > 
> > > > The i.MX8M Mini has a similar implementation of the Hantro G1 and
> > > > h decoders, but the Mini uses the vpu-blk-ctrl for handling the
> > > > VPU resets through the power domain system.  As such, there are
> > > > functions present in the 8MQ that are not applicable to the Mini
> > > > which requires the driver to have a different compatible flags.
> > > > 
> > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > 
> > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> > > > index fb82b9297a2b..2aa1c520be50 100644
> > > > --- a/drivers/staging/media/hantro/hantro_drv.c
> > > > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > > > @@ -592,6 +592,8 @@ static const struct of_device_id of_hantro_match[] = {
> > > >         { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
> > > >  #endif
> > > >  #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> > > > +       { .compatible = "nxp,imx8mm-vpu", .data = &imx8mm_vpu_variant, },
> > > > +       { .compatible = "nxp,imx8mm-vpu-g2", .data = &imx8mm_vpu_g2_variant },
> > > >         { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
> > > >         { .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
> > > >  #endif
> > > > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > > > index 267a6d33a47b..ae7c3fff760c 100644
> > > > --- a/drivers/staging/media/hantro/hantro_hw.h
> > > > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > > > @@ -211,6 +211,8 @@ enum hantro_enc_fmt {
> > > >         ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
> > > >  };
> > > > 
> > > > +extern const struct hantro_variant imx8mm_vpu_g2_variant;
> > > > +extern const struct hantro_variant imx8mm_vpu_variant;
> > > >  extern const struct hantro_variant imx8mq_vpu_g2_variant;
> > > >  extern const struct hantro_variant imx8mq_vpu_variant;
> > > >  extern const struct hantro_variant px30_vpu_variant;
> > > > diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > > index ea919bfb9891..c68516c00c6d 100644
> > > > --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > > +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > > @@ -242,6 +242,32 @@ static const struct hantro_codec_ops imx8mq_vpu_g2_codec_ops[] = {
> > > >         },
> > > >  };
> > > > 
> > > > +static const struct hantro_codec_ops imx8mm_vpu_codec_ops[] = {
> > > > +       [HANTRO_MODE_MPEG2_DEC] = {
> > > > +               .run = hantro_g1_mpeg2_dec_run,
> > > > +               .init = hantro_mpeg2_dec_init,
> > > > +               .exit = hantro_mpeg2_dec_exit,
> > > > +       },
> > > > +       [HANTRO_MODE_VP8_DEC] = {
> > > > +               .run = hantro_g1_vp8_dec_run,
> > > > +               .init = hantro_vp8_dec_init,
> > > > +               .exit = hantro_vp8_dec_exit,
> > > > +       },
> > > > +       [HANTRO_MODE_H264_DEC] = {
> > > > +               .run = hantro_g1_h264_dec_run,
> > > > +               .init = hantro_h264_dec_init,
> > > > +               .exit = hantro_h264_dec_exit,
> > > > +       },
> > > > +};
> > > > +
> > > > +static const struct hantro_codec_ops imx8mm_vpu_g2_codec_ops[] = {
> > > > +       [HANTRO_MODE_HEVC_DEC] = {
> > > > +               .run = hantro_g2_hevc_dec_run,
> > > > +               .init = hantro_hevc_dec_init,
> > > > +               .exit = hantro_hevc_dec_exit,
> > > > +       },
> > > > +};
> > > > +
> > > 
> > > I believe you are missing VP9, which explains why you get
> > > a zero fluster score.
> > 
> > That's what I was thinking too and that's why I was wondering if I
> > should wait on G2 until more of those G2 patches have been finalized
> > and accepted.  Is there a way to test the HEVC?  I didn't see one in
> > the fluster list.
> > 
> 
> VP9 is on its way to be merged. There is a pull request from Hans
> already: see https://www.spinics.net/lists/linux-media/msg202448.html
> which includes the git repository and tag you can merge/rebase to test
> it.
> 
> It would be great if you can test G2 on top of that, but it's also fine
> if you want to just submit G1 for now. Up to you.
> 
> Regarding HEVC, currently Benjamin is who knows best how to test it.
> Thinking about it, perhaps we should document this somewhere?

There is GStreamer-H.265-V4L2SL-Gst1.0 decoder already in fluster. And GStreamer
support is still WIP.

https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079

We had put on hold the HEVC work in order to focus on VP9. Now that VP9 is on
its way (I've sent another MR today to GStreamer to fix some more tests). I
haven't tested myself imx8mq recently, will likely do soon, so I can give you
the expected score. Your VP8 and H264 score matches the result I got. Note that
H264 driver is missing interlace support, which is half the tests.

We will can resume this work. Help is welcome of course. The HEVC staging API is
by was the worst, so there is quite some work to move this API to stable and
then port all the drivers to the require changes that will be needed.

> 
> Regards,
> Ezequiel

Re: [RFC V2 1/2] media: hantro: Add support for i.MX8M Mini

[Adam Ford]

On Wed, Dec 1, 2021 at 3:03 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Le mercredi 01 décembre 2021 à 09:58 -0300, Ezequiel Garcia a écrit :
> > On Wed, 1 Dec 2021 at 09:36, Adam Ford <aford173@gmail.com> wrote:
> > >
> > > On Wed, Dec 1, 2021 at 6:25 AM Ezequiel Garcia
> > > <ezequiel@vanguardiasur.com.ar> wrote:
> > > >
> > > > Hi Adam,
> > > >
> > > > On Tue, 30 Nov 2021 at 22:33, Adam Ford <aford173@gmail.com> wrote:
> > > > >
> > > > > The i.MX8M Mini has a similar implementation of the Hantro G1 and
> > > > > h decoders, but the Mini uses the vpu-blk-ctrl for handling the
> > > > > VPU resets through the power domain system.  As such, there are
> > > > > functions present in the 8MQ that are not applicable to the Mini
> > > > > which requires the driver to have a different compatible flags.
> > > > >
> > > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > >
> > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> > > > > index fb82b9297a2b..2aa1c520be50 100644
> > > > > --- a/drivers/staging/media/hantro/hantro_drv.c
> > > > > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > > > > @@ -592,6 +592,8 @@ static const struct of_device_id of_hantro_match[] = {
> > > > >         { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
> > > > >  #endif
> > > > >  #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> > > > > +       { .compatible = "nxp,imx8mm-vpu", .data = &imx8mm_vpu_variant, },
> > > > > +       { .compatible = "nxp,imx8mm-vpu-g2", .data = &imx8mm_vpu_g2_variant },
> > > > >         { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
> > > > >         { .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
> > > > >  #endif
> > > > > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > > > > index 267a6d33a47b..ae7c3fff760c 100644
> > > > > --- a/drivers/staging/media/hantro/hantro_hw.h
> > > > > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > > > > @@ -211,6 +211,8 @@ enum hantro_enc_fmt {
> > > > >         ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
> > > > >  };
> > > > >
> > > > > +extern const struct hantro_variant imx8mm_vpu_g2_variant;
> > > > > +extern const struct hantro_variant imx8mm_vpu_variant;
> > > > >  extern const struct hantro_variant imx8mq_vpu_g2_variant;
> > > > >  extern const struct hantro_variant imx8mq_vpu_variant;
> > > > >  extern const struct hantro_variant px30_vpu_variant;
> > > > > diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > > > index ea919bfb9891..c68516c00c6d 100644
> > > > > --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > > > +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > > > @@ -242,6 +242,32 @@ static const struct hantro_codec_ops imx8mq_vpu_g2_codec_ops[] = {
> > > > >         },
> > > > >  };
> > > > >
> > > > > +static const struct hantro_codec_ops imx8mm_vpu_codec_ops[] = {
> > > > > +       [HANTRO_MODE_MPEG2_DEC] = {
> > > > > +               .run = hantro_g1_mpeg2_dec_run,
> > > > > +               .init = hantro_mpeg2_dec_init,
> > > > > +               .exit = hantro_mpeg2_dec_exit,
> > > > > +       },
> > > > > +       [HANTRO_MODE_VP8_DEC] = {
> > > > > +               .run = hantro_g1_vp8_dec_run,
> > > > > +               .init = hantro_vp8_dec_init,
> > > > > +               .exit = hantro_vp8_dec_exit,
> > > > > +       },
> > > > > +       [HANTRO_MODE_H264_DEC] = {
> > > > > +               .run = hantro_g1_h264_dec_run,
> > > > > +               .init = hantro_h264_dec_init,
> > > > > +               .exit = hantro_h264_dec_exit,
> > > > > +       },
> > > > > +};
> > > > > +
> > > > > +static const struct hantro_codec_ops imx8mm_vpu_g2_codec_ops[] = {
> > > > > +       [HANTRO_MODE_HEVC_DEC] = {
> > > > > +               .run = hantro_g2_hevc_dec_run,
> > > > > +               .init = hantro_hevc_dec_init,
> > > > > +               .exit = hantro_hevc_dec_exit,
> > > > > +       },
> > > > > +};
> > > > > +
> > > >
> > > > I believe you are missing VP9, which explains why you get
> > > > a zero fluster score.
> > >
> > > That's what I was thinking too and that's why I was wondering if I
> > > should wait on G2 until more of those G2 patches have been finalized
> > > and accepted.  Is there a way to test the HEVC?  I didn't see one in
> > > the fluster list.
> > >
> >
> > VP9 is on its way to be merged. There is a pull request from Hans
> > already: see https://www.spinics.net/lists/linux-media/msg202448.html
> > which includes the git repository and tag you can merge/rebase to test
> > it.
> >

Thanks for that.  I rebased my work and found some bugs, so I'll be
posting an RFC V3 later tonight.

> > It would be great if you can test G2 on top of that, but it's also fine
> > if you want to just submit G1 for now. Up to you.
> >
> > Regarding HEVC, currently Benjamin is who knows best how to test it.
> > Thinking about it, perhaps we should document this somewhere?
>
> There is GStreamer-H.265-V4L2SL-Gst1.0 decoder already in fluster. And GStreamer
> support is still WIP.
>
> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079
>
> We had put on hold the HEVC work in order to focus on VP9. Now that VP9 is on
> its way (I've sent another MR today to GStreamer to fix some more tests). I
> haven't tested myself imx8mq recently, will likely do soon, so I can give you
> the expected score. Your VP8 and H264 score matches the result I got. Note that
> H264 driver is missing interlace support, which is half the tests.
>
> We will can resume this work. Help is welcome of course. The HEVC staging API is
> by was the worst, so there is quite some work to move this API to stable and
> then port all the drivers to the require changes that will be needed.

With the latest gstreamer and the rebase off Hans' work along with
some improvements to my code, fluster now runs the VP9...at least for
a while.  It doesn't technically finish because the power domain
appears to choke which causes a hang.  This was reported by Tim
Harvey, and with some of my updates, I can reproduce it now too.  :-(

At least I know I'm off to the right start on the VP9.

adam
>
> >
> > Regards,
> > Ezequiel
>

[RFC V2 2/2] arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2

[Adam Ford]

Enable two hardware Hantro decoders called G1 and G2.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c2f3f118f82e..eb9dcd9d1a31 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1197,6 +1197,47 @@ gpu_2d: gpu@38008000 {
 			power-domains = <&pgc_gpu>;
 		};
 
+		vpu_g1: video-codec@38300000 {
+			compatible = "nxp,imx8mm-vpu";
+			reg = <0x38300000 0x10000>;
+			reg-names = "g1";
+			interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "g1";
+			clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>,
+				 <&clk IMX8MM_CLK_VPU_DEC_ROOT>;
+			clock-names = "g1", "bus";
+			assigned-clocks = <&clk IMX8MM_CLK_VPU_G1>,
+					  <&clk IMX8MM_CLK_VPU_BUS>,
+					  <&clk IMX8MM_VPU_PLL_BYPASS>;
+			assigned-clock-parents = <&clk IMX8MM_VPU_PLL_OUT>,
+						 <&clk IMX8MM_SYS_PLL1_800M>,
+						 <&clk IMX8MM_VPU_PLL>;
+			assigned-clock-rates = <600000000>,
+					       <800000000>,
+					       <0>;
+			power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G1>;
+		};
+
+		vpu_g2: video-codec@38310000 {
+			compatible = "nxp,imx8mm-vpu-g2";
+			reg = <0x38310000 0x10000>;
+			interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "g2";
+			clocks = <&clk IMX8MM_CLK_VPU_G2_ROOT>,
+				 <&clk IMX8MM_CLK_VPU_DEC_ROOT>;
+			clock-names = "g2",  "bus";
+			assigned-clocks = <&clk IMX8MM_CLK_VPU_G2>,
+					 <&clk IMX8MM_CLK_VPU_BUS>,
+					 <&clk IMX8MM_VPU_PLL_BYPASS>;
+			assigned-clock-parents = <&clk IMX8MM_VPU_PLL_OUT>,
+						<&clk IMX8MM_SYS_PLL1_800M>,
+						<&clk IMX8MM_VPU_PLL>;
+			assigned-clock-rates = <600000000>,
+					       <800000000>,
+					       <0>;
+			power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G2>;
+		};
+
 		vpu_blk_ctrl: blk-ctrl@38330000 {
 			compatible = "fsl,imx8mm-vpu-blk-ctrl", "syscon";
 			reg = <0x38330000 0x100>;
-- 
2.32.0

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Benjamin Gaignard]

Le 01/12/2021 à 02:33, Adam Ford a écrit :
> The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> to be related to the video decoders used on the i.MX8MQ, but because of
> how the Mini handles the power domains, the VPU driver does not need to
> handle all the functions, nor does it support the post-processor,
> so a new compatible flag is required.
>
> With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> it's unclear to me if that's an acceptable alternative.
>
> At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> results from Fluster. However, the G2 VPU appears to fail most tests.
>
> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> Ran 90/135 tests successfully               in 76.431 secs
>
>   ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> Ran 55/61 tests successfully               in 21.454 secs
>
> ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> Ran 0/303 tests successfully               in 20.016 secs
>
> Each day seems to show more and more G2 submissions, and gstreamer seems to be
> still working on the VP9, so I am not sure if I should drop G2 as well.

I think it is going in the good direction.
I'm trying to do the same on IMX6MQ but still have hang issue on G2.

Regards,
Benjamin

>
> Adam Ford (2):
>    media: hantro: Add support for i.MX8M Mini
>    arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
>
>   arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
>   drivers/staging/media/hantro/hantro_drv.c   |  2 +
>   drivers/staging/media/hantro/hantro_hw.h    |  2 +
>   drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
>   4 files changed, 102 insertions(+)
>

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Tim Harvey]

On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
>
> The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> to be related to the video decoders used on the i.MX8MQ, but because of
> how the Mini handles the power domains, the VPU driver does not need to
> handle all the functions, nor does it support the post-processor,
> so a new compatible flag is required.
>
> With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> it's unclear to me if that's an acceptable alternative.
>
> At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> results from Fluster. However, the G2 VPU appears to fail most tests.
>
> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> Ran 90/135 tests successfully               in 76.431 secs
>
>  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> Ran 55/61 tests successfully               in 21.454 secs
>
> ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> Ran 0/303 tests successfully               in 20.016 secs
>
> Each day seems to show more and more G2 submissions, and gstreamer seems to be
> still working on the VP9, so I am not sure if I should drop G2 as well.
>
>
> Adam Ford (2):
>   media: hantro: Add support for i.MX8M Mini
>   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
>
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
>  drivers/staging/media/hantro/hantro_drv.c   |  2 +
>  drivers/staging/media/hantro/hantro_hw.h    |  2 +
>  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
>  4 files changed, 102 insertions(+)
>

Adam,

That's for the patches!

I tested just this series on top of v5.16-rc3 on an
imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
is called for VPUMIX pd :
while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
...
[  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
[  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain

I added prints in imx_pgc_power_{up,down} and
imx8m_blk_ctrl_power_{on,off} to get some more context
...
Ran 55/61 tests successfully               in 8.685 secs
 17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
********************************************************************************
********************
Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
Using 4 parallel job(s)
********************************************************************************
********************

[TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
ESULT
----------------------------------------------------------------------
[ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
[ 1023.119669] imx_pgc_power_up vpumix
[ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
[ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain

While this wouldn't be an issue with this series it does indicate we
still have something racy in blk-ctrl. Can you reproduce this (and if
not what kernel are you based on)? Perhaps you or Lucas have some
ideas?

Best regards,

Tim

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Lucas Stach]

Hi Tim,

Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > 
> > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > to be related to the video decoders used on the i.MX8MQ, but because of
> > how the Mini handles the power domains, the VPU driver does not need to
> > handle all the functions, nor does it support the post-processor,
> > so a new compatible flag is required.
> > 
> > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > it's unclear to me if that's an acceptable alternative.
> > 
> > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > results from Fluster. However, the G2 VPU appears to fail most tests.
> > 
> > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > Ran 90/135 tests successfully               in 76.431 secs
> > 
> >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > Ran 55/61 tests successfully               in 21.454 secs
> > 
> > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > Ran 0/303 tests successfully               in 20.016 secs
> > 
> > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > still working on the VP9, so I am not sure if I should drop G2 as well.
> > 
> > 
> > Adam Ford (2):
> >   media: hantro: Add support for i.MX8M Mini
> >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > 
> >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> >  4 files changed, 102 insertions(+)
> > 
> 
> Adam,
> 
> That's for the patches!
> 
> I tested just this series on top of v5.16-rc3 on an
> imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> is called for VPUMIX pd :
> while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> ...
> [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> 
> I added prints in imx_pgc_power_{up,down} and
> imx8m_blk_ctrl_power_{on,off} to get some more context
> ...
> Ran 55/61 tests successfully               in 8.685 secs
>  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> ********************************************************************************
> ********************
> Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> Using 4 parallel job(s)
> ********************************************************************************
> ********************
> 
> [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> ESULT
> ----------------------------------------------------------------------
> [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> [ 1023.119669] imx_pgc_power_up vpumix
> [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> 
> While this wouldn't be an issue with this series it does indicate we
> still have something racy in blk-ctrl. Can you reproduce this (and if
> not what kernel are you based on)? Perhaps you or Lucas have some
> ideas?
> 
Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
domains" applied when running those tests? It has only recently been
picked up by Shawn and may have an influence on the bus domain
behavior.

Regards,
Lucas

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Tim Harvey]

On Wed, Dec 1, 2021 at 9:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Tim,
>
> Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > >
> > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > how the Mini handles the power domains, the VPU driver does not need to
> > > handle all the functions, nor does it support the post-processor,
> > > so a new compatible flag is required.
> > >
> > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > it's unclear to me if that's an acceptable alternative.
> > >
> > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > >
> > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > Ran 90/135 tests successfully               in 76.431 secs
> > >
> > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > Ran 55/61 tests successfully               in 21.454 secs
> > >
> > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > Ran 0/303 tests successfully               in 20.016 secs
> > >
> > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > >
> > >
> > > Adam Ford (2):
> > >   media: hantro: Add support for i.MX8M Mini
> > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > >
> > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > >  4 files changed, 102 insertions(+)
> > >
> >
> > Adam,
> >
> > That's for the patches!
> >
> > I tested just this series on top of v5.16-rc3 on an
> > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > is called for VPUMIX pd :
> > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > ...
> > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> >
> > I added prints in imx_pgc_power_{up,down} and
> > imx8m_blk_ctrl_power_{on,off} to get some more context
> > ...
> > Ran 55/61 tests successfully               in 8.685 secs
> >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > ********************************************************************************
> > ********************
> > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > Using 4 parallel job(s)
> > ********************************************************************************
> > ********************
> >
> > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > ESULT
> > ----------------------------------------------------------------------
> > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > [ 1023.119669] imx_pgc_power_up vpumix
> > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> >
> > While this wouldn't be an issue with this series it does indicate we
> > still have something racy in blk-ctrl. Can you reproduce this (and if
> > not what kernel are you based on)? Perhaps you or Lucas have some
> > ideas?
> >
> Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> domains" applied when running those tests? It has only recently been
> picked up by Shawn and may have an influence on the bus domain
> behavior.
>

Lucas,

Good point. I did have that originally before I started pruning down
to the bare minimum to reproduce the issue.

I added it back and now I have the following:
arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
media: hantro: Add support for i.MX8M Mini
soc: imx: gpcv2: keep i.MX8MM VPU-H1 bus clock active
soc: imx: gpcv2: Synchronously suspend MIX domains
Linux 5.16-rc3

Here's the latest with that patch:
...
[VP8-TEST-VECTORS] (GStreamer-VP8-V4L2SL-Gst1.0)
vp80-00-comprehensive-007 ... Success
[  316.632373] imx8m_blk_ctrl_power_off vpublk-g1
[  316.636908] imx_pgc_power_down vpu-g1
[  316.640983] imx_pgc_power_down vpumix
[  316.756869] imx8m_blk_ctrl_power_on vpublk-g1
[  316.761360] imx_pgc_power_up vpumix
[  316.765985] imx-pgc imx-pgc-domain.6: failed to command PGC
[  316.772743] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
^^^ hang

I believe there is some sort of simple test I can do to power the gpu
up/down to test as well but not clear what that is.

Tim

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Lucas Stach]

Am Mittwoch, dem 01.12.2021 um 10:16 -0800 schrieb Tim Harvey:
> On Wed, Dec 1, 2021 at 9:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > 
> > Hi Tim,
> > 
> > Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > > > 
> > > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > > how the Mini handles the power domains, the VPU driver does not need to
> > > > handle all the functions, nor does it support the post-processor,
> > > > so a new compatible flag is required.
> > > > 
> > > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > > it's unclear to me if that's an acceptable alternative.
> > > > 
> > > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > > > 
> > > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > > Ran 90/135 tests successfully               in 76.431 secs
> > > > 
> > > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > > Ran 55/61 tests successfully               in 21.454 secs
> > > > 
> > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > > Ran 0/303 tests successfully               in 20.016 secs
> > > > 
> > > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > > > 
> > > > 
> > > > Adam Ford (2):
> > > >   media: hantro: Add support for i.MX8M Mini
> > > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > 
> > > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > > >  4 files changed, 102 insertions(+)
> > > > 
> > > 
> > > Adam,
> > > 
> > > That's for the patches!
> > > 
> > > I tested just this series on top of v5.16-rc3 on an
> > > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > > is called for VPUMIX pd :
> > > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > > ...
> > > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > 
> > > I added prints in imx_pgc_power_{up,down} and
> > > imx8m_blk_ctrl_power_{on,off} to get some more context
> > > ...
> > > Ran 55/61 tests successfully               in 8.685 secs
> > >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > > ********************************************************************************
> > > ********************
> > > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > > Using 4 parallel job(s)
> > > ********************************************************************************
> > > ********************
> > > 
> > > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > > ESULT
> > > ----------------------------------------------------------------------
> > > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > > [ 1023.119669] imx_pgc_power_up vpumix
> > > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > 
> > > While this wouldn't be an issue with this series it does indicate we
> > > still have something racy in blk-ctrl. Can you reproduce this (and if
> > > not what kernel are you based on)? Perhaps you or Lucas have some
> > > ideas?
> > > 
> > Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> > domains" applied when running those tests? It has only recently been
> > picked up by Shawn and may have an influence on the bus domain
> > behavior.
> > 
> 
> Lucas,
> 
> Good point. I did have that originally before I started pruning down
> to the bare minimum to reproduce the issue.
> 
> I added it back and now I have the following:
> arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> media: hantro: Add support for i.MX8M Mini
> soc: imx: gpcv2: keep i.MX8MM VPU-H1 bus clock active
> soc: imx: gpcv2: Synchronously suspend MIX domains
> Linux 5.16-rc3
> 
> Here's the latest with that patch:
> ...
> [VP8-TEST-VECTORS] (GStreamer-VP8-V4L2SL-Gst1.0)
> vp80-00-comprehensive-007 ... Success
> [  316.632373] imx8m_blk_ctrl_power_off vpublk-g1
> [  316.636908] imx_pgc_power_down vpu-g1
> [  316.640983] imx_pgc_power_down vpumix
> [  316.756869] imx8m_blk_ctrl_power_on vpublk-g1
> [  316.761360] imx_pgc_power_up vpumix
> [  316.765985] imx-pgc imx-pgc-domain.6: failed to command PGC
> [  316.772743] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> ^^^ hang

Hm, I wonder if there's some broken error handling here somewhere, as a
failure to power up a domain shouldn't lead to a hang.

However, that doesn't explain why the PGC isn't completing the request.
Can you try to extend the timeout some more. Even though I think that
1msec should already be generous. Can you dump the content of the
GPC_PU_PGC_SW_PUP_REQ and GPC_A53_PU_PGC_PUP_STATUSn (all 3 of them)
registers, when the failure condition is hit?

Regards,
Lucas 

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Adam Ford]

On Wed, Dec 1, 2021 at 12:37 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Mittwoch, dem 01.12.2021 um 10:16 -0800 schrieb Tim Harvey:
> > On Wed, Dec 1, 2021 at 9:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > >
> > > Hi Tim,
> > >
> > > Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > > > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > > > >
> > > > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > > > how the Mini handles the power domains, the VPU driver does not need to
> > > > > handle all the functions, nor does it support the post-processor,
> > > > > so a new compatible flag is required.
> > > > >
> > > > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > > > it's unclear to me if that's an acceptable alternative.
> > > > >
> > > > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > > > >
> > > > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > > > Ran 90/135 tests successfully               in 76.431 secs
> > > > >
> > > > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > > > Ran 55/61 tests successfully               in 21.454 secs
> > > > >
> > > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > > > Ran 0/303 tests successfully               in 20.016 secs
> > > > >
> > > > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > > > >
> > > > >
> > > > > Adam Ford (2):
> > > > >   media: hantro: Add support for i.MX8M Mini
> > > > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > >
> > > > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > > > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > > > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > > > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > > > >  4 files changed, 102 insertions(+)
> > > > >
> > > >
> > > > Adam,
> > > >
> > > > That's for the patches!
> > > >
> > > > I tested just this series on top of v5.16-rc3 on an
> > > > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > > > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > > > is called for VPUMIX pd :
> > > > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > > > ...
> > > > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > >
> > > > I added prints in imx_pgc_power_{up,down} and
> > > > imx8m_blk_ctrl_power_{on,off} to get some more context
> > > > ...
> > > > Ran 55/61 tests successfully               in 8.685 secs
> > > >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > > > ********************************************************************************
> > > > ********************
> > > > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > > > Using 4 parallel job(s)
> > > > ********************************************************************************
> > > > ********************
> > > >
> > > > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > > > ESULT
> > > > ----------------------------------------------------------------------
> > > > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > > > [ 1023.119669] imx_pgc_power_up vpumix
> > > > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > >
> > > > While this wouldn't be an issue with this series it does indicate we
> > > > still have something racy in blk-ctrl. Can you reproduce this (and if
> > > > not what kernel are you based on)? Perhaps you or Lucas have some
> > > > ideas?
> > > >
> > > Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> > > domains" applied when running those tests? It has only recently been
> > > picked up by Shawn and may have an influence on the bus domain
> > > behavior.
> > >
> >
> > Lucas,
> >
> > Good point. I did have that originally before I started pruning down
> > to the bare minimum to reproduce the issue.
> >
> > I added it back and now I have the following:
> > arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > media: hantro: Add support for i.MX8M Mini
> > soc: imx: gpcv2: keep i.MX8MM VPU-H1 bus clock active
> > soc: imx: gpcv2: Synchronously suspend MIX domains
> > Linux 5.16-rc3
> >
> > Here's the latest with that patch:
> > ...
> > [VP8-TEST-VECTORS] (GStreamer-VP8-V4L2SL-Gst1.0)
> > vp80-00-comprehensive-007 ... Success
> > [  316.632373] imx8m_blk_ctrl_power_off vpublk-g1
> > [  316.636908] imx_pgc_power_down vpu-g1
> > [  316.640983] imx_pgc_power_down vpumix
> > [  316.756869] imx8m_blk_ctrl_power_on vpublk-g1
> > [  316.761360] imx_pgc_power_up vpumix
> > [  316.765985] imx-pgc imx-pgc-domain.6: failed to command PGC
> > [  316.772743] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > ^^^ hang
>
> Hm, I wonder if there's some broken error handling here somewhere, as a
> failure to power up a domain shouldn't lead to a hang.
>
> However, that doesn't explain why the PGC isn't completing the request.
> Can you try to extend the timeout some more. Even though I think that
> 1msec should already be generous. Can you dump the content of the
> GPC_PU_PGC_SW_PUP_REQ and GPC_A53_PU_PGC_PUP_STATUSn (all 3 of them)
> registers, when the failure condition is hit?

I submitted a patch [1]  to enable the commented-out if statement
which waits for the handshake if the gpc domain was invoked by the
blk-ctrl or we knew if the bus clock was operational.

I am not 100% certain it can work as-is with the vpumix, but based on
what I've seen from my testing, it's not hanging or causing errors.

[1] - https://lore.kernel.org/linux-arm-kernel/20211120194900.1309914-1-aford173@gmail.com/T/

I didn't have it applied to my latest RFC for the G1 and G2 because I
had not noticed a change in behavior one way or the other with that
patch.

adam
>
> Regards,
> Lucas
>

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Lucas Stach]

Am Mittwoch, dem 01.12.2021 um 12:52 -0600 schrieb Adam Ford:
> On Wed, Dec 1, 2021 at 12:37 PM Lucas Stach <l.stach@pengutronix.de> wrote:
> > 
> > Am Mittwoch, dem 01.12.2021 um 10:16 -0800 schrieb Tim Harvey:
> > > On Wed, Dec 1, 2021 at 9:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > > > 
> > > > Hi Tim,
> > > > 
> > > > Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > > > > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > 
> > > > > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > > > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > > > > how the Mini handles the power domains, the VPU driver does not need to
> > > > > > handle all the functions, nor does it support the post-processor,
> > > > > > so a new compatible flag is required.
> > > > > > 
> > > > > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > > > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > > > > it's unclear to me if that's an acceptable alternative.
> > > > > > 
> > > > > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > > > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > > > > > 
> > > > > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > > > > Ran 90/135 tests successfully               in 76.431 secs
> > > > > > 
> > > > > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > > > > Ran 55/61 tests successfully               in 21.454 secs
> > > > > > 
> > > > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > > > > Ran 0/303 tests successfully               in 20.016 secs
> > > > > > 
> > > > > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > > > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > > > > > 
> > > > > > 
> > > > > > Adam Ford (2):
> > > > > >   media: hantro: Add support for i.MX8M Mini
> > > > > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > > > 
> > > > > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > > > > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > > > > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > > > > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > > > > >  4 files changed, 102 insertions(+)
> > > > > > 
> > > > > 
> > > > > Adam,
> > > > > 
> > > > > That's for the patches!
> > > > > 
> > > > > I tested just this series on top of v5.16-rc3 on an
> > > > > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > > > > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > > > > is called for VPUMIX pd :
> > > > > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > > > > ...
> > > > > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > > 
> > > > > I added prints in imx_pgc_power_{up,down} and
> > > > > imx8m_blk_ctrl_power_{on,off} to get some more context
> > > > > ...
> > > > > Ran 55/61 tests successfully               in 8.685 secs
> > > > >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > > > > ********************************************************************************
> > > > > ********************
> > > > > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > > > > Using 4 parallel job(s)
> > > > > ********************************************************************************
> > > > > ********************
> > > > > 
> > > > > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > > > > ESULT
> > > > > ----------------------------------------------------------------------
> > > > > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > > > > [ 1023.119669] imx_pgc_power_up vpumix
> > > > > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > > 
> > > > > While this wouldn't be an issue with this series it does indicate we
> > > > > still have something racy in blk-ctrl. Can you reproduce this (and if
> > > > > not what kernel are you based on)? Perhaps you or Lucas have some
> > > > > ideas?
> > > > > 
> > > > Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> > > > domains" applied when running those tests? It has only recently been
> > > > picked up by Shawn and may have an influence on the bus domain
> > > > behavior.
> > > > 
> > > 
> > > Lucas,
> > > 
> > > Good point. I did have that originally before I started pruning down
> > > to the bare minimum to reproduce the issue.
> > > 
> > > I added it back and now I have the following:
> > > arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > media: hantro: Add support for i.MX8M Mini
> > > soc: imx: gpcv2: keep i.MX8MM VPU-H1 bus clock active
> > > soc: imx: gpcv2: Synchronously suspend MIX domains
> > > Linux 5.16-rc3
> > > 
> > > Here's the latest with that patch:
> > > ...
> > > [VP8-TEST-VECTORS] (GStreamer-VP8-V4L2SL-Gst1.0)
> > > vp80-00-comprehensive-007 ... Success
> > > [  316.632373] imx8m_blk_ctrl_power_off vpublk-g1
> > > [  316.636908] imx_pgc_power_down vpu-g1
> > > [  316.640983] imx_pgc_power_down vpumix
> > > [  316.756869] imx8m_blk_ctrl_power_on vpublk-g1
> > > [  316.761360] imx_pgc_power_up vpumix
> > > [  316.765985] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > [  316.772743] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > ^^^ hang
> > 
> > Hm, I wonder if there's some broken error handling here somewhere, as a
> > failure to power up a domain shouldn't lead to a hang.
> > 
> > However, that doesn't explain why the PGC isn't completing the request.
> > Can you try to extend the timeout some more. Even though I think that
> > 1msec should already be generous. Can you dump the content of the
> > GPC_PU_PGC_SW_PUP_REQ and GPC_A53_PU_PGC_PUP_STATUSn (all 3 of them)
> > registers, when the failure condition is hit?
> 
> I submitted a patch [1]  to enable the commented-out if statement
> which waits for the handshake if the gpc domain was invoked by the
> blk-ctrl or we knew if the bus clock was operational.
> 
> I am not 100% certain it can work as-is with the vpumix, but based on
> what I've seen from my testing, it's not hanging or causing errors.
> 
> [1] - https://lore.kernel.org/linux-arm-kernel/20211120194900.1309914-1-aford173@gmail.com/T/
> 
> I didn't have it applied to my latest RFC for the G1 and G2 because I
> had not noticed a change in behavior one way or the other with that
> patch.

That's not going to work with all the MIX domains. The handshake
requires some clocks to be enabled in the blk-ctrl (the secondary clock
gates in the blk-ctrl) to work properly. This is only done by the blk-
ctrl driver _after_ the GPC bus domain is powered up, so you can not
wait for the handshake to complete inside the GPC power up routine.

Regards,
Lucas

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Adam Ford]

On Wed, Dec 1, 2021 at 1:04 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Mittwoch, dem 01.12.2021 um 12:52 -0600 schrieb Adam Ford:
> > On Wed, Dec 1, 2021 at 12:37 PM Lucas Stach <l.stach@pengutronix.de> wrote:
> > >
> > > Am Mittwoch, dem 01.12.2021 um 10:16 -0800 schrieb Tim Harvey:
> > > > On Wed, Dec 1, 2021 at 9:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > > > >
> > > > > Hi Tim,
> > > > >
> > > > > Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > > > > > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > >
> > > > > > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > > > > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > > > > > how the Mini handles the power domains, the VPU driver does not need to
> > > > > > > handle all the functions, nor does it support the post-processor,
> > > > > > > so a new compatible flag is required.
> > > > > > >
> > > > > > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > > > > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > > > > > it's unclear to me if that's an acceptable alternative.
> > > > > > >
> > > > > > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > > > > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > > > > > >
> > > > > > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > > > > > Ran 90/135 tests successfully               in 76.431 secs
> > > > > > >
> > > > > > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > > > > > Ran 55/61 tests successfully               in 21.454 secs
> > > > > > >
> > > > > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > > > > > Ran 0/303 tests successfully               in 20.016 secs
> > > > > > >
> > > > > > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > > > > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > > > > > >
> > > > > > >
> > > > > > > Adam Ford (2):
> > > > > > >   media: hantro: Add support for i.MX8M Mini
> > > > > > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > > > >
> > > > > > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > > > > > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > > > > > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > > > > > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > > > > > >  4 files changed, 102 insertions(+)
> > > > > > >
> > > > > >
> > > > > > Adam,
> > > > > >
> > > > > > That's for the patches!
> > > > > >
> > > > > > I tested just this series on top of v5.16-rc3 on an
> > > > > > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > > > > > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > > > > > is called for VPUMIX pd :
> > > > > > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > > > > > ...
> > > > > > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > > > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > > >
> > > > > > I added prints in imx_pgc_power_{up,down} and
> > > > > > imx8m_blk_ctrl_power_{on,off} to get some more context
> > > > > > ...
> > > > > > Ran 55/61 tests successfully               in 8.685 secs
> > > > > >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > > > > > ********************************************************************************
> > > > > > ********************
> > > > > > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > > > > > Using 4 parallel job(s)
> > > > > > ********************************************************************************
> > > > > > ********************
> > > > > >
> > > > > > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > > > > > ESULT
> > > > > > ----------------------------------------------------------------------
> > > > > > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > > > > > [ 1023.119669] imx_pgc_power_up vpumix
> > > > > > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > > > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > > >
> > > > > > While this wouldn't be an issue with this series it does indicate we
> > > > > > still have something racy in blk-ctrl. Can you reproduce this (and if
> > > > > > not what kernel are you based on)? Perhaps you or Lucas have some
> > > > > > ideas?
> > > > > >
> > > > > Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> > > > > domains" applied when running those tests? It has only recently been
> > > > > picked up by Shawn and may have an influence on the bus domain
> > > > > behavior.
> > > > >
> > > >
> > > > Lucas,
> > > >
> > > > Good point. I did have that originally before I started pruning down
> > > > to the bare minimum to reproduce the issue.
> > > >
> > > > I added it back and now I have the following:
> > > > arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > media: hantro: Add support for i.MX8M Mini
> > > > soc: imx: gpcv2: keep i.MX8MM VPU-H1 bus clock active
> > > > soc: imx: gpcv2: Synchronously suspend MIX domains
> > > > Linux 5.16-rc3
> > > >
> > > > Here's the latest with that patch:
> > > > ...
> > > > [VP8-TEST-VECTORS] (GStreamer-VP8-V4L2SL-Gst1.0)
> > > > vp80-00-comprehensive-007 ... Success
> > > > [  316.632373] imx8m_blk_ctrl_power_off vpublk-g1
> > > > [  316.636908] imx_pgc_power_down vpu-g1
> > > > [  316.640983] imx_pgc_power_down vpumix
> > > > [  316.756869] imx8m_blk_ctrl_power_on vpublk-g1
> > > > [  316.761360] imx_pgc_power_up vpumix
> > > > [  316.765985] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > [  316.772743] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > ^^^ hang
> > >
> > > Hm, I wonder if there's some broken error handling here somewhere, as a
> > > failure to power up a domain shouldn't lead to a hang.
> > >
> > > However, that doesn't explain why the PGC isn't completing the request.
> > > Can you try to extend the timeout some more. Even though I think that
> > > 1msec should already be generous. Can you dump the content of the
> > > GPC_PU_PGC_SW_PUP_REQ and GPC_A53_PU_PGC_PUP_STATUSn (all 3 of them)
> > > registers, when the failure condition is hit?
> >
> > I submitted a patch [1]  to enable the commented-out if statement
> > which waits for the handshake if the gpc domain was invoked by the
> > blk-ctrl or we knew if the bus clock was operational.
> >
> > I am not 100% certain it can work as-is with the vpumix, but based on
> > what I've seen from my testing, it's not hanging or causing errors.
> >
> > [1] - https://lore.kernel.org/linux-arm-kernel/20211120194900.1309914-1-aford173@gmail.com/T/
> >
> > I didn't have it applied to my latest RFC for the G1 and G2 because I
> > had not noticed a change in behavior one way or the other with that
> > patch.
>
> That's not going to work with all the MIX domains. The handshake
> requires some clocks to be enabled in the blk-ctrl (the secondary clock
> gates in the blk-ctrl) to work properly. This is only done by the blk-
> ctrl driver _after_ the GPC bus domain is powered up, so you can not
> wait for the handshake to complete inside the GPC power up routine.

I wasn't exactly sure how the handshake worked.  What your saying makes sense.
Will you NAK my patch so it doesn't accidentally get applied.

adam
>
> Regards,
> Lucas
>

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Tim Harvey]

On Wed, Dec 1, 2021 at 10:37 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Mittwoch, dem 01.12.2021 um 10:16 -0800 schrieb Tim Harvey:
> > On Wed, Dec 1, 2021 at 9:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > >
> > > Hi Tim,
> > >
> > > Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > > > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > > > >
> > > > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > > > how the Mini handles the power domains, the VPU driver does not need to
> > > > > handle all the functions, nor does it support the post-processor,
> > > > > so a new compatible flag is required.
> > > > >
> > > > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > > > it's unclear to me if that's an acceptable alternative.
> > > > >
> > > > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > > > >
> > > > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > > > Ran 90/135 tests successfully               in 76.431 secs
> > > > >
> > > > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > > > Ran 55/61 tests successfully               in 21.454 secs
> > > > >
> > > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > > > Ran 0/303 tests successfully               in 20.016 secs
> > > > >
> > > > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > > > >
> > > > >
> > > > > Adam Ford (2):
> > > > >   media: hantro: Add support for i.MX8M Mini
> > > > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > >
> > > > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > > > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > > > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > > > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > > > >  4 files changed, 102 insertions(+)
> > > > >
> > > >
> > > > Adam,
> > > >
> > > > That's for the patches!
> > > >
> > > > I tested just this series on top of v5.16-rc3 on an
> > > > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > > > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > > > is called for VPUMIX pd :
> > > > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > > > ...
> > > > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > >
> > > > I added prints in imx_pgc_power_{up,down} and
> > > > imx8m_blk_ctrl_power_{on,off} to get some more context
> > > > ...
> > > > Ran 55/61 tests successfully               in 8.685 secs
> > > >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > > > ********************************************************************************
> > > > ********************
> > > > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > > > Using 4 parallel job(s)
> > > > ********************************************************************************
> > > > ********************
> > > >
> > > > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > > > ESULT
> > > > ----------------------------------------------------------------------
> > > > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > > > [ 1023.119669] imx_pgc_power_up vpumix
> > > > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > >
> > > > While this wouldn't be an issue with this series it does indicate we
> > > > still have something racy in blk-ctrl. Can you reproduce this (and if
> > > > not what kernel are you based on)? Perhaps you or Lucas have some
> > > > ideas?
> > > >
> > > Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> > > domains" applied when running those tests? It has only recently been
> > > picked up by Shawn and may have an influence on the bus domain
> > > behavior.
> > >
> >
> > Lucas,
> >
> > Good point. I did have that originally before I started pruning down
> > to the bare minimum to reproduce the issue.
> >
> > I added it back and now I have the following:
> > arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > media: hantro: Add support for i.MX8M Mini
> > soc: imx: gpcv2: keep i.MX8MM VPU-H1 bus clock active
> > soc: imx: gpcv2: Synchronously suspend MIX domains
> > Linux 5.16-rc3
> >
> > Here's the latest with that patch:
> > ...
> > [VP8-TEST-VECTORS] (GStreamer-VP8-V4L2SL-Gst1.0)
> > vp80-00-comprehensive-007 ... Success
> > [  316.632373] imx8m_blk_ctrl_power_off vpublk-g1
> > [  316.636908] imx_pgc_power_down vpu-g1
> > [  316.640983] imx_pgc_power_down vpumix
> > [  316.756869] imx8m_blk_ctrl_power_on vpublk-g1
> > [  316.761360] imx_pgc_power_up vpumix
> > [  316.765985] imx-pgc imx-pgc-domain.6: failed to command PGC
> > [  316.772743] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > ^^^ hang
>
> Hm, I wonder if there's some broken error handling here somewhere, as a
> failure to power up a domain shouldn't lead to a hang.
>
> However, that doesn't explain why the PGC isn't completing the request.
> Can you try to extend the timeout some more. Even though I think that
> 1msec should already be generous. Can you dump the content of the
> GPC_PU_PGC_SW_PUP_REQ and GPC_A53_PU_PGC_PUP_STATUSn (all 3 of them)
> registers, when the failure condition is hit?
>

Adam,

Adding keep_clocks=true to VPUG1/VPUG2 domains did not help

Lucas,

I bumped the regmap_read_poll_timeout timeouts from 1m to 100ms and
still saw the same issue.

Here's some added debugging to show the regs:
[  648.037903] imx8m_blk_ctrl_power_on vpublk-g1
[  648.042346] imx_pgc_power_up vpumix
[  648.146178] imx-pgc imx-pgc-domain.6: imx_pgc_power_up: failed to command PGC
[  648.153355] imx-pgc imx-pgc-domain.6: GPC_PU_PGC_SW_PUP_REQ(0x0f8)=0x00000100
[  648.162339] imx-pgc imx-pgc-domain.6:
GPC_A53_PU_PGC_PUP_STATUS0(0x14c)=0x00000000
[  648.169988] imx-pgc imx-pgc-domain.6:
GPC_A53_PU_PGC_PUP_STATUS1(0x150)=0x00000000
[  648.177618] imx-pgc imx-pgc-domain.6:
GPC_A53_PU_PGC_PUP_STATUS2(0x154)=0x00000000
[  648.185281] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 8176380b02e6..8124a3434655 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -67,6 +67,9 @@

 #define GPC_PU_PGC_SW_PUP_REQ          0x0f8
 #define GPC_PU_PGC_SW_PDN_REQ          0x104
+#define GPC_A53_PU_PGC_PUP_STATUS0     0x14c
+#define GPC_A53_PU_PGC_PUP_STATUS1     0x150
+#define GPC_A53_PU_PGC_PUP_STATUS2     0x154

 #define IMX7_USB_HSIC_PHY_SW_Pxx_REQ           BIT(4)
 #define IMX7_USB_OTG2_PHY_SW_Pxx_REQ           BIT(3)
@@ -224,6 +227,7 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
        u32 reg_val, pgc;
        int ret;

+printk("%s %s\n", __func__, genpd->name);
        ret = pm_runtime_get_sync(domain->dev);
        if (ret < 0) {
                pm_runtime_put_noidle(domain->dev);
@@ -258,9 +262,17 @@ static int imx_pgc_power_up(struct
generic_pm_domain *genpd)
                ret = regmap_read_poll_timeout(domain->regmap,
                                               GPC_PU_PGC_SW_PUP_REQ, reg_val,
                                               !(reg_val & domain->bits.pxx),
-                                              0, USEC_PER_MSEC);
+                                              0, 100 * USEC_PER_MSEC);
                if (ret) {
-                       dev_err(domain->dev, "failed to command PGC\n");
+                       dev_err(domain->dev, "%s: failed to command
PGC\n", __func__);
+                       if (!regmap_read(domain->regmap,
GPC_PU_PGC_SW_PUP_REQ, &reg_val))
+                               dev_err(domain->dev,
"GPC_PU_PGC_SW_PUP_REQ(0x%03x)=0x%08x\n", GPC_PU_PGC_SW_PUP_REQ,
reg_val);
+                       if (!regmap_read(domain->regmap,
GPC_A53_PU_PGC_PUP_STATUS0, &reg_val))
+                               dev_err(domain->dev,
"GPC_A53_PU_PGC_PUP_STATUS0(0x%03x)=0x%08x\n",
GPC_A53_PU_PGC_PUP_STATUS0, reg_val);
+                       if (!regmap_read(domain->regmap,
GPC_A53_PU_PGC_PUP_STATUS1, &reg_val))
+                               dev_err(domain->dev,
"GPC_A53_PU_PGC_PUP_STATUS1(0x%03x)=0x%08x\n",
GPC_A53_PU_PGC_PUP_STATUS1, reg_val);
+                       if (!regmap_read(domain->regmap,
GPC_A53_PU_PGC_PUP_STATUS2, &reg_val))
+                               dev_err(domain->dev,
"GPC_A53_PU_PGC_PUP_STATUS2(0x%03x)=0x%08x\n",
GPC_A53_PU_PGC_PUP_STATUS2, reg_val);
                        goto out_clk_disable;
                }

@@ -318,6 +330,7 @@ static int imx_pgc_power_down(struct
generic_pm_domain *genpd)
        u32 reg_val, pgc;
        int ret;

+printk("%s %s\n", __func__, genpd->name);
        /* Enable reset clocks for all devices in the domain */
        if (!domain->keep_clocks) {
                ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
@@ -335,7 +348,7 @@ static int imx_pgc_power_down(struct
generic_pm_domain *genpd)
                ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
                                               reg_val,
                                               !(reg_val & domain->bits.hskack),
-                                              0, USEC_PER_MSEC);
+                                              0, 100 * USEC_PER_MSEC);
                if (ret) {
                        dev_err(domain->dev, "failed to power down ADB400\n");
                        goto out_clk_disable;
@@ -359,9 +372,9 @@ static int imx_pgc_power_down(struct
generic_pm_domain *genpd)
                ret = regmap_read_poll_timeout(domain->regmap,
                                               GPC_PU_PGC_SW_PDN_REQ, reg_val,
                                               !(reg_val & domain->bits.pxx),
-                                              0, USEC_PER_MSEC);
+                                              0, 100 * USEC_PER_MSEC);
                if (ret) {
-                       dev_err(domain->dev, "failed to command PGC\n");
+                       dev_err(domain->dev, "%s: failed to command
PGC\n", __func__);
                        goto out_clk_disable;
                }
        }
@@ -712,6 +725,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
                        .map = IMX8MM_VPUG1_A53_DOMAIN,
                },
                .pgc   = BIT(IMX8MM_PGC_VPUG1),
+               .keep_clocks = true,
        },

        [IMX8MM_POWER_DOMAIN_VPUG2] = {
@@ -723,6 +737,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
                        .map = IMX8MM_VPUG2_A53_DOMAIN,
                },
                .pgc   = BIT(IMX8MM_PGC_VPUG2),
+               .keep_clocks = true,
        },

        [IMX8MM_POWER_DOMAIN_VPUH1] = {
diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
index 519b3651d1d9..028f38d45892 100644
--- a/drivers/soc/imx/imx8m-blk-ctrl.c
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -68,6 +68,7 @@ static int imx8m_blk_ctrl_power_on(struct
generic_pm_domain *genpd)
        struct imx8m_blk_ctrl *bc = domain->bc;
        int ret;

+printk("%s %s\n", __func__, genpd->name);
        /* make sure bus domain is awake */
        ret = pm_runtime_get_sync(bc->bus_power_dev);
        if (ret < 0) {
@@ -119,6 +120,7 @@ static int imx8m_blk_ctrl_power_off(struct
generic_pm_domain *genpd)
        const struct imx8m_blk_ctrl_domain_data *data = domain->data;
        struct imx8m_blk_ctrl *bc = domain->bc;

+printk("%s %s\n", __func__, genpd->name);
        /* put devices into reset and disable clocks */
        regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
        regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);

Tim

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Adam Ford]

On Wed, Dec 1, 2021 at 2:04 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Wed, Dec 1, 2021 at 10:37 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > Am Mittwoch, dem 01.12.2021 um 10:16 -0800 schrieb Tim Harvey:
> > > On Wed, Dec 1, 2021 at 9:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > > >
> > > > Hi Tim,
> > > >
> > > > Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > > > > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > >
> > > > > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > > > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > > > > how the Mini handles the power domains, the VPU driver does not need to
> > > > > > handle all the functions, nor does it support the post-processor,
> > > > > > so a new compatible flag is required.
> > > > > >
> > > > > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > > > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > > > > it's unclear to me if that's an acceptable alternative.
> > > > > >
> > > > > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > > > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > > > > >
> > > > > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > > > > Ran 90/135 tests successfully               in 76.431 secs
> > > > > >
> > > > > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > > > > Ran 55/61 tests successfully               in 21.454 secs
> > > > > >
> > > > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > > > > Ran 0/303 tests successfully               in 20.016 secs
> > > > > >
> > > > > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > > > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > > > > >
> > > > > >
> > > > > > Adam Ford (2):
> > > > > >   media: hantro: Add support for i.MX8M Mini
> > > > > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > > >
> > > > > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > > > > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > > > > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > > > > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > > > > >  4 files changed, 102 insertions(+)
> > > > > >
> > > > >
> > > > > Adam,
> > > > >
> > > > > That's for the patches!
> > > > >
> > > > > I tested just this series on top of v5.16-rc3 on an
> > > > > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > > > > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > > > > is called for VPUMIX pd :
> > > > > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > > > > ...
> > > > > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > >
> > > > > I added prints in imx_pgc_power_{up,down} and
> > > > > imx8m_blk_ctrl_power_{on,off} to get some more context
> > > > > ...
> > > > > Ran 55/61 tests successfully               in 8.685 secs
> > > > >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > > > > ********************************************************************************
> > > > > ********************
> > > > > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > > > > Using 4 parallel job(s)
> > > > > ********************************************************************************
> > > > > ********************
> > > > >
> > > > > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > > > > ESULT
> > > > > ----------------------------------------------------------------------
> > > > > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > > > > [ 1023.119669] imx_pgc_power_up vpumix
> > > > > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > >
> > > > > While this wouldn't be an issue with this series it does indicate we
> > > > > still have something racy in blk-ctrl. Can you reproduce this (and if
> > > > > not what kernel are you based on)? Perhaps you or Lucas have some
> > > > > ideas?
> > > > >
> > > > Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> > > > domains" applied when running those tests? It has only recently been
> > > > picked up by Shawn and may have an influence on the bus domain
> > > > behavior.
> > > >
> > >
> > > Lucas,
> > >
> > > Good point. I did have that originally before I started pruning down
> > > to the bare minimum to reproduce the issue.
> > >
> > > I added it back and now I have the following:
> > > arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > media: hantro: Add support for i.MX8M Mini
> > > soc: imx: gpcv2: keep i.MX8MM VPU-H1 bus clock active
> > > soc: imx: gpcv2: Synchronously suspend MIX domains
> > > Linux 5.16-rc3
> > >
> > > Here's the latest with that patch:
> > > ...
> > > [VP8-TEST-VECTORS] (GStreamer-VP8-V4L2SL-Gst1.0)
> > > vp80-00-comprehensive-007 ... Success
> > > [  316.632373] imx8m_blk_ctrl_power_off vpublk-g1
> > > [  316.636908] imx_pgc_power_down vpu-g1
> > > [  316.640983] imx_pgc_power_down vpumix
> > > [  316.756869] imx8m_blk_ctrl_power_on vpublk-g1
> > > [  316.761360] imx_pgc_power_up vpumix
> > > [  316.765985] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > [  316.772743] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > ^^^ hang
> >
> > Hm, I wonder if there's some broken error handling here somewhere, as a
> > failure to power up a domain shouldn't lead to a hang.
> >
> > However, that doesn't explain why the PGC isn't completing the request.
> > Can you try to extend the timeout some more. Even though I think that
> > 1msec should already be generous. Can you dump the content of the
> > GPC_PU_PGC_SW_PUP_REQ and GPC_A53_PU_PGC_PUP_STATUSn (all 3 of them)
> > registers, when the failure condition is hit?
> >

I haven't been able to repeat your findings on G1, but when testing
VP9 on the G2 decoder, I get the following:

[VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
vp90-2-07-frame_parallel.webm                   ... Success
[VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
vp90-2-07-frame_parallel-1.webm                 ... Success
[VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
vp90-2-08-tile_1x4_frame_parallel.webm          ... Timeout
[VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
vp90-2-08-tile_1x2.webm                         ... Timeout
[VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
vp90-2-02-size-lf-1920x1080.webm                ... Timeout
[VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
vp90-2-08-tile_1x2_frame_parallel.webm          ... Timeout
[  192.971101] cma: cma_alloc: reserved: alloc failed, req-size: 3548
pages, ret: -12
[  192.979748] hantro-vpu 38310000.video-codec: dma alloc of size
14532608 failed
[  192.988683] cma: cma_alloc: reserved: alloc failed, req-size: 938
pages, ret: -12
[VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
vp90-2-08-tile_1x8.webm                         ... Error
[  195.296712] imx-pgc imx-pgc-domain.6: failed to command PGC
[  195.302396] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain

At some point the G2 times out, starts to choke on memory, then the
power domain fails, and then the system hangs.  I have a heartbeat
GPIO running so I know if the kernel is alive or not.  The heartbeat
stops, so I know it's locked up tightly.
I guess the good news is that the G2 decoder is able to run 100-ish
successful tests before it falls down.

Either way, I'll post a RFC V3 for the Hantro stuff, but I wonder if
the order of operations on the power-domain powerdown is an issue.

adam

>
> Adam,
>
> Adding keep_clocks=true to VPUG1/VPUG2 domains did not help
>
> Lucas,
>
> I bumped the regmap_read_poll_timeout timeouts from 1m to 100ms and
> still saw the same issue.
>
> Here's some added debugging to show the regs:
> [  648.037903] imx8m_blk_ctrl_power_on vpublk-g1
> [  648.042346] imx_pgc_power_up vpumix
> [  648.146178] imx-pgc imx-pgc-domain.6: imx_pgc_power_up: failed to command PGC
> [  648.153355] imx-pgc imx-pgc-domain.6: GPC_PU_PGC_SW_PUP_REQ(0x0f8)=0x00000100
> [  648.162339] imx-pgc imx-pgc-domain.6:
> GPC_A53_PU_PGC_PUP_STATUS0(0x14c)=0x00000000
> [  648.169988] imx-pgc imx-pgc-domain.6:
> GPC_A53_PU_PGC_PUP_STATUS1(0x150)=0x00000000
> [  648.177618] imx-pgc imx-pgc-domain.6:
> GPC_A53_PU_PGC_PUP_STATUS2(0x154)=0x00000000
> [  648.185281] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
>
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 8176380b02e6..8124a3434655 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -67,6 +67,9 @@
>
>  #define GPC_PU_PGC_SW_PUP_REQ          0x0f8
>  #define GPC_PU_PGC_SW_PDN_REQ          0x104
> +#define GPC_A53_PU_PGC_PUP_STATUS0     0x14c
> +#define GPC_A53_PU_PGC_PUP_STATUS1     0x150
> +#define GPC_A53_PU_PGC_PUP_STATUS2     0x154
>
>  #define IMX7_USB_HSIC_PHY_SW_Pxx_REQ           BIT(4)
>  #define IMX7_USB_OTG2_PHY_SW_Pxx_REQ           BIT(3)
> @@ -224,6 +227,7 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>         u32 reg_val, pgc;
>         int ret;
>
> +printk("%s %s\n", __func__, genpd->name);
>         ret = pm_runtime_get_sync(domain->dev);
>         if (ret < 0) {
>                 pm_runtime_put_noidle(domain->dev);
> @@ -258,9 +262,17 @@ static int imx_pgc_power_up(struct
> generic_pm_domain *genpd)
>                 ret = regmap_read_poll_timeout(domain->regmap,
>                                                GPC_PU_PGC_SW_PUP_REQ, reg_val,
>                                                !(reg_val & domain->bits.pxx),
> -                                              0, USEC_PER_MSEC);
> +                                              0, 100 * USEC_PER_MSEC);
>                 if (ret) {
> -                       dev_err(domain->dev, "failed to command PGC\n");
> +                       dev_err(domain->dev, "%s: failed to command
> PGC\n", __func__);
> +                       if (!regmap_read(domain->regmap,
> GPC_PU_PGC_SW_PUP_REQ, &reg_val))
> +                               dev_err(domain->dev,
> "GPC_PU_PGC_SW_PUP_REQ(0x%03x)=0x%08x\n", GPC_PU_PGC_SW_PUP_REQ,
> reg_val);
> +                       if (!regmap_read(domain->regmap,
> GPC_A53_PU_PGC_PUP_STATUS0, &reg_val))
> +                               dev_err(domain->dev,
> "GPC_A53_PU_PGC_PUP_STATUS0(0x%03x)=0x%08x\n",
> GPC_A53_PU_PGC_PUP_STATUS0, reg_val);
> +                       if (!regmap_read(domain->regmap,
> GPC_A53_PU_PGC_PUP_STATUS1, &reg_val))
> +                               dev_err(domain->dev,
> "GPC_A53_PU_PGC_PUP_STATUS1(0x%03x)=0x%08x\n",
> GPC_A53_PU_PGC_PUP_STATUS1, reg_val);
> +                       if (!regmap_read(domain->regmap,
> GPC_A53_PU_PGC_PUP_STATUS2, &reg_val))
> +                               dev_err(domain->dev,
> "GPC_A53_PU_PGC_PUP_STATUS2(0x%03x)=0x%08x\n",
> GPC_A53_PU_PGC_PUP_STATUS2, reg_val);
>                         goto out_clk_disable;
>                 }
>
> @@ -318,6 +330,7 @@ static int imx_pgc_power_down(struct
> generic_pm_domain *genpd)
>         u32 reg_val, pgc;
>         int ret;
>
> +printk("%s %s\n", __func__, genpd->name);
>         /* Enable reset clocks for all devices in the domain */
>         if (!domain->keep_clocks) {
>                 ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
> @@ -335,7 +348,7 @@ static int imx_pgc_power_down(struct
> generic_pm_domain *genpd)
>                 ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
>                                                reg_val,
>                                                !(reg_val & domain->bits.hskack),
> -                                              0, USEC_PER_MSEC);
> +                                              0, 100 * USEC_PER_MSEC);
>                 if (ret) {
>                         dev_err(domain->dev, "failed to power down ADB400\n");
>                         goto out_clk_disable;
> @@ -359,9 +372,9 @@ static int imx_pgc_power_down(struct
> generic_pm_domain *genpd)
>                 ret = regmap_read_poll_timeout(domain->regmap,
>                                                GPC_PU_PGC_SW_PDN_REQ, reg_val,
>                                                !(reg_val & domain->bits.pxx),
> -                                              0, USEC_PER_MSEC);
> +                                              0, 100 * USEC_PER_MSEC);
>                 if (ret) {
> -                       dev_err(domain->dev, "failed to command PGC\n");
> +                       dev_err(domain->dev, "%s: failed to command
> PGC\n", __func__);
>                         goto out_clk_disable;
>                 }
>         }
> @@ -712,6 +725,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
>                         .map = IMX8MM_VPUG1_A53_DOMAIN,
>                 },
>                 .pgc   = BIT(IMX8MM_PGC_VPUG1),
> +               .keep_clocks = true,
>         },
>
>         [IMX8MM_POWER_DOMAIN_VPUG2] = {
> @@ -723,6 +737,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
>                         .map = IMX8MM_VPUG2_A53_DOMAIN,
>                 },
>                 .pgc   = BIT(IMX8MM_PGC_VPUG2),
> +               .keep_clocks = true,
>         },
>
>         [IMX8MM_POWER_DOMAIN_VPUH1] = {
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> index 519b3651d1d9..028f38d45892 100644
> --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -68,6 +68,7 @@ static int imx8m_blk_ctrl_power_on(struct
> generic_pm_domain *genpd)
>         struct imx8m_blk_ctrl *bc = domain->bc;
>         int ret;
>
> +printk("%s %s\n", __func__, genpd->name);
>         /* make sure bus domain is awake */
>         ret = pm_runtime_get_sync(bc->bus_power_dev);
>         if (ret < 0) {
> @@ -119,6 +120,7 @@ static int imx8m_blk_ctrl_power_off(struct
> generic_pm_domain *genpd)
>         const struct imx8m_blk_ctrl_domain_data *data = domain->data;
>         struct imx8m_blk_ctrl *bc = domain->bc;
>
> +printk("%s %s\n", __func__, genpd->name);
>         /* put devices into reset and disable clocks */
>         regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
>         regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
>
> Tim

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Adam Ford]

On Wed, Dec 1, 2021 at 7:07 PM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Dec 1, 2021 at 2:04 PM Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Wed, Dec 1, 2021 at 10:37 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > >
> > > Am Mittwoch, dem 01.12.2021 um 10:16 -0800 schrieb Tim Harvey:
> > > > On Wed, Dec 1, 2021 at 9:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > > > >
> > > > > Hi Tim,
> > > > >
> > > > > Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > > > > > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > >
> > > > > > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > > > > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > > > > > how the Mini handles the power domains, the VPU driver does not need to
> > > > > > > handle all the functions, nor does it support the post-processor,
> > > > > > > so a new compatible flag is required.
> > > > > > >
> > > > > > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > > > > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > > > > > it's unclear to me if that's an acceptable alternative.
> > > > > > >
> > > > > > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > > > > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > > > > > >
> > > > > > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > > > > > Ran 90/135 tests successfully               in 76.431 secs
> > > > > > >
> > > > > > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > > > > > Ran 55/61 tests successfully               in 21.454 secs
> > > > > > >
> > > > > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > > > > > Ran 0/303 tests successfully               in 20.016 secs
> > > > > > >
> > > > > > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > > > > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > > > > > >
> > > > > > >
> > > > > > > Adam Ford (2):
> > > > > > >   media: hantro: Add support for i.MX8M Mini
> > > > > > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > > > >
> > > > > > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > > > > > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > > > > > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > > > > > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > > > > > >  4 files changed, 102 insertions(+)
> > > > > > >
> > > > > >
> > > > > > Adam,
> > > > > >
> > > > > > That's for the patches!
> > > > > >
> > > > > > I tested just this series on top of v5.16-rc3 on an
> > > > > > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > > > > > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > > > > > is called for VPUMIX pd :
> > > > > > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > > > > > ...
> > > > > > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > > > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > > >
> > > > > > I added prints in imx_pgc_power_{up,down} and
> > > > > > imx8m_blk_ctrl_power_{on,off} to get some more context
> > > > > > ...
> > > > > > Ran 55/61 tests successfully               in 8.685 secs
> > > > > >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > > > > > ********************************************************************************
> > > > > > ********************
> > > > > > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > > > > > Using 4 parallel job(s)
> > > > > > ********************************************************************************
> > > > > > ********************
> > > > > >
> > > > > > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > > > > > ESULT
> > > > > > ----------------------------------------------------------------------
> > > > > > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > > > > > [ 1023.119669] imx_pgc_power_up vpumix
> > > > > > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > > > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > > >
> > > > > > While this wouldn't be an issue with this series it does indicate we
> > > > > > still have something racy in blk-ctrl. Can you reproduce this (and if
> > > > > > not what kernel are you based on)? Perhaps you or Lucas have some
> > > > > > ideas?
> > > > > >
> > > > > Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> > > > > domains" applied when running those tests? It has only recently been
> > > > > picked up by Shawn and may have an influence on the bus domain
> > > > > behavior.
> > > > >
> > > >
> > > > Lucas,
> > > >
> > > > Good point. I did have that originally before I started pruning down
> > > > to the bare minimum to reproduce the issue.
> > > >
> > > > I added it back and now I have the following:
> > > > arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > > > media: hantro: Add support for i.MX8M Mini
> > > > soc: imx: gpcv2: keep i.MX8MM VPU-H1 bus clock active
> > > > soc: imx: gpcv2: Synchronously suspend MIX domains
> > > > Linux 5.16-rc3
> > > >
> > > > Here's the latest with that patch:
> > > > ...
> > > > [VP8-TEST-VECTORS] (GStreamer-VP8-V4L2SL-Gst1.0)
> > > > vp80-00-comprehensive-007 ... Success
> > > > [  316.632373] imx8m_blk_ctrl_power_off vpublk-g1
> > > > [  316.636908] imx_pgc_power_down vpu-g1
> > > > [  316.640983] imx_pgc_power_down vpumix
> > > > [  316.756869] imx8m_blk_ctrl_power_on vpublk-g1
> > > > [  316.761360] imx_pgc_power_up vpumix
> > > > [  316.765985] imx-pgc imx-pgc-domain.6: failed to command PGC
> > > > [  316.772743] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> > > > ^^^ hang
> > >
> > > Hm, I wonder if there's some broken error handling here somewhere, as a
> > > failure to power up a domain shouldn't lead to a hang.
> > >
> > > However, that doesn't explain why the PGC isn't completing the request.
> > > Can you try to extend the timeout some more. Even though I think that
> > > 1msec should already be generous. Can you dump the content of the
> > > GPC_PU_PGC_SW_PUP_REQ and GPC_A53_PU_PGC_PUP_STATUSn (all 3 of them)
> > > registers, when the failure condition is hit?
> > >
>
> I haven't been able to repeat your findings on G1, but when testing
> VP9 on the G2 decoder, I get the following:
>
> [VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
> vp90-2-07-frame_parallel.webm                   ... Success
> [VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
> vp90-2-07-frame_parallel-1.webm                 ... Success
> [VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
> vp90-2-08-tile_1x4_frame_parallel.webm          ... Timeout
> [VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
> vp90-2-08-tile_1x2.webm                         ... Timeout
> [VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
> vp90-2-02-size-lf-1920x1080.webm                ... Timeout
> [VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
> vp90-2-08-tile_1x2_frame_parallel.webm          ... Timeout
> [  192.971101] cma: cma_alloc: reserved: alloc failed, req-size: 3548
> pages, ret: -12
> [  192.979748] hantro-vpu 38310000.video-codec: dma alloc of size
> 14532608 failed
> [  192.988683] cma: cma_alloc: reserved: alloc failed, req-size: 938
> pages, ret: -12
> [VP9-TEST-VECTORS] (GStreamer-VP9-V4L2SL-Gst1.0)
> vp90-2-08-tile_1x8.webm                         ... Error
> [  195.296712] imx-pgc imx-pgc-domain.6: failed to command PGC
> [  195.302396] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
>
> At some point the G2 times out, starts to choke on memory, then the
> power domain fails, and then the system hangs.  I have a heartbeat
> GPIO running so I know if the kernel is alive or not.  The heartbeat
> stops, so I know it's locked up tightly.
> I guess the good news is that the G2 decoder is able to run 100-ish
> successful tests before it falls down.
>
> Either way, I'll post a RFC V3 for the Hantro stuff, but I wonder if
> the order of operations on the power-domain powerdown is an issue.
>
> adam
>
> >
> > Adam,
> >
> > Adding keep_clocks=true to VPUG1/VPUG2 domains did not help
> >
> > Lucas,
> >
> > I bumped the regmap_read_poll_timeout timeouts from 1m to 100ms and
> > still saw the same issue.
> >
> > Here's some added debugging to show the regs:
> > [  648.037903] imx8m_blk_ctrl_power_on vpublk-g1
> > [  648.042346] imx_pgc_power_up vpumix
> > [  648.146178] imx-pgc imx-pgc-domain.6: imx_pgc_power_up: failed to command PGC
> > [  648.153355] imx-pgc imx-pgc-domain.6: GPC_PU_PGC_SW_PUP_REQ(0x0f8)=0x00000100
> > [  648.162339] imx-pgc imx-pgc-domain.6:
> > GPC_A53_PU_PGC_PUP_STATUS0(0x14c)=0x00000000
> > [  648.169988] imx-pgc imx-pgc-domain.6:
> > GPC_A53_PU_PGC_PUP_STATUS1(0x150)=0x00000000
> > [  648.177618] imx-pgc imx-pgc-domain.6:
> > GPC_A53_PU_PGC_PUP_STATUS2(0x154)=0x00000000
> > [  648.185281] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> >

Tim / Lucas,

I was able to run the G2 decoder for 203 seconds using fluster, and it
didn't lock up.  I used a combination of the synchronous suspend, and
I removed the VPU reset reference.
Looking at the NXP power domain controller in both Linux and ATF,
neither appear to be referencing IMX8MQ_RESET_VPU_RESET anywhere.  We
are referencing it in the pgc_vpumix node, so I commented it out.
With those two items changed, I was able to get the G2 operational.
I'm going to post another RFC with the reset in the vpumix removed and
a note to what I am using for a starting point.

Ran 127/303 tests successfully               in 203.873 secs

adam

> > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> > index 8176380b02e6..8124a3434655 100644
> > --- a/drivers/soc/imx/gpcv2.c
> > +++ b/drivers/soc/imx/gpcv2.c
> > @@ -67,6 +67,9 @@
> >
> >  #define GPC_PU_PGC_SW_PUP_REQ          0x0f8
> >  #define GPC_PU_PGC_SW_PDN_REQ          0x104
> > +#define GPC_A53_PU_PGC_PUP_STATUS0     0x14c
> > +#define GPC_A53_PU_PGC_PUP_STATUS1     0x150
> > +#define GPC_A53_PU_PGC_PUP_STATUS2     0x154
> >
> >  #define IMX7_USB_HSIC_PHY_SW_Pxx_REQ           BIT(4)
> >  #define IMX7_USB_OTG2_PHY_SW_Pxx_REQ           BIT(3)
> > @@ -224,6 +227,7 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> >         u32 reg_val, pgc;
> >         int ret;
> >
> > +printk("%s %s\n", __func__, genpd->name);
> >         ret = pm_runtime_get_sync(domain->dev);
> >         if (ret < 0) {
> >                 pm_runtime_put_noidle(domain->dev);
> > @@ -258,9 +262,17 @@ static int imx_pgc_power_up(struct
> > generic_pm_domain *genpd)
> >                 ret = regmap_read_poll_timeout(domain->regmap,
> >                                                GPC_PU_PGC_SW_PUP_REQ, reg_val,
> >                                                !(reg_val & domain->bits.pxx),
> > -                                              0, USEC_PER_MSEC);
> > +                                              0, 100 * USEC_PER_MSEC);
> >                 if (ret) {
> > -                       dev_err(domain->dev, "failed to command PGC\n");
> > +                       dev_err(domain->dev, "%s: failed to command
> > PGC\n", __func__);
> > +                       if (!regmap_read(domain->regmap,
> > GPC_PU_PGC_SW_PUP_REQ, &reg_val))
> > +                               dev_err(domain->dev,
> > "GPC_PU_PGC_SW_PUP_REQ(0x%03x)=0x%08x\n", GPC_PU_PGC_SW_PUP_REQ,
> > reg_val);
> > +                       if (!regmap_read(domain->regmap,
> > GPC_A53_PU_PGC_PUP_STATUS0, &reg_val))
> > +                               dev_err(domain->dev,
> > "GPC_A53_PU_PGC_PUP_STATUS0(0x%03x)=0x%08x\n",
> > GPC_A53_PU_PGC_PUP_STATUS0, reg_val);
> > +                       if (!regmap_read(domain->regmap,
> > GPC_A53_PU_PGC_PUP_STATUS1, &reg_val))
> > +                               dev_err(domain->dev,
> > "GPC_A53_PU_PGC_PUP_STATUS1(0x%03x)=0x%08x\n",
> > GPC_A53_PU_PGC_PUP_STATUS1, reg_val);
> > +                       if (!regmap_read(domain->regmap,
> > GPC_A53_PU_PGC_PUP_STATUS2, &reg_val))
> > +                               dev_err(domain->dev,
> > "GPC_A53_PU_PGC_PUP_STATUS2(0x%03x)=0x%08x\n",
> > GPC_A53_PU_PGC_PUP_STATUS2, reg_val);
> >                         goto out_clk_disable;
> >                 }
> >
> > @@ -318,6 +330,7 @@ static int imx_pgc_power_down(struct
> > generic_pm_domain *genpd)
> >         u32 reg_val, pgc;
> >         int ret;
> >
> > +printk("%s %s\n", __func__, genpd->name);
> >         /* Enable reset clocks for all devices in the domain */
> >         if (!domain->keep_clocks) {
> >                 ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
> > @@ -335,7 +348,7 @@ static int imx_pgc_power_down(struct
> > generic_pm_domain *genpd)
> >                 ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
> >                                                reg_val,
> >                                                !(reg_val & domain->bits.hskack),
> > -                                              0, USEC_PER_MSEC);
> > +                                              0, 100 * USEC_PER_MSEC);
> >                 if (ret) {
> >                         dev_err(domain->dev, "failed to power down ADB400\n");
> >                         goto out_clk_disable;
> > @@ -359,9 +372,9 @@ static int imx_pgc_power_down(struct
> > generic_pm_domain *genpd)
> >                 ret = regmap_read_poll_timeout(domain->regmap,
> >                                                GPC_PU_PGC_SW_PDN_REQ, reg_val,
> >                                                !(reg_val & domain->bits.pxx),
> > -                                              0, USEC_PER_MSEC);
> > +                                              0, 100 * USEC_PER_MSEC);
> >                 if (ret) {
> > -                       dev_err(domain->dev, "failed to command PGC\n");
> > +                       dev_err(domain->dev, "%s: failed to command
> > PGC\n", __func__);
> >                         goto out_clk_disable;
> >                 }
> >         }
> > @@ -712,6 +725,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> >                         .map = IMX8MM_VPUG1_A53_DOMAIN,
> >                 },
> >                 .pgc   = BIT(IMX8MM_PGC_VPUG1),
> > +               .keep_clocks = true,
> >         },
> >
> >         [IMX8MM_POWER_DOMAIN_VPUG2] = {
> > @@ -723,6 +737,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> >                         .map = IMX8MM_VPUG2_A53_DOMAIN,
> >                 },
> >                 .pgc   = BIT(IMX8MM_PGC_VPUG2),
> > +               .keep_clocks = true,
> >         },
> >
> >         [IMX8MM_POWER_DOMAIN_VPUH1] = {
> > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> > index 519b3651d1d9..028f38d45892 100644
> > --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> > @@ -68,6 +68,7 @@ static int imx8m_blk_ctrl_power_on(struct
> > generic_pm_domain *genpd)
> >         struct imx8m_blk_ctrl *bc = domain->bc;
> >         int ret;
> >
> > +printk("%s %s\n", __func__, genpd->name);
> >         /* make sure bus domain is awake */
> >         ret = pm_runtime_get_sync(bc->bus_power_dev);
> >         if (ret < 0) {
> > @@ -119,6 +120,7 @@ static int imx8m_blk_ctrl_power_off(struct
> > generic_pm_domain *genpd)
> >         const struct imx8m_blk_ctrl_domain_data *data = domain->data;
> >         struct imx8m_blk_ctrl *bc = domain->bc;
> >
> > +printk("%s %s\n", __func__, genpd->name);
> >         /* put devices into reset and disable clocks */
> >         regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> >         regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
> >
> > Tim

Re: [RFC V2 0/2] arm64: imx8mm: Enable Hantro VPUs

[Adam Ford]

On Wed, Dec 1, 2021 at 11:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Tim,
>
> Am Mittwoch, dem 01.12.2021 um 09:23 -0800 schrieb Tim Harvey:
> > On Tue, Nov 30, 2021 at 5:33 PM Adam Ford <aford173@gmail.com> wrote:
> > >
> > > The i.MX8M has two Hantro video decoders, called G1 and G2 which appear
> > > to be related to the video decoders used on the i.MX8MQ, but because of
> > > how the Mini handles the power domains, the VPU driver does not need to
> > > handle all the functions, nor does it support the post-processor,
> > > so a new compatible flag is required.
> > >
> > > With the suggestion from Hans Verkuil, I was able to get the G2 splat to go away
> > > with changes to FORCE_MAX_ZONEORDER, but I found I could also set cma=512M, however
> > > it's unclear to me if that's an acceptable alternative.
> > >
> > > At the suggestion of Ezequiel Garcia and Nicolas Dufresne I have some
> > > results from Fluster. However, the G2 VPU appears to fail most tests.
> > >
> > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > > Ran 90/135 tests successfully               in 76.431 secs
> > >
> > >  ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > > Ran 55/61 tests successfully               in 21.454 secs
> > >
> > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > > Ran 0/303 tests successfully               in 20.016 secs
> > >
> > > Each day seems to show more and more G2 submissions, and gstreamer seems to be
> > > still working on the VP9, so I am not sure if I should drop G2 as well.
> > >
> > >
> > > Adam Ford (2):
> > >   media: hantro: Add support for i.MX8M Mini
> > >   arm64: dts: imx8mm: Enable VPU-G1 and VPU-G2
> > >
> > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi   | 41 +++++++++++++++
> > >  drivers/staging/media/hantro/hantro_drv.c   |  2 +
> > >  drivers/staging/media/hantro/hantro_hw.h    |  2 +
> > >  drivers/staging/media/hantro/imx8m_vpu_hw.c | 57 +++++++++++++++++++++
> > >  4 files changed, 102 insertions(+)
> > >
> >
> > Adam,
> >
> > That's for the patches!
> >
> > I tested just this series on top of v5.16-rc3 on an
> > imx8mm-venice-gw73xx-0x and found that if I loop fluster I can end up
> > getting a hang within 10 to 15 mins or so when imx8m_blk_ctrl_power_on
> > is called for VPUMIX pd :
> > while [ 1 ]; do uptime; ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0; done
> > ...
> > [  618.838436] imx-pgc imx-pgc-domain.6: failed to command PGC
> > [  618.844407] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> >
> > I added prints in imx_pgc_power_{up,down} and
> > imx8m_blk_ctrl_power_{on,off} to get some more context
> > ...
> > Ran 55/61 tests successfully               in 8.685 secs
> >  17:16:34 up 17 min,  0 users,  load average: 3.97, 2.11, 0.93
> > ********************************************************************************
> > ********************
> > Running test suite VP8-TEST-VECTORS with decoder GStreamer-VP8-V4L2SL-Gst1.0
> > Using 4 parallel job(s)
> > ********************************************************************************
> > ********************
> >
> > [TEST SUITE      ] (DECODER                    ) TEST VECTOR               ... R
> > ESULT
> > ----------------------------------------------------------------------
> > [ 1023.114806] imx8m_blk_ctrl_power_on vpublk-g1
> > [ 1023.119669] imx_pgc_power_up vpumix
> > [ 1023.124307] imx-pgc imx-pgc-domain.6: failed to command PGC
> > [ 1023.130006] imx8m-blk-ctrl 38330000.blk-ctrl: failed to power up bus domain
> >
> > While this wouldn't be an issue with this series it does indicate we
> > still have something racy in blk-ctrl. Can you reproduce this (and if
> > not what kernel are you based on)? Perhaps you or Lucas have some
> > ideas?

i have not seen an issue with my implementation, but used
media-staging [1] for the last attempt to try to get as much of the
latest hantro driver integration, although the VP9 stuff isn't quite
ready yet on the g2-VPU

[1] - https://git.linuxtv.org/media_stage.git/log/drivers/staging/media/hantro

> >
> Did you have "[PATCH] soc: imx: gpcv2: Synchronously suspend MIX
> domains" applied when running those tests? It has only recently been
> picked up by Shawn and may have an influence on the bus domain
> behavior.

I didn't know about this one either, so I'll try it.

When I was attempting to read registers from the H1 vpu, I had to set
"keep_clocks = true" for the H1 power domain or it would hang.  If the
patch Lucas suggests doesn't work, you could try keeing the G1 or G2
clocks on.  I believe it's already set for the vpumix, but the G1, G2
and H1 clocks are not touched by the vpumix, just the
IMX8MM_CLK_VPU_DEC_ROOT.

>
> Regards,
> Lucas
>