2021-12-16 11:13:19

by Adam Ford

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

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
--
2.32.0



2021-12-16 11:13:28

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 01/10] dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains

From: Lucas Stach <[email protected]>

This adds the defines for the power domains provided by the VPU
blk-ctrl on the i.MX8MQ.

Signed-off-by: Lucas Stach <[email protected]>

diff --git a/include/dt-bindings/power/imx8mq-power.h b/include/dt-bindings/power/imx8mq-power.h
index 8a513bd9166e..9f7d0f1e7c32 100644
--- a/include/dt-bindings/power/imx8mq-power.h
+++ b/include/dt-bindings/power/imx8mq-power.h
@@ -18,4 +18,7 @@
#define IMX8M_POWER_DOMAIN_MIPI_CSI2 9
#define IMX8M_POWER_DOMAIN_PCIE2 10

+#define IMX8MQ_VPUBLK_PD_G1 0
+#define IMX8MQ_VPUBLK_PD_G2 1
+
#endif
--
2.32.0


2021-12-16 11:13:29

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 02/10] dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl

From: Lucas Stach <[email protected]>

This adds the DT binding for the i.MX8MQ VPU blk-ctrl.

Signed-off-by: Lucas Stach <[email protected]>
Signed-off-by: Adam Ford <[email protected]>

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml
new file mode 100644
index 000000000000..7263ebedf09f
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8MQ VPU blk-ctrl
+
+maintainers:
+ - Lucas Stach <[email protected]>
+
+description:
+ The i.MX8MQ VPU blk-ctrl is a top-level peripheral providing access to
+ the NoC and ensuring proper power sequencing of the VPU peripherals
+ located in the VPU domain of the SoC.
+
+properties:
+ compatible:
+ items:
+ - const: fsl,imx8mq-vpu-blk-ctrl
+
+ reg:
+ maxItems: 1
+
+ '#power-domain-cells':
+ const: 1
+
+ power-domains:
+ minItems: 3
+ maxItems: 3
+
+ power-domain-names:
+ items:
+ - const: bus
+ - const: g1
+ - const: g2
+
+ clocks:
+ minItems: 2
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: g1
+ - const: g2
+
+required:
+ - compatible
+ - reg
+ - power-domains
+ - power-domain-names
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx8mq-clock.h>
+ #include <dt-bindings/power/imx8mq-power.h>
+
+ vpu_blk_ctrl: blk-ctrl@38320000 {
+ compatible = "fsl,imx8mq-vpu-blk-ctrl";
+ reg = <0x38320000 0x100>;
+ power-domains = <&pgc_vpu>, <&pgc_vpu>, <&pgc_vpu>;
+ power-domain-names = "bus", "g1", "g2";
+ clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
+ <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
+ clock-names = "g1", "g2";
+ #power-domain-cells = <1>;
+ };
--
2.32.0


2021-12-16 11:13:33

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 03/10] soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl

From: Lucas Stach <[email protected]>

This adds the necessary bits to drive the VPU blk-ctrl on the i.MX8MQ, to
avoid putting more of this functionality into the decoder driver.

Signed-off-by: Lucas Stach <[email protected]>
Signed-off-by: Adam Ford <[email protected]>

diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
index 519b3651d1d9..a7c12e8fa89b 100644
--- a/drivers/soc/imx/imx8m-blk-ctrl.c
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -14,6 +14,7 @@
#include <linux/clk.h>

#include <dt-bindings/power/imx8mm-power.h>
+#include <dt-bindings/power/imx8mq-power.h>

#define BLK_SFT_RSTN 0x0
#define BLK_CLK_EN 0x4
@@ -498,6 +499,68 @@ static const struct imx8m_blk_ctrl_data imx8mm_disp_blk_ctl_dev_data = {
.num_domains = ARRAY_SIZE(imx8mm_disp_blk_ctl_domain_data),
};

+static int imx8mq_vpu_power_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
+ power_nb);
+
+ if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
+ return NOTIFY_OK;
+
+ /*
+ * The ADB in the VPUMIX domain has no separate reset and clock
+ * enable bits, but is ungated and reset together with the VPUs. The
+ * reset and clock enable inputs to the ADB is a logical OR of the
+ * VPU bits. In order to set the G2 fuse bits, the G2 clock must
+ * also be enabled.
+ */
+ regmap_set_bits(bc->regmap, BLK_SFT_RSTN, BIT(0) | BIT(1));
+ regmap_set_bits(bc->regmap, BLK_CLK_EN, BIT(0) | BIT(1));
+
+ if (action == GENPD_NOTIFY_ON) {
+ /*
+ * On power up we have no software backchannel to the GPC to
+ * wait for the ADB handshake to happen, so we just delay for a
+ * bit. On power down the GPC driver waits for the handshake.
+ */
+ udelay(5);
+
+ /* set "fuse" bits to enable the VPUs */
+ regmap_set_bits(bc->regmap, 0x8, 0xffffffff);
+ regmap_set_bits(bc->regmap, 0xc, 0xffffffff);
+ regmap_set_bits(bc->regmap, 0x10, 0xffffffff);
+ }
+
+ return NOTIFY_OK;
+}
+
+static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[] = {
+ [IMX8MQ_VPUBLK_PD_G1] = {
+ .name = "vpublk-g1",
+ .clk_names = (const char *[]){ "g1", },
+ .num_clks = 1,
+ .gpc_name = "g1",
+ .rst_mask = BIT(1),
+ .clk_mask = BIT(1),
+ },
+ [IMX8MQ_VPUBLK_PD_G2] = {
+ .name = "vpublk-g2",
+ .clk_names = (const char *[]){ "g2", },
+ .num_clks = 1,
+ .gpc_name = "g2",
+ .rst_mask = BIT(0),
+ .clk_mask = BIT(0),
+ },
+};
+
+static const struct imx8m_blk_ctrl_data imx8mq_vpu_blk_ctl_dev_data = {
+ .max_reg = 0x14,
+ .power_notifier_fn = imx8mq_vpu_power_notifier,
+ .domains = imx8mq_vpu_blk_ctl_domain_data,
+ .num_domains = ARRAY_SIZE(imx8mq_vpu_blk_ctl_domain_data),
+};
+
static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
{
.compatible = "fsl,imx8mm-vpu-blk-ctrl",
@@ -505,7 +568,10 @@ static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
}, {
.compatible = "fsl,imx8mm-disp-blk-ctrl",
.data = &imx8mm_disp_blk_ctl_dev_data
- } ,{
+ }, {
+ .compatible = "fsl,imx8mq-vpu-blk-ctrl",
+ .data = &imx8mq_vpu_blk_ctl_dev_data
+ }, {
/* Sentinel */
}
};
--
2.32.0


2021-12-16 11:13:35

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 05/10] media: hantro: Allow i.MX8MQ G1 and G2 to run independently

The VPU in the i.MX8MQ is really the combination of Hantro G1 and
Hantro G2. With the updated vpu-blk-ctrl, the power domains system
can enable and disable them separately as well as pull them out of
reset. This simplifies the code and lets them run independently
while still retaining backwards compatibility with older device
trees for those using G1.

Signed-off-by: Adam Ford <[email protected]>

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index ab2467998d29..e7afda388ee5 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -609,6 +609,7 @@ static const struct of_device_id of_hantro_match[] = {
#endif
#ifdef CONFIG_VIDEO_HANTRO_IMX8M
{ .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
+ { .compatible = "nxp,imx8mq-vpu-g1", .data = &imx8mq_vpu_g1_variant },
{ .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
#endif
#ifdef CONFIG_VIDEO_HANTRO_SAMA5D4
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index cff817ca8d22..96b14b43a4af 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -299,6 +299,7 @@ enum hantro_enc_fmt {
ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
};

+extern const struct hantro_variant imx8mq_vpu_g1_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 1a43f6fceef9..4925f2a07d4c 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -223,13 +223,6 @@ static void imx8m_vpu_g1_reset(struct hantro_ctx *ctx)
imx8m_soft_reset(vpu, RESET_G1);
}

-static void imx8m_vpu_g2_reset(struct hantro_ctx *ctx)
-{
- struct hantro_dev *vpu = ctx->dev;
-
- imx8m_soft_reset(vpu, RESET_G2);
-}
-
/*
* Supported codec ops.
*/
@@ -255,17 +248,33 @@ static const struct hantro_codec_ops imx8mq_vpu_codec_ops[] = {
},
};

+static const struct hantro_codec_ops imx8mq_vpu_g1_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 imx8mq_vpu_g2_codec_ops[] = {
[HANTRO_MODE_HEVC_DEC] = {
.run = hantro_g2_hevc_dec_run,
- .reset = imx8m_vpu_g2_reset,
.init = hantro_hevc_dec_init,
.exit = hantro_hevc_dec_exit,
},
[HANTRO_MODE_VP9_DEC] = {
.run = hantro_g2_vp9_dec_run,
.done = hantro_g2_vp9_dec_done,
- .reset = imx8m_vpu_g2_reset,
.init = hantro_vp9_dec_init,
.exit = hantro_vp9_dec_exit,
},
@@ -285,6 +294,10 @@ 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 imx8mq_g1_clk_names[] = { "g1" };
+static const char * const imx8mq_g1_reg_names[] = { "g1" };
+static const char * const imx8mq_g2_clk_names[] = { "g2" };
+static const char * const imx8mq_g2_reg_names[] = { "g2" };

const struct hantro_variant imx8mq_vpu_variant = {
.dec_fmts = imx8m_vpu_dec_fmts,
@@ -305,6 +318,21 @@ const struct hantro_variant imx8mq_vpu_variant = {
.num_regs = ARRAY_SIZE(imx8mq_reg_names)
};

+const struct hantro_variant imx8mq_vpu_g1_variant = {
+ .dec_fmts = imx8m_vpu_dec_fmts,
+ .num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
+ .postproc_fmts = imx8m_vpu_postproc_fmts,
+ .num_postproc_fmts = ARRAY_SIZE(imx8m_vpu_postproc_fmts),
+ .postproc_ops = &hantro_g1_postproc_ops,
+ .codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
+ HANTRO_H264_DECODER,
+ .codec_ops = imx8mq_vpu_g1_codec_ops,
+ .irqs = imx8mq_irqs,
+ .num_irqs = ARRAY_SIZE(imx8mq_irqs),
+ .clk_names = imx8mq_g1_clk_names,
+ .num_clocks = ARRAY_SIZE(imx8mq_g1_clk_names),
+};
+
const struct hantro_variant imx8mq_vpu_g2_variant = {
.dec_offset = 0x0,
.dec_fmts = imx8m_vpu_g2_dec_fmts,
@@ -314,10 +342,8 @@ const struct hantro_variant imx8mq_vpu_g2_variant = {
.postproc_ops = &hantro_g2_postproc_ops,
.codec = HANTRO_HEVC_DECODER | HANTRO_VP9_DECODER,
.codec_ops = imx8mq_vpu_g2_codec_ops,
- .init = imx8mq_vpu_hw_init,
- .runtime_resume = imx8mq_runtime_resume,
.irqs = imx8mq_g2_irqs,
.num_irqs = ARRAY_SIZE(imx8mq_g2_irqs),
- .clk_names = imx8mq_clk_names,
- .num_clocks = ARRAY_SIZE(imx8mq_clk_names),
+ .clk_names = imx8mq_g2_clk_names,
+ .num_clocks = ARRAY_SIZE(imx8mq_g2_clk_names),
};
--
2.32.0


2021-12-16 11:13:38

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 04/10] dt-bindings: media: nxp,imx8mq-vpu: Split G1 and G2 nodes

The G1 and G2 are separate decoder blocks that are enabled by the
vpu-blk-ctrl power-domain controller, which now has a proper driver.
Update the bindings to support separate nodes for the G1 and G2
decoders with vpu-blk-ctrl power-domain support.

Signed-off-by: Adam Ford <[email protected]>

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
index 762be3f96ce9..c1e157251de7 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
@@ -15,33 +15,20 @@ description:

properties:
compatible:
- const: nxp,imx8mq-vpu
+ oneOf:
+ - const: nxp,imx8mq-vpu
+ deprecated: true
+ - const: nxp,imx8mq-vpu-g1
+ - const: nxp,imx8mq-vpu-g2

reg:
- maxItems: 3
-
- reg-names:
- items:
- - const: g1
- - const: g2
- - const: ctrl
+ maxItems: 1

interrupts:
- maxItems: 2
-
- interrupt-names:
- items:
- - const: g1
- - const: g2
+ maxItems: 1

clocks:
- maxItems: 3
-
- clock-names:
- items:
- - const: g1
- - const: g2
- - const: bus
+ maxItems: 1

power-domains:
maxItems: 1
@@ -49,31 +36,33 @@ properties:
required:
- compatible
- reg
- - reg-names
- interrupts
- - interrupt-names
- clocks
- - clock-names

additionalProperties: false

examples:
- |
#include <dt-bindings/clock/imx8mq-clock.h>
+ #include <dt-bindings/power/imx8mq-power.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ vpu_g1: video-codec@38300000 {
+ compatible = "nxp,imx8mq-vpu-g1";
+ reg = <0x38300000 0x10000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>;
+ power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>;
+ };
+ - |
+ #include <dt-bindings/clock/imx8mq-clock.h>
+ #include <dt-bindings/power/imx8mq-power.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>

- vpu: video-codec@38300000 {
- compatible = "nxp,imx8mq-vpu";
- reg = <0x38300000 0x10000>,
- <0x38310000 0x10000>,
- <0x38320000 0x10000>;
- reg-names = "g1", "g2", "ctrl";
- interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "g1", "g2";
- clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
- <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
- <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
- clock-names = "g1", "g2", "bus";
- power-domains = <&pgc_vpu>;
+ vpu_g2: video-codec@38300000 {
+ compatible = "nxp,imx8mq-vpu-g2";
+ reg = <0x38310000 0x10000>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
+ power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>;
};
--
2.32.0


2021-12-16 11:13:41

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 07/10] arm64: dts: imx8mm: Fix VPU Hanging

The vpumix power domain has a reset assigned to it, however
when used, it causes a system hang. Testing has shown that
it does not appear to be needed anywhere.

Fixes: d39d4bb15310 ("arm64: dts: imx8mm: add GPC node")
Signed-off-by: Adam Ford <[email protected]>

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c2f3f118f82e..f13d31ebfcbd 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -681,7 +681,6 @@ pgc_vpumix: power-domain@6 {
clocks = <&clk IMX8MM_CLK_VPU_DEC_ROOT>;
assigned-clocks = <&clk IMX8MM_CLK_VPU_BUS>;
assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>;
- resets = <&src IMX8MQ_RESET_VPU_RESET>;
};

pgc_vpu_g1: power-domain@7 {
--
2.32.0


2021-12-16 11:13:47

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 08/10] dt-bindings: media: nxp,imx8mq-vpu: Add support for G1 and G2 on imx8mm

The i.MX8M mini appears to have a similar G1 and G2 decoder but the
post-procesing isn't present, so different compatible flags are requred.
Since all the other parameters are the same with imx8mq, just add
the new compatible flags to nxp,imx8mq-vpu.yaml.

Signed-off-by: Adam Ford <[email protected]>

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
index c1e157251de7..b1f24c48c73b 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
@@ -5,7 +5,7 @@
$id: "http://devicetree.org/schemas/media/nxp,imx8mq-vpu.yaml#"
$schema: "http://devicetree.org/meta-schemas/core.yaml#"

-title: Hantro G1/G2 VPU codecs implemented on i.MX8MQ SoCs
+title: Hantro G1/G2 VPU codecs implemented on i.MX8MQ/i.MX8MM SoCs

maintainers:
- Philipp Zabel <[email protected]>
@@ -20,6 +20,8 @@ properties:
deprecated: true
- const: nxp,imx8mq-vpu-g1
- const: nxp,imx8mq-vpu-g2
+ - const: nxp,imx8mm-vpu-g1
+ - const: nxp,imx8mm-vpu-g2

reg:
maxItems: 1
@@ -66,3 +68,27 @@ examples:
clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>;
};
+ - |
+ #include <dt-bindings/clock/imx8mm-clock.h>
+ #include <dt-bindings/power/imx8mm-power.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ vpu_g1: video-codec@38300000 {
+ compatible = "nxp,imx8mm-vpu-g1";
+ reg = <0x38300000 0x10000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>;
+ power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G1>;
+ };
+ - |
+ #include <dt-bindings/clock/imx8mm-clock.h>
+ #include <dt-bindings/power/imx8mm-power.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ vpu_g2: video-codec@38300000 {
+ compatible = "nxp,imx8mm-vpu-g2";
+ reg = <0x38310000 0x10000>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MM_CLK_VPU_G2_ROOT>;
+ power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G2>;
+ };
--
2.32.0


2021-12-16 11:13:50

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 06/10] arm64: dts: imx8mq: Enable both G1 and G2 VPU's with vpu-blk-ctrl

With the Hantro G1 and G2 now setup to run independently, update
the device tree to allow both to operate. This requires the
vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs
certain clock enabled to handle the gating of the G1 and G2
fuses, the clock-parents and clock-rates for the various VPU's
to be moved into the pgc_vpu because they cannot get re-parented
once enabled, and the pgc_vpu is the highest in the chain.

Signed-off-by: Adam Ford <[email protected]>

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 972766b67a15..5029c121d48b 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -711,7 +711,21 @@ pgc_gpu: power-domain@5 {
pgc_vpu: power-domain@6 {
#power-domain-cells = <0>;
reg = <IMX8M_POWER_DOMAIN_VPU>;
- clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
+ clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>,
+ <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
+ <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
+ assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>,
+ <&clk IMX8MQ_CLK_VPU_G2>,
+ <&clk IMX8MQ_CLK_VPU_BUS>,
+ <&clk IMX8MQ_VPU_PLL_BYPASS>;
+ assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>,
+ <&clk IMX8MQ_VPU_PLL_OUT>,
+ <&clk IMX8MQ_SYS1_PLL_800M>,
+ <&clk IMX8MQ_VPU_PLL>;
+ assigned-clock-rates = <600000000>,
+ <600000000>,
+ <800000000>,
+ <0>;
};

pgc_disp: power-domain@7 {
@@ -1432,30 +1446,31 @@ usb3_phy1: usb-phy@382f0040 {
status = "disabled";
};

- vpu: video-codec@38300000 {
- compatible = "nxp,imx8mq-vpu";
- reg = <0x38300000 0x10000>,
- <0x38310000 0x10000>,
- <0x38320000 0x10000>;
- reg-names = "g1", "g2", "ctrl";
- interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "g1", "g2";
+ vpu_g1: video-codec@38300000 {
+ compatible = "nxp,imx8mq-vpu-g1";
+ reg = <0x38300000 0x10000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>;
+ power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>;
+ };
+
+ vpu_g2: video-codec@38310000 {
+ compatible = "nxp,imx8mq-vpu-g2";
+ reg = <0x38310000 0x10000>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
+ power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>;
+ };
+
+ vpu_blk_ctrl: blk-ctrl@38320000 {
+ compatible = "fsl,imx8mq-vpu-blk-ctrl";
+ reg = <0x38320000 0x100>;
+ power-domains = <&pgc_vpu>, <&pgc_vpu>, <&pgc_vpu>;
+ power-domain-names = "bus", "g1", "g2";
clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
- <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
- <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
- clock-names = "g1", "g2", "bus";
- assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>,
- <&clk IMX8MQ_CLK_VPU_G2>,
- <&clk IMX8MQ_CLK_VPU_BUS>,
- <&clk IMX8MQ_VPU_PLL_BYPASS>;
- assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>,
- <&clk IMX8MQ_VPU_PLL_OUT>,
- <&clk IMX8MQ_SYS1_PLL_800M>,
- <&clk IMX8MQ_VPU_PLL>;
- assigned-clock-rates = <600000000>, <600000000>,
- <800000000>, <0>;
- power-domains = <&pgc_vpu>;
+ <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
+ clock-names = "g1", "g2";
+ #power-domain-cells = <1>;
};

pcie0: pcie@33800000 {
--
2.32.0


2021-12-16 11:13:55

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 09/10] media: hantro: Add support for i.MX8MM

The i.MX8MM has a G1 and G2 video decoder, so add support in
the driver for it with the post-processing removed.

Signed-off-by: Adam Ford <[email protected]>

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index e7afda388ee5..118c4fa3d556 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -608,6 +608,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-g1", .data = &imx8mm_vpu_g1_variant, },
+ { .compatible = "nxp,imx8mm-vpu-g2", .data = &imx8mm_vpu_g2_variant },
{ .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
{ .compatible = "nxp,imx8mq-vpu-g1", .data = &imx8mq_vpu_g1_variant },
{ .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 96b14b43a4af..6ae1aeed2e16 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -299,6 +299,8 @@ enum hantro_enc_fmt {
ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
};

+extern const struct hantro_variant imx8mm_vpu_g1_variant;
+extern const struct hantro_variant imx8mm_vpu_g2_variant;
extern const struct hantro_variant imx8mq_vpu_g1_variant;
extern const struct hantro_variant imx8mq_vpu_g2_variant;
extern const struct hantro_variant imx8mq_vpu_variant;
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index 4925f2a07d4c..dc7fc19283e8 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -347,3 +347,26 @@ const struct hantro_variant imx8mq_vpu_g2_variant = {
.clk_names = imx8mq_g2_clk_names,
.num_clocks = ARRAY_SIZE(imx8mq_g2_clk_names),
};
+
+const struct hantro_variant imx8mm_vpu_g1_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 = imx8mq_vpu_g1_codec_ops,
+ .irqs = imx8mq_irqs,
+ .num_irqs = ARRAY_SIZE(imx8mq_irqs),
+ .clk_names = imx8mq_g1_clk_names,
+ .num_clocks = ARRAY_SIZE(imx8mq_g1_clk_names),
+};
+
+const struct hantro_variant imx8mm_vpu_g2_variant = {
+ .dec_fmts = imx8m_vpu_g2_dec_fmts,
+ .num_dec_fmts = ARRAY_SIZE(imx8m_vpu_g2_dec_fmts),
+ .codec = HANTRO_HEVC_DECODER | HANTRO_VP9_DECODER,
+ .codec_ops = imx8mq_vpu_g2_codec_ops,
+ .irqs = imx8mq_g2_irqs,
+ .num_irqs = ARRAY_SIZE(imx8mq_g2_irqs),
+ .clk_names = imx8mq_g2_clk_names,
+ .num_clocks = ARRAY_SIZE(imx8mq_g2_clk_names),
+};
--
2.32.0


2021-12-16 11:14:00

by Adam Ford

[permalink] [raw]
Subject: [PATCH V2 10/10] arm64: dts: imx8mm: Enable Hantro G1 and G2 video decoders

There are two decoders on the i.MX8M Mini controlled by the
vpu-blk-ctrl. The G1 supports H264 and VP8 while the
G2 support HEVC and VP9.

Signed-off-by: Adam Ford <[email protected]>

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index f13d31ebfcbd..ad1a83c25cc9 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1196,6 +1196,22 @@ gpu_2d: gpu@38008000 {
power-domains = <&pgc_gpu>;
};

+ vpu_g1: video-codec@38300000 {
+ compatible = "nxp,imx8mm-vpu-g1";
+ reg = <0x38300000 0x10000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>;
+ 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>;
+ clocks = <&clk IMX8MM_CLK_VPU_G2_ROOT>;
+ 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>;
@@ -1206,6 +1222,12 @@ vpu_blk_ctrl: blk-ctrl@38330000 {
<&clk IMX8MM_CLK_VPU_G2_ROOT>,
<&clk IMX8MM_CLK_VPU_H1_ROOT>;
clock-names = "g1", "g2", "h1";
+ assigned-clocks = <&clk IMX8MM_CLK_VPU_G1>,
+ <&clk IMX8MM_CLK_VPU_G2>;
+ assigned-clock-parents = <&clk IMX8MM_VPU_PLL_OUT>,
+ <&clk IMX8MM_VPU_PLL_OUT>;
+ assigned-clock-rates = <600000000>,
+ <600000000>;
#power-domain-cells = <1>;
};

--
2.32.0


2021-12-16 12:09:40

by Ezequiel Garcia

[permalink] [raw]
Subject: Re: [PATCH V2 05/10] media: hantro: Allow i.MX8MQ G1 and G2 to run independently

Hi Adam,

On Thu, Dec 16, 2021 at 05:12:50AM -0600, Adam Ford wrote:
> The VPU in the i.MX8MQ is really the combination of Hantro G1 and
> Hantro G2. With the updated vpu-blk-ctrl, the power domains system
> can enable and disable them separately as well as pull them out of
> reset. This simplifies the code and lets them run independently
> while still retaining backwards compatibility with older device
> trees for those using G1.
>
> Signed-off-by: Adam Ford <[email protected]>
>
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index ab2467998d29..e7afda388ee5 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -609,6 +609,7 @@ static const struct of_device_id of_hantro_match[] = {
> #endif
> #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
> + { .compatible = "nxp,imx8mq-vpu-g1", .data = &imx8mq_vpu_g1_variant },
> { .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
> #endif
> #ifdef CONFIG_VIDEO_HANTRO_SAMA5D4
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> index cff817ca8d22..96b14b43a4af 100644
> --- a/drivers/staging/media/hantro/hantro_hw.h
> +++ b/drivers/staging/media/hantro/hantro_hw.h
> @@ -299,6 +299,7 @@ enum hantro_enc_fmt {
> ROCKCHIP_VPU_ENC_FMT_UYVY422 = 3,
> };
>
> +extern const struct hantro_variant imx8mq_vpu_g1_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 1a43f6fceef9..4925f2a07d4c 100644
> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> @@ -223,13 +223,6 @@ static void imx8m_vpu_g1_reset(struct hantro_ctx *ctx)
> imx8m_soft_reset(vpu, RESET_G1);
> }
>
> -static void imx8m_vpu_g2_reset(struct hantro_ctx *ctx)
> -{
> - struct hantro_dev *vpu = ctx->dev;
> -
> - imx8m_soft_reset(vpu, RESET_G2);
> -}
> -
> /*
> * Supported codec ops.
> */
> @@ -255,17 +248,33 @@ static const struct hantro_codec_ops imx8mq_vpu_codec_ops[] = {
> },
> };
>
> +static const struct hantro_codec_ops imx8mq_vpu_g1_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 imx8mq_vpu_g2_codec_ops[] = {
> [HANTRO_MODE_HEVC_DEC] = {
> .run = hantro_g2_hevc_dec_run,
> - .reset = imx8m_vpu_g2_reset,
> .init = hantro_hevc_dec_init,
> .exit = hantro_hevc_dec_exit,
> },
> [HANTRO_MODE_VP9_DEC] = {
> .run = hantro_g2_vp9_dec_run,
> .done = hantro_g2_vp9_dec_done,
> - .reset = imx8m_vpu_g2_reset,
> .init = hantro_vp9_dec_init,
> .exit = hantro_vp9_dec_exit,
> },
> @@ -285,6 +294,10 @@ 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 imx8mq_g1_clk_names[] = { "g1" };
> +static const char * const imx8mq_g1_reg_names[] = { "g1" };
> +static const char * const imx8mq_g2_clk_names[] = { "g2" };
> +static const char * const imx8mq_g2_reg_names[] = { "g2" };
>

I believe the g1 and g2 _reg_names are now unused?

Thanks,
Ezequiel

> const struct hantro_variant imx8mq_vpu_variant = {
> .dec_fmts = imx8m_vpu_dec_fmts,
> @@ -305,6 +318,21 @@ const struct hantro_variant imx8mq_vpu_variant = {
> .num_regs = ARRAY_SIZE(imx8mq_reg_names)
> };
>
> +const struct hantro_variant imx8mq_vpu_g1_variant = {
> + .dec_fmts = imx8m_vpu_dec_fmts,
> + .num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
> + .postproc_fmts = imx8m_vpu_postproc_fmts,
> + .num_postproc_fmts = ARRAY_SIZE(imx8m_vpu_postproc_fmts),
> + .postproc_ops = &hantro_g1_postproc_ops,
> + .codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
> + HANTRO_H264_DECODER,
> + .codec_ops = imx8mq_vpu_g1_codec_ops,
> + .irqs = imx8mq_irqs,
> + .num_irqs = ARRAY_SIZE(imx8mq_irqs),
> + .clk_names = imx8mq_g1_clk_names,
> + .num_clocks = ARRAY_SIZE(imx8mq_g1_clk_names),
> +};
> +
> const struct hantro_variant imx8mq_vpu_g2_variant = {
> .dec_offset = 0x0,
> .dec_fmts = imx8m_vpu_g2_dec_fmts,
> @@ -314,10 +342,8 @@ const struct hantro_variant imx8mq_vpu_g2_variant = {
> .postproc_ops = &hantro_g2_postproc_ops,
> .codec = HANTRO_HEVC_DECODER | HANTRO_VP9_DECODER,
> .codec_ops = imx8mq_vpu_g2_codec_ops,
> - .init = imx8mq_vpu_hw_init,
> - .runtime_resume = imx8mq_runtime_resume,
> .irqs = imx8mq_g2_irqs,
> .num_irqs = ARRAY_SIZE(imx8mq_g2_irqs),
> - .clk_names = imx8mq_clk_names,
> - .num_clocks = ARRAY_SIZE(imx8mq_clk_names),
> + .clk_names = imx8mq_g2_clk_names,
> + .num_clocks = ARRAY_SIZE(imx8mq_g2_clk_names),
> };
> --
> 2.32.0
>

2021-12-16 12:35:14

by Ezequiel Garcia

[permalink] [raw]
Subject: Re: [PATCH V2 00/10] media: hantro: imx8mq/imx8mm: Let VPU decoders get controlled by vpu-blk-ctrl

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/[email protected]/

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
>

2021-12-16 13:10:11

by Adam Ford

[permalink] [raw]
Subject: Re: [PATCH V2 00/10] media: hantro: imx8mq/imx8mm: Let VPU decoders get controlled by vpu-blk-ctrl

On Thu, Dec 16, 2021 at 6:35 AM Ezequiel Garcia
<[email protected]> 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/[email protected]/
>
> 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
> >

2021-12-16 13:53:14

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 04/10] dt-bindings: media: nxp,imx8mq-vpu: Split G1 and G2 nodes

On Thu, 16 Dec 2021 05:12:49 -0600, Adam Ford wrote:
> The G1 and G2 are separate decoder blocks that are enabled by the
> vpu-blk-ctrl power-domain controller, which now has a proper driver.
> Update the bindings to support separate nodes for the G1 and G2
> decoders with vpu-blk-ctrl power-domain support.
>
> Signed-off-by: Adam Ford <[email protected]>
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml:18:6: [warning] wrong indentation: expected 4 but found 5 (indentation)
./Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml:19:7: [warning] wrong indentation: expected 7 but found 6 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1569050

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


2021-12-16 14:58:47

by Benjamin Gaignard

[permalink] [raw]
Subject: Re: [PATCH V2 00/10] media: hantro: imx8mq/imx8mm: Let VPU decoders get controlled by vpu-blk-ctrl


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 <[email protected]>

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

2021-12-16 17:29:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 04/10] dt-bindings: media: nxp,imx8mq-vpu: Split G1 and G2 nodes

On Thu, Dec 16, 2021 at 05:12:49AM -0600, Adam Ford wrote:
> The G1 and G2 are separate decoder blocks that are enabled by the
> vpu-blk-ctrl power-domain controller, which now has a proper driver.
> Update the bindings to support separate nodes for the G1 and G2
> decoders with vpu-blk-ctrl power-domain support.

You could expand this a bit more with Lucas' explanation and being
explicit on the compatibility implications.

Otherwise, with that and indentation fixed:

Reviewed-by: Rob Herring <[email protected]>

>
> Signed-off-by: Adam Ford <[email protected]>
>
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> index 762be3f96ce9..c1e157251de7 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> @@ -15,33 +15,20 @@ description:
>
> properties:
> compatible:
> - const: nxp,imx8mq-vpu
> + oneOf:
> + - const: nxp,imx8mq-vpu
> + deprecated: true
> + - const: nxp,imx8mq-vpu-g1
> + - const: nxp,imx8mq-vpu-g2
>
> reg:
> - maxItems: 3
> -
> - reg-names:
> - items:
> - - const: g1
> - - const: g2
> - - const: ctrl
> + maxItems: 1
>
> interrupts:
> - maxItems: 2
> -
> - interrupt-names:
> - items:
> - - const: g1
> - - const: g2
> + maxItems: 1
>
> clocks:
> - maxItems: 3
> -
> - clock-names:
> - items:
> - - const: g1
> - - const: g2
> - - const: bus
> + maxItems: 1
>
> power-domains:
> maxItems: 1
> @@ -49,31 +36,33 @@ properties:
> required:
> - compatible
> - reg
> - - reg-names
> - interrupts
> - - interrupt-names
> - clocks
> - - clock-names
>
> additionalProperties: false
>
> examples:
> - |
> #include <dt-bindings/clock/imx8mq-clock.h>
> + #include <dt-bindings/power/imx8mq-power.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + vpu_g1: video-codec@38300000 {
> + compatible = "nxp,imx8mq-vpu-g1";
> + reg = <0x38300000 0x10000>;
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>;
> + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>;
> + };
> + - |
> + #include <dt-bindings/clock/imx8mq-clock.h>
> + #include <dt-bindings/power/imx8mq-power.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
>
> - vpu: video-codec@38300000 {
> - compatible = "nxp,imx8mq-vpu";
> - reg = <0x38300000 0x10000>,
> - <0x38310000 0x10000>,
> - <0x38320000 0x10000>;
> - reg-names = "g1", "g2", "ctrl";
> - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-names = "g1", "g2";
> - clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
> - <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
> - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
> - clock-names = "g1", "g2", "bus";
> - power-domains = <&pgc_vpu>;
> + vpu_g2: video-codec@38300000 {
> + compatible = "nxp,imx8mq-vpu-g2";
> + reg = <0x38310000 0x10000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
> + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>;
> };
> --
> 2.32.0
>
>

2021-12-16 21:04:46

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 01/10] dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains

On Thu, 16 Dec 2021 05:12:46 -0600, Adam Ford wrote:
> From: Lucas Stach <[email protected]>
>
> This adds the defines for the power domains provided by the VPU
> blk-ctrl on the i.MX8MQ.
>
> Signed-off-by: Lucas Stach <[email protected]>
>

Acked-by: Rob Herring <[email protected]>

2021-12-16 21:05:23

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 02/10] dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl

On Thu, 16 Dec 2021 05:12:47 -0600, Adam Ford wrote:
> From: Lucas Stach <[email protected]>
>
> This adds the DT binding for the i.MX8MQ VPU blk-ctrl.
>
> Signed-off-by: Lucas Stach <[email protected]>
> Signed-off-by: Adam Ford <[email protected]>
>

Reviewed-by: Rob Herring <[email protected]>

2021-12-16 21:08:52

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 08/10] dt-bindings: media: nxp,imx8mq-vpu: Add support for G1 and G2 on imx8mm

On Thu, Dec 16, 2021 at 05:12:53AM -0600, Adam Ford wrote:
> The i.MX8M mini appears to have a similar G1 and G2 decoder but the
> post-procesing isn't present, so different compatible flags are requred.

post-processing

> Since all the other parameters are the same with imx8mq, just add
> the new compatible flags to nxp,imx8mq-vpu.yaml.
>
> Signed-off-by: Adam Ford <[email protected]>
>
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> index c1e157251de7..b1f24c48c73b 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> @@ -5,7 +5,7 @@
> $id: "http://devicetree.org/schemas/media/nxp,imx8mq-vpu.yaml#"
> $schema: "http://devicetree.org/meta-schemas/core.yaml#"
>
> -title: Hantro G1/G2 VPU codecs implemented on i.MX8MQ SoCs
> +title: Hantro G1/G2 VPU codecs implemented on i.MX8MQ/i.MX8MM SoCs

Just 'i.MX8' so we don't have to change this everytime?

>
> maintainers:
> - Philipp Zabel <[email protected]>
> @@ -20,6 +20,8 @@ properties:
> deprecated: true
> - const: nxp,imx8mq-vpu-g1
> - const: nxp,imx8mq-vpu-g2
> + - const: nxp,imx8mm-vpu-g1
> + - const: nxp,imx8mm-vpu-g2

Not compatible with the imx8mq variants?

>
> reg:
> maxItems: 1
> @@ -66,3 +68,27 @@ examples:
> clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
> power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>;
> };
> + - |
> + #include <dt-bindings/clock/imx8mm-clock.h>
> + #include <dt-bindings/power/imx8mm-power.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + vpu_g1: video-codec@38300000 {
> + compatible = "nxp,imx8mm-vpu-g1";
> + reg = <0x38300000 0x10000>;
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>;
> + power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G1>;
> + };
> + - |
> + #include <dt-bindings/clock/imx8mm-clock.h>
> + #include <dt-bindings/power/imx8mm-power.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + vpu_g2: video-codec@38300000 {
> + compatible = "nxp,imx8mm-vpu-g2";
> + reg = <0x38310000 0x10000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MM_CLK_VPU_G2_ROOT>;
> + power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G2>;
> + };

No point in more examples just for a different compatible.

> --
> 2.32.0
>
>

2021-12-16 21:21:37

by Adam Ford

[permalink] [raw]
Subject: Re: [PATCH V2 08/10] dt-bindings: media: nxp,imx8mq-vpu: Add support for G1 and G2 on imx8mm

On Thu, Dec 16, 2021 at 3:07 PM Rob Herring <[email protected]> wrote:
>
> On Thu, Dec 16, 2021 at 05:12:53AM -0600, Adam Ford wrote:
> > The i.MX8M mini appears to have a similar G1 and G2 decoder but the
> > post-procesing isn't present, so different compatible flags are requred.
>
> post-processing
>
> > Since all the other parameters are the same with imx8mq, just add
> > the new compatible flags to nxp,imx8mq-vpu.yaml.
> >
> > Signed-off-by: Adam Ford <[email protected]>
> >
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > index c1e157251de7..b1f24c48c73b 100644
> > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > @@ -5,7 +5,7 @@
> > $id: "http://devicetree.org/schemas/media/nxp,imx8mq-vpu.yaml#"
> > $schema: "http://devicetree.org/meta-schemas/core.yaml#"
> >
> > -title: Hantro G1/G2 VPU codecs implemented on i.MX8MQ SoCs
> > +title: Hantro G1/G2 VPU codecs implemented on i.MX8MQ/i.MX8MM SoCs
>
> Just 'i.MX8' so we don't have to change this everytime?

Are you OK with i.MX8M? 8MQ, 8MM, and 8MP all appear to have G1 and
G2 decoders. The i.MX8 is different.
>
> >
> > maintainers:
> > - Philipp Zabel <[email protected]>
> > @@ -20,6 +20,8 @@ properties:
> > deprecated: true
> > - const: nxp,imx8mq-vpu-g1
> > - const: nxp,imx8mq-vpu-g2
> > + - const: nxp,imx8mm-vpu-g1
> > + - const: nxp,imx8mm-vpu-g2
>
> Not compatible with the imx8mq variants?

No, the structures associated with these compatible flags telling the
driver what features are available have options for the post-processor
in the 8MQ which are not present in the 8MM.

>
> >
> > reg:
> > maxItems: 1
> > @@ -66,3 +68,27 @@ examples:
> > clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>;
> > power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>;
> > };
> > + - |
> > + #include <dt-bindings/clock/imx8mm-clock.h>
> > + #include <dt-bindings/power/imx8mm-power.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + vpu_g1: video-codec@38300000 {
> > + compatible = "nxp,imx8mm-vpu-g1";
> > + reg = <0x38300000 0x10000>;
> > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>;
> > + power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G1>;
> > + };
> > + - |
> > + #include <dt-bindings/clock/imx8mm-clock.h>
> > + #include <dt-bindings/power/imx8mm-power.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + vpu_g2: video-codec@38300000 {
> > + compatible = "nxp,imx8mm-vpu-g2";
> > + reg = <0x38310000 0x10000>;
> > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk IMX8MM_CLK_VPU_G2_ROOT>;
> > + power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G2>;
> > + };
>
> No point in more examples just for a different compatible.

