Re: [PATCH] drm/i915: Balance assert_rpm_wakelock_held() for !IS_ENABLED(CONFIG_PM)

[Imre Deak <imre.deak@intel.com>]

On Thu, 2016-02-25 at 21:10 +0000, Chris Wilson wrote:
> commit 09731280028ce03e6a27e1998137f1775a2839f3
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Wed Feb 17 14:17:42 2016 +0200
> 
>     drm/i915: Add helper to get a display power ref if it was already
> enabled
> 
> left the rpm wakelock assertions unbalanced if CONFIG_PM was disabled
> as
> intel_runtime_pm_get_if_in_use() would return true without
> incrementing
> the local bookkeeping required for the assertions.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Mika Kuoppala <mika.kuoppala@intel.com>
> CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>

Arg, I broke this in v3. Thanks for catching it:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 26 ++++++++++++-----------
> ---
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index e2329768902c..4172e73212cd 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2365,22 +2365,20 @@ bool intel_runtime_pm_get_if_in_use(struct
> drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  	struct device *device = &dev->pdev->dev;
> -	int ret;
>  
> -	if (!IS_ENABLED(CONFIG_PM))
> -		return true;
> +	if (IS_ENABLED(CONFIG_PM)) {
> +		int ret = pm_runtime_get_if_in_use(device);
>  
> -	ret = pm_runtime_get_if_in_use(device);
> -
> -	/*
> -	 * In cases runtime PM is disabled by the RPM core and we
> get an
> -	 * -EINVAL return value we are not supposed to call this
> function,
> -	 * since the power state is undefined. This applies atm to
> the
> -	 * late/early system suspend/resume handlers.
> -	 */
> -	WARN_ON_ONCE(ret < 0);
> -	if (ret <= 0)
> -		return false;
> +		/*
> +		 * In cases runtime PM is disabled by the RPM core
> and we get
> +		 * an -EINVAL return value we are not supposed to
> call this
> +		 * function, since the power state is undefined.
> This applies
> +		 * atm to the late/early system suspend/resume
> handlers.
> +		 */
> +		WARN_ON_ONCE(ret < 0);
> +		if (ret <= 0)
> +			return false;
> +	}
>  
>  	atomic_inc(&dev_priv->pm.wakeref_count);
>  	assert_rpm_wakelock_held(dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx