[PATCH v5 0/3] Add vcodec lat and core nodes for mt8192

[PATCH v5 0/3] Add vcodec lat and core nodes for mt8192

[Allen-KH Cheng]

This series is based on linux-next.

Change from v4:
 - Add patch for removing dma-ranges
 - Update child names to video-codec
 - Update patch title and message

Change from v3:
 - Drop `vcodec_lat:` and `vcodec_core:`

Change from v2:
 - Add a dt-bindings patch for changing "vcodec" to "video-codec"
 - Rename child names for vcodec lat and core nodes

Change from v1:
 - Replace node names with video-codec
  [Allen-KH Cheng <allen-kh.cheng@mediatek.com>]

Allen-KH Cheng (3):
  media: dt-bindings: media: mediatek: Rename child node names for
    decoder
  media: dt-bindings: media: mediatek: Remove "dma-ranges" property for
    decoder
  arm64: dts: mt8192: Add video-codec nodes

 .../media/mediatek,vcodec-subdev-decoder.yaml | 67 ++-----------------
 arch/arm64/boot/dts/mediatek/mt8192.dtsi      | 59 ++++++++++++++++
 2 files changed, 63 insertions(+), 63 deletions(-)

-- 
2.18.0

[PATCH v5 1/3] media: dt-bindings: media: mediatek: Rename child node names for decoder

[Allen-KH Cheng]

In order to make the names of the child nodes more generic, we rename
"vcodec-lat" and "vcodec-core" to "video-codec" for decoder in
patternProperties and example.

Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
---
 .../media/mediatek,vcodec-subdev-decoder.yaml | 60 ++-----------------
 1 file changed, 4 insertions(+), 56 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
index c4f20acdc1f8..695402041e04 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
@@ -91,12 +91,13 @@ properties:
 
 # Required child node:
 patternProperties:
-  '^vcodec-lat@[0-9a-f]+$':
+  '^video-codec@[0-9a-f]+$':
     type: object
 
     properties:
       compatible:
         enum:
+          - mediatek,mtk-vcodec-core
           - mediatek,mtk-vcodec-lat
           - mediatek,mtk-vcodec-lat-soc
 
@@ -145,59 +146,6 @@ patternProperties:
 
     additionalProperties: false
 
-  '^vcodec-core@[0-9a-f]+$':
-    type: object
-
-    properties:
-      compatible:
-        const: mediatek,mtk-vcodec-core
-
-      reg:
-        maxItems: 1
-
-      interrupts:
-        maxItems: 1
-
-      iommus:
-        minItems: 1
-        maxItems: 32
-        description: |
-          List of the hardware port in respective IOMMU block for current Socs.
-          Refer to bindings/iommu/mediatek,iommu.yaml.
-
-      clocks:
-        maxItems: 5
-
-      clock-names:
-        items:
-          - const: sel
-          - const: soc-vdec
-          - const: soc-lat
-          - const: vdec
-          - const: top
-
-      assigned-clocks:
-        maxItems: 1
-
-      assigned-clock-parents:
-        maxItems: 1
-
-      power-domains:
-        maxItems: 1
-
-    required:
-      - compatible
-      - reg
-      - interrupts
-      - iommus
-      - clocks
-      - clock-names
-      - assigned-clocks
-      - assigned-clock-parents
-      - power-domains
-
-    additionalProperties: false
-
 required:
   - compatible
   - reg
@@ -241,7 +189,7 @@ examples:
             #size-cells = <2>;
             ranges = <0 0 0 0x16000000 0 0x40000>;
             reg = <0 0x16000000 0 0x1000>;		/* VDEC_SYS */
