[PATCH v1 0/2] arm64: dts: mediatek: mt8195: Power domains fixes

[PATCH v1 0/2] arm64: dts: mediatek: mt8195: Power domains fixes

[AngeloGioacchino Del Regno]

This series fixes hardware access issues happening on the PCI-Express MAC
and on the video encoder and decoder IPs embedded in the MT8195 SoCs,
fixing system suspend functionality.

AngeloGioacchino Del Regno (2):
  arm64: dts: mediatek: mt8195: Add subsys clks for PCIe power domains
  arm64: dts: mediatek: mt8195: Reparent vdec1/2 and venc1 power domains

 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 54 ++++++++++++++----------
 1 file changed, 31 insertions(+), 23 deletions(-)

-- 
2.48.1

[PATCH v1 1/2] arm64: dts: mediatek: mt8195: Add subsys clks for PCIe power domains

[AngeloGioacchino Del Regno]

The PCIe MAC needs the sram to be powered on for internal IP
access and it has always worked before because the bootloader
on Chromebooks was leaving the PCIe PERI_AO MEM clocks on
before booting the kernel.
Add the SRAM (mem) clock as a subsystem clock on the PCIe MAC
P0 and P1 to correctly describe the hardware and to avoid any
issue with bootloaders behaving differently.

Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index b33726da900b..0cb96cba727a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -792,12 +792,16 @@ power-domain@MT8195_POWER_DOMAIN_CAM_MRAW {
 
 				power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P0 {
 					reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P0>;
+					clocks = <&pericfg_ao CLK_PERI_AO_PCIE_P0_MEM>;
+					clock-names = "ss-pextp0-mem";
 					mediatek,infracfg = <&infracfg_ao>;
 					#power-domain-cells = <0>;
 				};
 
 				power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P1 {
 					reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P1>;
+					clocks = <&pericfg_ao CLK_PERI_AO_PCIE_P1_MEM>;
+					clock-names = "ss-pextp1-mem";
 					mediatek,infracfg = <&infracfg_ao>;
 					#power-domain-cells = <0>;
 				};
-- 
2.48.1

Re: [PATCH v1 1/2] arm64: dts: mediatek: mt8195: Add subsys clks for PCIe power domains

[Chen-Yu Tsai]

On Wed, Apr 2, 2025 at 5:10 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> The PCIe MAC needs the sram to be powered on for internal IP
> access and it has always worked before because the bootloader
> on Chromebooks was leaving the PCIe PERI_AO MEM clocks on
> before booting the kernel.
> Add the SRAM (mem) clock as a subsystem clock on the PCIe MAC
> P0 and P1 to correctly describe the hardware and to avoid any
> issue with bootloaders behaving differently.
>
> Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index b33726da900b..0cb96cba727a 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -792,12 +792,16 @@ power-domain@MT8195_POWER_DOMAIN_CAM_MRAW {
>
>                                 power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P0 {
>                                         reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P0>;
> +                                       clocks = <&pericfg_ao CLK_PERI_AO_PCIE_P0_MEM>;
> +                                       clock-names = "ss-pextp0-mem";

Doesn't the PCIe host controller already reference this clock?

>                                         mediatek,infracfg = <&infracfg_ao>;
>                                         #power-domain-cells = <0>;
>                                 };
>
>                                 power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P1 {
>                                         reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P1>;
> +                                       clocks = <&pericfg_ao CLK_PERI_AO_PCIE_P1_MEM>;
> +                                       clock-names = "ss-pextp1-mem";

Not this one though, since:

         /* Designer has connect pcie1 with peri_mem_p0 clock */
         <&pericfg_ao CLK_PERI_AO_PCIE_P0_MEM>;


ChenYu

>                                         mediatek,infracfg = <&infracfg_ao>;
>                                         #power-domain-cells = <0>;
>                                 };
> --
> 2.48.1
>
>

Re: [PATCH v1 1/2] arm64: dts: mediatek: mt8195: Add subsys clks for PCIe power domains

[AngeloGioacchino Del Regno]

Il 02/04/25 11:34, Chen-Yu Tsai ha scritto:
> On Wed, Apr 2, 2025 at 5:10 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> The PCIe MAC needs the sram to be powered on for internal IP
>> access and it has always worked before because the bootloader
>> on Chromebooks was leaving the PCIe PERI_AO MEM clocks on
>> before booting the kernel.
>> Add the SRAM (mem) clock as a subsystem clock on the PCIe MAC
>> P0 and P1 to correctly describe the hardware and to avoid any
>> issue with bootloaders behaving differently.
>>
>> Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> index b33726da900b..0cb96cba727a 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> @@ -792,12 +792,16 @@ power-domain@MT8195_POWER_DOMAIN_CAM_MRAW {
>>
>>                                  power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P0 {
>>                                          reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P0>;
>> +                                       clocks = <&pericfg_ao CLK_PERI_AO_PCIE_P0_MEM>;
>> +                                       clock-names = "ss-pextp0-mem";
> 
> Doesn't the PCIe host controller already reference this clock?
> 
>>                                          mediatek,infracfg = <&infracfg_ao>;
>>                                          #power-domain-cells = <0>;
>>                                  };
>>
>>                                  power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P1 {
>>                                          reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P1>;
>> +                                       clocks = <&pericfg_ao CLK_PERI_AO_PCIE_P1_MEM>;
>> +                                       clock-names = "ss-pextp1-mem";
> 
> Not this one though, since:
> 
>           /* Designer has connect pcie1 with peri_mem_p0 clock */
>           <&pericfg_ao CLK_PERI_AO_PCIE_P0_MEM>;
> 

I'm not sure what this comment refers to - as in, whether this is referring
to the board designer or to the SoC//IP designer...

...but if MediaTek can clarify, I'd be happy :-)

Cheers,
Angelo

> 
> ChenYu
> 
>>                                          mediatek,infracfg = <&infracfg_ao>;
>>                                          #power-domain-cells = <0>;
>>                                  };
>> --
>> 2.48.1
>>
>>

Re: [PATCH v1 1/2] arm64: dts: mediatek: mt8195: Add subsys clks for PCIe power domains

[Chen-Yu Tsai]

On Wed, Apr 02, 2025 at 12:07:16PM +0200, AngeloGioacchino Del Regno wrote:
> Il 02/04/25 11:34, Chen-Yu Tsai ha scritto:
> > On Wed, Apr 2, 2025 at 5:10 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> > > 
> > > The PCIe MAC needs the sram to be powered on for internal IP
> > > access and it has always worked before because the bootloader
> > > on Chromebooks was leaving the PCIe PERI_AO MEM clocks on
> > > before booting the kernel.
> > > Add the SRAM (mem) clock as a subsystem clock on the PCIe MAC
> > > P0 and P1 to correctly describe the hardware and to avoid any
> > > issue with bootloaders behaving differently.
> > > 
> > > Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
> > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > ---
> > >   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > > index b33726da900b..0cb96cba727a 100644
> > > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > > @@ -792,12 +792,16 @@ power-domain@MT8195_POWER_DOMAIN_CAM_MRAW {
> > > 
> > >                                  power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P0 {
> > >                                          reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P0>;
> > > +                                       clocks = <&pericfg_ao CLK_PERI_AO_PCIE_P0_MEM>;
> > > +                                       clock-names = "ss-pextp0-mem";
> > 
> > Doesn't the PCIe host controller already reference this clock?
> > 
> > >                                          mediatek,infracfg = <&infracfg_ao>;
> > >                                          #power-domain-cells = <0>;
> > >                                  };
> > > 
> > >                                  power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P1 {
> > >                                          reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P1>;
> > > +                                       clocks = <&pericfg_ao CLK_PERI_AO_PCIE_P1_MEM>;
> > > +                                       clock-names = "ss-pextp1-mem";
> > 
> > Not this one though, since:
> > 
> >           /* Designer has connect pcie1 with peri_mem_p0 clock */
> >           <&pericfg_ao CLK_PERI_AO_PCIE_P0_MEM>;
> > 
> 
> I'm not sure what this comment refers to - as in, whether this is referring
> to the board designer or to the SoC//IP designer...

When MediaTek says "designer" they are referring to the hardware designer,
in this case I believe it would be the SoC-level hardware designer /
integrator.

> ...but if MediaTek can clarify, I'd be happy :-)
> 
> Cheers,
> Angelo
> 
> > 
> > ChenYu
> > 
> > >                                          mediatek,infracfg = <&infracfg_ao>;
> > >                                          #power-domain-cells = <0>;
> > >                                  };
> > > --
> > > 2.48.1
> > > 
> > > 
> 

[PATCH v1 2/2] arm64: dts: mediatek: mt8195: Reparent vdec1/2 and venc1 power domains

[AngeloGioacchino Del Regno]

By hardware, the first and second core of the video decoder IP
need the VDEC_SOC to be powered up in order to be able to be
accessed (both internally, by firmware, and externally, by the
kernel).
Similarly, for the video encoder IP, the second core needs the
first core to be powered up in order to be accessible.

Fix that by reparenting the VDEC1/2 power domains to be children
of VDEC0 (VDEC_SOC), and the VENC1 to be a child of VENC0.

Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 50 +++++++++++++-----------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 0cb96cba727a..aeca7eeb4067 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -619,22 +619,6 @@ power-domain@MT8195_POWER_DOMAIN_VPPSYS0 {
 					#size-cells = <0>;
 					#power-domain-cells = <1>;
 
-					power-domain@MT8195_POWER_DOMAIN_VDEC1 {
-						reg = <MT8195_POWER_DOMAIN_VDEC1>;
-						clocks = <&vdecsys CLK_VDEC_LARB1>;
-						clock-names = "vdec1-0";
-						mediatek,infracfg = <&infracfg_ao>;
-						#power-domain-cells = <0>;
-					};
-
-					power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
-						reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
-						clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>;
-						clock-names = "venc1-larb";
-						mediatek,infracfg = <&infracfg_ao>;
-						#power-domain-cells = <0>;
-					};
-
 					power-domain@MT8195_POWER_DOMAIN_VDOSYS0 {
 						reg = <MT8195_POWER_DOMAIN_VDOSYS0>;
 						clocks = <&topckgen CLK_TOP_CFG_VDO0>,
@@ -680,15 +664,25 @@ power-domain@MT8195_POWER_DOMAIN_VDEC0 {
 							clocks = <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
 							clock-names = "vdec0-0";
 							mediatek,infracfg = <&infracfg_ao>;
+							#address-cells = <1>;
+							#size-cells = <0>;
 							#power-domain-cells = <0>;
-						};
 
-						power-domain@MT8195_POWER_DOMAIN_VDEC2 {
-							reg = <MT8195_POWER_DOMAIN_VDEC2>;
-							clocks = <&vdecsys_core1 CLK_VDEC_CORE1_LARB1>;
-							clock-names = "vdec2-0";
-							mediatek,infracfg = <&infracfg_ao>;
-							#power-domain-cells = <0>;
+							power-domain@MT8195_POWER_DOMAIN_VDEC1 {
+								reg = <MT8195_POWER_DOMAIN_VDEC1>;
+								clocks = <&vdecsys CLK_VDEC_LARB1>;
+								clock-names = "vdec1-0";
+								mediatek,infracfg = <&infracfg_ao>;
+								#power-domain-cells = <0>;
+							};
+
+							power-domain@MT8195_POWER_DOMAIN_VDEC2 {
+								reg = <MT8195_POWER_DOMAIN_VDEC2>;
+								clocks = <&vdecsys_core1 CLK_VDEC_CORE1_LARB1>;
+								clock-names = "vdec2-0";
+								mediatek,infracfg = <&infracfg_ao>;
+								#power-domain-cells = <0>;
+							};
 						};
 
 						power-domain@MT8195_POWER_DOMAIN_VENC {
@@ -696,7 +690,17 @@ power-domain@MT8195_POWER_DOMAIN_VENC {
 							clocks = <&vencsys CLK_VENC_LARB>;
 							clock-names = "venc0-larb";
 							mediatek,infracfg = <&infracfg_ao>;
+							#address-cells = <1>;
+							#size-cells = <0>;
 							#power-domain-cells = <0>;
+
+							power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
+								reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
+								clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>;
+								clock-names = "venc1-larb";
+								mediatek,infracfg = <&infracfg_ao>;
+								#power-domain-cells = <0>;
+							};
 						};
 
 						power-domain@MT8195_POWER_DOMAIN_VDOSYS1 {
-- 
2.48.1

Re: [PATCH v1 2/2] arm64: dts: mediatek: mt8195: Reparent vdec1/2 and venc1 power domains

[Chen-Yu Tsai]

On Wed, Apr 2, 2025 at 5:11 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> By hardware, the first and second core of the video decoder IP
> need the VDEC_SOC to be powered up in order to be able to be
> accessed (both internally, by firmware, and externally, by the
> kernel).
> Similarly, for the video encoder IP, the second core needs the
> first core to be powered up in order to be accessible.
>
> Fix that by reparenting the VDEC1/2 power domains to be children
> of VDEC0 (VDEC_SOC), and the VENC1 to be a child of VENC0.
>
> Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Changes look correct. Would need MediaTek to confirm whether the power
domain hierarchy matches what is claimed here.

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

Re: (subset) [PATCH v1 0/2] arm64: dts: mediatek: mt8195: Power domains fixes

[AngeloGioacchino Del Regno]

On Wed, 02 Apr 2025 11:06:13 +0200, AngeloGioacchino Del Regno wrote:
> This series fixes hardware access issues happening on the PCI-Express MAC
> and on the video encoder and decoder IPs embedded in the MT8195 SoCs,
> fixing system suspend functionality.
> 
> AngeloGioacchino Del Regno (2):
>   arm64: dts: mediatek: mt8195: Add subsys clks for PCIe power domains
>   arm64: dts: mediatek: mt8195: Reparent vdec1/2 and venc1 power domains
> 
> [...]

Applied to v6.15-next/dts64, thanks!

[2/2] arm64: dts: mediatek: mt8195: Reparent vdec1/2 and venc1 power domains
      commit: 394f29033324e2317bfd6a7ed99b9a60832b36a2

Cheers,
Angelo