Hi,
On 12/13/22 14:03, Mark Brown wrote:
On Tue, Dec 13, 2022 at 01:55:50PM +0100, Cezary Rojewski wrote:
From i2s (non-sdw) Realtek codec drivers found in sound/soc/codecs it seems that only rt9120.c defines PM ops and configures the PM for the device. OTOH, there are several which define suspend/resume on ASoC component level
Thus, voting for a complete revert.
Note that with ACPI you can have runtime PM things happening at the ACPI level so even if a driver does nothing when suspended there can be some benefit from using runtime PM. No idea if this applies to any systems using these devices or not though.
Yes I was thinking the same thing, I said in my other email that I was fine with a revert because in my experience the codec ACPI devices don't have a _PS0 / _PS3 method.
But to make sure I just checked and I noticed this is not entirely true.
E.g. on the ThinkPad 10 tablet there is:
Method (_PS3, 0, NotSerialized) // _PS3: Power State 3 { CKC3 = Zero }
Method (_PS0, 0, NotSerialized) // _PS0: Power State 0 { CKC3 = One }
However the "board" driver already directly controls this clock, including switching to an internal RC oscillator for jack detection when idle, see:
sound/soc/intel/boards/cht_bsw_rt5672.c:
ret = clk_prepare_enable(ctx->mclk);
clk_disable_unprepare(ctx->mclk);
drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
So also having runtime-pm muck with this is not really useful and I guess might actually cause problems in some cases (at least it is one more thing which can go wrong).
Note _PS0 and _PS3 will still run on system suspend/resume but by then we should have already powered-down the codec, including calling clk_disable_unprepare(ctx->mclk); .
TL;DR: I'm still fine with reverting the original commit.
Regards,
Hans