[PATCH v2 00/11] Clock fixes for MSM8998 GCC, MMCC, GPUCC

[PATCH v2 00/11] Clock fixes for MSM8998 GCC, MMCC, GPUCC

[AngeloGioacchino Del Regno]

This patch series fixes some issues with the MSM8998 clocks and, in
particular, brings a very important fix to the GCC PLLs.

These fixes are enhancing this SoC's stability and also makes it
possible to eventually enable the Adreno GPU (with proper clock
scaling) and other components.

This patch series was tested on:
- Sony Xperia XZ Premium (MSM8998)
- F(x)Tec Pro1 (MSM8998)

AngeloGioacchino Del Regno (11):
  dt-bindings: clocks: gcc-msm8998: Add GCC_MMSS_GPLL0_CLK definition
  clk: qcom: gcc-msm8998: Wire up gcc_mmss_gpll0 clock
  dt-bindings: clock: gcc-msm8998: Add HMSS_GPLL0_CLK_SRC definition
  clk: qcom: gcc-msm8998: Add missing hmss_gpll0_clk_src clock
  clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical
  clk: qcom: gcc-msm8998: Fix Alpha PLL type for all GPLLs
  clk: qcom: mmcc-msm8998: Set CLK_GET_RATE_NOCACHE to pixel/byte clks
  clk: qcom: mmcc-msm8998: Add hardware clockgating registers to some
    clks
  clk: qcom: mmcc-msm8998: Set bimc_smmu_gdsc always on
  clk: qcom: gpucc-msm8998: Add resets, cxc, fix flags on gpu_gx_gdsc
  clk: qcom: gpucc-msm8998: Allow fabia gpupll0 rate setting

 drivers/clk/qcom/gcc-msm8998.c               | 143 ++++++++++++-------
 drivers/clk/qcom/gpucc-msm8998.c             |  18 ++-
 drivers/clk/qcom/mmcc-msm8998.c              |  20 ++-
 include/dt-bindings/clock/qcom,gcc-msm8998.h |   2 +
 4 files changed, 125 insertions(+), 58 deletions(-)

-- 
2.29.2

[PATCH v2 01/11] dt-bindings: clocks: gcc-msm8998: Add GCC_MMSS_GPLL0_CLK definition

[AngeloGioacchino Del Regno]

Add new clock definition to gcc-msm8998 dt-bindings.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 include/dt-bindings/clock/qcom,gcc-msm8998.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/qcom,gcc-msm8998.h b/include/dt-bindings/clock/qcom,gcc-msm8998.h
index 6a73a174f049..47ca17df780b 100644
--- a/include/dt-bindings/clock/qcom,gcc-msm8998.h
+++ b/include/dt-bindings/clock/qcom,gcc-msm8998.h
@@ -184,6 +184,7 @@
 #define GCC_MSS_MNOC_BIMC_AXI_CLK				175
 #define GCC_BIMC_GFX_CLK					176
 #define UFS_UNIPRO_CORE_CLK_SRC					177
+#define GCC_MMSS_GPLL0_CLK					178
 
 #define PCIE_0_GDSC						0
 #define UFS_GDSC						1
-- 
2.29.2

Re: [PATCH v2 01/11] dt-bindings: clocks: gcc-msm8998: Add GCC_MMSS_GPLL0_CLK definition

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:49)
> Add new clock definition to gcc-msm8998 dt-bindings.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 02/11] clk: qcom: gcc-msm8998: Wire up gcc_mmss_gpll0 clock

[AngeloGioacchino Del Regno]

This clock enables the GPLL0 output to the multimedia subsystem
clock controller.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/gcc-msm8998.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index 9d7016bcd680..d51c556851ca 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -1341,6 +1341,22 @@ static struct clk_branch gcc_boot_rom_ahb_clk = {
 	},
 };
 
+static struct clk_branch gcc_mmss_gpll0_clk = {
+	.halt_check = BRANCH_HALT_DELAY,
+	.clkr = {
+		.enable_reg = 0x5200c,
+		.enable_mask = BIT(1),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_mmss_gpll0_clk",
+			.parent_names = (const char *[]){
+				"gpll0_out_main",
+			},
+			.num_parents = 1,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_mss_gpll0_div_clk_src = {
 	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
@@ -2944,6 +2960,7 @@ static struct clk_regmap *gcc_msm8998_clocks[] = {
 	[GCC_MSS_GPLL0_DIV_CLK_SRC] = &gcc_mss_gpll0_div_clk_src.clkr,
 	[GCC_MSS_SNOC_AXI_CLK] = &gcc_mss_snoc_axi_clk.clkr,
 	[GCC_MSS_MNOC_BIMC_AXI_CLK] = &gcc_mss_mnoc_bimc_axi_clk.clkr,
+	[GCC_MMSS_GPLL0_CLK] = &gcc_mmss_gpll0_clk.clkr,
 };
 
 static struct gdsc *gcc_msm8998_gdscs[] = {
-- 
2.29.2

Re: [PATCH v2 02/11] clk: qcom: gcc-msm8998: Wire up gcc_mmss_gpll0 clock

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:50)
> This clock enables the GPLL0 output to the multimedia subsystem
> clock controller.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 03/11] dt-bindings: clock: gcc-msm8998: Add HMSS_GPLL0_CLK_SRC definition

[AngeloGioacchino Del Regno]

Add new clock definition to gcc-msm8998 dt-bindings

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 include/dt-bindings/clock/qcom,gcc-msm8998.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/qcom,gcc-msm8998.h b/include/dt-bindings/clock/qcom,gcc-msm8998.h
index 47ca17df780b..72c99e486d86 100644
--- a/include/dt-bindings/clock/qcom,gcc-msm8998.h
+++ b/include/dt-bindings/clock/qcom,gcc-msm8998.h
@@ -185,6 +185,7 @@
 #define GCC_BIMC_GFX_CLK					176
 #define UFS_UNIPRO_CORE_CLK_SRC					177
 #define GCC_MMSS_GPLL0_CLK					178
+#define HMSS_GPLL0_CLK_SRC					179
 
 #define PCIE_0_GDSC						0
 #define UFS_GDSC						1
-- 
2.29.2

Re: [PATCH v2 03/11] dt-bindings: clock: gcc-msm8998: Add HMSS_GPLL0_CLK_SRC definition

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:51)
> Add new clock definition to gcc-msm8998 dt-bindings
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 04/11] clk: qcom: gcc-msm8998: Add missing hmss_gpll0_clk_src clock