No problem.
>
> > --
> > 2.32.0
> >
> >

2021-12-16 23:03:59

by Ezequiel Garcia

[permalink] [raw]
Subject: Re: [PATCH V2 08/10] dt-bindings: media: nxp,imx8mq-vpu: Add support for G1 and G2 on imx8mm

Hi Adam,

On Thu, 16 Dec 2021 at 18:21, Adam Ford <[email protected]> wrote:
>
> On Thu, Dec 16, 2021 at 3:07 PM Rob Herring <[email protected]> wrote:
> >
> > On Thu, Dec 16, 2021 at 05:12:53AM -0600, Adam Ford wrote:
> > > The i.MX8M mini appears to have a similar G1 and G2 decoder but the
> > > post-procesing isn't present, so different compatible flags are requred.
> >
> > post-processing
> >
> > > Since all the other parameters are the same with imx8mq, just add
> > > the new compatible flags to nxp,imx8mq-vpu.yaml.
> > >
> > > Signed-off-by: Adam Ford <[email protected]>
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > index c1e157251de7..b1f24c48c73b 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > @@ -5,7 +5,7 @@
> > > $id: "http://devicetree.org/schemas/media/nxp,imx8mq-vpu.yaml#"
> > > $schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > >
> > > -title: Hantro G1/G2 VPU codecs implemented on i.MX8MQ SoCs
> > > +title: Hantro G1/G2 VPU codecs implemented on i.MX8MQ/i.MX8MM SoCs
> >
> > Just 'i.MX8' so we don't have to change this everytime?
>
> Are you OK with i.MX8M? 8MQ, 8MM, and 8MP all appear to have G1 and
> G2 decoders. The i.MX8 is different.
> >
> > >
> > > maintainers:
> > > - Philipp Zabel <[email protected]>
> > > @@ -20,6 +20,8 @@ properties:
> > > deprecated: true
> > > - const: nxp,imx8mq-vpu-g1
> > > - const: nxp,imx8mq-vpu-g2
> > > + - const: nxp,imx8mm-vpu-g1
> > > + - const: nxp,imx8mm-vpu-g2
> >
> > Not compatible with the imx8mq variants?
>
> No, the structures associated with these compatible flags telling the
> driver what features are available have options for the post-processor
> in the 8MQ which are not present in the 8MM.
>

Just as G1 and G2 are different blocks, their "post-processor"
features are really different too.

The G2 core typically produces a tiled format, NV12_4L4,
and it an inline post-processor to convert that to linear NV12.

How does this work on the 8MM? What pixel format does it
produce natively?

It's hard to imagine the G2 block doesn't do linear NV12,
so I'm inclined to think it has that post-processing feature.

Thanks,
Ezequiel