On Thu, Dec 9, 2021 at 4:26 AM Ezequiel Garcia
[off-list ref] wrote:

Hi,

Thanks for the patch.

On Wed, Dec 08, 2021 at 04:50:23PM -0600, Adam Ford wrote:

quoted

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 using the proper driver or the older unified node with
the legacy controls.

To be compatible with older DT the driver, mark certain items as
deprecated and retain the backwards compatible example.

Signed-off-by: Adam Ford <redacted>
---
 .../bindings/media/nxp,imx8mq-vpu.yaml        | 83 ++++++++++++++-----
 1 file changed, 64 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
index 762be3f96ce9..eeb7bd6281f9 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml

@@ -15,29 +15,39 @@ 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:
+    minItems: 1
     maxItems: 3

Is it really useful to keep the deprecated binding nxp,imx8mq-vpu
as something supported by the binding file?

Since I was told that the driver needed to be backwards compatible, i
wanted to make sure that any attempts to build the old device tree
would not fail

In other words, can we drop the deprecated binding from this file,
while keeping the support in the driver for legacy device-trees?

I was trying to represent both the old driver binding and the new one
at the same time.  I thought that's what I was told to do.

[..]

quoted

+
+  # VPU G1 with vpu-blk-ctrl
+  - |
+    #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>;
+        reg-names "g1";
+        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "g1";
+        clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>;
+        clock-names = "g1";

reg-names, interrupt-names and clock-names should be removed
given for this device there's only one of each.

I attempted to remove the reg-names, but it failed to enumerate for me
when I did that.

This will make the binding actually quite easier, but it also
means you need to make some changes to struct hantro_variant imx8mq_vpu_g1_variant
to make it work properly.

See Rob's feedback on the SAMA5 VPU binding:

https://yhbt.net/lore/all/20210324151715.GA3070006@robh.at.kernel.org/

Also, take a look at drivers/staging/media/hantro/sama5d4_vdec_hw.c
for reference.

I can try again using this as an example.

quoted

+        power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>;
+    };
+
+  # VPU G2 with vpu-blk-ctrl
+  - |
+    #include <dt-bindings/clock/imx8mq-clock.h>
+    #include <dt-bindings/power/imx8mq-power.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    vpu_g2: video-codec@38310000 {
+        compatible = "nxp,imx8mq-vpu-g2";
+        reg = <0x38310000 0x10000>;
+        reg-names "g2";

And same here.

Thanks!
Ezequiel