mbox series

[V2,00/10] media: hantro: imx8mq/imx8mm: Let VPU decoders get controlled by vpu-blk-ctrl

Message ID 20211216111256.2362683-1-aford173@gmail.com (mailing list archive)
Headers show
Series media: hantro: imx8mq/imx8mm: Let VPU decoders get controlled by vpu-blk-ctrl | expand

Message

Adam Ford Dec. 16, 2021, 11:12 a.m. UTC 
Both the i.MX8MQ and i.MX8MM have G1 and G2 decoders.
The two decoders are similar, but the imx8mm lacks the
post-processor, so they will have distinct compatible flags.

From what I can tell, the G2 decoder wasn't working, so splitting
the i.MX8MQ VPU into G1 and G2 makes it easier to control them 
independently since the TRM of both the i.MX8MQ and
i.MX8MM list them as distinct IP blocks. This also allowed G2 to
become available.

With them being split, the power-domain can shift to the
vpu-blk-ctrl which is available on both i.MX8MQ and i.MX8MM,
but some of bits are different, so they'll have separate bindings.

Lastly, with the G1 and G2 operational, enable the i.MX8MM.
On the i.MX8MM, the clock speed of 600MHz was chosen to match
the default of the kernel repo from NXP and can be overwritten
by board files for anyone who under/over volts the power rail.

There seems to be some disagreement between the TRM and the Datasheet
for the imx8mq as to whether the speed should be 300MHz (TRM) or
600MHz (datasheet), so feedback from NXP would be very much
appreciated.

The repo used as the starting point was:
git://linuxtv.org/hverkuil/media_tree.git for-v5.17e

Fluster was run on both i.MX8MM and i.MX8MQ

At 600 MHz, the i.MX8MM had the following:

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

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

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

The i.MX8MQ had the following:

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

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

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

V2:  Remove references to legacy dt-binding from YAML, but keep
     it in the driver so older device trees can still be used.
     Fix typos in YAML
     Remove reg-names, interrupt-names, and clock-names from YAML,
     since each node will only have one of each, they're not necessary
     Add Fluster scores to cover letter for i.MX8MQ

Adam Ford (7):
  dt-bindings: media: nxp,imx8mq-vpu: Split G1 and G2 nodes
  media: hantro: Allow i.MX8MQ G1 and G2 to run independently
  arm64: dts: imx8mq: Enable both G1 and G2 VPU's with vpu-blk-ctrl
  arm64: dts: imx8mm: Fix VPU Hanging
  dt-bindings: media: nxp,imx8mq-vpu: Add support for G1 and G2 on
    imx8mm
  media: hantro: Add support for i.MX8MM
  arm64: dts: imx8mm: Enable Hantro G1 and G2 video decoders

Lucas Stach (3):
  dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains
  dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl
  soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl

 .../bindings/media/nxp,imx8mq-vpu.yaml        | 93 +++++++++++--------
 .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml      | 71 ++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 23 ++++-
 arch/arm64/boot/dts/freescale/imx8mq.dtsi     | 63 ++++++++-----
 drivers/soc/imx/imx8m-blk-ctrl.c              | 68 +++++++++++++-
 drivers/staging/media/hantro/hantro_drv.c     |  3 +
 drivers/staging/media/hantro/hantro_hw.h      |  3 +
 drivers/staging/media/hantro/imx8m_vpu_hw.c   | 75 ++++++++++++---
 include/dt-bindings/power/imx8mq-power.h      |  3 +
 9 files changed, 324 insertions(+), 78 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml


base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c

Comments

Ezequiel Garcia Dec. 16, 2021, 12:35 p.m. UTC | #1 
Hi Adam,

The series looks really great.

On Thu, Dec 16, 2021 at 05:12:45AM -0600, Adam Ford wrote:
> Both the i.MX8MQ and i.MX8MM have G1 and G2 decoders.
> The two decoders are similar, but the imx8mm lacks the
> post-processor, so they will have distinct compatible flags.
> 
> From what I can tell, the G2 decoder wasn't working, so splitting
> the i.MX8MQ VPU into G1 and G2 makes it easier to control them 
> independently since the TRM of both the i.MX8MQ and
> i.MX8MM list them as distinct IP blocks. This also allowed G2 to
> become available.
> 
> With them being split, the power-domain can shift to the
> vpu-blk-ctrl which is available on both i.MX8MQ and i.MX8MM,
> but some of bits are different, so they'll have separate bindings.
> 
> Lastly, with the G1 and G2 operational, enable the i.MX8MM.
> On the i.MX8MM, the clock speed of 600MHz was chosen to match
> the default of the kernel repo from NXP and can be overwritten
> by board files for anyone who under/over volts the power rail.
> 
> There seems to be some disagreement between the TRM and the Datasheet
> for the imx8mq as to whether the speed should be 300MHz (TRM) or
> 600MHz (datasheet), so feedback from NXP would be very much
> appreciated.
> 
> The repo used as the starting point was:
> git://linuxtv.org/hverkuil/media_tree.git for-v5.17e
> 

