[PATCH 0/3] Add support to reconfigure PLL

[PATCH 0/3] Add support to reconfigure PLL

[Taniya Das]

During boot-up, there is a possibility that the PLL configuration might
be missed even after invoking pll_configure() from the clock controller
probe. This is often due to the PLL being connected to rail or rails
that are in an OFF state and current clock controller also cannot vote
on multiple rails. As a result, the PLL may be enabled with suboptimal
settings, leading to functional issues.

The PLL configuration, now part of clk_alpha_pll, can be reused to
reconfigure the PLL to a known good state before scaling for frequency.
The 'clk_alpha_pll_reconfigure()' can be updated to support more PLLs
in future.

The IRIS driver support added
https://lore.kernel.org/all/20241212-qcom-video-iris-v9-0-e8c2c6bd4041@quicinc.com/
observes the pll latch warning during boot up due to the dependency of
the PLL not in good state, thus add config support for SM8550 Video
clock controller PLLs.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
Taniya Das (3):
      clk: qcom: clk-alpha-pll: Integrate PLL configuration into PLL structure
      clk: qcom: clk-alpha-pll: Add support to reconfigure PLL
      clk: qcom: videocc-sm8550: Update the pll config for Video PLLs

 drivers/clk/qcom/clk-alpha-pll.c  | 30 ++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h  |  2 ++
 drivers/clk/qcom/videocc-sm8550.c |  2 ++
 3 files changed, 34 insertions(+)
---
base-commit: 37136bf5c3a6f6b686d74f41837a6406bec6b7bc
change-id: 20250113-support-pll-reconfigure-9a9cbb43a90b

Best regards,
-- 
Taniya Das <quic_tdas@quicinc.com>

[PATCH 1/3] clk: qcom: clk-alpha-pll: Integrate PLL configuration into PLL structure

[Taniya Das]

Integrate the PLL configuration into clk_alpha_pll to facilitate future
reuse as needed. This is particularly useful when the PLL requires
reconfiguration.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/clk-alpha-pll.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index 79aca8525262211ae5295245427d4540abf1e09a..943320cdcd10a6c07fcd74dccb88be847dc086c2 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -81,6 +81,7 @@ struct pll_vco {
  * struct clk_alpha_pll - phase locked loop (PLL)
  * @offset: base address of registers
  * @regs: alpha pll register map (see @clk_alpha_pll_regs)
+ * @config: array of pll settings
  * @vco_table: array of VCO settings
  * @num_vco: number of VCO settings in @vco_table
  * @flags: bitmask to indicate features supported by the hardware
@@ -90,6 +91,7 @@ struct clk_alpha_pll {
 	u32 offset;
 	const u8 *regs;
 
+	const struct alpha_pll_config *config;
 	const struct pll_vco *vco_table;
 	size_t num_vco;
 #define SUPPORTS_OFFLINE_REQ		BIT(0)

-- 
2.45.2

[PATCH 2/3] clk: qcom: clk-alpha-pll: Add support to reconfigure PLL

[Taniya Das]

During boot-up, there is a possibility that the PLL configuration might
be missed even after invoking pll_configure() from the clock controller
probe. This is often due to the PLL being connected to rail or rails
that are in an OFF state and current clock controller also cannot vote
on multiple rails. As a result, the PLL may be enabled with suboptimal
settings, leading to functional issues.

The PLL configuration, now part of clk_alpha_pll, can be reused to
reconfigure the PLL to a known good state before scaling for frequency.
The 'clk_alpha_pll_reconfigure()' can be updated to support more PLLs
in future.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/clk-alpha-pll.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index 9a65d14acf71c97912664be4f6f78891cab4afa3..eb27c0992c7f9281dac4f2fc792084292c21a6c1 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -1750,6 +1750,26 @@ static int alpha_pll_lucid_prepare(struct clk_hw *hw)
 	return __alpha_pll_trion_prepare(hw, LUCID_PCAL_DONE);
 }
 
+#define GET_PLL_TYPE(pll)    ((pll->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
+static void clk_alpha_pll_reconfigure(struct clk_alpha_pll *pll)
+{
+	if (!pll->config || !pll->regs)
+		return;
+
+	pr_debug("configuring the PLL again!\n");
+
+	switch (GET_PLL_TYPE(pll)) {
+	case CLK_ALPHA_PLL_TYPE_LUCID_OLE:
+		clk_lucid_ole_pll_configure(pll, pll->clkr.regmap, pll->config);
+		break;
+	case CLK_ALPHA_PLL_TYPE_LUCID_EVO:
+		clk_lucid_evo_pll_configure(pll, pll->clkr.regmap, pll->config);
+		break;
+	default:
+		break;
+	}
+}
+
 static int __alpha_pll_trion_set_rate(struct clk_hw *hw, unsigned long rate,
 				      unsigned long prate, u32 latch_bit, u32 latch_ack)
 {
@@ -1765,6 +1785,11 @@ static int __alpha_pll_trion_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (ret < 0)
 		return ret;
 
+	regmap_read(pll->clkr.regmap, PLL_L_VAL(pll), &val);
+	/* Check if the PLL is in good state to accept set rate requests. */
+	if (!(val & LUCID_EVO_PLL_L_VAL_MASK))
+		clk_alpha_pll_reconfigure(pll);
+
 	regmap_update_bits(pll->clkr.regmap, PLL_L_VAL(pll), LUCID_EVO_PLL_L_VAL_MASK,  l);
 	regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a);
 
@@ -2372,6 +2397,11 @@ static int alpha_pll_lucid_evo_enable(struct clk_hw *hw)
 	if (trion_pll_is_enabled(pll, regmap))
 		return 0;
 
+	regmap_read(pll->clkr.regmap, PLL_L_VAL(pll), &val);
+	/* Check if the PLL is in good state to accept enable requests */
+	if (!(val & LUCID_EVO_PLL_L_VAL_MASK))
+		clk_alpha_pll_reconfigure(pll);
+
 	ret = regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
 	if (ret)
 		return ret;

-- 
2.45.2

[PATCH 3/3] clk: qcom: videocc-sm8550: Update the pll config for Video PLLs

[Taniya Das]

The video plls are on MxC rail and needs to be configured before being
used for functional use case, so update the pll configs as part of the
pll structure.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/videocc-sm8550.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
index 7c25a50cfa970dff55d701cb24bc3aa5924ca12d..ed94a72d6c1b064fd767df0c691d0273ef106e84 100644
--- a/drivers/clk/qcom/videocc-sm8550.c
+++ b/drivers/clk/qcom/videocc-sm8550.c
@@ -51,6 +51,7 @@ static struct alpha_pll_config video_cc_pll0_config = {
 
 static struct clk_alpha_pll video_cc_pll0 = {
 	.offset = 0x0,
+	.config = &video_cc_pll0_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
@@ -82,6 +83,7 @@ static struct alpha_pll_config video_cc_pll1_config = {
 
 static struct clk_alpha_pll video_cc_pll1 = {
 	.offset = 0x1000,
+	.config = &video_cc_pll1_config,
 	.vco_table = lucid_ole_vco,
 	.num_vco = ARRAY_SIZE(lucid_ole_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],

-- 
2.45.2

Re: [PATCH 0/3] Add support to reconfigure PLL

[Stefan Schmidt]

Hello Taniya,

On Mon, 13 Jan 2025 at 18:27, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> During boot-up, there is a possibility that the PLL configuration might
> be missed even after invoking pll_configure() from the clock controller
> probe. This is often due to the PLL being connected to rail or rails
> that are in an OFF state and current clock controller also cannot vote
> on multiple rails. As a result, the PLL may be enabled with suboptimal
> settings, leading to functional issues.
>
> The PLL configuration, now part of clk_alpha_pll, can be reused to
> reconfigure the PLL to a known good state before scaling for frequency.
> The 'clk_alpha_pll_reconfigure()' can be updated to support more PLLs
> in future.
>
> The IRIS driver support added
> https://lore.kernel.org/all/20241212-qcom-video-iris-v9-0-e8c2c6bd4041@quicinc.com/
> observes the pll latch warning during boot up due to the dependency of
> the PLL not in good state, thus add config support for SM8550 Video
> clock controller PLLs.
>
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
> Taniya Das (3):
>       clk: qcom: clk-alpha-pll: Integrate PLL configuration into PLL structure
>       clk: qcom: clk-alpha-pll: Add support to reconfigure PLL
>       clk: qcom: videocc-sm8550: Update the pll config for Video PLLs
>
>  drivers/clk/qcom/clk-alpha-pll.c  | 30 ++++++++++++++++++++++++++++++
>  drivers/clk/qcom/clk-alpha-pll.h  |  2 ++
>  drivers/clk/qcom/videocc-sm8550.c |  2 ++
>  3 files changed, 34 insertions(+)
> ---
> base-commit: 37136bf5c3a6f6b686d74f41837a6406bec6b7bc
> change-id: 20250113-support-pll-reconfigure-9a9cbb43a90b

I tested the full patchset on my X Elite Dell XPS and can confirm that
the Lucid PLL latch failed warning is gone when using the iris driver.

Tested-by: Stefan Schmidt <stefan.schmidt@linaro.org> # x1e80100 (Dell
XPS 13 9345)

regards
Stefan Schmidt

Re: [PATCH 0/3] Add support to reconfigure PLL

[Johan Hovold]

On Mon, Jan 13, 2025 at 10:57:03PM +0530, Taniya Das wrote:
> During boot-up, there is a possibility that the PLL configuration might
> be missed even after invoking pll_configure() from the clock controller
> probe. This is often due to the PLL being connected to rail or rails
> that are in an OFF state and current clock controller also cannot vote
> on multiple rails. As a result, the PLL may be enabled with suboptimal
> settings, leading to functional issues.
> 
> The PLL configuration, now part of clk_alpha_pll, can be reused to
> reconfigure the PLL to a known good state before scaling for frequency.
> The 'clk_alpha_pll_reconfigure()' can be updated to support more PLLs
> in future.

This sounds like a hack. You already describe the underlying problem (and
indirectly its solution) in the first paragraph above, namely that the
video clock controller has not enabled the power domain needed to
configure the PLL.

I believe support for clock controllers that need to enable multiple
power domains is on its way into 6.15:

	https://lore.kernel.org/lkml/20250117-b4-linux-next-24-11-18-clock-multiple-power-domains-v10-0-13f2bb656dad@linaro.org/

Perhaps that's what you need to fix this properly.

> The IRIS driver support added
> https://lore.kernel.org/all/20241212-qcom-video-iris-v9-0-e8c2c6bd4041@quicinc.com/
> observes the pll latch warning during boot up due to the dependency of
> the PLL not in good state, thus add config support for SM8550 Video
> clock controller PLLs.

> Taniya Das (3):
>       clk: qcom: clk-alpha-pll: Integrate PLL configuration into PLL structure
>       clk: qcom: clk-alpha-pll: Add support to reconfigure PLL
>       clk: qcom: videocc-sm8550: Update the pll config for Video PLLs

Johan

Re: [PATCH 0/3] Add support to reconfigure PLL

[Taniya Das]

On 2/4/2025 3:38 PM, Johan Hovold wrote:
> On Mon, Jan 13, 2025 at 10:57:03PM +0530, Taniya Das wrote:
>> During boot-up, there is a possibility that the PLL configuration might
>> be missed even after invoking pll_configure() from the clock controller
>> probe. This is often due to the PLL being connected to rail or rails
>> that are in an OFF state and current clock controller also cannot vote
>> on multiple rails. As a result, the PLL may be enabled with suboptimal
>> settings, leading to functional issues.
>>
>> The PLL configuration, now part of clk_alpha_pll, can be reused to
>> reconfigure the PLL to a known good state before scaling for frequency.
>> The 'clk_alpha_pll_reconfigure()' can be updated to support more PLLs
>> in future.
> 
> This sounds like a hack. You already describe the underlying problem (and
> indirectly its solution) in the first paragraph above, namely that the
> video clock controller has not enabled the power domain needed to
> configure the PLL.
> 

This is not a hack, but another alternative way to ensure the PLL is 
configured to the right configuration before being used.

> I believe support for clock controllers that need to enable multiple
> power domains is on its way into 6.15:
> 
> 	https://lore.kernel.org/lkml/20250117-b4-linux-next-24-11-18-clock-multiple-power-domains-v10-0-13f2bb656dad@linaro.org/
> 
> Perhaps that's what you need to fix this properly.
> 

Yes, this is just to add a dependency on clock controller to put the 
rail vote, but this series does not fully solve the clock controller's 
PLL requirement problems.

>> The IRIS driver support added
>> https://lore.kernel.org/all/20241212-qcom-video-iris-v9-0-e8c2c6bd4041@quicinc.com/
>> observes the pll latch warning during boot up due to the dependency of
>> the PLL not in good state, thus add config support for SM8550 Video
>> clock controller PLLs.
> 
>> Taniya Das (3):
>>        clk: qcom: clk-alpha-pll: Integrate PLL configuration into PLL structure
>>        clk: qcom: clk-alpha-pll: Add support to reconfigure PLL
>>        clk: qcom: videocc-sm8550: Update the pll config for Video PLLs
> 
> Johan

-- 
Thanks & Regards,
Taniya Das.

Re: [PATCH 0/3] Add support to reconfigure PLL

[Johan Hovold]

On Tue, Feb 04, 2025 at 11:13:08PM +0530, Taniya Das wrote:
> On 2/4/2025 3:38 PM, Johan Hovold wrote:
> > On Mon, Jan 13, 2025 at 10:57:03PM +0530, Taniya Das wrote:
> >> During boot-up, there is a possibility that the PLL configuration might
> >> be missed even after invoking pll_configure() from the clock controller
> >> probe. This is often due to the PLL being connected to rail or rails
> >> that are in an OFF state and current clock controller also cannot vote
> >> on multiple rails. As a result, the PLL may be enabled with suboptimal
> >> settings, leading to functional issues.
> >>
> >> The PLL configuration, now part of clk_alpha_pll, can be reused to
> >> reconfigure the PLL to a known good state before scaling for frequency.
> >> The 'clk_alpha_pll_reconfigure()' can be updated to support more PLLs
> >> in future.
> > 
> > This sounds like a hack. You already describe the underlying problem (and
> > indirectly its solution) in the first paragraph above, namely that the
> > video clock controller has not enabled the power domain needed to
> > configure the PLL.
> 
> This is not a hack, but another alternative way to ensure the PLL is 
> configured to the right configuration before being used.

I say it's a hack since it sounds like since you're relying on some
other entity to have enabled resources that this clock controller
depends on.

> > I believe support for clock controllers that need to enable multiple
> > power domains is on its way into 6.15:
> > 
> > 	https://lore.kernel.org/lkml/20250117-b4-linux-next-24-11-18-clock-multiple-power-domains-v10-0-13f2bb656dad@linaro.org/
> > 
> > Perhaps that's what you need to fix this properly.
> 
> Yes, this is just to add a dependency on clock controller to put the 
> rail vote, but this series does not fully solve the clock controller's 
> PLL requirement problems.

Why not? What else is needed beyond enabling the video (?) power domain
before configuring the PLL?

Johan