On Thu, Oct 5, 2023 at 6:31 PM Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> wrote: > > Clocks for each power domain are split into big categories: pd clocks > and subsys clocks. > According to the binding, all clocks which have a dash '-' in their name > are treated as subsys clocks, and must be placed at the end of the list. > The other clocks which are pd clocks must come first. > Fixed the naming and the placing of all clocks in the power domains. > For the avoidance of doubt, prefixed all subsys clocks with the 'subsys' > prefix. The binding does not enforce strict clock names, the driver > uses them in bulk, only making a difference for pd clocks vs subsys clocks. > > The above problem appears to be trivial, however, it leads to incorrect > power up and power down sequence of the power domains, because some > clocks will be mistakenly taken for subsys clocks and viceversa. > One consequence is the fact that if the DIS power domain goes power down > and power back up during the boot process, when it comes back up, there > are still transactions left on the bus which makes the display inoperable. > > Some of the clocks for the DIS power domain were wrongly using '_' instead > of '-', which again made these clocks being treated as pd clocks instead of > subsys clocks. > > Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller") > Signed-off-by: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> Tested-by: Chen-Yu Tsai <wenst@xxxxxxxxxxxx> This brings the display back to life on my MT8186 device. :D Thank you for tracking down the issue! > --- > arch/arm64/boot/dts/mediatek/mt8186.dtsi | 42 +++++++++++++++--------- > 1 file changed, 27 insertions(+), 15 deletions(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi > index af6f6687de35..7121d4312bee 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi > @@ -924,7 +924,8 @@ power-domain@MT8186_POWER_DOMAIN_CSIRX_TOP { > reg = <MT8186_POWER_DOMAIN_CSIRX_TOP>; > clocks = <&topckgen CLK_TOP_SENINF>, > <&topckgen CLK_TOP_SENINF1>; > - clock-names = "csirx_top0", "csirx_top1"; > + clock-names = "subsys-csirx-top0", > + "subsys-csirx-top1"; > #power-domain-cells = <0>; > }; > > @@ -942,7 +943,8 @@ power-domain@MT8186_POWER_DOMAIN_ADSP_AO { > reg = <MT8186_POWER_DOMAIN_ADSP_AO>; > clocks = <&topckgen CLK_TOP_AUDIODSP>, > <&topckgen CLK_TOP_ADSP_BUS>; > - clock-names = "audioadsp", "adsp_bus"; > + clock-names = "audioadsp", > + "subsys-adsp-bus"; > #address-cells = <1>; > #size-cells = <0>; > #power-domain-cells = <1>; > @@ -975,8 +977,11 @@ power-domain@MT8186_POWER_DOMAIN_DIS { > <&mmsys CLK_MM_SMI_COMMON>, > <&mmsys CLK_MM_SMI_GALS>, > <&mmsys CLK_MM_SMI_IOMMU>; > - clock-names = "disp", "mdp", "smi_infra", "smi_common", > - "smi_gals", "smi_iommu"; > + clock-names = "disp", "mdp", > + "subsys-smi-infra", > + "subsys-smi-common", > + "subsys-smi-gals", > + "subsys-smi-iommu"; > mediatek,infracfg = <&infracfg_ao>; > #address-cells = <1>; > #size-cells = <0>; > @@ -993,15 +998,17 @@ power-domain@MT8186_POWER_DOMAIN_VDEC { > > power-domain@MT8186_POWER_DOMAIN_CAM { > reg = <MT8186_POWER_DOMAIN_CAM>; > - clocks = <&topckgen CLK_TOP_CAM>, > - <&topckgen CLK_TOP_SENINF>, > + clocks = <&topckgen CLK_TOP_SENINF>, > <&topckgen CLK_TOP_SENINF1>, > <&topckgen CLK_TOP_SENINF2>, > <&topckgen CLK_TOP_SENINF3>, > + <&camsys CLK_CAM2MM_GALS>, > <&topckgen CLK_TOP_CAMTM>, > - <&camsys CLK_CAM2MM_GALS>; > - clock-names = "cam-top", "cam0", "cam1", "cam2", > - "cam3", "cam-tm", "gals"; > + <&topckgen CLK_TOP_CAM>; > + clock-names = "cam0", "cam1", "cam2", > + "cam3", "gals", > + "subsys-cam-tm", > + "subsys-cam-top"; > mediatek,infracfg = <&infracfg_ao>; > #address-cells = <1>; > #size-cells = <0>; > @@ -1020,9 +1027,9 @@ power-domain@MT8186_POWER_DOMAIN_CAM_RAWA { > > power-domain@MT8186_POWER_DOMAIN_IMG { > reg = <MT8186_POWER_DOMAIN_IMG>; > - clocks = <&topckgen CLK_TOP_IMG1>, > - <&imgsys1 CLK_IMG1_GALS_IMG1>; > - clock-names = "img-top", "gals"; > + clocks = <&imgsys1 CLK_IMG1_GALS_IMG1>, > + <&topckgen CLK_TOP_IMG1>; > + clock-names = "gals", "subsys-img-top"; > mediatek,infracfg = <&infracfg_ao>; > #address-cells = <1>; > #size-cells = <0>; > @@ -1041,8 +1048,11 @@ power-domain@MT8186_POWER_DOMAIN_IPE { > <&ipesys CLK_IPE_LARB20>, > <&ipesys CLK_IPE_SMI_SUBCOM>, > <&ipesys CLK_IPE_GALS_IPE>; > - clock-names = "ipe-top", "ipe-larb0", "ipe-larb1", > - "ipe-smi", "ipe-gals"; > + clock-names = "subsys-ipe-top", > + "subsys-ipe-larb0", > + "subsys-ipe-larb1", > + "subsys-ipe-smi", > + "subsys-ipe-gals"; > mediatek,infracfg = <&infracfg_ao>; > #power-domain-cells = <0>; > }; > @@ -1061,7 +1071,9 @@ power-domain@MT8186_POWER_DOMAIN_WPE { > clocks = <&topckgen CLK_TOP_WPE>, > <&wpesys CLK_WPE_SMI_LARB8_CK_EN>, > <&wpesys CLK_WPE_SMI_LARB8_PCLK_EN>; > - clock-names = "wpe0", "larb-ck", "larb-pclk"; > + clock-names = "wpe0", > + "subsys-larb-ck", > + "subsys-larb-pclk"; > mediatek,infracfg = <&infracfg_ao>; > #power-domain-cells = <0>; > }; > -- > 2.34.1 > >