| Message ID | 20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-v5-5-56eb7a4d5b8e@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/meson: add support for MIPI DSI Display | expand |
On Tue 30 May 2023 at 09:38, Neil Armstrong <neil.armstrong@linaro.org> 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@linaro.org> > --- > 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);
On 30/05/2023 10:14, Jerome Brunet wrote: > > On Tue 30 May 2023 at 09:38, Neil Armstrong <neil.armstrong@linaro.org> 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. Sure, will see how to switch to that, seem Martin did than on Meson8. Neil > >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> 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); >
Hi Neil, On Tue, May 30, 2023 at 5:57 PM Neil Armstrong <neil.armstrong@linaro.org> wrote: [...] > >> 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. > > Sure, will see how to switch to that, seem Martin did than on Meson8. You can start here: [0] It may not be the nicest logic but so far it works (for me). Please note that I don't mix between CCF and direct register IO clock handling: For the old SoCs I'm relying only on CCF to manage the clocks. Best regards, Martin [0] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.3-20230410/drivers/gpu/drm/meson/meson_vclk.c#L1177-L1179
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);
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. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/clk/meson/g12a.c | 131 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 120 insertions(+), 11 deletions(-)