On 2020-04-23 17:26, Stephen Boyd wrote:
Quoting Tanmay Shah (2020-03-31 17:30:30)
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..aa845d0 --- /dev/null +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c @@ -0,0 +1,401 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- Copyright (c) 2016-2020, The Linux Foundation. All rights
reserved.
- */
+/*
- Display Port PLL driver block diagram for branch clocks
+------------------------------+
| DP_VCO_CLK |
| |
| +-------------------+ |
| | (DP PLL/VCO) | |
| +---------+---------+ |
| v |
| +----------+-----------+ |
| | hsclk_divsel_clk_src | |
| +----------+-----------+ |
+------------------------------+
|
+---------<---------v------------>----------+
| |
- +--------v---------+ |
- | dp_phy_pll | |
- | link_clk | |
- +--------+---------+ |
| |
| |
v v
- Input to DISPCC block |
- for link clk, crypto clk |
- and interface clock |
|
|
+--------<------------+-----------------+---<---+
| | |
- +----v---------+ +--------v-----+ +--------v------+
- | vco_divided | | vco_divided | | vco_divided |
- | _clk_src | | _clk_src | | _clk_src |
- | | | | | |
- |divsel_six | | divsel_two | | divsel_four |
- +-------+------+ +-----+--------+ +--------+------+
| | |
v---->----------v-------------<------v
|
+----------+---------+
| dp_phy_pll_vco |
| div_clk |
+---------+----------+
|
v
Input to DISPCC block
for DP pixel clockI suspect this shouldn't be a complicated clk provider at all. Take a look at commit 42d068472ddf ("phy: Add DisplayPort configuration options") for how the phy should manage the link rate, etc. If the dispcc pixel clock needs to know what rate is coming in, then a single clk_hw can be implemented here that tells the consumer (i.e. dispcc) the rate that it will see at the output of this node. Otherwise, modeling the clk tree inside this PLL block like this is super overly complicated and wasteful. Don't do it.
This will be taken care at later point of time. For now we have removed PLL bindings and made PLL as module of DP driver.
- */
+#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/regmap.h>
+#include "dp_pll_10nm.h"
+#define NUM_PROVIDED_CLKS 2
+#define DP_LINK_CLK_SRC 0 +#define DP_PIXEL_CLK_SRC 1
[...]
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..fff2e8d --- /dev/null +++ b/drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c @@ -0,0 +1,524 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- Copyright (c) 2016-2020, The Linux Foundation. All rights
reserved.
- */
+#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <linux/delay.h> +#include <linux/err.h> +#include <linux/iopoll.h> +#include <linux/kernel.h>
+#include "dp_hpd.h" +#include "dp_pll.h" +#include "dp_pll_10nm.h"
[...]
+static int dp_config_vco_rate_10nm(struct msm_dp_pll *pll,
unsigned long rate)+{
u32 res = 0;struct dp_pll_10nm *dp_res = to_dp_pll_10nm(pll);res = dp_vco_pll_init_db_10nm(pll, rate);if (res) {DRM_ERROR("VCO Init DB failed\n");return res;}if (dp_res->lane_cnt != 4) {if (dp_res->orientation == ORIENTATION_CC2)PLL_REG_W(dp_res->phy_base, REG_DP_PHY_PD_CTL,0x6d);
elsePLL_REG_W(dp_res->phy_base, REG_DP_PHY_PD_CTL,0x75);
} else {PLL_REG_W(dp_res->phy_base, REG_DP_PHY_PD_CTL, 0x7d);}For example, this part here can be done through the phy configuration ops. A lane count check in the set rate clk op is quite odd!
Same here.
+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 = to_msm_dp_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;elserrate = pll->max_rate;This is basically link rate setting through the clk framework. Calling clk_set_rate() on the pixel clk is complicated and opaque. I'd expect to see the DP controller driver set the link rate on the phy with phy_configure(), and then that can change the rate that is seen downstream at the pixel clk. Does the pixel clk need to do anything with the rate? Probably not? I suspect it can just enable the pixel clk when it needs to and disable it when it doesn't need it.
Same here.
DRM_DEBUG_DP("%s: rrate=%ld\n", __func__, rrate);*parent_rate = rrate;
Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno