Messages in this thread | | | Subject | Re: [PATCH v10 3/5] drm/msm/dp: add support for DP PLL driver | From | Dmitry Baryshkov <> | Date | Sat, 15 Aug 2020 14:45:17 +0300 |
| |
On 15/08/2020 02:22, Tanmay Shah wrote: > On 2020-08-14 10:05, Dmitry Baryshkov wrote: >> On 12/08/2020 07:42, Tanmay Shah wrote: >>> From: Chandan Uddaraju <chandanu@codeaurora.org> >>> >>> 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?
Not exactly. We'd need fixed_factor_clk[4] for 10nm rather than 6. It's not that important, just a small nitpick.
>>> + 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.
Please exuse me for not being exact enough. Of course I meant dp_vco_FOO_10nm() functions, not init/exit.
BTW: as I'm looking onto 7nm dp pll, which naming would you prefer?
We can have separate DP_PLL_10nm/DP_PLL_7nm namespaces (as DSI PLLs do) or I can override only a set of constants (like downstream driver does).
-- With best wishes Dmitry
| |