Re: [PATCH] PM: runtime: Fix unpaired parent child_count for force_resume

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

On 05/05/2021 14:09, Tony Lindgren wrote:
> As pm_runtime_need_not_resume() relies also on usage_count, it can return
> a different value in pm_runtime_force_suspend() compared to when called in
> pm_runtime_force_resume(). Different return values can happen if anything
> calls PM runtime functions in between, and causes the parent child_count
> to increase on every resume.
> 
> So far I've seen the issue only for omapdrm that does complicated things
> with PM runtime calls during system suspend for legacy reasons:
> 
> omap_atomic_commit_tail() for omapdrm.0
>   dispc_runtime_get()
>    wakes up 58000000.dss as it's the dispc parent
>     dispc_runtime_resume()
>      rpm_resume() increases parent child_count
>   dispc_runtime_put() won't idle, PM runtime suspend blocked
> pm_runtime_force_suspend() for 58000000.dss, !pm_runtime_need_not_resume()
>   __update_runtime_status()
> system suspended
> pm_runtime_force_resume() for 58000000.dss, pm_runtime_need_not_resume()
>   pm_runtime_enable() only called because of pm_runtime_need_not_resume()
> omap_atomic_commit_tail() for omapdrm.0
>   dispc_runtime_get()
>    wakes up 58000000.dss as it's the dispc parent
>     dispc_runtime_resume()
>      rpm_resume() increases parent child_count
>   dispc_runtime_put() won't idle, PM runtime suspend blocked
> ...
> rpm_suspend for 58000000.dss but parent child_count is now unbalanced
> 
> Let's fix the issue by adding a flag for needs_force_resume and use it in
> pm_runtime_force_resume() instead of pm_runtime_need_not_resume().
> 
> Additionally omapdrm system suspend could be simplified later on to avoid
> lots of unnecessary PM runtime calls and the complexity it adds. The
> driver can just use internal functions that are shared between the PM
> runtime and system suspend related functions.
> 
> Fixes: 4918e1f87c5f ("PM / runtime: Rework pm_runtime_force_suspend/resume()")
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>   drivers/base/power/runtime.c | 10 +++++++---
>   include/linux/pm.h           |  1 +
>   2 files changed, 8 insertions(+), 3 deletions(-)

Tested on DRA76 EVM, with and without HDMI plugged in. I can see DSS 
shutting down properly by looking at the CM_DSS_DSS_CLKCTRL register.

Tested-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi