Re: Regression apparently caused by commit 088a40980efbc2c449b72f0f2c7ebd82f71d08e2 "ASoC: amd: acp: add pm ops support for acp pci driver"

From: syed saba kareem
Date: Fri Nov 03 2023 - 09:31:17 EST



On 11/3/23 04:41, Bagas Sanjaya wrote:
On Thu, Aug 03, 2023 at 10:22:07PM +0300, Marian Postevca wrote:
I'm trying to develop a sound machine driver based on the acp legacy driver.
The first version of the driver was sent for review on the alsa mailing list this
spring: https://lore.kernel.org/all/20230320203519.20137-1-posteuca@xxxxxxxxx

I'm trying to fix some of the issues that were brought up during the review back then,
but when I ported the patches to the latest commit on the for-next
branch, I noticed a regression where I couldn't hear any sound at all.

So I started a bisect session and found that the first bad commit is:
ASoC: amd: acp: add pm ops support for acp pci driver
commit 088a40980efbc2c449b72f0f2c7ebd82f71d08e2
https://lore.kernel.org/lkml/20230622152406.3709231-11-Syed.SabaKareem@xxxxxxx

If I revert this commit sound works as expected. So I started tinkering a little bit
with it and I believe that what happens is that the acp pci driver
enters the autosuspend state and never leaves this state at all.
I noticed this because if I increase the autosuspend delay to a much
larger value, then the sound works until that delay passes.
I added traces and I can see that when the delay expires the suspend callback snd_acp_suspend()
gets called, but the resume callback snd_acp_resume() never gets called.

I'm no expert in runtime power management (though I did read a bit on it), so I don't understand
all the things that happen underneath, but one thing that is not clear to me is who's supposed
to mark activity on this device and keep it from entering autosuspend if the user wants to play
some sound? Shouldn't there be some counterpart that calls pm_runtime_mark_last_busy() ?
I looked through the code and can't find who's calling pm_runtime_mark_last_busy().

Some help here would be welcome. Is there something missing in my machine driver code, or
is the runtime pm handling in acp pci driver wrong?
Thanks for the regression report. I'm adding it to regzbot:

#regzbot ^introduced: 088a40980efbc2

We were working on some other priority tasks, will upstream the changes

by next week.