-            vcodec-lat@10000 {
+            video-codec@10000 {
                 compatible = "mediatek,mtk-vcodec-lat";
                 reg = <0 0x10000 0 0x800>;
                 interrupts = <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -264,7 +212,7 @@ examples:
                 power-domains = <&spm MT8192_POWER_DOMAIN_VDEC>;
             };
 
-            vcodec-core@25000 {
+            video-codec@25000 {
                 compatible = "mediatek,mtk-vcodec-core";
                 reg = <0 0x25000 0 0x1000>;
                 interrupts = <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH 0>;
-- 
2.18.0

[PATCH v5 2/3] media: dt-bindings: media: mediatek: Remove "dma-ranges" property for decoder

[Allen-KH Cheng]

Since commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus
controllers") was merge, we don't need a dma-ranges property for IOMMU
in mediatek video codec.

We remove the dma-ranges property and fix the example in
mediatek,vcodec-subdev-decoder.yaml

Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
---
 .../bindings/media/mediatek,vcodec-subdev-decoder.yaml     | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
index 695402041e04..7c5b4a91c59b 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
@@ -76,11 +76,6 @@ properties:
       The node of system control processor (SCP), using
       the remoteproc & rpmsg framework.
 
-  dma-ranges:
-    maxItems: 1
-    description: |
-      Describes the physical address space of IOMMU maps to memory.
-
   "#address-cells":
     const: 2
 
@@ -151,7 +146,6 @@ required:
   - reg
   - iommus
   - mediatek,scp
-  - dma-ranges
   - ranges
 
 if:
@@ -184,7 +178,6 @@ examples:
             compatible = "mediatek,mt8192-vcodec-dec";
             mediatek,scp = <&scp>;
             iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>;
-            dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>;
             #address-cells = <2>;
             #size-cells = <2>;
             ranges = <0 0 0 0x16000000 0 0x40000>;
-- 
2.18.0

[PATCH v5 3/3] arm64: dts: mt8192: Add video-codec nodes

[Allen-KH Cheng]

Add video-codec lat and core nodes for mt8192 SoC.

Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 59 ++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index 424fc89cc6f7..eb5606204f22 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -1452,6 +1452,65 @@
 			power-domains = <&spm MT8192_POWER_DOMAIN_ISP2>;
 		};
 
+		vcodec_dec: video-codec@16000000 {
+			compatible = "mediatek,mt8192-vcodec-dec";
+			reg = <0 0x16000000 0 0x1000>;
+			mediatek,scp = <&scp>;
+			iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges = <0 0 0 0x16000000 0 0x26000>;
+
+			video-codec@10000 {
+				compatible = "mediatek,mtk-vcodec-lat";
+				reg = <0x0 0x10000 0 0x800>;
+				interrupts = <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH 0>;
+				iommus = <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD_EXT>,
+					 <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD2_EXT>,
+					 <&iommu0 M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT>,
+					 <&iommu0 M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT>,
+					 <&iommu0 M4U_PORT_L5_VDEC_LAT0_TILE_EXT>,
+					 <&iommu0 M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>,
+					 <&iommu0 M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>,
+					 <&iommu0 M4U_PORT_L5_VDEC_UFO_ENC_EXT>;
+				clocks = <&topckgen CLK_TOP_VDEC_SEL>,
+					 <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
+					 <&vdecsys_soc CLK_VDEC_SOC_LAT>,
+					 <&vdecsys_soc CLK_VDEC_SOC_LARB1>,
+					 <&topckgen CLK_TOP_MAINPLL_D4>;
+				clock-names = "sel", "soc-vdec", "soc-lat", "vdec", "top";
+				assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>;
+				assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>;
+				power-domains = <&spm MT8192_POWER_DOMAIN_VDEC>;
+			};
+
+			video-codec@25000 {
+				compatible = "mediatek,mtk-vcodec-core";
+				reg = <0 0x25000 0 0x1000>;
+				interrupts = <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH 0>;
+				iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_UFO_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_PP_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_PRED_RD_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_PRED_WR_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_PPWRAP_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_TILE_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_VLD_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_VLD2_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_AVC_MV_EXT>,
+					 <&iommu0 M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT>;
+				clocks = <&topckgen CLK_TOP_VDEC_SEL>,
+					 <&vdecsys CLK_VDEC_VDEC>,
+					 <&vdecsys CLK_VDEC_LAT>,
+					 <&vdecsys CLK_VDEC_LARB1>,
+					 <&topckgen CLK_TOP_MAINPLL_D4>;
+				clock-names = "sel", "soc-vdec", "soc-lat", "vdec", "top";
+				assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>;
+				assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>;
+				power-domains = <&spm MT8192_POWER_DOMAIN_VDEC2>;
+			};
+		};
+
 		larb5: larb@1600d000 {
 			compatible = "mediatek,mt8192-smi-larb";
 			reg = <0 0x1600d000 0 0x1000>;
-- 
2.18.0

Re: [PATCH v5 2/3] media: dt-bindings: media: mediatek: Remove "dma-ranges" property for decoder

[AngeloGioacchino Del Regno]

Il 28/11/22 15:38, Allen-KH Cheng ha scritto:
> Since commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus
> controllers") was merge, we don't need a dma-ranges property for IOMMU
> in mediatek video codec.
> 
> We remove the dma-ranges property and fix the example in
> mediatek,vcodec-subdev-decoder.yaml
> 
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v5 3/3] arm64: dts: mt8192: Add video-codec nodes

[AngeloGioacchino Del Regno]

Il 28/11/22 15:38, Allen-KH Cheng ha scritto:
> Add video-codec lat and core nodes for mt8192 SoC.
> 
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v5 0/3] Add vcodec lat and core nodes for mt8192

[Chen-Yu Tsai]

On Mon, Nov 28, 2022 at 10:38 PM Allen-KH Cheng
<allen-kh.cheng@mediatek.com> wrote:
>
> This series is based on linux-next.
>
> Change from v4:
>  - Add patch for removing dma-ranges
>  - Update child names to video-codec
>  - Update patch title and message
>
> Change from v3:
>  - Drop `vcodec_lat:` and `vcodec_core:`
>
> Change from v2:
>  - Add a dt-bindings patch for changing "vcodec" to "video-codec"
>  - Rename child names for vcodec lat and core nodes
>
> Change from v1:
>  - Replace node names with video-codec
>   [Allen-KH Cheng <allen-kh.cheng@mediatek.com>]
>
> Allen-KH Cheng (3):
>   media: dt-bindings: media: mediatek: Rename child node names for
>     decoder
>   media: dt-bindings: media: mediatek: Remove "dma-ranges" property for
>     decoder
>   arm64: dts: mt8192: Add video-codec nodes

Whole series is:

Tested-by: Chen-Yu Tsai <wenst@chromium.org>

Re: [PATCH v5 1/3] media: dt-bindings: media: mediatek: Rename child node names for decoder

[Krzysztof Kozlowski]

On 28/11/2022 15:38, Allen-KH Cheng wrote:
> In order to make the names of the child nodes more generic, we rename
> "vcodec-lat" and "vcodec-core" to "video-codec" for decoder in
> patternProperties and example.
> 
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
> ---
>  .../media/mediatek,vcodec-subdev-decoder.yaml | 60 ++-----------------
>  1 file changed, 4 insertions(+), 56 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v5 2/3] media: dt-bindings: media: mediatek: Remove "dma-ranges" property for decoder

