[Freedreno] [DPU PATCH v5 4/5] drm/msm/dp: add support for DP PLL driver
tanmay at codeaurora.org
tanmay at codeaurora.org
Tue Jun 9 00:16:04 UTC 2020
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 clock
>
> I 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);
>> + else
>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_PD_CTL,
> 0x75);
>> + } else {
>> + PLL_REG_W(dp_res->phy_base, REG_DP_PHY_PD_CTL, 0x7d);
>> + }
>
> 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;
>> + else
>> + rrate = 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
More information about the dri-devel
mailing list