[PATCH 0/6] MT8188 DT and binding fixes

[PATCH 0/6] MT8188 DT and binding fixes

[Fei Shao]

Hi all,

This series is split from a previous series[1] to focus on few fixes and
improvements around MediaTek MT8188 device tree and bindings, and also
addressed comments and carried tags from the mentioned series.

[1]:
https://lore.kernel.org/all/20240909111535.528624-1-fshao@chromium.org/

Regards,
Fei


Fei Shao (6):
  dt-bindings: power: mediatek: Add another nested power-domain layer
  dt-bindings: PCI: mediatek-gen3: Allow exact number of clocks only
  arm64: dts: mediatek: mt8188: Define CPU big core cluster
  arm64: dts: mediatek: mt8188: Add missing dma-ranges to soc node
  arm64: dts: mediatek: mt8188: Move vdec1 power domain under vdec0
  arm64: dts: mediatek: mt8188: Update vppsys node names to syscon

 .../bindings/pci/mediatek-pcie-gen3.yaml      |  5 ++-
 .../power/mediatek,power-controller.yaml      |  4 ++
 arch/arm64/boot/dts/mediatek/mt8188.dtsi      | 37 +++++++++++--------
 3 files changed, 28 insertions(+), 18 deletions(-)

-- 
2.46.0.792.g87dc391469-goog

[PATCH 1/6] dt-bindings: power: mediatek: Add another nested power-domain layer

[Fei Shao]

The MT8188 SoC has a more in-depth power-domain tree, and the
CHECK_DTBS=y check could fail because the current MediaTek power
dt-binding is insufficient to cover its CAM_SUBA and CAM_SUBB
sub-domains.

Add one more nested power-domain layer to pass the check.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Fei Shao <fshao@chromium.org>
---

 .../devicetree/bindings/power/mediatek,power-controller.yaml  | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
index 8985e2df8a56..a7df4041b745 100644
--- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -54,6 +54,10 @@ patternProperties:
             patternProperties:
               "^power-domain@[0-9a-f]+$":
                 $ref: "#/$defs/power-domain-node"
+                patternProperties:
+                  "^power-domain@[0-9a-f]+$":
+                    $ref: "#/$defs/power-domain-node"
+                    unevaluatedProperties: false
                 unevaluatedProperties: false
             unevaluatedProperties: false
         unevaluatedProperties: false
-- 
2.46.0.792.g87dc391469-goog

[PATCH 2/6] dt-bindings: PCI: mediatek-gen3: Allow exact number of clocks only

[Fei Shao]

In MediaTek PCIe gen3 bindings, "clocks" accepts a range of 1-6 clocks
across all SoCs. But in practice, each SoC requires a particular number
of clocks as defined in "clock-names", and the length of "clocks" and
"clock-names" can be inconsistent with current bindings.

For example:
- MT8188, MT8192 and MT8195 all require 6 clocks, while the bindings
  accept 4-6 clocks.
- MT7986 requires 4 clocks, while the bindings accept 4-6 clocks.

Update minItems and maxItems properties for individual SoCs as needed to
only accept the correct number of clocks.

Fixes: c6abd0eadec6 ("dt-bindings: PCI: mediatek-gen3: Add support for Airoha EN7581")
Signed-off-by: Fei Shao <fshao@chromium.org>
---

 .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml          | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
index 898c1be2d6a4..f05aab2b1add 100644
--- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
@@ -149,7 +149,7 @@ allOf:
     then:
       properties:
         clocks:
-          minItems: 4
+          minItems: 6
 
         clock-names:
           items:
@@ -178,7 +178,7 @@ allOf:
     then:
       properties:
         clocks:
-          minItems: 4
+          minItems: 6
 
         clock-names:
           items:
@@ -207,6 +207,7 @@ allOf:
       properties:
         clocks:
           minItems: 4
+          maxItems: 4
 
         clock-names:
           items:
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 2/6] dt-bindings: PCI: mediatek-gen3: Allow exact number of clocks only

[Krzysztof Kozlowski]

