2022-11-10 10:35:09

by Yunfei Dong (董云飞)

[permalink] [raw]
Subject: [PATCH v2,1/3] media: dt-bindings: media: mediatek: vcodec: Fix clock num not correctly

mt8195 and mt8192 have different clock numbers, can't write 'clocks' and
'clock-names' with const value.

Move 'assigned-clocks' and 'assigned-clock-parents' to parent node.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../media/mediatek,vcodec-subdev-decoder.yaml | 119 +++++++++++-------
1 file changed, 72 insertions(+), 47 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..794012853834 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
@@ -89,23 +89,33 @@ properties:

ranges: true

+ clocks:
+ minItems: 1
+ maxItems: 5
+
+ clock-names:
+ minItems: 1
+ maxItems: 5
+
+ assigned-clocks:
+ maxItems: 1
+
+ assigned-clock-parents:
+ maxItems: 1
+
# Required child node:
patternProperties:
- '^vcodec-lat@[0-9a-f]+$':
+ '^vcodec-lat-soc@[0-9a-f]+$':
type: object

properties:
compatible:
enum:
- - mediatek,mtk-vcodec-lat
- mediatek,mtk-vcodec-lat-soc

reg:
maxItems: 1

- interrupts:
- maxItems: 1
-
iommus:
minItems: 1
maxItems: 32
@@ -114,22 +124,55 @@ patternProperties:
Refer to bindings/iommu/mediatek,iommu.yaml.

clocks:
+ minItems: 1
maxItems: 5

clock-names:
- items:
- - const: sel
- - const: soc-vdec
- - const: soc-lat
- - const: vdec
- - const: top
+ minItems: 1
+ maxItems: 5

- assigned-clocks:
+ power-domains:
maxItems: 1

- assigned-clock-parents:
+ required:
+ - compatible
+ - reg
+ - iommus
+ - clocks
+ - clock-names
+ - power-domains
+
+ additionalProperties: false
+
+ '^vcodec-lat@[0-9a-f]+$':
+ type: object
+
+ properties:
+ compatible:
+ enum:
+ - mediatek,mtk-vcodec-lat
+
+ 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:
+ minItems: 1
+ maxItems: 5
+
+ clock-names:
+ minItems: 1
+ maxItems: 5
+
power-domains:
maxItems: 1

@@ -139,8 +182,6 @@ patternProperties:
- iommus
- clocks
- clock-names
- - assigned-clocks
- - assigned-clock-parents
- power-domains

additionalProperties: false
@@ -166,15 +207,12 @@ patternProperties:
Refer to bindings/iommu/mediatek,iommu.yaml.

clocks:
+ minItems: 1
maxItems: 5

clock-names:
- items:
- - const: sel
- - const: soc-vdec
- - const: soc-lat
- - const: vdec
- - const: top
+ minItems: 1
+ maxItems: 5

assigned-clocks:
maxItems: 1
@@ -188,12 +226,9 @@ patternProperties:
required:
- compatible
- reg
- - interrupts
- iommus
- clocks
- clock-names
- - assigned-clocks
- - assigned-clock-parents
- power-domains

additionalProperties: false
@@ -205,17 +240,10 @@ required:
- mediatek,scp
- dma-ranges
- ranges
-
-if:
- properties:
- compatible:
- contains:
- enum:
- - mediatek,mtk-vcodec-lat
-
-then:
- required:
- - interrupts
+ - clocks
+ - clock-names
+ - assigned-clocks
+ - assigned-clock-parents

additionalProperties: false

@@ -241,6 +269,11 @@ examples:
#size-cells = <2>;
ranges = <0 0 0 0x16000000 0 0x40000>;
reg = <0 0x16000000 0 0x1000>; /* VDEC_SYS */
+ clocks = <&topckgen CLK_TOP_VDEC_SEL>,
+ <&topckgen CLK_TOP_MAINPLL_D4>;
+ clock-names = "sel", "top";
+ assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>;
+ assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>;
vcodec-lat@10000 {
compatible = "mediatek,mtk-vcodec-lat";
reg = <0 0x10000 0 0x800>;
@@ -253,14 +286,10 @@ examples:
<&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>,
+ clocks = <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
<&vdecsys_soc CLK_VDEC_SOC_LAT>,
- <&vdecsys_soc CLK_VDEC_SOC_LARB1>,
- <&topckgen CLK_TOP_MAINPLL_D4>;
+ <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
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>;
};

