Re: [PATCH v2 2/2] clk: qcom : dispcc: Add support for display port clocks

[Stephen Boyd]

Quoting Taniya Das (2019-05-14 21:20:39)
> @@ -128,6 +144,82 @@ enum {
>         },
>  };
> 
> +static const struct freq_tbl ftbl_disp_cc_mdss_dp_aux_clk_src[] = {
> +       F(19200000, P_BI_TCXO, 1, 0, 0),
> +       { }
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_dp_aux_clk_src = {
> +       .cmd_rcgr = 0x219c,
> +       .mnd_width = 0,
> +       .hid_width = 5,
> +       .parent_map = disp_cc_parent_map_2,
> +       .freq_tbl = ftbl_disp_cc_mdss_dp_aux_clk_src,
> +       .clkr.hw.init = &(struct clk_init_data){
> +               .name = "disp_cc_mdss_dp_aux_clk_src",
> +               .parent_names = disp_cc_parent_names_2,
> +               .num_parents = 2,
> +               .flags = CLK_SET_RATE_PARENT,
> +               .ops = &clk_rcg2_ops,
> +       },
> +};
> +
> +static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = {
> +       .cmd_rcgr = 0x2154,
> +       .mnd_width = 0,
> +       .hid_width = 5,
> +       .parent_map = disp_cc_parent_map_1,
> +       .clkr.hw.init = &(struct clk_init_data){
> +               .name = "disp_cc_mdss_dp_crypto_clk_src",
> +               .parent_names = disp_cc_parent_names_1,
> +               .num_parents = 4,
> +               .flags = CLK_GET_RATE_NOCACHE,

Why do we need this flag on various clks here? I'd prefer this is
removed. If it can't be removed, we need to describe in a code comment
why this must be set.

If it's some sort of problem where the upstream PLL goes into bypass
across a reset, then we probably need to change the display code to
restore that rate across a reset by calling clk_set_rate() on the PLL
directly. And we might need to think about how to inform the framework
that this has happened, so that downstream clks can be notified of the
change in frequency.