On 25/09/2024 12:57, Fei Shao wrote:
> In MediaTek PCIe gen3 bindings, "clocks" accepts a range of 1-6 clocks
> across all SoCs. But in practice, each SoC requires a particular number
> of clocks as defined in "clock-names", and the length of "clocks" and
> "clock-names" can be inconsistent with current bindings.
> 
> For example:
> - MT8188, MT8192 and MT8195 all require 6 clocks, while the bindings
>   accept 4-6 clocks.
> - MT7986 requires 4 clocks, while the bindings accept 4-6 clocks.
> 
> Update minItems and maxItems properties for individual SoCs as needed to
> only accept the correct number of clocks.
> 


Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 2/6] dt-bindings: PCI: mediatek-gen3: Allow exact number of clocks only

[Bjorn Helgaas]

On Wed, Sep 25, 2024 at 06:57:46PM +0800, Fei Shao wrote:
> In MediaTek PCIe gen3 bindings, "clocks" accepts a range of 1-6 clocks
> across all SoCs. But in practice, each SoC requires a particular number
> of clocks as defined in "clock-names", and the length of "clocks" and
> "clock-names" can be inconsistent with current bindings.
> 
> For example:
> - MT8188, MT8192 and MT8195 all require 6 clocks, while the bindings
>   accept 4-6 clocks.
> - MT7986 requires 4 clocks, while the bindings accept 4-6 clocks.
> 
> Update minItems and maxItems properties for individual SoCs as needed to
> only accept the correct number of clocks.
> 
> Fixes: c6abd0eadec6 ("dt-bindings: PCI: mediatek-gen3: Add support for Airoha EN7581")
> Signed-off-by: Fei Shao <fshao@chromium.org>

It looks like most changes to this file have been merged via the PCI
tree.  I don't see dependencies on this in the rest of the series, so
I'm happy to take this via PCI if it makes sense.  Or if you prefer
that this be merged with the rest of the series, that's fine and you
can add my:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Let me know if I should pick this one up.

> ---
> 
>  .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml          | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> index 898c1be2d6a4..f05aab2b1add 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> @@ -149,7 +149,7 @@ allOf:
>      then:
>        properties:
>          clocks:
> -          minItems: 4
> +          minItems: 6
>  
>          clock-names:
>            items:
> @@ -178,7 +178,7 @@ allOf:
>      then:
>        properties:
>          clocks:
> -          minItems: 4
> +          minItems: 6
>  
>          clock-names:
>            items:
> @@ -207,6 +207,7 @@ allOf:
>        properties:
>          clocks:
>            minItems: 4
> +          maxItems: 4
>  
>          clock-names:
>            items:
> -- 
> 2.46.0.792.g87dc391469-goog
> 

Re: [PATCH 2/6] dt-bindings: PCI: mediatek-gen3: Allow exact number of clocks only

[Fei Shao]

On Wed, Oct 2, 2024 at 3:53 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Wed, Sep 25, 2024 at 06:57:46PM +0800, Fei Shao wrote:
> > In MediaTek PCIe gen3 bindings, "clocks" accepts a range of 1-6 clocks
> > across all SoCs. But in practice, each SoC requires a particular number
> > of clocks as defined in "clock-names", and the length of "clocks" and
> > "clock-names" can be inconsistent with current bindings.
> >
> > For example:
> > - MT8188, MT8192 and MT8195 all require 6 clocks, while the bindings
> >   accept 4-6 clocks.
> > - MT7986 requires 4 clocks, while the bindings accept 4-6 clocks.
> >
> > Update minItems and maxItems properties for individual SoCs as needed to
> > only accept the correct number of clocks.
> >
> > Fixes: c6abd0eadec6 ("dt-bindings: PCI: mediatek-gen3: Add support for Airoha EN7581")
> > Signed-off-by: Fei Shao <fshao@chromium.org>
>
> It looks like most changes to this file have been merged via the PCI
> tree.  I don't see dependencies on this in the rest of the series, so
> I'm happy to take this via PCI if it makes sense.  Or if you prefer
> that this be merged with the rest of the series, that's fine and you
> can add my:
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Let me know if I should pick this one up.
>

Yes please, thank you!

Regards,
Fei



> > ---
> >
> >  .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml          | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > index 898c1be2d6a4..f05aab2b1add 100644
> > --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > @@ -149,7 +149,7 @@ allOf:
> >      then:
> >        properties:
> >          clocks:
> > -          minItems: 4
> > +          minItems: 6
> >
> >          clock-names:
> >            items:
> > @@ -178,7 +178,7 @@ allOf:
> >      then:
> >        properties:
> >          clocks:
> > -          minItems: 4
> > +          minItems: 6
> >
> >          clock-names:
> >            items:
> > @@ -207,6 +207,7 @@ allOf:
> >        properties:
> >          clocks:
> >            minItems: 4
> > +          maxItems: 4
> >
> >          clock-names:
> >            items:
> > --
> > 2.46.0.792.g87dc391469-goog
> >

Re: [PATCH 2/6] dt-bindings: PCI: mediatek-gen3: Allow exact number of clocks only

[Bjorn Helgaas]

On Wed, Sep 25, 2024 at 06:57:46PM +0800, Fei Shao wrote:
> In MediaTek PCIe gen3 bindings, "clocks" accepts a range of 1-6 clocks
> across all SoCs. But in practice, each SoC requires a particular number
> of clocks as defined in "clock-names", and the length of "clocks" and
> "clock-names" can be inconsistent with current bindings.
> 
> For example:
> - MT8188, MT8192 and MT8195 all require 6 clocks, while the bindings
>   accept 4-6 clocks.
> - MT7986 requires 4 clocks, while the bindings accept 4-6 clocks.
> 
> Update minItems and maxItems properties for individual SoCs as needed to
> only accept the correct number of clocks.
> 
> Fixes: c6abd0eadec6 ("dt-bindings: PCI: mediatek-gen3: Add support for Airoha EN7581")
> Signed-off-by: Fei Shao <fshao@chromium.org>

This patch only applied to pci/dt-bindings with Krzysztof K's
reviewed-by for v6.13, thank you!

> ---
> 
>  .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml          | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> index 898c1be2d6a4..f05aab2b1add 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> @@ -149,7 +149,7 @@ allOf:
>      then:
>        properties:
>          clocks:
> -          minItems: 4
> +          minItems: 6
>  
>          clock-names:
>            items:
> @@ -178,7 +178,7 @@ allOf:
>      then:
>        properties:
>          clocks:
> -          minItems: 4
> +          minItems: 6
>  
>          clock-names:
>            items:
> @@ -207,6 +207,7 @@ allOf:
>        properties:
>          clocks:
>            minItems: 4
> +          maxItems: 4
>  
>          clock-names:
>            items:
> -- 
> 2.46.0.792.g87dc391469-goog
> 

[PATCH 3/6] arm64: dts: mediatek: mt8188: Define CPU big core cluster

[Fei Shao]

The MT8188 SoC features two CPU clusters: one with 6 little Cortex-A55
cores, and the other with 2 big Cortex-A78 cores.

Update the CPU topology to reflect the actual hardware configurations.

Signed-off-by: Fei Shao <fshao@chromium.org>
---

 arch/arm64/boot/dts/mediatek/mt8188.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
index cd27966d2e3c..51bf08b2ff9b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -195,12 +195,14 @@ core4 {
 				core5 {
 					cpu = <&cpu5>;
 				};
+			};
 
-				core6 {
+			cluster1 {
+				core0 {
 					cpu = <&cpu6>;
 				};
 
-				core7 {
+				core1 {
 					cpu = <&cpu7>;
 				};
 			};
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 3/6] arm64: dts: mediatek: mt8188: Define CPU big core cluster

[AngeloGioacchino Del Regno]

Il 25/09/24 12:57, Fei Shao ha scritto:
> The MT8188 SoC features two CPU clusters: one with 6 little Cortex-A55
> cores, and the other with 2 big Cortex-A78 cores.

No, it doesn't. It features only one cluster, so...

> 
> Update the CPU topology to reflect the actual hardware configurations.

...the actual hardware configuration is already reflected by the currently
declared CPU topology, so for this commit: NAK.

This SoC uses the ARM DynamIQ technology and embeds both LITTLE and big
cores in one single cluster.
Check the MT8188 datasheet for more information :-)

Cheers,
Angelo

> 
> Signed-off-by: Fei Shao <fshao@chromium.org>
> ---
> 
>   arch/arm64/boot/dts/mediatek/mt8188.dtsi | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> index cd27966d2e3c..51bf08b2ff9b 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> @@ -195,12 +195,14 @@ core4 {
>   				core5 {
>   					cpu = <&cpu5>;
>   				};
> +			};
>   
> -				core6 {
> +			cluster1 {
> +				core0 {
>   					cpu = <&cpu6>;
>   				};
>   
> -				core7 {
> +				core1 {
>   					cpu = <&cpu7>;
>   				};
>   			};

Re: [PATCH 3/6] arm64: dts: mediatek: mt8188: Define CPU big core cluster

[Fei Shao]

On Thu, Sep 26, 2024 at 4:33 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 25/09/24 12:57, Fei Shao ha scritto:
> > The MT8188 SoC features two CPU clusters: one with 6 little Cortex-A55
> > cores, and the other with 2 big Cortex-A78 cores.
>
> No, it doesn't. It features only one cluster, so...
>
> >
> > Update the CPU topology to reflect the actual hardware configurations.
>
> ...the actual hardware configuration is already reflected by the currently
> declared CPU topology, so for this commit: NAK.
>
> This SoC uses the ARM DynamIQ technology and embeds both LITTLE and big
> cores in one single cluster.
> Check the MT8188 datasheet for more information :-)

You are absolutely right. I found that description in the datasheet,
and this patch is completely wrong.
Thanks for pointing it out!

Regards,
Fei

>
> Cheers,
> Angelo

[PATCH 4/6] arm64: dts: mediatek: mt8188: Add missing dma-ranges to soc node

[Fei Shao]

Add the missing dma-ranges property to the soc node, similar to how it
was done for MT8195 and MT8192.

This allows the entire 16GB of iova range to be used and enables
multimedia processing usages, like vcodec and MIPI camera.

Signed-off-by: Fei Shao <fshao@chromium.org>
---

 arch/arm64/boot/dts/mediatek/mt8188.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
index 51bf08b2ff9b..ff5c8e0597f9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -880,6 +880,7 @@ soc {
 		#address-cells = <2>;
 		#size-cells = <2>;
 		compatible = "simple-bus";
+		dma-ranges = <0x0 0x0 0x0 0x0 0x4 0x0>;
 		ranges;
 
 		gic: interrupt-controller@c000000 {
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 4/6] arm64: dts: mediatek: mt8188: Add missing dma-ranges to soc node

[AngeloGioacchino Del Regno]

Il 25/09/24 12:57, Fei Shao ha scritto:
> Add the missing dma-ranges property to the soc node, similar to how it
> was done for MT8195 and MT8192.
> 
> This allows the entire 16GB of iova range to be used and enables
> multimedia processing usages, like vcodec and MIPI camera.
> 
> Signed-off-by: Fei Shao <fshao@chromium.org>

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

[PATCH 5/6] arm64: dts: mediatek: mt8188: Move vdec1 power domain under vdec0

[Fei Shao]

There are two hardware IP blocks in MT8188 video decoder pipeline:
vdec-lat and vdec-core, which are powered by vdec0 and vdec1 power
domains respectively.

We noticed that vdec-core needs to be powered down before vdec-lat
during suspend to prevent failures. It's unclear if it's an intended
hardware design or due to power isolation glitch. But in any case, we
observed a power-off sequence here, and it can be considered as an
indirect dependency implication between the vdec0 and vdec1 domains.

Given that, update vdec1 as a sub-domain of vdec0 to enforce the
sequence. Also, use more specific clock names for both power domains.

Signed-off-by: Fei Shao <fshao@chromium.org>
---

 arch/arm64/boot/dts/mediatek/mt8188.dtsi | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
index ff5c8e0597f9..a6cd08ea74eb 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -1064,20 +1064,22 @@ power-domain@MT8188_POWER_DOMAIN_VPPSYS1 {
 							#power-domain-cells = <0>;
 						};
 
-						power-domain@MT8188_POWER_DOMAIN_VDEC1 {
-							reg = <MT8188_POWER_DOMAIN_VDEC1>;
-							clocks = <&vdecsys CLK_VDEC2_LARB1>;
-							clock-names = "ss-vdec";
-							mediatek,infracfg = <&infracfg_ao>;
-							#power-domain-cells = <0>;
-						};
-
 						power-domain@MT8188_POWER_DOMAIN_VDEC0 {
 							reg = <MT8188_POWER_DOMAIN_VDEC0>;
 							clocks = <&vdecsys_soc CLK_VDEC1_SOC_LARB1>;
-							clock-names = "ss-vdec";
+							clock-names = "ss-vdec1-soc-l1";
 							mediatek,infracfg = <&infracfg_ao>;
-							#power-domain-cells = <0>;
+							#address-cells = <1>;
+							#size-cells = <0>;
+							#power-domain-cells = <1>;
+
+							power-domain@MT8188_POWER_DOMAIN_VDEC1 {
+								reg = <MT8188_POWER_DOMAIN_VDEC1>;
+								clocks = <&vdecsys CLK_VDEC2_LARB1>;
+								clock-names = "ss-vdec2-l1";
+								mediatek,infracfg = <&infracfg_ao>;
+								#power-domain-cells = <0>;
+							};
 						};
 
 						cam_vcore: power-domain@MT8188_POWER_DOMAIN_CAM_VCORE {
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 5/6] arm64: dts: mediatek: mt8188: Move vdec1 power domain under vdec0

[AngeloGioacchino Del Regno]

Il 25/09/24 12:57, Fei Shao ha scritto:
> There are two hardware IP blocks in MT8188 video decoder pipeline:
> vdec-lat and vdec-core, which are powered by vdec0 and vdec1 power
> domains respectively.
> 
> We noticed that vdec-core needs to be powered down before vdec-lat
> during suspend to prevent failures. It's unclear if it's an intended
> hardware design or due to power isolation glitch. But in any case, we
> observed a power-off sequence here, and it can be considered as an
> indirect dependency implication between the vdec0 and vdec1 domains.
> 
> Given that, update vdec1 as a sub-domain of vdec0 to enforce the
> sequence. Also, use more specific clock names for both power domains.
> 

As far as I know, yes, there is a sequence:
  - Cores (mtk-vcodec-core) gets suspended first
  - Then the LATs gets suspended (mtk-vcodec-lat)
  - Finally, the LAT SoC gets suspended (mtk-vcodec-lat-soc)

...but you checked that downstream, and your downstream misses the lat-soc HW
instance, and only has the lat one.

Are you sure that this is not the reason why you're getting this issue? :-)

Otherwise, I feel like we must ask for some clarification from MediaTek, as
I'm mostly sure that the two cores are independent from each other (but I
might, of course, be wrong!).

Cheers,
Angelo

> Signed-off-by: Fei Shao <fshao@chromium.org>
> ---
> 
>   arch/arm64/boot/dts/mediatek/mt8188.dtsi | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> index ff5c8e0597f9..a6cd08ea74eb 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> @@ -1064,20 +1064,22 @@ power-domain@MT8188_POWER_DOMAIN_VPPSYS1 {
>   							#power-domain-cells = <0>;
>   						};
>   
> -						power-domain@MT8188_POWER_DOMAIN_VDEC1 {
> -							reg = <MT8188_POWER_DOMAIN_VDEC1>;
> -							clocks = <&vdecsys CLK_VDEC2_LARB1>;
> -							clock-names = "ss-vdec";
> -							mediatek,infracfg = <&infracfg_ao>;
> -							#power-domain-cells = <0>;
> -						};
> -
>   						power-domain@MT8188_POWER_DOMAIN_VDEC0 {
>   							reg = <MT8188_POWER_DOMAIN_VDEC0>;
>   							clocks = <&vdecsys_soc CLK_VDEC1_SOC_LARB1>;
> -							clock-names = "ss-vdec";
> +							clock-names = "ss-vdec1-soc-l1";
>   							mediatek,infracfg = <&infracfg_ao>;
> -							#power-domain-cells = <0>;
> +							#address-cells = <1>;
> +							#size-cells = <0>;
> +							#power-domain-cells = <1>;
> +
> +							power-domain@MT8188_POWER_DOMAIN_VDEC1 {
> +								reg = <MT8188_POWER_DOMAIN_VDEC1>;
> +								clocks = <&vdecsys CLK_VDEC2_LARB1>;
> +								clock-names = "ss-vdec2-l1";
> +								mediatek,infracfg = <&infracfg_ao>;
> +								#power-domain-cells = <0>;
> +							};
>   						};
>   
>   						cam_vcore: power-domain@MT8188_POWER_DOMAIN_CAM_VCORE {

Re: [PATCH 5/6] arm64: dts: mediatek: mt8188: Move vdec1 power domain under vdec0

[Fei Shao]

On Thu, Sep 26, 2024 at 4:33 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 25/09/24 12:57, Fei Shao ha scritto:
> > There are two hardware IP blocks in MT8188 video decoder pipeline:
> > vdec-lat and vdec-core, which are powered by vdec0 and vdec1 power
> > domains respectively.
> >
> > We noticed that vdec-core needs to be powered down before vdec-lat
> > during suspend to prevent failures. It's unclear if it's an intended
> > hardware design or due to power isolation glitch. But in any case, we
> > observed a power-off sequence here, and it can be considered as an
> > indirect dependency implication between the vdec0 and vdec1 domains.
> >
> > Given that, update vdec1 as a sub-domain of vdec0 to enforce the
> > sequence. Also, use more specific clock names for both power domains.
> >
>
> As far as I know, yes, there is a sequence:
>   - Cores (mtk-vcodec-core) gets suspended first
>   - Then the LATs gets suspended (mtk-vcodec-lat)
>   - Finally, the LAT SoC gets suspended (mtk-vcodec-lat-soc)
>
> ...but you checked that downstream, and your downstream misses the lat-soc HW
> instance, and only has the lat one.
>
> Are you sure that this is not the reason why you're getting this issue? :-)
>
> Otherwise, I feel like we must ask for some clarification from MediaTek, as
> I'm mostly sure that the two cores are independent from each other (but I
> might, of course, be wrong!).

Yes I think I should... this is actually based on a downstream patch of theirs.
My understanding is that LAT SoC is not always in the vdec pipeline
for every MediaTek SoCs. Although the MT8188 and MT8195 have much in
common, I have a vague impression that MT8188 doesn't have a LAT SoC
HW, so the downstream video decoding works smoothly without describing
that in DT... but still, I could be wrong, and things just happen to work.

Anyway, I'll find someone on the MediaTek side for clarification. The
datasheet I have doesn't seem to contain such information.

Regards,
Fei

>
> Cheers,
> Angelo

Re: [PATCH 5/6] arm64: dts: mediatek: mt8188: Move vdec1 power domain under vdec0

[Fei Shao]

On Thu, Sep 26, 2024 at 6:42 PM Fei Shao <fshao@chromium.org> wrote:
>
> On Thu, Sep 26, 2024 at 4:33 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
> >
> > Il 25/09/24 12:57, Fei Shao ha scritto:
> > > There are two hardware IP blocks in MT8188 video decoder pipeline:
> > > vdec-lat and vdec-core, which are powered by vdec0 and vdec1 power
> > > domains respectively.
> > >
> > > We noticed that vdec-core needs to be powered down before vdec-lat
> > > during suspend to prevent failures. It's unclear if it's an intended
> > > hardware design or due to power isolation glitch. But in any case, we
> > > observed a power-off sequence here, and it can be considered as an
> > > indirect dependency implication between the vdec0 and vdec1 domains.
> > >
> > > Given that, update vdec1 as a sub-domain of vdec0 to enforce the
> > > sequence. Also, use more specific clock names for both power domains.
> > >
> >
> > As far as I know, yes, there is a sequence:
> >   - Cores (mtk-vcodec-core) gets suspended first
> >   - Then the LATs gets suspended (mtk-vcodec-lat)
> >   - Finally, the LAT SoC gets suspended (mtk-vcodec-lat-soc)
> >
> > ...but you checked that downstream, and your downstream misses the lat-soc HW
> > instance, and only has the lat one.
> >
> > Are you sure that this is not the reason why you're getting this issue? :-)
> >
> > Otherwise, I feel like we must ask for some clarification from MediaTek, as
> > I'm mostly sure that the two cores are independent from each other (but I
> > might, of course, be wrong!).
>
> Yes I think I should... this is actually based on a downstream patch of theirs.
> My understanding is that LAT SoC is not always in the vdec pipeline
> for every MediaTek SoCs. Although the MT8188 and MT8195 have much in
> common, I have a vague impression that MT8188 doesn't have a LAT SoC
> HW, so the downstream video decoding works smoothly without describing
> that in DT... but still, I could be wrong, and things just happen to work.
>
> Anyway, I'll find someone on the MediaTek side for clarification. The
> datasheet I have doesn't seem to contain such information.
>

MediaTek confirmed that MT8188 doesn't have a LAT SoC block. Its vdec
pipeline is (mostly if not exactly) the same as MT8192 which is
composed of Core + LAT only.
Also, there *is* a hardware dependency between Core and LAT's power
domains in both MT8192 and MT8188.
And for reference, MT8192 DT described that dependency correctly. That
suggests how the power domain tree should be like in MT8188 DT.

I plan to send a v3 of this series to include more fixs I found last
week and exclude the invalid patch, and I'll refine the commit message
of this patch all together. I'll also update the bindings to document
the information above so people can reference that easier in the
future.

Regards,
Fei

[PATCH 6/6] arm64: dts: mediatek: mt8188: Update vppsys node names to syscon

[Fei Shao]

The MediaTek mmsys is more than just a clock controller; it's a
system controller. In addition to clock controls, it provides display
pipeline routing controls and other miscellaneous control registers.

On the MT8188 and MT8195 SoCs, the mmsys blocks utilize the same mmsys
driver but have been aliased to "vdosys" and "vppsys", likely to better
represent their actual functionality.

Update the vppsys node names and compatibles in MT8188 DT to reflect
that and fix dtbs_check errors against mediatek/mt8188-evb.dtb.

Signed-off-by: Fei Shao <fshao@chromium.org>
---

 arch/arm64/boot/dts/mediatek/mt8188.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
