Hi Steev,

On Fri, 17 Dec 2021 at 17:16, Steev Klimaszewski [off-list ref] wrote:

quoted

quoted

quoted

+       .cmd_rcgr = 0x205c,
+       .mnd_width = 8,
+       .hid_width = 5,
+       .parent_map = disp_cc_parent_map_4,
+       .clkr.hw.init = &(struct clk_init_data){
+               .name = "disp_cc_mdss_pclk0_clk_src",
+               .parent_data = disp_cc_parent_data_4,
+               .num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
+               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,

These last two flags are needed for what?

NOCACHE is probably useless with mainline.

I've added OPS_PARENT_ENABLE because AFAIU changing clock rate can
lead to parent switch, and parent switch can only be done if parent
clocks are enabled for rcg2 clocks. Otherwise the update fails and we
get the following:
     disp_cc_mdss_pclk0_clk_src: rcg didn't update its configuration.
     WARNING: CPU: 2 PID: 77 at drivers/clk/qcom/clk-rcg2.c:122
update_config+0xe0/0xf0

I'm a bit surprised other similar dispcc drivers don't use the same
flags though.

So, we do actually see this on other devices - in particular, on the
Lenovo Yoga C630, and people have been trying to track down the initial
cause for a while.  I tried here adding CLK_OPS_PARENT_ENABLE to both
disp_cc_mdss_mdp_clk and disp_cc_mdss_pclk0_clk in dispcc-sdm845.c as
well as for video_cc_venus_clk_src in videocc-sdm845.c and while it does
seem to cause the messages to go away for disp_cc_mdss_mdp_clk and
disp_cc_mdss_pclk0_clk, the one for venus seems to continue to show up here.

video_cc_venus_clk_src: rcg didn't update its configuration. WARNING:
CPU: 1 PID: 404 at drivers/clk/qcom/clk-rcg2.c:122 update_config+0xd0/0xe0

I'm not sure if this is due to venus being a module and not built-in.

I think what you should do is dumping clk configuration before update
(basically for clk_rcg2_set_parent, __clk_rcg2_configure), especially
which parent source is configured for this clock. As Dmitry said, For
set_rate and set_parent to succeed, both old and new parents must be
running at the time of the operation. But in videocc-sdm845.cc I see
that some parents for video_cc_venus_clk_src are commented, would it
be possible that video_cc_venus_clk_src is configured with one of
these unhandled parents at boot time, making parent switch impossible?

Regards,
Loic