Re: [PATCH v5 05/17] clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF
From: Jerome Brunet
Date: Tue May 30 2023 - 04:25:36 EST
On Tue 30 May 2023 at 09:38, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:
> In order to setup the DSI clock, let's make the unused VCLK2 clock path
> configuration via CCF.
>
> The nocache option is removed from following clocks:
> - vclk2_sel
> - vclk2_input
> - vclk2_div
> - vclk2
> - vclk_div1
> - vclk2_div2_en
> - vclk2_div4_en
> - vclk2_div6_en
> - vclk2_div12_en
> - vclk2_div2
> - vclk2_div4
> - vclk2_div6
> - vclk2_div12
> - cts_encl_sel
>
> The missing vclk2 reset sequence is handled via new clkc notifiers
> in order to reset the vclk2 after each rate change as done by Amlogic
> in the vendor implementation.
>
> In order to set a rate on cts_encl via the vclk2 clock path,
> the NO_REPARENT flag is set on cts_encl_sel & vclk2_sel in order
> to keep CCF from selection a parent.
> The parents of cts_encl_sel & vclk2_sel are expected to be defined
> in DT.
>
> The following clock scheme is to be used for DSI:
>
> xtal
> \_ gp0_pll_dco
> \_ gp0_pll
> |- vclk2_sel
> | \_ vclk2_input
> | \_ vclk2_div
> | \_ vclk2
> | \_ vclk2_div1
> | \_ cts_encl_sel
> | \_ cts_encl -> to VPU LCD Encoder
> |- mipi_dsi_pxclk_sel
> \_ mipi_dsi_pxclk_div
> \_ mipi_dsi_pxclk -> to DSI controller
>
> The mipi_dsi_pxclk_div is set as RO in order to use the same GP0
> for mipi_dsi_pxclk and vclk2_input.
I don't think notifiers is the appropriate approach here.
Whenever there is clock change the motifiers would trigger an off/on of
the clock, regardless of the clock usage or state.
If you have several consummers on this vclk2, this would
cause glitches and maybe this is not desirable.
I think it would be better to handle the enable and reset with a
specific gate driver, in prepare() or enable(), and the give the clock
CLK_SET_RATE_GATE flag.
This would require the clock to be properly turn off before changing the
rate.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> ---
> drivers/clk/meson/g12a.c | 131 +++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 120 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
> index 461ebd79497c..e4053f4957d5 100644
> --- a/drivers/clk/meson/g12a.c
> +++ b/drivers/clk/meson/g12a.c
> @@ -3163,7 +3163,7 @@ static struct clk_regmap g12a_vclk2_sel = {
> .ops = &clk_regmap_mux_ops,
> .parent_hws = g12a_vclk_parent_hws,
> .num_parents = ARRAY_SIZE(g12a_vclk_parent_hws),
> - .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
> + .flags = CLK_SET_RATE_NO_REPARENT,
> },
> };
>
> @@ -3191,7 +3191,6 @@ static struct clk_regmap g12a_vclk2_input = {
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) { &g12a_vclk2_sel.hw },
> .num_parents = 1,
> - .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> },
> };
>
> @@ -3212,6 +3211,40 @@ static struct clk_regmap g12a_vclk_div = {
> },
> };
>
> +struct g12a_vclk_div_notifier {
> + struct clk_regmap *clk;
> + unsigned int offset;
> + u8 en_bit_idx;
> + u8 reset_bit_idx;
> + struct notifier_block nb;
> +};
> +
> +static int g12a_vclk_div_notifier_cb(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct g12a_vclk_div_notifier *nb_data =
> + container_of(nb, struct g12a_vclk_div_notifier, nb);
> +
> + switch (event) {
> + case PRE_RATE_CHANGE:
> + /* disable and reset vclk2 divider */
> + regmap_update_bits(nb_data->clk->map, nb_data->offset,
> + BIT(nb_data->en_bit_idx) |
> + BIT(nb_data->reset_bit_idx),
> + BIT(nb_data->reset_bit_idx));
> + return NOTIFY_OK;
> + case POST_RATE_CHANGE:
> + /* enabled and release reset */
> + regmap_update_bits(nb_data->clk->map, nb_data->offset,
> + BIT(nb_data->en_bit_idx) |
> + BIT(nb_data->reset_bit_idx),
> + BIT(nb_data->en_bit_idx));
> + return NOTIFY_OK;
> + default:
> + return NOTIFY_DONE;
> + };
> +};
> +
> static struct clk_regmap g12a_vclk2_div = {
> .data = &(struct clk_regmap_div_data){
> .offset = HHI_VIID_CLK_DIV,
> @@ -3225,10 +3258,18 @@ static struct clk_regmap g12a_vclk2_div = {
> &g12a_vclk2_input.hw
> },
> .num_parents = 1,
> - .flags = CLK_GET_RATE_NOCACHE,
> + .flags = CLK_DIVIDER_ROUND_CLOSEST,
> },
> };
>
> +static struct g12a_vclk_div_notifier g12a_vclk2_div_data = {
> + .clk = &g12a_vclk2_div,
> + .offset = HHI_VIID_CLK_DIV,
> + .en_bit_idx = 16,
> + .reset_bit_idx = 17,
> + .nb.notifier_call = g12a_vclk_div_notifier_cb,
> +};
> +
> static struct clk_regmap g12a_vclk = {
> .data = &(struct clk_regmap_gate_data){
> .offset = HHI_VID_CLK_CNTL,
> @@ -3243,6 +3284,33 @@ static struct clk_regmap g12a_vclk = {
> },
> };
>
> +struct g12a_vclk_reset_notifier {
> + struct clk_regmap *clk;
> + unsigned int offset;
> + u8 bit_idx;
> + struct notifier_block nb;
> +};
> +
> +static int g12a_vclk_notifier_cb(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct g12a_vclk_reset_notifier *nb_data =
> + container_of(nb, struct g12a_vclk_reset_notifier, nb);
> +
> + switch (event) {
> + case POST_RATE_CHANGE:
> + /* reset vclk2 */
> + regmap_update_bits(nb_data->clk->map, nb_data->offset,
> + BIT(nb_data->bit_idx), BIT(nb_data->bit_idx));
> + regmap_update_bits(nb_data->clk->map, nb_data->offset,
> + BIT(nb_data->bit_idx), 0);
> +
> + return NOTIFY_OK;
> + default:
> + return NOTIFY_DONE;
> + };
> +}
> +
> static struct clk_regmap g12a_vclk2 = {
> .data = &(struct clk_regmap_gate_data){
> .offset = HHI_VIID_CLK_CNTL,
> @@ -3253,10 +3321,17 @@ static struct clk_regmap g12a_vclk2 = {
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) { &g12a_vclk2_div.hw },
> .num_parents = 1,
> - .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> +static struct g12a_vclk_reset_notifier g12a_vclk2_data = {
> + .clk = &g12a_vclk2,
> + .offset = HHI_VIID_CLK_CNTL,
> + .bit_idx = 15,
> + .nb.notifier_call = g12a_vclk_notifier_cb,
> +};
> +
> static struct clk_regmap g12a_vclk_div1 = {
> .data = &(struct clk_regmap_gate_data){
> .offset = HHI_VID_CLK_CNTL,
> @@ -3337,7 +3412,7 @@ static struct clk_regmap g12a_vclk2_div1 = {
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
> .num_parents = 1,
> - .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3351,7 +3426,7 @@ static struct clk_regmap g12a_vclk2_div2_en = {
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
> .num_parents = 1,
> - .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3365,7 +3440,7 @@ static struct clk_regmap g12a_vclk2_div4_en = {
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
> .num_parents = 1,
> - .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3379,7 +3454,7 @@ static struct clk_regmap g12a_vclk2_div6_en = {
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
> .num_parents = 1,
> - .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3393,7 +3468,7 @@ static struct clk_regmap g12a_vclk2_div12_en = {
> .ops = &clk_regmap_gate_ops,
> .parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
> .num_parents = 1,
> - .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3459,6 +3534,7 @@ static struct clk_fixed_factor g12a_vclk2_div2 = {
> &g12a_vclk2_div2_en.hw
> },
> .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3472,6 +3548,7 @@ static struct clk_fixed_factor g12a_vclk2_div4 = {
> &g12a_vclk2_div4_en.hw
> },
> .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3485,6 +3562,7 @@ static struct clk_fixed_factor g12a_vclk2_div6 = {
> &g12a_vclk2_div6_en.hw
> },
> .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3498,6 +3576,7 @@ static struct clk_fixed_factor g12a_vclk2_div12 = {
> &g12a_vclk2_div12_en.hw
> },
> .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3559,7 +3638,7 @@ static struct clk_regmap g12a_cts_encl_sel = {
> .ops = &clk_regmap_mux_ops,
> .parent_hws = g12a_cts_parent_hws,
> .num_parents = ARRAY_SIZE(g12a_cts_parent_hws),
> - .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
> + .flags = CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
> },
> };
>
> @@ -3727,7 +3806,7 @@ static struct clk_regmap g12a_mipi_dsi_pxclk_div = {
> },
> .hw.init = &(struct clk_init_data){
> .name = "mipi_dsi_pxclk_div",
> - .ops = &clk_regmap_divider_ops,
> + .ops = &clk_regmap_divider_ro_ops,
> .parent_hws = (const struct clk_hw *[]) {
> &g12a_mipi_dsi_pxclk_sel.hw
> },
> @@ -5421,6 +5500,32 @@ static int meson_g12a_dvfs_setup(struct platform_device *pdev)
> return 0;
> }
>
> +static int meson_g12a_vclk_setup(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct clk *notifier_clk;
> + int ret;
> +
> + /* Setup clock notifier for vclk2 */
> + notifier_clk = devm_clk_hw_get_clk(dev, &g12a_vclk2.hw, DVFS_CON_ID);
> + ret = devm_clk_notifier_register(dev, notifier_clk, &g12a_vclk2_data.nb);
> + if (ret) {
> + dev_err(dev, "failed to register the vlkc2 notifier\n");
> + return ret;
> + }
> +
> + /* Setup clock notifier for vclk2_div */
> + notifier_clk = devm_clk_hw_get_clk(dev, &g12a_vclk2_div.hw, DVFS_CON_ID);
> + ret = devm_clk_notifier_register(dev, notifier_clk,
> + &g12a_vclk2_div_data.nb);
> + if (ret) {
> + dev_err(dev, "failed to register the vclk2_div notifier\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> struct meson_g12a_data {
> const struct meson_eeclkc_data eeclkc_data;
> int (*dvfs_setup)(struct platform_device *pdev);
> @@ -5443,6 +5548,10 @@ static int meson_g12a_probe(struct platform_device *pdev)
> g12a_data = container_of(eeclkc_data, struct meson_g12a_data,
> eeclkc_data);
>
> + ret = meson_g12a_vclk_setup(pdev);
> + if (ret)
> + return ret;
> +
> if (g12a_data->dvfs_setup)
> return g12a_data->dvfs_setup(pdev);