| Message ID | 1539191759-14836-4-git-send-email-chandanu@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | Add support for DisplayPort driver on SnapDragon 845 | expand |
On Wed, Oct 10, 2018 at 10:15:59AM -0700, Chandan Uddaraju wrote: > 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. > > Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org> > --- > drivers/gpu/drm/msm/Kconfig | 16 + > drivers/gpu/drm/msm/Makefile | 6 + > drivers/gpu/drm/msm/dp/dp_ctrl.c | 1 + > drivers/gpu/drm/msm/dp/dp_display.c | 50 +++ > drivers/gpu/drm/msm/dp/dp_display.h | 3 + > drivers/gpu/drm/msm/dp/dp_parser.h | 3 + > drivers/gpu/drm/msm/dp/dp_power.c | 77 +++- > drivers/gpu/drm/msm/dp/dp_power.h | 2 + > drivers/gpu/drm/msm/dp/pll/dp_pll.c | 153 ++++++++ > drivers/gpu/drm/msm/dp/pll/dp_pll.h | 64 ++++ > drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c | 401 +++++++++++++++++++ > drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h | 94 +++++ > drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c | 531 ++++++++++++++++++++++++++ > 13 files changed, 1389 insertions(+), 12 deletions(-) > 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 c363f24..1e0b9158 100644 > --- a/drivers/gpu/drm/msm/Kconfig > +++ b/drivers/gpu/drm/msm/Kconfig > @@ -58,6 +58,22 @@ config DRM_MSM_DP > driver. DP external 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 > + default y The default should be n > + 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 Should this be DRM_MSM_DP_PLL instead? > + default y The default should be n > + 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 765a8d8..8d18353 100644 > --- a/drivers/gpu/drm/msm/Makefile > +++ b/drivers/gpu/drm/msm/Makefile > @@ -137,4 +137,10 @@ 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 > > +ifeq ($(CONFIG_DRM_MSM_DP_PLL),y) > +msm-y += dp/pll/dp_pll.o > +msm-y += dp/pll/dp_pll_10nm.o > +msm-y += dp/pll/dp_pll_10nm_util.o Instead of the ifeq, these should follow the form: msm-$(CONFIG_BLAH) += > +endif > + > obj-$(CONFIG_DRM_MSM) += msm.o > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index 08a52f5..e23beee 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c > @@ -1051,6 +1051,7 @@ static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl) > { > int ret = 0; > > + ctrl->power->set_link_clk_parent(ctrl->power); > ctrl->power->set_pixel_clk_parent(ctrl->power); > > dp_ctrl_set_clock_rate(ctrl, "ctrl_link_clk", > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index 8c98399..2bf6635 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -72,6 +72,48 @@ 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) { > + pr_err("Invalid Arguments\n"); > + return -EINVAL; > + } > + > + pdev = dp_priv->pdev; > + dp_parser = dp_priv->parser; > + > + if (!dp_parser) { > + pr_err("Parser not initialized.\n"); > + return -EINVAL; > + } Can we please remove the unnecessary null checks from this patch too? > + > + pll_node = of_parse_phandle(pdev->dev.of_node, "pll-node", 0); > + if (!pll_node) { > + dev_err(&pdev->dev, "cannot find pll device\n"); > + return -ENXIO; > + } > + > + pll_pdev = of_find_device_by_node(pll_node); > + if (pll_pdev) > + dp_parser->pll = platform_get_drvdata(pll_pdev); What if the pll driver goes away before the dp driver? > + > + of_node_put(pll_node); > + > + if (!pll_pdev || !dp_parser->pll) { > + dev_err(&pdev->dev, "%s: pll driver is not ready\n", __func__); This patch (and even just this function) has both pr_err and dev_err, oy! Please convert everything to one logging medium. The msm driver was recently converted to DRM_DEV_*, so let's go with that for all of msm/dp/* > + 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; > @@ -125,6 +167,12 @@ static int dp_display_bind(struct device *dev, struct device *master, > goto end; > } > > + rc = dp_get_pll(dp); > + if (rc) { > + pr_err(" DP get PLL instance failed\n"); Print rc, here and everywhere else. > + goto end; > + } > + > rc = dp->aux->drm_aux_register(dp->aux); > if (rc) { > pr_err("DRM DP AUX register failed\n"); > @@ -921,6 +969,7 @@ int __init msm_dp_register(void) > { > int ret; > > + msm_dp_pll_driver_register(); > ret = platform_driver_register(&dp_display_driver); > if (ret) { > pr_err("driver register failed"); > @@ -932,6 +981,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 ca5eae5..9cd6a6b 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.h > +++ b/drivers/gpu/drm/msm/dp/dp_display.h > @@ -52,4 +52,7 @@ struct msm_dp { > > int dp_display_get_num_of_displays(void); > int dp_display_get_displays(void **displays, int count); > +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 b39ea02..372997e 100644 > --- a/drivers/gpu/drm/msm/dp/dp_parser.h > +++ b/drivers/gpu/drm/msm/dp/dp_parser.h > @@ -16,6 +16,7 @@ > #define _DP_PARSER_H_ > > #include "dpu_io_util.h" > +#include "pll/dp_pll.h" > > #define DP_LABEL "MDSS DP DISPLAY" > #define AUX_CFG_LEN 10 > @@ -175,6 +176,8 @@ struct dp_parser { > struct dp_pinctrl pinctrl; > struct dp_io io; > struct dp_display_data disp_data; > + struct msm_dp_pll *pll; > + struct device *pll_dev; Store pll_dev in msm_dp_pll as 'dev' and remove this? > > u8 l_map[4]; > struct dp_aux_cfg aux_cfg[AUX_CFG_LEN]; > diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c > index 1d58480..3a1679c 100644 > --- a/drivers/gpu/drm/msm/dp/dp_power.c > +++ b/drivers/gpu/drm/msm/dp/dp_power.c > @@ -23,7 +23,9 @@ struct dp_power_private { > struct dp_parser *parser; > struct platform_device *pdev; > struct clk *pixel_clk_rcg; > - struct clk *pixel_parent; > + struct clk *link_clk_src; > + struct clk *pixel_provider; > + struct clk *link_provider; > > struct dp_power dp_power; > > @@ -154,6 +156,16 @@ static int dp_power_clk_init(struct dp_power_private *power, bool enable) > goto exit; > } > > + if (power->parser->pll && power->parser->pll->get_provider) { > + rc = power->parser->pll->get_provider(power->parser->pll, > + &power->link_provider, &power->pixel_provider); Hopefully this gets simplified when you de-abstract the rest of the dp driver. > + if (rc) { > + pr_info("%s: can't get provider from pll, don't set parent\n", > + __func__); > + return 0; > + } > + } > + > if (enable) { > rc = msm_dss_get_clk(dev, core->clk_config, core->num_clk); > if (rc) { > @@ -173,17 +185,10 @@ static int dp_power_clk_init(struct dp_power_private *power, bool enable) > if (IS_ERR(power->pixel_clk_rcg)) { > pr_debug("Unable to get DP pixel clk RCG\n"); > power->pixel_clk_rcg = NULL; > - } > - > - power->pixel_parent = devm_clk_get(dev, "pixel_parent"); > - if (IS_ERR(power->pixel_parent)) { > - pr_debug("Unable to get DP pixel RCG parent\n"); > - power->pixel_parent = NULL; > + rc = -ENODEV; > + goto ctrl_get_error; > } > } else { > - if (power->pixel_parent) > - devm_clk_put(dev, power->pixel_parent); > - > if (power->pixel_clk_rcg) > devm_clk_put(dev, power->pixel_clk_rcg); > > @@ -459,6 +464,49 @@ static void dp_power_client_deinit(struct dp_power *dp_power) > > } > > +static int dp_power_set_link_clk_parent(struct dp_power *dp_power) > +{ > + int rc = 0; > + struct dp_power_private *power; > + u32 num; > + struct dss_clk *cfg; > + char *name = "ctrl_link_clk"; > + > + if (!dp_power) { > + pr_err("invalid power data\n"); > + rc = -EINVAL; > + goto exit; > + } > + > + power = container_of(dp_power, struct dp_power_private, dp_power); > + > + num = power->parser->mp[DP_CTRL_PM].num_clk; > + cfg = power->parser->mp[DP_CTRL_PM].clk_config; > + > + while (num && strcmp(cfg->clk_name, name)) { I think I commented on this in the other patch, but don't use strings to do lookups, please just store the pointer directly if you need a reference. > + num--; > + cfg++; > + } > + > + if (num && power->link_provider) { > + power->link_clk_src = clk_get_parent(cfg->clk); Check return for ERR_PTR > + if (power->link_clk_src) { Indenting is wrong on this block > + clk_set_parent(power->link_clk_src, power->link_provider); > + pr_debug("%s: is the parent of clk=%s\n", > + __clk_get_name(power->link_provider), > + __clk_get_name(power->link_clk_src)); > + } else { > + pr_err("couldn't get parent for clk=%s\n", name); > + rc = -EINVAL; Make thi: if (!power->link_clk_src) { DRM_DEV_ERROR(...) return -EINVAL; } ret = clk_set_parent(power->link_clk_src, power->link_provider); if (ret) { /* propertly handle error */ } pr_debug("%s: is the parent of clk=%s\n", __clk_get_name(power->link_provider), __clk_get_name(power->link_clk_src)); > + } > + } else { > + pr_err("%s clock could not be set parent\n", name); > + rc = -EINVAL; > + } Same thing here, flip the if condition to check for error and return early, thus saving yourself a level of indentation for the successful case. > +exit: > + return rc; Remove exit label and return directly above > +} > + > static int dp_power_set_pixel_clk_parent(struct dp_power *dp_power) > { > int rc = 0; > @@ -472,8 +520,12 @@ static int dp_power_set_pixel_clk_parent(struct dp_power *dp_power) > > power = container_of(dp_power, struct dp_power_private, dp_power); > > - if (power->pixel_clk_rcg && power->pixel_parent) > - clk_set_parent(power->pixel_clk_rcg, power->pixel_parent); > + if (power->pixel_clk_rcg && power->pixel_provider) { > + clk_set_parent(power->pixel_clk_rcg, power->pixel_provider); s/pixel_parent/pixel_provider/ isn't related to this change, can you please use the final name in the main dp patch so it's correct in this one? > + pr_debug("%s: is the parent of clk=%s\n", > + __clk_get_name(power->pixel_provider), > + __clk_get_name(power->pixel_clk_rcg)); Same comment here, this debug can go in the main patch. > + } > exit: > return rc; > } > @@ -577,6 +629,7 @@ struct dp_power *dp_power_get(struct dp_parser *parser) > dp_power->init = dp_power_init; > dp_power->deinit = dp_power_deinit; > dp_power->clk_enable = dp_power_clk_enable; > + dp_power->set_link_clk_parent = dp_power_set_link_clk_parent; Same comment here regarding the unnecessary indirection, just call this thing directly where applicable. > dp_power->set_pixel_clk_parent = dp_power_set_pixel_clk_parent; > dp_power->power_client_init = dp_power_client_init; > dp_power->power_client_deinit = dp_power_client_deinit; > diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h > index d1adaaf..5effd32 100644 > --- a/drivers/gpu/drm/msm/dp/dp_power.h > +++ b/drivers/gpu/drm/msm/dp/dp_power.h > @@ -24,6 +24,7 @@ > * @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 { > @@ -31,6 +32,7 @@ struct dp_power { > int (*deinit)(struct dp_power *power); > int (*clk_enable)(struct dp_power *power, enum dp_pm_type pm_type, > bool enable); > + int (*set_link_clk_parent)(struct dp_power *power); > int (*set_pixel_clk_parent)(struct dp_power *power); > int (*power_client_init)(struct dp_power *power); > void (*power_client_deinit)(struct dp_power *power); > 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..a8612b5 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll.c > @@ -0,0 +1,153 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ SPDX please (and elsewhere). > + > +#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) { > + pr_err("clocks are not defined\n"); > + goto clk_err; > + } > + > + mp->clk_config = devm_kzalloc(&pdev->dev, > + 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: remove clk_err and return directly above > + return rc; > +} > + > +struct msm_dp_pll *msm_dp_pll_init(struct platform_device *pdev, static please > + 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)) { > + dev_err(dev, "%s: failed to init DP PLL\n", __func__); > + return pll; > + } > + > + pll->type = type; This should be set in the type-specific init function (ie: msm_dp_pll_10nm_init()). That will let you simplify this entire function to: { switch (type) { case MSM_DP_PLL_10NM: return msm_dp_pll_10nm_init(pdev, id); default: DRM_DEV_ERROR(&pdev->dev, "Invalid pll type: %d\n", type); return ERR_PTR(-ENXIO); } } > + > + DBG("DP:%d PLL registered", id); > + > + return pll; > +} > + > +static const struct of_device_id dp_pll_dt_match[] = { > +#ifdef CONFIG_DRM_MSM_DP_10NM_PLL I don't think you need this ifdef check, you've already provided a stub for the init function > + { .compatible = "qcom,dp-pll-10nm" }, > +#endif > + {} > +}; > + > +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; > + enum msm_dp_pll_type type; > + > + match = of_match_node(dp_pll_dt_match, dev->of_node); > + if (!match) > + return -ENODEV; > + > + if (!strcmp(match->compatible, "qcom,dp-pll-10nm")) > + type = MSM_DP_PLL_10NM; This is what of_device_id->data is for, use it to store the type. Can you also check the rest of the driver and do the same there? > + else > + type = MSM_DP_PLL_MAX; > + > + pll = msm_dp_pll_init(pdev, type, 0); > + if (IS_ERR_OR_NULL(pll)) { > + dev_info(dev, Why info level? > + "%s: pll init failed: %ld, need separate pll clk driver\n", > + __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) { > + //msm_dsi_pll_destroy(pll); Hmm. > + pll = NULL; No need to clear a local variable > + } > + > + platform_set_drvdata(pdev, NULL); You don't need to clear this either > + > + 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..d52a81a > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll.h > @@ -0,0 +1,64 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#ifndef __DP_PLL_H > +#define __DP_PLL_H > + > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/platform_device.h> > + > +#include "dpu_io_util.h" > +#include "msm_drv.h" > + > +#define PLL_REG_W(base, offset, data) \ > + writel_relaxed((data), (base) + (offset)) > +#define PLL_REG_R(base, offset) readl_relaxed((base) + (offset)) These macros aren't really useful, tbh. It'd be better to have a function that takes a msm_dp_pll, offset and data. > + > +enum msm_dp_pll_type { > + MSM_DP_PLL_10NM, > + MSM_DP_PLL_MAX > +}; > + > +struct msm_dp_pll { > + enum msm_dp_pll_type type; Storing this doesn't seem to gain you anything. So move the dp_pll_type enum into msm_dp_pll.c, turf the _MAX and make embed it in the .data element of the of_device_id structs. Then you can use it when you're initializing and promptly throw it away. > + 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; The clk_hw/etc part of this patch should be reviewed by swboyd (cc'd). It's not really in my wheelhouse, tbh. > + void *priv; Was going to comment on using an opaque pointer, but it not used anywhere \o/ Please remove > + /* 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) Please make this a type-safe inline function. > + > +struct msm_dp_pll *msm_dp_pll_init(struct platform_device *pdev, > + enum msm_dp_pll_type type, int id); Remove this and make that func static > + > +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 > +struct msm_dp_pll *msm_dp_pll_10nm_init(struct platform_device *pdev, int id) > +{ > + 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..a180413 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c > @@ -0,0 +1,401 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +/* > + * Display Port PLL driver block diagram for branch clocks > + * > + * +------------------------------+ > + * | DP_VCO_CLK | > + * | | > + * | +-------------------+ | > + * | | (DP PLL/VCO) | | > + * | +---------+---------+ | > + * | v | > + * | +----------+-----------+ | > + * | | hsclk_divsel_clk_src | | > + * | +----------+-----------+ | > + * +------------------------------+ > + * | > + * +------------<---------v------------>----------+ > + * | | > + * +-----v------------+ | > + * | dp_link_clk_src | | > + * | divsel_ten | | > + * +---------+--------+ | > + * | | > + * | | > + * 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 > + * | > + * +----------+---------+ > + * | vco_divided_clk | > + * | _src_mux | > + * +---------+----------+ > + * | > + * v > + * Input to DISPCC block > + * for DP pixel clock Some of your vertical lines seem misaligned here, can you please fix this up? > + * > + */ > + > +#include <linux/kernel.h> > +#include <linux/err.h> > +#include <linux/delay.h> > +#include <linux/regmap.h> > +#include <linux/clk.h> > + > +#include "dp_pll_10nm.h" > + > +#define NUM_PROVIDED_CLKS 2 > + > +#define DP_LINK_CLK_SRC 0 > +#define DP_PIXEL_CLK_SRC 1 Instead of using an array to store the clocks in one place, why not just store a pointer for each clk? Then you can get rid of these and just use the clk directly. > + > +static struct dp_pll_10nm *dp_pdb; This isn't used (nor should it be). > + > +/* Op structures */ > +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) typesafe static inline please. Everywhere you use pclksel, you just grab ->pll from the result and never use pclksel again. So make the function return dp_pll_10nm * directly and save yourself the local var. > + > +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, DP_PHY_VCO_DIV); > + auxclk_div &= ~0x03; /* bits 0 to 1 */ This comment isn't really useful :) Could you please #define all of the hardcoded values in the patch? > + > + 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; > + > + PLL_REG_W(dp_res->phy_base, > + DP_PHY_VCO_DIV, auxclk_div); > + /* Make sure the PHY registers writes are done */ > + wmb(); Same comments about needing wmb to work around using _relaxed > + pr_debug("%s: mux=%d auxclk_div=%x\n", __func__, val, auxclk_div); > + > + 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; > + > + pr_err("clk_hw->init->name = %s\n", hw->init->name); > + auxclk_div = PLL_REG_R(dp_res->phy_base, 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; > + > + pr_debug("%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) > +{ > + int ret = 0; no need to init this to 0 > + > + ret = __clk_mux_determine_rate_closest(hw, req); > + if (ret) > + return ret; > + > + /* Set the new parent of mux if there is a new valid parent */ > + if (hw->clk && req->best_parent_hw->clk) > + clk_set_parent(hw->clk, req->best_parent_hw->clk); What about the return value? All other clk_set_parent calls in this patch are also unchecked, so please add those as well. > + > + return 0; > +} > + > +static unsigned long mux_recalc_rate(struct clk_hw *hw, > + 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); > + if (!div_clk) According to the header documentation, clk_get_parent can return ERR_PTR as well. Same comment applies to other callsites. > + return 0; > + > + vco_clk = clk_get_parent(div_clk); > + if (!vco_clk) > + return 0; > + > + vco = hw_clk_to_pll(__clk_get_hw(vco_clk)); > + if (!vco) > + return 0; > + > + if (vco->rate == DP_VCO_HSCLK_RATE_8100MHZDIV1000) > + return (vco->rate / 6); Unnecessary () here and below > + else if (vco->rate == DP_VCO_HSCLK_RATE_5400MHZDIV1000) > + return (vco->rate / 4); > + else > + return (vco->rate / 2); > +} > + > +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; > + > + return 0; Why have a return value when the only place always returns 0? Make it void and simplify error checking at the callsite. > +} > + > +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" }, > + .num_parents = 3, > + .name = "dp_vco_divided_clk_src_mux", > + .flags = 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; pclksel_init goes out of scope at the end of the function, yet it is persisted in pll_pclksel. That should be fixed. > + > + 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" }, > + .num_parents = 1, > + .name = vco_name, > + .flags = CLK_IGNORE_UNUSED, > + .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; > + > + DBG("DP->id = %d", pll_10nm->id); > + > + hw_data = devm_kzalloc(dev, sizeof(*hw_data) + > + 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; Same comment about scope here > + 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_link_clk_divsel_ten"); > + snprintf(parent, 32, "dp_vco_clk"); > + 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; I'm going to leave this chunk for Stephen to comment on, but again I'd rather not store these clocks as an array and do string lookups on them. > + > + 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) { > + dev_err(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); > + > + DBG("DP PLL%d", id); Please remove (or convert to DRM_DEV_DEBUG) > + > + 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)) { > + dev_err(&pdev->dev, "failed to map CMN PLL base\n"); Print the error if pll_base is ERR_PTR, same for below. > + return ERR_PTR(-ENOMEM); You should preserve the error if pll_base is an ERR_PTR, same for below. > + } > + > + dp_10nm_pll->phy_base = msm_ioremap(pdev, "phy_base", "DP_PHY"); > + if (IS_ERR_OR_NULL(dp_10nm_pll->phy_base)) { > + dev_err(&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)) { > + dev_err(&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)) { > + dev_err(&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) { > + pr_err("Unable to get the cell-index ret=%d\n", ret); If this is truly an error condition, we should return an error. If it's not, downgrade this to info > + dp_10nm_pll->index = 0; > + } > + > + ret = msm_dp_pll_util_parse_dt_clock(pdev, &dp_10nm_pll->base); > + if (ret) { > + pr_err("Unable to parse dt clocks ret=%d\n", ret); > + return ERR_PTR(ret); > + } > + > + ret = dp_pll_10nm_register(dp_10nm_pll); > + if (ret) { > + dev_err(&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; > + 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..f966beb > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h > @@ -0,0 +1,94 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#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 Isn't MHZDIV1000 just KHZ? Same for below > +#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 These can go in the c file (and probably the others too) > + > +#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) typesafe inline pls > + > +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..9eb6881 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c > @@ -0,0 +1,531 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#define pr_fmt(fmt) "%s: " fmt, __func__ > + > +#include <linux/kernel.h> > +#include <linux/err.h> > +#include <linux/iopoll.h> > +#include <linux/delay.h> > + > +#include "dp_pll.h" > +#include "dp_pll_10nm.h" > +#include "dp_extcon.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, DP_PHY_SPARE0); > + dp_res->lane_cnt = spare_value & 0x0F; > + dp_res->orientation = (spare_value & 0xF0) >> 4; > + > + pr_debug("%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: > + pr_debug("%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: > + pr_debug("%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: > + pr_debug("%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: > + pr_debug("%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; Since this is all static, a static const lookup table would probably be more appropriate. > + default: > + return -EINVAL; Log this please > + } > + 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) { > + pr_err("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, DP_PHY_PD_CTL, 0x6d); > + else > + PLL_REG_W(dp_res->phy_base, DP_PHY_PD_CTL, 0x75); > + } else { > + PLL_REG_W(dp_res->phy_base, DP_PHY_PD_CTL, 0x7d); > + } > + > + /* Make sure the PHY register writes are done */ > + wmb(); > + > + 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); > + /* Make sure the PLL register writes are done */ > + wmb(); > + > + 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); > + /* Make sure the PHY register writes are done */ > + wmb(); > + > + if (dp_res->orientation == ORIENTATION_CC2) > + PLL_REG_W(dp_res->phy_base, DP_PHY_MODE, 0x4c); > + else > + PLL_REG_W(dp_res->phy_base, DP_PHY_MODE, 0x5c); > + /* Make sure the PLL register writes are done */ > + wmb(); > + > + /* TX Lane configuration */ > + PLL_REG_W(dp_res->phy_base, > + DP_PHY_TX0_TX1_LANE_CTL, 0x05); > + PLL_REG_W(dp_res->phy_base, > + DP_PHY_TX2_TX3_LANE_CTL, 0x05); > + > + /* TX-0 register configuration */ > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TRANSCEIVER_BIAS_EN, 0x1a); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_VMODE_CTRL1, 0x40); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_PRE_STALL_LDO_BOOST_EN, 0x30); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_INTERFACE_SELECT, 0x3d); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_CLKBUF_ENABLE, 0x0f); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_RESET_TSYNC_EN, 0x03); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TRAN_DRVR_EMP_EN, 0x03); > + PLL_REG_W(dp_res->ln_tx0_base, > + TXn_PARRATE_REC_DETECT_IDLE_EN, 0x00); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_INTERFACE_MODE, 0x00); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_BAND, 0x4); > + > + /* TX-1 register configuration */ > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TRANSCEIVER_BIAS_EN, 0x1a); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_VMODE_CTRL1, 0x40); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_PRE_STALL_LDO_BOOST_EN, 0x30); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_INTERFACE_SELECT, 0x3d); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_CLKBUF_ENABLE, 0x0f); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_RESET_TSYNC_EN, 0x03); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TRAN_DRVR_EMP_EN, 0x03); > + PLL_REG_W(dp_res->ln_tx1_base, > + TXn_PARRATE_REC_DETECT_IDLE_EN, 0x00); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_INTERFACE_MODE, 0x00); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_BAND, 0x4); > + /* Make sure the PHY register writes are done */ > + wmb(); > + > + /* dependent on the vco frequency */ > + PLL_REG_W(dp_res->phy_base, 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)) { > + pr_err("%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 + > + DP_PHY_STATUS), > + status, > + ((status & (BIT(1))) > 0), > + DP_PHY_PLL_POLL_SLEEP_US, > + DP_PHY_PLL_POLL_TIMEOUT_US)) { > + pr_err("%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, DP_PHY_AUX_CFG2, 0x04); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x01); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x05); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x01); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x09); > + wmb(); /* Make sure the PHY register writes are done */ > + > + PLL_REG_W(dp_res->pll_base, QSERDES_COM_RESETSM_CNTRL, 0x20); > + wmb(); /* Make sure the PLL register writes are done */ > + > + if (!dp_10nm_pll_lock_status(dp_res)) { > + rc = -EINVAL; > + goto lock_err; > + } > + > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x19); > + /* Make sure the PHY register writes are done */ > + wmb(); > + /* poll for PHY ready status */ > + if (!dp_10nm_phy_rdy_status(dp_res)) { > + rc = -EINVAL; > + goto lock_err; > + } > + > + pr_debug("%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, > + TXn_HIGHZ_DRVR_EN, drvr_en); > + PLL_REG_W(dp_res->ln_tx1_base, > + TXn_TRANSCEIVER_BIAS_EN, bias_en); > + } else { > + PLL_REG_W(dp_res->ln_tx0_base, > + TXn_HIGHZ_DRVR_EN, drvr_en); > + PLL_REG_W(dp_res->ln_tx0_base, > + TXn_TRANSCEIVER_BIAS_EN, bias_en); > + } > + } else { > + PLL_REG_W(dp_res->ln_tx0_base, TXn_HIGHZ_DRVR_EN, drvr_en); > + PLL_REG_W(dp_res->ln_tx0_base, > + TXn_TRANSCEIVER_BIAS_EN, bias_en); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_HIGHZ_DRVR_EN, drvr_en); > + PLL_REG_W(dp_res->ln_tx1_base, > + TXn_TRANSCEIVER_BIAS_EN, bias_en); > + } I think you could probably simplify this code block with a bit of effort, especially the top condition. > + > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_POL_INV, 0x0a); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_POL_INV, 0x0a); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x18); > + udelay(2000); why udelay? > + > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x19); > + > + /* > + * Make sure all the register writes are completed before > + * doing any other operation > + */ > + wmb(); > + > + /* 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, TXn_TX_DRV_LVL, 0x38); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_DRV_LVL, 0x38); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_EMP_POST1_LVL, 0x20); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_EMP_POST1_LVL, 0x20); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_RES_CODE_LANE_OFFSET_TX, 0x06); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_RES_CODE_LANE_OFFSET_TX, 0x06); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_RES_CODE_LANE_OFFSET_RX, 0x07); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_RES_CODE_LANE_OFFSET_RX, 0x07); > + /* Make sure the PHY register writes are done */ > + wmb(); > + > +lock_err: > + return rc; Remove lock_err and return directly above. > +} > + > +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, DP_PHY_PD_CTL, 0x2); > + /* > + * Make sure all the register writes to disable PLL are > + * completed before doing any other operation > + */ > + wmb(); > + > + 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); > + > + pr_debug("rate=%ld\n", pll->rate); > + if ((dp_res->vco_cached_rate != 0) > + && (dp_res->vco_cached_rate == pll->rate)) { if (dp_res->vco_cached_rate && dp_res->vco_cached_rate == pll->rate) { > + rc = pll->clk_hw.init->ops->set_rate(hw, > + dp_res->vco_cached_rate, dp_res->vco_cached_rate); > + if (rc) { > + pr_err("index=%d vco_set_rate failed. rc=%d\n", > + rc, dp_res->index); > + goto error; > + } > + } > + > + rc = dp_pll_enable_10nm(hw); > + if (rc) { > + pr_err("ndx=%d failed to enable dp pll\n", > + dp_res->index); > + goto error; > + } > + > + pll->pll_on = true; Do you need locking around the prepare/unprepare functions? > +error: > + return rc; Same comment as always > +} > + > +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) { > + pr_err("Invalid input parameter\n"); > + return; > + } > + > + if (!pll->pll_on) { > + pr_err("pll resource can't be enabled\n"); > + return; > + } > + dp_res->vco_cached_rate = pll->rate; > + dp_pll_disable_10nm(hw); > + > + //dp_res->handoff_resources = false; ?? > + 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; > + > + pr_debug("DP lane CLK rate=%ld\n", rate); > + > + rc = dp_config_vco_rate_10nm(pll, rate); > + if (rc) > + pr_err("%s: Failed to set clk rate\n", __func__); Should you return early here? > + > + 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 { > + pr_debug("unknown divider. forcing to default\n"); > + hsclk_div = 5; > + } > + > + div = PLL_REG_R(dp_res->phy_base, 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 > + pr_err("%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; > + } > + > + pr_debug("returning vco rate = %lu\n", (unsigned long)vco_rate); > + > + dp_res->vco_cached_rate = pll->rate = vco_rate; > + return (unsigned long)vco_rate; Hopefully this function will become easier to parse with #define'd values > +} > + > +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; You're assuming min_rate is < 2.7MHz and max_rate > 5.4 MHz. This is true in the current code, but could change in the future. Fortunatley min/max_rate are only used here. So delete them from the struct and use the DP_VCO_HSCLK_RATE_* values directly here. > + > + pr_debug("%s: rrate=%ld\n", __func__, rrate); > + > + *parent_rate = rrate; > + return rrate; > +} > + > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
On Thu, Nov 01, 2018 at 05:03:15PM -0400, Sean Paul wrote: > On Wed, Oct 10, 2018 at 10:15:59AM -0700, Chandan Uddaraju wrote: > > 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. > > > > Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org> > > --- > > drivers/gpu/drm/msm/Kconfig | 16 + > > drivers/gpu/drm/msm/Makefile | 6 + > > drivers/gpu/drm/msm/dp/dp_ctrl.c | 1 + > > drivers/gpu/drm/msm/dp/dp_display.c | 50 +++ > > drivers/gpu/drm/msm/dp/dp_display.h | 3 + > > drivers/gpu/drm/msm/dp/dp_parser.h | 3 + > > drivers/gpu/drm/msm/dp/dp_power.c | 77 +++- > > drivers/gpu/drm/msm/dp/dp_power.h | 2 + > > drivers/gpu/drm/msm/dp/pll/dp_pll.c | 153 ++++++++ > > drivers/gpu/drm/msm/dp/pll/dp_pll.h | 64 ++++ > > drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c | 401 +++++++++++++++++++ > > drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h | 94 +++++ > > drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c | 531 ++++++++++++++++++++++++++ > > 13 files changed, 1389 insertions(+), 12 deletions(-) > > 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 <snip> > > +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); > > + > > + DBG("DP PLL%d", id); > > Please remove (or convert to DRM_DEV_DEBUG) > > > + > > + 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)) { > > + dev_err(&pdev->dev, "failed to map CMN PLL base\n"); > > Print the error if pll_base is ERR_PTR, same for below. FWIW the return value from msm_ioremap is always ERR_PTR never NULL. And msm_ioremap prints error messages on all failures so all you need is: if (IS_ERR(ptr)) return ERR_CAST(ptr); > > + return ERR_PTR(-ENOMEM); > > You should preserve the error if pll_base is an ERR_PTR, same for below. > > > + } > > + > > + dp_10nm_pll->phy_base = msm_ioremap(pdev, "phy_base", "DP_PHY"); > > + if (IS_ERR_OR_NULL(dp_10nm_pll->phy_base)) { > > + dev_err(&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)) { > > + dev_err(&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)) { > > + dev_err(&pdev->dev, "failed to map CMN LN_TX1 base\n"); > > + return ERR_PTR(-ENOMEM); > > + } <snip> Jordan
Quoting Sean Paul (2018-11-01 14:03:15) > On Wed, Oct 10, 2018 at 10:15:59AM -0700, Chandan Uddaraju wrote: > > > +} > > + > > static int dp_power_set_pixel_clk_parent(struct dp_power *dp_power) > > { > > int rc = 0; > > @@ -472,8 +520,12 @@ static int dp_power_set_pixel_clk_parent(struct dp_power *dp_power) > > > > power = container_of(dp_power, struct dp_power_private, dp_power); > > > > - if (power->pixel_clk_rcg && power->pixel_parent) > > - clk_set_parent(power->pixel_clk_rcg, power->pixel_parent); > > + if (power->pixel_clk_rcg && power->pixel_provider) { > > + clk_set_parent(power->pixel_clk_rcg, power->pixel_provider); > > s/pixel_parent/pixel_provider/ isn't related to this change, can you please use > the final name in the main dp patch so it's correct in this one? > > > + pr_debug("%s: is the parent of clk=%s\n", > > + __clk_get_name(power->pixel_provider), > > + __clk_get_name(power->pixel_clk_rcg)); > > Same comment here, this debug can go in the main patch. Is there any reason why assigned-clock-parents wouldn't work here? > > diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h > > index d1adaaf..5effd32 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_power.h > > +++ b/drivers/gpu/drm/msm/dp/dp_power.h > > @@ -24,6 +24,7 @@ > > * @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 { [...] > > + > > +static int clk_mux_determine_rate(struct clk_hw *hw, > > + struct clk_rate_request *req) > > +{ > > + int ret = 0; > > no need to init this to 0 > > > + > > + ret = __clk_mux_determine_rate_closest(hw, req); > > + if (ret) > > + return ret; > > + > > + /* Set the new parent of mux if there is a new valid parent */ > > + if (hw->clk && req->best_parent_hw->clk) > > + clk_set_parent(hw->clk, req->best_parent_hw->clk); > > What about the return value? All other clk_set_parent calls in this patch are > also unchecked, so please add those as well. The determine_rate clk_op should _never_ change clk hardware. It's a function that's about determining what rate a clk can support based on the rate that's requested and what the hardware can do. Ok, let me just take a pass at the overall clk parts of this patch instead of doing an "and also" review on Sean's review.
This patch is too big. Please split it up more. I have far too many review comments on it so it will help if you keep splitting the patch up into smaller parts that can be more easily reviewed and fixed. Quoting Chandan Uddaraju (2018-10-10 10:15:59) > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index 08a52f5..e23beee 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c > @@ -1051,6 +1051,7 @@ static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl) > { > int ret = 0; > > + ctrl->power->set_link_clk_parent(ctrl->power); > ctrl->power->set_pixel_clk_parent(ctrl->power); > > dp_ctrl_set_clock_rate(ctrl, "ctrl_link_clk", > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index 8c98399..2bf6635 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -72,6 +72,48 @@ 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) { > + pr_err("Invalid Arguments\n"); > + return -EINVAL; > + } > + > + pdev = dp_priv->pdev; > + dp_parser = dp_priv->parser; > + > + if (!dp_parser) { > + pr_err("Parser not initialized.\n"); > + return -EINVAL; > + } > + > + pll_node = of_parse_phandle(pdev->dev.of_node, "pll-node", 0); > + if (!pll_node) { > + dev_err(&pdev->dev, "cannot find pll device\n"); > + 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) { > + dev_err(&pdev->dev, "%s: pll driver is not ready\n", __func__); > + return -EPROBE_DEFER; > + } > + > + dp_parser->pll_dev = get_device(&pll_pdev->dev); Why do we need this code? Are we working around some lack of ability to link different devices together by resource requesting? > + > + return 0; > +} > + > static irqreturn_t dp_display_irq(int irq, void *dev_id) > { > struct dp_display_private *dp = dev_id; > @@ -125,6 +167,12 @@ static int dp_display_bind(struct device *dev, struct device *master, > goto end; > } > > + rc = dp_get_pll(dp); > + if (rc) { > + pr_err(" DP get PLL instance failed\n"); > + goto end; > + } > + > rc = dp->aux->drm_aux_register(dp->aux); > if (rc) { > pr_err("DRM DP AUX register failed\n"); > @@ -921,6 +969,7 @@ int __init msm_dp_register(void) > { > int ret; > > + msm_dp_pll_driver_register(); > ret = platform_driver_register(&dp_display_driver); > if (ret) { > pr_err("driver register failed"); > @@ -932,6 +981,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 ca5eae5..9cd6a6b 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.h > +++ b/drivers/gpu/drm/msm/dp/dp_display.h > @@ -52,4 +52,7 @@ struct msm_dp { > > int dp_display_get_num_of_displays(void); > int dp_display_get_displays(void **displays, int count); > +void __init msm_dp_pll_driver_register(void); > +void __exit msm_dp_pll_driver_unregister(void); __init and __exit in header files does nothing. > + > #endif /* _DP_DISPLAY_H_ */ > diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c > index 1d58480..3a1679c 100644 > --- a/drivers/gpu/drm/msm/dp/dp_power.c > +++ b/drivers/gpu/drm/msm/dp/dp_power.c > @@ -23,7 +23,9 @@ struct dp_power_private { > struct dp_parser *parser; > struct platform_device *pdev; > struct clk *pixel_clk_rcg; > - struct clk *pixel_parent; > + struct clk *link_clk_src; > + struct clk *pixel_provider; > + struct clk *link_provider; > > struct dp_power dp_power; > > @@ -154,6 +156,16 @@ static int dp_power_clk_init(struct dp_power_private *power, bool enable) > goto exit; > } > > + if (power->parser->pll && power->parser->pll->get_provider) { > + rc = power->parser->pll->get_provider(power->parser->pll, > + &power->link_provider, &power->pixel_provider); > + if (rc) { > + pr_info("%s: can't get provider from pll, don't set parent\n", > + __func__); > + return 0; > + } > + } > + > if (enable) { > rc = msm_dss_get_clk(dev, core->clk_config, core->num_clk); > if (rc) { > @@ -173,17 +185,10 @@ static int dp_power_clk_init(struct dp_power_private *power, bool enable) > if (IS_ERR(power->pixel_clk_rcg)) { > pr_debug("Unable to get DP pixel clk RCG\n"); > power->pixel_clk_rcg = NULL; > - } > - > - power->pixel_parent = devm_clk_get(dev, "pixel_parent"); > - if (IS_ERR(power->pixel_parent)) { > - pr_debug("Unable to get DP pixel RCG parent\n"); > - power->pixel_parent = NULL; > + rc = -ENODEV; > + goto ctrl_get_error; > } > } else { > - if (power->pixel_parent) > - devm_clk_put(dev, power->pixel_parent); > - > if (power->pixel_clk_rcg) > devm_clk_put(dev, power->pixel_clk_rcg); > > @@ -459,6 +464,49 @@ static void dp_power_client_deinit(struct dp_power *dp_power) > > } > > +static int dp_power_set_link_clk_parent(struct dp_power *dp_power) > +{ > + int rc = 0; > + struct dp_power_private *power; > + u32 num; > + struct dss_clk *cfg; > + char *name = "ctrl_link_clk"; > + > + if (!dp_power) { > + pr_err("invalid power data\n"); > + rc = -EINVAL; > + goto exit; > + } > + > + power = container_of(dp_power, struct dp_power_private, dp_power); > + > + num = power->parser->mp[DP_CTRL_PM].num_clk; > + cfg = power->parser->mp[DP_CTRL_PM].clk_config; > + > + while (num && strcmp(cfg->clk_name, name)) { > + num--; > + cfg++; > + } > + > + if (num && power->link_provider) { > + power->link_clk_src = clk_get_parent(cfg->clk); > + if (power->link_clk_src) { > + clk_set_parent(power->link_clk_src, power->link_provider); > + pr_debug("%s: is the parent of clk=%s\n", > + __clk_get_name(power->link_provider), > + __clk_get_name(power->link_clk_src)); > + } else { > + pr_err("couldn't get parent for clk=%s\n", name); > + rc = -EINVAL; > + } > + } else { > + pr_err("%s clock could not be set parent\n", name); > + rc = -EINVAL; > + } Hopefully this can just be done with assigned-clock-parents. > +exit: > + return rc; > +} > + > static int dp_power_set_pixel_clk_parent(struct dp_power *dp_power) > { > int rc = 0; > diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h > index d1adaaf..5effd32 100644 > --- a/drivers/gpu/drm/msm/dp/dp_power.h > +++ b/drivers/gpu/drm/msm/dp/dp_power.h > @@ -24,6 +24,7 @@ > * @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 { > @@ -31,6 +32,7 @@ struct dp_power { > int (*deinit)(struct dp_power *power); > int (*clk_enable)(struct dp_power *power, enum dp_pm_type pm_type, > bool enable); > + int (*set_link_clk_parent)(struct dp_power *power); > int (*set_pixel_clk_parent)(struct dp_power *power); Seeing these clk wrappers is quite scary. Is this driver written with some sort of linux abstraction layer inside? > int (*power_client_init)(struct dp_power *power); > void (*power_client_deinit)(struct dp_power *power); > 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..a8612b5 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll.c > @@ -0,0 +1,153 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#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) { > + pr_err("clocks are not defined\n"); > + goto clk_err; > + } > + > + mp->clk_config = devm_kzalloc(&pdev->dev, > + 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; Is this custom assigned-clock-rates? > + } > + > +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)) { > + dev_err(dev, "%s: failed to init DP PLL\n", __func__); > + return pll; > + } > + > + pll->type = type; > + > + DBG("DP:%d PLL registered", id); > + > + return pll; > +} > + > +static const struct of_device_id dp_pll_dt_match[] = { > +#ifdef CONFIG_DRM_MSM_DP_10NM_PLL Don't do this. > + { .compatible = "qcom,dp-pll-10nm" }, > +#endif > + {} > +}; > + > +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; > + enum msm_dp_pll_type type; > + > + match = of_match_node(dp_pll_dt_match, dev->of_node); > + if (!match) > + return -ENODEV; > + > + if (!strcmp(match->compatible, "qcom,dp-pll-10nm")) > + type = MSM_DP_PLL_10NM; > + else > + type = MSM_DP_PLL_MAX; > + > + pll = msm_dp_pll_init(pdev, type, 0); > + if (IS_ERR_OR_NULL(pll)) { IS_ERR_OR_NULL is typically a bad sign. > + dev_info(dev, > + "%s: pll init failed: %ld, need separate pll clk driver\n", > + __func__, PTR_ERR(pll)); Well if it's NULL, then it's not a PTR_ERR. > + return -ENODEV; And then we just overwrite whatever is returned, ouch. > + } > + > + 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) { > + //msm_dsi_pll_destroy(pll); Leftovers? > + 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); > +} Consider module_platform_driver()? Why is this directly probed from somewhere else? Some sort of DPU design? > 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..d52a81a > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll.h > @@ -0,0 +1,64 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#ifndef __DP_PLL_H > +#define __DP_PLL_H > + > +#include <linux/clk.h> > +#include <linux/clk-provider.h> Typically a bad sign to see a clk provider and consumer in the same place. > +#include <linux/platform_device.h> > + > +#include "dpu_io_util.h" > +#include "msm_drv.h" > + > +#define PLL_REG_W(base, offset, data) \ > + writel_relaxed((data), (base) + (offset)) > +#define PLL_REG_R(base, offset) readl_relaxed((base) + (offset)) Can't we just use writel and readl directly? > + > +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 */ Why are we duplicating what is known to the framework? > + u64 min_rate; /* min vco rate */ > + u64 max_rate; /* max vco rate */ Use clk_hw_set_rate_range? > + bool pll_on; More duplicating of what clk framework knows? > + 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) Usually this would be called to_msm_dp_pll(). > + > +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, > 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..a180413 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c > @@ -0,0 +1,401 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +/* > + * Display Port PLL driver block diagram for branch clocks > + * > + * +------------------------------+ > + * | DP_VCO_CLK | > + * | | > + * | +-------------------+ | > + * | | (DP PLL/VCO) | | > + * | +---------+---------+ | > + * | v | > + * | +----------+-----------+ | > + * | | hsclk_divsel_clk_src | | > + * | +----------+-----------+ | Is this a divider? > + * +------------------------------+ > + * | > + * +------------<---------v------------>----------+ > + * | | > + * +-----v------------+ | > + * | dp_link_clk_src | | > + * | divsel_ten | | > + * +---------+--------+ | And dispcc gets a fixed div-10 on the output of the PLL divider? > + * | | > + * | | > + * 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 > + * | > + * +----------+---------+ > + * | vco_divided_clk | > + * | _src_mux | > + * +---------+----------+ > + * | > + * v > + * Input to DISPCC block > + * for DP pixel clock > + * > + */ > + > +#include <linux/kernel.h> > +#include <linux/err.h> > +#include <linux/delay.h> > +#include <linux/regmap.h> > +#include <linux/clk.h> I'd expect clk-provider.h here, instead of clk.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; > + > +/* Op structures */ Drop useless comments please. > +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, 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; Is this really a mux? It looks like it's a divider that divides the PLL frequency down. Why add in an extra level of set_parent things when we could just as easily do division and call up to parent with some multiplied rate? > + > + PLL_REG_W(dp_res->phy_base, > + DP_PHY_VCO_DIV, auxclk_div); > + /* Make sure the PHY registers writes are done */ Which this wmb() doesn't do. > + wmb(); > + pr_debug("%s: mux=%d auxclk_div=%x\n", __func__, val, auxclk_div); > + > + 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; > + > + pr_err("clk_hw->init->name = %s\n", hw->init->name); > + auxclk_div = PLL_REG_R(dp_res->phy_base, 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; > + > + pr_debug("%s: auxclk_div=%d, val=%d\n", __func__, auxclk_div, val); > + > + return val; Yes, all very odd! > +} > + > +static int clk_mux_determine_rate(struct clk_hw *hw, > + struct clk_rate_request *req) > +{ > + int ret = 0; > + > + ret = __clk_mux_determine_rate_closest(hw, req); > + if (ret) > + return ret; > + > + /* Set the new parent of mux if there is a new valid parent */ > + if (hw->clk && req->best_parent_hw->clk) > + clk_set_parent(hw->clk, req->best_parent_hw->clk); No idea what this is, but again we shouldn't be touching the hardware in determine_rate, and we certainly shouldn't be using clk consumer APIs from within clk provider ops. > + > + return 0; > +} > + > +static unsigned long mux_recalc_rate(struct clk_hw *hw, > + 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); > + 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)); > + if (!vco) > + return 0; All this parent checking should go away. > + > + 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); I'd expect the recalc_rate callback to read some hardware register and figure out what rate is coming through the clk. This doesn't look to be doing that. Instead it's reaching up two levels and then looking at some cached rate inside there to figure out what rate is coming here? That is weird. > +} > + > +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; > + > + return 0; > +} I don't know what this function is for, but it looks like some weird wrapper around kernel things which just makes for more reading and less transferable knowledge because we have to learn how different drivers decide to wrap kernel constructs. > + > +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" }, As I said earlier, this should probably just be a divider and not a mux. > + .num_parents = 3, > + .name = "dp_vco_divided_clk_src_mux", > + .flags = CLK_IGNORE_UNUSED, Why have this flag? Seems like a workaround for some problem, but who knows what that problem is. > + .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; And it is a divider? But not really? > + 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" }, > + .num_parents = 1, > + .name = vco_name, > + .flags = CLK_IGNORE_UNUSED, > + .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; > + > + DBG("DP->id = %d", pll_10nm->id); > + > + hw_data = devm_kzalloc(dev, sizeof(*hw_data) + > + 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_link_clk_divsel_ten"); > + snprintf(parent, 32, "dp_vco_clk"); Why are we snprintf strings that are static? Why not just pass them in places they're used? > + 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; The good news is that this should be sharply reduced by implementing a divider instead of fixed dividers that a virtual mux picks from. > + > + 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) { > + dev_err(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); > + > + DBG("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)) { > + dev_err(&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)) { > + dev_err(&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)) { > + dev_err(&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)) { > + dev_err(&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) { > + pr_err("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) { > + pr_err("Unable to parse dt clocks ret=%d\n", ret); > + return ERR_PTR(ret); > + } > + > + ret = dp_pll_10nm_register(dp_10nm_pll); > + if (ret) { > + dev_err(&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; Pass this to the framework with clk_hw_set_rate_range() and let the common clk framework clamp min/max for you. > + 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..f966beb > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h > @@ -0,0 +1,94 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#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 PLLs is plural, not possessive. > + * suspend/resume scenario. Cached the vco rate for such plls. There you go! > + */ > + unsigned long vco_cached_rate; > + u32 cached_cfg0; > + u32 cached_cfg1; > + u32 cached_outdiv; Is this to restore across suspend/resume? > + > + uint32_t index; We use u32 in kernel code, not uint32_t. > +}; > + > +#define to_dp_pll_10nm(x) container_of(x, struct dp_pll_10nm, base) > + > +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..9eb6881 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c > @@ -0,0 +1,531 @@ > +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#define pr_fmt(fmt) "%s: " fmt, __func__ > + > +#include <linux/kernel.h> > +#include <linux/err.h> > +#include <linux/iopoll.h> > +#include <linux/delay.h> Where's clk-provider.h? > + > +#include "dp_pll.h" > +#include "dp_pll_10nm.h" > +#include "dp_extcon.h" Is this used? > + > +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, DP_PHY_SPARE0); > + dp_res->lane_cnt = spare_value & 0x0F; > + dp_res->orientation = (spare_value & 0xF0) >> 4; > + > + pr_debug("%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: > + pr_debug("%s: VCO rate: %ld\n", __func__, > + DP_VCO_RATE_9720MHZDIV1000); These debug prints don't match the case statement above. Why? > + 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: > + pr_debug("%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: > + pr_debug("%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: > + pr_debug("%s: VCO rate: %ld\n", __func__, > + DP_VCO_RATE_8100MHZDIV1000); Ah this one matches though. > + 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) { > + pr_err("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, DP_PHY_PD_CTL, 0x6d); > + else > + PLL_REG_W(dp_res->phy_base, DP_PHY_PD_CTL, 0x75); > + } else { > + PLL_REG_W(dp_res->phy_base, DP_PHY_PD_CTL, 0x7d); > + } > + > + /* Make sure the PHY register writes are done */ > + wmb(); > + > + 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); All these lines could be much shorter with a local variable, 'base', that got assigned once at the start and then used throughout. > + > + /* 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); > + /* Make sure the PLL register writes are done */ > + wmb(); Is this to make sure that the next register writes happen after the above register writes? The comment could say that instead, but then realize that they may be sitting somewhere in the bus waiting to write to the device still, so if it really has to be written to the device we need to read back the last register written to be sure that the write posted to the device. > + > + 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); > + /* Make sure the PHY register writes are done */ > + wmb(); > + > + if (dp_res->orientation == ORIENTATION_CC2) > + PLL_REG_W(dp_res->phy_base, DP_PHY_MODE, 0x4c); > + else > + PLL_REG_W(dp_res->phy_base, DP_PHY_MODE, 0x5c); > + /* Make sure the PLL register writes are done */ > + wmb(); > + > + /* TX Lane configuration */ > + PLL_REG_W(dp_res->phy_base, > + DP_PHY_TX0_TX1_LANE_CTL, 0x05); > + PLL_REG_W(dp_res->phy_base, > + DP_PHY_TX2_TX3_LANE_CTL, 0x05); > + > + /* TX-0 register configuration */ > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TRANSCEIVER_BIAS_EN, 0x1a); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_VMODE_CTRL1, 0x40); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_PRE_STALL_LDO_BOOST_EN, 0x30); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_INTERFACE_SELECT, 0x3d); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_CLKBUF_ENABLE, 0x0f); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_RESET_TSYNC_EN, 0x03); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TRAN_DRVR_EMP_EN, 0x03); > + PLL_REG_W(dp_res->ln_tx0_base, > + TXn_PARRATE_REC_DETECT_IDLE_EN, 0x00); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_INTERFACE_MODE, 0x00); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_BAND, 0x4); > + > + /* TX-1 register configuration */ > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TRANSCEIVER_BIAS_EN, 0x1a); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_VMODE_CTRL1, 0x40); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_PRE_STALL_LDO_BOOST_EN, 0x30); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_INTERFACE_SELECT, 0x3d); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_CLKBUF_ENABLE, 0x0f); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_RESET_TSYNC_EN, 0x03); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TRAN_DRVR_EMP_EN, 0x03); > + PLL_REG_W(dp_res->ln_tx1_base, > + TXn_PARRATE_REC_DETECT_IDLE_EN, 0x00); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_INTERFACE_MODE, 0x00); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_BAND, 0x4); > + /* Make sure the PHY register writes are done */ > + wmb(); > + > + /* dependent on the vco frequency */ > + PLL_REG_W(dp_res->phy_base, 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 + Why atomic? As far as I can tell this is done in sleepable context because it all falls into clk_prepare() path. > + QSERDES_COM_C_READY_STATUS), > + status, > + ((status & BIT(0)) > 0), > + DP_PHY_PLL_POLL_SLEEP_US, > + DP_PHY_PLL_POLL_TIMEOUT_US)) { > + pr_err("%s: C_READY status is not high. Status=%x\n", > + __func__, status); > + pll_locked = false; Just 'return false'? > + } else { > + pll_locked = true; > + } > + > + return pll_locked; And reduce this to 'return true'? > +} > + > +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 + No need for atomic? > + DP_PHY_STATUS), > + status, > + ((status & (BIT(1))) > 0), > + DP_PHY_PLL_POLL_SLEEP_US, > + DP_PHY_PLL_POLL_TIMEOUT_US)) { > + pr_err("%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, DP_PHY_AUX_CFG2, 0x04); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x01); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x05); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x01); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x09); > + wmb(); /* Make sure the PHY register writes are done */ > + > + PLL_REG_W(dp_res->pll_base, QSERDES_COM_RESETSM_CNTRL, 0x20); > + wmb(); /* Make sure the PLL register writes are done */ > + > + if (!dp_10nm_pll_lock_status(dp_res)) { > + rc = -EINVAL; > + goto lock_err; > + } > + > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x19); > + /* Make sure the PHY register writes are done */ > + wmb(); > + /* poll for PHY ready status */ > + if (!dp_10nm_phy_rdy_status(dp_res)) { > + rc = -EINVAL; > + goto lock_err; > + } > + > + pr_debug("%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, > + TXn_HIGHZ_DRVR_EN, drvr_en); > + PLL_REG_W(dp_res->ln_tx1_base, > + TXn_TRANSCEIVER_BIAS_EN, bias_en); > + } else { > + PLL_REG_W(dp_res->ln_tx0_base, > + TXn_HIGHZ_DRVR_EN, drvr_en); > + PLL_REG_W(dp_res->ln_tx0_base, > + TXn_TRANSCEIVER_BIAS_EN, bias_en); > + } > + } else { > + PLL_REG_W(dp_res->ln_tx0_base, TXn_HIGHZ_DRVR_EN, drvr_en); > + PLL_REG_W(dp_res->ln_tx0_base, > + TXn_TRANSCEIVER_BIAS_EN, bias_en); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_HIGHZ_DRVR_EN, drvr_en); > + PLL_REG_W(dp_res->ln_tx1_base, > + TXn_TRANSCEIVER_BIAS_EN, bias_en); > + } > + > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_POL_INV, 0x0a); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_POL_INV, 0x0a); > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x18); > + udelay(2000); What are we waiting for? You need an explicit readl() if you're waiting for that DP_PHY_CFG write to post and do something. > + > + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x19); > + > + /* > + * Make sure all the register writes are completed before > + * doing any other operation > + */ > + wmb(); > + > + /* 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, TXn_TX_DRV_LVL, 0x38); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_DRV_LVL, 0x38); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_EMP_POST1_LVL, 0x20); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_EMP_POST1_LVL, 0x20); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_RES_CODE_LANE_OFFSET_TX, 0x06); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_RES_CODE_LANE_OFFSET_TX, 0x06); > + PLL_REG_W(dp_res->ln_tx0_base, TXn_RES_CODE_LANE_OFFSET_RX, 0x07); > + PLL_REG_W(dp_res->ln_tx1_base, TXn_RES_CODE_LANE_OFFSET_RX, 0x07); > + /* Make sure the PHY register writes are done */ > + wmb(); > + > +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, DP_PHY_PD_CTL, 0x2); > + /* > + * Make sure all the register writes to disable PLL are > + * completed before doing any other operation > + */ > + wmb(); > + > + 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); > + > + pr_debug("rate=%ld\n", pll->rate); > + if ((dp_res->vco_cached_rate != 0) > + && (dp_res->vco_cached_rate == pll->rate)) { > + rc = pll->clk_hw.init->ops->set_rate(hw, > + dp_res->vco_cached_rate, dp_res->vco_cached_rate); > + if (rc) { > + pr_err("index=%d vco_set_rate failed. rc=%d\n", > + rc, dp_res->index); > + goto error; Please just return instead of goto return. > + } > + } > + > + rc = dp_pll_enable_10nm(hw); > + if (rc) { > + pr_err("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) { > + pr_err("Invalid input parameter\n"); > + return; > + } > + > + if (!pll->pll_on) { > + pr_err("pll resource can't be enabled\n"); > + return; > + } > + dp_res->vco_cached_rate = pll->rate; Why are we caching rates on unprepare? I'd think we would want to save away the rate regardless of on/off state and then restore the rate when the device is powered back on. That wouldn't necessarily follow the on/off state. > + dp_pll_disable_10nm(hw); > + > + //dp_res->handoff_resources = false; Please remove commented out code. > + 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; > + > + pr_debug("DP lane CLK rate=%ld\n", rate); > + > + rc = dp_config_vco_rate_10nm(pll, rate); > + if (rc) > + pr_err("%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 { > + pr_debug("unknown divider. forcing to default\n"); > + hsclk_div = 5; > + } > + > + div = PLL_REG_R(dp_res->phy_base, 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 > + pr_err("%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; > + } > + > + pr_debug("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) Implement determine_rate op instead. It lets you clamp rates better. > +{ > + 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; > + > + pr_debug("%s: rrate=%ld\n", __func__, rrate); > + > + *parent_rate = rrate; > + return rrate; This whole function looks not very useful though. Just pass up the rate to the parent and have it do the rounding?
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index c363f24..1e0b9158 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -58,6 +58,22 @@ config DRM_MSM_DP driver. DP external 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 + default y + 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 + default y + 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 765a8d8..8d18353 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -137,4 +137,10 @@ 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 +ifeq ($(CONFIG_DRM_MSM_DP_PLL),y) +msm-y += dp/pll/dp_pll.o +msm-y += dp/pll/dp_pll_10nm.o +msm-y += dp/pll/dp_pll_10nm_util.o +endif + obj-$(CONFIG_DRM_MSM) += msm.o diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 08a52f5..e23beee 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1051,6 +1051,7 @@ static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl) { int ret = 0; + ctrl->power->set_link_clk_parent(ctrl->power); ctrl->power->set_pixel_clk_parent(ctrl->power); dp_ctrl_set_clock_rate(ctrl, "ctrl_link_clk", diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 8c98399..2bf6635 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -72,6 +72,48 @@ 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) { + pr_err("Invalid Arguments\n"); + return -EINVAL; + } + + pdev = dp_priv->pdev; + dp_parser = dp_priv->parser; + + if (!dp_parser) { + pr_err("Parser not initialized.\n"); + return -EINVAL; + } + + pll_node = of_parse_phandle(pdev->dev.of_node, "pll-node", 0); + if (!pll_node) { + dev_err(&pdev->dev, "cannot find pll device\n"); + 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) { + dev_err(&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; @@ -125,6 +167,12 @@ static int dp_display_bind(struct device *dev, struct device *master, goto end; } + rc = dp_get_pll(dp); + if (rc) { + pr_err(" DP get PLL instance failed\n"); + goto end; + } + rc = dp->aux->drm_aux_register(dp->aux); if (rc) { pr_err("DRM DP AUX register failed\n"); @@ -921,6 +969,7 @@ int __init msm_dp_register(void) { int ret; + msm_dp_pll_driver_register(); ret = platform_driver_register(&dp_display_driver); if (ret) { pr_err("driver register failed"); @@ -932,6 +981,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 ca5eae5..9cd6a6b 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -52,4 +52,7 @@ struct msm_dp { int dp_display_get_num_of_displays(void); int dp_display_get_displays(void **displays, int count); +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 b39ea02..372997e 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.h +++ b/drivers/gpu/drm/msm/dp/dp_parser.h @@ -16,6 +16,7 @@ #define _DP_PARSER_H_ #include "dpu_io_util.h" +#include "pll/dp_pll.h" #define DP_LABEL "MDSS DP DISPLAY" #define AUX_CFG_LEN 10 @@ -175,6 +176,8 @@ struct dp_parser { struct dp_pinctrl pinctrl; struct dp_io io; struct dp_display_data disp_data; + struct msm_dp_pll *pll; + struct device *pll_dev; u8 l_map[4]; struct dp_aux_cfg aux_cfg[AUX_CFG_LEN]; diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c index 1d58480..3a1679c 100644 --- a/drivers/gpu/drm/msm/dp/dp_power.c +++ b/drivers/gpu/drm/msm/dp/dp_power.c @@ -23,7 +23,9 @@ struct dp_power_private { struct dp_parser *parser; struct platform_device *pdev; struct clk *pixel_clk_rcg; - struct clk *pixel_parent; + struct clk *link_clk_src; + struct clk *pixel_provider; + struct clk *link_provider; struct dp_power dp_power; @@ -154,6 +156,16 @@ static int dp_power_clk_init(struct dp_power_private *power, bool enable) goto exit; } + if (power->parser->pll && power->parser->pll->get_provider) { + rc = power->parser->pll->get_provider(power->parser->pll, + &power->link_provider, &power->pixel_provider); + if (rc) { + pr_info("%s: can't get provider from pll, don't set parent\n", + __func__); + return 0; + } + } + if (enable) { rc = msm_dss_get_clk(dev, core->clk_config, core->num_clk); if (rc) { @@ -173,17 +185,10 @@ static int dp_power_clk_init(struct dp_power_private *power, bool enable) if (IS_ERR(power->pixel_clk_rcg)) { pr_debug("Unable to get DP pixel clk RCG\n"); power->pixel_clk_rcg = NULL; - } - - power->pixel_parent = devm_clk_get(dev, "pixel_parent"); - if (IS_ERR(power->pixel_parent)) { - pr_debug("Unable to get DP pixel RCG parent\n"); - power->pixel_parent = NULL; + rc = -ENODEV; + goto ctrl_get_error; } } else { - if (power->pixel_parent) - devm_clk_put(dev, power->pixel_parent); - if (power->pixel_clk_rcg) devm_clk_put(dev, power->pixel_clk_rcg); @@ -459,6 +464,49 @@ static void dp_power_client_deinit(struct dp_power *dp_power) } +static int dp_power_set_link_clk_parent(struct dp_power *dp_power) +{ + int rc = 0; + struct dp_power_private *power; + u32 num; + struct dss_clk *cfg; + char *name = "ctrl_link_clk"; + + if (!dp_power) { + pr_err("invalid power data\n"); + rc = -EINVAL; + goto exit; + } + + power = container_of(dp_power, struct dp_power_private, dp_power); + + num = power->parser->mp[DP_CTRL_PM].num_clk; + cfg = power->parser->mp[DP_CTRL_PM].clk_config; + + while (num && strcmp(cfg->clk_name, name)) { + num--; + cfg++; + } + + if (num && power->link_provider) { + power->link_clk_src = clk_get_parent(cfg->clk); + if (power->link_clk_src) { + clk_set_parent(power->link_clk_src, power->link_provider); + pr_debug("%s: is the parent of clk=%s\n", + __clk_get_name(power->link_provider), + __clk_get_name(power->link_clk_src)); + } else { + pr_err("couldn't get parent for clk=%s\n", name); + rc = -EINVAL; + } + } else { + pr_err("%s clock could not be set parent\n", name); + rc = -EINVAL; + } +exit: + return rc; +} + static int dp_power_set_pixel_clk_parent(struct dp_power *dp_power) { int rc = 0; @@ -472,8 +520,12 @@ static int dp_power_set_pixel_clk_parent(struct dp_power *dp_power) power = container_of(dp_power, struct dp_power_private, dp_power); - if (power->pixel_clk_rcg && power->pixel_parent) - clk_set_parent(power->pixel_clk_rcg, power->pixel_parent); + if (power->pixel_clk_rcg && power->pixel_provider) { + clk_set_parent(power->pixel_clk_rcg, power->pixel_provider); + pr_debug("%s: is the parent of clk=%s\n", + __clk_get_name(power->pixel_provider), + __clk_get_name(power->pixel_clk_rcg)); + } exit: return rc; } @@ -577,6 +629,7 @@ struct dp_power *dp_power_get(struct dp_parser *parser) dp_power->init = dp_power_init; dp_power->deinit = dp_power_deinit; dp_power->clk_enable = dp_power_clk_enable; + dp_power->set_link_clk_parent = dp_power_set_link_clk_parent; dp_power->set_pixel_clk_parent = dp_power_set_pixel_clk_parent; dp_power->power_client_init = dp_power_client_init; dp_power->power_client_deinit = dp_power_client_deinit; diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h index d1adaaf..5effd32 100644 --- a/drivers/gpu/drm/msm/dp/dp_power.h +++ b/drivers/gpu/drm/msm/dp/dp_power.h @@ -24,6 +24,7 @@ * @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 { @@ -31,6 +32,7 @@ struct dp_power { int (*deinit)(struct dp_power *power); int (*clk_enable)(struct dp_power *power, enum dp_pm_type pm_type, bool enable); + int (*set_link_clk_parent)(struct dp_power *power); int (*set_pixel_clk_parent)(struct dp_power *power); int (*power_client_init)(struct dp_power *power); void (*power_client_deinit)(struct dp_power *power); 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..a8612b5 --- /dev/null +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll.c @@ -0,0 +1,153 @@ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#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) { + pr_err("clocks are not defined\n"); + goto clk_err; + } + + mp->clk_config = devm_kzalloc(&pdev->dev, + 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)) { + dev_err(dev, "%s: failed to init DP PLL\n", __func__); + return pll; + } + + pll->type = type; + + DBG("DP:%d PLL registered", id); + + return pll; +} + +static const struct of_device_id dp_pll_dt_match[] = { +#ifdef CONFIG_DRM_MSM_DP_10NM_PLL + { .compatible = "qcom,dp-pll-10nm" }, +#endif + {} +}; + +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; + enum msm_dp_pll_type type; + + match = of_match_node(dp_pll_dt_match, dev->of_node); + if (!match) + return -ENODEV; + + if (!strcmp(match->compatible, "qcom,dp-pll-10nm")) + type = MSM_DP_PLL_10NM; + else + type = MSM_DP_PLL_MAX; + + pll = msm_dp_pll_init(pdev, type, 0); + if (IS_ERR_OR_NULL(pll)) { + dev_info(dev, + "%s: pll init failed: %ld, need separate pll clk driver\n", + __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) { + //msm_dsi_pll_destroy(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..d52a81a --- /dev/null +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll.h @@ -0,0 +1,64 @@ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#ifndef __DP_PLL_H +#define __DP_PLL_H + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/platform_device.h> + +#include "dpu_io_util.h" +#include "msm_drv.h" + +#define PLL_REG_W(base, offset, data) \ + writel_relaxed((data), (base) + (offset)) +#define PLL_REG_R(base, offset) readl_relaxed((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; + 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) + +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 +struct msm_dp_pll *msm_dp_pll_10nm_init(struct platform_device *pdev, int id) +{ + 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..a180413 --- /dev/null +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c @@ -0,0 +1,401 @@ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +/* + * Display Port PLL driver block diagram for branch clocks + * + * +------------------------------+ + * | DP_VCO_CLK | + * | | + * | +-------------------+ | + * | | (DP PLL/VCO) | | + * | +---------+---------+ | + * | v | + * | +----------+-----------+ | + * | | hsclk_divsel_clk_src | | + * | +----------+-----------+ | + * +------------------------------+ + * | + * +------------<---------v------------>----------+ + * | | + * +-----v------------+ | + * | dp_link_clk_src | | + * | divsel_ten | | + * +---------+--------+ | + * | | + * | | + * 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 + * | + * +----------+---------+ + * | vco_divided_clk | + * | _src_mux | + * +---------+----------+ + * | + * v + * Input to DISPCC block + * for DP pixel clock + * + */ + +#include <linux/kernel.h> +#include <linux/err.h> +#include <linux/delay.h> +#include <linux/regmap.h> +#include <linux/clk.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; + +/* Op structures */ +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, 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; + + PLL_REG_W(dp_res->phy_base, + DP_PHY_VCO_DIV, auxclk_div); + /* Make sure the PHY registers writes are done */ + wmb(); + pr_debug("%s: mux=%d auxclk_div=%x\n", __func__, val, auxclk_div); + + 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; + + pr_err("clk_hw->init->name = %s\n", hw->init->name); + auxclk_div = PLL_REG_R(dp_res->phy_base, 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; + + pr_debug("%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) +{ + int ret = 0; + + ret = __clk_mux_determine_rate_closest(hw, req); + if (ret) + return ret; + + /* Set the new parent of mux if there is a new valid parent */ + if (hw->clk && req->best_parent_hw->clk) + clk_set_parent(hw->clk, req->best_parent_hw->clk); + + return 0; +} + +static unsigned long mux_recalc_rate(struct clk_hw *hw, + 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); + 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)); + 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); +} + +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; + + 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" }, + .num_parents = 3, + .name = "dp_vco_divided_clk_src_mux", + .flags = 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" }, + .num_parents = 1, + .name = vco_name, + .flags = CLK_IGNORE_UNUSED, + .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; + + DBG("DP->id = %d", pll_10nm->id); + + hw_data = devm_kzalloc(dev, sizeof(*hw_data) + + 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_link_clk_divsel_ten"); + snprintf(parent, 32, "dp_vco_clk"); + 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) { + dev_err(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); + + DBG("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)) { + dev_err(&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)) { + dev_err(&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)) { + dev_err(&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)) { + dev_err(&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) { + pr_err("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) { + pr_err("Unable to parse dt clocks ret=%d\n", ret); + return ERR_PTR(ret); + } + + ret = dp_pll_10nm_register(dp_10nm_pll); + if (ret) { + dev_err(&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; + 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..f966beb --- /dev/null +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h @@ -0,0 +1,94 @@ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#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) + +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..9eb6881 --- /dev/null +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c @@ -0,0 +1,531 @@ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#define pr_fmt(fmt) "%s: " fmt, __func__ + +#include <linux/kernel.h> +#include <linux/err.h> +#include <linux/iopoll.h> +#include <linux/delay.h> + +#include "dp_pll.h" +#include "dp_pll_10nm.h" +#include "dp_extcon.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, DP_PHY_SPARE0); + dp_res->lane_cnt = spare_value & 0x0F; + dp_res->orientation = (spare_value & 0xF0) >> 4; + + pr_debug("%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: + pr_debug("%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: + pr_debug("%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: + pr_debug("%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: + pr_debug("%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) { + pr_err("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, DP_PHY_PD_CTL, 0x6d); + else + PLL_REG_W(dp_res->phy_base, DP_PHY_PD_CTL, 0x75); + } else { + PLL_REG_W(dp_res->phy_base, DP_PHY_PD_CTL, 0x7d); + } + + /* Make sure the PHY register writes are done */ + wmb(); + + 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); + /* Make sure the PLL register writes are done */ + wmb(); + + 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); + /* Make sure the PHY register writes are done */ + wmb(); + + if (dp_res->orientation == ORIENTATION_CC2) + PLL_REG_W(dp_res->phy_base, DP_PHY_MODE, 0x4c); + else + PLL_REG_W(dp_res->phy_base, DP_PHY_MODE, 0x5c); + /* Make sure the PLL register writes are done */ + wmb(); + + /* TX Lane configuration */ + PLL_REG_W(dp_res->phy_base, + DP_PHY_TX0_TX1_LANE_CTL, 0x05); + PLL_REG_W(dp_res->phy_base, + DP_PHY_TX2_TX3_LANE_CTL, 0x05); + + /* TX-0 register configuration */ + PLL_REG_W(dp_res->ln_tx0_base, TXn_TRANSCEIVER_BIAS_EN, 0x1a); + PLL_REG_W(dp_res->ln_tx0_base, TXn_VMODE_CTRL1, 0x40); + PLL_REG_W(dp_res->ln_tx0_base, TXn_PRE_STALL_LDO_BOOST_EN, 0x30); + PLL_REG_W(dp_res->ln_tx0_base, TXn_INTERFACE_SELECT, 0x3d); + PLL_REG_W(dp_res->ln_tx0_base, TXn_CLKBUF_ENABLE, 0x0f); + PLL_REG_W(dp_res->ln_tx0_base, TXn_RESET_TSYNC_EN, 0x03); + PLL_REG_W(dp_res->ln_tx0_base, TXn_TRAN_DRVR_EMP_EN, 0x03); + PLL_REG_W(dp_res->ln_tx0_base, + TXn_PARRATE_REC_DETECT_IDLE_EN, 0x00); + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_INTERFACE_MODE, 0x00); + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_BAND, 0x4); + + /* TX-1 register configuration */ + PLL_REG_W(dp_res->ln_tx1_base, TXn_TRANSCEIVER_BIAS_EN, 0x1a); + PLL_REG_W(dp_res->ln_tx1_base, TXn_VMODE_CTRL1, 0x40); + PLL_REG_W(dp_res->ln_tx1_base, TXn_PRE_STALL_LDO_BOOST_EN, 0x30); + PLL_REG_W(dp_res->ln_tx1_base, TXn_INTERFACE_SELECT, 0x3d); + PLL_REG_W(dp_res->ln_tx1_base, TXn_CLKBUF_ENABLE, 0x0f); + PLL_REG_W(dp_res->ln_tx1_base, TXn_RESET_TSYNC_EN, 0x03); + PLL_REG_W(dp_res->ln_tx1_base, TXn_TRAN_DRVR_EMP_EN, 0x03); + PLL_REG_W(dp_res->ln_tx1_base, + TXn_PARRATE_REC_DETECT_IDLE_EN, 0x00); + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_INTERFACE_MODE, 0x00); + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_BAND, 0x4); + /* Make sure the PHY register writes are done */ + wmb(); + + /* dependent on the vco frequency */ + PLL_REG_W(dp_res->phy_base, 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)) { + pr_err("%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 + + DP_PHY_STATUS), + status, + ((status & (BIT(1))) > 0), + DP_PHY_PLL_POLL_SLEEP_US, + DP_PHY_PLL_POLL_TIMEOUT_US)) { + pr_err("%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, DP_PHY_AUX_CFG2, 0x04); + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x01); + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x05); + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x01); + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x09); + wmb(); /* Make sure the PHY register writes are done */ + + PLL_REG_W(dp_res->pll_base, QSERDES_COM_RESETSM_CNTRL, 0x20); + wmb(); /* Make sure the PLL register writes are done */ + + if (!dp_10nm_pll_lock_status(dp_res)) { + rc = -EINVAL; + goto lock_err; + } + + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x19); + /* Make sure the PHY register writes are done */ + wmb(); + /* poll for PHY ready status */ + if (!dp_10nm_phy_rdy_status(dp_res)) { + rc = -EINVAL; + goto lock_err; + } + + pr_debug("%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, + TXn_HIGHZ_DRVR_EN, drvr_en); + PLL_REG_W(dp_res->ln_tx1_base, + TXn_TRANSCEIVER_BIAS_EN, bias_en); + } else { + PLL_REG_W(dp_res->ln_tx0_base, + TXn_HIGHZ_DRVR_EN, drvr_en); + PLL_REG_W(dp_res->ln_tx0_base, + TXn_TRANSCEIVER_BIAS_EN, bias_en); + } + } else { + PLL_REG_W(dp_res->ln_tx0_base, TXn_HIGHZ_DRVR_EN, drvr_en); + PLL_REG_W(dp_res->ln_tx0_base, + TXn_TRANSCEIVER_BIAS_EN, bias_en); + PLL_REG_W(dp_res->ln_tx1_base, TXn_HIGHZ_DRVR_EN, drvr_en); + PLL_REG_W(dp_res->ln_tx1_base, + TXn_TRANSCEIVER_BIAS_EN, bias_en); + } + + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_POL_INV, 0x0a); + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_POL_INV, 0x0a); + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x18); + udelay(2000); + + PLL_REG_W(dp_res->phy_base, DP_PHY_CFG, 0x19); + + /* + * Make sure all the register writes are completed before + * doing any other operation + */ + wmb(); + + /* 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, TXn_TX_DRV_LVL, 0x38); + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_DRV_LVL, 0x38); + PLL_REG_W(dp_res->ln_tx0_base, TXn_TX_EMP_POST1_LVL, 0x20); + PLL_REG_W(dp_res->ln_tx1_base, TXn_TX_EMP_POST1_LVL, 0x20); + PLL_REG_W(dp_res->ln_tx0_base, TXn_RES_CODE_LANE_OFFSET_TX, 0x06); + PLL_REG_W(dp_res->ln_tx1_base, TXn_RES_CODE_LANE_OFFSET_TX, 0x06); + PLL_REG_W(dp_res->ln_tx0_base, TXn_RES_CODE_LANE_OFFSET_RX, 0x07); + PLL_REG_W(dp_res->ln_tx1_base, TXn_RES_CODE_LANE_OFFSET_RX, 0x07); + /* Make sure the PHY register writes are done */ + wmb(); + +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, DP_PHY_PD_CTL, 0x2); + /* + * Make sure all the register writes to disable PLL are + * completed before doing any other operation + */ + wmb(); + + 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); + + pr_debug("rate=%ld\n", pll->rate); + if ((dp_res->vco_cached_rate != 0) + && (dp_res->vco_cached_rate == pll->rate)) { + rc = pll->clk_hw.init->ops->set_rate(hw, + dp_res->vco_cached_rate, dp_res->vco_cached_rate); + if (rc) { + pr_err("index=%d vco_set_rate failed. rc=%d\n", + rc, dp_res->index); + goto error; + } + } + + rc = dp_pll_enable_10nm(hw); + if (rc) { + pr_err("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) { + pr_err("Invalid input parameter\n"); + return; + } + + if (!pll->pll_on) { + pr_err("pll resource can't be enabled\n"); + return; + } + dp_res->vco_cached_rate = pll->rate; + dp_pll_disable_10nm(hw); + + //dp_res->handoff_resources = false; + 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; + + pr_debug("DP lane CLK rate=%ld\n", rate); + + rc = dp_config_vco_rate_10nm(pll, rate); + if (rc) + pr_err("%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 { + pr_debug("unknown divider. forcing to default\n"); + hsclk_div = 5; + } + + div = PLL_REG_R(dp_res->phy_base, 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 + pr_err("%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; + } + + pr_debug("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; + + pr_debug("%s: rrate=%ld\n", __func__, rrate); + + *parent_rate = rrate; + return rrate; +} +
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. Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org> --- drivers/gpu/drm/msm/Kconfig | 16 + drivers/gpu/drm/msm/Makefile | 6 + drivers/gpu/drm/msm/dp/dp_ctrl.c | 1 + drivers/gpu/drm/msm/dp/dp_display.c | 50 +++ drivers/gpu/drm/msm/dp/dp_display.h | 3 + drivers/gpu/drm/msm/dp/dp_parser.h | 3 + drivers/gpu/drm/msm/dp/dp_power.c | 77 +++- drivers/gpu/drm/msm/dp/dp_power.h | 2 + drivers/gpu/drm/msm/dp/pll/dp_pll.c | 153 ++++++++ drivers/gpu/drm/msm/dp/pll/dp_pll.h | 64 ++++ drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c | 401 +++++++++++++++++++ drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h | 94 +++++ drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c | 531 ++++++++++++++++++++++++++ 13 files changed, 1389 insertions(+), 12 deletions(-) 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