| Message ID | 20250213221702.606-1-linux@martijnvandeventer.nl (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clk: meson: g12a: Fix kernel warnings when no display attached | expand |
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. > > 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, > }, > };
On 13/02/2025 23:17, Martijn van Deventer 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"). > > 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, > }, > }; > Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
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
On Wed 26 Feb 2025 at 21:39, <linux@martijnvandeventer.nl> wrote: > 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. You have identified which clocks are not properly claimed, by what they are not claimed and even when. 50% of the job is done. Thanks for this. > > 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. Maybe so, but it does not make this change appropriate. The problem is the DRM driver which does not enable what it needs to properly operate. This should be fixed. > >> > >> > 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, >> > }, >> > }; >>
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. > > You have identified which clocks are not properly claimed, by what they > are not claimed and even when. 50% of the job is done. Thanks for this. You're welcome, no problem. > > > > 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. > > Maybe so, but it does not make this change appropriate. The problem > is the DRM driver which does not enable what it needs to properly > operate. This should be fixed. I understand. So I guess that is the end of the line for this patch. Because this patch will not be accepted and if someone else finds the time and has the knowledge to fix this the proper way, it will be a completely different patch. Although I, of course, agree with you that it should be fixed properly, I find it a bit difficult to accept that if we accidentally break something, while trying to make things better, we are not allowed to revert it because it was already somewhat broken. Resulting in a more broken situation than before... On the other hand, I also understand that if you, as a maintainer, allow that, chances are it will never see a proper fix. :-) Cheers! > > > >> > > >> > 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 = { -- Best regards, Martijn
On Thu 06 Mar 2025 at 17:48, <linux@martijnvandeventer.nl> wrote: > 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. >> >> You have identified which clocks are not properly claimed, by what they >> are not claimed and even when. 50% of the job is done. Thanks for this. > > You're welcome, no problem. > >> > >> > 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. >> >> Maybe so, but it does not make this change appropriate. The problem >> is the DRM driver which does not enable what it needs to properly >> operate. This should be fixed. > > I understand. So I guess that is the end of the line for this patch. > Because this patch will not be accepted and if someone else finds the > time and has the knowledge to fix this the proper way, it will be a > completely different patch. > > Although I, of course, agree with you that it should be fixed properly, > I find it a bit difficult to accept that if we accidentally break something, > while trying to make things better, we are not allowed to revert it > because it was already somewhat broken. Resulting in a more broken > situation than before... Once again, you are encouraged to fix things up ... where fixes are needed. DRM is hardly immutable. If you don't feel like you can do it on your own, you can still engage with the other contributors who may know this better and help you. > > On the other hand, I also understand that if you, as a maintainer, allow > that, chances are it will never see a proper fix. :-) > > Cheers! > >> > >> >> > >> >> > 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 = {
Hi Jerome, > > Once again, you are encouraged to fix things up ... where fixes are needed. Encouragement is not the problem, time is ;-) > DRM is hardly immutable. If you don't feel like you can do it on your own, > you can still engage with the other contributors who may know this > better and help you. Maybe in a few months when I have some more time. Thanks. > > > > On the other hand, I also understand that if you, as a maintainer, allow > > that, chances are it will never see a proper fix. :-) > > > > Cheers! > >
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, }, };
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"). 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(-)