[Rob Herring]

On Mon, 28 Nov 2022 22:38:31 +0800, Allen-KH Cheng wrote:
> Since commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus
> controllers") was merge, we don't need a dma-ranges property for IOMMU
> in mediatek video codec.
> 
> We remove the dma-ranges property and fix the example in
> mediatek,vcodec-subdev-decoder.yaml
> 
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
> ---
>  .../bindings/media/mediatek,vcodec-subdev-decoder.yaml     | 7 -------
>  1 file changed, 7 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 2/3] media: dt-bindings: media: mediatek: Remove "dma-ranges" property for decoder

[Nícolas F. R. A. Prado]

On Mon, Nov 28, 2022 at 10:38:31PM +0800, Allen-KH Cheng wrote:
> Since commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus
> controllers") was merge, we don't need a dma-ranges property for IOMMU
> in mediatek video codec.

This description isn't right, as it gives the impression that the mentioned
commit removed the need for having a dma-ranges property, but that isn't the
case, the property wasn't needed even before that commit. That commit did
however expose the fact that having this property didn't make sense in the first
place.

Here's a suggestion:

	The decoder nodes already make use of the iommus property to configure the IOMMU
	for address translations, so also having a dma-ranges property doesn't make
	sense.

	In fact, after commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus
	controllers"), having a dma-ranges property causes IOMMU faults.

	Remove the dma-ranges property and update the example.

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

