| Message ID | 20250515-videocc-pll-multi-pd-voting-v4-0-571c63297d01@quicinc.com |
|---|---|
| Headers | show |
| Series | clk: qcom: Add support to attach multiple power domains in cc probe | expand |
On Thu, May 15, 2025 at 12:38:49AM +0530, Jagadeesh Kona wrote: > From: Taniya Das <quic_tdas@quicinc.com> > > To properly configure the PLLs on recent chipsets, it often requires more > than one power domain to be kept ON. The support to enable multiple power > domains is being added in qcom_cc_really_probe() and PLLs should be > configured post all the required power domains are enabled. > > Hence integrate PLL configuration into clk_alpha_pll structure and add > support for qcom_clk_alpha_pll_configure() function which can be called > from qcom_cc_really_probe() to configure the clock controller PLLs after > all required power domains are enabled. > > Signed-off-by: Taniya Das <quic_tdas@quicinc.com> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- > drivers/clk/qcom/clk-alpha-pll.c | 57 ++++++++++++++++++++++++++++++++++++++++ > drivers/clk/qcom/clk-alpha-pll.h | 3 +++ > 2 files changed, 60 insertions(+) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On Thu, May 15, 2025 at 12:38:56AM +0530, Jagadeesh Kona wrote: > Camera PLLs on SM8650 require both MMCX and MXC rails to be kept ON > to configure the PLLs properly. Hence move runtime power management, > PLL configuration and enabling critical clocks to qcom_cc_really_probe() > which ensures all required power domains are in enabled state before > configuring the PLLs or enabling the clocks. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- > drivers/clk/qcom/camcc-sm8650.c | 83 +++++++++++++++++++++-------------------- > 1 file changed, 42 insertions(+), 41 deletions(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On Thu, May 15, 2025 at 12:38:57AM +0530, Jagadeesh Kona wrote: > Camera PLLs on X1E80100 require both MMCX and MXC rails to be kept ON > to configure the PLLs properly. Hence move runtime power management, > PLL configuration and enabling critical clocks to qcom_cc_really_probe() > which ensures all required power domains are in enabled state before > configuring the PLLs or enabling the clocks. > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Dell Inspiron > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- > drivers/clk/qcom/camcc-x1e80100.c | 67 +++++++++++++++++++-------------------- > 1 file changed, 32 insertions(+), 35 deletions(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On Thu, May 15, 2025 at 12:39:01AM +0530, Jagadeesh Kona wrote: > Camcc requires both MMCX and MXC rails to be powered ON to configure > the camera PLLs on SM8450 platform. Hence add MXC power domain to > camcc node on SM8450. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sm8450.dtsi | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On Thu, May 15, 2025 at 12:39:02AM +0530, Jagadeesh Kona wrote: > From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > > Camcc requires both MMCX and MXC rails to be powered ON to configure > the camera PLLs on SM8550 platform. Hence add MXC power domain to > camcc node on SM8550. While at it, update SM8550_MMCX macro to RPMHPD_MMCX > to align towards common macros. > > Fixes: e271b59e39a6f ("arm64: dts: qcom: sm8550: Add camera clock controller") > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > Reviewed-by: Taniya Das <quic_tdas@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sm8550.dtsi | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On Thu, May 15, 2025 at 12:39:03AM +0530, Jagadeesh Kona wrote: > Camcc requires both MMCX and MXC rails to be powered ON to configure > the camera PLLs on SM8650 platform. Hence add MXC power domain to > camcc node on SM8650. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On Thu, May 15, 2025 at 12:38:51AM +0530, Jagadeesh Kona wrote: > Add support to configure PLLS and clk registers in qcom_cc_really_probe(). > This ensures all required power domains are enabled and kept ON by runtime > PM code in qcom_cc_really_probe() before configuring the PLLS or clock > registers. > > Add support for qcom_cc_driver_data struct to maintain the clock > controllers PLLs and CBCRs data, and a pointer of it can be stored in > clock descriptor structure. If any clock controller driver requires to > program some additional misc register settings, it can register the > clk_regs_configure() callback in the driver data. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- > drivers/clk/qcom/common.c | 39 +++++++++++++++++++++++++++++++++++++++ > drivers/clk/qcom/common.h | 9 +++++++++ > 2 files changed, 48 insertions(+) Not quite what I expected, but it works and looks good. Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On 14/05/2025 20:08, Jagadeesh Kona wrote: > Add support for runtime power management in qcom_cc_really_probe() to > commonize it across all the clock controllers. The runtime power management > is not required for all clock controllers, hence handle the rpm based on > use_rpm flag in clock controller descriptor. > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 14/05/2025 20:08, Jagadeesh Kona wrote: > + if (!pll->config || !pll->regs) { > + pr_err("%s: missing pll config or regs\n", init->name); > + continue; > + } If you are printing error, why aren't you returning error ? I understand that it probably makes platform bringup easier if we print instead of error here. I think this should be a failure case with a -EINVAL or some other indicator you prefer. Assuming you amend to return an error you may add my Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 5/14/25 9:09 PM, Jagadeesh Kona wrote: > Camcc requires both MMCX and MXC rails to be powered ON to configure > the camera PLLs on SM8650 platform. Hence add MXC power domain to > camcc node on SM8650. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:09 PM, Jagadeesh Kona wrote: > From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > > Camcc requires both MMCX and MXC rails to be powered ON to configure > the camera PLLs on SM8550 platform. Hence add MXC power domain to > camcc node on SM8550. While at it, update SM8550_MMCX macro to RPMHPD_MMCX > to align towards common macros. > > Fixes: e271b59e39a6f ("arm64: dts: qcom: sm8550: Add camera clock controller") > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > Reviewed-by: Taniya Das <quic_tdas@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:09 PM, Jagadeesh Kona wrote: > Camcc requires both MMCX and MXC rails to be powered ON to configure > the camera PLLs on SM8450 platform. Hence add MXC power domain to > camcc node on SM8450. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:08 PM, Jagadeesh Kona wrote: > Video PLLs on SM8450/SM8475 require both MMCX and MXC rails to be kept ON > to configure the PLLs properly. Hence move runtime power management, PLL > configuration and enable critical clocks to qcom_cc_really_probe() which > ensures all required power domains are in enabled state before configuring > the PLLs or enabling the clocks. > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:08 PM, Jagadeesh Kona wrote: > Video PLLs on SM8550/SM8650 require both MMCX and MXC rails to be kept ON > to configure the PLLs properly. Hence move runtime power management, PLL > configuration and enable critical clocks to qcom_cc_really_probe() which > ensures all required power domains are in enabled state before configuring > the PLLs or enabling the clocks. > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- [...] > - > - pm_runtime_put(&pdev->dev); > + /* Sleep clock offset changed to 0x8150 on SM8650 */ > + video_cc_sm8550_critical_cbcrs[2] = 0x8150; > + } Because we tend to sort these by address, this index will likely break the next time someone touches this please introduce a separate array for 8650 instead Konrad
On 5/14/25 9:08 PM, Jagadeesh Kona wrote: > Camera PLLs on SM8450/SM8475 require both MMCX and MXC rails to be > kept ON to configure the PLLs properly. Hence move runtime power > management, PLL configuration and enable critical clocks to > qcom_cc_really_probe() which ensures all required power domains are in > enabled state before configuring the PLLs or enabling the clocks. > > This change also removes the modelling for cam_cc_gdsc_clk and keeps it > always ON from probe since using CLK_IS_CRITICAL will prevent the clock > controller associated power domains from collapsing due to clock framework > invoking clk_pm_runtime_get() during prepare. generally we want two commits for such things, but let's say this time it's okay Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:08 PM, Jagadeesh Kona wrote: > Camera PLLs on SM8550 require both MMCX and MXC rails to be kept ON to > configure the PLLs properly. Hence move runtime power management, PLL > configuration and enabling critical clocks to qcom_cc_really_probe() which > ensures all required power domains are in enabled state before configuring > the PLLs or enabling the clocks. > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:08 PM, Jagadeesh Kona wrote: > Camera PLLs on SM8650 require both MMCX and MXC rails to be kept ON > to configure the PLLs properly. Hence move runtime power management, > PLL configuration and enabling critical clocks to qcom_cc_really_probe() > which ensures all required power domains are in enabled state before > configuring the PLLs or enabling the clocks. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:08 PM, Jagadeesh Kona wrote: > Camera PLLs on X1E80100 require both MMCX and MXC rails to be kept ON > to configure the PLLs properly. Hence move runtime power management, > PLL configuration and enabling critical clocks to qcom_cc_really_probe() > which ensures all required power domains are in enabled state before > configuring the PLLs or enabling the clocks. > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Dell Inspiron > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:08 PM, Jagadeesh Kona wrote: > Videocc requires both MMCX and MXC rails to be powered ON to configure > the video PLLs on SM8450 platform. Hence add MXC power domain to videocc > node on SM8450. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:08 PM, Jagadeesh Kona wrote: > Videocc requires both MMCX and MXC rails to be powered ON to configure > the video PLLs on SM8550 platform. Hence add MXC power domain to videocc > node on SM8550. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/14/25 9:09 PM, Jagadeesh Kona wrote: > Videocc requires both MMCX and MXC rails to be powered ON to configure > the video PLLs on SM8650 platform. Hence add MXC power domain to videocc > node on SM8650. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On 5/15/2025 12:04 PM, Bryan O'Donoghue wrote: > On 14/05/2025 20:08, Jagadeesh Kona wrote: >> + if (!pll->config || !pll->regs) { >> + pr_err("%s: missing pll config or regs\n", init->name); >> + continue; >> + } > > If you are printing error, why aren't you returning error ? > > I understand that it probably makes platform bringup easier if we print instead of error here. > > I think this should be a failure case with a -EINVAL or some other indicator you prefer. > > Assuming you amend to return an error you may add my > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Sure, will check and return error in case of failure in next series Thanks, Jagadeesh
On 5/15/2025 9:08 PM, Konrad Dybcio wrote: > On 5/14/25 9:08 PM, Jagadeesh Kona wrote: >> Video PLLs on SM8550/SM8650 require both MMCX and MXC rails to be kept ON >> to configure the PLLs properly. Hence move runtime power management, PLL >> configuration and enable critical clocks to qcom_cc_really_probe() which >> ensures all required power domains are in enabled state before configuring >> the PLLs or enabling the clocks. >> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> >> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >> --- > > [...] > >> - >> - pm_runtime_put(&pdev->dev); >> + /* Sleep clock offset changed to 0x8150 on SM8650 */ >> + video_cc_sm8550_critical_cbcrs[2] = 0x8150; >> + } > > Because we tend to sort these by address, this index will likely break > the next time someone touches this > > please introduce a separate array for 8650 instead > Sure, will introduce separate array for SM8650 in next series. Thanks, Jagadeesh > Konrad
In recent QCOM chipsets, PLLs require more than one power domain to be kept ON to configure the PLL. But the current code doesn't enable all the required power domains while configuring the PLLs, this leads to functional issues due to suboptimal settings of PLLs. To address this, add support for handling runtime power management, configuring plls and enabling critical clocks from qcom_cc_really_probe. The clock controller can specify PLLs, critical clocks, and runtime PM requirements using the descriptor data. The code in qcom_cc_really_probe() ensures all necessary power domains are enabled before configuring PLLs or critical clocks. This series fixes the below warning reported in SM8550 venus testing due to video_cc_pll0 not properly getting configured during videocc probe [ 46.535132] Lucid PLL latch failed. Output may be unstable! The patch adding support to configure the PLLs from common code is picked from below series and updated it. https://lore.kernel.org/all/20250113-support-pll-reconfigure-v1-0-1fae6bc1062d@quicinc.com/ This series is dependent on bindings patch in below Vladimir's series, hence included the Vladimir's series patches also in this series and updated them. https://lore.kernel.org/all/20250303225521.1780611-1-vladimir.zapolskiy@linaro.org/ Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> --- Changes in v4: - Updated the SC8280XP camcc bindings patch to fix the required-opps warning reported by kernel bot - Updated the description of power-domains, required-opps of SM8450 camcc bindings as per review comments on v3 [Bryan] - Moved the PLL config checks to calling function code [Dmitry] - Removed qcom_clk_reg_setting struct and regmap_update_bits() code. Added a .clk_regs_configure() callback that clock drivers can implement if they require to update some misc register settings [Dmitry] - Moved the PLLs and CBCRs data to a separate qcom_cc_driver_data struct to avoid bloating up the CC descriptor structure - Updated the videocc and camcc driver patches to incorporate above qcom_cc_driver_data change - Updated the commit text of DT patches [Bryan] - Added the R-By, T-By tags received on v3 - Link to v3: https://lore.kernel.org/r/20250327-videocc-pll-multi-pd-voting-v3-0-895fafd62627@quicinc.com Changes in v3: - Updated the videocc bindings patch to add required-opps for MXC power domain [Dmitry] and added Bryan & Rob R/A-By tags received for this patch on v1. - Included the Vladimir's bindings patch for SM8450 camcc bindings to add multiple PD support and updated them to fix the bot warnings. - Moved SC8280XP camcc bindings to SA8775P camcc since SC8280XP only require single MMCX power domain - Split runtime PM and PLL configuration to separate patches [Dmitry] - Removed direct regmap_update_bits to configure clock CBCR's and using clock helpers to configure the CBCR registers [Dmitry, Bryan] - Added new helpers to configure all PLLs & update misc clock register settings from common code [Dmitry, Bryan] - Updated the name of qcom_clk_cfg structure to qcom_clk_reg_setting [Konrad] - Updated the fields in structure from unsigned int to u32 and added val field to this structure [Konrad] - Added a new u32 array for cbcr branch clocks & num_clk_cbcrs fields to maintain the list of critical clock cbcrs in clock controller descriptor [Konrad] - Updated the plls field to alpha_plls in descriptor structure [Konrad] - Added WARN() in PLL configure function if PLL type passed is not supported. The suggestion is to use BUG(), but updated it to WARN() to avoid checkpatch warning. [Bjorn] - Moved the pll configure and helper macros to PLL code from common code [Bjorn] - Updated camcc drivers for SM8450, SM8550, SM8650 and X1E80100 targets with support to configure PLLs from common code and added MXC power domain in corresponding camcc DT nodes. [Bryan] - Added Dmitry and Bryan R-By tags received on videocc DT node changes in v1 - Link to v2: https://lore.kernel.org/r/20250306-videocc-pll-multi-pd-voting-v2-0-0cd00612bc0e@quicinc.com Changes in v2: - Added support to handle rpm, PLL configuration and enable critical clocks from qcom_cc_really_probe() in common code as per v1 commments from Bryan, Konrad and Dmitry - Added patches to configure PLLs from common code - Updated the SM8450, SM8550 videocc patches to use the newly added support to handle rpm, configure PLLs from common code - Split the DT change for each target separately as per Dmitry comments - Added R-By and A-By tags received on v1 - Link to v1: https://lore.kernel.org/r/20250218-videocc-pll-multi-pd-voting-v1-0-cfe6289ea29b@quicinc.com --- Jagadeesh Kona (15): dt-bindings: clock: qcom,sm8450-videocc: Add MXC power domain dt-bindings: clock: qcom: Update sc8280xp camcc bindings clk: qcom: common: Handle runtime power management in qcom_cc_really_probe clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe clk: qcom: videocc-sm8450: Move PLL & clk configuration to really probe clk: qcom: videocc-sm8550: Move PLL & clk configuration to really probe clk: qcom: camcc-sm8450: Move PLL & clk configuration to really probe clk: qcom: camcc-sm8550: Move PLL & clk configuration to really probe clk: qcom: camcc-sm8650: Move PLL & clk configuration to really probe clk: qcom: camcc-x1e80100: Move PLL & clk configuration to really probe arm64: dts: qcom: sm8450: Additionally manage MXC power domain in videocc arm64: dts: qcom: sm8550: Additionally manage MXC power domain in videocc arm64: dts: qcom: sm8650: Additionally manage MXC power domain in videocc arm64: dts: qcom: sm8450: Additionally manage MXC power domain in camcc arm64: dts: qcom: sm8650: Additionally manage MXC power domain in camcc Taniya Das (1): clk: qcom: clk-alpha-pll: Add support for common PLL configuration function Vladimir Zapolskiy (2): dt-bindings: clock: qcom: sm8450-camcc: Allow to specify two power domains arm64: dts: qcom: sm8550: Additionally manage MXC power domain in camcc .../bindings/clock/qcom,sa8775p-camcc.yaml | 15 ++++ .../bindings/clock/qcom,sm8450-camcc.yaml | 20 +++-- .../bindings/clock/qcom,sm8450-videocc.yaml | 18 +++-- arch/arm64/boot/dts/qcom/sm8450.dtsi | 12 ++- arch/arm64/boot/dts/qcom/sm8550.dtsi | 12 ++- arch/arm64/boot/dts/qcom/sm8650.dtsi | 6 +- drivers/clk/qcom/camcc-sm8450.c | 89 +++++++++++----------- drivers/clk/qcom/camcc-sm8550.c | 85 +++++++++++---------- drivers/clk/qcom/camcc-sm8650.c | 83 ++++++++++---------- drivers/clk/qcom/camcc-x1e80100.c | 67 ++++++++-------- drivers/clk/qcom/clk-alpha-pll.c | 57 ++++++++++++++ drivers/clk/qcom/clk-alpha-pll.h | 3 + drivers/clk/qcom/common.c | 76 +++++++++++++++--- drivers/clk/qcom/common.h | 10 +++ drivers/clk/qcom/videocc-sm8450.c | 58 ++++++-------- drivers/clk/qcom/videocc-sm8550.c | 59 +++++++------- 16 files changed, 409 insertions(+), 261 deletions(-) --- base-commit: 138cfc44b3c4a5fb800388c6e27be169970fb9f7 change-id: 20250218-videocc-pll-multi-pd-voting-d614dce910e7 Best regards,