[DPU PATCH v4 4/5] drm/msm/dp: add support for DP PLL driver
varar at codeaurora.org
varar at codeaurora.org
Fri Mar 27 20:33:01 UTC 2020
Hi Stephen Boyd,
I have updated our comments inline for the queries.
Could you please check and let know your views.
I have addressed most of the comments from Rob/Jordon and planning to
update new patchset in couple of days.
Thanks,
--Vara
On 2020-03-27 13:28, varar at codeaurora.org wrote:
> On 2020-03-19 15:17, Stephen Boyd wrote:
>> Quoting Vara Reddy (2020-03-04 16:10:27)
>>> From: Chandan Uddaraju <chandanu at codeaurora.org>
>>>
>>> Add the needed DP PLL specific files to support
>>> display port interface on msm targets.
>>>
>>> The DP driver calls the DP PLL driver registration.
>>> The DP driver sets the link and pixel clock sources.
>>>
>>> Changes in v2:
>>> -- Update copyright markings on all relevant files.
>>> -- Use DRM_DEBUG_DP for debug msgs.
>>>
>>> Changes in v4:
>>> -- Update the DP link clock provider names
>>>
>>> Signed-off-by: Chandan Uddaraju <chandanu at codeaurora.org>
>>> Signed-off-by: Vara Reddy <varar at codeaurora.org>
>>> ---
>>> drivers/gpu/drm/msm/Kconfig | 13 +
>>> drivers/gpu/drm/msm/Makefile | 4 +
>>> drivers/gpu/drm/msm/dp/dp_display.c | 51 +++
>>> drivers/gpu/drm/msm/dp/dp_display.h | 3 +
>>> drivers/gpu/drm/msm/dp/dp_parser.h | 4 +
>>> drivers/gpu/drm/msm/dp/dp_power.h | 1 -
>>> drivers/gpu/drm/msm/dp/pll/dp_pll.c | 136 +++++++
>>> drivers/gpu/drm/msm/dp/pll/dp_pll.h | 57 +++
>>> drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c | 406
>>> ++++++++++++++++++++
>>> drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h | 86 +++++
>>> drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c | 524
>>> ++++++++++++++++++++++++++
>>> 11 files changed, 1284 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll.c
>>> create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll.h
>>> create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c
>>> create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h
>>> create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c
>>>
>>> diff --git a/drivers/gpu/drm/msm/Kconfig
>>> b/drivers/gpu/drm/msm/Kconfig
>>> index 7946cb1..e73ad23 100644
>>> --- a/drivers/gpu/drm/msm/Kconfig
>>> +++ b/drivers/gpu/drm/msm/Kconfig
>>> @@ -66,6 +66,19 @@ config DRM_MSM_DP
>>> display support is enabled through this config option. It
>>> can
>>> be primary or secondary display on device.
>>>
>>> +config DRM_MSM_DP_PLL
>>> + bool "Enable DP PLL driver in MSM DRM"
>>> + depends on DRM_MSM_DP && COMMON_CLK
>>> + help
>>> + Choose this option to enable DP PLL driver which provides
>>> DP
>>> + source clocks under common clock framework.
>>> +
>>> +config DRM_MSM_DP_10NM_PLL
>>> + bool "Enable DP 10nm PLL driver in MSM DRM (used by SDM845)"
>>> + depends on DRM_MSM_DP_PLL
>>> + help
>>> + Choose this option if DP PLL on SDM845 is used on the
>>> platform.
>>> +
>>> config DRM_MSM_DSI
>>> bool "Enable DSI support in MSM DRM driver"
>>> depends on DRM_MSM
>>> diff --git a/drivers/gpu/drm/msm/Makefile
>>> b/drivers/gpu/drm/msm/Makefile
>>> index c846599..1621f3f 100644
>>> --- a/drivers/gpu/drm/msm/Makefile
>>> +++ b/drivers/gpu/drm/msm/Makefile
>>> @@ -140,4 +140,8 @@ msm-$(CONFIG_DRM_MSM_DSI_14NM_PHY) +=
>>> dsi/pll/dsi_pll_14nm.o
>>> msm-$(CONFIG_DRM_MSM_DSI_10NM_PHY) += dsi/pll/dsi_pll_10nm.o
>>> endif
>>>
>>> +msm-$(CONFIG_DRM_MSM_DP_PLL)+= dp/pll/dp_pll.o
>>> +msm-$(CONFIG_DRM_MSM_DP_10NM_PLL)+= dp/pll/dp_pll_10nm.o \
>>> + dp/pll/dp_pll_10nm_util.o
>>> +
>>> obj-$(CONFIG_DRM_MSM) += msm.o
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
>>> b/drivers/gpu/drm/msm/dp/dp_display.c
>>> index 5525405..ded8f5c 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>> @@ -61,6 +61,51 @@ struct dp_display_private {
>>> {}
>>> };
>>>
>>> +static int dp_get_pll(struct dp_display_private *dp_priv)
>>> +{
>>> + struct platform_device *pdev = NULL;
>>> + struct platform_device *pll_pdev;
>>> + struct device_node *pll_node;
>>> + struct dp_parser *dp_parser = NULL;
>>> +
>>> + if (!dp_priv) {
>>> + DRM_ERROR("Invalid Arguments\n");
>>
>> It's a static function. Is this even possible? The bad code would be
>> in
>> the same file.
>>
>>> + return -EINVAL;
>>> + }
>>> +
>>> + pdev = dp_priv->pdev;
>>> + dp_parser = dp_priv->parser;
>>> +
>>> + if (!dp_parser) {
>>> + DRM_DEV_ERROR(&pdev->dev, "%s: Parser not
>>> initialized\n",
>>> + __func__);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + pll_node = of_parse_phandle(pdev->dev.of_node, "pll-node",
>>> 0);
>>> + if (!pll_node) {
>>> + DRM_DEV_ERROR(&pdev->dev, "%s: cannot find pll
>>> device\n",
>>> + __func__);
>>> + return -ENXIO;
>>> + }
>>> +
>>> + pll_pdev = of_find_device_by_node(pll_node);
>>> + if (pll_pdev)
>>> + dp_parser->pll = platform_get_drvdata(pll_pdev);
>>> +
>>> + of_node_put(pll_node);
>>> +
>>> + if (!pll_pdev || !dp_parser->pll) {
>>> + DRM_DEV_ERROR(&pdev->dev, "%s: pll driver is not
>>> ready\n",
>>> + __func__);
>>> + return -EPROBE_DEFER;
>>> + }
>>> +
>>> + dp_parser->pll_dev = get_device(&pll_pdev->dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static irqreturn_t dp_display_irq(int irq, void *dev_id)
>>> {
>>> struct dp_display_private *dp = dev_id;
>>> @@ -114,6 +159,10 @@ static int dp_display_bind(struct device *dev,
>>> struct device *master,
>>> goto end;
>>> }
>>>
>>> + rc = dp_get_pll(dp);
>>> + if (rc)
>>> + goto end;
>>> +
>>> rc = dp_aux_register(dp->aux);
>>> if (rc) {
>>> DRM_ERROR("DRM DP AUX register failed\n");
>>> @@ -837,6 +886,7 @@ int __init msm_dp_register(void)
>>> {
>>> int ret;
>>>
>>> + msm_dp_pll_driver_register();
>>> ret = platform_driver_register(&dp_display_driver);
>>> if (ret) {
>>> DRM_ERROR("driver register failed");
>>> @@ -848,6 +898,7 @@ int __init msm_dp_register(void)
>>>
>>> void __exit msm_dp_unregister(void)
>>> {
>>> + msm_dp_pll_driver_unregister();
>>> platform_driver_unregister(&dp_display_driver);
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
>>> b/drivers/gpu/drm/msm/dp/dp_display.h
>>> index dad8610..4c53ed5 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_display.h
>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
>>> @@ -25,4 +25,7 @@ int dp_display_get_modes(struct msm_dp *dp_display,
>>> bool dp_display_check_video_test(struct msm_dp *dp_display);
>>> int dp_display_get_test_bpp(struct msm_dp *dp_display);
>>>
>>> +void __init msm_dp_pll_driver_register(void);
>>> +void __exit msm_dp_pll_driver_unregister(void);
>>> +
>>> #endif /* _DP_DISPLAY_H_ */
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h
>>> b/drivers/gpu/drm/msm/dp/dp_parser.h
>>> index 1dc592f..e775cbc 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
>>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
>>> @@ -6,6 +6,8 @@
>>> #ifndef _DP_PARSER_H_
>>> #define _DP_PARSER_H_
>>>
>>> +#include "pll/dp_pll.h"
>>> +
>>> #define DP_LABEL "MDSS DP DISPLAY"
>>> #define AUX_CFG_LEN 10
>>> #define DP_MAX_PIXEL_CLK_KHZ 675000
>>> @@ -193,6 +195,8 @@ struct dp_parser {
>>> bool combo_phy_en;
>>> struct dp_io io;
>>> struct dp_display_data disp_data;
>>> + struct msm_dp_pll *pll;
>>> + struct device *pll_dev;
>>> const struct dp_regulator_cfg *regulator_cfg;
>>> u8 l_map[4];
>>> struct dp_aux_cfg aux_cfg[AUX_CFG_LEN];
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_power.h
>>> b/drivers/gpu/drm/msm/dp/dp_power.h
>>> index 52b6194..b24b2c5 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_power.h
>>> +++ b/drivers/gpu/drm/msm/dp/dp_power.h
>>> @@ -14,7 +14,6 @@
>>> * @init: initializes the regulators/core clocks/GPIOs/pinctrl
>>> * @deinit: turns off the regulators/core clocks/GPIOs/pinctrl
>>> * @clk_enable: enable/disable the DP clocks
>>> - * @set_link_clk_parent: set the parent of DP link clock
>>> * @set_pixel_clk_parent: set the parent of DP pixel clock
>>> */
>>> struct dp_power {
>>
>> This can be another patch to update documentation?
>>
>>> diff --git a/drivers/gpu/drm/msm/dp/pll/dp_pll.c
>>> b/drivers/gpu/drm/msm/dp/pll/dp_pll.c
>>> new file mode 100644
>>> index 0000000..11e26cf
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll.c
>>> @@ -0,0 +1,136 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2016-2020, The Linux Foundation. All rights
>>> reserved.
>>> + */
>>> +
>>> +#include "dp_pll.h"
>>> +
>>> +int msm_dp_pll_util_parse_dt_clock(struct platform_device *pdev,
>>> + struct msm_dp_pll *pll)
>>> +{
>>> + u32 i = 0, rc = 0;
>>> + struct dss_module_power *mp = &pll->mp;
>>> + const char *clock_name;
>>> + u32 clock_rate;
>>> +
>>> + mp->num_clk = of_property_count_strings(pdev->dev.of_node,
>>> +
>>> "clock-names");
>>> + if (mp->num_clk <= 0) {
>>> + DRM_DEV_ERROR(&pdev->dev, "%s:clocks are not
>>> defined\n",
>>> + __func__);
>>> + goto clk_err;
>>> + }
>>> +
>>> + mp->clk_config = devm_kzalloc(&pdev->dev,
>>
>> Use devm_kcalloc() for arrays.
>>
>>> + sizeof(struct dss_clk) * mp->num_clk,
>>> GFP_KERNEL);
>>> + if (!mp->clk_config) {
>>> + rc = -ENOMEM;
>>> + mp->num_clk = 0;
>>> + goto clk_err;
>>> + }
>>> +
>>> + for (i = 0; i < mp->num_clk; i++) {
>>> + of_property_read_string_index(pdev->dev.of_node,
>>> "clock-names",
>>> + i,
>>> &clock_name);
>>> + strlcpy(mp->clk_config[i].clk_name, clock_name,
>>> + sizeof(mp->clk_config[i].clk_name));
>>> +
>>> + of_property_read_u32_index(pdev->dev.of_node,
>>> "clock-rate",
>>> + i,
>>> &clock_rate);
>>> + mp->clk_config[i].rate = clock_rate;
>>> +
>>> + if (!clock_rate)
>>> + mp->clk_config[i].type = DSS_CLK_AHB;
>>> + else
>>> + mp->clk_config[i].type = DSS_CLK_PCLK;
>>> + }
>>> +
>>> +clk_err:
>>> + return rc;
>>> +}
>>> +
>>> +struct msm_dp_pll *msm_dp_pll_init(struct platform_device *pdev,
>>> + enum msm_dp_pll_type type, int id)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct msm_dp_pll *pll;
>>> +
>>> + switch (type) {
>>> + case MSM_DP_PLL_10NM:
>>> + pll = msm_dp_pll_10nm_init(pdev, id);
>>> + break;
>>> + default:
>>> + pll = ERR_PTR(-ENXIO);
>>> + break;
>>> + }
>>> +
>>> + if (IS_ERR(pll)) {
>>> + DRM_DEV_ERROR(dev, "%s: failed to init DP PLL\n",
>>> __func__);
>>> + return pll;
>>> + }
>>> +
>>> + pll->type = type;
>>> +
>>> + DRM_DEBUG_DP("DP:%d PLL registered", id);
>>> +
>>> + return pll;
>>> +}
>>> +
>>> +static const struct of_device_id dp_pll_dt_match[] = {
>>> + { .compatible = "qcom,dp-pll-10nm" },
>>
>> Add a terminator here { }.
>>
>>> +};
>>> +
>>> +static int dp_pll_driver_probe(struct platform_device *pdev)
>>> +{
>>> + struct msm_dp_pll *pll;
>>> + struct device *dev = &pdev->dev;
>>> + const struct of_device_id *match;
>>> +
>>> + match = of_match_node(dp_pll_dt_match, dev->of_node);
>>> + if (!match)
>>> + return -ENODEV;
>>> +
>>> + /* Currently supporting only 10nm-DP PLL */
>>
>> Please remove this useless comment that can only require being updated
>> in the future.
>>
>>> + pll = msm_dp_pll_init(pdev, MSM_DP_PLL_10NM, 0);
>>> + if (IS_ERR_OR_NULL(pll)) {
>>> + DRM_DEV_ERROR(dev,
>>> + "%s: pll init failed: %ld, need separate pll
>>> clk driver\n",
>>
>> What does this error mean?
>>
>>> + __func__, PTR_ERR(pll));
>>> + return -ENODEV;
>>> + }
>>> +
>>> + platform_set_drvdata(pdev, pll);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int dp_pll_driver_remove(struct platform_device *pdev)
>>> +{
>>> + struct msm_dp_pll *pll = platform_get_drvdata(pdev);
>>> +
>>> + if (pll)
>>> + pll = NULL;
>>> +
>>> + platform_set_drvdata(pdev, NULL);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct platform_driver dp_pll_platform_driver = {
>>> + .probe = dp_pll_driver_probe,
>>> + .remove = dp_pll_driver_remove,
>>> + .driver = {
>>> + .name = "msm_dp_pll",
>>> + .of_match_table = dp_pll_dt_match,
>>> + },
>>> +};
>>> +
>>> +void __init msm_dp_pll_driver_register(void)
>>> +{
>>> + platform_driver_register(&dp_pll_platform_driver);
>>> +}
>>> +
>>> +void __exit msm_dp_pll_driver_unregister(void)
>>> +{
>>> + platform_driver_unregister(&dp_pll_platform_driver);
>>> +}
>>> diff --git a/drivers/gpu/drm/msm/dp/pll/dp_pll.h
>>> b/drivers/gpu/drm/msm/dp/pll/dp_pll.h
>>> new file mode 100644
>>> index 0000000..185f1e9
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll.h
>>> @@ -0,0 +1,57 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright (c) 2016-2020, The Linux Foundation. All rights
>>> reserved.
>>> + */
>>> +
>>> +#ifndef __DP_PLL_H
>>> +#define __DP_PLL_H
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clk-provider.h>
>>
>> You're making a clk provider. Please Cc clk maintainers and
>> linux-clk at vger.kernel.org
>>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#include "dpu_io_util.h"
>>> +#include "msm_drv.h"
>>> +
>>> +#define PLL_REG_W(base, offset, data) \
>>> + writel((data), (base) + (offset))
>>> +#define PLL_REG_R(base, offset) readl((base) + (offset))
>>> +
>>> +enum msm_dp_pll_type {
>>> + MSM_DP_PLL_10NM,
>>> + MSM_DP_PLL_MAX
>>> +};
>>> +
>>> +struct msm_dp_pll {
>>> + enum msm_dp_pll_type type;
>>> + struct clk_hw clk_hw;
>>> + unsigned long rate; /* current vco rate */
>>> + u64 min_rate; /* min vco rate */
>>> + u64 max_rate; /* max vco rate */
>>> + bool pll_on;
>>
>> Are these struct members here to save/restore the rate across
>> suspend/resume?
>
> We are trying to retain the current vco rate across suspend/resume.
> min_rate and max_rate are the minimum and maximum vco rates that are
> possible for the chipset.
>
>>
>>> + void *priv;
>>> + /* Pll specific resources like GPIO, power supply, clocks,
>>> etc*/
>>> + struct dss_module_power mp;
>>> + int (*get_provider)(struct msm_dp_pll *pll,
>>> + struct clk **link_clk_provider,
>>> + struct clk **pixel_clk_provider);
>>> +};
>>> +
>>> +#define hw_clk_to_pll(x) container_of(x, struct msm_dp_pll, clk_hw)
>>
>> Why not to_msm_dp_pll() for this macro.
>>
>>> +
>>> +struct msm_dp_pll *msm_dp_pll_init(struct platform_device *pdev,
>>> + enum msm_dp_pll_type type, int id);
>>> +
>>> +int msm_dp_pll_util_parse_dt_clock(struct platform_device *pdev,
>>> + struct msm_dp_pll *pll);
>>> +
>>> +#ifdef CONFIG_DRM_MSM_DP_10NM_PLL
>>> +struct msm_dp_pll *msm_dp_pll_10nm_init(struct platform_device
>>> *pdev, int id);
>>> +#else
>>> +static inline struct msm_dp_pll *msm_dp_pll_10nm_init
>>> + (struct platform_device *pdev, int
>>> id)
>>
>> I would write this as
>>
>> static inline struct msm_dp_pll *
>> msm_dp_pll_10nm_init(struct platform_device *pdev, int id)
>>
>> so that we don't have parameters all by themselves.
>>
>>> +{
>>> + return ERR_PTR(-ENODEV);
>>> +}
>>> +#endif
>>> +#endif /* __DP_PLL_H */
>>> diff --git a/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c
>>> b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c
>>> new file mode 100644
>>> index 0000000..5991e09
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c
>>> @@ -0,0 +1,406 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2016-2020, The Linux Foundation. All rights
>>> reserved.
>>> + */
>>> +
>>> +/*
>>> + * Display Port PLL driver block diagram for branch clocks
>>> + *
>>> + * +------------------------------+
>>> + * | DP_VCO_CLK |
>>> + * | |
>>> + * | +-------------------+ |
>>> + * | | (DP PLL/VCO) | |
>>> + * | +---------+---------+ |
>>> + * | v |
>>> + * | +----------+-----------+ |
>>> + * | | hsclk_divsel_clk_src | |
>>> + * | +----------+-----------+ |
>>> + * +------------------------------+
>>> + * |
>>> + * +---------<---------v------------>----------+
>>> + * | |
>>> + * +--------v---------+ |
>>> + * | dp_phy_pll | |
>>> + * | link_clk | |
>>> + * +--------+---------+ |
>>> + * | |
>>> + * | |
>>> + * v v
>>> + * Input to DISPCC block |
>>> + * for link clk, crypto clk |
>>> + * and interface clock |
>>> + * |
>>> + * |
>>> + * +--------<------------+-----------------+---<---+
>>> + * | | |
>>> + * +----v---------+ +--------v-----+ +--------v------+
>>> + * | vco_divided | | vco_divided | | vco_divided |
>>> + * | _clk_src | | _clk_src | | _clk_src |
>>> + * | | | | | |
>>> + * |divsel_six | | divsel_two | | divsel_four |
>>> + * +-------+------+ +-----+--------+ +--------+------+
>>> + * | | |
>>> + * v---->----------v-------------<------v
>>> + * |
>>> + * +----------+---------+
>>> + * | dp_phy_pll_vco |
>>> + * | div_clk |
>>> + * +---------+----------+
>>> + * |
>>> + * v
>>> + * Input to DISPCC block
>>> + * for DP pixel clock
>>> + *
>>
>> Awesome diagram!
>>
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>
>> Please try to avoid making a clk provider a clk consumer at the same
>> time.
>>
>>> +#include <linux/delay.h>
>>> +#include <linux/err.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/regmap.h>
>>> +
>>> +#include "dp_pll_10nm.h"
>>> +
>>> +#define NUM_PROVIDED_CLKS 2
>>> +
>>> +#define DP_LINK_CLK_SRC 0
>>> +#define DP_PIXEL_CLK_SRC 1
>>> +
>>> +static struct dp_pll_10nm *dp_pdb;
>>> +
>>> +static const struct clk_ops dp_10nm_vco_clk_ops = {
>>> + .recalc_rate = dp_vco_recalc_rate_10nm,
>>> + .set_rate = dp_vco_set_rate_10nm,
>>> + .round_rate = dp_vco_round_rate_10nm,
>>> + .prepare = dp_vco_prepare_10nm,
>>> + .unprepare = dp_vco_unprepare_10nm,
>>> +};
>>> +
>>> +struct dp_pll_10nm_pclksel {
>>> + struct clk_hw hw;
>>> +
>>> + /* divider params */
>>> + u8 shift;
>>> + u8 width;
>>> + u8 flags; /* same flags as used by clk_divider struct */
>>> +
>>> + struct dp_pll_10nm *pll;
>>> +};
>>> +#define to_pll_10nm_pclksel(_hw) \
>>> + container_of(_hw, struct dp_pll_10nm_pclksel, hw)
>>> +
>>> +static int dp_mux_set_parent_10nm(struct clk_hw *hw, u8 val)
>>> +{
>>> + struct dp_pll_10nm_pclksel *pclksel =
>>> to_pll_10nm_pclksel(hw);
>>> + struct dp_pll_10nm *dp_res = pclksel->pll;
>>> + u32 auxclk_div;
>>> +
>>> + auxclk_div = PLL_REG_R(dp_res->phy_base, REG_DP_PHY_VCO_DIV);
>>> + auxclk_div &= ~0x03; /* bits 0 to 1 */
>>> +
>>> + if (val == 0) /* mux parent index = 0 */
>>> + auxclk_div |= 1;
>>> + else if (val == 1) /* mux parent index = 1 */
>>> + auxclk_div |= 2;
>>> + else if (val == 2) /* mux parent index = 2 */
>>> + auxclk_div |= 0;
>>
>> These comments are just saying what the code says. Can you remove
>> them?
>>
>>> +
>>> + PLL_REG_W(dp_res->phy_base,
>>> + REG_DP_PHY_VCO_DIV, auxclk_div);
>>> + DRM_DEBUG_DP("%s: mux=%d auxclk_div=%x\n", __func__, val,
>>> auxclk_div);
>>
>> Is it a mux and divider combined? The diagram seems to imply that the
>> mux picks between fixed dividers but I don't see why that isn't just a
>> divider clk because the parent is always the same for the fixed
>> dividers. Put another way, why is a parent API being used for this? It
>> would make more sense to just treat this like a divider and use rate
>> setting APIs instead.
>
> I am not sure if I completely understand the concern. I am thinking
> that you are suggesting that we remove the mux altogether
> and just use the fixed dividers, while programming the registers as
> part of the the set_rate for those dividers.
> If that is the case, then we would have to list 3 parents for the RCG
> clock in display clock controller (dispcc). However, as per the
> clock plan, the RCG clocks have a well defined set of parents (DP PLL,
> or the reference clock). It would be incorrect
> to list all these PLL specific clocks as parents of the RCG. We are
> required to list a a single provider clock for DP Pixel clock.
> Hence the choice of a mux clock.
>
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static u8 dp_mux_get_parent_10nm(struct clk_hw *hw)
>>> +{
>>> + u32 auxclk_div = 0;
>>> + struct dp_pll_10nm_pclksel *pclksel =
>>> to_pll_10nm_pclksel(hw);
>>> + struct dp_pll_10nm *dp_res = pclksel->pll;
>>> + u8 val = 0;
>>> +
>>> + auxclk_div = PLL_REG_R(dp_res->phy_base, REG_DP_PHY_VCO_DIV);
>>> + auxclk_div &= 0x03;
>>> +
>>> + if (auxclk_div == 1) /* Default divider */
>>> + val = 0;
>>> + else if (auxclk_div == 2)
>>> + val = 1;
>>> + else if (auxclk_div == 0)
>>> + val = 2;
>>> +
>>> + DRM_DEBUG_DP("%s: auxclk_div=%d, val=%d\n", __func__,
>>> auxclk_div, val);
>>> +
>>> + return val;
>>> +}
>>> +
>>> +static int clk_mux_determine_rate(struct clk_hw *hw,
>>> + struct clk_rate_request *req)
>>> +{
>>> + unsigned long rate = 0;
>>> + int ret = 0;
>>> +
>>> + rate = clk_get_rate(hw->clk);
>>> +
>>> + if (rate <= 0) {
>>> + DRM_ERROR("Rate is not set properly\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + req->rate = rate;
>>> +
>>> + DRM_DEBUG_DP("%s: rate=%ld\n", __func__, req->rate);
>>> + /* Set the new parent of mux if there is a new valid parent
>>> */
>>> + if (hw->clk && req->best_parent_hw->clk) {
>>> + ret = clk_set_parent(hw->clk,
>>> req->best_parent_hw->clk);
>>> + if (ret) {
>>> + DRM_ERROR("%s: clk_set_parent failed:
>>> ret=%d\n",
>>> + __func__, ret);
>>> + return ret;
>>> + }
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static unsigned long mux_recalc_rate(struct clk_hw *hw,
>>
>> Please prefix all functions with dp_pll_
>>
>>> + unsigned long parent_rate)
>>> +{
>>> + struct clk *div_clk = NULL, *vco_clk = NULL;
>>> + struct msm_dp_pll *vco = NULL;
>>> +
>>> + div_clk = clk_get_parent(hw->clk);
>>
>> We have clk_hw_get_parent() for this stuff. Please use it.
>>
>>> + if (!div_clk)
>>> + return 0;
>>> +
>>> + vco_clk = clk_get_parent(div_clk);
>>> + if (!vco_clk)
>>> + return 0;
>>> +
>>> + vco = hw_clk_to_pll(__clk_get_hw(vco_clk));
>>
>> Please don't use __clk_get_hw().
> Could you please elaborate on this or what to use.
> Should we use the api implementation directly.
>>
>>> + if (!vco)
>>> + return 0;
>>> +
>>> + if (vco->rate == DP_VCO_HSCLK_RATE_8100MHZDIV1000)
>>> + return (vco->rate / 6);
>>> + else if (vco->rate == DP_VCO_HSCLK_RATE_5400MHZDIV1000)
>>> + return (vco->rate / 4);
>>> + else
>>> + return (vco->rate / 2);
>>
>> else return is shunned. Just have return instead.
>>
>>> +}
>>> +
>>> +static int dp_pll_10nm_get_provider(struct msm_dp_pll *pll,
>>> + struct clk **link_clk_provider,
>>> + struct clk **pixel_clk_provider)
>>> +{
>>> + struct dp_pll_10nm *pll_10nm = to_dp_pll_10nm(pll);
>>> + struct clk_hw_onecell_data *hw_data = pll_10nm->hw_data;
>>> +
>>> + if (link_clk_provider)
>>> + *link_clk_provider =
>>> hw_data->hws[DP_LINK_CLK_SRC]->clk;
>>> + if (pixel_clk_provider)
>>> + *pixel_clk_provider =
>>> hw_data->hws[DP_PIXEL_CLK_SRC]->clk;
>>
>> I don't know what this is but it looks weird. Is this a clk provider
>> on
>> top of another clk provider?
> Here we are setting the parent of RCG.
> RCG needs called by the consumer to set the parent.
> DP is a consumer setting a rate on provider.
>
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct clk_ops dp_10nm_pclksel_clk_ops = {
>>> + .get_parent = dp_mux_get_parent_10nm,
>>> + .set_parent = dp_mux_set_parent_10nm,
>>> + .recalc_rate = mux_recalc_rate,
>>> + .determine_rate = clk_mux_determine_rate,
>>> +};
>>> +
>>> +static struct clk_hw *dp_pll_10nm_pixel_clk_sel(struct dp_pll_10nm
>>> *pll_10nm)
>>> +{
>>> + struct device *dev = &pll_10nm->pdev->dev;
>>> + struct dp_pll_10nm_pclksel *pll_pclksel;
>>> + struct clk_init_data pclksel_init = {
>>> + .parent_names = (const char *[]){
>>> + "dp_vco_divsel_two_clk_src",
>>> + "dp_vco_divsel_four_clk_src",
>>> + "dp_vco_divsel_six_clk_src" },
>>
>> It looks like a clk divider still, not a mux.
>>
>>> + .num_parents = 3,
>>> + .name = "dp_phy_pll_vco_div_clk",
>>> + .flags = CLK_IGNORE_UNUSED,
>>
>> Why? Please comment why this is needed in the code. But more
>> importantly, please just don't use this flag.
> We are working to remove this flag CLK_IGNORE_UNUSED.
>>
>>> + .ops = &dp_10nm_pclksel_clk_ops,
>>> + };
>>> + int ret;
>>> +
>>> + pll_pclksel = devm_kzalloc(dev, sizeof(*pll_pclksel),
>>> GFP_KERNEL);
>>> + if (!pll_pclksel)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + pll_pclksel->pll = pll_10nm;
>>> + pll_pclksel->shift = 0;
>>> + pll_pclksel->width = 4;
>>> + pll_pclksel->flags = CLK_DIVIDER_ONE_BASED;
>>> + pll_pclksel->hw.init = &pclksel_init;
>>> +
>>> + ret = clk_hw_register(dev, &pll_pclksel->hw);
>>> + if (ret)
>>> + return ERR_PTR(ret);
>>> +
>>> + return &pll_pclksel->hw;
>>> +}
>>> +
>>> +static int dp_pll_10nm_register(struct dp_pll_10nm *pll_10nm)
>>> +{
>>> + char clk_name[32], parent[32], vco_name[32];
>>> + struct clk_init_data vco_init = {
>>> + .parent_names = (const char *[]){ "bi_tcxo" },
>>
>> Please use the new way of specifying clk parents through .fw_name and
>> DT
>> indices.
>>
>>> + .num_parents = 1,
>>> + .name = vco_name,
>>> + .flags = CLK_IGNORE_UNUSED,
>>
>> Ugh there's that flag again.
>>
>>> + .ops = &dp_10nm_vco_clk_ops,
>>> + };
>>> + struct device *dev = &pll_10nm->pdev->dev;
>>> + struct clk_hw **hws = pll_10nm->hws;
>>> + struct clk_hw_onecell_data *hw_data;
>>> + struct clk_hw *hw;
>>> + int num = 0;
>>> + int ret;
>>> +
>>> + DRM_DEBUG_DP("DP->id = %d", pll_10nm->id);
>>> +
>>> + hw_data = devm_kzalloc(dev, sizeof(*hw_data) +
>>
>> Use devm_kcalloc()
>>
>>> + NUM_PROVIDED_CLKS * sizeof(struct
>>> clk_hw *),
>>> + GFP_KERNEL);
>>> + if (!hw_data)
>>> + return -ENOMEM;
>>> +
>>> + snprintf(vco_name, 32, "dp_vco_clk");
>>> + pll_10nm->base.clk_hw.init = &vco_init;
>>> + ret = clk_hw_register(dev, &pll_10nm->base.clk_hw);
>>> + if (ret)
>>> + return ret;
>>> + hws[num++] = &pll_10nm->base.clk_hw;
>>> +
>>> + snprintf(clk_name, 32, "dp_phy_pll_link_clk");
>>> + snprintf(parent, 32, "dp_vco_clk");
>>
>> Is there a need to make these names with snprintf?
>>
>>> + hw = clk_hw_register_fixed_factor(dev, clk_name, parent,
>>> + CLK_SET_RATE_PARENT, 1,
>>> 10);
>>> + if (IS_ERR(hw))
>>> + return PTR_ERR(hw);
>>> + hws[num++] = hw;
>>> + hw_data->hws[DP_LINK_CLK_SRC] = hw;
>>> +
>>> + snprintf(clk_name, 32, "dp_vco_divsel_two_clk_src");
>>> + snprintf(parent, 32, "dp_vco_clk");
>>> + hw = clk_hw_register_fixed_factor(dev, clk_name, parent,
>>> + 0, 1, 2);
>>> + if (IS_ERR(hw))
>>> + return PTR_ERR(hw);
>>> + hws[num++] = hw;
>>> +
>>> + snprintf(clk_name, 32, "dp_vco_divsel_four_clk_src");
>>> + snprintf(parent, 32, "dp_vco_clk");
>>> + hw = clk_hw_register_fixed_factor(dev, clk_name, parent,
>>> + 0, 1, 4);
>>> + if (IS_ERR(hw))
>>> + return PTR_ERR(hw);
>>> + hws[num++] = hw;
>>> +
>>> + snprintf(clk_name, 32, "dp_vco_divsel_six_clk_src");
>>> + snprintf(parent, 32, "dp_vco_clk");
>>> + hw = clk_hw_register_fixed_factor(dev, clk_name, parent,
>>> + 0, 1, 6);
>>> + if (IS_ERR(hw))
>>> + return PTR_ERR(hw);
>>> + hws[num++] = hw;
>>> +
>>> + hw = dp_pll_10nm_pixel_clk_sel(pll_10nm);
>>> + if (IS_ERR(hw))
>>> + return PTR_ERR(hw);
>>> +
>>> + hws[num++] = hw;
>>> + hw_data->hws[DP_PIXEL_CLK_SRC] = hw;
>>> +
>>> + pll_10nm->num_hws = num;
>>> +
>>> + hw_data->num = NUM_PROVIDED_CLKS;
>>> + pll_10nm->hw_data = hw_data;
>>> +
>>> + ret = of_clk_add_hw_provider(dev->of_node,
>>> of_clk_hw_onecell_get,
>>> + pll_10nm->hw_data);
>>> + if (ret) {
>>> + DRM_DEV_ERROR(dev, "failed to register clk provider:
>>> %d\n",
>>> + ret);
>>> + return ret;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +struct msm_dp_pll *msm_dp_pll_10nm_init(struct platform_device
>>> *pdev, int id)
>>> +{
>>> + struct dp_pll_10nm *dp_10nm_pll;
>>> + struct msm_dp_pll *pll;
>>> + int ret;
>>> +
>>> + if (!pdev)
>>> + return ERR_PTR(-ENODEV);
>>> +
>>> + dp_10nm_pll = devm_kzalloc(&pdev->dev,
>>> + sizeof(*dp_10nm_pll),
>>> GFP_KERNEL);
>>> + if (!dp_10nm_pll)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + DRM_DEBUG_DP("DP PLL%d", id);
>>> +
>>> + dp_10nm_pll->pdev = pdev;
>>> + dp_10nm_pll->id = id;
>>> + dp_pdb = dp_10nm_pll;
>>> +
>>> + dp_10nm_pll->pll_base = msm_ioremap(pdev, "pll_base",
>>> "DP_PLL");
>>> + if (IS_ERR_OR_NULL(dp_10nm_pll->pll_base)) {
>>> + DRM_DEV_ERROR(&pdev->dev, "failed to map CMN PLL
>>> base\n");
>>> + return ERR_PTR(-ENOMEM);
>>> + }
>>> +
>>> + dp_10nm_pll->phy_base = msm_ioremap(pdev, "phy_base",
>>> "DP_PHY");
>>> + if (IS_ERR_OR_NULL(dp_10nm_pll->phy_base)) {
>>> + DRM_DEV_ERROR(&pdev->dev, "failed to map CMN PHY
>>> base\n");
>>> + return ERR_PTR(-ENOMEM);
>>> + }
>>> +
>>> + dp_10nm_pll->ln_tx0_base = msm_ioremap(pdev, "ln_tx0_base",
>>> + "DP_LN_TX0");
>>> + if (IS_ERR_OR_NULL(dp_10nm_pll->ln_tx0_base)) {
>>> + DRM_DEV_ERROR(&pdev->dev, "failed to map CMN LN_TX0
>>> base\n");
>>> + return ERR_PTR(-ENOMEM);
>>> + }
>>> +
>>> + dp_10nm_pll->ln_tx1_base = msm_ioremap(pdev, "ln_tx1_base",
>>> + "DP_LN_TX1");
>>> + if (IS_ERR_OR_NULL(dp_10nm_pll->ln_tx1_base)) {
>>> + DRM_DEV_ERROR(&pdev->dev, "failed to map CMN LN_TX1
>>> base\n");
>>> + return ERR_PTR(-ENOMEM);
>>> + }
>>> +
>>> + ret = of_property_read_u32(pdev->dev.of_node, "cell-index",
>>> + &dp_10nm_pll->index);
>>> + if (ret) {
>>> + DRM_ERROR("Unable to get the cell-index ret=%d\n",
>>> ret);
>>> + dp_10nm_pll->index = 0;
>>> + }
>>> +
>>> + ret = msm_dp_pll_util_parse_dt_clock(pdev,
>>> &dp_10nm_pll->base);
>>> + if (ret) {
>>> + DRM_ERROR("Unable to parse dt clocks ret=%d\n", ret);
>>> + return ERR_PTR(ret);
>>> + }
>>> +
>>> + ret = dp_pll_10nm_register(dp_10nm_pll);
>>> + if (ret) {
>>> + DRM_DEV_ERROR(&pdev->dev, "failed to register PLL:
>>> %d\n", ret);
>>> + return ERR_PTR(ret);
>>> + }
>>> +
>>> + pll = &dp_10nm_pll->base;
>>> + pll->min_rate = DP_VCO_HSCLK_RATE_1620MHZDIV1000;
>>> + pll->max_rate = DP_VCO_HSCLK_RATE_8100MHZDIV1000;
>>
>> Maybe min/max can be set with clk_hw_set_rate_range()?
> Here pll->min_rate and pll->max_rate stores the VCO range in local
> structure.
>
>>
>>> + pll->get_provider = dp_pll_10nm_get_provider;
>>> +
>>> + return pll;
>>> +}
>>> diff --git a/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h
>>> b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h
>>> new file mode 100644
>>> index 0000000..8466463
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h
>>> @@ -0,0 +1,86 @@
>>> +/* 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_10nm {
>>> + struct msm_dp_pll base;
>>> +
>>> + int id;
>>> + struct platform_device *pdev;
>>> +
>>> + void __iomem *pll_base;
>>> + void __iomem *phy_base;
>>> + void __iomem *ln_tx0_base;
>>> + void __iomem *ln_tx1_base;
>>> +
>>> + /* private clocks: */
>>> + struct clk_hw *hws[NUM_DP_CLOCKS_MAX];
>>> + u32 num_hws;
>>> +
>>> + /* clock-provider: */
>>> + struct clk_hw_onecell_data *hw_data;
>>> +
>>> + /* 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;
>>> +};
>>> +
>>> +#define to_dp_pll_10nm(x) container_of(x, struct dp_pll_10nm,
>>> base)
>>
>> Why is this struct in the header file? Does something outside of the
>> PLL
>> driver need to access this struct?
>>
>>> +
>>> +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);
>>> +#endif /* __DP_PLL_10NM_H */
>>> diff --git a/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c
>>> b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c
>>> new file mode 100644
>>> index 0000000..db3983d5
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c
>>> @@ -0,0 +1,524 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2016-2020, The Linux Foundation. All rights
>>> reserved.
>>> + */
>>> +
>>> +#define pr_fmt(fmt) "%s: " fmt, __func__
>>> +
>>> +#include <linux/delay.h>
>>> +#include <linux/err.h>
>>> +#include <linux/iopoll.h>
>>> +#include <linux/kernel.h>
>>> +
>>> +#include "dp_hpd.h"
>>> +#include "dp_pll.h"
>>> +#include "dp_pll_10nm.h"
>>> +
>>> +static int dp_vco_pll_init_db_10nm(struct msm_dp_pll *pll,
>>> + unsigned long rate)
>>> +{
>>> + struct dp_pll_10nm *dp_res = to_dp_pll_10nm(pll);
>>> + u32 spare_value = 0;
>>> +
>>> + spare_value = PLL_REG_R(dp_res->phy_base, REG_DP_PHY_SPARE0);
>>> + dp_res->lane_cnt = spare_value & 0x0F;
>>> + dp_res->orientation = (spare_value & 0xF0) >> 4;
>>
>> Can this be read at any time from the spare register? Just curious why
>> we need to read this here in init vs. when it's actually used. We
>> could
>> avoid stashing away these values in the struct. It also looks like the
>> PLL part is intimately tied to the phy, but there isn't an actual phy
>> driver. Why? It looks like the phy driver is a clk driver, and the clk
>> framework is not prepared to handle things like lane count and
>> orientation. I'd expect that to come through a PHY framework and then
>> have the phy driver control the PLL by powering it back up and
>> changing
>> it's rates, etc. Not everything needs to be done through clk APIs.
>
> Presently we need this spare register information in many functions
> to program dp hardware. There are atleast 6 instances we use them, so
> we are trying
> to store in the structure once.
> Unfortunately msm DP pll and phy is one hardware tightly bound.
>
>>
>>> +
>>> + DRM_DEBUG_DP("%s: spare_value=0x%x, ln_cnt=0x%x,
>>> orientation=0x%x\n",
>>> + __func__, spare_value, dp_res->lane_cnt,
>>> + dp_res->orientation);
>>> +
>>> + switch (rate) {
>>> + case DP_VCO_HSCLK_RATE_1620MHZDIV1000:
>>> + DRM_DEBUG_DP("%s: VCO rate: %ld\n", __func__,
>>> + DP_VCO_RATE_9720MHZDIV1000);
>>> + dp_res->hsclk_sel = 0x0c;
>>> + dp_res->dec_start_mode0 = 0x69;
>>> + dp_res->div_frac_start1_mode0 = 0x00;
>>> + dp_res->div_frac_start2_mode0 = 0x80;
>>> + dp_res->div_frac_start3_mode0 = 0x07;
>>> + dp_res->integloop_gain0_mode0 = 0x3f;
>>> + dp_res->integloop_gain1_mode0 = 0x00;
>>> + dp_res->vco_tune_map = 0x00;
>>> + dp_res->lock_cmp1_mode0 = 0x6f;
>>> + dp_res->lock_cmp2_mode0 = 0x08;
>>> + dp_res->lock_cmp3_mode0 = 0x00;
>>> + dp_res->phy_vco_div = 0x1;
>>> + dp_res->lock_cmp_en = 0x00;
>>> + break;
>>> + case DP_VCO_HSCLK_RATE_2700MHZDIV1000:
>>> + DRM_DEBUG_DP("%s: VCO rate: %ld\n", __func__,
>>> + DP_VCO_RATE_10800MHZDIV1000);
>>> + dp_res->hsclk_sel = 0x04;
>>> + dp_res->dec_start_mode0 = 0x69;
>>> + dp_res->div_frac_start1_mode0 = 0x00;
>>> + dp_res->div_frac_start2_mode0 = 0x80;
>>> + dp_res->div_frac_start3_mode0 = 0x07;
>>> + dp_res->integloop_gain0_mode0 = 0x3f;
>>> + dp_res->integloop_gain1_mode0 = 0x00;
>>> + dp_res->vco_tune_map = 0x00;
>>> + dp_res->lock_cmp1_mode0 = 0x0f;
>>> + dp_res->lock_cmp2_mode0 = 0x0e;
>>> + dp_res->lock_cmp3_mode0 = 0x00;
>>> + dp_res->phy_vco_div = 0x1;
>>> + dp_res->lock_cmp_en = 0x00;
>>> + break;
>>> + case DP_VCO_HSCLK_RATE_5400MHZDIV1000:
>>> + DRM_DEBUG_DP("%s: VCO rate: %ld\n", __func__,
>>> + DP_VCO_RATE_10800MHZDIV1000);
>>> + dp_res->hsclk_sel = 0x00;
>>> + dp_res->dec_start_mode0 = 0x8c;
>>> + dp_res->div_frac_start1_mode0 = 0x00;
>>> + dp_res->div_frac_start2_mode0 = 0x00;
>>> + dp_res->div_frac_start3_mode0 = 0x0a;
>>> + dp_res->integloop_gain0_mode0 = 0x3f;
>>> + dp_res->integloop_gain1_mode0 = 0x00;
>>> + dp_res->vco_tune_map = 0x00;
>>> + dp_res->lock_cmp1_mode0 = 0x1f;
>>> + dp_res->lock_cmp2_mode0 = 0x1c;
>>> + dp_res->lock_cmp3_mode0 = 0x00;
>>> + dp_res->phy_vco_div = 0x2;
>>> + dp_res->lock_cmp_en = 0x00;
>>> + break;
>>> + case DP_VCO_HSCLK_RATE_8100MHZDIV1000:
>>> + DRM_DEBUG_DP("%s: VCO rate: %ld\n", __func__,
>>> + DP_VCO_RATE_8100MHZDIV1000);
>>> + dp_res->hsclk_sel = 0x03;
>>> + dp_res->dec_start_mode0 = 0x69;
>>> + dp_res->div_frac_start1_mode0 = 0x00;
>>> + dp_res->div_frac_start2_mode0 = 0x80;
>>> + dp_res->div_frac_start3_mode0 = 0x07;
>>> + dp_res->integloop_gain0_mode0 = 0x3f;
>>> + dp_res->integloop_gain1_mode0 = 0x00;
>>> + dp_res->vco_tune_map = 0x00;
>>> + dp_res->lock_cmp1_mode0 = 0x2f;
>>> + dp_res->lock_cmp2_mode0 = 0x2a;
>>> + dp_res->lock_cmp3_mode0 = 0x00;
>>> + dp_res->phy_vco_div = 0x0;
>>> + dp_res->lock_cmp_en = 0x08;
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static int dp_config_vco_rate_10nm(struct msm_dp_pll *pll,
>>> + unsigned long rate)
>>> +{
>>> + u32 res = 0;
>>> + struct dp_pll_10nm *dp_res = to_dp_pll_10nm(pll);
>>> +
>>> + res = dp_vco_pll_init_db_10nm(pll, rate);
>>> + if (res) {
>>> + DRM_ERROR("VCO Init DB failed\n");
>>> + return res;
>>> + }
>>> +
>>> + if (dp_res->lane_cnt != 4) {
>>> + if (dp_res->orientation == ORIENTATION_CC2)
>>> + PLL_REG_W(dp_res->phy_base,
>>> REG_DP_PHY_PD_CTL, 0x6d);
>>> + else
>>> + PLL_REG_W(dp_res->phy_base,
>>> REG_DP_PHY_PD_CTL, 0x75);
>>> + } else {
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_PD_CTL, 0x7d);
>>> + }
>>> +
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_SVS_MODE_CLK_SEL,
>>> 0x01);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_SYSCLK_EN_SEL, 0x37);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_SYS_CLK_CTRL, 0x02);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_CLK_ENABLE1, 0x0e);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_SYSCLK_BUF_ENABLE,
>>> 0x06);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_CLK_SEL, 0x30);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_CMN_CONFIG, 0x02);
>>> +
>>> + /* Different for each clock rates */
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_HSCLK_SEL, dp_res->hsclk_sel);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_DEC_START_MODE0,
>>> dp_res->dec_start_mode0);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_DIV_FRAC_START1_MODE0,
>>> + dp_res->div_frac_start1_mode0);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_DIV_FRAC_START2_MODE0,
>>> + dp_res->div_frac_start2_mode0);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_DIV_FRAC_START3_MODE0,
>>> + dp_res->div_frac_start3_mode0);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_INTEGLOOP_GAIN0_MODE0,
>>> + dp_res->integloop_gain0_mode0);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_INTEGLOOP_GAIN1_MODE0,
>>> + dp_res->integloop_gain1_mode0);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_VCO_TUNE_MAP, dp_res->vco_tune_map);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_LOCK_CMP1_MODE0,
>>> dp_res->lock_cmp1_mode0);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_LOCK_CMP2_MODE0,
>>> dp_res->lock_cmp2_mode0);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_LOCK_CMP3_MODE0,
>>> dp_res->lock_cmp3_mode0);
>>> +
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_BG_TIMER, 0x0a);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_CORECLK_DIV_MODE0,
>>> 0x0a);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_VCO_TUNE_CTRL, 0x00);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_BIAS_EN_CLKBUFLR_EN,
>>> 0x3f);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_CORE_CLK_EN, 0x1f);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_PLL_IVCO, 0x07);
>>> + PLL_REG_W(dp_res->pll_base,
>>> + QSERDES_COM_LOCK_CMP_EN, dp_res->lock_cmp_en);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_PLL_CCTRL_MODE0,
>>> 0x36);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_PLL_RCTRL_MODE0,
>>> 0x16);
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_CP_CTRL_MODE0, 0x06);
>>> +
>>> + if (dp_res->orientation == ORIENTATION_CC2)
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_MODE, 0x4c);
>>> + else
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_MODE, 0x5c);
>>> +
>>> + /* TX Lane configuration */
>>> + PLL_REG_W(dp_res->phy_base,
>>> + REG_DP_PHY_TX0_TX1_LANE_CTL, 0x05);
>>> + PLL_REG_W(dp_res->phy_base,
>>> + REG_DP_PHY_TX2_TX3_LANE_CTL, 0x05);
>>> +
>>> + /* TX-0 register configuration */
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN, 0x1a);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_VMODE_CTRL1, 0x40);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_PRE_STALL_LDO_BOOST_EN, 0x30);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_INTERFACE_SELECT, 0x3d);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_CLKBUF_ENABLE, 0x0f);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_RESET_TSYNC_EN, 0x03);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_TRAN_DRVR_EMP_EN, 0x03);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_PARRATE_REC_DETECT_IDLE_EN,
>>> 0x00);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_TX_INTERFACE_MODE, 0x00);
>>> + PLL_REG_W(dp_res->ln_tx0_base, REG_DP_PHY_TXn_TX_BAND, 0x4);
>>> +
>>> + /* TX-1 register configuration */
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN, 0x1a);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_VMODE_CTRL1, 0x40);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_PRE_STALL_LDO_BOOST_EN, 0x30);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_INTERFACE_SELECT, 0x3d);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_CLKBUF_ENABLE, 0x0f);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_RESET_TSYNC_EN, 0x03);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_TRAN_DRVR_EMP_EN, 0x03);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_PARRATE_REC_DETECT_IDLE_EN,
>>> 0x00);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_TX_INTERFACE_MODE, 0x00);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_TX_BAND, 0x4);
>>> +
>>> + /* dependent on the vco frequency */
>>> + PLL_REG_W(dp_res->phy_base,
>>> + REG_DP_PHY_VCO_DIV, dp_res->phy_vco_div);
>>> +
>>> + return res;
>>> +}
>>> +
>>> +static bool dp_10nm_pll_lock_status(struct dp_pll_10nm *dp_res)
>>> +{
>>> + u32 status;
>>> + bool pll_locked;
>>> +
>>> + /* poll for PLL lock status */
>>> + if (readl_poll_timeout_atomic((dp_res->pll_base +
>>> + QSERDES_COM_C_READY_STATUS),
>>> + status,
>>> + ((status & BIT(0)) > 0),
>>> + DP_PHY_PLL_POLL_SLEEP_US,
>>> + DP_PHY_PLL_POLL_TIMEOUT_US)) {
>>> + DRM_ERROR("%s: C_READY status is not high.
>>> Status=%x\n",
>>> + __func__, status);
>>> + pll_locked = false;
>>> + } else {
>>> + pll_locked = true;
>>> + }
>>> +
>>> + return pll_locked;
>>> +}
>>> +
>>> +static bool dp_10nm_phy_rdy_status(struct dp_pll_10nm *dp_res)
>>> +{
>>> + u32 status;
>>> + bool phy_ready = true;
>>> +
>>> + /* poll for PHY ready status */
>>> + if (readl_poll_timeout_atomic((dp_res->phy_base +
>>> + REG_DP_PHY_STATUS),
>>> + status,
>>> + ((status & (BIT(1))) > 0),
>>> + DP_PHY_PLL_POLL_SLEEP_US,
>>> + DP_PHY_PLL_POLL_TIMEOUT_US)) {
>>> + DRM_ERROR("%s: Phy_ready is not high. Status=%x\n",
>>> + __func__, status);
>>> + phy_ready = false;
>>> + }
>>> +
>>> + return phy_ready;
>>> +}
>>> +
>>> +static int dp_pll_enable_10nm(struct clk_hw *hw)
>>> +{
>>> + int rc = 0;
>>> + struct msm_dp_pll *pll = hw_clk_to_pll(hw);
>>> + struct dp_pll_10nm *dp_res = to_dp_pll_10nm(pll);
>>> + u32 bias_en, drvr_en;
>>> +
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_AUX_CFG2, 0x04);
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_CFG, 0x01);
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_CFG, 0x05);
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_CFG, 0x01);
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_CFG, 0x09);
>>> +
>>> + PLL_REG_W(dp_res->pll_base, QSERDES_COM_RESETSM_CNTRL, 0x20);
>>> +
>>> + if (!dp_10nm_pll_lock_status(dp_res)) {
>>> + rc = -EINVAL;
>>> + goto lock_err;
>>> + }
>>> +
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_CFG, 0x19);
>>> + /* poll for PHY ready status */
>>> + if (!dp_10nm_phy_rdy_status(dp_res)) {
>>> + rc = -EINVAL;
>>> + goto lock_err;
>>> + }
>>> +
>>> + DRM_DEBUG_DP("%s: PLL is locked\n", __func__);
>>> +
>>> + if (dp_res->lane_cnt == 1) {
>>> + bias_en = 0x3e;
>>> + drvr_en = 0x13;
>>> + } else {
>>> + bias_en = 0x3f;
>>> + drvr_en = 0x10;
>>> + }
>>> +
>>> + if (dp_res->lane_cnt != 4) {
>>> + if (dp_res->orientation == ORIENTATION_CC1) {
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_HIGHZ_DRVR_EN,
>>> drvr_en);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN,
>>> bias_en);
>>> + } else {
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_HIGHZ_DRVR_EN,
>>> drvr_en);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN,
>>> bias_en);
>>> + }
>>> + } else {
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_HIGHZ_DRVR_EN,
>>> drvr_en);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN,
>>> bias_en);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_HIGHZ_DRVR_EN,
>>> drvr_en);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN,
>>> bias_en);
>>> + }
>>> +
>>> + PLL_REG_W(dp_res->ln_tx0_base, REG_DP_PHY_TXn_TX_POL_INV,
>>> 0x0a);
>>> + PLL_REG_W(dp_res->ln_tx1_base, REG_DP_PHY_TXn_TX_POL_INV,
>>> 0x0a);
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_CFG, 0x18);
>>> + udelay(2000);
>>> +
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_CFG, 0x19);
>>> +
>>> + /* poll for PHY ready status */
>>> + if (!dp_10nm_phy_rdy_status(dp_res)) {
>>> + rc = -EINVAL;
>>> + goto lock_err;
>>> + }
>>> +
>>> + PLL_REG_W(dp_res->ln_tx0_base, REG_DP_PHY_TXn_TX_DRV_LVL,
>>> 0x38);
>>> + PLL_REG_W(dp_res->ln_tx1_base, REG_DP_PHY_TXn_TX_DRV_LVL,
>>> 0x38);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> REG_DP_PHY_TXn_TX_EMP_POST1_LVL, 0x20);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> REG_DP_PHY_TXn_TX_EMP_POST1_LVL, 0x20);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_RES_CODE_LANE_OFFSET_TX,
>>> 0x06);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_RES_CODE_LANE_OFFSET_TX,
>>> 0x06);
>>> + PLL_REG_W(dp_res->ln_tx0_base,
>>> + REG_DP_PHY_TXn_RES_CODE_LANE_OFFSET_RX,
>>> 0x07);
>>> + PLL_REG_W(dp_res->ln_tx1_base,
>>> + REG_DP_PHY_TXn_RES_CODE_LANE_OFFSET_RX,
>>> 0x07);
>>> +
>>> +lock_err:
>>> + return rc;
>>> +}
>>> +
>>> +static int dp_pll_disable_10nm(struct clk_hw *hw)
>>> +{
>>> + int rc = 0;
>>> + struct msm_dp_pll *pll = hw_clk_to_pll(hw);
>>> + struct dp_pll_10nm *dp_res = to_dp_pll_10nm(pll);
>>> +
>>> + /* Assert DP PHY power down */
>>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_PD_CTL, 0x2);
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +
>>> +int dp_vco_prepare_10nm(struct clk_hw *hw)
>>> +{
>>> + int rc = 0;
>>> + struct msm_dp_pll *pll = hw_clk_to_pll(hw);
>>> + struct dp_pll_10nm *dp_res = to_dp_pll_10nm(pll);
>>> +
>>> + DRM_DEBUG_DP("%s: rate = %ld\n", __func__, pll->rate);
>>> + if ((dp_res->vco_cached_rate != 0)
>>> + && (dp_res->vco_cached_rate == pll->rate)) {
>>> + rc = dp_vco_set_rate_10nm(hw,
>>> + dp_res->vco_cached_rate,
>>> dp_res->vco_cached_rate);
>>> + if (rc) {
>>> + DRM_ERROR("index=%d vco_set_rate failed.
>>> rc=%d\n",
>>> + rc, dp_res->index);
>>> + goto error;
>>> + }
>>> + }
>>> +
>>> + rc = dp_pll_enable_10nm(hw);
>>> + if (rc) {
>>> + DRM_ERROR("ndx=%d failed to enable dp pll\n",
>>> + dp_res->index);
>>> + goto error;
>>> + }
>>> +
>>> + pll->pll_on = true;
>>> +error:
>>> + return rc;
>>> +}
>>> +
>>> +void dp_vco_unprepare_10nm(struct clk_hw *hw)
>>> +{
>>> + struct msm_dp_pll *pll = hw_clk_to_pll(hw);
>>> + struct dp_pll_10nm *dp_res = to_dp_pll_10nm(pll);
>>> +
>>> + if (!dp_res) {
>>> + DRM_ERROR("Invalid input parameter\n");
>>> + return;
>>> + }
>>> +
>>> + if (!pll->pll_on) {
>>> + DRM_ERROR("pll resource can't be enabled\n");
>>> + return;
>>> + }
>>> + dp_res->vco_cached_rate = pll->rate;
>>> + dp_pll_disable_10nm(hw);
>>> +
>>> + pll->pll_on = false;
>>> +}
>>> +
>>> +int dp_vco_set_rate_10nm(struct clk_hw *hw, unsigned long rate,
>>> + unsigned long parent_rate)
>>> +{
>>> + struct msm_dp_pll *pll = hw_clk_to_pll(hw);
>>> + int rc;
>>> +
>>> + DRM_DEBUG_DP("DP lane CLK rate=%ld\n", rate);
>>> +
>>> + rc = dp_config_vco_rate_10nm(pll, rate);
>>> + if (rc)
>>> + DRM_ERROR("%s: Failed to set clk rate\n", __func__);
>>> +
>>> + pll->rate = rate;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +unsigned long dp_vco_recalc_rate_10nm(struct clk_hw *hw,
>>> + unsigned long parent_rate)
>>> +{
>>> + struct msm_dp_pll *pll = hw_clk_to_pll(hw);
>>> + struct dp_pll_10nm *dp_res = to_dp_pll_10nm(pll);
>>> + u32 div, hsclk_div, link_clk_div = 0;
>>> + u64 vco_rate;
>>> +
>>> + div = PLL_REG_R(dp_res->pll_base, QSERDES_COM_HSCLK_SEL);
>>> + div &= 0x0f;
>>> +
>>> + if (div == 12)
>>> + hsclk_div = 6; /* Default */
>>> + else if (div == 4)
>>> + hsclk_div = 4;
>>> + else if (div == 0)
>>> + hsclk_div = 2;
>>> + else if (div == 3)
>>> + hsclk_div = 1;
>>> + else {
>>> + DRM_DEBUG_DP("unknown divider. forcing to
>>> default\n");
>>> + hsclk_div = 5;
>>> + }
>>> +
>>> + div = PLL_REG_R(dp_res->phy_base, REG_DP_PHY_AUX_CFG2);
>>> + div >>= 2;
>>> +
>>> + if ((div & 0x3) == 0)
>>> + link_clk_div = 5;
>>> + else if ((div & 0x3) == 1)
>>> + link_clk_div = 10;
>>> + else if ((div & 0x3) == 2)
>>> + link_clk_div = 20;
>>> + else
>>> + DRM_ERROR("%s: unsupported div. Phy_mode: %d\n",
>>> __func__, div);
>>> +
>>> + if (link_clk_div == 20) {
>>> + vco_rate = DP_VCO_HSCLK_RATE_2700MHZDIV1000;
>>> + } else {
>>> + if (hsclk_div == 6)
>>> + vco_rate = DP_VCO_HSCLK_RATE_1620MHZDIV1000;
>>> + else if (hsclk_div == 4)
>>> + vco_rate = DP_VCO_HSCLK_RATE_2700MHZDIV1000;
>>> + else if (hsclk_div == 2)
>>> + vco_rate = DP_VCO_HSCLK_RATE_5400MHZDIV1000;
>>> + else
>>> + vco_rate = DP_VCO_HSCLK_RATE_8100MHZDIV1000;
>>> + }
>>> +
>>> + DRM_DEBUG_DP("returning vco rate = %lu\n", (unsigned
>>> long)vco_rate);
>>> +
>>> + dp_res->vco_cached_rate = pll->rate = vco_rate;
>>> + return (unsigned long)vco_rate;
>>> +}
>>> +
>>> +long dp_vco_round_rate_10nm(struct clk_hw *hw, unsigned long rate,
>>> + unsigned long *parent_rate)
>>> +{
>>> + unsigned long rrate = rate;
>>> + struct msm_dp_pll *pll = hw_clk_to_pll(hw);
>>> +
>>> + if (rate <= pll->min_rate)
>>> + rrate = pll->min_rate;
>>> + else if (rate <= DP_VCO_HSCLK_RATE_2700MHZDIV1000)
>>> + rrate = DP_VCO_HSCLK_RATE_2700MHZDIV1000;
>>> + else if (rate <= DP_VCO_HSCLK_RATE_5400MHZDIV1000)
>>> + rrate = DP_VCO_HSCLK_RATE_5400MHZDIV1000;
>>> + else
>>> + rrate = pll->max_rate;
>>
>> A switch statement would be more readable. Not sure why pll->min_rate
>> is
>> needed though.
>
> pll->min_rate is the minimum vco clock that is supported on the
> chipset.
> We are trying to make sure that we dont go below the min vco
> level for that chipset.
>
>>
>> switch (rate) {
>> case 0 .. pll->min_rate:
>> rrate =
>> break;
>> case pll->min_rate .. DP_VCO_HSCLK_RATE_2700MHZDIV1000:
>> ...
>> }
>>
>>> +
>>> + DRM_DEBUG_DP("%s: rrate=%ld\n", __func__, rrate);
>>> +
>>> + *parent_rate = rrate;
>>> + return rrate;
>>> +}
More information about the dri-devel
mailing list