I believe you should be able to rebase on top of
media_tree master. As far as I can see, it contains the VP9
support you need.

Also, please cherry-pick the following fix from Benjamin

https://patchwork.linuxtv.org/project/linux-media/patch/20211208164418.848790-1-benjamin.gaignard@collabora.com/

This is queued and should land on the master branch very soon.

> Fluster was run on both i.MX8MM and i.MX8MQ
> 
> At 600 MHz, the i.MX8MM had the following:
> 
> ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> Ran 55/61 tests successfully               in 8.299 secs
> 
> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> Ran 90/135 tests successfully               in 71.200 secs
> 
> ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> Ran 139/303 tests successfully               in 218.079 secs

I imagine the reason H264 and VP9 tests take so long
is some pixelformat conversion somewhere. It would be great
if Fluster could have test vectors ready in the pixelformat
the hardware produces :-)

> 
> The i.MX8MQ had the following:
> 
> ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> Ran 55/61 tests successfully               in 7.732 secs
> 
> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> Ran 90/135 tests successfully               in 58.558 secs
> 
> ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> Ran 144/303 tests successfully               in 271.373 secs
> 

... in any case, the fact that fluster is passing is already
telling us the driver is in good shape. How many jobs is the above
running in parallel?

If you want to do some other tests, you can build a gstreamer
pipeline, with sync=false, and decode a few 1080p video, e.g.
https://jell.yfish.us/.

Something like gst-launch-1.0 filesrc ! decodebin ! fakevideosink, or
so.

Then, you can run the pipeline in parallel as many times as you want:

gst-launch-1.0 filesrc ! decodebin ! fakevideosink filesrc ! decodebin ! fakevideosink filesrc ! decodebin ! fakevideosink ...

(GStreamer lets you concatenate src ! sink src ! sink, in the same
gst-launch-1.0 invocation).

> V2:  Remove references to legacy dt-binding from YAML, but keep
>      it in the driver so older device trees can still be used.
>      Fix typos in YAML
>      Remove reg-names, interrupt-names, and clock-names from YAML,
>      since each node will only have one of each, they're not necessary
>      Add Fluster scores to cover letter for i.MX8MQ
> 

Looks great.

Thanks,
Ezequiel