Re: [PATCH v5 3/3] arm64: dts: mt8192: Add video-codec nodes

[Nícolas F. R. A. Prado]

On Mon, Nov 28, 2022 at 10:38:32PM +0800, Allen-KH Cheng wrote:
> Add video-codec lat and core nodes for mt8192 SoC.

Even though the rule is to use generic names in the DT, here in the commit
message and title I would have called them "video decoder nodes", to be more
specific and differentiate from the encoder nodes. But the way you wrote is not
wrong, so feel free to ignore this comment and leave it as it is.

> 
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

Re: [PATCH v5 1/3] media: dt-bindings: media: mediatek: Rename child node names for decoder

[Nícolas F. R. A. Prado]

On Mon, Nov 28, 2022 at 10:38:30PM +0800, Allen-KH Cheng wrote:
> In order to make the names of the child nodes more generic, we rename
> "vcodec-lat" and "vcodec-core" to "video-codec" for decoder in
> patternProperties and example.
> 
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
> ---
>  .../media/mediatek,vcodec-subdev-decoder.yaml | 60 ++-----------------
>  1 file changed, 4 insertions(+), 56 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> index c4f20acdc1f8..695402041e04 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> @@ -91,12 +91,13 @@ properties:
>  
>  # Required child node:
>  patternProperties:
> -  '^vcodec-lat@[0-9a-f]+$':
> +  '^video-codec@[0-9a-f]+$':
>      type: object
>  
>      properties:
>        compatible:
>          enum:
> +          - mediatek,mtk-vcodec-core
>            - mediatek,mtk-vcodec-lat
>            - mediatek,mtk-vcodec-lat-soc
>  
> @@ -145,59 +146,6 @@ patternProperties:
>  
>      additionalProperties: false
>  
> -  '^vcodec-core@[0-9a-f]+$':
> -    type: object
> -
> -    properties:
> -      compatible:
> -        const: mediatek,mtk-vcodec-core
> -
> -      reg:
> -        maxItems: 1
> -
> -      interrupts:
> -        maxItems: 1
> -
> -      iommus:
> -        minItems: 1
> -        maxItems: 32
> -        description: |
> -          List of the hardware port in respective IOMMU block for current Socs.
> -          Refer to bindings/iommu/mediatek,iommu.yaml.
> -
> -      clocks:
> -        maxItems: 5
> -
> -      clock-names:
> -        items:
> -          - const: sel
> -          - const: soc-vdec
> -          - const: soc-lat
> -          - const: vdec
> -          - const: top
> -
> -      assigned-clocks:
> -        maxItems: 1
> -
> -      assigned-clock-parents:
> -        maxItems: 1
> -
> -      power-domains:
> -        maxItems: 1
> -
> -    required:
> -      - compatible
> -      - reg
> -      - interrupts

Looks like interrupts was required for vcodec-core, but it isn't for the generic
video-codec node. Which seems correct, given that the vcodec-lat-soc doesn't
have an interrupt [1]. So I guess this is just the generic video-codec node in
the binding being too generic for some cases. Ideally we would override
interrupts to be required based on which subnode we're dealing with (for lat and
core, but not lat-soc), but given these are subnodes matched through
patternProperties, I'm not sure that would be possible.

[1] https://lore.kernel.org/all/20221202034450.3808-3-yunfei.dong@mediatek.com/

Thanks,
Nícolas

> -      - iommus
> -      - clocks
> -      - clock-names
> -      - assigned-clocks
> -      - assigned-clock-parents
> -      - power-domains
> -
> -    additionalProperties: false
> -