@@ -279,14 +308,10 @@ examples:
<&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>,
+ clocks = <&vdecsys CLK_VDEC_VDEC>,
<&vdecsys CLK_VDEC_LAT>,
- <&vdecsys CLK_VDEC_LARB1>,
- <&topckgen CLK_TOP_MAINPLL_D4>;
+ <&vdecsys CLK_VDEC_LARB1>;
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>;
};
};
--
2.18.0



2022-11-10 11:03:43

by Yunfei Dong (董云飞)

[permalink] [raw]
Subject: [PATCH v2,3/3] arm64: dts: mt8195: Add video decoder node

Add video decoder node to mt8195 device tree.

Signed-off-by: Yunfei Dong <[email protected]>
---
Compared with v1:
- add description in yaml, and remove /* ... */ for each reg.
---
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 63 ++++++++++++++++++++++++
1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 905d1a90b406..3ef7eef02415 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -1874,6 +1874,69 @@
power-domains = <&spm MT8195_POWER_DOMAIN_CAM>;
};

+ video-codec@18000000 {
+ compatible = "mediatek,mt8195-vcodec-dec";
+ mediatek,scp = <&scp>;
+ iommus = <&iommu_vdo M4U_PORT_L21_VDEC_MC_EXT>;
+ dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ reg = <0 0x18000000 0 0x1000>,
+ <0 0x18004000 0 0x1000>;
+ ranges = <0 0 0 0x18000000 0 0x26000>;
+ clocks = <&topckgen CLK_TOP_VDEC>,
+ <&topckgen CLK_TOP_UNIVPLL_D4>;
+ clock-names = "vdec-sel", "top";
+ assigned-clocks = <&topckgen CLK_TOP_VDEC>;
+ assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D4>;
+
+ vcodec-lat-soc@2000 {
+ compatible = "mediatek,mtk-vcodec-lat-soc";
+ reg = <0 0x2000 0 0x800>;
+ iommus = <&iommu_vpp M4U_PORT_L23_VDEC_UFO_ENC_EXT>,
+ <&iommu_vpp M4U_PORT_L23_VDEC_RDMA_EXT>;
+ clocks = <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
+ <&vdecsys_soc CLK_VDEC_SOC_LAT>;
+ clock-names = "vdec-soc-vdec", "vdec-soc-lat";
+ power-domains = <&spm MT8195_POWER_DOMAIN_VDEC0>;
+ };
+
+ vcodec-lat@10000 {
+ compatible = "mediatek,mtk-vcodec-lat";
+ reg = <0 0x10000 0 0x800>;
+ interrupts = <GIC_SPI 708 IRQ_TYPE_LEVEL_HIGH 0>;
+ iommus = <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_VLD_EXT>,
+ <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_VLD2_EXT>,
+ <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_AVC_MC_EXT>,
+ <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_PRED_RD_EXT>,
+ <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_TILE_EXT>,
+ <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_WDMA_EXT>;
+ clocks = <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
+ <&vdecsys_soc CLK_VDEC_SOC_LAT>;
+ clock-names = "vdec-soc-vdec", "vdec-soc-lat";
+ power-domains = <&spm MT8195_POWER_DOMAIN_VDEC0>;
+ };
+
+ vcodec-core@25000 {
+ compatible = "mediatek,mtk-vcodec-core";
+ reg = <0 0x25000 0 0x1000>;
+ interrupts = <GIC_SPI 707 IRQ_TYPE_LEVEL_HIGH 0>;
+ iommus = <&iommu_vdo M4U_PORT_L21_VDEC_MC_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_UFO_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_PP_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_PRED_RD_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_PRED_WR_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_PPWRAP_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_TILE_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_VLD_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_VLD2_EXT>,
+ <&iommu_vdo M4U_PORT_L21_VDEC_AVC_MV_EXT>;
+ clocks = <&vdecsys CLK_VDEC_VDEC>, <&vdecsys CLK_VDEC_LAT>;
+ clock-names = "vdec-vdec", "vdec-lat";
+ power-domains = <&spm MT8195_POWER_DOMAIN_VDEC1>;
+ };
+ };
+
larb24: larb@1800d000 {
compatible = "mediatek,mt8195-smi-larb";
reg = <0 0x1800d000 0 0x1000>;
--
2.18.0


2022-11-10 12:01:10

by Yunfei Dong (董云飞)

[permalink] [raw]
Subject: [PATCH v2,2/3] media: dt-bindings: media: mediatek: vcodec: Adding racing control register base

Need to add racing control register base in device node for mt8195 support
inner racing mode. Removing 'maxItems' and adding 'minItems'.

Adding description for each reg.

Signed-off-by: Yunfei Dong <[email protected]>
---
compared with v1:
- add description for 'VDEC_SYS'
- add description for 'VDEC_RACING_CTRL'
- add description for 'VDEC_MISC'
- change maxItems -> minItems according to AngeloGioacchino's suggestion
- Fix dt_binding_check fail
---
.../bindings/media/mediatek,vcodec-subdev-decoder.yaml | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
index 794012853834..9af58db294d3 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
@@ -61,7 +61,10 @@ properties:
- mediatek,mt8195-vcodec-dec

reg:
- maxItems: 1
+ minItems: 1
+ items:
+ - description: VDEC_SYS register space
+ - description: VDEC_RACING_CTRL register space

iommus:
minItems: 1
@@ -115,6 +118,7 @@ patternProperties:

reg:
maxItems: 1
+ description: VDEC_MISC register space

iommus:
minItems: 1
@@ -154,6 +158,7 @@ patternProperties:

reg:
maxItems: 1
+ description: VDEC_MISC register space

interrupts:
maxItems: 1
@@ -195,6 +200,7 @@ patternProperties:

reg:
maxItems: 1
+ description: VDEC_MISC register space

interrupts:
maxItems: 1
--
2.18.0


2022-11-16 17:37:56

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2,2/3] media: dt-bindings: media: mediatek: vcodec: Adding racing control register base


On Thu, 10 Nov 2022 18:28:33 +0800, Yunfei Dong wrote:
> Need to add racing control register base in device node for mt8195 support
> inner racing mode. Removing 'maxItems' and adding 'minItems'.
>
> Adding description for each reg.
>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> compared with v1:
> - add description for 'VDEC_SYS'
> - add description for 'VDEC_RACING_CTRL'
> - add description for 'VDEC_MISC'
> - change maxItems -> minItems according to AngeloGioacchino's suggestion
> - Fix dt_binding_check fail
> ---
> .../bindings/media/mediatek,vcodec-subdev-decoder.yaml | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>

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

2022-11-16 17:59:21

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2,1/3] media: dt-bindings: media: mediatek: vcodec: Fix clock num not correctly

On Thu, Nov 10, 2022 at 06:28:32PM +0800, Yunfei Dong wrote:
> mt8195 and mt8192 have different clock numbers, can't write 'clocks' and
> 'clock-names' with const value.

Not a compatible change. Explain why that is okay if it is.

>
> Move 'assigned-clocks' and 'assigned-clock-parents' to parent node.
>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> .../media/mediatek,vcodec-subdev-decoder.yaml | 119 +++++++++++-------
> 1 file changed, 72 insertions(+), 47 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..794012853834 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> @@ -89,23 +89,33 @@ properties:
>
> ranges: true
>
> + clocks:
> + minItems: 1
> + maxItems: 5
> +
> + clock-names:
> + minItems: 1
> + maxItems: 5

Why do both the parent and child have clocks?

> +
> + assigned-clocks:
> + maxItems: 1
> +
> + assigned-clock-parents:
> + maxItems: 1

You can just drop assigned-clock properties. They are allowed in any
node with 'clocks'.

> +
> # Required child node:
> patternProperties:
> - '^vcodec-lat@[0-9a-f]+$':
> + '^vcodec-lat-soc@[0-9a-f]+$':
> type: object
>
> properties:
> compatible:
> enum:
> - - mediatek,mtk-vcodec-lat
> - mediatek,mtk-vcodec-lat-soc
>
> reg:
> maxItems: 1
>
> - interrupts:
> - maxItems: 1
> -

Dropping interrupts? Not explained in the commit msg (why?).

> iommus:
> minItems: 1
> maxItems: 32
> @@ -114,22 +124,55 @@ patternProperties:
> Refer to bindings/iommu/mediatek,iommu.yaml.
>
> clocks:
> + minItems: 1
> maxItems: 5
>
> clock-names:
> - items:
> - - const: sel
> - - const: soc-vdec
> - - const: soc-lat
> - - const: vdec
> - - const: top
> + minItems: 1
> + maxItems: 5

We had names defined and now we don't. That's a step backwards.

>
> - assigned-clocks:
> + power-domains:

Adding power-domains?

Rob

2022-11-17 03:16:23

by Yunfei Dong (董云飞)

[permalink] [raw]
Subject: Re: [PATCH v2,1/3] media: dt-bindings: media: mediatek: vcodec: Fix clock num not correctly

Hi Rob,

Thanks for your comments.
On Wed, 2022-11-16 at 11:29 -0600, Rob Herring wrote:
> On Thu, Nov 10, 2022 at 06:28:32PM +0800, Yunfei Dong wrote:
> > mt8195 and mt8192 have different clock numbers, can't write
> > 'clocks' and
> > 'clock-names' with const value.
>
> Not a compatible change. Explain why that is okay if it is.
>
This change is used for mt8195 platform for some architecture changed.
Need to separate vcodec-lat with vcodec-lat-soc into different child
node.

At the same time, vcodec-lat-soc don't have interrupt, but having power
domain and clks.
> >
> > Move 'assigned-clocks' and 'assigned-clock-parents' to parent node.
> >
> > Signed-off-by: Yunfei Dong <[email protected]>
> > ---
> > .../media/mediatek,vcodec-subdev-decoder.yaml | 119 +++++++++++---
> > ----
> > 1 file changed, 72 insertions(+), 47 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..794012853834 100644
> > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > subdev-decoder.yaml
> > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > subdev-decoder.yaml
> > @@ -89,23 +89,33 @@ properties:
> >
> > ranges: true
> >
> > + clocks:
> > + minItems: 1
> > + maxItems: 5
> > +
> > + clock-names:
> > + minItems: 1
> > + maxItems: 5
>
> Why do both the parent and child have clocks?
>
If move assigned-clock-parents to child node, need to add 'ssigned-
clock-parents' and 'assigned-clocks' for each child node. Only need to
add one in parent node, child node no need to add if add 'ssigned-
clock-parents' and 'assigned-clocks' in parent node.

Adding 'assigned-clock-parents' and 'assigned-clocks' need to add
'clocks' and 'clock-names', or will check fail.
> > +
> > + assigned-clocks:
> > + maxItems: 1
> > +
> > + assigned-clock-parents:
> > + maxItems: 1
>
> You can just drop assigned-clock properties. They are allowed in any
> node with 'clocks'.
>
Only need to add one in parent node, or need to add for each child
node.
> > +
> > # Required child node:
> > patternProperties:
> > - '^vcodec-lat@[0-9a-f]+$':
> > + '^vcodec-lat-soc@[0-9a-f]+$':
> > type: object
> >
> > properties:
> > compatible:
> > enum:
> > - - mediatek,mtk-vcodec-lat
> > - mediatek,mtk-vcodec-lat-soc
> >
> > reg:
> > maxItems: 1
> >
> > - interrupts:
> > - maxItems: 1
> > -
>
> Dropping interrupts? Not explained in the commit msg (why?).
>
vcodec-lat-soc no need interrupts, will add detail commit message in
next patch.
> > iommus:
> > minItems: 1
> > maxItems: 32
> > @@ -114,22 +124,55 @@ patternProperties:
> > Refer to bindings/iommu/mediatek,iommu.yaml.
> >
> > clocks:
> > + minItems: 1
> > maxItems: 5
> >
> > clock-names:
> > - items:
> > - - const: sel
> > - - const: soc-vdec
> > - - const: soc-lat
> > - - const: vdec
> > - - const: top
> > + minItems: 1
> > + maxItems: 5
>
> We had names defined and now we don't. That's a step backwards.
>
Mt8195/mt8192/mt8186/mt8188 have different clock number and clock
names, so change it like this, do you have any other suggestion?
> >
> > - assigned-clocks:
> > + power-domains:
>
> Adding power-domains?
Vcodec-lat-soc need power domain and add one new child node vcodec-lat-
soc.

Best Regards,
Yunfei Dong
>
> Rob

2022-11-22 02:30:14

