Message-ID: <d4cafb3f-2045-40c1-a8fd-58dd46485232@linux.intel.com>
Date: Wed, 30 Apr 2025 08:59:27 +0800
From: "Liao, Bard" <yung-chuan.liao@...ux.intel.com>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>, Vinod Koul <vkoul@...nel.org>,
 Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>
Cc: LKML <linux-kernel@...r.kernel.org>, Linux PM <linux-pm@...r.kernel.org>,
 Sanyog Kale <sanyog.r.kale@...el.com>, linux-sound@...r.kernel.org
Subject: Re: [PATCH v2] soundwire: intel_auxdevice: Fix system suspend/resume
 handling



On 4/29/2025 3:50 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> Before commit bca84a7b93fd ("PM: sleep: Use DPM_FLAG_SMART_SUSPEND
> conditionally") the runtime PM status of the device in intel_resume()
> had always been RPM_ACTIVE because setting DPM_FLAG_SMART_SUSPEND had
> caused the core to call pm_runtime_set_active() for that device during
> the "noirq" resume phase.  For this reason, the pm_runtime_suspended()
> check in intel_resume() had never triggered and the code depending on
> it had never run.  That had not caused any observable functional issues
> to appear, so effectively the code in question had never been needed.
> 
> After commit bca84a7b93fd the core does not call pm_runtime_set_active()
> for all devices with DPM_FLAG_SMART_SUSPEND set any more and the code
> depending on the pm_runtime_suspended() check in intel_resume() runs if
> the device is runtime-suspended prior to a system-wide suspend
> transition.  Unfortunately, when it runs, it breaks things due to the
> attempt to runtime-resume bus->dev which most likely is not ready for a
> runtime resume at that point.
> 
> It also does other more-or-less questionable things.  Namely, it
> calls pm_runtime_idle() for a device with a nonzero runtime PM usage
> counter which has no effect (all devices have nonzero runtime PM
> usage counters during system-wide suspend and resume).  It also calls
> pm_runtime_mark_last_busy() for the device even though devices cannot
> runtime-suspend during system-wide suspend and resume (because their
> runtime PM usage counters are nonzero) and an analogous call is made
> in the same function later.  Moreover, it sets the runtime PM status
> of the device to RPM_ACTIVE before activating it.
> 
> For the reasons listed above, remove that code altogether.
> 
> On top of that, add a pm_runtime_disable() call to intel_suspend() to
> prevent the device from being runtime-resumed at any point after
> intel_suspend() has started to manipulate it because the changes
> made by that function would be undone by a runtime-suspend of the
> device.
> 
> Next, once runtime PM has been disabled, the runtime PM status of the
> device cannot change, so pm_runtime_status_suspended() can be used
> instead of pm_runtime_suspended() in intel_suspend().
> 
> Finally, make intel_resume() call pm_runtime_set_active() at the end to
> set the runtime PM status of the device to "active" because it has just
> been activated and re-enable runtime PM for it after that.
> 
> Additionally, drop the setting of DPM_FLAG_SMART_SUSPEND from the
> driver because it has no effect on devices handled by it.
> 
> Fixes: bca84a7b93fd ("PM: sleep: Use DPM_FLAG_SMART_SUSPEND conditionally")
> Reported-by: Bard Liao <yung-chuan.liao@...ux.intel.com>
> Tested-by: Bard Liao <yung-chuan.liao@...ux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> 
> v1 -> v2: New changelog
> 
> Since it fixes a recent regression in 6.15-rc, I can route it through the
> PM tree unless that would be a major concern.
> 

 Acked-by: Bard Liao <yung-chuan.liao@...ux.intel.com>

Powered by blists - more mailing lists