| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c5662d54073bc6d3f7ea872cb2bbbcdb@codeaurora.org>
Date: Mon, 17 Aug 2020 11:06:51 -0700
From: Tanmay Shah <tanmay@...eaurora.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: devicetree@...r.kernel.org, airlied@...ux.ie,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org, swboyd@...omium.org,
khsieh@...eaurora.org, seanpaul@...omium.org,
abhinavk@...eaurora.org, Vara Reddy <varar@...eaurora.org>,
aravindh@...eaurora.org, freedreno@...ts.freedesktop.org,
Chandan Uddaraju <chandanu@...eaurora.org>,
dri-devel <dri-devel-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH v10 3/5] drm/msm/dp: add support for DP PLL driver
On 2020-08-15 04:45, Dmitry Baryshkov wrote:
> 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@...eaurora.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.
>
Ok got it. Sorry I didn't mean to nitpick here.
> BTW: as I'm looking onto 7nm dp pll, which naming would you prefer?
>
I didn't get this. Did you mean naming convention of functions and
structure between
7nm and 10 nm? Could you point me where 7nm dp pll code is posted?
> 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).
Having separate namespace sounds good.
Powered by blists - more mailing lists