Re: [PATCH v2 1/4] dt-bindings: remoteproc: Add VCP support for mt8196

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 14/09/25 14:29, Xiangzhi Tang ha scritto:
> Add the new binding document for MediaTek Video Companion
> Processor(VCP) on MediaTek mt8196.
> 
> Signed-off-by: Xiangzhi Tang <xiangzhi.tang@mediatek.com>
> ---
>   .../remoteproc/mediatek,mt8196-vcp.yaml       | 165 ++++++++++++++++++
>   1 file changed, 165 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/remoteproc/mediatek,mt8196-vcp.yaml
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/mediatek,mt8196-vcp.yaml b/Documentation/devicetree/bindings/remoteproc/mediatek,mt8196-vcp.yaml
> new file mode 100644
> index 000000000000..71a55943843b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/mediatek,mt8196-vcp.yaml
> @@ -0,0 +1,165 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/mediatek,mt8196-vcp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Video Companion Processor (VCP)
> +
> +maintainers:
> +  - Xiangzhi Tang <Xiangzhi.Tang@mediatek.com>
> +
> +description:
> +  The MediaTek VCP enables the SoC control the MediaTek Video Companion Risc-V coprocessor.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,mt8196-vcp
> +
> +  reg:
> +    items:
> +      - description: sram base
> +      - description: cfg group IO
> +      - description: cfg core group IO
> +      - description: cfg sec group IO
> +      - description: vcp rdy group IO
> +
> +  reg-names:
> +    items:
> +      - const: sram
> +      - const: cfg
> +      - const: cfg_core

sram, cfg-global, cfg-core, cfg-sec, vcp-vlp-ao-ready

> +      - const: cfg_sec
> +      - const: vcp_vlp_ao_rsvd7
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  mboxes:
> +    maxItems: 5
> +
> +  mbox-names:
> +    maxItems: 5
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  iommus:
> +    description:
> +      Using MediaTek iommu to apply larb ports for Multimedia Memory
> +      Management Unit and address translation
> +      Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> +    maxItems: 1
> +
> +  memory-region:
> +    maxItems: 1
> +
> +patternProperties:
> +  "^vcp@[a-f0-9]+$":
> +    type: object
> +    description:
> +      The MediaTek VCP integrated to SoC might be a multi-core version.

"may feature one or multiple cores"

> +      The other cores are represented as child nodes of the boot core.
> +      There are some integration differences for the IP like the usage of
> +      address translator for translating SoC bus addresses into address
> +      space for the processor.
> +
> +      The SRAM are shared by all cores, each VCP core only using a piece

s/piece/portion of/g

> +      SRAM memory. The power of SRAM should be enabled before booting VCP cores.

How do you enable the SRAM power?

Is there a regulator, a power domain, both, or what?

> +      The size of SRAM are varied on differnt SoCs.
> +
> +      The VCP cores has differences on different SoCs to support for
> +      Hart.
> +
> +    properties:
> +      compatible:
> +        enum:
> +          - mediatek,vcp-core
> +
> +      reg:
> +        description: The base address and size of SRAM.
> +        maxItems: 1
> +
> +      reg-names:
> +        const: sram
> +
> +      mtk,vcp-core-twohart:

1. s/mtk,/mediatek,/g
2. there's no description, what does "twohart" mean?

> +        enum: [0, 1]
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +      mtk,vcp-sram-offset:
> +        description:
> +          Allocated SRAM memory for each VCP core used.

vcp-sram-offset is the same as the iostart in `reg`, so you don't need this
property at all.

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +    required:
> +      - compatible
> +      - reg
> +      - reg-names
> +      - mtk,vcp-core-twohart
> +      - mtk,vcp-sram-offset
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - mboxes
> +  - mbox-names
> +  - power-domains
> +  - iommus
> +  - memory-region
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/power/mt8196-power.h>
> +
> +    vcp: vcp@31800000 {
> +        compatible = "mediatek,mt8196-vcp";
> +        reg = <0x31800000 0x60000>,
> +              <0x31a04000 0xa000>,
> +              <0x31bd0000 0x1000>,
> +              <0x31a70020 0x100>,
> +              <0x1c00091c 0x4>;
> +        reg-names = "sram",
> +                    "cfg",
> +                    "cfg_core",
> +                    "cfg_sec",
> +                    "vcp_vlp_ao_rsvd7";
> +
> +        interrupts = <GIC_SPI 787 IRQ_TYPE_LEVEL_HIGH 0>;
> +
> +        mboxes = <&vcp_mailbox0>,
> +                 <&vcp_mailbox1>,
> +                 <&vcp_mailbox2>,
> +                 <&vcp_mailbox3>,
> +                 <&vcp_mailbox4>;
> +        mbox-names = "mbox0", "mbox1", "mbox2", "mbox3", "mbox4";

Is there any descriptive name that can be used for mbox-names?

As in, what is mbox0 used for? what is mbox1 used for? etc.

> +
> +        power-domains = <&scpsys MT8196_POWER_DOMAIN_MM_PROC_DORMANT>;
> +        iommus = <&mm_smmu 160>;
> +        memory-region = <&vcp_resv_mem>;
> +
> +        vcp@0 {
> +            compatible = "mediatek,vcp-core";
> +            reg = <0x0 0x31000>;
> +            reg-names = "sram";
> +            mtk,vcp-core-twohart = <1>;

Is the first core always "twohart"?

If it is, there's no need to even have this property, as you can add that to
the driver, either in form of platform data (if this changes per-SoC) or just
hardcoded.

Regards,
Angelo

> +            mtk,vcp-sram-offset = <0x0>;
> +        };
> +
> +        vcp@31000 {
> +            compatible = "mediatek,vcp-core";
> +            reg = <0x31000 0x60000>;
> +            reg-names = "sram";
> +            mtk,vcp-core-twohart = <0>;
> +            mtk,vcp-sram-offset = <0x31000>;
> +        };
> +    };