From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> To: Tanmay Shah <tanmay@codeaurora.org> Cc: swboyd@chromium.org, devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, robdclark@gmail.com, linux-kernel@vger.kernel.org, freedreno@lists.freedesktop.org, seanpaul@chromium.org, daniel@ffwll.ch, airlied@linux.ie, aravindh@codeaurora.org, abhinavk@codeaurora.org, khsieh@codeaurora.org, Chandan Uddaraju <chandanu@codeaurora.org>, Vara Reddy <varar@codeaurora.org> Subject: Re: [PATCH v10 3/5] drm/msm/dp: add support for DP PLL driver Date: Sat, 15 Aug 2020 14:45:17 +0300 [thread overview] Message-ID: <28b1f678-ab8f-cf6a-af9f-fcd79131bdc1@linaro.org> (raw) In-Reply-To: <f6b330778c07abd3003da9acab4d3398@codeaurora.org> 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
WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> To: Tanmay Shah <tanmay@codeaurora.org> Cc: devicetree@vger.kernel.org, airlied@linux.ie, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, swboyd@chromium.org, khsieh@codeaurora.org, seanpaul@chromium.org, abhinavk@codeaurora.org, Vara Reddy <varar@codeaurora.org>, aravindh@codeaurora.org, freedreno@lists.freedesktop.org, Chandan Uddaraju <chandanu@codeaurora.org> Subject: Re: [PATCH v10 3/5] drm/msm/dp: add support for DP PLL driver Date: Sat, 15 Aug 2020 14:45:17 +0300 [thread overview] Message-ID: <28b1f678-ab8f-cf6a-af9f-fcd79131bdc1@linaro.org> (raw) In-Reply-To: <f6b330778c07abd3003da9acab4d3398@codeaurora.org> 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-08-15 22:01 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-08-12 4:42 [PATCH v10 0/5] Add support for DisplayPort driver on SnapDragon Tanmay Shah 2020-08-12 4:42 ` Tanmay Shah 2020-08-12 4:42 ` [PATCH v10 1/5] drm: add constant N value in helper file Tanmay Shah 2020-08-12 4:42 ` Tanmay Shah 2020-08-12 4:42 ` [PATCH v10 2/5] drm/msm/dp: add displayPort driver support Tanmay Shah 2020-08-12 4:42 ` Tanmay Shah 2020-08-14 17:12 ` Dmitry Baryshkov 2020-08-14 17:12 ` Dmitry Baryshkov 2020-08-14 17:56 ` [Freedreno] " Tanmay Shah 2020-08-14 17:56 ` Tanmay Shah 2020-08-14 21:14 ` Tanmay Shah 2020-08-14 21:14 ` Tanmay Shah 2020-08-12 4:42 ` [PATCH v10 3/5] drm/msm/dp: add support for DP PLL driver Tanmay Shah 2020-08-12 4:42 ` Tanmay Shah 2020-08-14 17:05 ` Dmitry Baryshkov 2020-08-14 17:05 ` Dmitry Baryshkov 2020-08-14 23:22 ` Tanmay Shah 2020-08-14 23:22 ` Tanmay Shah 2020-08-15 11:45 ` Dmitry Baryshkov [this message] 2020-08-15 11:45 ` Dmitry Baryshkov 2020-08-17 18:06 ` Tanmay Shah 2020-08-17 18:06 ` Tanmay Shah 2020-08-15 20:20 ` Rob Clark 2020-08-15 20:20 ` Rob Clark 2020-08-15 21:21 ` [Freedreno] " Jonathan Marek 2020-08-15 21:21 ` Jonathan Marek 2020-08-15 22:45 ` Rob Clark 2020-08-15 22:45 ` Rob Clark 2020-08-17 20:00 ` Tanmay Shah 2020-08-17 20:00 ` Tanmay Shah 2020-08-17 20:32 ` Dmitry Baryshkov 2020-08-17 20:32 ` Dmitry Baryshkov 2020-08-17 20:37 ` Rob Clark 2020-08-17 20:37 ` Rob Clark 2020-08-12 4:42 ` [PATCH v10 4/5] drm/msm/dpu: add display port support in DPU Tanmay Shah 2020-08-12 4:42 ` Tanmay Shah 2020-08-12 4:42 ` [PATCH v10 5/5] drm/msm/dp: Add Display Port HPD feature Tanmay Shah 2020-08-12 4:42 ` Tanmay Shah
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=28b1f678-ab8f-cf6a-af9f-fcd79131bdc1@linaro.org \ --to=dmitry.baryshkov@linaro.org \ --cc=abhinavk@codeaurora.org \ --cc=airlied@linux.ie \ --cc=aravindh@codeaurora.org \ --cc=chandanu@codeaurora.org \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=freedreno@lists.freedesktop.org \ --cc=khsieh@codeaurora.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=robdclark@gmail.com \ --cc=seanpaul@chromium.org \ --cc=swboyd@chromium.org \ --cc=tanmay@codeaurora.org \ --cc=varar@codeaurora.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.