Repository navigation
Conversation
Runtime PM was being initialized by PCI/ACPI/SPI driver probe AND after topology after topology loading. Consolidate this init to be in one place so that FW boot or topology load failures to not invoke any subsequent runtime PM actions and leave DSP in default D0 state. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
|
@ranj063 can you take a look here since you have been working on PM lately, I will be test driving this myself but my priority is ABI testing today. Thanks. |
plbossart
left a comment
plbossart
left a comment
There was a problem hiding this comment.
I don't think this is correct. You have a parent-child relationship and need to enable runtime_pm at the parent level and then at the children levels individually. Each of those devices have a state.
|
@plbossart @lgirdwood this looks OK but let me test it on GLK and get back with results. |
|
@plbossart I dont this this was respected in the driver as the parent would try and suspend the child even if the child said no. I assume you mean the pci->dev as parent ? which should == sdev->dev, our child devices would be ASoC component devices and these would only care about kcontrols and PCMs as PM resume events. |
|
Checking sdev->dev is child, so current code was registering both with PM which ignored any dependency. It seems like the so dependency needs fixed to ensure parent doesn't change state if child does not register. @ranj063 any clues from your recent work ? |
|
@lgirdwood i just tested this on the GLK but with this change the device doesnt enter runtime_suspend at all. |
| * runtime pm for pci device is "forbidden" by default. | ||
| * so call pm_runtime_allow() to enable it. | ||
| */ | ||
| pm_runtime_allow(sdev->dev); |
There was a problem hiding this comment.
@lgirdwood the problem is here I think. I think this should be pm_runtime_allow(&sdev->pci->dev)
There was a problem hiding this comment.
but if this is a common routine it needs to deal with both the PCI and non-PCI cases, can't hard-code as suggested.
There was a problem hiding this comment.
@plbossart agree. I only tested for the pci case. We will need to rework this to address all cases
|
|
||
| /* follow recommendation in pci-driver.c to decrement usage counter */ | ||
| if (sdev->pci) | ||
| pm_runtime_put_noidle(sdev->dev); |
There was a problem hiding this comment.
@lgirdwood and here. this should also be pm_runtime_put_nodle(&sdev->pci->dev).
I've just tested it with these 2 changes and things look good.
There was a problem hiding this comment.
@lgirdwood yes that was my point, you need to be careful on which device you interact with. The common routine only handled sdev->dev.
There was a problem hiding this comment.
@ranj063 probably want to be sdev->parent->dev instead of sdev->pci->dev here as this will cover ACPI, SPI devices etc. Can you send a new PR ? and ping @mengdonglin to have it tested on other devices tomorrow to make sure it's good.
There was a problem hiding this comment.
@ranj063 Now we observed PM failure on APL thesofproject/sof#611 and thesofproject/sof#613
Once you've a new PR, we can test it on APL and GLK.
The current selftest sk_bypass_prot_mem only supports 4K page. When running with 64K page on arm64, the following failure happens: ... check_bypass:FAIL:no bypass unexpected no bypass: actual 3 <= expected 32 ... thesofproject#385/1 sk_bypass_prot_mem/TCP :FAIL ... check_bypass:FAIL:no bypass unexpected no bypass: actual 4 <= expected 32 ... thesofproject#385/2 sk_bypass_prot_mem/UDP :FAIL ... Adding support to 64K page as well fixed the failure. Cc: Kuniyuki Iwashima <kuniyu@google.com> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> Link: https://lore.kernel.org/r/20260113061028.3798326-1-yonghong.song@linux.dev Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Runtime PM was being initialized by PCI/ACPI/SPI driver probe AND after
topology after topology loading. Consolidate this init to be in one
place so that FW boot or topology load failures to not invoke any
subsequent runtime PM actions and leave DSP in default D0 state.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com