Quoting Dean Wallace (2018-10-25 16:25:17)
I have found a regression in 4.18.15 that means I lose sound on my old Toshiba Chromebook 2 (Swanky). My system details are:-
Toshiba Chromebook (Swanky) MrChromebox UEFI coreboot Arch Linux running latest alsa/pulseaudio
Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By output I mean, the card is still detected, the module loaded, all apps showing sound is being playing, but no actual audible sound comes through. Upgraded to 4.18.16 same issue.
Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f clk: x86: Stop marking clocks as CLK_IS_CRITICAL "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail devices not being able to reach S0i3 greatly decreasing their battery drain when suspended."
I reverted it and compiled 4.18.16 and have sound back again. Could this be looked into, with possibility of fix.
Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Dean Wallace (2018-10-25 16:25:17)
I have found a regression in 4.18.15 that means I lose sound on my old Toshiba Chromebook 2 (Swanky). My system details are:-
Toshiba Chromebook (Swanky) MrChromebox UEFI coreboot Arch Linux running latest alsa/pulseaudio
Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By output I mean, the card is still detected, the module loaded, all apps showing sound is being playing, but no actual audible sound comes through. Upgraded to 4.18.16 same issue.
Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f clk: x86: Stop marking clocks as CLK_IS_CRITICAL "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail devices not being able to reach S0i3 greatly decreasing their battery drain when suspended."
I reverted it and compiled 4.18.16 and have sound back again. Could this be looked into, with possibility of fix.
Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Dean Wallace (2018-10-25 16:25:17)
I have found a regression in 4.18.15 that means I lose sound on my old Toshiba Chromebook 2 (Swanky). My system details are:-
Toshiba Chromebook (Swanky) MrChromebox UEFI coreboot Arch Linux running latest alsa/pulseaudio
Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By output I mean, the card is still detected, the module loaded, all apps showing sound is being playing, but no actual audible sound comes through. Upgraded to 4.18.16 same issue.
Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f clk: x86: Stop marking clocks as CLK_IS_CRITICAL "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail devices not being able to reach S0i3 greatly decreasing their battery drain when suspended."
I reverted it and compiled 4.18.16 and have sound back again. Could this be looked into, with possibility of fix.
Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
Btw, what the drivers are in use for the machine you have? It's better you run alsa-info.sh or alike to collect necessary information along with output of `lsmod`, `dmesg`, etc.
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Dean Wallace (2018-10-25 16:25:17)
I have found a regression in 4.18.15 that means I lose sound on my old Toshiba Chromebook 2 (Swanky). My system details are:-
Toshiba Chromebook (Swanky) MrChromebox UEFI coreboot Arch Linux running latest alsa/pulseaudio
Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By output I mean, the card is still detected, the module loaded, all apps showing sound is being playing, but no actual audible sound comes through. Upgraded to 4.18.16 same issue.
Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f clk: x86: Stop marking clocks as CLK_IS_CRITICAL "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail devices not being able to reach S0i3 greatly decreasing their battery drain when suspended."
I reverted it and compiled 4.18.16 and have sound back again. Could this be looked into, with possibility of fix.
Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
Btw, what the drivers are in use for the machine you have? It's better you run alsa-info.sh or alike to collect necessary information along with output of `lsmod`, `dmesg`, etc.
-- With Best Regards, Andy Shevchenko
alsa-info.txt [1], cpuinfo [2], dmesg [3], and 'ls /sys/bus/i2c/devices' [4] [1] https://gist.github.com/duffydack/480be8ddced44515dbf981dc09f593ec#file-alsa... [2] https://gist.github.com/duffydack/480be8ddced44515dbf981dc09f593ec#file-cpui... [3] https://gist.github.com/duffydack/480be8ddced44515dbf981dc09f593ec#file-dmes... [4] https://gist.github.com/duffydack/480be8ddced44515dbf981dc09f593ec#file-sys-...
Regards
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Dean Wallace (2018-10-25 16:25:17)
I have found a regression in 4.18.15 that means I lose sound on my old Toshiba Chromebook 2 (Swanky). My system details are:-
Toshiba Chromebook (Swanky) MrChromebox UEFI coreboot Arch Linux running latest alsa/pulseaudio
Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By output I mean, the card is still detected, the module loaded, all apps showing sound is being playing, but no actual audible sound comes through. Upgraded to 4.18.16 same issue.
Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f clk: x86: Stop marking clocks as CLK_IS_CRITICAL "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail devices not being able to reach S0i3 greatly decreasing their battery drain when suspended."
I reverted it and compiled 4.18.16 and have sound back again. Could this be looked into, with possibility of fix.
Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware, not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
Btw, what the drivers are in use for the machine you have? It's better you run alsa-info.sh or alike to collect necessary information along with output of `lsmod`, `dmesg`, etc.
-- With Best Regards, Andy Shevchenko
alsa-info.txt [1], cpuinfo [2], dmesg [3], and 'ls /sys/bus/i2c/devices' [4] [1] https://gist.github.com/duffydack/480be8ddced44515dbf981dc09f593ec#file-alsa... [2] https://gist.github.com/duffydack/480be8ddced44515dbf981dc09f593ec#file-cpui... [3] https://gist.github.com/duffydack/480be8ddced44515dbf981dc09f593ec#file-dmes... [4] https://gist.github.com/duffydack/480be8ddced44515dbf981dc09f593ec#file-sys-...
Regards
Hi Pierre-Louis,
On 29-10-18 23:03, Pierre-Louis Bossart wrote:
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Dean Wallace (2018-10-25 16:25:17)
I have found a regression in 4.18.15 that means I lose sound on my old Toshiba Chromebook 2 (Swanky). My system details are:-
Toshiba Chromebook (Swanky) MrChromebox UEFI coreboot Arch Linux running latest alsa/pulseaudio
Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By output I mean, the card is still detected, the module loaded, all apps showing sound is being playing, but no actual audible sound comes through. Upgraded to 4.18.16 same issue.
Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f clk: x86: Stop marking clocks as CLK_IS_CRITICAL "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail devices not being able to reach S0i3 greatly decreasing their battery drain when suspended."
I reverted it and compiled 4.18.16 and have sound back again. Could this be looked into, with possibility of fix.
Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware,
I believe that the statement "It's indeed the expectation that the audio mclk is handled by the firmware" is not correct for BYT/CHT platforms (and the commit causing the regression only affects BYT/CHT platforms). Various machine drivers under sound/soc/intel/boards have code to deal with the mclk themselves, like this:
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3"); ...
if (SND_SOC_DAPM_EVENT_ON(event)) { ret = clk_prepare_enable(ctx->mclk); ... } else { clk_disable_unprepare(ctx->mclk); }
The above code is from sound/soc/intel/boards/cht_bsw_max98090_ti.c which is the machine driver used on the machines for which problems are now being reported.
And my commit introducing the problem is in essence a revert of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Which is from 2017-07-14 and the ASoC code for BYT/CHT platforms is much older then that.
Also I asked Carlo why he wrote that patch and it was to fix a problem with ethernet on some laptops.
not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
It is necessary, because if it is set the clock never gets disabled and on x86 platforms using suspend2idle we are responsible for *all* the hardware power-management as OS. If we do not disable the clocks then we can only reach S0i1 instead of S0i3 when suspended leading to increased battery drain during suspend.
Also note that this patch is only causing problems on CHT + max98090 codec using machines. I've tested it on many other BYT/CHT machines myself and we've not had any other bug reports related to this.
So this clearly points to a problem with the clock management in the cht_bsw_max98090_ti.c machine driver.
I've some ideas how to fix this and I will prepare some patches to test.
Regards,
Hans
Hi,
On 30-10-18 11:17, Hans de Goede wrote:
Hi Pierre-Louis,
On 29-10-18 23:03, Pierre-Louis Bossart wrote:
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Dean Wallace (2018-10-25 16:25:17) > I have found a regression in 4.18.15 that means I lose sound on my old > Toshiba Chromebook 2 (Swanky). My system details are:- > > Toshiba Chromebook (Swanky) > MrChromebox UEFI coreboot > Arch Linux running latest alsa/pulseaudio > > Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By > output I mean, the card is still detected, the module loaded, all apps > showing sound is being playing, but no actual audible sound comes > through. Upgraded to 4.18.16 same issue. > > Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f > clk: x86: Stop marking clocks as CLK_IS_CRITICAL > "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail > devices not being able to reach S0i3 greatly decreasing their battery > drain when suspended." > > I reverted it and compiled 4.18.16 and have sound back again. Could > this be looked into, with possibility of fix. > Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware,
I believe that the statement "It's indeed the expectation that the audio mclk is handled by the firmware" is not correct for BYT/CHT platforms (and the commit causing the regression only affects BYT/CHT platforms). Various machine drivers under sound/soc/intel/boards have code to deal with the mclk themselves, like this:
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3"); ...
if (SND_SOC_DAPM_EVENT_ON(event)) { ret = clk_prepare_enable(ctx->mclk); ... } else { clk_disable_unprepare(ctx->mclk); }
The above code is from sound/soc/intel/boards/cht_bsw_max98090_ti.c which is the machine driver used on the machines for which problems are now being reported.
And my commit introducing the problem is in essence a revert of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Which is from 2017-07-14 and the ASoC code for BYT/CHT platforms is much older then that.
Also I asked Carlo why he wrote that patch and it was to fix a problem with ethernet on some laptops.
not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
It is necessary, because if it is set the clock never gets disabled and on x86 platforms using suspend2idle we are responsible for *all* the hardware power-management as OS. If we do not disable the clocks then we can only reach S0i1 instead of S0i3 when suspended leading to increased battery drain during suspend.
Also note that this patch is only causing problems on CHT + max98090 codec using machines. I've tested it on many other BYT/CHT machines myself and we've not had any other bug reports related to this.
So this clearly points to a problem with the clock management in the cht_bsw_max98090_ti.c machine driver.
I've some ideas how to fix this and I will prepare some patches to test.
p.s.
It seems that a sub-thread of this thread where me and the reporter have been working on debugging this has lost most of the people in the Cc.
So here is a quick status update. So far this issue has been seen on Swanky and Clapper model chromebooks.
Reverting the patch causing this regression and then doing:
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done
has shown that before my patch to not set CLK_IS_CRITICAL the CLK_IS_CRITICAL was being set on pmc_plt_clk_0. Which shows that that clock is somehow being used for sounds on these boards.
I think that pmc_plt_clk_0 is being used instead of pmc_plt_clk_3, but it could also be the case that both are being used.
I'm currently preparing patches to test both scenarios.
Regards,
Hans
On Tue, Oct 30, 2018 at 1:05 PM Hans de Goede hdegoede@redhat.com wrote:
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done
Just a side note about nice tool, called `grep` :-)
grep -H . /sys/kernel/debug/clk/pmc_plt_clk_*/clk_flags
does above in one call.
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
Regards,
Hans
On 29-10-18 23:03, Pierre-Louis Bossart wrote:
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Dean Wallace (2018-10-25 16:25:17)
I have found a regression in 4.18.15 that means I lose sound on my old Toshiba Chromebook 2 (Swanky). My system details are:-
Toshiba Chromebook (Swanky) MrChromebox UEFI coreboot Arch Linux running latest alsa/pulseaudio
Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By output I mean, the card is still detected, the module loaded, all apps showing sound is being playing, but no actual audible sound comes through. Upgraded to 4.18.16 same issue.
Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f clk: x86: Stop marking clocks as CLK_IS_CRITICAL "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail devices not being able to reach S0i3 greatly decreasing their battery drain when suspended."
I reverted it and compiled 4.18.16 and have sound back again. Could this be looked into, with possibility of fix.
Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware,
I believe that the statement "It's indeed the expectation that the audio mclk is handled by the firmware" is not correct for BYT/CHT platforms (and the commit causing the regression only affects BYT/CHT platforms). Various machine drivers under sound/soc/intel/boards have code to deal with the mclk themselves, like this:
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3"); ...
if (SND_SOC_DAPM_EVENT_ON(event)) { ret = clk_prepare_enable(ctx->mclk); ... } else { clk_disable_unprepare(ctx->mclk); }
The above code is from sound/soc/intel/boards/cht_bsw_max98090_ti.c which is the machine driver used on the machines for which problems are now being reported.
And my commit introducing the problem is in essence a revert of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Which is from 2017-07-14 and the ASoC code for BYT/CHT platforms is much older then that.
Also I asked Carlo why he wrote that patch and it was to fix a problem with ethernet on some laptops.
not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
It is necessary, because if it is set the clock never gets disabled and on x86 platforms using suspend2idle we are responsible for *all* the hardware power-management as OS. If we do not disable the clocks then we can only reach S0i1 instead of S0i3 when suspended leading to increased battery drain during suspend.
Also note that this patch is only causing problems on CHT + max98090 codec using machines. I've tested it on many other BYT/CHT machines myself and we've not had any other bug reports related to this.
So this clearly points to a problem with the clock management in the cht_bsw_max98090_ti.c machine driver.
I've some ideas how to fix this and I will prepare some patches to test.
Regards,
Hans
On 30-10-18, Hans de Goede wrote:
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
Regards,
Hans
On 29-10-18 23:03, Pierre-Louis Bossart wrote:
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Dean Wallace (2018-10-25 16:25:17) > I have found a regression in 4.18.15 that means I lose sound on my old > Toshiba Chromebook 2 (Swanky). My system details are:- > > Toshiba Chromebook (Swanky) > MrChromebox UEFI coreboot > Arch Linux running latest alsa/pulseaudio > > Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By > output I mean, the card is still detected, the module loaded, all apps > showing sound is being playing, but no actual audible sound comes > through. Upgraded to 4.18.16 same issue. > > Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f > clk: x86: Stop marking clocks as CLK_IS_CRITICAL > "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail > devices not being able to reach S0i3 greatly decreasing their battery > drain when suspended." > > I reverted it and compiled 4.18.16 and have sound back again. Could > this be looked into, with possibility of fix. > Thanks for the bug report. I'm adding some people involved in the commit you mention is causing audio regressions. The best plan is to probably revert the commit from the 4.18 linux stable tree. Or there may be another patch missing that would be useful to make this backported patch work. Hopefully Hans or Andy knows.
Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware,
I believe that the statement "It's indeed the expectation that the audio mclk is handled by the firmware" is not correct for BYT/CHT platforms (and the commit causing the regression only affects BYT/CHT platforms). Various machine drivers under sound/soc/intel/boards have code to deal with the mclk themselves, like this:
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");...
if (SND_SOC_DAPM_EVENT_ON(event)) { ret = clk_prepare_enable(ctx->mclk); ... } else { clk_disable_unprepare(ctx->mclk); }The above code is from sound/soc/intel/boards/cht_bsw_max98090_ti.c which is the machine driver used on the machines for which problems are now being reported.
And my commit introducing the problem is in essence a revert of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Which is from 2017-07-14 and the ASoC code for BYT/CHT platforms is much older then that.
Also I asked Carlo why he wrote that patch and it was to fix a problem with ethernet on some laptops.
not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
It is necessary, because if it is set the clock never gets disabled and on x86 platforms using suspend2idle we are responsible for *all* the hardware power-management as OS. If we do not disable the clocks then we can only reach S0i1 instead of S0i3 when suspended leading to increased battery drain during suspend.
Also note that this patch is only causing problems on CHT + max98090 codec using machines. I've tested it on many other BYT/CHT machines myself and we've not had any other bug reports related to this.
So this clearly points to a problem with the clock management in the cht_bsw_max98090_ti.c machine driver.
I've some ideas how to fix this and I will prepare some patches to test.
Regards,
Hans
Excellent work Hans. Compiled 4.19 with 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch, sound works as before.
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done /sys/kernel/debug/clk/pmc_plt_clk_0: /sys/kernel/debug/clk/pmc_plt_clk_1: /sys/kernel/debug/clk/pmc_plt_clk_2: /sys/kernel/debug/clk/pmc_plt_clk_3: /sys/kernel/debug/clk/pmc_plt_clk_4: /sys/kernel/debug/clk/pmc_plt_clk_5:
Regards
Hi,
On 30-10-18 15:38, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
Regards,
Hans
On 29-10-18 23:03, Pierre-Louis Bossart wrote:
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote: > Quoting Dean Wallace (2018-10-25 16:25:17) >> I have found a regression in 4.18.15 that means I lose sound on my old >> Toshiba Chromebook 2 (Swanky). My system details are:- >> >> Toshiba Chromebook (Swanky) >> MrChromebox UEFI coreboot >> Arch Linux running latest alsa/pulseaudio >> >> Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By >> output I mean, the card is still detected, the module loaded, all apps >> showing sound is being playing, but no actual audible sound comes >> through. Upgraded to 4.18.16 same issue. >> >> Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f >> clk: x86: Stop marking clocks as CLK_IS_CRITICAL >> "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail >> devices not being able to reach S0i3 greatly decreasing their battery >> drain when suspended." >> >> I reverted it and compiled 4.18.16 and have sound back again. Could >> this be looked into, with possibility of fix. >> > Thanks for the bug report. I'm adding some people involved in the commit > you mention is causing audio regressions. The best plan is to probably > revert the commit from the 4.18 linux stable tree. Or there may be > another patch missing that would be useful to make this backported patch > work. Hopefully Hans or Andy knows. Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware,
I believe that the statement "It's indeed the expectation that the audio mclk is handled by the firmware" is not correct for BYT/CHT platforms (and the commit causing the regression only affects BYT/CHT platforms). Various machine drivers under sound/soc/intel/boards have code to deal with the mclk themselves, like this:
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");...
if (SND_SOC_DAPM_EVENT_ON(event)) { ret = clk_prepare_enable(ctx->mclk); ... } else { clk_disable_unprepare(ctx->mclk); }The above code is from sound/soc/intel/boards/cht_bsw_max98090_ti.c which is the machine driver used on the machines for which problems are now being reported.
And my commit introducing the problem is in essence a revert of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Which is from 2017-07-14 and the ASoC code for BYT/CHT platforms is much older then that.
Also I asked Carlo why he wrote that patch and it was to fix a problem with ethernet on some laptops.
not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
It is necessary, because if it is set the clock never gets disabled and on x86 platforms using suspend2idle we are responsible for *all* the hardware power-management as OS. If we do not disable the clocks then we can only reach S0i1 instead of S0i3 when suspended leading to increased battery drain during suspend.
Also note that this patch is only causing problems on CHT + max98090 codec using machines. I've tested it on many other BYT/CHT machines myself and we've not had any other bug reports related to this.
So this clearly points to a problem with the clock management in the cht_bsw_max98090_ti.c machine driver.
I've some ideas how to fix this and I will prepare some patches to test.
Regards,
Hans
Excellent work Hans. Compiled 4.19 with 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch, sound works as before.
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done /sys/kernel/debug/clk/pmc_plt_clk_0: /sys/kernel/debug/clk/pmc_plt_clk_1: /sys/kernel/debug/clk/pmc_plt_clk_2: /sys/kernel/debug/clk/pmc_plt_clk_3: /sys/kernel/debug/clk/pmc_plt_clk_4: /sys/kernel/debug/clk/pmc_plt_clk_5:
Ok, so as I expected the Swanky is using pmc_plt_clk_0 as mclk instead of pmc_plt_clk_3. Now the question becomes is this true for all the designs using the max98090 codec?
Mogens, can you give the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch a try on your Clapper model, using the SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH kernel option together with the asoundrc from Dean?
I have the feeling that your Clapper is also using the pmc_plt_clk_0.
Pierre-Louis, as maintainer/reviewer of all the drivers under sound/soc/intel/boards, how would you like to proceed with this?
I see 2 possible ways forwards:
1) Unconditionally use pmc_plt_clk_0 as mclk (as my test patch does) in the cht_bsw_max98090_ti.c machine driver
2) Use DMI quirks for the Swansky (and probably also the Clapper) to use pmc_plt_clk_0 as mclk there while keeping pmc_plt_clk_3 as the default.
If you (Pierre-Louis) can let me know which solution you prepare then I can prepare and submit a patch to fix this.
Regards,
Hans
On Tue, Oct 30, 2018 at 4:48 PM Hans de Goede hdegoede@redhat.com wrote:
On 30-10-18 15:38, Dean Wallace wrote:
Excellent work Hans. Compiled 4.19 with 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch, sound works as before.
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done /sys/kernel/debug/clk/pmc_plt_clk_0: /sys/kernel/debug/clk/pmc_plt_clk_1: /sys/kernel/debug/clk/pmc_plt_clk_2: /sys/kernel/debug/clk/pmc_plt_clk_3: /sys/kernel/debug/clk/pmc_plt_clk_4: /sys/kernel/debug/clk/pmc_plt_clk_5:
Ok, so as I expected the Swanky is using pmc_plt_clk_0 as mclk instead of pmc_plt_clk_3. Now the question becomes is this true for all the designs using the max98090 codec?
- Unconditionally use pmc_plt_clk_0 as mclk (as my test patch does)
in the cht_bsw_max98090_ti.c machine driver
I don't know the details, but the main question here indeed do we ever had a working example of that machine with CLK #3 in use?
P.S. I would go your first proposal until the opposite will be proved.
On 10/30/18 10:03 AM, Andy Shevchenko wrote:
On Tue, Oct 30, 2018 at 4:48 PM Hans de Goede hdegoede@redhat.com wrote:
On 30-10-18 15:38, Dean Wallace wrote:
Excellent work Hans. Compiled 4.19 with 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch, sound works as before.
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done /sys/kernel/debug/clk/pmc_plt_clk_0: /sys/kernel/debug/clk/pmc_plt_clk_1: /sys/kernel/debug/clk/pmc_plt_clk_2: /sys/kernel/debug/clk/pmc_plt_clk_3: /sys/kernel/debug/clk/pmc_plt_clk_4: /sys/kernel/debug/clk/pmc_plt_clk_5:
Ok, so as I expected the Swanky is using pmc_plt_clk_0 as mclk instead of pmc_plt_clk_3. Now the question becomes is this true for all the designs using the max98090 codec?
- Unconditionally use pmc_plt_clk_0 as mclk (as my test patch does)
in the cht_bsw_max98090_ti.c machine driver
I don't know the details, but the main question here indeed do we ever had a working example of that machine with CLK #3 in use?
yes, Rambi/Orco (Baytrail based).
P.S. I would go your first proposal until the opposite will be proved.
On 10/30/18 9:38 AM, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
For Baytrail devices, the audio platform clocks are not managed by the firmware. They are for CHT-based devices - as can be seen by clock resources being described in the DSDT. We used to have a if(baytrail) in the code which was replaced by this CRITICAL label, but the point remains that there is a difference between the two SOC versions.
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though, however you still have the problem of trying to manage from the kernel what the firmware already manages.
Regards,
Hans
On 29-10-18 23:03, Pierre-Louis Bossart wrote:
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
Cc: Pierre as well.
On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote: > Quoting Dean Wallace (2018-10-25 16:25:17) >> I have found a regression in 4.18.15 that means I lose sound on my old >> Toshiba Chromebook 2 (Swanky). My system details are:- >> >> Toshiba Chromebook (Swanky) >> MrChromebox UEFI coreboot >> Arch Linux running latest alsa/pulseaudio >> >> Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By >> output I mean, the card is still detected, the module loaded, all apps >> showing sound is being playing, but no actual audible sound comes >> through. Upgraded to 4.18.16 same issue. >> >> Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f >> clk: x86: Stop marking clocks as CLK_IS_CRITICAL >> "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail >> devices not being able to reach S0i3 greatly decreasing their battery >> drain when suspended." >> >> I reverted it and compiled 4.18.16 and have sound back again. Could >> this be looked into, with possibility of fix. >> > Thanks for the bug report. I'm adding some people involved in the commit > you mention is causing audio regressions. The best plan is to probably > revert the commit from the 4.18 linux stable tree. Or there may be > another patch missing that would be useful to make this backported patch > work. Hopefully Hans or Andy knows. Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. I have a feeling that the problem can be fixed by properly handling clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out better than me.
Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware,
I believe that the statement "It's indeed the expectation that the audio mclk is handled by the firmware" is not correct for BYT/CHT platforms (and the commit causing the regression only affects BYT/CHT platforms). Various machine drivers under sound/soc/intel/boards have code to deal with the mclk themselves, like this:
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");...
if (SND_SOC_DAPM_EVENT_ON(event)) { ret = clk_prepare_enable(ctx->mclk); ... } else { clk_disable_unprepare(ctx->mclk); }The above code is from sound/soc/intel/boards/cht_bsw_max98090_ti.c which is the machine driver used on the machines for which problems are now being reported.
And my commit introducing the problem is in essence a revert of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Which is from 2017-07-14 and the ASoC code for BYT/CHT platforms is much older then that.
Also I asked Carlo why he wrote that patch and it was to fix a problem with ethernet on some laptops.
not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
It is necessary, because if it is set the clock never gets disabled and on x86 platforms using suspend2idle we are responsible for *all* the hardware power-management as OS. If we do not disable the clocks then we can only reach S0i1 instead of S0i3 when suspended leading to increased battery drain during suspend.
Also note that this patch is only causing problems on CHT + max98090 codec using machines. I've tested it on many other BYT/CHT machines myself and we've not had any other bug reports related to this.
So this clearly points to a problem with the clock management in the cht_bsw_max98090_ti.c machine driver.
I've some ideas how to fix this and I will prepare some patches to test.
Regards,
Hans
Excellent work Hans. Compiled 4.19 with 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch, sound works as before.
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done /sys/kernel/debug/clk/pmc_plt_clk_0: /sys/kernel/debug/clk/pmc_plt_clk_1: /sys/kernel/debug/clk/pmc_plt_clk_2: /sys/kernel/debug/clk/pmc_plt_clk_3: /sys/kernel/debug/clk/pmc_plt_clk_4: /sys/kernel/debug/clk/pmc_plt_clk_5:
Regards
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
On 10/30/18 9:38 AM, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
For Baytrail devices, the audio platform clocks are not managed by the firmware. They are for CHT-based devices - as can be seen by clock resources being described in the DSDT. We used to have a if(baytrail) in the code which was replaced by this CRITICAL label, but the point remains that there is a difference between the two SOC versions.
As I mentioned before the CRITICAL flag was only added a year ago to workaround an issue with on board ethernet needing plt_clk_4 on some laptops, this never had anything to do with sound.
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though,
Ok, so we need to have a DMI based quirk for the Swanky and maybe also the clapper to use plt_clk_0 there. Asking for 2 clks if we only need one does not seem like a good plan.
however you still have the problem of trying to manage from the kernel what the firmware already manages.
The firmware only manages it when going to D3 state, with ASoC most of the codecs gets turned off (and we no longer need the clock) but we do not put the device in D3 / execute the _PS3 method. So from a pm pov it is better if we manage the clk ourselves.
Once we do actually put the device in D3 (on suspend) the kernel will have already turned off the clk and the _OFF method of the CLK3 power resource which directly pokes mmio, will just set the enable bit to 0 when it already is 0, so no problem.
Regards,
Hans
Regards,
Hans
On 29-10-18 23:03, Pierre-Louis Bossart wrote:
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote:
On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko andy.shevchenko@gmail.com wrote: > Cc: Pierre as well. > > On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org wrote: >> Quoting Dean Wallace (2018-10-25 16:25:17) >>> I have found a regression in 4.18.15 that means I lose sound on my old >>> Toshiba Chromebook 2 (Swanky). My system details are:- >>> >>> Toshiba Chromebook (Swanky) >>> MrChromebox UEFI coreboot >>> Arch Linux running latest alsa/pulseaudio >>> >>> Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound output. By >>> output I mean, the card is still detected, the module loaded, all apps >>> showing sound is being playing, but no actual audible sound comes >>> through. Upgraded to 4.18.16 same issue. >>> >>> Dug around and found Upstream commit 648e921888ad96ea3dc922739e96716ad3225d7f >>> clk: x86: Stop marking clocks as CLK_IS_CRITICAL >>> "This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail >>> devices not being able to reach S0i3 greatly decreasing their battery >>> drain when suspended." >>> >>> I reverted it and compiled 4.18.16 and have sound back again. Could >>> this be looked into, with possibility of fix. >>> >> Thanks for the bug report. I'm adding some people involved in the commit >> you mention is causing audio regressions. The best plan is to probably >> revert the commit from the 4.18 linux stable tree. Or there may be >> another patch missing that would be useful to make this backported patch >> work. Hopefully Hans or Andy knows. > Hans has been investigating S0ix issues on Baytrail and Cherrytrail machines. > I have a feeling that the problem can be fixed by properly handling > clock in ASoC driver(s). Perhaps Hans and Pierre can figure this out > better than me. Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no suspend-resume hooks. Perhaps, adding them like in the commit ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware,
I believe that the statement "It's indeed the expectation that the audio mclk is handled by the firmware" is not correct for BYT/CHT platforms (and the commit causing the regression only affects BYT/CHT platforms). Various machine drivers under sound/soc/intel/boards have code to deal with the mclk themselves, like this:
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3"); ...
if (SND_SOC_DAPM_EVENT_ON(event)) { ret = clk_prepare_enable(ctx->mclk); ... } else { clk_disable_unprepare(ctx->mclk); }
The above code is from sound/soc/intel/boards/cht_bsw_max98090_ti.c which is the machine driver used on the machines for which problems are now being reported.
And my commit introducing the problem is in essence a revert of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Which is from 2017-07-14 and the ASoC code for BYT/CHT platforms is much older then that.
Also I asked Carlo why he wrote that patch and it was to fix a problem with ethernet on some laptops.
not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
It is necessary, because if it is set the clock never gets disabled and on x86 platforms using suspend2idle we are responsible for *all* the hardware power-management as OS. If we do not disable the clocks then we can only reach S0i1 instead of S0i3 when suspended leading to increased battery drain during suspend.
Also note that this patch is only causing problems on CHT + max98090 codec using machines. I've tested it on many other BYT/CHT machines myself and we've not had any other bug reports related to this.
So this clearly points to a problem with the clock management in the cht_bsw_max98090_ti.c machine driver.
I've some ideas how to fix this and I will prepare some patches to test.
Regards,
Hans
Excellent work Hans. Compiled 4.19 with 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch, sound works as before.
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done /sys/kernel/debug/clk/pmc_plt_clk_0: /sys/kernel/debug/clk/pmc_plt_clk_1: /sys/kernel/debug/clk/pmc_plt_clk_2: /sys/kernel/debug/clk/pmc_plt_clk_3: /sys/kernel/debug/clk/pmc_plt_clk_4: /sys/kernel/debug/clk/pmc_plt_clk_5:
Regards
Hi,
On 30-10-18 16:46, Hans de Goede wrote:
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
On 10/30/18 9:38 AM, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
For Baytrail devices, the audio platform clocks are not managed by the firmware. They are for CHT-based devices - as can be seen by clock resources being described in the DSDT. We used to have a if(baytrail) in the code which was replaced by this CRITICAL label, but the point remains that there is a difference between the two SOC versions.
As I mentioned before the CRITICAL flag was only added a year ago to workaround an issue with on board ethernet needing plt_clk_4 on some laptops, this never had anything to do with sound.
Ah I see now that you later made some changes based on the patch to fix the ethernet:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Note though that that does not touch the machine driver which we are discussing now and the reporters machine is also BYT based.
As explained below (in my original reply) I think it is fine to always manage the clk from within the kernel. But if you think this is a bad idea, we could re-introduce the is_valleyview() checks in machine drivers which are used on CHT devices.
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though,
Ok, so we need to have a DMI based quirk for the Swanky and maybe also the clapper to use plt_clk_0 there. Asking for 2 clks if we only need one does not seem like a good plan.
however you still have the problem of trying to manage from the kernel what the firmware already manages.
The firmware only manages it when going to D3 state, with ASoC most of the codecs gets turned off (and we no longer need the clock) but we do not put the device in D3 / execute the _PS3 method. So from a pm pov it is better if we manage the clk ourselves.
Once we do actually put the device in D3 (on suspend) the kernel will have already turned off the clk and the _OFF method of the CLK3 power resource which directly pokes mmio, will just set the enable bit to 0 when it already is 0, so no problem.
Regards,
Hans
On 10/30/18 11:02 AM, Hans de Goede wrote:
Hi,
On 30-10-18 16:46, Hans de Goede wrote:
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
On 10/30/18 9:38 AM, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
For Baytrail devices, the audio platform clocks are not managed by the firmware. They are for CHT-based devices - as can be seen by clock resources being described in the DSDT. We used to have a if(baytrail) in the code which was replaced by this CRITICAL label, but the point remains that there is a difference between the two SOC versions.
As I mentioned before the CRITICAL flag was only added a year ago to workaround an issue with on board ethernet needing plt_clk_4 on some laptops, this never had anything to do with sound.
Ah I see now that you later made some changes based on the patch to fix the ethernet:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Note though that that does not touch the machine driver which we are discussing now and the reporters machine is also BYT based.
As explained below (in my original reply) I think it is fine to always manage the clk from within the kernel. But if you think this is a bad idea, we could re-introduce the is_valleyview() checks in machine drivers which are used on CHT devices.
I am having difficulties following the proposal
for audio, managing the platform clocks from the kernel is only useful for Baytrail, and the code handling the CRITICAL flags is really equivalent to having a is_valleyview() test in the machine drivers. I don't quite understand the S0ix-related changes and I also don't get how using plt_clk_0 makes things better or wonder if audio on Swanky worked before the commit 7735bce05a9c ('ASoC: Intel: boards: use devm_clk_get() unconditionally'.
In other words, I don't know if we are dealing with a platform that only started working with 7735bce05a9c and does need a quirk to make use of plt_clk_0 or a fundamental issue with clock/power management.
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though,
Ok, so we need to have a DMI based quirk for the Swanky and maybe also the clapper to use plt_clk_0 there. Asking for 2 clks if we only need one does not seem like a good plan.
however you still have the problem of trying to manage from the kernel what the firmware already manages.
The firmware only manages it when going to D3 state, with ASoC most of the codecs gets turned off (and we no longer need the clock) but we do not put the device in D3 / execute the _PS3 method. So from a pm pov it is better if we manage the clk ourselves.
Once we do actually put the device in D3 (on suspend) the kernel will have already turned off the clk and the _OFF method of the CLK3 power resource which directly pokes mmio, will just set the enable bit to 0 when it already is 0, so no problem.
Regards,
Hans
Hi Pierre-Louis,
On 30-10-18 17:27, Pierre-Louis Bossart wrote:
On 10/30/18 11:02 AM, Hans de Goede wrote:
Hi,
On 30-10-18 16:46, Hans de Goede wrote:
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
On 10/30/18 9:38 AM, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
For Baytrail devices, the audio platform clocks are not managed by the firmware. They are for CHT-based devices - as can be seen by clock resources being described in the DSDT. We used to have a if(baytrail) in the code which was replaced by this CRITICAL label, but the point remains that there is a difference between the two SOC versions.
As I mentioned before the CRITICAL flag was only added a year ago to workaround an issue with on board ethernet needing plt_clk_4 on some laptops, this never had anything to do with sound.
Ah I see now that you later made some changes based on the patch to fix the ethernet:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Note though that that does not touch the machine driver which we are discussing now and the reporters machine is also BYT based.
As explained below (in my original reply) I think it is fine to always manage the clk from within the kernel. But if you think this is a bad idea, we could re-introduce the is_valleyview() checks in machine drivers which are used on CHT devices.
I am having difficulties following the proposal
for audio, managing the platform clocks from the kernel is only useful for Baytrail, and the code handling the CRITICAL flags is really equivalent to having a is_valleyview() test in the machine drivers. I don't quite understand the S0ix-related changes and I also don't get how using plt_clk_0 makes things better or wonder if audio on Swanky worked before the commit 7735bce05a9c ('ASoC: Intel: boards: use devm_clk_get() unconditionally'.
In other words, I don't know if we are dealing with a platform that only started working with 7735bce05a9c and does need a quirk to make use of plt_clk_0 or a fundamental issue with clock/power management.
Ok let me try to clarify this by breaking this out into the 3 separate things which I believe are in play here:
1) The PMC clocks should NOT be marked as CLK_IS_CRITICAL
Commit d31fd43c0f9a ("clk: x86: Do not gate clocks enabled by the firmware") should, in hindsight, never have been merged. CLK_IS_CRITICAL disables all management of the clk in question and is really intended for clks which are e.g. driving the CPU core itself or DRAM. The proper fix for the ethernet problem that patch was meant to address is for the ethernet driver to get and enable the clock itself. As I've done in a recent series which has as purpose to revert d31fd43c0f9a.
d31fd43c0f9a marks any PMC clock which is enabled by the firmware as boot as critical, not only the codec mclk, but *any* clk. This means that unless all clks which are enabled at boot are controlled by the firmware as either ACPI pwr-resources, or by a device's _PS3 method, some clks will stay on during suspend, see e.g. :
https://bugzilla.kernel.org/show_bug.cgi?id=193891#c102 https://bugzilla.kernel.org/show_bug.cgi?id=196861
Also on BYT this means that if the firmware has enabled the coded mclk at boot it will get marked as CLK_IS_CRITICAL and we will now never disable it.
If some clocks are still on during suspend then the SoC cannot reach its deepest sleep states while suspended, which obviously is bad.
TL;DR: CLK_IS_CRITICAL block reaching the deepest sleep states so d31fd43c0f9a needs to be reverted.
2) Dropping the CLK_IS_CRITICAL flag means we are now also controlling the codec mclk on CHT.
Just like setting the CLK_IS_CRITICAL flag on the mclk meant that we were effectively no longer controlling the mclk on BYT, reverting this change combined with your 7735bce05a9c ("ASoC: Intel: boards: use devm_clk_get() unconditionally") change means that we are now controlling the mclk on CHT platforms.
As you point out the firmware also controls the mclk on these platforms, so arguably this is undesirable. As I tried to explain I do not believe that this is actually a problem. But if you think this is a problem then we will need to revert 7735bce05a9c. If you prefer to go that route then I can prepare a patch for this.
3) No longer marking PMC clocks enabled at boot as CLK_IS_CRITICAL has caused a regression on Swanky and Clapper model chromebooks
You wrote: "I wonder if audio on Swanky worked before the commit 7735bce05a9c" I don't think that that commit has impacted Swanky at all since Swanky is BYT based. Instead I believe that d31fd43c0f9a ("clk: x86: Do not gate clocks enabled by the firmware") has accidentally fixed audio on Swanky laptops. This also explains why my "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit has broken it again as that is effectively a revert of d31fd43c0f9.
As I wrote before the as a result of d31fd43c0f9a the clk enable/disable behavior is not only ignored on CHT, as you noticed when writing 7735bce05a9c, but it also causes it to be ignored on BYT, including the initial disable of unused clocks just before the kernel starts /sbin/init. This not only fixed the ethernet on the laptop model that d31fd43c0f9a targeted but also caused us to no longer disable plt_clk_0 on Swanky (debugging done outside of this thread has shown that d31fd43c0f9a sets CLK_IS_CRITICAL on plt_clk_0).
TL;DR: d31fd43c0f9a ("clk: x86: Do not gate clocks enabled by the firmware") caused plt_clk_0 to be marked as CLK_IS_CRITICAL on Swanky, accidentally fixing audio. The proper fix for this is to make the cht_bsw_max98090_ti.c code control pmc_plt_clk_0 instead of pmc_plt_clk_3 as the Swanky board is actually using pmc_plt_clk_0 and since this is a BYT board we should be controlling the mclk there.
I hope this helps to clarify things, if not please keep asking questions.
Regards,
Hans
On 10/30/18 10:46 AM, Hans de Goede wrote:
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
On 10/30/18 9:38 AM, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi Dean,
Attached are 2 different attempts at fixing this.
When trying these patches do not forget to remove the revert of the "Stop-marking-clocks-as-CLK_IS_CRITICAL" commit.
Please first try the 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch patch I expect that one to do the trick indicating that the Swanky model uses a different pmc clk then which is normally used for the codec clock.
If that patch does not fix things, please give the other patch a try.
For Baytrail devices, the audio platform clocks are not managed by the firmware. They are for CHT-based devices - as can be seen by clock resources being described in the DSDT. We used to have a if(baytrail) in the code which was replaced by this CRITICAL label, but the point remains that there is a difference between the two SOC versions.
As I mentioned before the CRITICAL flag was only added a year ago to workaround an issue with on board ethernet needing plt_clk_4 on some laptops, this never had anything to do with sound.
see commit 7735bce05a9c ('ASoC: Intel: boards: use devm_clk_get() unconditionally')
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though,
Ok, so we need to have a DMI based quirk for the Swanky and maybe also the clapper to use plt_clk_0 there. Asking for 2 clks if we only need one does not seem like a good plan.
however you still have the problem of trying to manage from the kernel what the firmware already manages.
The firmware only manages it when going to D3 state, with ASoC most of the codecs gets turned off (and we no longer need the clock) but we do not put the device in D3 / execute the _PS3 method. So from a pm pov it is better if we manage the clk ourselves.
Once we do actually put the device in D3 (on suspend) the kernel will have already turned off the clk and the _OFF method of the CLK3 power resource which directly pokes mmio, will just set the enable bit to 0 when it already is 0, so no problem.
Regards,
Hans
Regards,
Hans
On 29-10-18 23:03, Pierre-Louis Bossart wrote:
On 10/29/18 2:08 PM, Dean Wallace wrote:
On 29-10-18, Andy Shevchenko wrote: > On Mon, Oct 29, 2018 at 7:52 PM Andy Shevchenko > andy.shevchenko@gmail.com wrote: >> Cc: Pierre as well. >> >> On Mon, Oct 29, 2018 at 7:48 PM Stephen Boyd sboyd@kernel.org >> wrote: >>> Quoting Dean Wallace (2018-10-25 16:25:17) >>>> I have found a regression in 4.18.15 that means I lose sound >>>> on my old >>>> Toshiba Chromebook 2 (Swanky). My system details are:- >>>> >>>> Toshiba Chromebook (Swanky) >>>> MrChromebox UEFI coreboot >>>> Arch Linux running latest alsa/pulseaudio >>>> >>>> Upgraded kernel from 4.18.14 to 4.18.15 and lost all sound >>>> output. By >>>> output I mean, the card is still detected, the module loaded, >>>> all apps >>>> showing sound is being playing, but no actual audible sound >>>> comes >>>> through. Upgraded to 4.18.16 same issue. >>>> >>>> Dug around and found Upstream commit >>>> 648e921888ad96ea3dc922739e96716ad3225d7f >>>> clk: x86: Stop marking clocks as CLK_IS_CRITICAL >>>> "This commit removes the CLK_IS_CRITICAL marking, fixing >>>> Cherry Trail >>>> devices not being able to reach S0i3 greatly decreasing their >>>> battery >>>> drain when suspended." >>>> >>>> I reverted it and compiled 4.18.16 and have sound back >>>> again. Could >>>> this be looked into, with possibility of fix. >>>> >>> Thanks for the bug report. I'm adding some people involved in >>> the commit >>> you mention is causing audio regressions. The best plan is to >>> probably >>> revert the commit from the 4.18 linux stable tree. Or there >>> may be >>> another patch missing that would be useful to make this >>> backported patch >>> work. Hopefully Hans or Andy knows. >> Hans has been investigating S0ix issues on Baytrail and >> Cherrytrail machines. >> I have a feeling that the problem can be fixed by properly >> handling >> clock in ASoC driver(s). Perhaps Hans and Pierre can figure >> this out >> better than me. > Looking to sound/soc/intel/boards/cht_bsw_rt5672.c I see no > suspend-resume hooks. Perhaps, adding them like in the commit > ac8bd9e13be2 ("r8169: Disable clk during suspend / resume") would > help.
I missed this change while i was away. It's indeed the expectation that the audio mclk is handled by the firmware,
I believe that the statement "It's indeed the expectation that the audio mclk is handled by the firmware" is not correct for BYT/CHT platforms (and the commit causing the regression only affects BYT/CHT platforms). Various machine drivers under sound/soc/intel/boards have code to deal with the mclk themselves, like this:
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3"); ...
if (SND_SOC_DAPM_EVENT_ON(event)) { ret = clk_prepare_enable(ctx->mclk); ... } else { clk_disable_unprepare(ctx->mclk); }
The above code is from sound/soc/intel/boards/cht_bsw_max98090_ti.c which is the machine driver used on the machines for which problems are now being reported.
And my commit introducing the problem is in essence a revert of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id...
Which is from 2017-07-14 and the ASoC code for BYT/CHT platforms is much older then that.
Also I asked Carlo why he wrote that patch and it was to fix a problem with ethernet on some laptops.
not sure I understand why removing the CLK_IS_CRITICAL was necessary or what it has to do with S0ix.
It is necessary, because if it is set the clock never gets disabled and on x86 platforms using suspend2idle we are responsible for *all* the hardware power-management as OS. If we do not disable the clocks then we can only reach S0i1 instead of S0i3 when suspended leading to increased battery drain during suspend.
Also note that this patch is only causing problems on CHT + max98090 codec using machines. I've tested it on many other BYT/CHT machines myself and we've not had any other bug reports related to this.
So this clearly points to a problem with the clock management in the cht_bsw_max98090_ti.c machine driver.
I've some ideas how to fix this and I will prepare some patches to test.
Regards,
Hans
Excellent work Hans. Compiled 4.19 with 0001-ASoC-intel-cht_bsw_max98090_ti-Use-pmc_plt_clk_0-ins.patch, sound works as before.
for i in /sys/kernel/debug/clk/pmc_plt_clk_?; do echo -n "$i: "; cat $i/clk_flags; echo; done /sys/kernel/debug/clk/pmc_plt_clk_0: /sys/kernel/debug/clk/pmc_plt_clk_1: /sys/kernel/debug/clk/pmc_plt_clk_2: /sys/kernel/debug/clk/pmc_plt_clk_3: /sys/kernel/debug/clk/pmc_plt_clk_4: /sys/kernel/debug/clk/pmc_plt_clk_5:
Regards
Hi,
On 30-10-18 16:46, Hans de Goede wrote:
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though,
Ok, so we need to have a DMI based quirk for the Swanky and maybe also the clapper to use plt_clk_0 there. Asking for 2 clks if we only need one does not seem like a good plan.
Dean, Mogens,
To write a proper patch for this I'm going to need DMI strings from your devices.
Can you please run (as normal user):
grep . /sys/class/dmi/id/* 2> /dev/null
And reply with the output of this command?
Thanks,
Hans
On 30-10-18, Hans de Goede wrote:
Hi,
On 30-10-18 16:46, Hans de Goede wrote:
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though,
Ok, so we need to have a DMI based quirk for the Swanky and maybe also the clapper to use plt_clk_0 there. Asking for 2 clks if we only need one does not seem like a good plan.
Dean, Mogens,
To write a proper patch for this I'm going to need DMI strings from your devices.
Can you please run (as normal user):
grep . /sys/class/dmi/id/* 2> /dev/null
And reply with the output of this command?
Here's mine, for a coreboot uefi based swanky.
https://ptpb.pw/~swanky-dmi-log
-- Dean
Hi,
On 30-10-18 17:15, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi,
On 30-10-18 16:46, Hans de Goede wrote:
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though,
Ok, so we need to have a DMI based quirk for the Swanky and maybe also the clapper to use plt_clk_0 there. Asking for 2 clks if we only need one does not seem like a good plan.
Dean, Mogens,
To write a proper patch for this I'm going to need DMI strings from your devices.
Can you please run (as normal user):
grep . /sys/class/dmi/id/* 2> /dev/null
And reply with the output of this command?
Here's mine, for a coreboot uefi based swanky.
Thanks, can you give the attached patch a try. This does the same as the previous one you tested, but then only on the Swanky.
Note I've added:
Reported-and-tested-by: Dean Wallace duffydack73@gmail.com
To the commit message, I hope that is ok with you, if not let me know and I will drop it.
Once you can confirm that this version also fixes things I will submit this version upstream.
Regards,
Hans
On 31-10-18, Hans de Goede wrote:
Thanks, can you give the attached patch a try. This does the same as the previous one you tested, but then only on the Swanky.
Note I've added:
Reported-and-tested-by: Dean Wallace duffydack73@gmail.com
To the commit message, I hope that is ok with you, if not let me know and I will drop it.
Once you can confirm that this version also fixes things I will submit this version upstream.
Regards,
Thanks, I don't mind at all. I can confirm 4.19 compiled with your quirk has working sound.
On 31-10-18, Hans de Goede wrote:
Hi,
On 30-10-18 17:15, Dean Wallace wrote:
On 30-10-18, Hans de Goede wrote:
Hi,
On 30-10-18 16:46, Hans de Goede wrote:
Hi,
On 30-10-18 16:04, Pierre-Louis Bossart wrote:
In addition I am not aware of any baytrail device using plt_clk_0, so moving a common machine driver such a cht_bsw_max98090_ti to use plt_clk0 only would break other devices (e.g. Rambi/Orco). Asking for both clocks to be on might work though,
Ok, so we need to have a DMI based quirk for the Swanky and maybe also the clapper to use plt_clk_0 there. Asking for 2 clks if we only need one does not seem like a good plan.
Dean, Mogens,
To write a proper patch for this I'm going to need DMI strings from your devices.
Can you please run (as normal user):
grep . /sys/class/dmi/id/* 2> /dev/null
And reply with the output of this command?
Here's mine, for a coreboot uefi based swanky.
Thanks, can you give the attached patch a try. This does the same as the previous one you tested, but then only on the Swanky.
Note I've added:
Reported-and-tested-by: Dean Wallace duffydack73@gmail.com
To the commit message, I hope that is ok with you, if not let me know and I will drop it.
Once you can confirm that this version also fixes things I will submit this version upstream.
Regards,
Hans
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
``` picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlocked ```
sound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
regards
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
On 31-10-18, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Hi Pierre. I'm not overly bothered by it, I lived with it for a long time when it was worse pre cht_bsw days. It's very hit and miss, like I have just done a few reboots and nothing is coming up in journal and switching to headphones works fine. Was quite a frequent issue back before it was switched to chtmax, but after that, for me anyway, it was all but nonexistant, apart from a very rare occassion once maybe twice. It's always been slightly unstable I guess, but seeing those messages in journal, something I look at frequently, and not seeing them for a long time, hmmm. I'll do some more tests with older kernel see how it compares and report back. Then again, I could just ignore it :) Regards
-Dean
On 31-10-18, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
Hi,
On 01-11-18 11:37, Dean Wallace wrote:
On 31-10-18, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
So you can no longer reproduce. Bummer. Note this might be caused by the temperature of the laptop when you were running the tests...
Anyways if you hit this again and you can reproduce it, please give adding a msleep(10) after code mucking with the clk a try.
Regards,
Hans
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 11:37, Dean Wallace wrote:
On 31-10-18, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
So you can no longer reproduce. Bummer. Note this might be caused by the temperature of the laptop when you were running the tests...
Anyways if you hit this again and you can reproduce it, please give adding a msleep(10) after code mucking with the clk a try.
Regards,
Hans
Right then, I can make it unlock and 'daleks' by going into pavucontrol and switching the Profile back and forth from Stereo Output to Stereo Output+Analog Mono Input, which is actually something I've done to make it correct itself as well. I don't use the mic or anything so I've had it set to Stereo Ouput only which I 'think' has somehow made it more stable for me. With all my playing around, one of the things I did was clean out my .config/pulse folder which meant by default the 'Profile' in pavucontrol was set to Output+Input, which seems to help trigger the PLL issue when inserting headphones.
So what would you like me to do, as I can trigger it on demand it seems.
Hi,
On 01-11-18 15:28, Dean Wallace wrote:
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 11:37, Dean Wallace wrote:
On 31-10-18, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
So you can no longer reproduce. Bummer. Note this might be caused by the temperature of the laptop when you were running the tests...
Anyways if you hit this again and you can reproduce it, please give adding a msleep(10) after code mucking with the clk a try.
Regards,
Hans
Right then, I can make it unlock and 'daleks' by going into pavucontrol and switching the Profile back and forth from Stereo Output to Stereo Output+Analog Mono Input, which is actually something I've done to make it correct itself as well. I don't use the mic or anything so I've had it set to Stereo Ouput only which I 'think' has somehow made it more stable for me. With all my playing around, one of the things I did was clean out my .config/pulse folder which meant by default the 'Profile' in pavucontrol was set to Output+Input, which seems to help trigger the PLL issue when inserting headphones.
So what would you like me to do, as I can trigger it on demand it seems.
Please give the attached patch a try (on top of my patch for the clk quirk) and let us know if that fixes these errors.
Regards,
Hans
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 15:28, Dean Wallace wrote:
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 11:37, Dean Wallace wrote:
On 31-10-18, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
So you can no longer reproduce. Bummer. Note this might be caused by the temperature of the laptop when you were running the tests...
Anyways if you hit this again and you can reproduce it, please give adding a msleep(10) after code mucking with the clk a try.
Regards,
Hans
Right then, I can make it unlock and 'daleks' by going into pavucontrol and switching the Profile back and forth from Stereo Output to Stereo Output+Analog Mono Input, which is actually something I've done to make it correct itself as well. I don't use the mic or anything so I've had it set to Stereo Ouput only which I 'think' has somehow made it more stable for me. With all my playing around, one of the things I did was clean out my .config/pulse folder which meant by default the 'Profile' in pavucontrol was set to Output+Input, which seems to help trigger the PLL issue when inserting headphones.
So what would you like me to do, as I can trigger it on demand it seems.
Please give the attached patch a try (on top of my patch for the clk quirk) and let us know if that fixes these errors.
Regards,
Hans
Ok Hans, I tried your patch on top of the quirk, and sound is immediately garbled (daleks again), and switching pulseuadio profiles or inserting headphones doesn't make it correct itself, whereas before sound would be fine, UNTIL, I did something like insert earphone etc.
-Dean
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 15:28, Dean Wallace wrote:
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 11:37, Dean Wallace wrote:
On 31-10-18, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
So you can no longer reproduce. Bummer. Note this might be caused by the temperature of the laptop when you were running the tests...
Anyways if you hit this again and you can reproduce it, please give adding a msleep(10) after code mucking with the clk a try.
Regards,
Hans
Right then, I can make it unlock and 'daleks' by going into pavucontrol and switching the Profile back and forth from Stereo Output to Stereo Output+Analog Mono Input, which is actually something I've done to make it correct itself as well. I don't use the mic or anything so I've had it set to Stereo Ouput only which I 'think' has somehow made it more stable for me. With all my playing around, one of the things I did was clean out my .config/pulse folder which meant by default the 'Profile' in pavucontrol was set to Output+Input, which seems to help trigger the PLL issue when inserting headphones.
So what would you like me to do, as I can trigger it on demand it seems.
Please give the attached patch a try (on top of my patch for the clk quirk) and let us know if that fixes these errors.
Regards,
Hans
This is weird, now sound is ok on boot, until I plug in earphones, like before. Still get spammed with errors until I fool around with pulse profile switching (it's hit and miss).
-Dean
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 15:28, Dean Wallace wrote:
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 11:37, Dean Wallace wrote:
On 31-10-18, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
So you can no longer reproduce. Bummer. Note this might be caused by the temperature of the laptop when you were running the tests...
Anyways if you hit this again and you can reproduce it, please give adding a msleep(10) after code mucking with the clk a try.
Regards,
Hans
Right then, I can make it unlock and 'daleks' by going into pavucontrol and switching the Profile back and forth from Stereo Output to Stereo Output+Analog Mono Input, which is actually something I've done to make it correct itself as well. I don't use the mic or anything so I've had it set to Stereo Ouput only which I 'think' has somehow made it more stable for me. With all my playing around, one of the things I did was clean out my .config/pulse folder which meant by default the 'Profile' in pavucontrol was set to Output+Input, which seems to help trigger the PLL issue when inserting headphones.
So what would you like me to do, as I can trigger it on demand it seems.
Please give the attached patch a try (on top of my patch for the clk quirk) and let us know if that fixes these errors.
Regards,
Hans
Sorry, it's not being consistent with me Now, fresh boot again, no errors in journal, no errors while plugging/unplugging earphones. I think it's definitely more stable with the profile in pavucontrol set to Output only (no + Input), but I can still trigger it by switching it, and it never corrects itself until I switch a few more times.
-Dean
Hi,
On 01-11-18 16:50, Dean Wallace wrote:
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 15:28, Dean Wallace wrote:
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 11:37, Dean Wallace wrote:
On 31-10-18, Pierre-Louis Bossart wrote:
> Just thought it worth mentioning, this new patch that fixes sound > again, seems to have ressurected an old issue with PLL unlock. I'm > seeing journal entries after fresh boot ...... > > ``` > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard systemd[462]: Started Sound Service. > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > picard kernel: max98090_pll_work: 141 callbacks suppressed > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > ``` > > sound is ok, but sometimes plugging in headphones spams journal with > those PLL messages, and sound turns into "daleks", and I have to > remove/insert headphones few times or stop/start audio to fix it. > It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error? Are you 100% sure this was not present in the previous install you used (4.18.14 as mentioned earlier in the thread)?
Thanks
-Pierre
Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
So you can no longer reproduce. Bummer. Note this might be caused by the temperature of the laptop when you were running the tests...
Anyways if you hit this again and you can reproduce it, please give adding a msleep(10) after code mucking with the clk a try.
Regards,
Hans
Right then, I can make it unlock and 'daleks' by going into pavucontrol and switching the Profile back and forth from Stereo Output to Stereo Output+Analog Mono Input, which is actually something I've done to make it correct itself as well. I don't use the mic or anything so I've had it set to Stereo Ouput only which I 'think' has somehow made it more stable for me. With all my playing around, one of the things I did was clean out my .config/pulse folder which meant by default the 'Profile' in pavucontrol was set to Output+Input, which seems to help trigger the PLL issue when inserting headphones.
So what would you like me to do, as I can trigger it on demand it seems.
Please give the attached patch a try (on top of my patch for the clk quirk) and let us know if that fixes these errors.
Regards,
Hans
Sorry, it's not being consistent with me Now, fresh boot again, no errors in journal, no errors while plugging/unplugging earphones. I think it's definitely more stable with the profile in pavucontrol set to Output only (no + Input), but I can still trigger it by switching it, and it never corrects itself until I switch a few more times.
Ok, so there are 2 issues here as I see it:
1) It does not always reproduce 2) It does still reproduce sometimes with the patch, so the patch does not fix it.
So I think we just have to live with this for now.
Regards,
Hans
On 02-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 16:50, Dean Wallace wrote:
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 15:28, Dean Wallace wrote:
On 01-11-18, Hans de Goede wrote:
Hi,
On 01-11-18 11:37, Dean Wallace wrote:
On 31-10-18, Pierre-Louis Bossart wrote: > > > Just thought it worth mentioning, this new patch that fixes sound > > again, seems to have ressurected an old issue with PLL unlock. I'm > > seeing journal entries after fresh boot ...... > > > > ``` > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard systemd[462]: Started Sound Service. > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > picard kernel: max98090_pll_work: 141 callbacks suppressed > > picard kernel: max98090 i2c-193C9890:00: PLL unlocked > > ``` > > > > sound is ok, but sometimes plugging in headphones spams journal with > > those PLL messages, and sound turns into "daleks", and I have to > > remove/insert headphones few times or stop/start audio to fix it. > > It's a very old issue, maybe you'd know more about it. > > I noticed this error on my Orco device used for tests many moons ago, but I > could never find out what led to this error case, it wasn't deterministic > and didn't impact the audio quality. All I could do is rate_limit it... If > we have an A vs. B situation it'd be really helpful to diagnose further. > > Is there really a causality between the changes from Hans and this PLL > unlock error? Are you 100% sure this was not present in the previous install > you used (4.18.14 as mentioned earlier in the thread)? > > Thanks > > -Pierre > Well, numerous boots, kernels, headphone inserting - no PLL or 'Daleks'. My laptop must have been haunted that day (halloween). I'll put it to bed.
So you can no longer reproduce. Bummer. Note this might be caused by the temperature of the laptop when you were running the tests...
Anyways if you hit this again and you can reproduce it, please give adding a msleep(10) after code mucking with the clk a try.
Regards,
Hans
Right then, I can make it unlock and 'daleks' by going into pavucontrol and switching the Profile back and forth from Stereo Output to Stereo Output+Analog Mono Input, which is actually something I've done to make it correct itself as well. I don't use the mic or anything so I've had it set to Stereo Ouput only which I 'think' has somehow made it more stable for me. With all my playing around, one of the things I did was clean out my .config/pulse folder which meant by default the 'Profile' in pavucontrol was set to Output+Input, which seems to help trigger the PLL issue when inserting headphones.
So what would you like me to do, as I can trigger it on demand it seems.
Please give the attached patch a try (on top of my patch for the clk quirk) and let us know if that fixes these errors.
Regards,
Hans
Sorry, it's not being consistent with me Now, fresh boot again, no errors in journal, no errors while plugging/unplugging earphones. I think it's definitely more stable with the profile in pavucontrol set to Output only (no + Input), but I can still trigger it by switching it, and it never corrects itself until I switch a few more times.
Ok, so there are 2 issues here as I see it:
- It does not always reproduce
- It does still reproduce sometimes with the patch, so the patch does not
fix it.
So I think we just have to live with this for now.
Regards,
Hans
Yeah, I've never known it act any different so I'm used to it. I do know a few people in the past who have just bought a usb sound device because they want to use a mic as well, and that just didn't work quite right for them, even if sound output worked they had issues as soon as they tried using mic. I forget which, 4.1 or 4.4, 1 guy mentioned still using it just because output/input worked flawlessly. ¯_(ツ)_/¯
-Dean
Hi,
On 31-10-18 23:27, Pierre-Louis Bossart wrote:
Just thought it worth mentioning, this new patch that fixes sound again, seems to have ressurected an old issue with PLL unlock. I'm seeing journal entries after fresh boot ......
picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard systemd[462]: Started Sound Service. picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090 i2c-193C9890:00: PLL unlocked picard kernel: max98090_pll_work: 141 callbacks suppressed picard kernel: max98090 i2c-193C9890:00: PLL unlockedsound is ok, but sometimes plugging in headphones spams journal with those PLL messages, and sound turns into "daleks", and I have to remove/insert headphones few times or stop/start audio to fix it. It's a very old issue, maybe you'd know more about it.
I noticed this error on my Orco device used for tests many moons ago, but I could never find out what led to this error case, it wasn't deterministic and didn't impact the audio quality. All I could do is rate_limit it... If we have an A vs. B situation it'd be really helpful to diagnose further.
Is there really a causality between the changes from Hans and this PLL unlock error?
That is actually not unlikely. We have the mclk as the source clk for the PLL and if we enable/disable it and/or change the src-clk then the PLL will loose its lock and it needs some time to re-lock.
So these errors indicate that we need to either:
1) Sleep a bit after enabling the mclk; (and/or after changing the PLL src clk I've not checked if this driver does this) 2) Or poll the PLL locked bit after enabling the mclk until it indicate it has locked (and/or after changing the PLL src clk)
I've had to do similar things in u-boot when turning on PLL-s for DRAM and stuff, so this sounds familiar.
Dean since you have the hardware, this is probably easiest for you to fix (since you can reproduce the issue) do you feel up to giving fixing this a shot? I think it is fine if you just go with the simple solution of adding a msleep(xx) at the right place. I would expect 10 (ms) to do the trick. If that works you may also try to give 5ms a shot.
Regards,
Hans
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Dean Wallace -
Hans de Goede -
Pierre-Louis Bossart -
Stephen Boyd