RE: [PATCH] clk: meson: g12a: Fix kernel warnings when no display attached

[<linux@martijnvandeventer.nl>]

Hi Jerome,

Thank you for reviewing, and apologies for my late response due to a holiday.

> On Thu 13 Feb 2025 at 23:17, Martijn van Deventer
> <linux@martijnvandeventer.nl> wrote:
> 
> > When booting SM1 or G12A boards without a dislay attached to HDMI,
> > the kernel shows the following warning:
> >
> > [CRTC:46:meson_crtc] vblank wait timed out
> > WARNING: CPU: 2 PID: 265 at drivers/gpu/drm/drm_atomic_helper.c:1682
> drm_atomic_helper_wait_for_vblanks.part.0+0x240/0x264
> > CPU: 2 UID: 0 PID: 265 Comm: setfont Tainted: G         C
> > Tainted: [C]=CRAP
> > pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > pc : drm_atomic_helper_wait_for_vblanks.part.0+0x240/0x264
> > lr : drm_atomic_helper_wait_for_vblanks.part.0+0x240/0x264
> > Call trace:
> >  drm_atomic_helper_wait_for_vblanks.part.0+0x240/0x264
> >  drm_atomic_helper_commit_tail_rpm+0x84/0xa0
> >  commit_tail+0xa4/0x18c
> >  drm_atomic_helper_commit+0x164/0x178
> >  drm_atomic_commit+0xb4/0xec
> >  drm_client_modeset_commit_atomic+0x210/0x270
> >  drm_client_modeset_commit_locked+0x5c/0x188
> >  drm_fb_helper_pan_display+0xb8/0x1d4
> >  fb_pan_display+0x7c/0x120
> >  bit_update_start+0x20/0x48
> >  fbcon_switch+0x418/0x54c
> >  el0t_64_sync+0x194/0x198
> >
> > This happens when the kernel disables the unused clocks.
> > Sometimes this causes the boot to hang.
> >
> > By (re)adding the flag CLK_IGNORE_UNUSED to the VCLK2 clocks, these
> > clocks will not be disabled.
> >
> > This partially reverts commit b70cb1a21a54 ("clk: meson: g12a:
> > make VCLK2 and ENCL clock path configurable by CCF").
> 
> It looks like DRM needs those clock enabled regardless of connection
> status on HDMI. Even with this change applied, you would get the same
> problem again if the bootloader does not take of turning the clock on,
> which is not a given.
> 
> CLK_IGNORE_UNUSED gives not guarantee a clock will be enabled or stay
> enabled at any point.
> 
> A proper fix to this issue should be done in DRM, IMO.

I know and I totally agree. Unfortunately, I don't have access to any vendor 
documentation, nor do I have any real knowledge about the DRM/HDMI 
subsystem to fix that.

And I guess if it were as easy as adding a clock to the DT and calling 
clk_prepare_enable on it in the probe function, Neil would have done that 
already.

So, all I can do, for now, is revert to the previous situation when it did  work
for (probably) most boards. 

> >
> > Fixes: b70cb1a21a54 ("clk: meson: g12a: make VCLK2 and ENCL clock path
> configurable by CCF").
> > Signed-off-by: Martijn van Deventer <linux@martijnvandeventer.nl>
> > ---
> >  drivers/clk/meson/g12a.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
> > index cfffd434e998..1651898658f5 100644
> > --- a/drivers/clk/meson/g12a.c
> > +++ b/drivers/clk/meson/g12a.c
> > @@ -3234,7 +3234,7 @@ static struct clk_regmap g12a_vclk2_div = {
> >  			&g12a_vclk2_input.hw
> >  		},
> >  		.num_parents = 1,
> > -		.flags = CLK_SET_RATE_GATE,
> > +		.flags = CLK_SET_RATE_GATE | CLK_IGNORE_UNUSED,
> >  	},
> >  };
> >
> > @@ -3270,7 +3270,7 @@ static struct clk_regmap g12a_vclk2 = {
> >  		.ops = &meson_vclk_gate_ops,
> >  		.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2_div.hw
> },
> >  		.num_parents = 1,
> > -		.flags = CLK_SET_RATE_PARENT,
> > +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> >  	},
> >  };
> >
> > @@ -3354,7 +3354,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,
> > +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> >  	},
> >  };
> >
> > @@ -3368,7 +3368,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,
> > +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> >  	},
> >  };
> >
> > @@ -3382,7 +3382,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,
> > +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> >  	},
> >  };
> >
> > @@ -3396,7 +3396,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,
> > +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> >  	},
> >  };
> 
--
Martijn


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic