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