index a6cd08ea74eb..98ba3485a8bd 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -1783,8 +1783,8 @@ mfgcfg: clock-controller@13fbf000 {
 			#clock-cells = <1>;
 		};
 
-		vppsys0: clock-controller@14000000 {
-			compatible = "mediatek,mt8188-vppsys0";
+		vppsys0: syscon@14000000 {
+			compatible = "mediatek,mt8188-vppsys0", "syscon";
 			reg = <0 0x14000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
@@ -1801,8 +1801,8 @@ wpesys_vpp0: clock-controller@14e02000 {
 			#clock-cells = <1>;
 		};
 
-		vppsys1: clock-controller@14f00000 {
-			compatible = "mediatek,mt8188-vppsys1";
+		vppsys1: syscon@14f00000 {
+			compatible = "mediatek,mt8188-vppsys1", "syscon";
 			reg = <0 0x14f00000 0 0x1000>;
 			#clock-cells = <1>;
 		};
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 6/6] arm64: dts: mediatek: mt8188: Update vppsys node names to syscon

[AngeloGioacchino Del Regno]

Il 25/09/24 12:57, Fei Shao ha scritto:
> The MediaTek mmsys is more than just a clock controller; it's a
> system controller. In addition to clock controls, it provides display
> pipeline routing controls and other miscellaneous control registers.
> 
> On the MT8188 and MT8195 SoCs, the mmsys blocks utilize the same mmsys
> driver but have been aliased to "vdosys" and "vppsys", likely to better
> represent their actual functionality.
> 
> Update the vppsys node names and compatibles in MT8188 DT to reflect
> that and fix dtbs_check errors against mediatek/mt8188-evb.dtb.
> 
> Signed-off-by: Fei Shao <fshao@chromium.org>

Fair point.

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