> Adam Ford (7):
>   dt-bindings: media: nxp,imx8mq-vpu: Split G1 and G2 nodes
>   media: hantro: Allow i.MX8MQ G1 and G2 to run independently
>   arm64: dts: imx8mq: Enable both G1 and G2 VPU's with vpu-blk-ctrl
>   arm64: dts: imx8mm: Fix VPU Hanging
>   dt-bindings: media: nxp,imx8mq-vpu: Add support for G1 and G2 on
>     imx8mm
>   media: hantro: Add support for i.MX8MM
>   arm64: dts: imx8mm: Enable Hantro G1 and G2 video decoders
> 
> Lucas Stach (3):
>   dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains
>   dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl
>   soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl
> 
>  .../bindings/media/nxp,imx8mq-vpu.yaml        | 93 +++++++++++--------
>  .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml      | 71 ++++++++++++++
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 23 ++++-
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi     | 63 ++++++++-----
>  drivers/soc/imx/imx8m-blk-ctrl.c              | 68 +++++++++++++-
>  drivers/staging/media/hantro/hantro_drv.c     |  3 +
>  drivers/staging/media/hantro/hantro_hw.h      |  3 +
>  drivers/staging/media/hantro/imx8m_vpu_hw.c   | 75 ++++++++++++---
>  include/dt-bindings/power/imx8mq-power.h      |  3 +
>  9 files changed, 324 insertions(+), 78 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml
> 
> 
> base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c
> -- 
> 2.32.0
>
Adam Ford Dec. 16, 2021, 1:09 p.m. UTC | #2 
On Thu, Dec 16, 2021 at 6:35 AM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> Hi Adam,
>
> The series looks really great.
>
> On Thu, Dec 16, 2021 at 05:12:45AM -0600, Adam Ford wrote:
> > Both the i.MX8MQ and i.MX8MM have G1 and G2 decoders.
> > The two decoders are similar, but the imx8mm lacks the
> > post-processor, so they will have distinct compatible flags.
> >
> > From what I can tell, the G2 decoder wasn't working, so splitting
> > the i.MX8MQ VPU into G1 and G2 makes it easier to control them
> > independently since the TRM of both the i.MX8MQ and
> > i.MX8MM list them as distinct IP blocks. This also allowed G2 to
> > become available.
> >
> > With them being split, the power-domain can shift to the
> > vpu-blk-ctrl which is available on both i.MX8MQ and i.MX8MM,
> > but some of bits are different, so they'll have separate bindings.
> >
> > Lastly, with the G1 and G2 operational, enable the i.MX8MM.
> > On the i.MX8MM, the clock speed of 600MHz was chosen to match
> > the default of the kernel repo from NXP and can be overwritten
> > by board files for anyone who under/over volts the power rail.
> >
> > There seems to be some disagreement between the TRM and the Datasheet
> > for the imx8mq as to whether the speed should be 300MHz (TRM) or
> > 600MHz (datasheet), so feedback from NXP would be very much
> > appreciated.
> >
> > The repo used as the starting point was:
> > git://linuxtv.org/hverkuil/media_tree.git for-v5.17e
> >
>
> I believe you should be able to rebase on top of
> media_tree master. As far as I can see, it contains the VP9
> support you need.
>
> Also, please cherry-pick the following fix from Benjamin
>
> https://patchwork.linuxtv.org/project/linux-media/patch/20211208164418.848790-1-benjamin.gaignard@collabora.com/
>
> This is queued and should land on the master branch very soon.
>
> > Fluster was run on both i.MX8MM and i.MX8MQ
> >
> > At 600 MHz, the i.MX8MM had the following:
> >
> > ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > Ran 55/61 tests successfully               in 8.299 secs
> >
> > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > Ran 90/135 tests successfully               in 71.200 secs
> >
> > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > Ran 139/303 tests successfully               in 218.079 secs
>
> I imagine the reason H264 and VP9 tests take so long
> is some pixelformat conversion somewhere. It would be great
> if Fluster could have test vectors ready in the pixelformat
> the hardware produces :-)
>
> >
> > The i.MX8MQ had the following:
> >
> > ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > Ran 55/61 tests successfully               in 7.732 secs
> >
> > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > Ran 90/135 tests successfully               in 58.558 secs
> >
> > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> > Ran 144/303 tests successfully               in 271.373 secs
> >
>
> ... in any case, the fact that fluster is passing is already
> telling us the driver is in good shape. How many jobs is the above
> running in parallel?
>
> If you want to do some other tests, you can build a gstreamer
> pipeline, with sync=false, and decode a few 1080p video, e.g.
> https://jell.yfish.us/.
>
> Something like gst-launch-1.0 filesrc ! decodebin ! fakevideosink, or
> so.
>
> Then, you can run the pipeline in parallel as many times as you want:
>
> gst-launch-1.0 filesrc ! decodebin ! fakevideosink filesrc ! decodebin ! fakevideosink filesrc ! decodebin ! fakevideosink ...
>
> (GStreamer lets you concatenate src ! sink src ! sink, in the same
> gst-launch-1.0 invocation).
>
> > V2:  Remove references to legacy dt-binding from YAML, but keep
> >      it in the driver so older device trees can still be used.
> >      Fix typos in YAML
> >      Remove reg-names, interrupt-names, and clock-names from YAML,
> >      since each node will only have one of each, they're not necessary
> >      Add Fluster scores to cover letter for i.MX8MQ
> >
>
> Looks great.

Any change you could give your acked-by or reviewed-by to the patches?
 Even if they need some work, I can carry those along to the next
revision when I do the work you request.

adam



