[PATCH] ASoC: mediatek: mt8188: Enable apll1 clock during reg rw to prevent hang
Nícolas F. R. A. Prado
nfraprado at collabora.com
Thu Jan 30 05:12:39 PST 2025
On Wed, Dec 11, 2024 at 01:47:32PM +0000, Trevor Wu (吳文良) wrote:
> On Mon, 2024-12-09 at 17:07 -0300, Nícolas F. R. A. Prado wrote:
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >
> >
> > On Fri, Dec 06, 2024 at 06:57:00AM +0000, Trevor Wu (吳文良) wrote:
> > > On Thu, 2024-12-05 at 13:51 +0100, AngeloGioacchino Del Regno
> > > wrote:
> > > >
> > > >
> > > > Il 04/12/24 13:17, Trevor Wu (吳文良) ha scritto:
> > > > > On Tue, 2024-12-03 at 17:07 -0300, Nícolas F. R. A. Prado
> > > > > wrote:
> > > > >
> > > > > >
> > > > > > Currently, booting the Genio 700 EVK board with the MT8188
> > > > > > sound
> > > > > > platform driver configured as a module
> > > > > > (CONFIG_SND_SOC_MT8188=m)
> > > > > > results
> > > > > > in a system hang right when the HW registers for the audio
> > > > > > controller
> > > > > > are read:
> > > > > >
> > > > > > mt8188-audio 10b10000.audio-controller: No cache defaults,
> > > > > > reading
> > > > > > back from HW
> > > > > >
> > > > > > The hang doesn't occur with the driver configured as builtin
> > > > > > as
> > > > > > then
> > > > > > the
> > > > > > unused clocks are still enabled.
> > > > > >
> > > > > > Enable the apll1 clock during register read/write to prevent
> > > > > > the
> > > > > > hang.
> > > > > >
> > > > > > Signed-off-by: Nícolas F. R. A. Prado <
> > > > > > nfraprado at collabora.com>
> > > > > > ---
> > > > > > sound/soc/mediatek/mt8188/mt8188-afe-clk.c | 4 ++++
> > > > > > 1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
> > > > > > b/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
> > > > > > index
> > > > > > e69c1bb2cb239596dee50b166c20192d5408be10..fb8cf286df3f02ac076
> > > > > > 528b
> > > > > > 898f
> > > > > > d0d7a708ec1ea 100644
> > > > > > --- a/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
> > > > > > +++ b/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
> > > > > > @@ -587,6 +587,8 @@ int mt8188_afe_enable_reg_rw_clk(struct
> > > > > > mtk_base_afe *afe)
> > > > > > mt8188_afe_enable_clk(afe, afe_priv-
> > > > > > > clk[MT8188_CLK_AUD_A1SYS_HP]);
> > > > > >
> > > > > > mt8188_afe_enable_clk(afe, afe_priv-
> > > > > > > clk[MT8188_CLK_AUD_A1SYS]);
> > > > > >
> > > > > > + mt8188_afe_enable_clk(afe, afe_priv-
> > > > > > > clk[MT8188_CLK_APMIXED_APLL1]);
> > > > > >
> > > > > > +
> > > > > > return 0;
> > > > > > }
> > > > >
> > > > > Hi Nicolas,
> > > > >
> > > > > If I understand correctly, APLL1 should be the parent clock of
> > > > > AUD_A1SYS_HP and AUD_A1SYS, so it should be enabled
> > > > > automatically
> > > > > by
> > > > > CCF.
> > > > >
> > > > > I'm not sure why you resolved the hang issue after enabling
> > > > > APLL1.
> > > > > Could you share more details about the solution?
> > > > >
> > > >
> > > > Hmm. Now I see what's happening here...
> > > >
> > > > Nicolas, Trevor,
> > > >
> > > > Possible parents for top_a1sys_hp are:
> > > > - clk26m
> > > > - apll1_d4
> > > >
> > > > ...what's happening here most probably is that after the clock
> > > > gets
> > > > disabled as
> > > > unused, it gets parented to clk26m by default as that is parent
> > > > index
> > > > 0... and
> > > > something else in AFE needs APLL1 to feed a clock to .. something
> > > > ..
> > > > to allow
> > > > register access.
> > > >
> > > > Trevor, do you know why is this IP unaccessible when A1SYS is
> > > > parented to clk26m?
> > >
> > > Hi Angelo,
> > >
> > > As far as I know, it should work even though the clock is parented
> > > to
> > > clk26m.
> > >
> > > Unfortunately, I have no idea about why APLL1 enabling can resolve
> > > the
> > > hang issue. I'm also curious about how Nicolas found the solution
> > > to
> > > resolve the problem.
> > >
> > > From the description, it seems that the problem is related to
> > > register
> > > r/w hang. If I remembered correctly, when the mtcmos of ADSP_INFRA
> > > is
> > > ON, register r/w won't cause the cpu to hang. However, ADSP_INFRA
> > > has
> > > been configured as ALWAYS_ON in the driver. I'm not sure if it
> > > doesn't
> > > work correctly when the driver is configured as a module. Maybe
> > > Nicolas
> > > can also check this.
> >
> > Indeed, as suggested by Angelo, adding
> >
> > assigned-clocks = <&topckgen CLK_TOP_A1SYS_HP>;
> > assigned-clock-parents = <&topckgen CLK_TOP_APLL1_D4>;
> >
> > to the afe node also fixes this issue.
> >
> > In mt8188.dtsi, we currently have
> >
> > afe: audio-controller at 10b10000 {
> > ...
> > assigned-clocks = <&topckgen CLK_TOP_A1SYS_HP>;
> > assigned-clock-parents = <&clk26m>;
> >
> > So the question is, do other MT8188 platforms need clk26m to be the
> > parent of
> > a1sys_hp? Depending on that I can either update the parent on the
> > common
> > mt8188.dtsi or on the genio700-evk.dts, which is where I observed the
> > issue. I
> > don't have access to other mt8188 platforms. Trevor, do you know?
>
>
> If I remember correctly, the reason for assigning 26m as the parent of
> a1sys_hp is to save power[1], as APLL is not necessary in all cases.
>
> [1]
> https://lore.kernel.org/r/20230510035526.18137-5-trevor.wu@mediatek.com
Hi,
Following the suggestions, I have enabled the debug logs in
mt8188-afe-clk.c, though that didn't reveal anything (the logs are only printed
in case of errors, so there are none). I also verified that forcing the
ADSP_INFRA power domain to be powered on during boot (and before these register
reads occur) doesn't change anything (this domain is marked as ALWAYS_ON, so
normally assumed to already be on and not touched).
I've also narrowed the issue further. The hang happens in
mt8188_afe_pcm_dev_probe()
devm_regmap_init_mmio()
regcache_hw_init()
specifically in the "fill the reg_defaults" loop.
What's more, it doesn't happen for every register, but rather a few specific
ones.
The following register reads caused the hang:
0x280
0x410 (AFE_GAIN1_CON0)
0x830 (AFE_CONN0_5)
The following ranges were read without a problem:
0x0 (AUDIO_TOP_CON0) ... 0x27c
0x380 (AFE_SPDIF_OUT_CON0) ... 0x40c
0x550 (SPDIFIN_FREQ_INFO) ... 0x82c
And again, if I enable the apll1 clock in mt8188_afe_enable_reg_rw_clk(), as
done in this patch, or parent a1sys_hp to apll1_d4 in the AFE DT node, then
those registers are read without any hangs.
So it seems that certain registers require the apll1 clock to be enabled
in order to be read, while others don't. Trevor, is the dependency between those
specific registers and the apll1 clock documented anywhere?
Unless there's a better solution, I think we should go forward with either
this patch or the DT clock parenting suggested by Angelo, as the power savings
come second to the machine actually being bootable.
Thanks,
Nícolas
More information about the Linux-mediatek
mailing list