Re: [PATCH v10 3/5] drm/msm/dp: add support for DP PLL driver

From: Tanmay Shah
Date: Fri Aug 14 2020 - 19:23:23 EST


On 2020-08-14 10:05, Dmitry Baryshkov wrote:
On 12/08/2020 07:42, Tanmay Shah wrote:
From: Chandan Uddaraju <chandanu@xxxxxxxxxxxxxx>

Add the needed DP PLL specific files to support
display port interface on msm targets.

[skipped]

diff --git a/drivers/gpu/drm/msm/dp/dp_pll_private.h b/drivers/gpu/drm/msm/dp/dp_pll_private.h
new file mode 100644
index 000000000000..475ba6ed59ab
--- /dev/null
+++ b/drivers/gpu/drm/msm/dp/dp_pll_private.h
@@ -0,0 +1,98 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __DP_PLL_10NM_H
+#define __DP_PLL_10NM_H
+
+#include "dp_pll.h"
+#include "dp_reg.h"
+
+#define DP_VCO_HSCLK_RATE_1620MHZDIV1000 1620000UL
+#define DP_VCO_HSCLK_RATE_2700MHZDIV1000 2700000UL
+#define DP_VCO_HSCLK_RATE_5400MHZDIV1000 5400000UL
+#define DP_VCO_HSCLK_RATE_8100MHZDIV1000 8100000UL
+
+#define NUM_DP_CLOCKS_MAX 6
+
+#define DP_PHY_PLL_POLL_SLEEP_US 500
+#define DP_PHY_PLL_POLL_TIMEOUT_US 10000
+
+#define DP_VCO_RATE_8100MHZDIV1000 8100000UL
+#define DP_VCO_RATE_9720MHZDIV1000 9720000UL
+#define DP_VCO_RATE_10800MHZDIV1000 10800000UL
+
+struct dp_pll_vco_clk {
+ struct clk_hw hw;
+ unsigned long rate; /* current vco rate */
+ u64 min_rate; /* min vco rate */
+ u64 max_rate; /* max vco rate */
+ void *priv;
+};
+
+struct dp_pll_db {

This struct should probably go into dp_pll_10nm.c. dp_pll_7nm.c, for
example, will use slightly different structure.


Sure, it sounds good. I will give it try. Thanks!

+ struct msm_dp_pll *base;
+
+ int id;
+ struct platform_device *pdev;
+
+ /* private clocks: */
+ bool fixed_factor_clk[NUM_DP_CLOCKS_MAX];
+ struct clk_hw *hws[NUM_DP_CLOCKS_MAX];

Then these two fields can use exact number of clocks rather than
NUM_DP_CLOCKS_MAX.


I didn't get this. I think NUM_DP_CLOCKS_MAX is doing same?

+ u32 num_hws;
+
+ /* lane and orientation settings */
+ u8 lane_cnt;
+ u8 orientation;
+
+ /* COM PHY settings */
+ u32 hsclk_sel;
+ u32 dec_start_mode0;
+ u32 div_frac_start1_mode0;
+ u32 div_frac_start2_mode0;
+ u32 div_frac_start3_mode0;
+ u32 integloop_gain0_mode0;
+ u32 integloop_gain1_mode0;
+ u32 vco_tune_map;
+ u32 lock_cmp1_mode0;
+ u32 lock_cmp2_mode0;
+ u32 lock_cmp3_mode0;
+ u32 lock_cmp_en;
+
+ /* PHY vco divider */
+ u32 phy_vco_div;
+ /*
+ * Certain pll's needs to update the same vco rate after resume in
+ * suspend/resume scenario. Cached the vco rate for such plls.
+ */
+ unsigned long vco_cached_rate;
+ u32 cached_cfg0;
+ u32 cached_cfg1;
+ u32 cached_outdiv;
+
+ uint32_t index;
+};
+
+static inline struct dp_pll_vco_clk *to_dp_vco_hw(struct clk_hw *hw)
+{
+ return container_of(hw, struct dp_pll_vco_clk, hw);
+}
+
+#define to_msm_dp_pll(vco) ((struct msm_dp_pll *)vco->priv)
+
+#define to_dp_pll_db(x) ((struct dp_pll_db *)x->priv)
+
+int dp_vco_set_rate_10nm(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate);
+unsigned long dp_vco_recalc_rate_10nm(struct clk_hw *hw,
+ unsigned long parent_rate);
+long dp_vco_round_rate_10nm(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate);
+int dp_vco_prepare_10nm(struct clk_hw *hw);
+void dp_vco_unprepare_10nm(struct clk_hw *hw);
+
+int msm_dp_pll_10nm_init(struct msm_dp_pll *dp_pll, int id);
+void msm_dp_pll_10nm_deinit(struct msm_dp_pll *dp_pll);

These functions don't seem to be used outside of dp_pll_10nm. What
about making them static?

I can't declare static to "init" and "deinit" as they are exported to dp_pll.c.
Rest of them I can move to dp_pll_10nm and then define static.