>
> Thanks,
> Ezequiel
>
> > Adam Ford (7):
> >   dt-bindings: media: nxp,imx8mq-vpu: Split G1 and G2 nodes
> >   media: hantro: Allow i.MX8MQ G1 and G2 to run independently
> >   arm64: dts: imx8mq: Enable both G1 and G2 VPU's with vpu-blk-ctrl
> >   arm64: dts: imx8mm: Fix VPU Hanging
> >   dt-bindings: media: nxp,imx8mq-vpu: Add support for G1 and G2 on
> >     imx8mm
> >   media: hantro: Add support for i.MX8MM
> >   arm64: dts: imx8mm: Enable Hantro G1 and G2 video decoders
> >
> > Lucas Stach (3):
> >   dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains
> >   dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl
> >   soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl
> >
> >  .../bindings/media/nxp,imx8mq-vpu.yaml        | 93 +++++++++++--------
> >  .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml      | 71 ++++++++++++++
> >  arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 23 ++++-
> >  arch/arm64/boot/dts/freescale/imx8mq.dtsi     | 63 ++++++++-----
> >  drivers/soc/imx/imx8m-blk-ctrl.c              | 68 +++++++++++++-
> >  drivers/staging/media/hantro/hantro_drv.c     |  3 +
> >  drivers/staging/media/hantro/hantro_hw.h      |  3 +
> >  drivers/staging/media/hantro/imx8m_vpu_hw.c   | 75 ++++++++++++---
> >  include/dt-bindings/power/imx8mq-power.h      |  3 +
> >  9 files changed, 324 insertions(+), 78 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml
> >
> >
> > base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c
> > --
> > 2.32.0
> >
Benjamin Gaignard Dec. 16, 2021, 2:58 p.m. UTC | #3 
Le 16/12/2021 à 12:12, Adam Ford a écrit :
> Both the i.MX8MQ and i.MX8MM have G1 and G2 decoders.
> The two decoders are similar, but the imx8mm lacks the
> post-processor, so they will have distinct compatible flags.
>
>  From what I can tell, the G2 decoder wasn't working, so splitting
> the i.MX8MQ VPU into G1 and G2 makes it easier to control them
> independently since the TRM of both the i.MX8MQ and
> i.MX8MM list them as distinct IP blocks. This also allowed G2 to
> become available.
>
> With them being split, the power-domain can shift to the
> vpu-blk-ctrl which is available on both i.MX8MQ and i.MX8MM,
> but some of bits are different, so they'll have separate bindings.
>
> Lastly, with the G1 and G2 operational, enable the i.MX8MM.
> On the i.MX8MM, the clock speed of 600MHz was chosen to match
> the default of the kernel repo from NXP and can be overwritten
> by board files for anyone who under/over volts the power rail.
>
> There seems to be some disagreement between the TRM and the Datasheet
> for the imx8mq as to whether the speed should be 300MHz (TRM) or
> 600MHz (datasheet), so feedback from NXP would be very much
> appreciated.
>
> The repo used as the starting point was:
> git://linuxtv.org/hverkuil/media_tree.git for-v5.17e
>
> Fluster was run on both i.MX8MM and i.MX8MQ
>
> At 600 MHz, the i.MX8MM had the following:
>
> ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> Ran 55/61 tests successfully               in 8.299 secs
>
> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> Ran 90/135 tests successfully               in 71.200 secs
>
> ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> Ran 139/303 tests successfully               in 218.079 secs
>
> The i.MX8MQ had the following:
>
> ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> Ran 55/61 tests successfully               in 7.732 secs
>
> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> Ran 90/135 tests successfully               in 58.558 secs
>
> ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0
> Ran 144/303 tests successfully               in 271.373 secs

I have tested this series on top of media_stage/master branch
with IMX6MQ:

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

./fluster.py run -d GStreamer-H.264-V4L2SL-Gst1.0
Ran 89/135 tests successfully               in 131.632 secs

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

./fluster.py run -d GStreamer-H.265-V4L2SL-Gst1.0 -j 1
Ran 77/147 tests successfully               in 1047.211 secs

For this series:

Tested-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

Thanks to fix this problem.
Benjamin

>
> V2:  Remove references to legacy dt-binding from YAML, but keep
>       it in the driver so older device trees can still be used.
>       Fix typos in YAML
>       Remove reg-names, interrupt-names, and clock-names from YAML,
>       since each node will only have one of each, they're not necessary
>       Add Fluster scores to cover letter for i.MX8MQ
>
> Adam Ford (7):
>    dt-bindings: media: nxp,imx8mq-vpu: Split G1 and G2 nodes
>    media: hantro: Allow i.MX8MQ G1 and G2 to run independently
>    arm64: dts: imx8mq: Enable both G1 and G2 VPU's with vpu-blk-ctrl
>    arm64: dts: imx8mm: Fix VPU Hanging
>    dt-bindings: media: nxp,imx8mq-vpu: Add support for G1 and G2 on
>      imx8mm
>    media: hantro: Add support for i.MX8MM
>    arm64: dts: imx8mm: Enable Hantro G1 and G2 video decoders
>
> Lucas Stach (3):
>    dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains
>    dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl
>    soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl
>
>   .../bindings/media/nxp,imx8mq-vpu.yaml        | 93 +++++++++++--------
>   .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml      | 71 ++++++++++++++
>   arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 23 ++++-
>   arch/arm64/boot/dts/freescale/imx8mq.dtsi     | 63 ++++++++-----
>   drivers/soc/imx/imx8m-blk-ctrl.c              | 68 +++++++++++++-
>   drivers/staging/media/hantro/hantro_drv.c     |  3 +
>   drivers/staging/media/hantro/hantro_hw.h      |  3 +
>   drivers/staging/media/hantro/imx8m_vpu_hw.c   | 75 ++++++++++++---
>   include/dt-bindings/power/imx8mq-power.h      |  3 +
>   9 files changed, 324 insertions(+), 78 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml
>
>
> base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c