Hi Steev,
On Fri, 17 Dec 2021 at 17:16, Steev Klimaszewski <steev@xxxxxxxx> wrote:
> >>> + .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