by Yunfei Dong (董云飞)

[permalink] [raw]
Subject: Re: [PATCH v2,1/3] media: dt-bindings: media: mediatek: vcodec: Fix clock num not correctly

Hi Rob,

Sorry to disturb you.
Could you please help to check the comments in last mail when you are
free?

Best Regards,
Yunfei Dong

On Thu, 2022-11-17 at 10:16 +0800, yunfei.dong wrote:
> Hi Rob,
>
> Thanks for your comments.
> On Wed, 2022-11-16 at 11:29 -0600, Rob Herring wrote:
> > On Thu, Nov 10, 2022 at 06:28:32PM +0800, Yunfei Dong wrote:
> > > mt8195 and mt8192 have different clock numbers, can't write
> > > 'clocks' and
> > > 'clock-names' with const value.
> >
> > Not a compatible change. Explain why that is okay if it is.
> >
>
> This change is used for mt8195 platform for some architecture
> changed.
> Need to separate vcodec-lat with vcodec-lat-soc into different child
> node.
>
> At the same time, vcodec-lat-soc don't have interrupt, but having
> power
> domain and clks.
> > >
> > > Move 'assigned-clocks' and 'assigned-clock-parents' to parent
> > > node.
> > >
> > > Signed-off-by: Yunfei Dong <[email protected]>
> > > ---
> > > .../media/mediatek,vcodec-subdev-decoder.yaml | 119 +++++++++++-
> > > --
> > > ----
> > > 1 file changed, 72 insertions(+), 47 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..794012853834 100644
> > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > > subdev-decoder.yaml
> > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > > subdev-decoder.yaml
> > > @@ -89,23 +89,33 @@ properties:
> > >
> > > ranges: true
> > >
> > > + clocks:
> > > + minItems: 1
> > > + maxItems: 5
> > > +
> > > + clock-names:
> > > + minItems: 1
> > > + maxItems: 5
> >
> > Why do both the parent and child have clocks?
> >
>
> If move assigned-clock-parents to child node, need to add 'ssigned-
> clock-parents' and 'assigned-clocks' for each child node. Only need
> to
> add one in parent node, child node no need to add if add 'ssigned-
> clock-parents' and 'assigned-clocks' in parent node.
>
> Adding 'assigned-clock-parents' and 'assigned-clocks' need to add
> 'clocks' and 'clock-names', or will check fail.
> > > +
> > > + assigned-clocks:
> > > + maxItems: 1
> > > +
> > > + assigned-clock-parents:
> > > + maxItems: 1
> >
> > You can just drop assigned-clock properties. They are allowed in
> > any
> > node with 'clocks'.
> >
>
> Only need to add one in parent node, or need to add for each child
> node.
> > > +
> > > # Required child node:
> > > patternProperties:
> > > - '^vcodec-lat@[0-9a-f]+$':
> > > + '^vcodec-lat-soc@[0-9a-f]+$':
> > > type: object
> > >
> > > properties:
> > > compatible:
> > > enum:
> > > - - mediatek,mtk-vcodec-lat
> > > - mediatek,mtk-vcodec-lat-soc
> > >
> > > reg:
> > > maxItems: 1
> > >
> > > - interrupts:
> > > - maxItems: 1
> > > -
> >
> > Dropping interrupts? Not explained in the commit msg (why?).
> >
>
> vcodec-lat-soc no need interrupts, will add detail commit message in
> next patch.
> > > iommus:
> > > minItems: 1
> > > maxItems: 32
> > > @@ -114,22 +124,55 @@ patternProperties:
> > > Refer to bindings/iommu/mediatek,iommu.yaml.
> > >
> > > clocks:
> > > + minItems: 1
> > > maxItems: 5
> > >
> > > clock-names:
> > > - items:
> > > - - const: sel
> > > - - const: soc-vdec
> > > - - const: soc-lat
> > > - - const: vdec
> > > - - const: top
> > > + minItems: 1
> > > + maxItems: 5
> >
> > We had names defined and now we don't. That's a step backwards.
> >
>
> Mt8195/mt8192/mt8186/mt8188 have different clock number and clock
> names, so change it like this, do you have any other suggestion?
> > >
> > > - assigned-clocks:
> > > + power-domains:
> >
> > Adding power-domains?
>
> Vcodec-lat-soc need power domain and add one new child node vcodec-
> lat-
> soc.
>
> Best Regards,
> Yunfei Dong
> >
> > Rob

2022-11-23 18:27:47

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH v2,3/3] arm64: dts: mt8195: Add video decoder node

On Thu, Nov 10, 2022 at 06:28:34PM +0800, Yunfei Dong wrote:
> Add video decoder node to mt8195 device tree.
>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> Compared with v1:
> - add description in yaml, and remove /* ... */ for each reg.
> ---
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 63 ++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 905d1a90b406..3ef7eef02415 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -1874,6 +1874,69 @@
> power-domains = <&spm MT8195_POWER_DOMAIN_CAM>;
> };
>
> + video-codec@18000000 {
> + compatible = "mediatek,mt8195-vcodec-dec";
> + mediatek,scp = <&scp>;
> + iommus = <&iommu_vdo M4U_PORT_L21_VDEC_MC_EXT>;
> + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>;

Hi,

similarly to what I've commented for the mt8192 node [1], having dma-ranges
on this node causes IOMMU faults on the latest next. Since the iommu is already
used in this node, please drop the dma-ranges property.

The binding will also need updating to remove this property (given it was marked
as required), so please include that change in either your or Allen's series.

+cc Allen

[1] https://lore.kernel.org/all/20221118141039.y2ap7dzdp26ih2la@notapiano

Thanks,
N?colas

> + #address-cells = <2>;
> + #size-cells = <2>;
> + reg = <0 0x18000000 0 0x1000>,
> + <0 0x18004000 0 0x1000>;
> + ranges = <0 0 0 0x18000000 0 0x26000>;
> + clocks = <&topckgen CLK_TOP_VDEC>,
> + <&topckgen CLK_TOP_UNIVPLL_D4>;
> + clock-names = "vdec-sel", "top";
> + assigned-clocks = <&topckgen CLK_TOP_VDEC>;
> + assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D4>;
> +
> + vcodec-lat-soc@2000 {
> + compatible = "mediatek,mtk-vcodec-lat-soc";
> + reg = <0 0x2000 0 0x800>;
> + iommus = <&iommu_vpp M4U_PORT_L23_VDEC_UFO_ENC_EXT>,
> + <&iommu_vpp M4U_PORT_L23_VDEC_RDMA_EXT>;
> + clocks = <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
> + <&vdecsys_soc CLK_VDEC_SOC_LAT>;
> + clock-names = "vdec-soc-vdec", "vdec-soc-lat";
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDEC0>;
> + };
> +
> + vcodec-lat@10000 {
> + compatible = "mediatek,mtk-vcodec-lat";
> + reg = <0 0x10000 0 0x800>;
> + interrupts = <GIC_SPI 708 IRQ_TYPE_LEVEL_HIGH 0>;
> + iommus = <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_VLD_EXT>,
> + <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_VLD2_EXT>,
> + <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_AVC_MC_EXT>,
> + <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_PRED_RD_EXT>,
> + <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_TILE_EXT>,
> + <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_WDMA_EXT>;
> + clocks = <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
> + <&vdecsys_soc CLK_VDEC_SOC_LAT>;
> + clock-names = "vdec-soc-vdec", "vdec-soc-lat";
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDEC0>;
> + };
> +
> + vcodec-core@25000 {
> + compatible = "mediatek,mtk-vcodec-core";
> + reg = <0 0x25000 0 0x1000>;
> + interrupts = <GIC_SPI 707 IRQ_TYPE_LEVEL_HIGH 0>;
> + iommus = <&iommu_vdo M4U_PORT_L21_VDEC_MC_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_UFO_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_PP_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_PRED_RD_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_PRED_WR_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_PPWRAP_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_TILE_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_VLD_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_VLD2_EXT>,
> + <&iommu_vdo M4U_PORT_L21_VDEC_AVC_MV_EXT>;
> + clocks = <&vdecsys CLK_VDEC_VDEC>, <&vdecsys CLK_VDEC_LAT>;
> + clock-names = "vdec-vdec", "vdec-lat";
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDEC1>;
> + };
> + };
> +
> larb24: larb@1800d000 {
> compatible = "mediatek,mt8195-smi-larb";
> reg = <0 0x1800d000 0 0x1000>;
> --
> 2.18.0
>
>
>