[AngeloGioacchino Del Regno]

To achieve CPR-Hardened functionality this clock must be on: add it
in order to be able to get it managed by the CPR3 driver.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/gcc-msm8998.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index d51c556851ca..c8d4c0348952 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -2160,6 +2160,25 @@ static struct clk_branch gcc_hmss_trig_clk = {
 	},
 };
 
+static struct freq_tbl ftbl_hmss_gpll0_clk_src[] = {
+	F( 300000000, P_GPLL0_OUT_MAIN, 2, 0, 0),
+	F( 600000000, P_GPLL0_OUT_MAIN, 1, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 hmss_gpll0_clk_src = {
+	.cmd_rcgr = 0x4805c,
+	.hid_width = 5,
+	.parent_map = gcc_parent_map_1,
+	.freq_tbl = ftbl_hmss_gpll0_clk_src,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "hmss_gpll0_clk_src",
+		.parent_names = gcc_parent_names_1,
+		.num_parents = ARRAY_SIZE(gcc_parent_names_1),
+		.ops = &clk_rcg2_ops,
+	},
+};
+
 static struct clk_branch gcc_mmss_noc_cfg_ahb_clk = {
 	.halt_reg = 0x9004,
 	.halt_check = BRANCH_HALT,
@@ -2961,6 +2980,7 @@ static struct clk_regmap *gcc_msm8998_clocks[] = {
 	[GCC_MSS_SNOC_AXI_CLK] = &gcc_mss_snoc_axi_clk.clkr,
 	[GCC_MSS_MNOC_BIMC_AXI_CLK] = &gcc_mss_mnoc_bimc_axi_clk.clkr,
 	[GCC_MMSS_GPLL0_CLK] = &gcc_mmss_gpll0_clk.clkr,
+	[HMSS_GPLL0_CLK_SRC] = &hmss_gpll0_clk_src.clkr,
 };
 
 static struct gdsc *gcc_msm8998_gdscs[] = {
-- 
2.29.2

Re: [PATCH v2 04/11] clk: qcom: gcc-msm8998: Add missing hmss_gpll0_clk_src clock

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:52)
> To achieve CPR-Hardened functionality this clock must be on: add it
> in order to be able to get it managed by the CPR3 driver.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 05/11] clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical

[AngeloGioacchino Del Regno]

The GPU IOMMU depends on this clock and the hypervisor will crash
the SoC if this clock gets disabled because the secure contexts
that have been set on this IOMMU by the bootloader will become
unaccessible (or they get reset).
Mark this clock as critical to avoid this issue when the Adreno
GPU is enabled.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/gcc-msm8998.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index c8d4c0348952..afea60a3ef43 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -2081,6 +2081,12 @@ static struct clk_branch gcc_gpu_cfg_ahb_clk = {
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_gpu_cfg_ahb_clk",
 			.ops = &clk_branch2_ops,
+			/*
+			 * The GPU IOMMU depends on this clock and hypervisor
+			 * will crash the SoC if this clock goes down, due to
+			 * secure contexts protection.
+			 */
+			.flags = CLK_IS_CRITICAL,
 		},
 	},
 };
-- 
2.29.2

Re: [PATCH v2 05/11] clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical

[Jeffrey Hugo]

On Thu, Jan 14, 2021 at 3:13 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@somainline.org> wrote:
>
> The GPU IOMMU depends on this clock and the hypervisor will crash
> the SoC if this clock gets disabled because the secure contexts
> that have been set on this IOMMU by the bootloader will become
> unaccessible (or they get reset).
> Mark this clock as critical to avoid this issue when the Adreno
> GPU is enabled.
>

You should go review the last attempt to do this -
https://lkml.org/lkml/2019/12/17/881

Re: [PATCH v2 05/11] clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical

[AngeloGioacchino Del Regno]

Il 14/01/21 23:37, Jeffrey Hugo ha scritto:
> On Thu, Jan 14, 2021 at 3:13 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@somainline.org> wrote:
>>
>> The GPU IOMMU depends on this clock and the hypervisor will crash
>> the SoC if this clock gets disabled because the secure contexts
>> that have been set on this IOMMU by the bootloader will become
>> unaccessible (or they get reset).
>> Mark this clock as critical to avoid this issue when the Adreno
>> GPU is enabled.
>>
> 
> You should go review the last attempt to do this -
> https://lkml.org/lkml/2019/12/17/881
> 

Thanks for the tip, but unfortunately this isn't possible on the 
gpu_cfg_ahb_clk, as it is also needed for the Adreno IOMMU, which has 
secure contexts that are set up from one of the bootloader stages and if 
you reset/"mess up" one of them (by - in this case - un-clocking the 
MMU), then the hypervisor will kick in and generate a fault, rebooting 
the SoC.

Of course, this scenario is for the case in which you want to boot the 
device without any gpucc nor any runtime pm user of that.. and the 
aforementioned issue makes that solution not really usable.

Again, unfortunately.

Re: [PATCH v2 05/11] clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical

[Jeffrey Hugo]

On Thu, Jan 14, 2021 at 4:05 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@somainline.org> wrote:
>
> Il 14/01/21 23:37, Jeffrey Hugo ha scritto:
> > On Thu, Jan 14, 2021 at 3:13 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@somainline.org> wrote:
> >>
> >> The GPU IOMMU depends on this clock and the hypervisor will crash
> >> the SoC if this clock gets disabled because the secure contexts
> >> that have been set on this IOMMU by the bootloader will become
> >> unaccessible (or they get reset).
> >> Mark this clock as critical to avoid this issue when the Adreno
> >> GPU is enabled.
> >>
> >
> > You should go review the last attempt to do this -
> > https://lkml.org/lkml/2019/12/17/881
> >
>
> Thanks for the tip, but unfortunately this isn't possible on the
> gpu_cfg_ahb_clk, as it is also needed for the Adreno IOMMU, which has
> secure contexts that are set up from one of the bootloader stages and if
> you reset/"mess up" one of them (by - in this case - un-clocking the
> MMU), then the hypervisor will kick in and generate a fault, rebooting
> the SoC.
>
> Of course, this scenario is for the case in which you want to boot the
> device without any gpucc nor any runtime pm user of that.. and the
> aforementioned issue makes that solution not really usable.
>
> Again, unfortunately.

Intresting, that's not true on all devices, but presumably you have
devices where it is.  Fun.

Re: [PATCH v2 05/11] clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:53)
> The GPU IOMMU depends on this clock and the hypervisor will crash
> the SoC if this clock gets disabled because the secure contexts
> that have been set on this IOMMU by the bootloader will become
> unaccessible (or they get reset).
> Mark this clock as critical to avoid this issue when the Adreno
> GPU is enabled.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---
>  drivers/clk/qcom/gcc-msm8998.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
> index c8d4c0348952..afea60a3ef43 100644
> --- a/drivers/clk/qcom/gcc-msm8998.c
> +++ b/drivers/clk/qcom/gcc-msm8998.c
> @@ -2081,6 +2081,12 @@ static struct clk_branch gcc_gpu_cfg_ahb_clk = {
>                 .hw.init = &(struct clk_init_data){
>                         .name = "gcc_gpu_cfg_ahb_clk",
>                         .ops = &clk_branch2_ops,
> +                       /*
> +                        * The GPU IOMMU depends on this clock and hypervisor
> +                        * will crash the SoC if this clock goes down, due to
> +                        * secure contexts protection.
> +                        */
> +                       .flags = CLK_IS_CRITICAL,
>                 },
>         },

Please send a followup patch that hits the branch on at probe time and
removes this clk from the kernel. That will save some memory and
overhead of worrying about this clk.

Re: [PATCH v2 05/11] clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical

[AngeloGioacchino Del Regno]

Il 08/02/21 19:18, Stephen Boyd ha scritto:
> Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:53)
>> The GPU IOMMU depends on this clock and the hypervisor will crash
>> the SoC if this clock gets disabled because the secure contexts
>> that have been set on this IOMMU by the bootloader will become
>> unaccessible (or they get reset).
>> Mark this clock as critical to avoid this issue when the Adreno
>> GPU is enabled.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>> ---
>>   drivers/clk/qcom/gcc-msm8998.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
>> index c8d4c0348952..afea60a3ef43 100644
>> --- a/drivers/clk/qcom/gcc-msm8998.c
>> +++ b/drivers/clk/qcom/gcc-msm8998.c
>> @@ -2081,6 +2081,12 @@ static struct clk_branch gcc_gpu_cfg_ahb_clk = {
>>                  .hw.init = &(struct clk_init_data){
>>                          .name = "gcc_gpu_cfg_ahb_clk",
>>                          .ops = &clk_branch2_ops,
>> +                       /*
>> +                        * The GPU IOMMU depends on this clock and hypervisor
>> +                        * will crash the SoC if this clock goes down, due to
>> +                        * secure contexts protection.
>> +                        */
>> +                       .flags = CLK_IS_CRITICAL,
>>                  },
>>          },
> 
> Please send a followup patch that hits the branch on at probe time and
> removes this clk from the kernel. That will save some memory and
> overhead of worrying about this clk.
> 

Will do!

Re: [PATCH v2 05/11] clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:53)
> The GPU IOMMU depends on this clock and the hypervisor will crash
> the SoC if this clock gets disabled because the secure contexts
> that have been set on this IOMMU by the bootloader will become
> unaccessible (or they get reset).
> Mark this clock as critical to avoid this issue when the Adreno
> GPU is enabled.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 06/11] clk: qcom: gcc-msm8998: Fix Alpha PLL type for all GPLLs

[AngeloGioacchino Del Regno]

All of the GPLLs in the MSM8998 Global Clock Controller are Fabia PLLs
and not generic alphas: this was producing bad effects over the entire
clock tree of MSM8998, where any GPLL child clock was declaring a false
clock rate, due to their parent also showing the same.

The issue resides in the calculation of the clock rate for the specific
Alpha PLL type, where Fabia has a different register layout; switching
the MSM8998 GPLLs to the correct Alpha Fabia PLL type fixes the rate
(calculation) reading. While at it, also make these PLLs fixed since
their rate is supposed to *never* be changed while the system runs, as
this would surely crash the entire SoC.

Now all the children of all the PLLs are also complying with their
specified clock table and system stability is improved.

Fixes: b5f5f525c547 ("clk: qcom: Add MSM8998 Global Clock Control (GCC) driver")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/gcc-msm8998.c | 100 ++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 50 deletions(-)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index afea60a3ef43..050c91af888e 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -135,7 +135,7 @@ static struct pll_vco fabia_vco[] = {
 
 static struct clk_alpha_pll gpll0 = {
 	.offset = 0x0,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.vco_table = fabia_vco,
 	.num_vco = ARRAY_SIZE(fabia_vco),
 	.clkr = {
@@ -145,58 +145,58 @@ static struct clk_alpha_pll gpll0 = {
 			.name = "gpll0",
 			.parent_names = (const char *[]){ "xo" },
 			.num_parents = 1,
-			.ops = &clk_alpha_pll_ops,
+			.ops = &clk_alpha_pll_fixed_fabia_ops,
 		}
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll0_out_even = {
 	.offset = 0x0,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll0_out_even",
 		.parent_names = (const char *[]){ "gpll0" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll0_out_main = {
 	.offset = 0x0,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll0_out_main",
 		.parent_names = (const char *[]){ "gpll0" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll0_out_odd = {
 	.offset = 0x0,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll0_out_odd",
 		.parent_names = (const char *[]){ "gpll0" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll0_out_test = {
 	.offset = 0x0,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll0_out_test",
 		.parent_names = (const char *[]){ "gpll0" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll gpll1 = {
 	.offset = 0x1000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.vco_table = fabia_vco,
 	.num_vco = ARRAY_SIZE(fabia_vco),
 	.clkr = {
@@ -206,58 +206,58 @@ static struct clk_alpha_pll gpll1 = {
 			.name = "gpll1",
 			.parent_names = (const char *[]){ "xo" },
 			.num_parents = 1,
-			.ops = &clk_alpha_pll_ops,
+			.ops = &clk_alpha_pll_fixed_fabia_ops,
 		}
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll1_out_even = {
 	.offset = 0x1000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll1_out_even",
 		.parent_names = (const char *[]){ "gpll1" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll1_out_main = {
 	.offset = 0x1000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll1_out_main",
 		.parent_names = (const char *[]){ "gpll1" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll1_out_odd = {
 	.offset = 0x1000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll1_out_odd",
 		.parent_names = (const char *[]){ "gpll1" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll1_out_test = {
 	.offset = 0x1000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll1_out_test",
 		.parent_names = (const char *[]){ "gpll1" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll gpll2 = {
 	.offset = 0x2000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.vco_table = fabia_vco,
 	.num_vco = ARRAY_SIZE(fabia_vco),
 	.clkr = {
@@ -267,58 +267,58 @@ static struct clk_alpha_pll gpll2 = {
 			.name = "gpll2",
 			.parent_names = (const char *[]){ "xo" },
 			.num_parents = 1,
-			.ops = &clk_alpha_pll_ops,
+			.ops = &clk_alpha_pll_fixed_fabia_ops,
 		}
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll2_out_even = {
 	.offset = 0x2000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll2_out_even",
 		.parent_names = (const char *[]){ "gpll2" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll2_out_main = {
 	.offset = 0x2000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll2_out_main",
 		.parent_names = (const char *[]){ "gpll2" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll2_out_odd = {
 	.offset = 0x2000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll2_out_odd",
 		.parent_names = (const char *[]){ "gpll2" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll2_out_test = {
 	.offset = 0x2000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll2_out_test",
 		.parent_names = (const char *[]){ "gpll2" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll gpll3 = {
 	.offset = 0x3000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.vco_table = fabia_vco,
 	.num_vco = ARRAY_SIZE(fabia_vco),
 	.clkr = {
@@ -328,58 +328,58 @@ static struct clk_alpha_pll gpll3 = {
 			.name = "gpll3",
 			.parent_names = (const char *[]){ "xo" },
 			.num_parents = 1,
-			.ops = &clk_alpha_pll_ops,
+			.ops = &clk_alpha_pll_fixed_fabia_ops,
 		}
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll3_out_even = {
 	.offset = 0x3000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll3_out_even",
 		.parent_names = (const char *[]){ "gpll3" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll3_out_main = {
 	.offset = 0x3000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll3_out_main",
 		.parent_names = (const char *[]){ "gpll3" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll3_out_odd = {
 	.offset = 0x3000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll3_out_odd",
 		.parent_names = (const char *[]){ "gpll3" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll3_out_test = {
 	.offset = 0x3000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll3_out_test",
 		.parent_names = (const char *[]){ "gpll3" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll gpll4 = {
 	.offset = 0x77000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.vco_table = fabia_vco,
 	.num_vco = ARRAY_SIZE(fabia_vco),
 	.clkr = {
@@ -389,52 +389,52 @@ static struct clk_alpha_pll gpll4 = {
 			.name = "gpll4",
 			.parent_names = (const char *[]){ "xo" },
 			.num_parents = 1,
-			.ops = &clk_alpha_pll_ops,
+			.ops = &clk_alpha_pll_fixed_fabia_ops,
 		}
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll4_out_even = {
 	.offset = 0x77000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll4_out_even",
 		.parent_names = (const char *[]){ "gpll4" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll4_out_main = {
 	.offset = 0x77000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll4_out_main",
 		.parent_names = (const char *[]){ "gpll4" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll4_out_odd = {
 	.offset = 0x77000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll4_out_odd",
 		.parent_names = (const char *[]){ "gpll4" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
 static struct clk_alpha_pll_postdiv gpll4_out_test = {
 	.offset = 0x77000,
-	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll4_out_test",
 		.parent_names = (const char *[]){ "gpll4" },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_postdiv_ops,
+		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
 
-- 
2.29.2

Re: [PATCH v2 06/11] clk: qcom: gcc-msm8998: Fix Alpha PLL type for all GPLLs

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:54)
> All of the GPLLs in the MSM8998 Global Clock Controller are Fabia PLLs
> and not generic alphas: this was producing bad effects over the entire
> clock tree of MSM8998, where any GPLL child clock was declaring a false
> clock rate, due to their parent also showing the same.
> 
> The issue resides in the calculation of the clock rate for the specific
> Alpha PLL type, where Fabia has a different register layout; switching
> the MSM8998 GPLLs to the correct Alpha Fabia PLL type fixes the rate
> (calculation) reading. While at it, also make these PLLs fixed since
> their rate is supposed to *never* be changed while the system runs, as
> this would surely crash the entire SoC.
> 
> Now all the children of all the PLLs are also complying with their
> specified clock table and system stability is improved.
> 
> Fixes: b5f5f525c547 ("clk: qcom: Add MSM8998 Global Clock Control (GCC) driver")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 07/11] clk: qcom: mmcc-msm8998: Set CLK_GET_RATE_NOCACHE to pixel/byte clks

[AngeloGioacchino Del Regno]

The pixel and byte clocks rate should not be cached, as a VCO shutdown
may clear the frequency setup and this may not be set again due to the
cached rate being present.
This will also be useful when shadow clocks will be implemented in
the DSI PLL for seamless timing/resolution switch.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/mmcc-msm8998.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/mmcc-msm8998.c b/drivers/clk/qcom/mmcc-msm8998.c
index dd68983fe22e..475e00a5fdf4 100644
--- a/drivers/clk/qcom/mmcc-msm8998.c
+++ b/drivers/clk/qcom/mmcc-msm8998.c
@@ -520,7 +520,7 @@ static struct clk_rcg2 byte0_clk_src = {
 		.parent_data = mmss_xo_dsibyte,
 		.num_parents = 4,
 		.ops = &clk_byte2_ops,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
 	},
 };
 
@@ -533,7 +533,7 @@ static struct clk_rcg2 byte1_clk_src = {
 		.parent_data = mmss_xo_dsibyte,
 		.num_parents = 4,
 		.ops = &clk_byte2_ops,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
 	},
 };
 
@@ -1084,7 +1084,7 @@ static struct clk_rcg2 pclk0_clk_src = {
 		.parent_data = mmss_xo_dsi0pll_dsi1pll,
 		.num_parents = 4,
 		.ops = &clk_pixel_ops,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
 	},
 };
 
@@ -1098,7 +1098,7 @@ static struct clk_rcg2 pclk1_clk_src = {
 		.parent_data = mmss_xo_dsi0pll_dsi1pll,
 		.num_parents = 4,
 		.ops = &clk_pixel_ops,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
 	},
 };
 
-- 
2.29.2

Re: [PATCH v2 07/11] clk: qcom: mmcc-msm8998: Set CLK_GET_RATE_NOCACHE to pixel/byte clks

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:55)
> The pixel and byte clocks rate should not be cached, as a VCO shutdown
> may clear the frequency setup and this may not be set again due to the
> cached rate being present.
> This will also be useful when shadow clocks will be implemented in
> the DSI PLL for seamless timing/resolution switch.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

Re: [PATCH v2 07/11] clk: qcom: mmcc-msm8998: Set CLK_GET_RATE_NOCACHE to pixel/byte clks

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:55)
> The pixel and byte clocks rate should not be cached, as a VCO shutdown
> may clear the frequency setup and this may not be set again due to the
> cached rate being present.
> This will also be useful when shadow clocks will be implemented in
> the DSI PLL for seamless timing/resolution switch.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---
>  drivers/clk/qcom/mmcc-msm8998.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

We didn't do this on sdm845, so I'm not going to apply this patch. The
rate caching thing is a problem with the display driver that should be
fixed some other way vs. setting nocache here.

Re: [PATCH v2 07/11] clk: qcom: mmcc-msm8998: Set CLK_GET_RATE_NOCACHE to pixel/byte clks

[AngeloGioacchino Del Regno]

Il 08/02/21 19:21, Stephen Boyd ha scritto:
> Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:55)
>> The pixel and byte clocks rate should not be cached, as a VCO shutdown
>> may clear the frequency setup and this may not be set again due to the
>> cached rate being present.
>> This will also be useful when shadow clocks will be implemented in
>> the DSI PLL for seamless timing/resolution switch.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>> ---
>>   drivers/clk/qcom/mmcc-msm8998.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> We didn't do this on sdm845, so I'm not going to apply this patch. The
> rate caching thing is a problem with the display driver that should be
> fixed some other way vs. setting nocache here.
> 

Ok, I agree.

[PATCH v2 08/11] clk: qcom: mmcc-msm8998: Add hardware clockgating registers to some clks

[AngeloGioacchino Del Regno]

Hardware clock gating is supported on some of the clocks declared in
there: ignoring that it does exist may lead to unstabilities on some
firmwares.
Add the HWCG registers where applicable to stop potential crashes.

This was verified on a smartphone shipped with a recent MSM8998
firmware, which will experience random crashes without this change.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/mmcc-msm8998.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/clk/qcom/mmcc-msm8998.c b/drivers/clk/qcom/mmcc-msm8998.c
index 475e00a5fdf4..f9510a248a36 100644
--- a/drivers/clk/qcom/mmcc-msm8998.c
+++ b/drivers/clk/qcom/mmcc-msm8998.c
@@ -1211,6 +1211,8 @@ static struct clk_rcg2 vfe1_clk_src = {
 
 static struct clk_branch misc_ahb_clk = {
 	.halt_reg = 0x328,
+	.hwcg_reg = 0x328,
+	.hwcg_bit = 1,
 	.clkr = {
 		.enable_reg = 0x328,
 		.enable_mask = BIT(0),
@@ -1241,6 +1243,8 @@ static struct clk_branch video_core_clk = {
 
 static struct clk_branch video_ahb_clk = {
 	.halt_reg = 0x1030,
+	.hwcg_reg = 0x1030,
+	.hwcg_bit = 1,
 	.clkr = {
 		.enable_reg = 0x1030,
 		.enable_mask = BIT(0),
@@ -1315,6 +1319,8 @@ static struct clk_branch video_subcore1_clk = {
 
 static struct clk_branch mdss_ahb_clk = {
 	.halt_reg = 0x2308,
+	.hwcg_reg = 0x2308,
+	.hwcg_bit = 1,
 	.clkr = {
 		.enable_reg = 0x2308,
 		.enable_mask = BIT(0),
@@ -2496,6 +2502,8 @@ static struct clk_branch mnoc_ahb_clk = {
 
 static struct clk_branch bimc_smmu_ahb_clk = {
 	.halt_reg = 0xe004,
+	.hwcg_reg = 0xe004,
+	.hwcg_bit = 1,
 	.clkr = {
 		.enable_reg = 0xe004,
 		.enable_mask = BIT(0),
@@ -2511,6 +2519,8 @@ static struct clk_branch bimc_smmu_ahb_clk = {
 
 static struct clk_branch bimc_smmu_axi_clk = {
 	.halt_reg = 0xe008,
+	.hwcg_reg = 0xe008,
+	.hwcg_bit = 1,
 	.clkr = {
 		.enable_reg = 0xe008,
 		.enable_mask = BIT(0),
-- 
2.29.2

Re: [PATCH v2 08/11] clk: qcom: mmcc-msm8998: Add hardware clockgating registers to some clks

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:56)
> Hardware clock gating is supported on some of the clocks declared in
> there: ignoring that it does exist may lead to unstabilities on some
> firmwares.
> Add the HWCG registers where applicable to stop potential crashes.
> 
> This was verified on a smartphone shipped with a recent MSM8998
> firmware, which will experience random crashes without this change.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 09/11] clk: qcom: mmcc-msm8998: Set bimc_smmu_gdsc always on

[AngeloGioacchino Del Regno]

This GDSC enables (or cuts!) power to the Multimedia Subsystem IOMMU
(mmss smmu), which has bootloader pre-set secure contexts.
In the event of a complete power loss, the secure contexts will be
reset and the hypervisor will crash the SoC.

To prevent this, and get a working multimedia subsystem, set this
GDSC as always on.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/mmcc-msm8998.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/mmcc-msm8998.c b/drivers/clk/qcom/mmcc-msm8998.c
index f9510a248a36..b49c4137b7d7 100644
--- a/drivers/clk/qcom/mmcc-msm8998.c
+++ b/drivers/clk/qcom/mmcc-msm8998.c
@@ -2663,7 +2663,7 @@ static struct gdsc bimc_smmu_gdsc = {
 		.name = "bimc_smmu",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = HW_CTRL,
+	.flags = HW_CTRL | ALWAYS_ON,
 };
 
 static struct clk_regmap *mmcc_msm8998_clocks[] = {
-- 
2.29.2

Re: [PATCH v2 09/11] clk: qcom: mmcc-msm8998: Set bimc_smmu_gdsc always on

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:57)
> This GDSC enables (or cuts!) power to the Multimedia Subsystem IOMMU
> (mmss smmu), which has bootloader pre-set secure contexts.
> In the event of a complete power loss, the secure contexts will be
> reset and the hypervisor will crash the SoC.
> 
> To prevent this, and get a working multimedia subsystem, set this
> GDSC as always on.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 10/11] clk: qcom: gpucc-msm8998: Add resets, cxc, fix flags on gpu_gx_gdsc

[AngeloGioacchino Del Regno]

The GPU GX GDSC has GPU_GX_BCR reset and gfx3d_clk CXC, as stated
on downstream kernels (and as verified upstream, because otherwise
random lockups happen).
Also, add PWRSTS_RET and NO_RET_PERIPH: also as found downstream,
and also as verified here, to avoid GPU related lockups it is
necessary to force retain mem, but *not* peripheral when enabling
this GDSC (and, of course, the inverse on disablement).

With this change, the GPU finally works flawlessly on my four
different MSM8998 devices from two different manufacturers.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/gpucc-msm8998.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c
index 9b3923af02a1..1a518c4915b4 100644
--- a/drivers/clk/qcom/gpucc-msm8998.c
+++ b/drivers/clk/qcom/gpucc-msm8998.c
@@ -253,12 +253,16 @@ static struct gdsc gpu_cx_gdsc = {
 static struct gdsc gpu_gx_gdsc = {
 	.gdscr = 0x1094,
 	.clamp_io_ctrl = 0x130,
+	.resets = (unsigned int []){ GPU_GX_BCR },
+	.reset_count = 1,
+	.cxcs = (unsigned int []){ 0x1098 },
+	.cxc_count = 1,
 	.pd = {
 		.name = "gpu_gx",
 	},
 	.parent = &gpu_cx_gdsc.pd,
-	.pwrsts = PWRSTS_OFF_ON,
-	.flags = CLAMP_IO | AON_RESET,
+	.pwrsts = PWRSTS_OFF_ON | PWRSTS_RET,
+	.flags = CLAMP_IO | SW_RESET | AON_RESET | NO_RET_PERIPH,
 };
 
 static struct clk_regmap *gpucc_msm8998_clocks[] = {
-- 
2.29.2

Re: [PATCH v2 10/11] clk: qcom: gpucc-msm8998: Add resets, cxc, fix flags on gpu_gx_gdsc

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:58)
> The GPU GX GDSC has GPU_GX_BCR reset and gfx3d_clk CXC, as stated
> on downstream kernels (and as verified upstream, because otherwise
> random lockups happen).
> Also, add PWRSTS_RET and NO_RET_PERIPH: also as found downstream,
> and also as verified here, to avoid GPU related lockups it is
> necessary to force retain mem, but *not* peripheral when enabling
> this GDSC (and, of course, the inverse on disablement).
> 
> With this change, the GPU finally works flawlessly on my four
> different MSM8998 devices from two different manufacturers.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

[PATCH v2 11/11] clk: qcom: gpucc-msm8998: Allow fabia gpupll0 rate setting

[AngeloGioacchino Del Regno]

The GPU PLL0 is not a fixed PLL and the rate can be set on it:
this is necessary especially on boards which bootloader is setting
a very low rate on this PLL before booting Linux, which would be
unsuitable for postdividing to reach the maximum allowed Adreno GPU
frequency of 710MHz (or, actually, even 670MHz..) on this SoC.

To allow setting rates on the GPU PLL0, also define VCO boundaries
and set the CLK_SET_RATE_PARENT flag to the GPU PLL0 postdivider.

With this change, the Adreno GPU is now able to scale through all
the available frequencies.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/clk/qcom/gpucc-msm8998.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c
index 1a518c4915b4..fedfffaf0a8d 100644
--- a/drivers/clk/qcom/gpucc-msm8998.c
+++ b/drivers/clk/qcom/gpucc-msm8998.c
@@ -50,6 +50,11 @@ static struct clk_branch gpucc_cxo_clk = {
 	},
 };
 
+static struct pll_vco fabia_vco[] = {
+	{ 249600000, 2000000000, 0 },
+	{ 125000000, 1000000000, 1 },
+};
+
 static const struct clk_div_table post_div_table_fabia_even[] = {
 	{ 0x0, 1 },
 	{ 0x1, 2 },
@@ -61,11 +66,13 @@ static const struct clk_div_table post_div_table_fabia_even[] = {
 static struct clk_alpha_pll gpupll0 = {
 	.offset = 0x0,
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
+	.vco_table = fabia_vco,
+	.num_vco = ARRAY_SIZE(fabia_vco),
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpupll0",
 		.parent_hws = (const struct clk_hw *[]){ &gpucc_cxo_clk.clkr.hw },
 		.num_parents = 1,
-		.ops = &clk_alpha_pll_fixed_fabia_ops,
+		.ops = &clk_alpha_pll_fabia_ops,
 	},
 };
 
@@ -80,6 +87,7 @@ static struct clk_alpha_pll_postdiv gpupll0_out_even = {
 		.name = "gpupll0_out_even",
 		.parent_hws = (const struct clk_hw *[]){ &gpupll0.clkr.hw },
 		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
 		.ops = &clk_alpha_pll_postdiv_fabia_ops,
 	},
 };
-- 
2.29.2

Re: [PATCH v2 11/11] clk: qcom: gpucc-msm8998: Allow fabia gpupll0 rate setting

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:59)
> The GPU PLL0 is not a fixed PLL and the rate can be set on it:
> this is necessary especially on boards which bootloader is setting
> a very low rate on this PLL before booting Linux, which would be
> unsuitable for postdividing to reach the maximum allowed Adreno GPU
> frequency of 710MHz (or, actually, even 670MHz..) on this SoC.
> 
> To allow setting rates on the GPU PLL0, also define VCO boundaries
> and set the CLK_SET_RATE_PARENT flag to the GPU PLL0 postdivider.
> 
> With this change, the Adreno GPU is now able to scale through all
> the available frequencies.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---

Applied to clk-next

Re: [PATCH v2 11/11] clk: qcom: gpucc-msm8998: Allow fabia gpupll0 rate setting

[Stephen Boyd]

Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:59)
> The GPU PLL0 is not a fixed PLL and the rate can be set on it:
> this is necessary especially on boards which bootloader is setting
> a very low rate on this PLL before booting Linux, which would be
> unsuitable for postdividing to reach the maximum allowed Adreno GPU
> frequency of 710MHz (or, actually, even 670MHz..) on this SoC.
> 
> To allow setting rates on the GPU PLL0, also define VCO boundaries
> and set the CLK_SET_RATE_PARENT flag to the GPU PLL0 postdivider.
> 
> With this change, the Adreno GPU is now able to scale through all
> the available frequencies.

BTW, you're probably undervolting your GPU and clocking it higher
than it should be at the voltage from boot.

> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---
>  drivers/clk/qcom/gpucc-msm8998.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c
> index 1a518c4915b4..fedfffaf0a8d 100644
> --- a/drivers/clk/qcom/gpucc-msm8998.c
> +++ b/drivers/clk/qcom/gpucc-msm8998.c
> @@ -50,6 +50,11 @@ static struct clk_branch gpucc_cxo_clk = {
>         },
>  };
>  
> +static struct pll_vco fabia_vco[] = {

Should be const.

> +       { 249600000, 2000000000, 0 },
> +       { 125000000, 1000000000, 1 },
> +};
> +
>  static const struct clk_div_table post_div_table_fabia_even[] = {
>         { 0x0, 1 },
>         { 0x1, 2 },

Re: [PATCH v2 11/11] clk: qcom: gpucc-msm8998: Allow fabia gpupll0 rate setting

[AngeloGioacchino Del Regno]

Il 08/02/21 19:24, Stephen Boyd ha scritto:
> Quoting AngeloGioacchino Del Regno (2021-01-14 14:10:59)
>> The GPU PLL0 is not a fixed PLL and the rate can be set on it:
>> this is necessary especially on boards which bootloader is setting
>> a very low rate on this PLL before booting Linux, which would be
>> unsuitable for postdividing to reach the maximum allowed Adreno GPU
>> frequency of 710MHz (or, actually, even 670MHz..) on this SoC.
>>
>> To allow setting rates on the GPU PLL0, also define VCO boundaries
>> and set the CLK_SET_RATE_PARENT flag to the GPU PLL0 postdivider.
>>
>> With this change, the Adreno GPU is now able to scale through all
>> the available frequencies.
> 
> BTW, you're probably undervolting your GPU and clocking it higher
> than it should be at the voltage from boot.
> 

Thanks for the consideration, but that's not the case, locally ;)

>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>> ---
>>   drivers/clk/qcom/gpucc-msm8998.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c
>> index 1a518c4915b4..fedfffaf0a8d 100644
>> --- a/drivers/clk/qcom/gpucc-msm8998.c
>> +++ b/drivers/clk/qcom/gpucc-msm8998.c
>> @@ -50,6 +50,11 @@ static struct clk_branch gpucc_cxo_clk = {
>>          },
>>   };
>>   
>> +static struct pll_vco fabia_vco[] = {
> 
> Should be const.
> 

Ack!

>> +       { 249600000, 2000000000, 0 },
>> +       { 125000000, 1000000000, 1 },
>> +};
>> +
>>   static const struct clk_div_table post_div_table_fabia_even[] = {
>>          { 0x0, 1 },
>>          { 0x1, 2 },

Re: [PATCH v2 00/11] Clock fixes for MSM8998 GCC, MMCC, GPUCC

[AngeloGioacchino Del Regno]

Il 14/01/21 23:10, AngeloGioacchino Del Regno ha scritto:
> This patch series fixes some issues with the MSM8998 clocks and, in
> particular, brings a very important fix to the GCC PLLs.
> 
> These fixes are enhancing this SoC's stability and also makes it
> possible to eventually enable the Adreno GPU (with proper clock
> scaling) and other components.
> 
> This patch series was tested on:
> - Sony Xperia XZ Premium (MSM8998)
> - F(x)Tec Pro1 (MSM8998)
> 

Forgot to write this part, so adding as a reply to myself...

Changes in v2:
  - Separated dt-bindings headers definitions additions from
    actual code, as per Rob Herring's review

> AngeloGioacchino Del Regno (11):
>    dt-bindings: clocks: gcc-msm8998: Add GCC_MMSS_GPLL0_CLK definition
>    clk: qcom: gcc-msm8998: Wire up gcc_mmss_gpll0 clock
>    dt-bindings: clock: gcc-msm8998: Add HMSS_GPLL0_CLK_SRC definition
>    clk: qcom: gcc-msm8998: Add missing hmss_gpll0_clk_src clock
>    clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical
>    clk: qcom: gcc-msm8998: Fix Alpha PLL type for all GPLLs
>    clk: qcom: mmcc-msm8998: Set CLK_GET_RATE_NOCACHE to pixel/byte clks
>    clk: qcom: mmcc-msm8998: Add hardware clockgating registers to some
>      clks
>    clk: qcom: mmcc-msm8998: Set bimc_smmu_gdsc always on
>    clk: qcom: gpucc-msm8998: Add resets, cxc, fix flags on gpu_gx_gdsc
>    clk: qcom: gpucc-msm8998: Allow fabia gpupll0 rate setting
> 
>   drivers/clk/qcom/gcc-msm8998.c               | 143 ++++++++++++-------
>   drivers/clk/qcom/gpucc-msm8998.c             |  18 ++-
>   drivers/clk/qcom/mmcc-msm8998.c              |  20 ++-
>   include/dt-bindings/clock/qcom,gcc-msm8998.h |   2 +
>   4 files changed, 125 insertions(+), 58 deletions(-)
> 

Re: [PATCH v2 00/11] Clock fixes for MSM8998 GCC, MMCC, GPUCC

[Konrad Dybcio]

On 14.01.2021 23:10, AngeloGioacchino Del Regno wrote:
> This patch series fixes some issues with the MSM8998 clocks and, in
> particular, brings a very important fix to the GCC PLLs.
>
> These fixes are enhancing this SoC's stability and also makes it
> possible to eventually enable the Adreno GPU (with proper clock
> scaling) and other components.
>
> This patch series was tested on:
> - Sony Xperia XZ Premium (MSM8998)
> - F(x)Tec Pro1 (MSM8998)
>
> AngeloGioacchino Del Regno (11):
>   dt-bindings: clocks: gcc-msm8998: Add GCC_MMSS_GPLL0_CLK definition
>   clk: qcom: gcc-msm8998: Wire up gcc_mmss_gpll0 clock
>   dt-bindings: clock: gcc-msm8998: Add HMSS_GPLL0_CLK_SRC definition
>   clk: qcom: gcc-msm8998: Add missing hmss_gpll0_clk_src clock
>   clk: qcom: gcc-msm8998: Mark gpu_cfg_ahb_clk as critical
>   clk: qcom: gcc-msm8998: Fix Alpha PLL type for all GPLLs
>   clk: qcom: mmcc-msm8998: Set CLK_GET_RATE_NOCACHE to pixel/byte clks
>   clk: qcom: mmcc-msm8998: Add hardware clockgating registers to some
>     clks
>   clk: qcom: mmcc-msm8998: Set bimc_smmu_gdsc always on
>   clk: qcom: gpucc-msm8998: Add resets, cxc, fix flags on gpu_gx_gdsc
>   clk: qcom: gpucc-msm8998: Allow fabia gpupll0 rate setting
>
>  drivers/clk/qcom/gcc-msm8998.c               | 143 ++++++++++++-------
>  drivers/clk/qcom/gpucc-msm8998.c             |  18 ++-
>  drivers/clk/qcom/mmcc-msm8998.c              |  20 ++-
>  include/dt-bindings/clock/qcom,gcc-msm8998.h |   2 +
>  4 files changed, 125 insertions(+), 58 deletions(-)


Bump! I can tell all of these patches work as they should :)