[Intel-gfx] [PATCH] drm/i915/display: Fix warning callstack for imbalance wakeref

[Imre Deak]

On Fri, Aug 12, 2022 at 10:17:24AM +0530, Mitul Golani wrote:
> While executing i915_selftest, wakeref imbalance warning is seen
> with i915_selftest failure.
> 
> When device is already suspended, wakeref is acquired by
> disable_rpm_wakeref_asserts and rpm ownership is transferred back
> to core. During this case wakeref_count will not be zero.
> Once driver is unregistered, this wakeref is released with
> enable_rpm_wakeref_asserts and balancing wakeref_count acquired
> by driver.
> 
> This patch will fix the warning callstack by adding check if device
> is already suspended and rpm ownership transfer is going on.
> 
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index deb8a8b76965..6530a8680cfd 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1670,7 +1670,13 @@ static int intel_runtime_resume(struct device *kdev)
>  
>  	drm_dbg(&dev_priv->drm, "Resuming device\n");
>  
> -	drm_WARN_ON_ONCE(&dev_priv->drm, atomic_read(&rpm->wakeref_count));
> +	/*
> +	 * When device is already suspended, Wakeref is acquired by disable_rpm_wakeref_asserts
> +	 * and rpm ownership is transferred back to core. During this case wakeref_count will
> +	 * not be zero. Once driver is unregistered, this wakeref is released with
> +	 * enable_rpm_wakeref_asserts and balancing wakeref_count acquired by driver.
> +	 */
> +	drm_WARN_ON_ONCE(&dev_priv->drm, atomic_read(&rpm->wakeref_count) && !rpm->suspended);

I can't see how disable/enable_rpm_wakeref_asserts() can lead to this
WARN. They are always called in pairs both in intel_runtime_suspend()
and intel_runtime_resume(), leaving rpm->wakeref_count unchanged.

The root cause is probably somewhere else, incrementing
rpm->wakeref_count without runtime resuming the device.

The WARN() condition is corret, we shouldn't get here with a non-zero
wakeref_count. rpm->suspended - set in intel_runtime_suspend() and
cleared in intel_runtime_resume() - should be always false here, so the
above change would just disable the WARN in all cases.

>  	disable_rpm_wakeref_asserts(rpm);
>  
>  	intel_opregion_notify_adapter(dev_priv, PCI_D0);
> -- 
> 2.25.1
>